diff --git a/.github/workflows/claude-review.yml b/.github/workflows/claude-review.yml index af093bc..c289f74 100644 --- a/.github/workflows/claude-review.yml +++ b/.github/workflows/claude-review.yml @@ -24,7 +24,7 @@ # (the action accepts that only in block mode). Block mode is revisited # before org-wide enforcement (build plan, phase 5). -name: claude-review +name: Claude of Duty on: pull_request_target @@ -32,6 +32,7 @@ permissions: {} jobs: collect: + name: Recon runs-on: ubuntu-24.04 timeout-minutes: 10 permissions: @@ -94,6 +95,7 @@ jobs: retention-days: 3 analyze: + name: Analysis needs: collect runs-on: ubuntu-24.04 timeout-minutes: 10 @@ -198,6 +200,7 @@ jobs: retention-days: 3 dependency-review: + name: Supply line check needs: collect runs-on: ubuntu-24.04 timeout-minutes: 10 @@ -226,6 +229,7 @@ jobs: show-openssf-scorecard: false oc-claude-review: + name: Mission report needs: [collect, analyze, dependency-review] if: always() runs-on: ubuntu-24.04 diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..69de248 --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +# Before and after JSON of owner-run releases (review/tools/release.py) +release-records/ diff --git a/Makefile b/Makefile new file mode 100644 index 0000000..b4d081d --- /dev/null +++ b/Makefile @@ -0,0 +1,22 @@ +# Owner-run release targets. Not used by any workflow or CI job: the release +# tool holds an Anthropic org:admin login and GitHub write at once, which no +# job may hold. See review/tools/release.py and docs/claude-review-design.md, +# "Each release" in "Auth and cost". +# +# make release-plan SHA= [NO_CONSOLE=1] read-only plan +# make release SHA= plan, then apply at once (no prompt), read back +# make release-rule-test RULE= SHA= Console-only edit of a scratch rule +# rollback: make release SHA= + +TOOL = cd review && uv run --frozen python tools/release.py + +.PHONY: release-plan release release-rule-test + +release-plan: + $(TOOL) plan --sha "$(SHA)" $(if $(NO_CONSOLE),--no-console) + +release: + $(TOOL) release --sha "$(SHA)" + +release-rule-test: + $(TOOL) rule-test --rule "$(RULE)" --sha "$(SHA)" diff --git a/README.md b/README.md index f339b8d..f2db90e 100644 --- a/README.md +++ b/README.md @@ -9,3 +9,18 @@ Every PR in every OperationCode repo gets an automatic review from Claude: a cod Design: [docs/claude-review-design.md](docs/claude-review-design.md) Build plan: [docs/build-plan.md](docs/build-plan.md) + +## Releases (owner only) + +A release points the production federation rule and every ruleset pin at one merge SHA on main. The owner runs it from a clone of this repo. No workflow runs it. See "Each release" in the [design](docs/claude-review-design.md) and the [build plan](docs/build-plan.md). + +One-time setup: install the [`ant` CLI](https://platform.claude.com/docs/en/cli-sdks-libraries/cli/authentication) with `brew install anthropics/tap/ant`, and log in to `gh` as an org owner. You do not log in to `ant` yourself and you keep no `ant` profile. The tool runs `ant auth login --profile admin --scope "org:admin"` in a private temp dir (your browser opens), and runs `ant auth logout --all` and deletes the dir when it exits. The federation rule endpoints refuse Admin API keys, and an `org:admin` login stored on disk is readable by any process that runs as you, so no key or token is kept. + +```sh +make release-plan SHA= # read-only; NO_CONSOLE=1 skips the Console reads +make release SHA= # plan, then apply at once (no prompt), read back; `make release-plan` is the read-only dry run +make release SHA= # rollback +make release-rule-test RULE= SHA= # Console-only edit of a scratch rule (refuses the production rule) +``` + +Before and after JSON goes to `release-records/` (git-ignored, no secrets). diff --git a/docs/build-plan.md b/docs/build-plan.md index 53c6ce1..3e267fa 100644 --- a/docs/build-plan.md +++ b/docs/build-plan.md @@ -198,6 +198,10 @@ Exit criteria: - [ ] Right after enabling the pilot ruleset, list the open PRs in the pilot repos again (do not reuse a list from earlier recon) and start a run on each (close and reopen, or a new push). A required workflow starts only on a new PR event, so a PR opened between the recon and the enable gets no review run: front-end PR 2027 (opened 2026-10-29T08:06Z) was missed this way. - [ ] Each week, record false positives, overrides, bypasses, failed runs by reason, egress blocks and cost per run. Keep cost numbers (any currency amount) in the owner's private notes and the Anthropic Console, never in this public repo. Public text may give ratios and timing only. - [ ] Fix false positives in prompts or code. Each fix goes through phase 2 again, then ships as in "Each release" below. +- [x] Release cf19216 (PR #21, spec K-18, K-19): npm registry responses capped at 128 MiB decoded, gzip requested with a bounded inflate, and visible failure reasons for a failed lookup. Found by the pilot: a false hard block on front-end PR 2027. +- [x] Release 96f4966 (PR #22, spec K-20, K-21): the byte budget counts received bytes with no up-front reservation, the budget is 8 GiB, script text is cut to 1000 chars, and the local harness `review/tools/dep_repro.py` reproduces the dependency stage. Rule: a dependency-check change is reproduced and validated locally with the harness before release. +- [x] Pilot result, 2026-10-31: the first real PR (`OperationCode/front-end` PR 2027, a large dependency update) was falsely blocked three times at earlier releases (the pnpm two-document lockfile parse, the lockfile diff size, then the npm cap and the byte-budget race). At 96f4966 it passes with 0 hard blocks and 0 findings, matching the local harness. `operationcode_infra` PR 191 passed with low, non-blocking findings. The test PRs were closed 2026-10-31. The pilot continues; the exit judgment is pending the owner. +- [ ] Release item (pending): "Claude of Duty" branding of the PR comment and check names (spec P4-8). Display text only; no change to markers, job ids, file names or the `test / unit-tests` check. - [x] Keep the fixture eval results as an artifact: `eval.yml` uploads `results.jsonl` and each case's `summary.md` as `eval-results-a` (14 days, `if: always()`). Before this, a release eval's per-case results existed only in the job log (phase 2 run 40445885087 and phase 3 run 40528717338 were read from logs). Ships as in "Each release". - [ ] Plan publication before writing: bounded continuation comments, pointed to from the sticky summary, hold every summary-only finding (changed keys, unverified, outside the diff) when one comment cannot. Then no explanation is cut from the PR comments and summary capacity no longer fails a run whose computed result is `pass` (spec "Publishing (code)", size limit residual; review r-p3-cut-final Important 1). Keep the fail-closed check until this path is complete. Ships as in "Each release". Plan (planned, not built): [docs/continuation-comments-plan.md](continuation-comments-plan.md). @@ -247,15 +251,13 @@ Exit criteria: ## Each release (from phase 4 on) -One federation rule allows one SHA, and every release is validated at its final SHA before a pin moves (spec "Auth and cost", "Each release"): - -- [ ] Owner: create a new federation rule R2 as a copy of the current rule R1, with the placeholder SHA `0000000000000000000000000000000000000000` in `job_workflow_ref`. Read it back (V26). -- [ ] Put the R2 ID in the central code. That commit is the release candidate S2. -- [ ] Owner: set `job_workflow_ref` of R2 to S2 and read it back. -- [ ] Validate S2, for every release: unit tests pass, the owner-approved fixture eval passes, and a run under the scratch test ruleset pinned to S2 gets a token through R2. -- [ ] Merge. If the merge makes a new SHA, check that its tree equals S2 (`git diff` is empty, so the content and rule ID are the same). If the trees differ, treat the merge SHA as a new candidate and go back to the step that sets R2. That SHA is the release SHA. Phase 3 learned: every squash merge makes a new SHA, so one release takes two Console edits per rule, the candidate SHA and then the merge SHA. -- [ ] Before the second Console edit, pin the scratch test ruleset to the merge SHA and run once: the exchange must be denied, because the rule still names the candidate SHA (phase 3: run 40583290922). Then the owner sets R2 to the merge SHA and reads it back. -- [ ] Pin the scratch test ruleset to the release SHA and do one token exchange through R2. Any new candidate, for example a fix commit after a failed validation, repeats from the step that sets R2 to the candidate SHA. -- [ ] Owner: move the pin of every ruleset that requires the workflow (the pilot ruleset, the org ruleset, or both) to the release SHA, then read each back: `repository_id`, path, `sha`, no `ref`. -- [ ] Start a new run on each open PR (push, or close and reopen). Do not re-run old runs: after R1 is deleted, a re-run at the old SHA fails auth. Do this after every pin move, also a move back: the box counts only runs at the pinned SHA, so a move back revives older passes at that SHA (spec "Pinned SHA only"). -- [ ] Owner: delete R1 only when each new run has completed at the release SHA (`job.workflow_sha`); a queued run does not count (V22). +Each release edits rule R1 in place, once, at the merge SHA, with the owner-run release tool (spec "Auth and cost", "Each release"; decision P4-9): + +- [ ] Merge the release PR. The merge SHA is the release SHA. +- [ ] Validate at the release SHA: `test.yml` (the push run on main) and the owner-approved fixture eval (`eval.yml`, a `workflow_dispatch` run on main) both conclude success. A pull_request run at the same head SHA does not count. +- [ ] Owner: `make release-plan SHA=`. Read the plan: every precondition `ok`, the R1 `job_workflow_ref` change, and the pin change of each ruleset. +- [ ] Owner: `make release SHA=`. There is no prompt: it prints the plan and applies at once, so read `make release-plan` first. The tool edits R1 first, then moves the pin of every ruleset that requires the workflow (the pilot ruleset and the scratch test ruleset), and reads all three back. A required run started between the rule edit and the last pin move fails closed. A non-zero exit after a write prints which object is at which SHA, the records path and the recovery (the same command again, or the rollback command it prints): follow it before anything else. +- [ ] Start a new run on each open PR (push, or close and reopen). Do not re-run old runs: a run at the old SHA fails auth. Do this after every pin move, also a move back: the box counts only runs at the pinned SHA (spec "Pinned SHA only"). +- [ ] Check one run under the scratch test ruleset (claude-review-test) gets a token at the release SHA. +- Rollback: `make release SHA=`, then new runs on each open PR. +- After a change to the release tool or to `ant`: `make release-rule-test RULE=fdrl_01AfACzupW3U4Bx361ZUbsyR SHA=` on the scratch rule R2 first. Done live on 2026-10-31 17:21Z (R2 `claude-review-p3-r2` moved `1cefe42` to `96f4966`, read-back showed only `job_workflow_ref` changed, logout and temp dir removed); repeat only after a later change to the tool or to `ant`. diff --git a/docs/claude-review-design.md b/docs/claude-review-design.md index 52b5fd0..8588a0f 100644 --- a/docs/claude-review-design.md +++ b/docs/claude-review-design.md @@ -181,7 +181,7 @@ Only the gate job writes to GitHub, and it writes only comments. It has no Anthr - **Trigger:** `pull_request_target` only. The ruleset starts the workflow and ignores `types`. `github.event` is the target repo's PR event. - **Which file a normal `pull_request_target` run uses:** a `pull_request_target` workflow that a repo starts itself (not the ruleset) runs the file from the repo's default branch, not from the PR head and not from the PR base branch (V3: a PR into a non-default base ran the file from `main`). So a PR cannot change the file that its own `pull_request_target` run uses. For staging, a test kit file for a `pull_request_target` workflow in a target repo must be on that repo's default branch before a PR can run it. `eval.yml` uses `workflow_dispatch` on the PR branch, not `pull_request_target`, so an eval runs the PR's copy (K-4). - **Drafts:** reviewed like any other PR. A ruleset-required workflow runs on `opened` for a draft and never sees `ready_for_review`, so a draft skip would leave a PR with no review when it becomes ready. Reviewing drafts removes that state. The cost of frequent draft pushes is accepted. -- **Job names:** `collect`, `analyze`, `dependency-review` and the gate job `oc-claude-review`. The rule reads the run, not job names. +- **Job names:** job ids `collect`, `analyze`, `dependency-review` and the gate job `oc-claude-review`, shown as `Recon`, `Analysis`, `Supply line check` and `Mission report` in workflow `Claude of Duty` (display text only; see "Publishing (code)"). The rule reads the run, not job names. - **No skips by `if:`:** no job has an `if:`, except `always()` on the gate job. The only step-level `if:` is on the four download steps of the gate job, in the exact form `needs..outputs.-artifact-id != ''` for the id of that step's own artifact (see "Job handoff and provenance"); a unit test enforces the form. No step uses `continue-on-error`. A job skipped by a condition counts as success ([troubleshooting required status checks](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/troubleshooting-required-status-checks)). - **Concurrency:** no workflow-level group and no job-level group. Ruleset-required workflows must not use `cancel-in-progress`. Runs for one PR are not serialized; "Which run counts" covers several runs on one head. Every job checks the snapshot before it does costly or visible work (see "Consistent snapshot"). - **Getting the review code:** each job checks out `OperationCode/.github` at `${{ job.workflow_sha }}`, "the commit SHA of the workflow file that defines the current job" ([contexts](https://docs.github.com/en/actions/reference/workflows-and-actions/contexts)). Never use `github.sha` (in `pull_request_target` it is the target repo's default branch) or `github.workflow_sha`. The checkout names `repository: OperationCode/.github` explicitly. The analyze job also checks that its OIDC `job_workflow_sha` claim equals `job.workflow_sha`. @@ -574,27 +574,30 @@ There is one build of the central workflow. Its gate job always applies every ru - **Escape all untrusted text** before posting: strings from Claude and strings from the PR (title, paths, package names, URLs, prefilter evidence). Remove control characters, replace `@` with `@​` (no pings), render as text with no Markdown links, images, HTML or autolinks, and truncate to fixed limits. Redact credential-like strings. Escaping also breaks issue references (`#123`, `GH-123`) and runs of 7 or more hex characters, so no text becomes a cross-reference or a commit link. - **Label model output:** each pass's text sits under a note that it is model output from untrusted input, to check before acting on it. The note stands at the top of the finding details and above the model summaries, both in the collapsed record. A finding title in the visible findings table is model text without the note: its "Found by" cell names the model pass that reported it. - **Inline comments:** one review per run, with `event: COMMENT` and `commit_id` set to the bundle's head SHA, for verified and inline-eligible findings only. Each finding has two keys, both computed from the shown values: each string redacted and cut to its field limit as the publisher shows it, never the raw value, so a published key is not a guess check for a redacted secret. The D14 key is the SHA-256 of file, side, title and quote, joined by NUL. The full key is the SHA-256 of the JSON list of every field that the reader sees: pass, category, severity, confidence, file, side, line, title, quote and explanation (Python `json.dumps` with its default settings, which gives the same ASCII text for the same values). Two findings share a full key only when they show the same text in every one of these fields. Both keys are kept in a hidden `` at the start of each inline comment and read back only from review comments by `github-actions[bot]` whose body starts with that marker and holds no other; a marker with one key is not read, so its finding is posted again. The login proves only that some Actions workflow in the repo wrote the comment (as for the summary), and the keys are public hashes of shown text, so the marker alone proves nothing: the code also checks the text of the comment (review r-p3-cut-final Important 2). In one run, findings are deduped by full key, so each distinct finding gets its own inline comment, even when a cut or a redaction makes two findings equal in file, title and quote (P3-2). Across runs, a finding is not posted again when an earlier comment has its D14 key, the same path and side, and the layout of an inline comment (the marker, a finding line of the schema enums and a pass name, the finding's escaped title, an explanation and the fixed note); the line and the explanation can differ, because the D14 key covers neither. The model words an explanation again on each call and a push moves lines, so the full key of the same issue is rarely the same on two runs, and the D14 key is what keeps a re-run or a push from posting every finding again. The summary points to the earlier comment only when that comment is, in full, the body this run would write for the finding (so its full key matches too), at the same path, side and line (`line`, or `original_line` when GitHub marks the comment outdated). Otherwise the summary shows the finding in full (title, severity, place, explanation and quote), marked as different from the inline comment of an earlier run. So no explanation is hidden behind an earlier comment that says something else, and a forged comment with the D14 key can at most move a finding from its inline comment to its full summary block. That block is subject to the summary size limit below: a high or critical finding is never cut, but a low or medium one can be cut at the largest sizes (residual, see below). If nothing is new, no review is posted. If GitHub rejects the review (HTTP 422, or any other failure), the code does not retry or split it: no inline comment is posted for this run, and the summary shows all of its findings in full. The gate does not change. -- **Summary comment:** one sticky comment, found by the hidden marker `` and an author check (the `github-actions[bot]` identity). A comment with the marker from any other author is ignored. The author check proves only that some Actions workflow in the repo wrote it, so the summary is never a trust source for the gate (see "Summary writes"). The comment is built for a fast human judgment: a reader gets the result and the next step in about three seconds, and everything else is one click away. It never says "approved" and never implies approval; the provenance line says "This bot never approves a PR." and nothing else in the comment repeats it. The override command and the links to override comments are built from trusted values (server URL, repository, PR number, comment ID), never from a field the API returned. The layout, from top to bottom: +- **Summary comment:** one sticky comment, found by the hidden marker `` and an author check (the `github-actions[bot]` identity). A comment with the marker from any other author is ignored. The author check proves only that some Actions workflow in the repo wrote it, so the summary is never a trust source for the gate (see "Summary writes"). The comment is built for a fast human judgment: a reader gets the result and the next step in about three seconds, and everything else is one click away. It never says "approved" and never implies approval; the footer line under the provenance says "Claude of Duty never approves a PR. A human reviewer gives the final approval." and nothing else in the comment repeats it. The override command and the links to override comments are built from trusted values (server URL, repository, PR number, comment ID), never from a field the API returned. The layout, from top to bottom: ``` hidden marker, first line - ## [light] RESULT: OC Claude review 1. result heading + ## [light] RESULT: Claude of Duty ... 1. result heading + *Mission: Review the code. Protect the branch. ...* 1a. mission line > [!CALLOUT] how serious, who can clear it, next step 2. one colored callout | Severity | Effect | Where | Finding | Found by | 3. findings table, worst first | File checks | Review | Security | Supply chain | ... | 4. checks strip (never cut) - run link, short head SHA, never approves 5. provenance + run link, short head SHA + never-approves line 5. provenance and footer
Waiver: the exact line to copy
6. only when waivable-only
Full review record
7. everything else, collapsed ``` - - **Result heading:** `## 🟢 PASS: OC Claude review` for a pass with no findings, `## 🟡 PASS: OC Claude review, N finding(s) to consider` for a pass with non-blocking findings, and `## 🔴 FAIL: OC Claude review` for every fail. A fail caused by a pass, job or data that did not complete reads `... did not complete`. The result comes from the gate (status, reasons, override), never from model text. Every emoji has its word next to it, so color never carries meaning alone. + - **Result heading:** the heading keeps its colored dot and takes one of five fixed texts. `## 🟢 All clear: Claude of Duty review passed` for a pass with no findings (or a pass by an applied waiver). `## 🟡 Clear to proceed: Claude of Duty logged N item(s) to consider` for a pass with non-blocking findings. `## 🔴 Hold deployment: Claude of Duty identified N item(s) requiring attention` for a fail, where N is the number of rows in the findings table (hard blocks, passes or jobs that did not run, and model finding rows, so it equals what the body lists before a size cut); a fail with no row reads `## 🔴 Hold deployment: Claude of Duty identified items requiring attention`, with no number. `## 🔴 Comms down: the Claude of Duty review did not complete` for a fail caused by a pass, job or data that did not complete. `## 🔴 Comms down: Claude of Duty could not post its full report` for the fixed overflow notice. N is "1 item" or "N items". The line `*Mission: Review the code. Protect the branch. Deploy the future.*` stands directly under every heading. The result comes from the gate (status, reasons, override), never from model text. Every emoji has its word next to it, so color never carries meaning alone. + - **Branding is display text only.** "Claude of Duty" and the job names `Recon` (collect), `Analysis` (analyze), `Supply line check` (dependency-review) and `Mission report` (oc-claude-review) are display names. The workflow file name, the job ids, the `` and `` markers, `needs` references, the ruleset path and the federation `job_workflow_ref` do not change, and no code reads a display name. The check `test / unit-tests` keeps its name because a ruleset requires it by name. + - **Size gate callout:** the oversize case reads `**Out of range.** This change is too large for the model review. File checks ran, but model checks did not. A maintainer can waive this.` - **Callout rules:** `[!TIP]` (green) for a pass with no findings. `[!NOTE]` (blue) for a pass with non-blocking findings. `[!TIP]` for a pass by an applied waiver, with "A maintainer waiver was applied by LOGIN. Findings stay listed." **Every fail uses the red `[!CAUTION]` callout, waivable or not (owner ruling, decision P3-5).** `[!WARNING]` is never used for a fail result. Whether a waiver can clear the fail stays visible in the callout text ("A maintainer can waive it." or "No waiver can clear it.") and in the Effect column. The callout holds two bold sentences: how serious and who can clear it, then the next step (for the author, or for a maintainer when the author has nothing to change). The wording is a fixed table keyed by the worst reason, with fixed labels for reason codes and check IDs ("bidi-control" reads "Hidden text-direction control"); no internal ID shows. - **Findings table:** one row per place, worst first: hard blocks (🚫), passes or jobs that did not run (⚠️), then model findings by severity (⛔ critical, 🔴 high, 🟠 medium, 🟡 low), verified before unverified within a severity. Columns: severity, effect, where (file and line in a code span), finding (the title, with a pointer to the inline comment when there is one), found by. The Effect column comes from gate code: "Blocks, no waiver", "Blocks, waivable", "Blocks, waived" (an applied waiver) or "Does not block". A medium finding in a blocking category reads "Does not block" (gate truth: a blocking category blocks only at high or critical severity). An unverified finding shows "❌ not checked" after its severity and still sorts by its severity. The table is left out when there are no rows. - **Merge rule:** model findings merge into one row only when the shown file, side, line and gate effect are all the same. Findings with different effects stay on separate rows. A merged row shows the highest severity, the title of the highest-severity member (ties go to the pass order: review, security, supply-chain), and every pass that reported it, once each, in pass order, with its highest severity. The inline pointer shows only when every member has an inline comment; otherwise the cell says the other findings are in the record only. A merged row can be cut only when every member can be cut (one protected member protects the row). A hard block never merges with a model finding: the hard block is code and the finding is model output. The record keeps every title, one row per finding. - **Checks strip:** one row with file checks (hard blocks and signals), each of the three passes (ran with its finding count, failed, skipped, missing), dependency review and coverage. It is never cut. - **Collapsed Waiver block:** only when the gate fails for waivable reasons only. It holds the exact override line in a code block with the full SHAs, who may post it, the re-run method, the no-edit and no-reopen rules, and that a waiver covers only this head and merge base. A hard block, a failed pass and every other not-waivable reason show no waiver line. - **Collapsed record:** the reviewed snapshot (full head SHA, merge base SHA, base ref, run), every reason with its waivable flag as a table with plain labels, the maintainer waivers (author, time, reason, comment link, applied, waived reasons), the code checks (hard block evidence, coverage with every unavailable file, dependency changes, dependency-review job), the signals (context only, never block; the first-time contributor signal keeps its caveat), the dependency changes, the finding details (a per-pass table with category, confidence, citation state, posting state and each pass's own title, plus the explanation and quote of every finding that has no inline comment and the posting state of each), the model summaries (one row per pass, with status, model and the escaped summary, or "No summary. Reason: ..."), and the token usage per pass. Usage is token counts only (input, output, cache write, cache read, attempts); a missing value reads "not recorded", never zero, and the total reads "Recorded total" when any value is missing. The code holds no price and the repo shows no currency. -- **Size limit and cut order:** a comment body is at most 65,536 characters, and the code aims at 64,000. The collapsed blocks count toward the limit. If the body is too long, it cuts only these blocks, in this order, each from its end: unverified low and medium findings, low and medium findings, dependency changes, prefilter signals. A cut removes whole table rows, list items or detail bullets, never part of one, and a merged findings row is cut only when every member is cuttable. A group that lost items shows one line with the exact phrase "N item(s) truncated, see the run log", as a table row inside a table so that the table stays whole, and the provenance line adds "Some details were truncated, see the run log." (its size is reserved up front). The `
` open and close lines are never cut, so the tags stay balanced. The heading, the callout, the findings rows for hard blocks and for high and critical findings (verified or not, so also one whose file path analyze cut), the checks strip, the provenance line, the snapshot, the reasons, the waiver block, the override data, the hard block evidence, the coverage rows, the model summaries and the token usage are never cut. If those alone do not fit, the run fails closed: the comment becomes a short notice of fixed size (heading `## 🔴 FAIL: OC Claude review could not publish`, a `[!CAUTION]` callout with the size note, the computed gate result, the snapshot, a table with a count per reason with its waivable flag, the number of valid overrides and the run link), and the job exits 1. This can turn a computed `pass` into a failed run. Residual (review r-p3-cut-final Important 1, coordinator ruling 2026-10-27): there is one summary comment and no continuation comment. So on a re-run whose findings changed keys at the largest allowed sizes, the cut can drop the explanation of a low or medium finding from the comment (it stays in the run log), and the never-cut high and critical blocks can exceed the limit, so the run fails closed even though the computed risk did not change. Bounded continuation comments that hold every summary-only finding are planned (build plan, phase 4). +- **Size limit and cut order:** a comment body is at most 65,536 characters, and the code aims at 64,000. The collapsed blocks count toward the limit. If the body is too long, it cuts only these blocks, in this order, each from its end: unverified low and medium findings, low and medium findings, dependency changes, prefilter signals. A cut removes whole table rows, list items or detail bullets, never part of one, and a merged findings row is cut only when every member is cuttable. A group that lost items shows one line with the exact phrase "N item(s) truncated, see the run log", as a table row inside a table so that the table stays whole, and the provenance line adds "Some details were truncated, see the run log." (its size is reserved up front). The `
` open and close lines are never cut, so the tags stay balanced. The heading, the callout, the findings rows for hard blocks and for high and critical findings (verified or not, so also one whose file path analyze cut), the checks strip, the provenance line, the snapshot, the reasons, the waiver block, the override data, the hard block evidence, the coverage rows, the model summaries and the token usage are never cut. If those alone do not fit, the run fails closed: the comment becomes a short notice of fixed size (heading `## 🔴 Comms down: Claude of Duty could not post its full report`, mission line, a `[!CAUTION]` callout with the size note, the computed gate result, the snapshot, a table with a count per reason with its waivable flag, the number of valid overrides and the run link), and the job exits 1. This can turn a computed `pass` into a failed run. Residual (review r-p3-cut-final Important 1, coordinator ruling 2026-10-27): there is one summary comment and no continuation comment. So on a re-run whose findings changed keys at the largest allowed sizes, the cut can drop the explanation of a low or medium finding from the comment (it stays in the run log), and the never-cut high and critical blocks can exceed the limit, so the run fails closed even though the computed risk did not change. Bounded continuation comments that hold every summary-only finding are planned (build plan, phase 4). - **Full record:** whenever the comment is cut, the publisher prints the full redacted record (reasons, overrides, every finding, prefilter results, coverage, dependency delta) to the gate job log, between `::stop-commands::` and `::::` so that untrusted text cannot run workflow commands. The record also goes to the step summary when it is at most 512 KiB; over that, the step summary says that the record is in the job log. The step summary follows the comment as it stands at the end of the run: when the summary is updated after the final check and the update is not cut, the step summary has no full record; when that update fails, it keeps the record of the first, cut summary. "The run log" in each cut note means this job log. - **Gate:** the required run's result. No code calls the Checks API, and no job has `checks: write`. - **Order of writes:** inline comments, then the summary, then the final check, then the exit code. Every write, including the update of the summary after the final check, is preceded by the snapshot check. If the check shows a stale run, or cannot run (a failed read), the code writes nothing more and exits 1. The step summary line of such a run has `result: fail` and the `stale` reason code, whatever the gate result computed before. Only the code's own write targets exist: `POST .../pulls/{n}/reviews` with `event: COMMENT`, `POST .../issues/{n}/comments` and `PATCH .../issues/comments/{id}`. Any other write is refused in code. @@ -604,7 +607,7 @@ There is one build of the central workflow. Its gate job always applies every ru ## Auth and cost -- **Anthropic credential:** workload identity federation only. The analyze job gets a GitHub OIDC token with audience `https://api.anthropic.com`, decodes its claims without verifying them and requires `job_workflow_sha` to equal `job.workflow_sha` (and `aud` to hold the audience), then writes the token to a file with mode 0600 under `RUNNER_TEMP`. The file is deleted after the calls, and the token is never printed or put in argv. The `anthropic` SDK (1.17.0) supports federation natively, and does the token exchange and refresh itself. Analyze does not set the SDK's `ANTHROPIC_*` environment variables. It passes the three IDs and the token file to `WorkloadIdentityCredentials` in code, because with an explicit credential the SDK reads no credential from the environment, so an `ANTHROPIC_API_KEY`, `ANTHROPIC_AUTH_TOKEN` or profile on the runner cannot stand in for federation. The client also pins the base URL to `https://api.anthropic.com` (so `ANTHROPIC_BASE_URL` cannot move the calls or the exchange), refuses a static credential, or a credential header such as `X-Api-Key` or `Authorization` from `ANTHROPIC_CUSTOM_HEADERS`, with the pass reason "static credential refused", and sends every request through a guard that refuses another host, a scheme other than https and a request after the deadline. The IDs are not secret. They are constants in `analyze.py`, in the pinned central code, not in repo variables or secrets, so a repo cannot change them. They are set (2026-10-27): rule R1 `claude-review-prod-r1` (`fdrl_01N2ZktTiVKqUvvXwywBFfys`), organization `2bc920f9-977b-43b2-870a-f840e4ce809c`, and service account `claude-review` (`svac_01PiMgfb6qJgNpLZUV8hprCC`), in the Console workspace `claude-review`. Placeholder (all-zero) IDs make analyze refuse to call: each pass is `failed` with "federation not configured". The unit test `test_release_federation_ids_are_not_placeholders`, marked `release` (skipped in the default run), fails on a placeholder, and CI runs it on every PR since phase 3. So each release names its own federation rule ID (see "Each release" below). +- **Anthropic credential:** workload identity federation only. The analyze job gets a GitHub OIDC token with audience `https://api.anthropic.com`, decodes its claims without verifying them and requires `job_workflow_sha` to equal `job.workflow_sha` (and `aud` to hold the audience), then writes the token to a file with mode 0600 under `RUNNER_TEMP`. The file is deleted after the calls, and the token is never printed or put in argv. The `anthropic` SDK (1.17.0) supports federation natively, and does the token exchange and refresh itself. Analyze does not set the SDK's `ANTHROPIC_*` environment variables. It passes the three IDs and the token file to `WorkloadIdentityCredentials` in code, because with an explicit credential the SDK reads no credential from the environment, so an `ANTHROPIC_API_KEY`, `ANTHROPIC_AUTH_TOKEN` or profile on the runner cannot stand in for federation. The client also pins the base URL to `https://api.anthropic.com` (so `ANTHROPIC_BASE_URL` cannot move the calls or the exchange), refuses a static credential, or a credential header such as `X-Api-Key` or `Authorization` from `ANTHROPIC_CUSTOM_HEADERS`, with the pass reason "static credential refused", and sends every request through a guard that refuses another host, a scheme other than https and a request after the deadline. The IDs are not secret. They are constants in `analyze.py`, in the pinned central code, not in repo variables or secrets, so a repo cannot change them. They are set (2026-10-27): rule R1 `claude-review-prod-r1` (`fdrl_01N2ZktTiVKqUvvXwywBFfys`), organization `2bc920f9-977b-43b2-870a-f840e4ce809c`, and service account `claude-review` (`svac_01PiMgfb6qJgNpLZUV8hprCC`), in the Console workspace `claude-review`. Placeholder (all-zero) IDs make analyze refuse to call: each pass is `failed` with "federation not configured". The unit test `test_release_federation_ids_are_not_placeholders`, marked `release` (skipped in the default run), fails on a placeholder, and CI runs it on every PR since phase 3. So the release code names the rule that each release edits in place (see "Each release" below). - **Federation rule** ([WIF reference](https://platform.claude.com/docs/en/manage-claude/wif-reference)). The production rule, in the shape that passed V3 (V3 used the scratch workflow path and SHA, and also matched `repository_owner`): ``` @@ -624,37 +627,44 @@ token lifetime: 10 minutes - **Subject in ID form.** GitHub's `sub` uses the immutable ID form `repo:OperationCode@10479364/@:`. The Console's hint `repo:YOUR_ORG/*` does not match it: a rule with the prefix `repo:OperationCode/*` denied every token (reason `match_subject_prefix`). Every rule for this org uses the prefix `repo:OperationCode@10479364/*` (V3). This holds only for a repo that sends the immutable form: see "Immutable subject" in "Pilot and enforcement". - **Pattern mode, not CEL.** The Console rule has two match modes, and a rule uses only one. CEL mode refuses a compound condition without a subject prefix (a guard for multi-tenant issuers such as GitHub). So the rule uses pattern mode: the subject prefix plus exact claims. There is no `startsWith` and no list of SHAs. - **Claims in a ruleset-required run.** `job_workflow_ref` ends in `@` (V2, V3). A `workflow_dispatch` run of the same file uses the ref form `@refs/heads/`, so it does not match. `ref` and `sha` name the target repo's default branch, so the rule does not match them. The subject prefix binds the org only; the rule does not match the rest of `sub` (it ends in `:pull_request` and does not name the actor, V2, V4). - - **One SHA per rule.** An exact claim holds one value, so one rule allows one release SHA. A release makes a new rule for the new SHA and deletes the old rule later (see "Each release" below). + - **One SHA per rule.** An exact claim holds one value, so one rule allows one release SHA. A release edits R1 in place to the new SHA, and the old SHA stops getting tokens at once (see "Each release" below). - **Scope and workspace.** The Console offers only the scopes `workspace:developer` (messages, agents, environments) and `org:admin`. The rule uses `workspace:developer`, and it is bound to the one review workspace, never to "all workspaces". The minted token is for that workspace only (V3). - **Denies.** The API answers every deny with the same `authentication_error` "Authentication failed". Only the Console authentication history shows the reason. -- **Each release.** The rule ID is in the pinned code, and the rule names the SHA of that code. The new rule ID goes into the candidate before validation, and every release is validated at its final SHA before a pin moves. The placeholder SHA is `0000000000000000000000000000000000000000` (40 zeros); no real commit has it. The steps go in this order: +- **Each release.** The release code names rule R1 (`ANTHROPIC_FEDERATION_RULE_ID` in `analyze.py`), and each release edits R1 in place, once, to the merge SHA (decision P4-9). Every release is validated at that SHA before the rule or a pin moves. The owner runs the release tool on their own machine (`review/tools/release.py`, through the root `Makefile`); no workflow or CI job runs it. The steps go in this order: ``` - 1. create rule R2: a copy of the current rule R1, - job_workflow_ref at the placeholder SHA, read it back (V26) - 2. commit the candidate with ANTHROPIC_FEDERATION_RULE_ID = R2 -> candidate SHA S2 - 3. set R2 job_workflow_ref to ...claude-review.yml@S2, read it back - 4. validate S2, every release: unit tests pass, the owner-approved - fixture eval passes, and a run under the scratch test ruleset - pinned to S2 gets a token through R2 - 5. merge; if the merge makes a new SHA, check that its tree equals - S2 (same content, same rule ID); if the trees differ, the merge - SHA is a new candidate: go back to step 3 - 5a. pin the scratch test ruleset to the merge SHA and run once: the - exchange must be denied, because R2 still names S2 (V26 b) - 5b. then set R2 to the merge SHA, read it back -> release SHA - 6. pin the scratch test ruleset to the release SHA and do one token - exchange through R2; read R2 back first (V26) - 7. move the pin of every ruleset to the release SHA, read each back (runs at S1 still use R1) - 8. start a new run on each open PR (push, or close and reopen), - wait until each has completed at the release SHA, then delete R1 (V22) + 1. merge the release PR; the merge SHA is the release SHA + 2. validate at the release SHA: test.yml (push to main) and the + owner-approved fixture eval (eval.yml) both conclude success + 3. make release-plan SHA= read-only, shows every change + 4. make release SHA= no prompt (applies at once after the plan; `make release-plan` is the read-only dry run), then: + a. preconditions (fail closed): 40-hex SHA on main; a test.yml + push run and an eval.yml workflow_dispatch run, both on main + with head_sha = SHA, succeeded (a pull_request run at the same + head does not count); analyze.py at that SHA + names R1; the gh user is an org owner; each ruleset has exactly + one entry for claude-review.yml, with a sha and no ref + b. Console: GET R1, change only the SHA suffix of + match.claims.job_workflow_ref, POST the whole match, GET back; + every other field must be unchanged + c. GitHub: for each ruleset that requires the workflow (the pilot + ruleset 26132750 and the scratch test ruleset 25835887), GET, + change only that entry's sha, PUT the rules, GET back; every + other field must be unchanged + 5. start a new run on each open PR (push, or close and reopen) (V22) + 6. check one run under the scratch test ruleset gets a token at the + release SHA ``` - Learned in phase 3 (V26): one release takes two Console edits per rule, the candidate SHA and then the merge SHA, because a squash merge always makes a new SHA (phase 3: S1 `3aa7ae9` and its merge M1 `590a387` have the same tree `6ace36e90f776f1fd9e821851b42dfc79dad13c0`, and `git diff S1 M1` is empty). Check the tree before the second edit. Run the denied exchange at the merge SHA before the second edit (5a): it shows that the rule binds to the SHA and not to the tree (phase 3: run 40583290922 at M1 on 21:02Z, all three passes `failed (authentication failed)`). After the second edit the exchange passes (run 40585413003 at M1). After any pin move, including a move back, start a new run on each open PR (see "Pinned SHA only" in "Which run counts"). + **Fail-closed window.** The rule moves first, then the pins (a pin move needs a rule for its SHA first, see "Pin moves"). From step 4b until the last pin moves in 4c, the rulesets still pin the old SHA and R1 already names the new one, so a required run started in that window fails closed with "authentication failed" (Console reason `match_claim_value_mismatch`). The window is seconds long. Any run at the old SHA, including a re-run of an old run, fails auth after 4b; step 5 starts the new runs. A failure at any step (an error, a read-back mismatch, an interrupt) stops the tool with a non-zero exit before the next write, and it does not undo the earlier steps. When a write had been sent, on every exit path (also a Ctrl-C or SIGTERM held during the logout and raised after it), the tool prints the state of R1 and of each ruleset (pending, unchanged, sent, written, done, failed; "sent" means the outcome is unknown, so it reads each object again and shows where it is now), the records path, and the recovery: the same command again, and `make release SHA=` when R1 and every ruleset had the same previous SHA. When the previous values differ, it prints a per-object recovery instead (a `gh api -X PUT` with the saved rollback body for a ruleset, the Console for R1). Running `make release` again with the same SHA is safe: a rule or ruleset already at the SHA gets no write, only a read-back. - Any new candidate, for example a fix commit after a failed validation, repeats from step 3. Step 4 cannot pass for a new SHA while R2 still names the old one. + **Rollback** is `make release SHA=`, the same checks and steps (that SHA is on main and its test and eval runs passed). Then start a new run on each open PR: the box counts only runs at the pinned SHA, so a move back revives older passes at that SHA (see "Pinned SHA only"). - A queued run does not count for step 8; only a completed run at the release SHA does (`job.workflow_sha`). Never use a re-run of a run at S1 to migrate a PR: after R1 is deleted, it fails auth. + **Credential of the release tool.** The rule endpoints (`GET` and `POST /v1/organizations/federation_rules/{id}`, CLI `ant organization:federation:rules retrieve` and `update`) take only an OAuth token with the `org:admin` scope; Admin API keys are refused for reads and writes ([WIF Admin API](https://platform.claude.com/docs/en/manage-claude/wif-admin-api)). The POST replaces `match` as a whole object, so the tool sends the full match it read, with null matchers left out. `ant auth login` stores the access token and a refresh token in a plain 0600 file under its config dir, which any process of the owner's user could read, AI agents included. So no admin token is kept between releases (owner decision): the tool makes a private 0700 temp dir, points `ANTHROPIC_CONFIG_DIR` at it for its own `ant` calls only, removes every other `ANTHROPIC_*` variable from their environment (an API key or auth token there overrides every profile), runs `ant auth login --profile admin --scope org:admin` itself, and on every exit path (success, failure, Ctrl-C, SIGTERM) runs `ant auth logout --all` and deletes the dir. The delete sits in an inner `finally`, so it runs even when the logout fails, times out (30 s) or is interrupted; SIGINT, SIGTERM and SIGHUP are held while cleanup runs. A failed delete prints the path and an `rm -rf` line, and the run exits non-zero. It never reads, prints, stores or exports the token, and never uses the owner's default `ant` profile. The tool holds an Anthropic admin login and GitHub write at once, which no job may hold, so it never runs in a workflow (a unit test checks that no workflow names it). Before and after JSON goes to the git-ignored `release-records/` and holds no credential. + + **Scratch rule test.** `make release-rule-test RULE= SHA=` runs only step 4b on another rule, for example the scratch rule R2 `fdrl_01AfACzupW3U4Bx361ZUbsyR` on the test workspace. It refuses R1. Use it to check the tool after a change to it or to `ant`. + + History: phases 3 and 4 used a two-rule flow (a new rule R2 per release at the placeholder SHA, its ID in the candidate, two Console edits, then delete R1; V26, P0-15). Every phase 4 release instead edited R1 in place at the merge SHA (cc710fb, cf19216, 96f4966), and P4-9 makes that the spec. The trade-off: no overlap window in which runs at the old SHA still get tokens, and a rollback is one more rule edit instead of a switch back to the old rule. - **Only analyze can mint a token.** Collect and publish have no `id-token` permission. This is how "the GitHub-writing job has no Anthropic credential" holds, because the federation rule cannot tell jobs in the same workflow apart. - **Fixture eval credential.** `eval.yml` runs on `workflow_dispatch`, so the review federation rule rejects its token. Evals use a second federation rule (its IDs are in `evals.py`, separate from the production IDs in `analyze.py`). It is bound to the test workspace `claude-review-p0-test`, which is not production and has its own spend limit (P2-1). It uses pattern mode, the subject prefix `repo:OperationCode@10479364/*`, the scope `workspace:developer` and the test workspace only. The scratch rules of phase 3 (V26 c) are on the same workspace, so evals and those tests share its spend limit: when it is spent, evals and those tests stop, and production reviews do not. Exact claims: `repository_owner_id`, `repository_id` of `.github`, `event_name: "workflow_dispatch"` and `environment: "eval"` ([OIDC](https://docs.github.com/en/actions/reference/security/oidc)). In a dispatch run `job_workflow_ref` ends in the branch ref (V3), and evals run on any branch, so an exact claim cannot name `eval.yml`. V27 passed on 2026-10-27 (UTC), on the throwaway branch `v27-probe`, which is never merged. P1 (approved `workflow_dispatch` run, job in environment `eval`): token issued (HTTP 200, req_011CgS2vx6h7nwH6wi3dAFNz, 03:30:02Z). N1 (no environment): 401 (req_011CgS2ticG9pbbKTffkfMeK, 03:29:32Z). N2 (approval rejected): the job never started and made no exchange. N3 (`push` event, approved): 401 (req_011CgS2r8BXRmeR8EPtPd9si, 03:28:57Z). N4 (`push` in claude-review-test, no environment): 401 (req_011CgS34fZVr9eZEVx8z8xZw, 03:31:47Z); the job has no environment and ran on `push`, so the missing environment or the `push` event alone explains the 401, and the `repository_id` check is not tested separately. The dispatch run with its own `eval` environment in claude-review-test was not run. U1 (unit tests, `test_workflows.py -k eval`, on a clean tree): pass. An independent check of the run logs agreed with these results and found no secret. Every deny returns the same 401 body, so the owner read the reasons in the Console federation authentication history on 2026-10-27 (all rows: issuer `github-actions`, rule `eval-federation-rule`, service account `claude-review-p0`; times in UTC). 03:27:59Z Failure `match_claim_value_mismatch` (the no-environment job in the push run). 03:28:57Z Failure `match_claim_value_mismatch` (N3, `push` event). 03:29:32Z Failure `match_claim_absent` (N1, no environment). 03:30:03Z Success (P1, approved dispatch; the probe log has 03:30:02Z, so the two clocks differ by one second). 03:30:34Z Failure `match_claim_absent` (the no-environment job in the rejected-approval run; the Console showed no row for N2 itself, the rejected job, as expected). 03:31:47Z Failure `match_claim_value_mismatch` (N4, other repo). The list view does not name the claim and does not show the scope of the P1 token: claim name and P1 scope not shown in the list view. The next sentence is inference from the probe token claims, not read from the Console: each failure code fits the expected cause, an absent `environment` claim or a mismatched `event_name` or `repository` claim, but the list view names no claim. The no-environment job in the push run got `match_claim_value_mismatch`, not `match_claim_absent`; this fits the `event_name` check failing first (also inference). - **Claim boundary.** The rule accepts a dispatch run of any workflow file in `.github` whose job uses the `eval` environment, not only `eval.yml`. That only `eval.yml` uses `eval` is a repo convention; the rule cannot enforce it. Two controls hold the credential separation: the owner's approval of each run, and a workflow unit test. The unit test reads every workflow file in `.github` and fails if a workflow other than `eval.yml` uses the `eval` environment, or if a job that uses `eval` sets no explicit job-level `permissions` or has any `write` permission other than `id-token: write`. Environment names are not case sensitive, so the test treats `EVAL` as `eval`, and an environment that is not a literal string (an expression) counts as a use of `eval`. Job-level `permissions` is required on every job that uses `eval`, even when the workflow level sets it, and it holds exactly `contents: read` and `id-token: write`. `eval.yml` triggers on `workflow_dispatch` only. The test handles the string forms: `write-all` at either level fails the test, and `read-all` or a missing job-level block also fails it, because the job must state `contents: read` and `id-token: write` itself. The eval job has `contents: read` and `id-token: write` only, so the job that holds the Anthropic credential has no GitHub write permission. @@ -695,11 +705,14 @@ token lifetime: 10 minutes schema/findings.schema.json schema/envelope.schema.json tools/dep_repro.py developer tool: collect's dependency stage on two real commits, live registries + tools/release.py owner-run release: R1 rule edit, ruleset pin moves, read-backs (never in CI) tests/ unit/ no network, no API calls (unit/fixtures/eval-smoke: runner smoke case) fixtures/ the live eval cases: bundle.json, expected.json, source/ eval_fixtures.py fixture builder: source files -> bundle.json through collect docs/claude-review-design.md this file + Makefile owner-run release targets (release-plan, release, release-rule-test) + .gitignore release-records/ (local before and after JSON of releases) CODEOWNERS README.md ``` @@ -812,7 +825,7 @@ The tasks, owner actions and exit criteria for each step are in [docs/build-plan 3. **Build** in this repo, with unit tests passing. There is one build; its gate job fails for real on `fail` (see "Pilot and enforcement"). 4. **Fixture evals and acceptance:** the fixture eval passes, and the workflow acceptance tests pass in the scratch repo, including a fork PR from an account outside the org. 5. **Pilot:** the "blocking" items must pass first. Check that the federation rule named in the release code has the release SHA. An org owner gives each pilot repo its own approval protection and checks that it sends the immutable OIDC subject (see "Immutable subject" in "Pilot and enforcement"), then creates the pilot ruleset (see "Pilot and enforcement") with the release SHA and the pilot repos, reads it back, and enables it. Then, right after the enable, list the open PRs again (not from earlier recon) and start a run on each (close and reopen, or a new push). A required workflow starts only on a new PR event, so a PR opened between the recon and the enable gets no run (front-end PR 2027, opened 2026-10-29T08:06Z, was missed this way). Watch the real results on real PRs: false positives, overrides, bypasses, egress and cost per run. The owner decides when the pilot is done. -6. **Each release** (from the pilot on): follow "Each release" in "Auth and cost". Create a new federation rule at the placeholder SHA, put its ID in the candidate code, and set the rule to the candidate SHA. Validate the candidate before any pin moves: unit tests, the owner-approved fixture eval and a token exchange under the scratch test ruleset. If the merge makes a new SHA, its tree must equal the tested candidate. Move the pin of every ruleset that requires the workflow and read each back. Start a new run on each open PR, and delete the old rule only when each has completed at the release SHA (V22). +6. **Each release** (from the pilot on): follow "Each release" in "Auth and cost". Merge, validate at the merge SHA (unit tests and the owner-approved fixture eval), then the owner runs `make release-plan` and `make release` with that SHA: the tool edits R1 in place, moves the pin of every ruleset that requires the workflow, and reads all three back. Start a new run on each open PR, and check one token exchange under the scratch test ruleset. 7. **Before org-wide enforcement:** the owner accepts in writing the residuals in "Which run counts" (an earlier pass on the same SHA, the cross-PR effect of one head SHA, a pin rollback, and run deletion, which V6 f confirmed for a Write user). An org owner fixes the org ruleset (see "Admin checklist"). Build the supported repo inventory (see "Supported repos"), and add CODEOWNERS to each supported repo. Revisit harden-runner block mode (K-14). Re-run the key acceptance tests. 8. **Org-wide enforcement:** enable the org ruleset with the supported repos that are not in the pilot, in small groups. Then move each pilot repo from the pilot ruleset to the org ruleset (see "Pilot and enforcement"), and delete the pilot ruleset. After each group, start a run on each open PR, and check that each shows the required workflow passing or failing on its own result. @@ -1006,3 +1019,6 @@ P4: phase 4 pilot start, 2026-10-28 and 2026-10-29 (owner and coordinator). The - P4-4. Direct-push check (V25), 12 months: none on `back-end`, `operationcode-pybot` and `operationcode_infra`. `front-end` had a force push on 2026-10-28 that rewrote three merged PRs, a cleanup of a malicious dependency payload; it was not new unreviewed code. - P4-5. Org 2FA. The owner turned on the org 2FA requirement in the web UI. All 7 members already had 2FA. The federation rule R1 `job_workflow_ref` was read back by the owner at M4 `9e2795b` (this closes the read-back gap in P3-7; the P3-7 text below still says "not read back"). - P4-6. Spec gap: the immutable subject (design change). Repos created before GitHub's immutable OIDC subject default send `sub` as `repo:OperationCode/:...`, and the federation rule's prefix `repo:OperationCode@10479364/` rejects it (Console reason `match_subject_prefix`). The first pilot runs failed closed as designed: all three passes "authentication failed", gate FAIL, exit 1. Fix, per repo: `PUT repos/OperationCode//actions/oidc/customization/sub` with `use_default` true and `use_immutable_subject` true (no org template). This changes `sub` for every OIDC workflow in the repo. `back-end` and `operationcode-pybot` push images through a cloud role whose trust policy had to gain the immutable subjects; the owner did that and kept the old values. After the change all four test PRs passed, with all four jobs `success`. The spec now lists the subject as a prerequisite for every target repo ("Pilot and enforcement"), and phases 4, 5 and 6 of the build plan have the step. +- P4-7. Pilot result and releases cf19216 and 96f4966 (owner and coordinator, 2026-10-31). Release cf19216 (PR #21) is K-18 and K-19; release 96f4966 (PR #22) is K-20 and K-21. Rule: dependency-check changes are reproduced and validated locally with `review/tools/dep_repro.py` before release. `OperationCode/front-end` PR 2027, a large dependency update, was falsely blocked three times at earlier releases (pnpm two-document lockfile parse, K-16; lockfile diff size, K-17; npm cap and byte-budget race, K-18 to K-21). At 96f4966 it passes with 0 hard blocks and 0 findings, the same as the local harness. `operationcode_infra` PR 191 passed with low, non-blocking findings. The test PRs were closed 2026-10-31. The pilot continues; the exit judgment is pending the owner. +- P4-8. "Claude of Duty" branding (owner-approved text, release item pending). The sticky comment headings, the mission line, the oversize callout, the footer, the workflow name (`Claude of Duty`) and the job display names (`Recon`, `Analysis`, `Supply line check`, `Mission report`) change. Display text only: the markers, job ids, workflow file name, `test / unit-tests` and the inline finding format do not change. See "Publishing (code)". +- P4-9. In-place release with an owner-run tool (owner decision, 2026-10-31). Each release edits rule R1 in place, once, at the merge SHA, as every phase 4 release already did; the R2-per-release flow of P0-15 and V26 is retired, and its validation moves to the merge SHA (unit tests, the owner-approved fixture eval, and one token exchange under the scratch test ruleset after the pins move). The exact SHA claim stays: a tag, a branch ref or a prefix match would save at most one edit and drop the Console as a gate outside GitHub. The owner runs `make release SHA=` (`review/tools/release.py`): fail-closed preconditions, no confirmation prompt (owner decision, 2026-10-31: the tool prints the plan, with current values and exact changes, and applies at once; `make release-plan` stays the read-only dry run and its header says so; `release-rule-test` likewise), the R1 edit through the Admin API, the ruleset pin moves, and a read-back of all three that must show only the SHA changed. The Console endpoints refuse Admin API keys, so the tool logs in with `ant auth login --scope org:admin` in a private temp config dir and logs out and deletes it on every exit; no admin token is stored between releases. The tool never runs in CI: it holds an Anthropic admin login and GitHub write at once. Live check on 2026-10-31: `make release-plan` at `96f4966` read R1 through `ant organization:federation:rules retrieve` (ant 2.4.0); the GET returns `audience` and `condition` as null, which the tool leaves out of the POST. The POST was tested live on 2026-10-31 17:21Z: the owner ran `make release-rule-test RULE=fdrl_01AfACzupW3U4Bx361ZUbsyR SHA=96f496603d968121609abcda0e10666871de2ddf`; scratch rule R2 (`claude-review-p3-r2`) moved from `1cefe4259bc26197ec4b063e8399ef856efdd4f2` to `96f4966`, the read-back showed only `job_workflow_ref` changed, and the logout and temp dir removal ran. The first production `make release` is still untested live. diff --git a/docs/continuation-comments-plan.md b/docs/continuation-comments-plan.md index 9dcbdf4..0a78582 100644 --- a/docs/continuation-comments-plan.md +++ b/docs/continuation-comments-plan.md @@ -31,7 +31,7 @@ All line numbers are at main `03e1b8d`. **The caps do not bound the input.** A pass over its cap adds a not-waivable `pass-records` reason and still keeps every finding ([verdict.py:950-957](../review/src/oc_review/verdict.py), [verdict.py:50](../review/src/oc_review/verdict.py)). So 35 is the maximum for passes within their caps, not the maximum input of the publisher. Such a run fails at the gate in any case. -**What the user sees.** The sticky comment becomes the notice `## 🔴 FAIL: OC Claude review could not publish` with a `[!CAUTION]` callout, the computed gate result, the snapshot, a count per reason and the number of valid overrides ([publish.py:1854-1884](../review/src/oc_review/publish.py)). The required run fails, so the merge box blocks, also when the computed result was `pass`. A low or medium bullet that was cut shows only the truncation note; its explanation is in the run log only. +**What the user sees.** The sticky comment becomes the notice `## 🔴 Comms down: Claude of Duty could not post its full report` with a `[!CAUTION]` callout, the computed gate result, the snapshot, a count per reason and the number of valid overrides ([publish.py:1854-1884](../review/src/oc_review/publish.py)). The required run fails, so the merge box blocks, also when the computed result was `pass`. A low or medium bullet that was cut shows only the truncation note; its explanation is in the run log only. **Writes today.** The gate job may write only a `COMMENT` review, a new issue comment, and a PATCH of an issue comment ([publish.py:216-220](../review/src/oc_review/publish.py), [publish.py:270-277](../review/src/oc_review/publish.py)). Writes are never retried ([publish.py:229-230](../review/src/oc_review/publish.py)), and a write sends no precondition header, so a PATCH cannot be made conditional on the current body ([publish.py:278-284](../review/src/oc_review/publish.py)). A write can raise after GitHub accepted it, for example a timeout while the response is read ([publish.py:288-304](../review/src/oc_review/publish.py)). The order is inline review, summary, final check, exit code, with a snapshot check before each write ([publish.py:2194-2259](../review/src/oc_review/publish.py)). A snapshot check is three reads with a valid bundle (PR, base tip, compare) and one read without ([publish.py:550-567](../review/src/oc_review/publish.py)). Each read has a 30 second timeout and at most 2 retries on a 429, a 5xx or a network error ([collect.py:76-77](../review/src/oc_review/collect.py), [collect.py:240-268](../review/src/oc_review/collect.py)). The gate job has `contents: read`, `pull-requests: write`, `issues: write` and `actions: read`, no Anthropic credential and a 10-minute timeout ([claude-review.yml:228-237](../.github/workflows/claude-review.yml)). Runs on one PR are not serialized: the workflow has no `concurrency` ([claude-review.yml:8](../.github/workflows/claude-review.yml)). @@ -204,10 +204,11 @@ Each outcome is defined, so no failure reads as an empty PR (review B, Important ``` owner marker, first line - ### OC Claude review: findings in full, part of , run attempt + ### Claude of Duty: findings in full, part of , run attempt Run [, attempt ](run URL). The current result is in the [summary comment](link). A part of another run than the one the summary names is out of date. - The required workflow in the merge box is the result that counts. This bot never approves a PR. + The required workflow in the merge box is the result that counts. + *Claude of Duty never approves a PR. A human reviewer gives the final approval.* > **Model output from untrusted input. Check it before you act on it.** Verified means ... one per bullet - diff --git a/review/src/oc_review/publish.py b/review/src/oc_review/publish.py index af2dd15..c5c068f 100644 --- a/review/src/oc_review/publish.py +++ b/review/src/oc_review/publish.py @@ -848,7 +848,7 @@ def build_review( if not comments: return None, outcome body = ( - f"OC Claude review, run {result.run_id}: {len(comments)} inline finding(s). " + f"Claude of Duty review, run {result.run_id}: {len(comments)} inline finding(s). " f"{UNTRUSTED_NOTE} The gate result is in the summary comment." ) request = {"commit_id": head_sha, "event": REVIEW_EVENT, "body": body, "comments": comments} @@ -1325,14 +1325,22 @@ def _model_rows(rows: list[_Row]) -> list[_Row]: return [r for r in rows if r.is_model] -def _status_heading(result: verdict.GateResult, model_rows: list[_Row]) -> str: +MISSION_LINE = "*Mission: Review the code. Protect the branch. Deploy the future.*" +APPROVAL_NOTE = "*Claude of Duty never approves a PR. A human reviewer gives the final approval.*" + + +def _status_heading(result: verdict.GateResult, model_rows: list[_Row], rows: list[_Row]) -> str: + """Display text only (D8). `rows` is the findings table, so a FAIL count + equals the rows the body lists; no row means no number.""" if result.status != verdict.FAIL: if model_rows and not result.override_applied: - return f"## 🟡 PASS: OC Claude review, {_n(len(model_rows), 'finding')} to consider" - return "## 🟢 PASS: OC Claude review" + return f"## 🟡 Clear to proceed: Claude of Duty logged {_n(len(model_rows), 'item')} to consider" + return "## 🟢 All clear: Claude of Duty review passed" if result.codes & _INCOMPLETE_CODES: - return "## 🔴 FAIL: OC Claude review did not complete" - return "## 🔴 FAIL: OC Claude review" + return "## 🔴 Comms down: the Claude of Duty review did not complete" + if rows: + return f"## 🔴 Hold deployment: Claude of Duty identified {_n(len(rows), 'item')} requiring attention" + return "## 🔴 Hold deployment: Claude of Duty identified items requiring attention" def _blocking_phrase(model_rows: list[_Row]) -> str: @@ -1403,7 +1411,7 @@ def _callout(result: verdict.GateResult, rows: list[_Row], hard: list[_Row], has first = f"**This review is out of date. No waiver can clear this.**{also}" nxt = "**Next step:** read the newest run in the merge box." elif verdict.R_SIZE_GATE in codes: - first = "**The change is too large for the model review. A maintainer can waive this.** File checks ran, model checks did not." + first = "**Out of range.** This change is too large for the model review. File checks ran, but model checks did not. A maintainer can waive this." nxt = '**Next step for the author:** split the change, or ask a maintainer for a waiver (under "Waiver" below).' else: phrase = _blocking_phrase(model) @@ -1465,10 +1473,10 @@ def _provenance(ctx: PublishContext, result: verdict.GateResult, truncated: bool where = f"for head `{snap.head_sha[:7]}` into {_esc('ref', snap.base_ref)}." else: where = f"for event head `{ctx.head_sha[:7]}`, merge base unknown." - text = f"{run} {where} The required workflow in the merge box is the result that counts. This bot never approves a PR." + text = f"{run} {where} The required workflow in the merge box is the result that counts." if truncated: text += " Some details were truncated, see the run log." - return _Block(f"\n{text}") + return _Block(f"\n{text}\n\n{APPROVAL_NOTE}") # --------------------------------------------------------------------------- @@ -1860,7 +1868,8 @@ def _overflow_body(ctx: PublishContext, result: verdict.GateResult) -> str: counts[r.code] = counts.get(r.code, 0) + 1 lines = [ verdict.STICKY_MARKER, - "## 🔴 FAIL: OC Claude review could not publish", + "## 🔴 Comms down: Claude of Duty could not post its full report", + MISSION_LINE, "", "> [!CAUTION]", f"> **The full review does not fit in one comment.** {OVERFLOW_NOTE}", @@ -1882,7 +1891,7 @@ def _overflow_body(ctx: PublishContext, result: verdict.GateResult) -> str: # The full record body is a separate surface from the pass comment, so its # "never approves" sentence is intended. lines.append(f"\nRun [{ctx.run_id}, attempt {ctx.run_attempt}]({ctx.run_url}). " - "The required workflow in the merge box is the result that counts. This bot never approves a PR.") + "The required workflow in the merge box is the result that counts.\n\n" + APPROVAL_NOTE) return "\n".join(lines) @@ -1901,7 +1910,7 @@ def _summary_blocks( model = _model_rows(rows) b = [ _Block(verdict.STICKY_MARKER), - _Block(_status_heading(result, model)), + _Block(_status_heading(result, model, rows) + "\n" + MISSION_LINE), _Block("\n" + _callout(result, rows, hard, payload is not None)), ] b += _findings_table(rows) diff --git a/review/tests/unit/fixtures/workflows/edits.yml b/review/tests/unit/fixtures/workflows/edits.yml index 2aa5e9e..8a2758f 100644 --- a/review/tests/unit/fixtures/workflows/edits.yml +++ b/review/tests/unit/fixtures/workflows/edits.yml @@ -140,8 +140,8 @@ expect: continue-on-error - id: continue-on-error-job file: claude-review.yml - old: " analyze:\n needs: collect\n" - new: " analyze:\n needs: collect\n continue-on-error: true\n" + old: " analyze:\n name: Analysis\n needs: collect\n" + new: " analyze:\n name: Analysis\n needs: collect\n continue-on-error: true\n" expect: continue-on-error # --- central workflow: trigger, permissions, concurrency, jobs --- @@ -222,20 +222,20 @@ expect: needs - id: environment-in-central file: claude-review.yml - old: " collect:\n runs-on: ubuntu-24.04\n" - new: " collect:\n runs-on: ubuntu-24.04\n environment: production\n" + old: " collect:\n name: Recon\n runs-on: ubuntu-24.04\n" + new: " collect:\n name: Recon\n runs-on: ubuntu-24.04\n environment: production\n" expect: environment - id: eval-environment-in-central file: claude-review.yml - old: " oc-claude-review:\n needs: [collect, analyze, dependency-review]\n" - new: " oc-claude-review:\n needs: [collect, analyze, dependency-review]\n environment: eval\n" + old: " oc-claude-review:\n name: Mission report\n needs: [collect, analyze, dependency-review]\n" + new: " oc-claude-review:\n name: Mission report\n needs: [collect, analyze, dependency-review]\n environment: eval\n" expect: [eval-environment, environment, eval-permissions] # --- central workflow: if --- - id: job-if-on-analyze file: claude-review.yml - old: " analyze:\n needs: collect\n" - new: " analyze:\n needs: collect\n if: always()\n" + old: " analyze:\n name: Analysis\n needs: collect\n" + new: " analyze:\n name: Analysis\n needs: collect\n if: always()\n" expect: job-if - id: gate-if-changed file: claude-review.yml diff --git a/review/tests/unit/test_publish.py b/review/tests/unit/test_publish.py index df8d339..98ad3dc 100644 --- a/review/tests/unit/test_publish.py +++ b/review/tests/unit/test_publish.py @@ -384,7 +384,7 @@ def test_clean_run_passes_and_posts_summary(tmp_path): assert not r.gh.bodies("POST", "/reviews") s = r.summary assert s.startswith(verdict.STICKY_MARKER) - assert "## 🟢 PASS: OC Claude review" in s + assert "## 🟢 All clear: Claude of Duty review passed" in s assert r.record["result"] == "pass" @@ -419,7 +419,7 @@ def test_summary_has_every_required_field(tmp_path): "| Review | 12000 | 900 | 11000 | 0 | 1 |", "| **Total** | 36000 | 2700 | 33000 | 0 | 3 |", "The required workflow in the merge box is the result that counts.", - "This bot never approves a PR.", + "Claude of Duty never approves a PR. A human reviewer gives the final approval.", ): assert text in s, text @@ -971,7 +971,7 @@ def many(cat, sev, n, quote="shell=True", line=10): assert len(s) < publish.SUMMARY_MAX_CHARS assert publish.TRUNCATED_NOTE in s # Never cut: result, reasons, SHAs. - assert "## 🔴 FAIL: OC Claude review" in s and f"`{HEAD}`" in s + assert "## 🔴 Hold deployment: Claude of Duty identified" in s and f"`{HEAD}`" in s assert has_reason(s, "blocking-finding", True) # Cut first: unverified findings; high findings stay. assert "low bug 0" not in s @@ -1073,7 +1073,7 @@ def test_critical_findings_with_a_cut_path_are_never_cut_from_the_summary(tmp_pa r = run(tmp_path, passes=passes) assert r.code == 1 and r.gh.bodies("POST", "/reviews") == [] s = plain(r.summary) - assert publish.OVERFLOW_NOTE in s and "could not publish" in s + assert publish.OVERFLOW_NOTE in s and "could not post its full report" in s assert not any(c in s for c in claims(passes)) # no partial list assert r.record["result"] == "fail" and r.record["publish_error"] rec = full_record_from_log(capsys.readouterr().out) @@ -1112,7 +1112,7 @@ def test_reasons_that_do_not_fit_fail_closed(tmp_path, capsys): s = r.summary assert r.code == 1 assert len(s) < publish.SUMMARY_MAX_CHARS - assert "## 🔴 FAIL: OC Claude review" in s and publish.OVERFLOW_NOTE in s + assert "## 🔴 Comms down: Claude of Duty could not post its full report" in s and publish.OVERFLOW_NOTE in s assert f"| `{HEAD}` | `{MB}` | main |" in s assert "| Hard block from the file checks | 🚫 no | 400 reason(s) |" in s # No partial reason list: either every reason or none. @@ -1190,7 +1190,7 @@ def push(gh, req): kw["passes"] = clean_passes(supply_chain=[BLOCKING]) r = run(tmp_path, gh, **kw) assert r.code == 1 - assert len(r.gh.summary_bodies()) == 1 and "## 🟢 PASS: OC Claude review" in r.summary + assert len(r.gh.summary_bodies()) == 1 and "## 🟢 All clear: Claude of Duty review passed" in r.summary assert r.gh.paths("PATCH") == [] assert "stale" in r.record["reasons"] @@ -1267,7 +1267,7 @@ def delete_grant(gh, req): r = run(tmp_path, gh, passes=clean_passes(supply_chain=[BLOCKING])) assert r.code == 1 assert len(r.gh.summary_bodies()) == 2 # pass summary, then the update to fail - assert "## 🔴 FAIL: OC Claude review" in r.summary + assert "\n## 🔴 Hold deployment: Claude of Duty identified 1 item requiring attention\n" in "\n" + r.summary # ---------------------------------------------------------------- final check @@ -1278,8 +1278,8 @@ def test_newer_run_supersedes_and_updates_summary(tmp_path): r = run(tmp_path, gh) assert r.code == 1 first, last = r.gh.summary_bodies() - assert "## 🟢 PASS: OC Claude review" in first - assert "## 🔴 FAIL: OC Claude review" in last and has_reason(last, "superseded", False) + assert "## 🟢 All clear: Claude of Duty review passed" in first + assert "## 🔴 Hold deployment: Claude of Duty identified" in last and has_reason(last, "superseded", False) assert r.gh.paths("PATCH") # the update edits the comment the run posted @@ -1358,6 +1358,10 @@ def test_size_gate_records_show_reason(tmp_path): passes={n: pass_doc(n, "skipped-size") for n in verdict.PASS_NAMES}) assert reason_codes(r.summary) == {"size-gate"} assert "too large for automatic review" in plain(r.summary) + assert ("**Out of range.** This change is too large for the model review. File checks ran, " + "but model checks did not. A maintainer can waive this.") in r.summary + # the size gate has one reason row and no finding row, so the count is 1 or absent but never wrong + assert "\n## 🔴 Hold deployment: Claude of Duty identified 1 item requiring attention\n" in "\n" + r.summary def test_missing_needs_and_outcome_fail_with_summary(tmp_path): diff --git a/review/tests/unit/test_release_tool.py b/review/tests/unit/test_release_tool.py new file mode 100644 index 0000000..31dfbad --- /dev/null +++ b/review/tests/unit/test_release_tool.py @@ -0,0 +1,919 @@ +"""Owner-run release tool (review/tools/release.py). No network: every gh and +ant call goes to a fake runner.""" + +import base64 +import copy +import importlib.util +import json +import os +import signal +import subprocess +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[3] +_spec = importlib.util.spec_from_file_location("release_tool", ROOT / "review" / "tools" / "release.py") +release = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(release) + +SHA_OLD = "96f496603d968121609abcda0e10666871de2ddf" +SHA_NEW = "3855f75" + "0" * 33 +R1 = release.PROD_RULE_ID +R2 = "fdrl_01AfACzupW3U4Bx361ZUbsyR" +REF = release.JOB_WORKFLOW_REF_PREFIX + + +def rule(rule_id=R1, sha=SHA_OLD, prefix=REF): + return { + "applies_to_all_workspaces": False, "archived_at": None, "description": None, + "id": rule_id, "issuer_id": "fdis_x", "name": "claude-review-prod-r1", + "match": { + "audience": None, "condition": None, "subject_prefix": "repo:OperationCode@10479364/*", + "claims": {"event_name": "pull_request_target", "job_workflow_ref": f"{prefix}{sha}", + "repository_owner_id": "10479364"}, + }, + "oauth_scope": "workspace:developer", + "target": {"service_account_id": "svac_x", "type": "service_account"}, + "token_lifetime_seconds": 600, "updated_at": "t0", "updated_by_actor_id": "user_a", + "workspace_id": "wrkspc_x", "workspace_ids": ["wrkspc_x"], + } + + +def ruleset(rid=26132750, sha=SHA_OLD): + return { + "id": rid, "name": "claude-review-pilot", "target": "branch", "enforcement": "active", + "conditions": {"ref_name": {"exclude": [], "include": ["~DEFAULT_BRANCH"]}, + "repository_id": {"repository_ids": [107068150, 135777271]}}, + "rules": [{"type": "workflows", "parameters": {"do_not_enforce_on_create": False, "workflows": [ + {"repository_id": release.WORKFLOW_REPO_ID, "path": release.WORKFLOW_PATH, "sha": sha}]}}], + "bypass_actors": [{"actor_id": None, "actor_type": "OrganizationAdmin", "bypass_mode": "pull_request"}], + "updated_at": "t0", + } + + +# ---------------------------------------------------------------- SHA and IDs + + +@pytest.mark.parametrize("sha", [SHA_OLD, "0" * 40]) +def test_validate_sha_accepts_40_hex(sha): + assert release.validate_sha(sha) == sha + + +@pytest.mark.parametrize("sha", [None, "", SHA_OLD[:39], SHA_OLD + "0", SHA_OLD.upper(), " " + SHA_OLD, + "g" * 40, "main", "refs/heads/main"]) +def test_validate_sha_refuses_anything_else(sha): + with pytest.raises(release.ReleaseError): + release.validate_sha(sha) + + +@pytest.mark.parametrize("rid", [None, "", "fdrl_short", "abc_01AfACzupW3U4Bx361ZUbsyR", R2 + "x"]) +def test_validate_rule_id_refuses(rid): + with pytest.raises(release.ReleaseError): + release.validate_rule_id(rid) + + +# ---------------------------------------------------------------- match patch + + +def test_patch_match_changes_only_the_sha_and_keeps_every_other_field(): + before = rule() + new = release.patch_match(before["match"], SHA_NEW, prefix=REF) + assert new["claims"]["job_workflow_ref"] == f"{REF}{SHA_NEW}" + assert release.diff_paths(before["match"], new) == ["claims.job_workflow_ref"] + assert new["subject_prefix"] == before["match"]["subject_prefix"] + assert {k: v for k, v in new["claims"].items() if k != "job_workflow_ref"} == { + "event_name": "pull_request_target", "repository_owner_id": "10479364"} + assert before["match"]["claims"]["job_workflow_ref"] == f"{REF}{SHA_OLD}" # input not mutated + + +def test_patch_match_drops_null_matchers_and_keeps_set_ones(): + m = rule()["match"] + assert "audience" not in release.patch_match(m, SHA_NEW) and "condition" not in release.patch_match(m, SHA_NEW) + m["audience"] = "https://api.anthropic.com" + m["claims"]["extra"] = "x" + new = release.patch_match(m, SHA_NEW) + assert new["audience"] == "https://api.anthropic.com" and new["claims"]["extra"] == "x" + + +def test_patch_match_without_prefix_keeps_a_scratch_path(): + m = rule(prefix="OperationCode/claude-review-test/.github/workflows/v.yml@")["match"] + new = release.patch_match(m, SHA_NEW) + assert new["claims"]["job_workflow_ref"] == f"OperationCode/claude-review-test/.github/workflows/v.yml@{SHA_NEW}" + + +@pytest.mark.parametrize("mutate", [ + lambda m: m["claims"].update(job_workflow_ref="OperationCode/other/.github/workflows/x.yml@" + SHA_OLD), + lambda m: m["claims"].update(job_workflow_ref=REF + "refs/heads/main"), + lambda m: m["claims"].update(job_workflow_ref="no-at-sign"), + lambda m: m["claims"].pop("job_workflow_ref"), + lambda m: m.pop("claims"), +]) +def test_patch_match_refuses_an_unexpected_rule(mutate): + m = rule()["match"] + mutate(m) + with pytest.raises(release.ReleaseError): + release.patch_match(m, SHA_NEW, prefix=REF) + + +def test_patch_match_refuses_no_match(): + with pytest.raises(release.ReleaseError): + release.patch_match(None, SHA_NEW) + + +# ---------------------------------------------------------------- rule read-back + + +def _after_rule(sha=SHA_NEW): + after = rule(sha=sha) + after["updated_at"] = "t1" + after["updated_by_actor_id"] = "user_b" + del after["match"]["audience"] # missing equals null + return after + + +def test_compare_rule_passes_when_only_the_ref_changed(): + before = rule() + expected = release.patch_match(before["match"], SHA_NEW, prefix=REF) + assert release.compare_rule(before, _after_rule(), expected) == [] + + +@pytest.mark.parametrize("mutate,where", [ + (lambda r: r["match"]["claims"].update(event_name="push"), "match.claims.event_name"), + (lambda r: r["match"]["claims"].pop("repository_owner_id"), "match.claims.repository_owner_id"), + (lambda r: r["match"]["claims"].update(repository="x"), "match.claims.repository"), + (lambda r: r["match"].update(subject_prefix="repo:*"), "match.subject_prefix"), + (lambda r: r["match"].update(condition="true"), "match.condition"), + (lambda r: r["match"]["claims"].update(job_workflow_ref=REF + SHA_OLD), "match.claims.job_workflow_ref"), + (lambda r: r.update(name="other"), "name"), + (lambda r: r.update(oauth_scope="org:admin"), "oauth_scope"), + (lambda r: r.update(token_lifetime_seconds=3600), "token_lifetime_seconds"), + (lambda r: r.update(workspace_ids=["wrkspc_x", "wrkspc_y"]), "workspace_ids"), + (lambda r: r.update(applies_to_all_workspaces=True), "applies_to_all_workspaces"), + (lambda r: r["target"].update(service_account_id="svac_y"), "target.service_account_id"), + (lambda r: r.update(archived_at="t2"), "archived_at"), +]) +def test_compare_rule_detects_any_other_change(mutate, where): + before = rule() + expected = release.patch_match(before["match"], SHA_NEW, prefix=REF) + after = _after_rule() + mutate(after) + assert where in " ".join(release.compare_rule(before, after, expected)) + + +# ---------------------------------------------------------------- ruleset patch and read-back + + +def test_patch_ruleset_changes_only_the_pin(): + before = ruleset() + body = release.patch_ruleset(before, SHA_NEW) + assert set(body) == {"rules"} + assert release.diff_paths(before["rules"], body["rules"]) == ["[0].parameters.workflows[0].sha"] + assert before["rules"][0]["parameters"]["workflows"][0]["sha"] == SHA_OLD # input not mutated + + +def test_patch_ruleset_keeps_other_rules_and_entries(): + before = ruleset() + before["rules"].insert(0, {"type": "deletion"}) + before["rules"][1]["parameters"]["workflows"].append( + {"repository_id": 1, "path": ".github/workflows/other.yml", "sha": SHA_OLD}) + body = release.patch_ruleset(before, SHA_NEW) + assert release.diff_paths(before["rules"], body["rules"]) == ["[1].parameters.workflows[0].sha"] + + +@pytest.mark.parametrize("mutate", [ + lambda rs: rs["rules"][0]["parameters"]["workflows"][0].update(ref="refs/heads/main"), + lambda rs: rs["rules"][0]["parameters"]["workflows"].append(copy.deepcopy(rs["rules"][0]["parameters"]["workflows"][0])), + lambda rs: rs["rules"][0]["parameters"]["workflows"].clear(), + lambda rs: rs["rules"].clear(), + lambda rs: rs["rules"][0]["parameters"]["workflows"][0].pop("sha"), + lambda rs: rs["rules"][0]["parameters"]["workflows"][0].update(repository_id=1432457406), +]) +def test_patch_ruleset_refuses_unexpected_shape(mutate): + rs = ruleset() + mutate(rs) + with pytest.raises(release.ReleaseError): + release.patch_ruleset(rs, SHA_NEW) + + +def test_compare_ruleset_passes_when_only_the_pin_moved(): + after = ruleset(sha=SHA_NEW) + after["updated_at"] = "t1" + assert release.compare_ruleset(ruleset(), after, SHA_NEW) == [] + + +@pytest.mark.parametrize("mutate", [ + lambda rs: rs.update(enforcement="disabled"), + lambda rs: rs["bypass_actors"].append({"actor_id": 5, "actor_type": "RepositoryRole", "bypass_mode": "always"}), + lambda rs: rs["conditions"]["repository_id"]["repository_ids"].append(1), + lambda rs: rs["conditions"]["ref_name"]["include"].append("refs/heads/x"), + lambda rs: rs["rules"].append({"type": "deletion"}), + lambda rs: rs["rules"][0]["parameters"].update(do_not_enforce_on_create=True), + lambda rs: rs["rules"][0]["parameters"]["workflows"][0].update(sha=SHA_OLD), + lambda rs: rs["rules"][0]["parameters"]["workflows"][0].update(ref="refs/heads/main"), + lambda rs: rs["rules"][0]["parameters"]["workflows"][0].update(path=".github/workflows/x.yml"), + lambda rs: rs.update(name="renamed"), +]) +def test_compare_ruleset_detects_any_other_change(mutate): + after = ruleset(sha=SHA_NEW) + mutate(after) + assert release.compare_ruleset(ruleset(), after, SHA_NEW) + + +def test_redact_hides_bearer_and_keys(): + out = release.redact('authorization: Bearer abc.def-123 and sk-ant-oat01-xyz"') + assert "abc.def" not in out and "oat01-xyz" not in out and out.count("[redacted]") == 2 + + +# ---------------------------------------------------------------- end to end with a fake runner + + +ANALYZE = f'ANTHROPIC_FEDERATION_RULE_ID = "{R1}"\n' + + +def good_run(wf, sha, **over): + event = "push" if wf == "test.yml" else "workflow_dispatch" + run = {"id": 1, "path": f".github/workflows/{wf}", "event": event, "head_branch": "main", + "head_sha": sha, "status": "completed", "conclusion": "success"} + run.update(over) + return run + + +class Fake: + """gh and ant stand-in. Records each call; keeps rule and ruleset state.""" + + def __init__(self, *, sha=SHA_NEW, ant_installed=True, help_text="retrieve update list", + analyze=ANALYZE, test_ok=True, compare_status="identical", role="admin", + rule_after=None, login_raises=None, runs=None, logout=None, put=None, on_retrieve=None): + self.calls: list[tuple[list[str], dict]] = [] + self.rules = {R1: rule(R1), R2: rule(R2, prefix="OperationCode/claude-review-test/.github/workflows/v.yml@")} + self.rulesets = {rid: ruleset(rid) for rid, _ in release.RULESETS} + self.ant_installed, self.help_text, self.analyze = ant_installed, help_text, analyze + self.test_ok, self.compare_status, self.role = test_ok, compare_status, role + self.rule_after, self.login_raises = rule_after, login_raises + self.sha, self.runs, self.logout, self.put = sha, runs, logout, put + self.config_dirs: set[str] = set() + self.on_retrieve, self.retrieves = on_retrieve, {} + + def __call__(self, cmd, **kw): + self.calls.append((list(cmd), kw)) + if cmd[0] == "ant": + return self.ant(cmd[1:], kw) + assert cmd[:2] == ["gh", "api"] + return self.gh(cmd, kw) + + @staticmethod + def ok(obj=None, rc=0, stderr=""): + return subprocess.CompletedProcess([], rc, json.dumps(obj) if obj is not None else "", stderr) + + def ant(self, args, kw): + if not self.ant_installed: + raise FileNotFoundError("ant") + env = kw["env"] + assert not any(k.startswith("ANTHROPIC_") and k != "ANTHROPIC_CONFIG_DIR" for k in env) + self.config_dirs.add(env["ANTHROPIC_CONFIG_DIR"]) + if args[-1] == "--help": + return subprocess.CompletedProcess([], 0, self.help_text, "") + if args[:2] == ["auth", "login"]: + assert "capture_output" not in kw # interactive: owner sees the URL + cred = Path(env["ANTHROPIC_CONFIG_DIR"]) / "credentials" / "admin.json" + cred.parent.mkdir(parents=True, exist_ok=True) + cred.write_text('{"access_token": "synthetic", "refresh_token": "synthetic"}') + if self.login_raises: + raise self.login_raises + return self.ok() + if args[:2] == ["auth", "logout"]: + if self.logout: + return self.logout(env) + return self.ok() + rid = args[args.index("--federation-rule-id") + 1] + if args[1] == "retrieve": + self.retrieves[rid] = self.retrieves.get(rid, 0) + 1 + if self.on_retrieve: + self.on_retrieve(self, rid, self.retrieves[rid]) + return self.ok(self.rules[rid]) + if args[1] == "update": + new = copy.deepcopy(self.rules[rid]) + new["match"] = json.loads(args[args.index("--match") + 1]) + new["updated_at"] = "t1" + if self.rule_after: + self.rule_after(new) + self.rules[rid] = new + return self.ok(new) + raise AssertionError(args) + + def gh(self, cmd, kw): + path = cmd[-1] if "-X" not in cmd else cmd[cmd.index("--input") - 1] + if "-X" in cmd: + assert cmd[cmd.index("-X") + 1] == "PUT" + rid = int(path.rsplit("/", 1)[1]) + new = copy.deepcopy(self.rulesets[rid]) + new.update(json.loads(kw["input"])) + new["updated_at"] = "t1" + if self.put: + return self.put(rid, new, self) + self.rulesets[rid] = new + return self.ok(new) + if "/compare/" in path: + return self.ok({"status": self.compare_status, "ahead_by": 0}) + if "/actions/workflows/" in path: + wf = path.split("/actions/workflows/")[1].split("/")[0] + if self.runs is not None: + return self.ok({"workflow_runs": self.runs.get(wf, [])}) + concl = "success" if self.test_ok or wf != "test.yml" else "failure" + return self.ok({"workflow_runs": [good_run(wf, self.sha, conclusion=concl)]}) + if "/contents/" in path: + return self.ok({"content": base64.b64encode(self.analyze.encode()).decode()}) + if path == "user": + return self.ok({"login": "owner"}) + if "/memberships/" in path: + return self.ok({"role": self.role}) + if "/rulesets/" in path: + return self.ok(self.rulesets[int(path.rsplit("/", 1)[1])]) + raise AssertionError(path) + + def writes(self): + return [c for c, _ in self.calls if ("-X" in c) or (len(c) > 2 and c[2] == "update")] + + +@pytest.fixture(autouse=True) +def _no_real_subprocess(monkeypatch): + """No test may start a real gh or ant (no browser login, no API call).""" + def refuse(*a, **kw): + raise AssertionError(f"real subprocess call in a unit test: {a!r}") + monkeypatch.setattr(release.subprocess, "run", refuse) + monkeypatch.setattr(release.subprocess, "Popen", refuse) + + +PLANTED_ENV = ("ANTHROPIC_API_KEY", "ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_BASE_URL", "ANTHROPIC_PROFILE") + + +def _repo_records(): + real = ROOT / "release-records" + return sorted(str(p) for p in real.rglob("*")) if real.exists() else [] + + +@pytest.fixture(autouse=True) +def _records(tmp_path, monkeypatch): + """Hermetic: records and every temp dir go under this test's tmp_path, + and the repo's release-records/ must not change.""" + before = _repo_records() + monkeypatch.setattr(release, "RECORDS", tmp_path / "records") + monkeypatch.setattr(release.tempfile, "tempdir", str(tmp_path / "tmp")) + (tmp_path / "tmp").mkdir() + # Planted before the test runs: ant must never see them (Fake.ant checks). + for k in PLANTED_ENV: + monkeypatch.setenv(k, "must-not-reach-ant") + yield + assert _repo_records() == before, "a test wrote to the repo's release-records/" + + +def change_r1_on_second_read(fake, rid, n): + if rid == R1 and n == 2: # the read just before the apply + fake.rules[R1]["name"] = "changed meanwhile" + + +def test_apply_runs_without_reading_stdin(monkeypatch, capsys): + class NoStdin: + def read(self, *a): + raise AssertionError("stdin was read") + + readline = readlines = read + + def isatty(self): + return False + + monkeypatch.setattr(release.sys, "stdin", NoStdin()) + monkeypatch.setattr("builtins.input", lambda *a: (_ for _ in ()).throw(AssertionError("input() called"))) + fake = Fake() + assert release.main(["release", "--sha", SHA_NEW], run=fake) == 0 + out = capsys.readouterr().out + assert fake.writes() != [] and "no prompt" in out and "Apply?" not in out + + +def test_rule_test_applies_without_reading_stdin(monkeypatch): + monkeypatch.setattr("builtins.input", lambda *a: (_ for _ in ()).throw(AssertionError("input() called"))) + fake = Fake() + assert release.main(["rule-test", "--rule", R2, "--sha", SHA_NEW], run=fake) == 0 + assert fake.writes() != [] + + +def test_plan_header_says_read_only_dry_run(capsys): + assert release.main(["plan", "--sha", SHA_NEW], run=Fake()) == 0 + assert "read-only dry run" in capsys.readouterr().out + + +def test_release_applies_console_first_then_pins_and_cleans_up(capsys): + fake = Fake() + rc = release.main(["release", "--sha", SHA_NEW], run=fake) + assert rc == 0, capsys.readouterr() + writes = fake.writes() + assert writes[0][0] == "ant" and writes[0][2] == "update" + assert [w[w.index("--input") - 1] for w in writes[1:]] == [f"orgs/OperationCode/rulesets/{rid}" for rid, _ in release.RULESETS] + assert fake.rules[R1]["match"]["claims"]["job_workflow_ref"] == REF + SHA_NEW + assert all(release.ruleset_pin(rs) == SHA_NEW for rs in fake.rulesets.values()) + logout = [c for c, _ in fake.calls if c[:3] == ["ant", "auth", "logout"]] + assert logout == [["ant", "auth", "logout", "--all"]] + assert fake.calls[-1][0][:3] != ["ant", "auth", "login"] + assert all(not os.path.exists(d) for d in fake.config_dirs) + recs = list((release.RECORDS).rglob("*.json")) + names = {p.name for p in recs} + assert f"rule-{R1}-before.json" in names and f"rule-{R1}-after.json" in names + assert all(f"ruleset-{rid}-after.json" in names for rid, _ in release.RULESETS) + assert not any("must-not-reach-ant" in p.read_text() for p in recs) + + +def test_release_rerun_at_the_same_sha_writes_nothing(): + fake = Fake() + fake.rules[R1] = rule(sha=SHA_NEW) + fake.rulesets = {rid: ruleset(rid, SHA_NEW) for rid, _ in release.RULESETS} + assert release.main(["release", "--sha", SHA_NEW], run=fake) == 0 + assert fake.writes() == [] + + +@pytest.mark.parametrize("kw,msg", [ + (dict(test_ok=False), "no successful test.yml push run on main"), + (dict(compare_status="diverged"), "is not on main"), + (dict(analyze='ANTHROPIC_FEDERATION_RULE_ID = "fdrl_000000000000000000000000"\n'), "names rule"), + (dict(role="member"), "needs an org owner"), +]) +def test_release_refuses_on_a_failed_precondition_before_any_console_call(kw, msg, capsys): + fake = Fake(**kw) + rc = release.main(["release", "--sha", SHA_NEW], run=fake) + assert rc == 2 and fake.writes() == [] + assert not any(c[0] == "ant" for c, _ in fake.calls) + out = capsys.readouterr() + assert msg in out.out and "precondition(s) failed" in out.err + + +def test_release_refuses_a_ruleset_with_a_ref(): + fake = Fake() + fake.rulesets[25835887]["rules"][0]["parameters"]["workflows"][0]["ref"] = "refs/heads/main" + assert release.main(["release", "--sha", SHA_NEW], run=fake) == 2 + assert fake.writes() == [] + + +def test_release_stops_before_the_pins_when_the_rule_reads_back_wrong(capsys): + fake = Fake(rule_after=lambda r: r.update(token_lifetime_seconds=3600)) + rc = release.main(["release", "--sha", SHA_NEW], run=fake) + assert rc == 2 + assert "token_lifetime_seconds" in capsys.readouterr().err + assert not any("-X" in c for c in fake.writes()) + + +def test_release_refuses_a_rule_changed_since_the_plan_read(): + fake = Fake(on_retrieve=change_r1_on_second_read) + + assert release.main(["release", "--sha", SHA_NEW], run=fake) == 2 + assert fake.writes() == [] + + +def test_release_refuses_no_console(): + fake = Fake() + assert release.main(["release", "--sha", SHA_NEW, "--no-console"], run=fake) == 2 + assert fake.writes() == [] + + +def test_plan_is_read_only_and_logs_out(capsys): + fake = Fake() + assert release.main(["plan", "--sha", SHA_NEW], run=fake) == 0 + assert fake.writes() == [] + out = capsys.readouterr().out + assert f"{SHA_OLD} -> {SHA_NEW}" in out and "Plan OK" in out + assert any(c[:3] == ["ant", "auth", "logout"] for c, _ in fake.calls) + + +def test_plan_no_console_makes_no_ant_call(): + fake = Fake() + assert release.main(["plan", "--sha", SHA_NEW, "--no-console"], run=fake) == 0 + assert not any(c[0] == "ant" for c, _ in fake.calls) + + +def test_plan_without_ant_fails_closed_with_a_clear_message(capsys): + fake = Fake(ant_installed=False) + assert release.main(["plan", "--sha", SHA_NEW], run=fake) == 2 + assert "ant is not installed" in capsys.readouterr().err + assert fake.writes() == [] + + +def test_plan_refuses_an_ant_without_the_rule_getter(capsys): + fake = Fake(help_text="list create archive update") + assert release.main(["plan", "--sha", SHA_NEW], run=fake) == 2 + assert "retrieve" in capsys.readouterr().err + assert not any(c[:3] == ["ant", "auth", "login"] for c, _ in fake.calls) + + +def test_bad_sha_makes_no_call(): + fake = Fake() + assert release.main(["release", "--sha", "main"], run=fake) == 2 + assert fake.calls == [] + + +def test_interrupt_during_login_still_logs_out_and_removes_the_dir(): + fake = Fake(login_raises=KeyboardInterrupt()) + assert release.main(["plan", "--sha", SHA_NEW], run=fake) == 130 + assert any(c[:3] == ["ant", "auth", "logout"] for c, _ in fake.calls) + assert fake.config_dirs and all(not os.path.exists(d) for d in fake.config_dirs) + + +def test_config_dir_is_private_during_the_run(): + modes = [] + fake = Fake() + orig = fake.ant + + def ant(args, kw): + if args[:2] == ["auth", "login"]: + modes.append(os.stat(kw["env"]["ANTHROPIC_CONFIG_DIR"]).st_mode & 0o777) + return orig(args, kw) + + fake.ant = ant + release.main(["plan", "--sha", SHA_NEW], run=fake) + assert modes == [0o700] + + +def test_rule_test_edits_only_the_given_rule(): + fake = Fake() + rc = release.main(["rule-test", "--rule", R2, "--sha", SHA_NEW], run=fake) + assert rc == 0 + assert fake.rules[R2]["match"]["claims"]["job_workflow_ref"].endswith("@" + SHA_NEW) + assert fake.rules[R1] == rule(R1) + assert not any(c[0] == "gh" for c, _ in fake.calls) + + +def test_rule_test_refuses_the_production_rule(): + fake = Fake() + assert release.main(["rule-test", "--rule", R1, "--sha", SHA_NEW], run=fake) == 2 + assert fake.calls == [] + + +# ---------------------------------------------------------------- never in CI + + +def test_no_workflow_runs_the_release_tool(): + for path in (ROOT / ".github" / "workflows").glob("*.y*ml"): + text = path.read_text(encoding="utf-8") + assert "release.py" not in text and "make release" not in text and "ant " not in text, path.name + + +def test_default_runner_is_guarded_in_tests(): + with pytest.raises(AssertionError, match="real subprocess"): + release.main(["plan", "--sha", SHA_NEW, "--no-console"]) + + +# ---------------------------------------------------------------- fix round 1: release run checks + + +def _gh_only(fake): + return release.github_preconditions(release.GitHub(fake), SHA_NEW, R1) + + +def test_preconditions_accept_push_and_dispatch_runs_on_main(): + assert _gh_only(Fake(runs={"test.yml": [good_run("test.yml", SHA_NEW)], + "eval.yml": [good_run("eval.yml", SHA_NEW)]})) == [] + + +@pytest.mark.parametrize("wf,over", [ + ("test.yml", {"event": "pull_request", "head_branch": "candidate"}), + ("test.yml", {"event": "pull_request"}), + ("test.yml", {"event": "pull_request_target"}), + ("test.yml", {"head_branch": "candidate"}), + ("test.yml", {"head_sha": SHA_OLD}), + ("test.yml", {"path": ".github/workflows/other.yml"}), + ("test.yml", {"status": "in_progress", "conclusion": None}), + ("eval.yml", {"event": "pull_request", "head_branch": "candidate"}), + ("eval.yml", {"event": "push"}), + ("eval.yml", {"head_branch": "candidate"}), + ("eval.yml", {"head_sha": SHA_OLD}), +]) +def test_preconditions_refuse_a_run_of_the_wrong_kind(wf, over, capsys): + runs = {w: [good_run(w, SHA_NEW)] for w in ("test.yml", "eval.yml")} + runs[wf] = [good_run(wf, SHA_NEW, **over)] + fails = _gh_only(Fake(runs=runs)) + assert len(fails) == 1 and wf in fails[0] + + +def test_release_refuses_when_only_pull_request_runs_passed_at_the_sha(capsys): + pr = {w: [good_run(w, SHA_NEW, event="pull_request", head_branch="candidate")] for w in ("test.yml", "eval.yml")} + fake = Fake(runs=pr) + assert release.main(["release", "--sha", SHA_NEW], run=fake) == 2 + assert fake.writes() == [] and not any(c[0] == "ant" for c, _ in fake.calls) + + +# ---------------------------------------------------------------- fix round 1: credential cleanup + + +def _cred_files(fake): + return [p for d in fake.config_dirs for p in Path(d).rglob("*") if p.is_file()] + + +def test_logout_raising_keyboardinterrupt_leaves_no_credential_file(): + def logout(env): + assert (Path(env["ANTHROPIC_CONFIG_DIR"]) / "credentials" / "admin.json").exists() + raise KeyboardInterrupt() + + fake = Fake(logout=logout) + assert release.main(["plan", "--sha", SHA_NEW], run=fake) == 130 + assert fake.config_dirs and _cred_files(fake) == [] + assert all(not os.path.exists(d) for d in fake.config_dirs) + + +def test_logout_raising_any_exception_leaves_no_credential_file(): + def logout(env): + raise RuntimeError("synthetic") + + fake = Fake(logout=logout) + with pytest.raises(RuntimeError): + release.main(["plan", "--sha", SHA_NEW], run=fake) + assert all(not os.path.exists(d) for d in fake.config_dirs) + + +def test_a_signal_during_cleanup_is_held_until_cleanup_ends(): + seen = [] + + def logout(env): + signal.raise_signal(signal.SIGINT) + seen.append("logout finished") # the signal must not cut logout short + return Fake.ok() + + fake = Fake(logout=logout) + assert release.main(["plan", "--sha", SHA_NEW], run=fake) == 130 + assert seen == ["logout finished"] + assert all(not os.path.exists(d) for d in fake.config_dirs) + assert signal.getsignal(signal.SIGINT) is signal.default_int_handler + + +def test_a_failed_delete_is_loud_and_makes_the_run_fail(monkeypatch, capsys): + real_rmtree = release.shutil.rmtree + dirs = [] + + def no_delete(path, onexc=None): + dirs.append(path) + onexc(os.rmdir, path, PermissionError("synthetic")) + + monkeypatch.setattr(release.shutil, "rmtree", no_delete) + fake = Fake() + rc = release.main(["plan", "--sha", SHA_NEW], run=fake) + err = capsys.readouterr().err + monkeypatch.setattr(release.shutil, "rmtree", real_rmtree) + for d in dirs: + real_rmtree(d, ignore_errors=True) + assert rc == 3 + assert "could not delete" in err and "rm -rf" in err and "CLEANUP FAILED" in err + + +# ---------------------------------------------------------------- fix round 1: partial failure report + + +# ---------------------------------------------------------------- fix round 2: signal at logout, logout timeout + + +def _interrupt_at_logout(how): + def logout(env): + if how == "signal": + signal.raise_signal(signal.SIGINT) # held during cleanup, raised at the context exit + return Fake.ok() + raise KeyboardInterrupt() + return logout + + +@pytest.mark.parametrize("how", ["signal", "raise"]) +def test_interrupt_during_logout_after_r1_moved_prints_the_state_report(how, capsys): + fake = Fake(logout=_interrupt_at_logout(how)) + assert _release(fake) == 130 + err = capsys.readouterr().err + assert "RELEASE INCOMPLETE" in err + lines = _state_lines(err) + assert "done" in lines[R1] and SHA_NEW in lines[R1] + assert all("pending" in lines[str(r)] and f"now {SHA_OLD}" in lines[str(r)] for r in (FIRST, SECOND)) + assert f"make release SHA={SHA_OLD}" in err and f"make release SHA={SHA_NEW}" in err + assert "Interrupted after a write was sent" in err and "Nothing was changed" not in err + assert not any("-X" in c for c in fake.writes()) # no pin moved after the interrupt + assert all(not os.path.exists(d) for d in fake.config_dirs) + assert err.count("RELEASE INCOMPLETE:") == 1 # the report prints once + + +def test_rule_test_interrupt_during_logout_prints_the_state_report(capsys): + fake = Fake(logout=_interrupt_at_logout("signal")) + rc = release.main(["rule-test", "--rule", R2, "--sha", SHA_NEW], run=fake) + assert rc == 130 + err = capsys.readouterr().err + assert "RELEASE INCOMPLETE" in err and f"make release-rule-test RULE={R2} SHA={SHA_OLD}" in err + assert "Interrupted after a write was sent" in err + + +def test_interrupt_before_any_write_says_nothing_changed(capsys): + def interrupt(_fake, rid, n): + if rid == R1 and n == 2: + raise KeyboardInterrupt() + + fake = Fake(on_retrieve=interrupt) + assert release.main(["release", "--sha", SHA_NEW], run=fake) == 130 + err = capsys.readouterr().err + assert "RELEASE INCOMPLETE" not in err and "Interrupted before any write was sent. Nothing was changed." in err + assert fake.writes() == [] + + +def test_interrupt_during_logout_of_a_plan_says_nothing_changed(capsys): + fake = Fake(logout=_interrupt_at_logout("signal")) + assert release.main(["plan", "--sha", SHA_NEW], run=fake) == 130 + err = capsys.readouterr().err + assert "RELEASE INCOMPLETE" not in err and "Nothing was changed" in err + + +def test_logout_has_a_timeout_and_the_dir_is_still_deleted(capsys): + seen = {} + + def logout(env): + raise subprocess.TimeoutExpired(["ant", "auth", "logout", "--all"], release.LOGOUT_TIMEOUT_S) + + fake = Fake(logout=logout) + real = fake.ant + + def ant(args, kw): + if args[:2] == ["auth", "logout"]: + seen["timeout"] = kw.get("timeout") + return real(args, kw) + + fake.ant = ant + assert release.main(["plan", "--sha", SHA_NEW], run=fake) == 0 + err = capsys.readouterr().err + assert seen["timeout"] == release.LOGOUT_TIMEOUT_S and 0 < release.LOGOUT_TIMEOUT_S <= 60 + assert "did not end within" in err and "local token files are deleted" in err + assert fake.config_dirs and all(not os.path.exists(d) for d in fake.config_dirs) + + +def test_temp_dirs_stay_under_the_test_tmp_path(tmp_path): + fake = Fake() + release.main(["plan", "--sha", SHA_NEW], run=fake) + assert fake.config_dirs and all(Path(d).is_relative_to(tmp_path) for d in fake.config_dirs) + + +def _fail_put_on(target, *, apply_first=False): + def put(rid, new, fake): + if rid == target: + if apply_first: + fake.rulesets[rid] = new # the write reached GitHub, the reply was lost + return Fake.ok(rc=1, stderr="synthetic ruleset failure") + fake.rulesets[rid] = new + return Fake.ok(new) + return put + + +FIRST, SECOND = (rid for rid, _ in release.RULESETS) + + +def _release(fake): + return release.main(["release", "--sha", SHA_NEW], run=fake) + + +def _state_lines(err): + return {ln.split()[1]: ln for ln in err.splitlines() if ln.startswith(" rule ") or ln.startswith(" ruleset ")} + + +def test_first_ruleset_failure_reports_state_and_rollback(capsys): + fake = Fake(put=_fail_put_on(FIRST)) + assert _release(fake) == 2 + err = capsys.readouterr().err + assert "RELEASE INCOMPLETE" in err + lines = _state_lines(err) + assert "done" in lines[R1] and SHA_NEW in lines[R1] + assert "failed" not in lines[str(FIRST)] and "sent" in lines[str(FIRST)] and f"now {SHA_OLD}" in lines[str(FIRST)] + assert "pending" in lines[str(SECOND)] and f"now {SHA_OLD}" in lines[str(SECOND)] + assert f"make release SHA={SHA_OLD}" in err and f"make release SHA={SHA_NEW}" in err + assert "Records: " in err + + +def test_second_ruleset_failure_reports_state_and_rollback(capsys): + fake = Fake(put=_fail_put_on(SECOND)) + assert _release(fake) == 2 + err = capsys.readouterr().err + lines = _state_lines(err) + assert "done" in lines[str(FIRST)] and f"now {SHA_NEW}" in lines[str(FIRST)] + assert "sent" in lines[str(SECOND)] and f"now {SHA_OLD}" in lines[str(SECOND)] + assert f"make release SHA={SHA_OLD}" in err + + +def test_ambiguous_write_is_reported_as_sent_with_the_real_state(capsys): + fake = Fake(put=_fail_put_on(FIRST, apply_first=True)) + assert _release(fake) == 2 + err = capsys.readouterr().err + lines = _state_lines(err) + assert "sent" in lines[str(FIRST)] and f"now {SHA_NEW}" in lines[str(FIRST)] # re-read shows it landed + assert "may have reached the server" in err + assert f"make release SHA={SHA_OLD}" in err + + +def test_ambiguous_rule_write_rereads_the_rule_before_logout(capsys): + fake = Fake() + real = fake.ant + + def ant(args, kw): + if len(args) > 1 and args[1] == "update": + real(args, kw) # lands on the server + return Fake.ok(rc=1, stderr="synthetic timeout") + return real(args, kw) + + fake.ant = ant + assert _release(fake) == 2 + err = capsys.readouterr().err + lines = _state_lines(err) + assert "sent" in lines[R1] and f"now {SHA_NEW}" in lines[R1] + assert all("pending" in lines[str(r)] for r in (FIRST, SECOND)) + assert not any("-X" in c for c in fake.writes()) + assert f"make release SHA={SHA_OLD}" in err + + +def test_differing_previous_pins_give_per_object_recovery_not_one_sha(capsys): + fake = Fake(put=_fail_put_on(SECOND)) + other = "1" * 40 + fake.rulesets[SECOND] = ruleset(SECOND, other) + assert _release(fake) == 2 + err = capsys.readouterr().err + assert "no single rollback SHA" in err + assert f"make release SHA={SHA_OLD}" not in err and f"make release SHA={other}" not in err + assert f"rulesets/{SECOND} --input" in err and f"ruleset-{SECOND}-rollback-body.json" in err + body = json.loads(next(release.RECORDS.rglob(f"ruleset-{SECOND}-rollback-body.json")).read_text()) + assert release.ruleset_pin({"id": SECOND, "rules": body["rules"]}) == other + + +def test_failure_before_any_write_prints_no_state_report(capsys): + fake = Fake(on_retrieve=change_r1_on_second_read) + + assert release.main(["release", "--sha", SHA_NEW], run=fake) == 2 + assert "RELEASE INCOMPLETE" not in capsys.readouterr().err + + +# ---------------------------------------------------------------- fix round 3 + + +def test_the_planted_credentials_are_in_the_environment_during_each_test(): + assert all(os.environ.get(k) == "must-not-reach-ant" for k in PLANTED_ENV) + + +def test_ant_env_drops_every_anthropic_variable_and_sets_only_the_config_dir(): + env = release.ant_env("/private/dir") + assert env["ANTHROPIC_CONFIG_DIR"] == "/private/dir" + assert [k for k in env if k.startswith("ANTHROPIC_")] == ["ANTHROPIC_CONFIG_DIR"] + assert "must-not-reach-ant" not in env.values() + + +def test_every_ant_call_of_a_release_gets_no_planted_credential(): + fake = Fake() + assert release.main(["release", "--sha", SHA_NEW], run=fake) == 0 + ant_calls = [kw for c, kw in fake.calls if c[0] == "ant"] + assert len(ant_calls) >= 5 + assert all("must-not-reach-ant" not in kw["env"].values() for kw in ant_calls) + + +def test_a_sent_write_counts_even_when_its_previous_sha_is_the_target(capsys): + progress = release.Progress(SHA_NEW, release.Records("t"), lambda s: f"make release SHA={s}") + progress.add("rule-x", "rule x", SHA_NEW) + progress.set("rule-x", "sent") + assert progress.wrote() + progress.report() + assert "RELEASE INCOMPLETE:" in capsys.readouterr().err + + +def test_a_sent_write_still_counts_after_its_step_is_done(): + progress = release.Progress(SHA_NEW, release.Records("t"), lambda s: f"make release SHA={s}") + progress.add("rule-x", "rule x", SHA_NEW) + for state in ("failed", "sent", "written", "done"): + progress.set("rule-x", state) + assert progress.wrote() + + +def test_interrupt_after_a_sent_write_at_the_same_sha_does_not_say_nothing_changed(capsys, monkeypatch): + """R1's ref is already at the SHA but another match field differs, so a + POST is sent; an interrupt during it must print the report.""" + fake = Fake() + fake.rules[R1] = rule(sha=SHA_NEW) + fake.rules[R1]["match"]["claims"]["job_workflow_ref"] = REF + SHA_NEW + fake.rules[R1]["match"]["audience"] = None + real = fake.ant + + def ant(args, kw): + if len(args) > 1 and args[1] == "update": + raise KeyboardInterrupt() + return real(args, kw) + + fake.ant = ant + real_patch = release.patch_match + + def patched(match, sha, *, prefix=None): + out = real_patch(match, sha, prefix=prefix) + out["claims"]["extra"] = "forces a POST" + return out + + monkeypatch.setattr(release, "patch_match", patched) + assert _release(fake) == 130 + err = capsys.readouterr().err + assert "RELEASE INCOMPLETE:" in err and "Nothing was changed" not in err + assert "Interrupted after a write was sent" in err + + +def test_an_unchanged_object_read_back_is_not_a_write(): + progress = release.Progress(SHA_NEW, release.Records("t"), lambda s: f"make release SHA={s}") + progress.add("rule-x", "rule x", SHA_NEW) + progress.set("rule-x", "unchanged") + progress.set("rule-x", "done") + assert not progress.wrote() diff --git a/review/tests/unit/test_summary_comment.py b/review/tests/unit/test_summary_comment.py index a2e5a1b..533c582 100644 --- a/review/tests/unit/test_summary_comment.py +++ b/review/tests/unit/test_summary_comment.py @@ -132,7 +132,7 @@ def table_rows(s: str, header_start: str) -> list[list[str]]: def test_a_pass_clean(tmp_path): s = case_a(tmp_path).summary - assert s.startswith(verdict.STICKY_MARKER + "\n## 🟢 PASS: OC Claude review\n") + assert s.startswith(verdict.STICKY_MARKER + "\n## 🟢 All clear: Claude of Duty review passed\n") assert callout_kind(s) == "TIP" assert "No findings. This review found no reason to block." in s assert "| Severity |" not in s # no findings table @@ -142,7 +142,7 @@ def test_a_pass_clean(tmp_path): def test_b_pass_with_a_minor_finding(tmp_path): s = case_b(tmp_path).summary - assert "## 🟡 PASS: OC Claude review, 1 finding to consider\n" in s + assert "## 🟡 Clear to proceed: Claude of Duty logged 1 item to consider\n" in s assert callout_kind(s) == "NOTE" assert "It does not block this PR." in s [row] = table_rows(s, "| Severity |") @@ -155,7 +155,7 @@ def test_c_waivable_fail_is_red_caution(tmp_path): r = case_c(tmp_path) s = r.summary assert r.code == 1 - assert "## 🔴 FAIL: OC Claude review\n" in s + assert "\n## 🔴 Hold deployment: Claude of Duty identified " in s assert callout_kind(s) == "CAUTION" and "[!WARNING]" not in s # ruling 1 assert "**One high-severity security finding blocks this PR.** A maintainer can waive it." in s assert "ask a maintainer to post the waiver line" in s @@ -168,7 +168,7 @@ def test_c_waivable_fail_is_red_caution(tmp_path): def test_d_hard_block_comes_first_and_has_no_waiver(tmp_path): r = case_d(tmp_path) s = r.summary - assert "## 🔴 FAIL: OC Claude review\n" in s and callout_kind(s) == "CAUTION" + assert "\n## 🔴 Hold deployment: Claude of Duty identified " in s and callout_kind(s) == "CAUTION" assert "**Hidden text-direction control blocks this PR. No waiver can clear it.**" in s assert "One high-severity" in s and "also blocks." in s rows = table_rows(s, "| Severity |") @@ -185,7 +185,7 @@ def test_d_hard_block_comes_first_and_has_no_waiver(tmp_path): def test_e_pass_that_did_not_run(tmp_path): r = case_e(tmp_path) s = r.summary - assert "## 🔴 FAIL: OC Claude review did not complete\n" in s and callout_kind(s) == "CAUTION" + assert "## 🔴 Comms down: the Claude of Duty review did not complete\n" in s and callout_kind(s) == "CAUTION" assert "The Security review did not run, so this PR could not be judged. No waiver can clear this." in s [row] = table_rows(s, "| Severity |") assert row[:3] == ["⚠️ **Not run**", "Blocks, no waiver", "Security pass"] @@ -199,7 +199,7 @@ def test_e_pass_that_did_not_run(tmp_path): def test_f_pr39_waivable_fail_is_red_caution_with_two_rows(tmp_path): s = case_f(tmp_path).summary - assert "## 🔴 FAIL: OC Claude review\n" in s + assert "\n## 🔴 Hold deployment: Claude of Duty identified " in s assert callout_kind(s) == "CAUTION" and "[!WARNING]" not in s # ruling 1: not the yellow signal assert "A maintainer can waive it." in s # Ruling 3: the high finding blocks, the two medium findings do not. Different effects, different rows. @@ -220,15 +220,15 @@ def test_a_fail_never_uses_the_warning_callout(rendered): s = r.summary assert "[!WARNING]" not in s if r.code == 1: - assert callout_kind(s) == "CAUTION" and "🔴 FAIL" in s + assert callout_kind(s) == "CAUTION" and "🔴 " in s else: - assert callout_kind(s) in ("TIP", "NOTE") and "PASS" in s + assert callout_kind(s) in ("TIP", "NOTE") and ("All clear" in s or "Clear to proceed" in s) def test_the_comment_never_implies_approval(rendered): _, r = rendered s = r.summary.lower() - assert "approved" not in s and "this bot never approves a pr" in s + assert "approved" not in s and "claude of duty never approves a pr" in s assert s.count("never approves") == 1 @@ -356,7 +356,7 @@ def test_a_waiver_applied_is_a_green_pass_and_the_findings_stay_listed(tmp_path) from .test_publish import override r = run(tmp_path, FakeGitHub(issue_comments=[override(1)]), passes=clean_passes(supply_chain=[BLOCKING])) s = r.summary - assert r.code == 0 and "## 🟢 PASS: OC Claude review\n" in s and callout_kind(s) == "TIP" + assert r.code == 0 and "## 🟢 All clear: Claude of Duty review passed\n" in s and callout_kind(s) == "TIP" assert "A maintainer waiver was applied by maintainer. Findings stay listed." in s assert s.lower().count("never approves") == 1 [row] = table_rows(s, "| Severity |") @@ -406,7 +406,7 @@ def test_g_no_bundle_is_not_an_author_fix(tmp_path): Path(out).mkdir(parents=True, exist_ok=True) (Path(out) / "g-no-bundle.md").write_text(s, encoding="utf-8") assert r.code == 1 - assert "## 🔴 FAIL: OC Claude review did not complete\n" in s and callout_kind(s) == "CAUTION" + assert "## 🔴 Comms down: the Claude of Duty review did not complete\n" in s and callout_kind(s) == "CAUTION" callout = "\n".join(line for line in s.split("\n") if line.startswith("> ")) assert "A required check did not complete" in callout assert "a maintainer re-runs the newest run" in callout @@ -449,3 +449,48 @@ def test_long_path_is_cut_once(monkeypatch): monkeypatch.setattr(publish, "_finding_effect", lambda *_: publish.EFFECT_NONE) [row] = publish._merge_findings(result, publish.InlineOutcome()) assert row.where.count("…") == 1 and "[truncated]" not in row.where + + +def test_mission_line_sits_directly_under_the_heading(rendered): + _, r = rendered + lines = r.summary.split("\n") + i = next(n for n, line in enumerate(lines) if line.startswith("## ")) + assert lines[i + 1] == publish.MISSION_LINE and r.summary.count(publish.MISSION_LINE) == 1 + assert publish.APPROVAL_NOTE in r.summary and "This bot never approves" not in r.summary + + +def test_fail_count_equals_findings_table_rows(tmp_path_factory): + checked = 0 + for name, case in CASES.items(): + r = case(tmp_path_factory.mktemp(name)) + if r.code != 1 or "Comms down" in r.summary: + continue + head = next(line for line in r.summary.split("\n") if line.startswith("## ")) + rows = [] + in_table = False + for ln in r.summary.split("\n"): + if ln.startswith("| Severity |"): + in_table = True + elif in_table and ln.startswith("|---"): + continue + elif in_table and ln.startswith("| "): + rows.append(ln) + elif in_table: + break + m = re.search(r"identified (\d+) items?", head) + assert m and int(m.group(1)) == len(rows) > 0, name + checked += 1 + assert checked >= 3 + + +def test_b2_pass_with_two_findings_uses_the_plural(tmp_path): + one = finding("bug", "low", title="First.", quote="shell=True") + two = finding("bug", "low", line=11, quote="line 11", title="Second.") + s = run(tmp_path, passes=clean_passes(review=[one, two])).summary + assert "\n## 🟡 Clear to proceed: Claude of Duty logged 2 items to consider\n" in "\n" + s + + +def test_fail_heading_without_rows_has_no_number(): + result = SimpleNamespace(status=verdict.FAIL, codes=set(), override_applied=False) + assert publish._status_heading(result, [], []) == ( + "## 🔴 Hold deployment: Claude of Duty identified items requiring attention") diff --git a/review/tools/release.py b/review/tools/release.py new file mode 100644 index 0000000..f136740 --- /dev/null +++ b/review/tools/release.py @@ -0,0 +1,789 @@ +"""Owner-run release tool: move the production federation rule and the ruleset pins. + + make release-plan SHA= read-only; NO_CONSOLE=1 skips the Console reads + make release SHA= plan, then apply at once (no prompt) and read back + make release-rule-test RULE= SHA= Console-only edit of a scratch rule + +Spec: docs/claude-review-design.md, "Each release" in "Auth and cost". The steps +of a release, in this order: + + 1. preconditions (fail closed): SHA is 40-hex and on main; test.yml and the + release eval.yml run at SHA concluded success; analyze.py at SHA names + rule R1; the caller is an org owner; each ruleset has exactly one + workflows entry for claude-review.yml, with a sha and no ref + 2. Console: GET R1, change only the SHA suffix of + match.claims.job_workflow_ref, POST the whole match, GET back, compare + 3. GitHub: for each ruleset, GET, change only that entry's sha, PUT the + rules, GET back, compare + +Between step 2 and the end of step 3 the rulesets still pin the old SHA and +R1 already names the new one, so a required run in that window fails closed +(authentication failed). Start a new run on each open PR after the release. +Rollback is the same command with the previous release SHA. + +Credential (owner decision, no persistent admin token): the Console endpoints +take only an OAuth token with the org:admin scope; Admin API keys are refused. +This tool makes a private temp dir (0700), points ANTHROPIC_CONFIG_DIR at it +for its own `ant` subprocesses only, removes every other ANTHROPIC_* variable +from their environment, runs `ant auth login --profile admin --scope org:admin` +itself (the owner completes it in the browser), and on every exit path runs +`ant auth logout --all` and deletes the dir. The tool never reads, prints, +stores or exports a token. GitHub calls use `gh api` with the owner's gh login. + +Before and after JSON goes to release-records/ at the repo root (git-ignored). +It holds rule and ruleset objects only, no credential. + +Not imported by production code and not run in CI or any workflow: it holds +both an Anthropic admin login and GitHub write, which no job may hold. +""" + +from __future__ import annotations + +import argparse +import base64 +import copy +import json +import os +import re +import shutil +import signal +import subprocess +import sys +import tempfile +from collections.abc import Callable, Iterator, Sequence +from contextlib import contextmanager +from datetime import UTC, datetime +from pathlib import Path +from typing import Any + +REPO = "OperationCode/.github" +ORG = "OperationCode" +WORKFLOW_PATH = ".github/workflows/claude-review.yml" +WORKFLOW_REPO_ID = 1432342572 +JOB_WORKFLOW_REF_PREFIX = f"{REPO}/{WORKFLOW_PATH}@" +PROD_RULE_ID = "fdrl_01N2ZktTiVKqUvvXwywBFfys" +ANALYZE_PATH = "review/src/oc_review/analyze.py" +# Every ruleset that requires the workflow: the pilot ruleset and the scratch +# test ruleset. Add the org ruleset here when it gets the pin (phase 6). +RULESETS: tuple[tuple[int, str], ...] = ((26132750, "claude-review-pilot"), (25835887, "p0-required-workflow")) +# The run each release needs at the SHA: (file, event, branch). test.yml runs +# on the push to main; the release eval is an owner-approved dispatch on main +# (as for cc710fb, cf19216 and 96f4966). A pull_request run at the same head +# SHA tested a merge ref, so it does not count. +RELEASE_WORKFLOWS: tuple[tuple[str, str, str], ...] = (("test.yml", "push", "main"), + ("eval.yml", "workflow_dispatch", "main")) +RULE_ID_RE = re.compile(r"^fdrl_[A-Za-z0-9]{24}$") +SHA_RE = re.compile(r"^[0-9a-f]{40}$") +RECORDS = Path(__file__).resolve().parents[2] / "release-records" +# Server-set fields that may change on any write. +VOLATILE_KEYS = frozenset({"updated_at", "updated_by_actor_id"}) +_REDACT_RE = re.compile(r"(?i)(bearer\s+|sk-ant-)[^\s\"']+") + +Runner = Callable[..., subprocess.CompletedProcess] +# `ant auth logout` runs while termination signals are held, so it must end. +LOGOUT_TIMEOUT_S = 30 + + +class ReleaseError(Exception): + """A refusal: the tool stops and changes nothing more.""" + + +# ---------------------------------------------------------------- pure logic + + +def validate_sha(sha: str | None) -> str: + if not sha or not SHA_RE.match(sha): + raise ReleaseError(f"SHA must be 40 lowercase hex characters, got {sha!r}") + return sha + + +def validate_rule_id(rule_id: str | None) -> str: + if not rule_id or not RULE_ID_RE.match(rule_id): + raise ReleaseError(f"RULE must look like fdrl_ plus 24 characters, got {rule_id!r}") + return rule_id + + +def _strip_nulls(value: Any) -> Any: + if isinstance(value, dict): + return {k: _strip_nulls(v) for k, v in value.items() if v is not None} + if isinstance(value, list): + return [_strip_nulls(v) for v in value] + return value + + +def patch_match(match: Any, sha: str, *, prefix: str | None = None) -> dict: + """The whole match object to POST: a copy of `match` with only the SHA + suffix of claims.job_workflow_ref changed. Null keys are dropped, because + the POST replaces match as a whole and an unset matcher is the same as an + absent one. With `prefix`, the ref before the SHA must equal it.""" + if not isinstance(match, dict): + raise ReleaseError("rule has no match object") + claims = match.get("claims") + if not isinstance(claims, dict): + raise ReleaseError("rule match has no claims object") + ref = claims.get("job_workflow_ref") + if not isinstance(ref, str) or "@" not in ref: + raise ReleaseError(f"rule has no job_workflow_ref claim of the form @: {ref!r}") + head, _, old = ref.rpartition("@") + if not SHA_RE.match(old): + raise ReleaseError(f"job_workflow_ref does not end in a 40-hex SHA: {ref!r}") + if prefix is not None and head + "@" != prefix: + raise ReleaseError(f"job_workflow_ref is {ref!r}, expected {prefix}") + new = _strip_nulls(copy.deepcopy(match)) + new["claims"]["job_workflow_ref"] = f"{head}@{sha}" + return new + + +def diff_paths(a: Any, b: Any, path: str = "") -> list[str]: + """Every path where a and b differ. A null and a missing key are equal.""" + if isinstance(a, dict) and isinstance(b, dict): + out: list[str] = [] + for key in sorted(set(a) | set(b), key=str): + out += diff_paths(a.get(key), b.get(key), f"{path}.{key}" if path else str(key)) + return out + if isinstance(a, list) and isinstance(b, list): + if len(a) != len(b): + return [f"{path} (length {len(a)} -> {len(b)})"] + out = [] + for i, (x, y) in enumerate(zip(a, b, strict=True)): + out += diff_paths(x, y, f"{path}[{i}]") + return out + return [] if a == b else [path or "(root)"] + + +def _without_volatile(obj: dict) -> dict: + return {k: v for k, v in obj.items() if k not in VOLATILE_KEYS} + + +def compare_rule(before: dict, after: dict, expected_match: dict) -> list[str]: + """Mismatches after a rule update: match must equal what was sent, and + every other field must equal the before state.""" + problems = [f"match.{p}" for p in diff_paths(expected_match, after.get("match"))] + rest_before = {k: v for k, v in _without_volatile(before).items() if k != "match"} + rest_after = {k: v for k, v in _without_volatile(after).items() if k != "match"} + problems += diff_paths(rest_before, rest_after) + return problems + + +def workflow_entry_index(ruleset: dict) -> tuple[int, int]: + """(rule index, workflow index) of the one entry for the central workflow. + Refuses zero or several entries, and an entry with a ref.""" + hits = [] + for ri, rule in enumerate(ruleset.get("rules") or []): + if not isinstance(rule, dict) or rule.get("type") != "workflows": + continue + for wi, wf in enumerate((rule.get("parameters") or {}).get("workflows") or []): + if wf.get("repository_id") == WORKFLOW_REPO_ID and wf.get("path") == WORKFLOW_PATH: + hits.append((ri, wi)) + rid = ruleset.get("id") + if len(hits) != 1: + raise ReleaseError(f"ruleset {rid}: expected 1 workflows entry for {WORKFLOW_PATH}, found {len(hits)}") + ri, wi = hits[0] + wf = ruleset["rules"][ri]["parameters"]["workflows"][wi] + if wf.get("ref") is not None: + raise ReleaseError(f"ruleset {rid}: the workflow entry has a ref ({wf['ref']!r}); the pin must be a sha only") + if not isinstance(wf.get("sha"), str) or not SHA_RE.match(wf["sha"]): + raise ReleaseError(f"ruleset {rid}: the workflow entry has no 40-hex sha") + return ri, wi + + +def ruleset_pin(ruleset: dict) -> str: + ri, wi = workflow_entry_index(ruleset) + return ruleset["rules"][ri]["parameters"]["workflows"][wi]["sha"] + + +def patch_ruleset(ruleset: dict, sha: str) -> dict: + """The PUT body: every rule as read, with only the pinned sha changed.""" + ri, wi = workflow_entry_index(ruleset) + rules = copy.deepcopy(ruleset["rules"]) + rules[ri]["parameters"]["workflows"][wi]["sha"] = sha + return {"rules": rules} + + +def compare_ruleset(before: dict, after: dict, sha: str) -> list[str]: + """Mismatches after a pin move: the pin must be `sha` with no ref, and + every other field must equal the before state.""" + try: + ri, wi = workflow_entry_index(after) + except ReleaseError as e: + return [str(e)] + problems = [] + if after["rules"][ri]["parameters"]["workflows"][wi]["sha"] != sha: + problems.append(f"rules[{ri}].parameters.workflows[{wi}].sha is not {sha}") + expected = copy.deepcopy(before) + expected["rules"] = patch_ruleset(before, sha)["rules"] + problems += diff_paths(_without_volatile(expected), _without_volatile(after)) + return problems + + +def redact(text: str) -> str: + return _REDACT_RE.sub(lambda m: m.group(1) + "[redacted]", text) + + +# ---------------------------------------------------------------- subprocess wrappers + + +def _check(cp: subprocess.CompletedProcess, what: str) -> str: + if cp.returncode != 0: + err = redact((cp.stderr or "").strip())[:2000] + raise ReleaseError(f"{what} failed (exit {cp.returncode}): {err}") + return cp.stdout or "" + + +def _json(text: str, what: str) -> Any: + try: + return json.loads(text) + except ValueError as e: + raise ReleaseError(f"{what}: output is not JSON ({e})") from None + + +class GitHub: + def __init__(self, run: Runner): + self.run = run + + def get(self, path: str) -> Any: + cp = self.run(["gh", "api", "-H", "Accept: application/vnd.github+json", path], + capture_output=True, text=True) + return _json(_check(cp, f"gh api GET {path}"), f"GET {path}") + + def put(self, path: str, body: dict) -> Any: + cp = self.run(["gh", "api", "-X", "PUT", "-H", "Accept: application/vnd.github+json", path, "--input", "-"], + input=json.dumps(body), capture_output=True, text=True) + return _json(_check(cp, f"gh api PUT {path}"), f"PUT {path}") + + +def ant_env(config_dir: str) -> dict[str, str]: + """Environment for `ant`: no ANTHROPIC_* from the caller (an API key or + auth token overrides every profile), and the private config dir.""" + env = {k: v for k, v in os.environ.items() if not k.startswith("ANTHROPIC_")} + env["ANTHROPIC_CONFIG_DIR"] = config_dir + return env + + +class Console: + """`ant` calls, all inside one private config dir.""" + + RULES = "organization:federation:rules" + + def __init__(self, run: Runner, config_dir: str): + self.run = run + self.env = ant_env(config_dir) + self.closed = False + + def _ant(self, args: Sequence[str], what: str) -> str: + if self.closed: + raise ReleaseError("the Console session is closed (logged out)") + cp = self.run(["ant", *args], env=self.env, capture_output=True, text=True) + return _check(cp, what) + + def login(self) -> None: + print("Console: opening the browser for `ant auth login --profile admin --scope org:admin`.") + print(" Log in as an org admin or owner. The token stays in a temp dir and is removed at exit.") + cp = self.run(["ant", "auth", "login", "--profile", "admin", "--scope", "org:admin"], env=self.env) + if cp.returncode != 0: + raise ReleaseError(f"ant auth login failed (exit {cp.returncode}); not logged in to the Console") + + def logout(self) -> bool: + """Log out with a bounded wait. False when it failed or timed out.""" + self.closed = True + try: + cp = self.run(["ant", "auth", "logout", "--all"], env=self.env, capture_output=True, text=True, + timeout=LOGOUT_TIMEOUT_S) + except subprocess.TimeoutExpired: + print(f"WARNING: `ant auth logout --all` did not end within {LOGOUT_TIMEOUT_S} s and was stopped.", + file=sys.stderr) + return False + except OSError: + return False + return cp.returncode == 0 + + def retrieve(self, rule_id: str) -> dict: + out = self._ant([self.RULES, "retrieve", "--federation-rule-id", rule_id, "--format", "json"], + f"ant {self.RULES} retrieve {rule_id}") + rule = _json(out, f"rule {rule_id}") + if not isinstance(rule, dict) or rule.get("id", rule_id) != rule_id: + raise ReleaseError(f"rule {rule_id}: the read returned a different object") + return rule + + def update(self, rule_id: str, match: dict) -> dict: + out = self._ant([self.RULES, "update", "--federation-rule-id", rule_id, + "--match", json.dumps(match, separators=(",", ":")), "--format", "json"], + f"ant {self.RULES} update {rule_id}") + return _json(out, f"update of rule {rule_id}") + + +def check_ant(run: Runner) -> None: + """`ant` is installed and has the rule commands this tool calls. The + getter name `retrieve` is not confirmed by the docs, so check it here.""" + empty = tempfile.mkdtemp(prefix="oc-release-ant-help-") # no profile in it + try: + cp = run(["ant", Console.RULES, "--help"], env=ant_env(empty), capture_output=True, text=True) + except FileNotFoundError: + raise ReleaseError("ant is not installed, so the Console cannot be read. Install it with " + "`brew install anthropics/tap/ant`, or run the plan with NO_CONSOLE=1 " + "to skip the Console part") from None + finally: + remove_dir(empty) + text = (cp.stdout or "") + (cp.stderr or "") + missing = [c for c in ("retrieve", "update") if not re.search(rf"\b{c}\b", text)] + if cp.returncode != 0 or missing: + raise ReleaseError(f"`ant {Console.RULES} --help` does not list {missing or 'its commands'}; " + "this ant build does not match the tool; read the help and fix the Console class") + + +# Cleanup problems that must make the run exit non-zero, even after success. +CLEANUP_FAILURES: list[str] = [] +_CLEANUP_SIGNALS = (signal.SIGINT, signal.SIGTERM, signal.SIGHUP) + + +def remove_dir(path: str) -> bool: + """Delete `path`. On any failure, say so loudly and record it.""" + errors: list[str] = [] + try: + shutil.rmtree(path, onexc=lambda _fn, p, exc: errors.append(f"{p}: {exc}")) + except BaseException as e: # noqa: BLE001 - report every failure + errors.append(repr(e)) + if errors or os.path.exists(path): + msg = (f"could not delete {path} ({'; '.join(errors) or 'still exists'}). It may hold an " + f"org:admin token: delete it now with: rm -rf '{path}'") + print(f"\nERROR: {msg}", file=sys.stderr) + CLEANUP_FAILURES.append(msg) + return False + return True + + +@contextmanager +def signals_deferred() -> Iterator[list[int]]: + """Hold SIGINT, SIGTERM and SIGHUP while cleanup runs. Yields the list of + signals that arrived; the caller decides whether to raise after.""" + pending: list[int] = [] + old = {} + try: + for sig in _CLEANUP_SIGNALS: + old[sig] = signal.signal(sig, lambda signum, _f: pending.append(signum)) + except ValueError: # not the main thread: signals cannot arrive here anyway + pass + try: + yield pending + finally: + for sig, handler in old.items(): + signal.signal(sig, handler) + + +def _cleanup_console(console: Console, config_dir: str) -> list[int]: + """Log out, then delete the dir in an inner finally, with termination + signals held. Returns the signals that arrived meanwhile.""" + with signals_deferred() as pending: + ok = False + try: + ok = console.logout() + finally: + removed = remove_dir(config_dir) + if not ok: + print("WARNING: `ant auth logout --all` did not complete. " + + ("The local token files are deleted, but " if removed else "") + + "the session may stay valid until it expires.", file=sys.stderr) + elif removed: + print("Console: logged out, temp config dir removed.") + return pending + + +@contextmanager +def ephemeral_console(run: Runner) -> Iterator[Console]: + """Log in to the Console in a fresh 0700 dir; log out and delete it on + every exit path.""" + config_dir = tempfile.mkdtemp(prefix="oc-release-ant-") + os.chmod(config_dir, 0o700) + console = Console(run, config_dir) + try: + console.login() + yield console + except BaseException: + _cleanup_console(console, config_dir) # the first exception wins + raise + pending = _cleanup_console(console, config_dir) + if pending: + raise KeyboardInterrupt(f"signal {pending[0]} during cleanup") + + +# ---------------------------------------------------------------- records + + +class Records: + def __init__(self, label: str, root: Path | None = None): + stamp = datetime.now(UTC).strftime("%Y%m%dT%H%M%SZ") + self.dir = (root or RECORDS) / f"{stamp}-{label}" + + def save(self, name: str, obj: Any) -> None: + self.dir.mkdir(parents=True, exist_ok=True, mode=0o700) + (self.dir / f"{name}.json").write_text(json.dumps(obj, indent=2, sort_keys=True) + "\n", encoding="utf-8") + + +# ---------------------------------------------------------------- preconditions + + +def release_run_ok(run: dict, wf: str, event: str, branch: str, sha: str) -> bool: + """A completed, successful run of that workflow file, for that event and + branch, whose head is exactly `sha`.""" + return (run.get("path") == f".github/workflows/{wf}" and run.get("event") == event + and run.get("head_branch") == branch and run.get("head_sha") == sha + and run.get("status") == "completed" and run.get("conclusion") == "success") + + +def github_preconditions(gh: GitHub, sha: str, rule_id: str) -> list[str]: + """Read-only checks. Returns the failures (empty when all pass).""" + fails: list[str] = [] + + def check(ok: bool, good: str, bad: str) -> None: + print(f" [{'ok' if ok else 'FAIL'}] {good if ok else bad}") + if not ok: + fails.append(bad) + + try: + cmp = gh.get(f"repos/{REPO}/compare/{sha}...main") + status = cmp.get("status") + check(status in ("ahead", "identical"), f"{sha} is on main ({status})", + f"{sha} is not on main (compare status {status!r})") + if status == "ahead": + print(f" note: main is {cmp.get('ahead_by')} commit(s) past {sha[:12]} (a rollback, or an old SHA)") + except ReleaseError as e: + check(False, "", f"cannot compare {sha} with main: {e}") + + for wf, event, branch in RELEASE_WORKFLOWS: + try: + runs = gh.get(f"repos/{REPO}/actions/workflows/{wf}/runs?head_sha={sha}&per_page=100").get("workflow_runs") or [] + except ReleaseError as e: + check(False, "", f"cannot read {wf} runs: {e}") + continue + good = [r for r in runs if release_run_ok(r, wf, event, branch, sha)] + check(bool(good), f"{wf} {event} run on {branch} succeeded at {sha[:12]} (run {good[0]['id'] if good else ''})", + f"no successful {wf} {event} run on {branch} at {sha} ({len(runs)} run(s) of any kind found)") + + try: + meta = gh.get(f"repos/{REPO}/contents/{ANALYZE_PATH}?ref={sha}") + text = base64.b64decode(meta.get("content") or "").decode("utf-8", "replace") + named = re.search(r'^ANTHROPIC_FEDERATION_RULE_ID\s*=\s*"([^"]+)"', text, re.M) + got = named.group(1) if named else None + check(got == rule_id, f"analyze.py at {sha[:12]} names rule {rule_id}", + f"analyze.py at {sha} names rule {got!r}, not {rule_id}") + except ReleaseError as e: + check(False, "", f"cannot read {ANALYZE_PATH} at {sha}: {e}") + + try: + login = gh.get("user").get("login") + role = gh.get(f"orgs/{ORG}/memberships/{login}").get("role") + check(role == "admin", f"gh user {login} is an {ORG} owner", + f"gh user {login} has org role {role!r}; moving a ruleset pin needs an org owner") + except ReleaseError as e: + check(False, "", f"cannot read the gh user or org role: {e}") + return fails + + +# ---------------------------------------------------------------- progress + +# Records dirs of the state reports printed in this run (main reads it). +REPORTS_PRINTED: list[Path] = [] + + +class Progress: + """The state of each object a release writes, for the failure report. + + pending not touched yet (still at its previous value) + unchanged already at the SHA, no write sent + sent a write was sent and its outcome is not known: it may have + reached the server (error, timeout or interrupt during the call) + written the write returned, the read-back has not passed yet + done written (or unchanged) and read back as planned + failed the step stopped before its write; the object was not written + """ + + def __init__(self, sha: str, rec: Records, rollback: Callable[[str], str]): + self.sha, self.rec, self.rollback = sha, rec, rollback + self.items: dict[str, dict[str, Any]] = {} + self.reported = False + + def add(self, key: str, label: str, previous: str, reread: Callable[[], str] | None = None) -> None: + self.items[key] = {"label": label, "previous": previous, "state": "pending", "reread": reread, + "was_sent": False} + + def set(self, key: str, state: str) -> None: + self.items[key]["state"] = state + if state == "sent": + self.items[key]["was_sent"] = True + + def wrote(self) -> bool: + """True when any write may have reached the server: a write was + sent for some object (whatever its previous SHA or later state), or + an object read back at a SHA other than its previous one.""" + return any(i["was_sent"] or (i["state"] == "done" and i["previous"] != self.sha) + for i in self.items.values()) + + def report(self) -> None: + """Print which object is at which SHA, the records and the recovery. + Prints once, and only when a write was sent.""" + if self.reported or not self.wrote(): + return + self.reported = True + REPORTS_PRINTED.append(self.rec.dir) + err = sys.stderr + print(f"\nRELEASE INCOMPLETE: some objects may be at {self.sha[:12]}, others at their previous SHA.", file=err) + print("While the rule and the pins name different SHAs, required runs fail closed.", file=err) + print("State of each object. 'now' is read again just now. 'sent' means a write was sent and its", + file=err) + print("outcome is unknown: it may have reached the server, so trust 'now', not the state.", file=err) + for key, i in self.items.items(): + now = "not read" + if i["reread"] is not None: + try: + now = i["reread"]() + except BaseException as e: # noqa: BLE001 - the report must finish + now = f"re-read failed ({redact(str(e))[:200]})" + if i["state"] == "done": + now = f"{self.sha} (read back by its step; {now})" + elif i["state"] == "done": + now = f"{self.sha} (read back)" + print(f" {i['label']:<28} {i['state']:<9} now {now} (previous {i['previous']})", file=err) + print(f"Records: {self.rec.dir}", file=err) + prevs = {i["previous"] for i in self.items.values()} + print("Recovery:", file=err) + print(f" - retry: fix the cause, then run `{self.rollback(self.sha)}` again (objects already at the SHA get no write)", file=err) + if len(prevs) == 1: + print(f" - roll back: `{self.rollback(prevs.pop())}`", file=err) + return + print(" - roll back: the previous values differ, so there is no single rollback SHA. Per object:", file=err) + for key, i in self.items.items(): + if key.startswith("ruleset-"): + rid = key.removeprefix("ruleset-") + print(f" {i['label']}: gh api -X PUT orgs/{ORG}/rulesets/{rid} " + f"--input '{self.rec.dir / f'ruleset-{rid}-rollback-body.json'}' (pin {i['previous']}), then read it back", file=err) + else: + print(f" {i['label']}: set job_workflow_ref back to ...@{i['previous']} in the Console, then read it back", file=err) + + +# ---------------------------------------------------------------- flows + + +def plan_rulesets(gh: GitHub, sha: str, rec: Records) -> tuple[list[tuple[int, str, dict]], list[str]]: + """GET each ruleset and print its pin change. Returns (ruleset id, name, + before) for each, and the failures.""" + out, fails = [], [] + for rid, name in RULESETS: + try: + before = gh.get(f"orgs/{ORG}/rulesets/{rid}") + pin = ruleset_pin(before) + except ReleaseError as e: + print(f" [FAIL] ruleset {rid} {name}: {e}") + fails.append(str(e)) + continue + rec.save(f"ruleset-{rid}-before", before) + state = "no change" if pin == sha else f"{pin} -> {sha}" + print(f" ruleset {rid} {before.get('name')!r} ({before.get('enforcement')}): pin {state}") + out.append((rid, name, before)) + return out, fails + + +def plan_rule(console: Console, rule_id: str, sha: str, rec: Records, prefix: str | None) -> tuple[dict, dict]: + before = console.retrieve(rule_id) + rec.save(f"rule-{rule_id}-before", before) + match = patch_match(before.get("match"), sha, prefix=prefix) + old = before["match"]["claims"]["job_workflow_ref"] + state = "no change" if old == match["claims"]["job_workflow_ref"] else f"{old}\n -> {match['claims']['job_workflow_ref']}" + print(f" rule {rule_id} {before.get('name')!r}: job_workflow_ref {state}") + print(f" POST body: {json.dumps({'match': match}, sort_keys=True)}") + return before, match + + +def rule_sha(rule: dict) -> str: + return str(((rule.get("match") or {}).get("claims") or {}).get("job_workflow_ref", "")).rpartition("@")[2] + + +def apply_rule(console: Console, rule_id: str, before: dict, match: dict, rec: Records, progress: Progress) -> None: + key = f"rule-{rule_id}" + progress.set(key, "failed") + now = console.retrieve(rule_id) + changed = diff_paths(_without_volatile(before), _without_volatile(now)) + if changed: + raise ReleaseError(f"rule {rule_id} changed since the plan read ({', '.join(changed)}); run again") + if before["match"] == match or _strip_nulls(before["match"]) == match: + print(f" rule {rule_id}: already at the SHA, no POST") + progress.set(key, "unchanged") + else: + progress.set(key, "sent") + response = console.update(rule_id, match) + progress.set(key, "written") + rec.save(f"rule-{rule_id}-response", response) + after = console.retrieve(rule_id) + rec.save(f"rule-{rule_id}-after", after) + problems = compare_rule(before, after, match) + if problems: + raise ReleaseError(f"rule {rule_id} read-back mismatch: {', '.join(problems)}") + progress.set(key, "done") + print(f" [ok] rule {rule_id} read back: only job_workflow_ref changed") + + +def apply_ruleset(gh: GitHub, rid: int, before: dict, sha: str, rec: Records, progress: Progress) -> None: + key = f"ruleset-{rid}" + progress.set(key, "failed") + now = gh.get(f"orgs/{ORG}/rulesets/{rid}") + changed = diff_paths(_without_volatile(before), _without_volatile(now)) + if changed: + raise ReleaseError(f"ruleset {rid} changed since the plan read ({', '.join(changed)}); run again") + if ruleset_pin(before) == sha: + print(f" ruleset {rid}: already at the SHA, no PUT") + progress.set(key, "unchanged") + else: + body = patch_ruleset(before, sha) + rec.save(f"ruleset-{rid}-body", body) + progress.set(key, "sent") + gh.put(f"orgs/{ORG}/rulesets/{rid}", body) + progress.set(key, "written") + after = gh.get(f"orgs/{ORG}/rulesets/{rid}") + rec.save(f"ruleset-{rid}-after", after) + problems = compare_ruleset(before, after, sha) + if problems: + raise ReleaseError(f"ruleset {rid} read-back mismatch: {', '.join(problems)}") + progress.set(key, "done") + print(f" [ok] ruleset {rid} read back: only the pin changed (repository_id, path, sha, no ref)") + + +def cmd_plan(args, run: Runner, *, apply: bool) -> int: + sha = validate_sha(args.sha) + gh = GitHub(run) + rec = Records(f"{'release' if apply else 'plan'}-{sha[:12]}") + print(f"{'Release (applies at once, no prompt)' if apply else 'Release plan (read-only dry run; `make release` applies)'} for {sha}\n\nPreconditions (GitHub, read-only):") + fails = github_preconditions(gh, sha, PROD_RULE_ID) + print("\nRulesets (GitHub, read-only):") + rulesets, rs_fails = plan_rulesets(gh, sha, rec) + fails += rs_fails + if apply and fails: + raise ReleaseError(f"{len(fails)} precondition(s) failed; nothing changed") + + if args.no_console: + if apply: + raise ReleaseError("NO_CONSOLE is for the plan only") + print("\nConsole: skipped (NO_CONSOLE=1). Rule R1 was not read.") + else: + print("\nConsole (rule R1):") + check_ant(run) + progress = Progress(sha, rec, lambda s: f"make release SHA={s}") + try: + return _console_and_pins(run, gh, sha, rec, rulesets, progress, apply=apply, fails=fails) + except BaseException: + # Every exit after a sent write prints the state, also a signal + # raised when the Console context closes (held during logout). + progress.report() + raise + return _plan_result(rec, fails) + + +def _console_and_pins(run: Runner, gh: GitHub, sha: str, rec: Records, rulesets, progress: Progress, + *, apply: bool, fails: list[str]) -> int: + """Read R1 (plan), or edit R1 and then move the pins (release).""" + with ephemeral_console(run) as console: + before, match = plan_rule(console, PROD_RULE_ID, sha, rec, JOB_WORKFLOW_REF_PREFIX) + if not apply: + return _plan_result(rec, fails) + progress.add(f"rule-{PROD_RULE_ID}", f"rule {PROD_RULE_ID}", rule_sha(before), + lambda: rule_sha(console.retrieve(PROD_RULE_ID))) + for rid, _, before_rs in rulesets: + rec.save(f"ruleset-{rid}-rollback-body", {"rules": before_rs["rules"]}) + progress.add(f"ruleset-{rid}", f"ruleset {rid}", ruleset_pin(before_rs), + lambda rid=rid: ruleset_pin(gh.get(f"orgs/{ORG}/rulesets/{rid}"))) + print(f"\nApplying now (no confirmation): set rule {PROD_RULE_ID} to {sha[:12]}, then move {len(rulesets)} " + "ruleset pin(s). From the rule change until the last pin moves, required runs fail closed (auth denied)") + try: + apply_rule(console, PROD_RULE_ID, before, match, rec, progress) + except BaseException: + progress.report() # still logged in: the rule is read again + raise + # Logged out now: a report shows R1 from its own read-back. + for rid, _, before_rs in rulesets: + apply_ruleset(gh, rid, before_rs, sha, rec, progress) + print(f"\nReleased {sha}. Records: {rec.dir}") + print("Next: start a new run on each open PR (push, or close and reopen). Do not re-run old runs.") + return 0 + + +def _plan_result(rec: Records, fails: list[str]) -> int: + print(f"\nRecords: {rec.dir}") + if fails: + print(f"PLAN FAILED: {len(fails)} precondition(s) failed:", file=sys.stderr) + for f in fails: + print(f" - {f}", file=sys.stderr) + return 1 + print("Plan OK. Nothing was changed.") + return 0 + + +def cmd_rule_test(args, run: Runner) -> int: + sha = validate_sha(args.sha) + rule_id = validate_rule_id(args.rule) + if rule_id == PROD_RULE_ID: + raise ReleaseError("release-rule-test refuses the production rule R1; use `make release`") + rec = Records(f"rule-test-{rule_id}-{sha[:12]}") + print(f"Rule test: {rule_id} -> {sha}\n\nConsole:") + check_ant(run) + progress = Progress(sha, rec, lambda s: f"make release-rule-test RULE={rule_id} SHA={s}") + try: + with ephemeral_console(run) as console: + before, match = plan_rule(console, rule_id, sha, rec, None) + progress.add(f"rule-{rule_id}", f"rule {rule_id}", rule_sha(before), + lambda: rule_sha(console.retrieve(rule_id))) + print(f"\nApplying now (no confirmation): set scratch rule {rule_id} to {sha[:12]}") + try: + apply_rule(console, rule_id, before, match, rec, progress) + except BaseException: + progress.report() # still logged in: the rule is read again + raise + except BaseException: + progress.report() # e.g. a signal held during logout, raised at the context exit + raise + print(f"\nRule test passed. Records: {rec.dir}") + return 0 + + +def _raise_on_signal(signum, _frame): + raise KeyboardInterrupt(f"signal {signum}") + + +def main(argv: Sequence[str] | None = None, run: Runner | None = None) -> int: + run = run or subprocess.run + p = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + sub = p.add_subparsers(dest="cmd", required=True) + for name in ("plan", "release"): + s = sub.add_parser(name) + s.add_argument("--sha", required=True) + s.add_argument("--no-console", action="store_true") + s = sub.add_parser("rule-test") + s.add_argument("--rule", required=True) + s.add_argument("--sha", required=True) + args = p.parse_args(argv) + if hasattr(sys.stdout, "reconfigure"): + sys.stdout.reconfigure(line_buffering=True) # keep our lines in order with ant's output + for sig in (signal.SIGTERM, signal.SIGHUP): + signal.signal(sig, _raise_on_signal) + CLEANUP_FAILURES.clear() + REPORTS_PRINTED.clear() + try: + if args.cmd == "rule-test": + rc = cmd_rule_test(args, run) + else: + rc = cmd_plan(args, run, apply=args.cmd == "release") + except ReleaseError as e: + print(f"\nREFUSED: {e}", file=sys.stderr) + rc = 2 + except KeyboardInterrupt: + if REPORTS_PRINTED: + print("\nInterrupted after a write was sent. Follow the RELEASE INCOMPLETE report above.", + file=sys.stderr) + else: + print("\nInterrupted before any write was sent. Nothing was changed.", file=sys.stderr) + rc = 130 + if CLEANUP_FAILURES: + print(f"\nCLEANUP FAILED ({len(CLEANUP_FAILURES)}): see the ERROR lines above.", file=sys.stderr) + rc = rc or 3 + return rc + + +if __name__ == "__main__": + sys.exit(main())