Skip to content

Parse and generate YAML in sync_back.ts#3556

Open
mbg wants to merge 7 commits intomainfrom
mbg/ts/sync-back-parse
Open

Parse and generate YAML in sync_back.ts#3556
mbg wants to merge 7 commits intomainfrom
mbg/ts/sync-back-parse

Conversation

@mbg
Copy link
Member

@mbg mbg commented Mar 7, 2026

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:

  • Parse the workflows in .github/workflows to extract the action versions
  • Then parses, updates, and renders the workflow templates in pr-checks/checks

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:

  • I looked into running eslint over the resulting action-versions.ts file, but this is currently sort-of blocked on me adding a tsconfig.json for pr-checks and then an eslint config. I'll do that, but separately from this.

Risk assessment

For internal use only. Please select the risk level of this change:

  • Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Which use cases does this change impact?

Environments:

  • Testing/None - This change does not impact any CodeQL workflows in production.

How did/will you validate this change?

  • Unit tests - I am depending on unit test coverage (i.e. tests in .test.ts files).
  • End-to-end tests - I am depending on PR checks (i.e. tests in pr-checks).

If something goes wrong after this change is released, what are the mitigation and rollback strategies?

  • Development/testing only - This change cannot cause any failures in production.

How will you know if something goes wrong after this change is released?

  • Other - Please provide details.

Are there any special considerations for merging or releasing this change?

  • No special considerations - This change can be merged at any time.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Consider adding a changelog entry for this change.
  • Confirm the readme and docs have been updated if necessary.

@mbg mbg requested a review from esbena March 7, 2026 00:34
@mbg mbg self-assigned this Mar 7, 2026
@github-actions github-actions bot added the size/L May be hard to review label Mar 7, 2026
@mbg mbg marked this pull request as ready for review March 9, 2026 13:19
@mbg mbg requested a review from a team as a code owner March 9, 2026 13:19
Copilot AI review requested due to automatic review settings March 9, 2026 13:19
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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:

  • Parse .github/workflows/__*.yml with yaml to extract uses: action refs (including inline comments) in sync_back.ts.
  • Generate/update pr-checks/action-versions.ts and update sync.ts to reference it via a helper (getActionRef).
  • Update pr-checks/checks/*.yml templates via YAML AST editing rather than regex substitution.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
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.

@mbg mbg force-pushed the mbg/ts/sync-back-parse branch from 818a4b4 to 25599b3 Compare March 9, 2026 14:29
Copy link
Contributor

@esbena esbena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +63 to +64
pair.key.value === "uses" &&
typeof pair.value.value === "string"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/L May be hard to review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants