Skip to content

fix(ci): App-free rerun commands at contributor tier, with fork /rerun-test under label approval - #2718

Merged
guapisolo merged 4 commits into
mainfrom
jiajun/fix-comment-ci-no-app
Aug 23, 2026
Merged

guapisolo merged 4 commits into
mainfrom
jiajun/fix-comment-ci-no-app

Conversation

@guapisolo

@guapisolo guapisolo commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Make the PR-comment commands /rerun-test <test-file> and /rerun-failed-ci work out of the box — no GitHub App, no CI_COMMAND_APP_* setup — at contributor tier instead of write, for same-repository and fork PRs alike. Label commands are CI policy and keep both their App requirement and their write gate.

Motivation

The point of /rerun-test is to let anyone iterating on a PR run one targeted test instead of burning a full CI pass. As shipped, the gateway sat behind a three-piece App setup for every command and required live write permission — so in practice nobody could use it, and the people who need it most (external contributors, whose PRs are necessarily forks) were structurally excluded.

Design constraints this PR encodes:

  1. The App is only irreplaceable for labels. A label added with GITHUB_TOKEN never fires the pull_request(labeled) CI workflows (GitHub's recursion guard). A file run starts through workflow_dispatch — a documented exception to that guard — and a failed-job rerun is a new attempt of an existing run, not a new event-triggered run. Neither needs the App.
  2. A fork head is the normal shape of a non-write contributor's PR — pushing a same-repository branch already requires write. So the policy tier (prior contributor or live write) is the whole gate for both head shapes; fork adds containment, not approval.

Changes

File Change
.github/workflows/comment-ci-command.yml Actions capability + 👍/reply feedback jobs run on the workflow's own GITHUB_TOKEN with job-scoped permissions (actions: write + pull-requests: read; issues: write for feedback). App mint remains only in the issues (label) path; a label command posted while the App is not enabled fails loudly instead of skipping silently.
.github/workflows/policies/comment-command-access.json (v3) New prior_contributor_access group (OWNER/MEMBER/COLLABORATOR/CONTRIBUTOR via the comment's author_association, else live write/admin) owns both run_test_file and rerun_failed_ci. add_label/clear_labels stay write/admin. First-time contributors and unknown users are denied.
.github/workflows/scripts/comment_ci_command.py Policy schema v3 validation; parse_event extracts and validates author_association; require_access admits an allowed association before the live-permission lookup. Fork /rerun-test runs the same policy gate as same-repo, plus the fork head identity is re-verified against its unique open PR before dispatch.
.github/workflows/run-ci-file.yml The trusted resolver derives fork-ness from the live PR (never from a dispatch input); fork runs receive no repository secrets — WANDB_API_KEY (CUDA) and HF_TOKEN (CPU) are withheld, matching the pr-test fork policy.
docs/ci/05-command-identity.md (new) Owns comment-command identity end to end: event identity bindings, the three access groups, the explicit tier constraints (labels = CI policy; rerun = runner time, not code trust; fork = the normal non-write contribution shape, so the tier is the whole gate), evaluation points, and per-command token identity. Registered in docs.json navigation.
docs/ci/01-label.md Keeps command behavior and the enablement runbook (now label-commands-only; App grant drops Actions: write); authorization moved to Command Identity.
tests Handler suite grows to 241 tests (fixtures carry author_association; new coverage: contributor dispatch without permission lookup on both head shapes, first-timer denial, association not leaking to label commands, fork head-identity verification, secret-withholding workflow invariants, fail-closed invalid association).

Authorization matrix

Commenter /rerun-test (same-repo or fork) /rerun-failed-ci /run-ci-*, /clear-labels
write / admin ✅ (App required)
prior contributor (merged commit)
first-time contributor / no history

Security posture

  • A failed-job rerun re-executes an already-authorized run with its original privileges and SHA — granting it to contributors adds runner time, not code trust. Held (action_required) runs are never selected for rerun.
  • Fork file runs execute fork code on suite runners, gated by the contributor tier; containment: no repository secrets (fork-ness derived server-side from the live PR, so a forged dispatch input cannot reclaim them), head identity pinned to its unique open PR, one registered file, max(1800, est*1.25)s timeout, per-PR queued serialization.
  • The command App token, when the label path is enabled, is scoped to Issues: write only and never reaches the jobs that execute PR code.

Verification

  • tests/ci/test/test_comment_ci_command.py: 241 passed; tests/ci/test/test_file_run.py: 21 passed; black 24.3.0 clean; YAML and docs.json parse.
  • Not verifiable before merge: issue_comment workflows run only from the default branch. Post-merge validation steps are kept in docs/ci/01-label.md (/rerun-failed-ci on a disposable failed run, then /rerun-test with one registered CUDA file; for the fork path, one fork PR commented by a prior contributor, confirming the dispatched run shows empty WANDB_API_KEY).
  • Residual risk: fork-head checkout in run-ci-file.yml fetches the bare PR head SHA from the base repo (reachable via refs/pull/N/head); if a fork force-pushes it away before the run starts, the run fails loudly at checkout.

🤖 Generated with Claude Code

…ributors

The comment gateway required a GitHub App (CI_COMMAND_APP_ENABLED,
CI_COMMAND_APP_CLIENT_ID, CI_COMMAND_APP_PRIVATE_KEY) for every command and
live write permission for /rerun-test, which defeated the point of targeted
single-file runs as the cheap alternative to full CI.

The App is only irreplaceable for label commands: a label added with
GITHUB_TOKEN never fires the pull_request(labeled) CI workflows. A file run
starts through workflow_dispatch, which is a documented exception to the
GITHUB_TOKEN recursion guard, and a failed-job rerun is a new attempt of an
existing run, so neither needs the App.

- comment-ci-command.yml: the actions capability and the reaction/reply
  feedback jobs run on the workflow's own GITHUB_TOKEN with job-scoped
  permissions; the App mint remains only in the issues path, and a label
  command posted while the App is not enabled fails loudly instead of
  skipping silently.
- comment_ci_command.py + policy v3: a new prior_contributor_access group
  admits the comment's author association (OWNER, MEMBER, COLLABORATOR,
  CONTRIBUTOR) for run_test_file without a live-permission lookup; everyone
  else still needs live write or admin. First-time contributors and users
  with no history are denied. Fork PRs, per-PR serialization, and the other
  commands' write gates are unchanged.
- docs/ci/01-label.md: the enablement runbook now covers only label
  commands, and the App permission grant drops Actions: write.

Verified with tests/ci/test/test_comment_ci_command.py (229 tests, 17 new)
and tests/ci/test/test_file_run.py under black 24.3.0 formatting.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

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

…label approval

Per review direction: label mutations are CI policy and stay write-gated;
both rerun commands drop to the prior-contributor tier, and /rerun-test
accepts fork heads under the repository's existing fork-trust anchor.

- policy v3: rerun_failed_ci joins run_test_file in prior_contributor_access.
  A failed-job rerun re-executes an already-authorized run with its original
  privileges and SHA, so the grant adds runner time, not code trust.
- comment_ci_command.py: /rerun-test on a fork head requires the commenter's
  own live write/admin, or for a prior contributor a maintainer-applied
  run-ci* label - the same standing Approve-and-run decision
  approve-trusted-ci encodes for held fork CI runs. The fork head identity
  is re-verified against its unique open PR before dispatch.
- run-ci-file.yml: the resolver derives fork-ness from the live pull request
  (never from a dispatch input) and fork runs receive no repository secrets
  (WANDB_API_KEY and HF_TOKEN withheld), matching the pr-test fork policy.
- docs/ci/01-label.md: authorization tiers and the fork trust boundary
  updated to match.

Verified: tests/ci/test/test_comment_ci_command.py (236 tests) and
tests/ci/test/test_file_run.py (21 tests) pass; black 24.3.0 clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@guapisolo guapisolo changed the title fix(ci): run /rerun-test without the command App and admit prior contributors fix(ci): App-free rerun commands at contributor tier, with fork /rerun-test under label approval Aug 23, 2026
guapisolo and others added 2 commits August 22, 2026 23:23
… the gate

A fork head is the normal shape of a non-write contributor's PR - pushing a
same-repository branch already requires write - so requiring a
maintainer-applied run-ci* label on top of the prior-contributor tier
excluded exactly the people the command exists for. The tier is now the
whole gate for both head shapes; the doc states this constraint explicitly.

Containment on fork heads is unchanged: head identity re-verified against
its unique open PR before dispatch, and the dispatched run receives no
repository secrets (resolver derives fork-ness from the live PR).

Verified: 241 handler tests + 21 file-run tests pass; black clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Authorization for the comment gateway was spread through Labels prose. A new
Command Identity page (docs/ci/05-command-identity.md) owns it end to end:
the event identity bindings (human-User author, sender match, numeric-ID and
association binding, fail-closed enum), the three access groups and which
command each gates, the explicit tier constraints (labels are CI policy so
they stay write-gated; rerun commands cost runner time not code trust so
they sit at the prior-contributor tier; a fork head is the normal shape of
a non-write contribution so the tier is the whole gate and fork adds only
containment), the two evaluation points, and the token identity each
command executes under.

Labels keeps command behavior, syntax, the App enablement runbook, and
failure semantics, and points at Command Identity for authorization. The
page is registered in docs.json navigation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@guapisolo

Copy link
Copy Markdown
Collaborator Author

quick merge this to unblock

@guapisolo
guapisolo merged commit b66cb4e into main Aug 23, 2026
25 checks passed
@guapisolo
guapisolo deleted the jiajun/fix-comment-ci-no-app branch August 23, 2026 09:16
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.

1 participant