Skip to content

fix(root): publish validated iyarc-prune pull requests - #10087

Merged
zahin-mohammad merged 2 commits into
masterfrom
codex/WAL-2232-iyarc-prune-create-pr
Nov 3, 2026
Merged

zahin-mohammad merged 2 commits into
masterfrom
codex/WAL-2232-iyarc-prune-create-pr

Conversation

@zahin-mohammad

@zahin-mohammad zahin-mohammad commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Problem

The weekly dependency-exclusion maintenance job could finish with validated edits but no pull request because the agent treated a checkout on master as a local run. A feature-branch manual dispatch also failed before the agent ran: the Amazon Web Services (AWS) role checks the workflow's branch identity, and checking out master does not change that identity. Failure investigation.

Fix

  • The workflow owns signed commits and non-draft pull-request publication; the agent leaves validated edits uncommitted and writes a report outside the repository.
  • Reject non-master dispatches early. Preserve the exact validated parent and file tree instead of letting publication rebase onto a newer master.
  • Check working-tree, staged, and untracked paths. Allow only the exclusion file, root manifests/lockfile, and direct workspace manifests; reject nested test/example manifests and hidden staged source edits.
  • Install the checksum-pinned scanner before the agent runs, and use the release severity gate instead of unsupported scanner flags.

Original defect evidence and verification.

Review map

The history has two commits: 0decbbf1d3 contains the publication fix; 03788c5302 extracts the helper and clarifies the workflow handoff.

  • Workflow: step order, permissions, environment, and pinned actions. Named steps show setup, agent validation, and publication responsibilities.
  • JavaScript helper: dependency-free operations. Start with requireAllowedChangesAndReport and preparePullRequest for publication policy; writeAgentSummary and requireSuccessfulCompletion handle the agent result. Setup operations handle scanner installation, AWS role credentials, and prompt loading.
  • Permanent regression tests: real isolated Git repositories exercise publication boundaries and failure paths.
  • Agent instructions: validation and handoff contract.

After the agent finishes, a named workflow step loads the helper from the original checkout commit into the runner's temporary directory. Later steps run this trusted copy with simple Node commands, so modifying the working-copy helper cannot bypass publication checks. No package installation is needed to run the helper. Loader and environment.

Verification

Run with the repository-pinned Node 24.22.1.

yarn test:scripts
46 passing (22s)

The checked-in tests cover allowed and forbidden paths, hidden staged changes, exact working-tree preservation, changed HEAD, missing reports, duplicate pull requests, GitHub query failures, case-insensitive label preservation, final-result precedence, private metadata filtering, summary-write failure, missing publication URLs, scanner checksum rejection, and modification of the working-copy helper. The trusted-helper regression executes the loader and publication commands directly from the workflow file.

node_modules/.bin/prettier --check scripts/iyarc-prune.js scripts/tests/iyarc-prune.test.js .github/workflows/iyarc-prune.yml .claude/agents/iyarc-prune.md
Checking formatting...
All matched files use Prettier code style!
nix shell github:NixOS/nixpkgs/e9b7fc70551fcd2e52077d58d2c3bcee2428cf8d#actionlint github:NixOS/nixpkgs/e9b7fc70551fcd2e52077d58d2c3bcee2428cf8d#shellcheck -c actionlint .github/workflows/iyarc-prune.yml

Exit 0, no workflow diagnostics. Nix emitted the previously observed eval-cores and lazy-trees setting warnings.

Earlier runtime checks exercised the previous helper loader with the pinned publisher against a local Git remote, and installed/executed the real Linux scanner in a container without repository dependencies. Historical runtime evidence. The current loader is covered by the checked-in workflow-command regression above.

Live verification still required

No successful hosted master run is claimed. After merge, dispatch from Use workflow from: master and verify model access, generated commit signature, pull request, labels, assignment, and required checks. The repository setting allowing Actions to create pull requests could not be read with the available token (403).

Generated pull requests use GITHUB_TOKEN; a maintainer may need to select Approve workflows to run before required checks start. Current GitHub behavior.

Ticket

WAL-2232

