Sorry, something went wrong.
There was a problem hiding this comment.
This PR refactors pr-checks/sync_back.ts to avoid regex-based workflow parsing by using the existing yaml library to parse generated workflows and update templates, and it factors Action version pinning into a dedicated action-versions.ts file consumed by sync.ts.
Changes:
Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file| pr-checks/sync_back.ts | Switches to YAML AST traversal for scanning/updating and introduces writing action-versions.ts. |
| pr-checks/sync_back.test.ts | Updates tests to validate YAML-based scanning and the new action-versions.ts update behavior. |
| pr-checks/sync.ts | Loads action version pins from action-versions.ts instead of hardcoded refs. |
| pr-checks/action-versions.ts | New generated mapping of action names to pinned refs (and optional comments). |
| pr-checks/checks/overlay-init-fallback.yml | Template formatting change as a result of re-serialization. |
| pr-checks/checks/init-with-registries.yml | Template formatting change as a result of re-serialization. |
| pr-checks/checks/autobuild-direct-tracing-with-working-dir.yml | Template formatting change as a result of re-serialization. |
Sorry, something went wrong.
There was a problem hiding this comment.
I love this approach. Thanks for this extra follow-up effort.
In addition to the yaml traversal comment, I have one comment that we'll discuss internally.
Sorry, something went wrong.
| pair.key.value === "uses" && | ||
| typeof pair.value.value === "string" |
There was a problem hiding this comment.
So the idea of this visitor is to recursively visit everything in the YAML document and then for any uses: <string> cases do something?
My worry is that this could hit too many things. Concretely a user could have env: uses: "foo"
I would prefer a setup where we specifically target the individual steps in a more semantic manner:
We know the exact object depth at which these step definitions can occur, both for workflow files and composite actions...
Sorry, something went wrong.
This is an alternative to #3547 which avoids the regular expressions entirely. It does this by using the yaml library that we already use in sync.ts to:
For sync.ts, I have extracted a mapping of action names to versions into a separate file (action-versions.ts) which can easily be re-generated by the sync_back.ts script.
A couple of extra notes for review:
Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Environments:
How did/will you validate this change?
If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist