Skip to content

fix: select reusable staging sweep run#2175

Merged
cquil11 merged 1 commit into
mainfrom
agent/fix-stage-results-selector
Jul 13, 2026
Merged

fix: select reusable staging sweep run#2175
cquil11 merged 1 commit into
mainfrom
agent/fix-stage-results-selector

Conversation

@cquil11

@cquil11 cquil11 commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • select the newest artifact-bearing full-sweep run from any commit still in the source PR
  • skip successful check-only/reuse-gate runs that produced no staging artifacts
  • allow an explicit run ID from an earlier commit that remains in the PR history
  • use the existing cross-repository PAT for authorization and PR comments

Root cause

The initial selector filtered to the current PR head and chose the newest successful run before checking artifacts. PR #2157 therefore selected a successful no-op reuse check and could not fall back to the prior completed sweep. GitHub also reduced the issue_comment GITHUB_TOKEN to read-only for the fork-PR context, so the failure comment returned 403.

Validation

@cquil11 cquil11 requested a review from a team July 13, 2026 17:37
@cquil11 cquil11 merged commit 2db4bef into main Jul 13, 2026
4 checks passed
@cquil11 cquil11 deleted the agent/fix-stage-results-selector branch July 13, 2026 17:41

@claude claude Bot 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.

This PR touches CI auth/permissions (drops the job-level permissions: block and swaps three github-token usages from github.token to secrets.INFX_FRONTEND_PAT), so I'd like a human to sign off. No bugs were found by the review pass — the run-selection refactor looks internally consistent and the finder's concern about removing bilingual failure comments was examined and ruled out (the remaining bilingual comments cover user-visible paths; the removed ones fired only when core.setFailed was already going to surface via the workflow status).

Extended reasoning...

Overview\n\nSingle-file change to .github/workflows/stage-results.yml. Two categories of edits: (1) refactor of the /stage-results source-run selector — replaces the head-SHA + belongs-to-PR filter with an in-PR commit-set membership check, moves all gating (path, event, label-timing, status, artifact presence) into a single validateRun helper, and iterates all completed runs on the PR branch via github.paginate until one with usable artifacts is found; (2) token/permissions change — removes the job-level permissions: block and points the three actions/github-script steps at secrets.INFX_FRONTEND_PAT instead of github.token.\n\n### Security risks\n\nThe workflow triggers on issue_comment and dispatches to a sibling repo, so it's inherently sensitive. The actor gate (write/maintain/admin via getCollaboratorPermissionLevel) and the command regex are unchanged, so the authorization surface is the same. The PAT swap is the load-bearing risk: INFX_FRONTEND_PAT was already used by the cross-repo dispatch step, so scope isn't expanding, but it's now also the identity that posts the ack comment and reads runs/artifacts/timeline/commits. A human should confirm the PAT's scopes are still appropriate and that removing the job-level permissions: block (now inheriting the top-level permissions: {}) doesn't break any step that still relies on GITHUB_TOKEN — spot-checking the diff, none of the remaining calls use github.token, so this should be fine.\n\n### Level of scrutiny\n\nHigh — CI workflow touching auth tokens on a comment-triggered dispatcher is exactly the shape I'd rather not shadow-approve, even when the diff is small and the rationale is clear. Blast radius on a mistake here is repo-wide.\n\n### Other factors\n\nRoot cause and validation (actionlint, zizmor, artifact spot-check on run 29181694248) are documented in the PR description. The candidate concern about removed bilingual PR feedback on the label-predating and missing-changelog-metadata paths was ruled out by the finder pass — those paths now surface only via core.setFailed, which is a UX regression at worst, not a correctness bug.

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

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

1 participant