@zahin-mohammad
zahin-mohammad requested review from a team as code owners October 28, 2026 18:00

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@linear-code

linear-code Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

WAL-2232

@zahin-mohammad
zahin-mohammad marked this pull request as draft October 28, 2026 18:00
@zahin-mohammad

Copy link
Copy Markdown
Contributor Author

@claude review always

@github-actions

This comment was marked as outdated.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment thread .github/workflows/iyarc-prune.yml Resolved
Comment thread .github/workflows/iyarc-prune.yml Resolved
Comment thread .github/workflows/iyarc-prune.yml Resolved
Comment thread .claude/agents/iyarc-prune.md Resolved
@zahin-mohammad

This comment was marked as outdated.

@zahin-mohammad

This comment was marked as outdated.

@zahin-mohammad
zahin-mohammad marked this pull request as ready for review October 30, 2026 15:57

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@zahin-mohammad
zahin-mohammad marked this pull request as draft October 30, 2026 15:57
@zahin-mohammad

Copy link
Copy Markdown
Contributor Author

@claude review always

@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @zahin-mohammad's task in 24s —— View job


Review of PR #10087 (latest push 1bf6038)

No blocking issues. I read the final workflow top to bottom. I did not run anything, including your decision harness.

The earlier concerns are addressed:

  • Agent commits: BASE_SHA comes from the checkout output, and the step fails if HEAD moved. The file check also covers staged and untracked files.
  • Label colors: labels are created only when missing, without --force.
  • Scanner: it is installed with a pinned checksum before the agent runs.
  • PR CI: the warning and the job summary now tell a maintainer to approve pending runs.

Optional follow-ups:

  1. Dedupe check runs after the agent. The open-PR check in "Check changes are ready" happens after the agent step. If a prune PR is already open, the run still pays for the full agent run and then discards the result. A cheap gh pr list step before "Run Claude Code" would skip it. The same check after the agent can stay as a race guard.
  2. Fail-closed on stray untracked files. git ls-files --others --exclude-standard rejects any non-ignored file the agent's build or test runs leave behind, such as a scratch log. That is safe, but one stray file discards a validated run. Consider ignoring files that are not under the allowlist only if they are untracked, since add-paths already keeps them out of the commit. Keeping it strict is also defensible.
  3. Agent token scope. The agent step has GH_TOKEN and contents: write, so it could push on its own despite the "do not commit" instruction. The HEAD guard catches a local commit but not a push to another branch. This is low risk, but a note in the agent doc, or dropping GH_TOKEN from that step if the agent doesn't need it, would tighten it.
  4. Commit footer. The commit message in create-pull-request says Ticket: HSM-429, while this PR is WAL-2232. Confirm that is the intended ticket for the generated commits.
  5. Live verification still open. As you noted, the following can only be checked after merge: the "Allow GitHub Actions to create and approve pull requests" setting, the commit signature, and Linux scanner execution. Dispatch from master once and check those.
    · Branch: codex/WAL-2232-iyarc-prune-create-pr

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code review found no issues

No high-confidence issues detected in this change.

@zahin-mohammad
zahin-mohammad marked this pull request as ready for review November 2, 2026 18:58

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

zahin-mohammad and others added 2 commits November 2, 2026 14:14
Publish signed dependency updates from the exact validated checkout.
Require master dispatches and provision the pinned vulnerability scanner.

Ticket: WAL-2232

Co-authored-by: Ralph CLI <ralph-cli@bitgo.com>
Ticket: WAL-2232

Co-authored-by: Ralph CLI <ralph-cli@bitgo.com>
Ralph: 18.12.3-bitgo.6 openrouter/openai/gpt-6.1-sol@default:high openrouter/z-ai/glm-5.3-flash@smol:low+task:medium openrouter/z-ai/glm-5.3@slow:max
@zahin-mohammad
zahin-mohammad force-pushed the codex/WAL-2232-iyarc-prune-create-pr branch from d92ae02 to 03788c5 Compare November 2, 2026 19:22
Comment thread scripts/iyarc-prune.js Resolved
@mukeshsp

mukeshsp commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

@claude review please

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @mukeshsp's task in 22s —— View job


Review of PR #10087

I read scripts/iyarc-prune.js and .github/workflows/iyarc-prune.yml in full. I did not run the tests or the workflow.

One blocking issue, one smaller one, and a few optional points.

🔴 Blocking: Nx cache files can fail a validated run

collectChangedFiles (scripts/iyarc-prune.js:177-185) adds every untracked file reported by git ls-files --others --exclude-standard. requireAllowedChangesAndReport (:145-153) throws Unexpected changed file on anything outside the allowlist.

  • The agent runs yarn lerna run build/unit-test, which can write Nx's project-graph data to .nx/workspace-data/ at the repo root. I checked that .gitignore has no .nx entry. Only .prettierignore:11 lists /.nx/workspace-data, so the repo already expects that directory to exist.
  • If that directory appears, the "Check allowed edits" step fails and ready is never set. A run that validated a fix would end red with no PR. This is the same silent-failure outcome the PR is meant to fix.
  • I did not run Lerna to confirm the files are written. It follows from lerna.json (useNx: false) and .prettierignore.
  • Rejecting these files protects nothing, because add-paths already keeps them out of the commit.

Suggested fixes, in order of preference:

  1. Add /.nx/ to .gitignore.
  2. Set NX_WORKSPACE_DATA_DIRECTORY: ${{ runner.temp }}/nx-workspace-data on the agent step.

The untracked scan should still catch real stray repository edits. A regression test with an untracked .nx/workspace-data/x file would lock this in.

🟡 Hidden changes inside ignored paths

--exclude-standard also hides ignored files. For example, an ignored modules/foo/package.json would not be listed. It also would not be published, since create-pull-request uses add-paths, so this looks harmless. Still, fix 1 above (ignoring .nx) only works because the scan honours .gitignore, so keep the scan as it is.

🟡 Optional suggestions

  1. Order of checks (:217-227): preparePullRequest runs requireAllowedChangesAndReport before looking for an open prune PR. A stray file therefore fails the job even when no PR would have been published anyway. That is fail-closed and fine, but swapping the order would save a red run.
  2. Duplicate-PR check runs after the agent: the earlier review's point still stands. A cheap gh pr list step before the agent would avoid paying for a run that gets discarded.
  3. Agent token scope: the agent step has GH_TOKEN and contents: write, so it could push on its own. The HEAD guard only catches local commits. Dropping GH_TOKEN from that step, if the agent does not need it, would tighten this.
  4. Commit footer (iyarc-prune.yml:135): it says Ticket: HSM-429, while this PR is WAL-2232. Please confirm that is intended for the generated commits.
  5. Non-draft PRs without CI: GITHUB_TOKEN-created PRs may need a maintainer to select "Approve workflows to run". This is documented in the summary. Good.

Looks good

  • The trusted helper is loaded from BASE_SHA after the agent finishes, so edits to the working-copy helper cannot bypass the checks (:90-96).
  • The master-ref guard runs before checkout.
  • preserveValidatedCheckout keeps the parent and tree exact.
  • Label creation no longer forces a color change.
  • The scanner is installed with a pinned checksum.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code review found no new issues

No new issues were found in this update; 1 finding from earlier reviews is still open above.

Still open from earlier reviews (1):

  • 🔴 scripts/iyarc-prune.js:182 — Weekly runs that validated a fix can fail red with no PR, because Lerna's Nx cache files count as unexpected edits. The…

If you have decided not to act on one of these findings, resolve its thread (a reply alone leaves it open) and the next review stops counting it. To review this commit again now, use Re-run on its "Claude Code Review" check.

@s84krish s84krish left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm, Claude's critical finding seems invalid:

Reproduction: ran the agent's exact commands (yarn lerna run build --scope=@bitgo/statics --stream, then unit-test) in a worktree of the PR head: build + tests pass, no .nx directory is created, and git ls-files --others --exclude-standard reports zero unexpected untracked files after both.

@zahin-mohammad
zahin-mohammad merged commit 3d2cf3a into master Nov 3, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants