From 47c7b2eeb935eed546b01db45055bbad671e9acf Mon Sep 17 00:00:00 2001 From: Irving Popovetsky Date: Tue, 27 Oct 2026 14:45:24 -0700 Subject: [PATCH 1/5] Phase 3 live results: record verification results, correct four spec claims, update release procedure Co-Authored-By: Claude Opus 5.5 --- docs/build-plan.md | 34 +++++++------ docs/claude-review-design.md | 93 ++++++++++++++++++++++++++++-------- 2 files changed, 93 insertions(+), 34 deletions(-) diff --git a/docs/build-plan.md b/docs/build-plan.md index 817536b..64cbeec 100644 --- a/docs/build-plan.md +++ b/docs/build-plan.md @@ -55,7 +55,7 @@ Tasks: Owner: - [x] Owner: create a scratch source repo in the org for the V tests, separate from claude-review-test ([claude-review-p0-source](https://github.com/OperationCode/claude-review-p0-source)). -- [x] Owner: Anthropic Console: a dedicated workspace with a hard monthly spend limit and a service account. Create the production federation rule as in spec "Auth and cost": pattern mode, subject prefix `repo:OperationCode@10479364/*`, scope `workspace:developer`, the review workspace only, and the placeholder SHA `0000000000000000000000000000000000000000` in `job_workflow_ref`. Read it back; if the Console rejects the placeholder, stop and resolve the order of rule ID and SHA first (spec V26 a). Its ID goes into the central code; phase 3 sets the candidate SHA. No API key. Done 2026-10-27: workspace `claude-review`, service account `claude-review`, rule R1 `claude-review-prod-r1` created; the Console accepted the placeholder SHA (V26 a). The read-back is not recorded yet; do it before V26 b. +- [x] Owner: Anthropic Console: a dedicated workspace with a hard monthly spend limit and a service account. Create the production federation rule as in spec "Auth and cost": pattern mode, subject prefix `repo:OperationCode@10479364/*`, scope `workspace:developer`, the review workspace only, and the placeholder SHA `0000000000000000000000000000000000000000` in `job_workflow_ref`. Read it back; if the Console rejects the placeholder, stop and resolve the order of rule ID and SHA first (spec V26 a). Its ID goes into the central code; phase 3 sets the candidate SHA. No API key. Done 2026-10-27: workspace `claude-review`, service account `claude-review`, rule R1 `claude-review-prod-r1` created; the Console accepted the placeholder SHA (V26 a). The read-back output is still not recorded; V26 b passed at the candidate SHA (2026-10-27), which shows R1 held it. - [ ] Owner: the test workspace (reused for evals, spec P2-1), and a federation rule for `workflow_dispatch` runs in `OperationCode/.github` whose job uses the environment `eval`: pattern mode, the same subject prefix, exact claims `repository_owner_id`, `repository_id`, `event_name` and `environment`, scope `workspace:developer`, that workspace only (spec "Auth and cost"). Its IDs are in `evals.py`. The rule cannot name `eval.yml`: any `.github` workflow that uses `eval` can match, so the owner's approval and the workflow unit test (1c) hold it to `eval.yml`. Test it as in spec V27 before phase 2. - [ ] Owner: on `.github`, create the GitHub environment `eval`: the owner is a required reviewer for every run, and any branch may deploy to it. "Prevent self-review" is off temporarily (owner, 2026-10-26), so that the coordinator, which runs with the owner's token, can approve eval runs it started from reviewed branches; the owner turns it on again in phase 5. Admin bypass stays on. During phase 2 the coordinator's approval stands in for the owner's under the owner's delegation. Before approving a run, check the run that waits for approval: the workflow file and commit it runs, every job in it that uses `eval`, and their effective permissions (no GitHub write). Also check that the PR does not change `eval.yml`, any other workflow file, the eval code, the prompts, the fixtures or the federation setup in a harmful way, and that `test.yml` passed. The approval is the control; the residual risk is an approver who approves without reading. Exception to the no-execute rule (K-4): evals of PRs to this repo may run that PR's code, only after an owner approves the `eval` environment run. Needed before phase 2. - [x] Owner: a test workspace with a very small spend limit for V9. @@ -153,20 +153,24 @@ Run the spec's workflow acceptance tests under a test org ruleset that requires - [x] Production IDs in `analyze.py` and the release tests in CI (the candidate S1). R1 `claude-review-prod-r1` (`fdrl_01N2ZktTiVKqUvvXwywBFfys`) and the production service account are set; `test.yml` runs `pytest tests/unit -m release`. - [x] V26 a: R1 created at the placeholder SHA and accepted by the Console (2026-10-27); R2 `claude-review-p3-r2` created the same way, inert until the release simulation. - [x] P3-2: an over-long string no longer fails a pass. Release eval run 40517202830 at S1 matched 12 of 13: one supply-chain quote went over 300 characters, which the API cannot enforce. Analyze now cuts every string over its schema `maxLength` (summary, file, quote, title, explanation) to that limit and keeps the finding (spec P3-2). Fix round 1: a cut file path ends in "/", so it names no changed file; when the file, title or quote is at its limit, the inline comment key covers every shown field, so a cut never hides a distinct explanation. Fix round 2: a redaction can make two titles equal far below the limit, so the key of every finding covers every shown field, at any length. Fix round 3: each inline comment carries the D14 key and the full key, both of the redacted shown text; one run dedupes by full key, a later run skips a D14 key already on the PR and shows the finding in full in the summary when its full key is new. Fix round 4: an earlier comment counts only when its text checks out (the D14 skip needs the inline layout, path, side and title; "see the earlier comment" needs the exact body at the same line), and an unverified high or critical finding is never cut from the summary. A candidate with this change needs its own eval. -- [ ] V26 b, before the acceptance tests: after the owner sets the production rule to the candidate SHA, a run at that SHA gets a token and a run at another SHA is denied. -- [ ] V26 c, before V22 b: release steps 1 to 6 with a scratch rule R2 on the test workspace, R1 unchanged. Include the second exchange at the release SHA, and check that the workflows rule accepts a scratch ruleset pinned to a SHA that is only on a non-default branch of `.github`. -- [ ] A fork PR from an account outside the org, including a draft that later becomes ready. The review starts with no approval click (V8 b). -- [ ] V8 a: a fork PR whose own workflow has a job named `oc-claude-review` that starts before the required run. -- [ ] V11: retarget from a non-default branch. -- [ ] V12: repo-level Actions settings. -- [ ] V22 a and b: pin behind main, then a pin move with PRs open. -- [ ] V23: a same-path workflow file in the target repo. -- [ ] V25: the pilot bypass. Maintain and Admin can merge past a failed run, Write cannot, and GitHub records each bypass. Record the role `actor_id` values, and whether a direct push to the default branch is blocked. -- [ ] The rest of the acceptance list in the spec's "Testing" section, including the outage process. Design change (spec P3-1): the invalid federation rule case runs live with a denied exchange; the exhausted budget case is covered by V9 (phase 0) and the unit tests, with no Console limit edit. +- [x] V26 b, before the acceptance tests: after the owner sets the production rule to the candidate SHA, a run at that SHA gets a token and a run at another SHA is denied. Run 40560632129 at S1 got a token (2026-10-27, 18:31Z); run 40561015054 at the probe SHA was denied, and the Console shows 6 rows `Failure` `match_claim_value_mismatch` on R1 at 18:33:49Z (owner). +- [x] V26 c, before V22 b: release steps 1 to 6 with a scratch rule R2 on the test workspace, R1 unchanged. Include the second exchange at the release SHA, and check that the workflows rule accepts a scratch ruleset pinned to a SHA that is only on a non-default branch of `.github`. S2 `9752ba7` (PR 10 into `p3-sim-base`) and M2 `1cefe42` both got a token (runs 40585724589 and 40587964524); the rule accepted SHAs that are only on non-default branches. R2 Console rows were not read, and no R2 read-back output is recorded. +- [x] Release eval at S1 (`3aa7ae9`): run 40528717338, 13 of 13 matched. +- [x] M1 tree check: M1 `590a387` and S1 have the same tree, and the R1 exchange works at M1 (run 40585413003) after the denied run at M1 (run 40583290922). +- [x] Release simulation (R2, S2, M2): steps 1 to 6 of "Each release", never merged to main. +- [ ] A fork PR from an account outside the org, including a draft that later becomes ready. The review starts with no approval click (V8 b). NOT YET RUN, pending account X. +- [ ] V8 a: a fork PR whose own workflow has a job named `oc-claude-review` that starts before the required run. NOT YET RUN, pending account X. A same-repo version by an org admin passed (PR 36, run 40572853203 started first; the box stayed blocked). +- [x] V11: retarget from a non-default branch. The retarget starts no run and the box stays blocked until a reopen run passes (PR 29, run 40561903948); an old pass on the same head counts (PR 30). +- [x] V12: repo-level Actions settings. With Actions disabled, and with a restrictive allowed-actions list, the required run still ran (runs 40562372022 and 40562688524). Settings restored. +- [x] V22 a and b: pin behind main, then a pin move with PRs open. `job.workflow_sha` equaled the pin in 33 of 33 attempts. After a forward pin move the old passes do not count; after a move back they count again (spec "Pinned SHA only"; PRs 28 and 37, 21:32 to 21:40Z). +- [ ] V23: a same-path workflow file in the target repo. NOT YET RUN, pending account X. A same-repo version by an org admin passed (PR 35, run 40572849014). +- [ ] V25 (NOT YET RUN, pending account X): the pilot bypass. Maintain and Admin can merge past a failed run, Write cannot, and GitHub records each bypass. Record the role `actor_id` values, and whether a direct push to the default branch is blocked. +- [x] The rest of the acceptance list in the spec's "Testing" section that needs no account X, including the outage process (spec "Phase 3 results"). Design change (spec P3-1): the invalid federation rule case ran live with two denied exchanges (runs 40561015054 and 40583290922); the exhausted budget case is covered by V9 (phase 0) and the unit tests, with no Console limit edit (ruling Q7). +- [ ] The acceptance cases that need account X: V6 f as a Write user, and the web UI "Re-run all jobs" button on a ruleset-started run (NOT YET RUN; the override cases ran with the REST re-run). Also not run, covered by unit tests: gate cancelled after inline comments with a refused summary write, approver role removed, edited override, artifact upload removed. Owner: -- [ ] Owner: set `job_workflow_ref` of the production federation rule to the candidate release SHA. The candidate's code must name that rule ID (spec "Auth and cost", "Each release"). +- [x] Owner: set `job_workflow_ref` of the production federation rule to the candidate release SHA. Done 2026-10-27 (S1, then M1 `590a387`; the run at M1 got a token). The candidate's code must name that rule ID (spec "Auth and cost", "Each release"). - [ ] Owner: an account outside the org opens the fork PRs. - [ ] Owner: test users with Write, Maintain and Admin on claude-review-test for V25. @@ -190,6 +194,7 @@ Exit criteria: - [ ] Start a run on each open PR in the pilot repos (push, or close and reopen). - [ ] 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. +- [ ] Keep the fixture eval results as an artifact: `eval.yml` uploads none today, so a release eval's per-case results exist 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". Depends on: phase 3. @@ -243,8 +248,9 @@ One federation rule allows one SHA, and every release is validated at its final - [ ] 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), then the owner sets R2 to that SHA and reads it back. 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. +- [ ] 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. +- [ ] 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). diff --git a/docs/claude-review-design.md b/docs/claude-review-design.md index 38592d9..2436fef 100644 --- a/docs/claude-review-design.md +++ b/docs/claude-review-design.md @@ -108,7 +108,7 @@ Facts about ruleset-required workflows ([troubleshooting rules](https://docs.git | Org member without write access to the repo | Same as an outside contributor (V8). | | Repo writer | Cannot forge or remove the rule. Can approve another writer's PR, which is the 1 approval a merge needs. Can cancel, re-run and delete workflow runs ([workflow runs API](https://docs.github.com/en/rest/actions/workflow-runs#delete-a-workflow-run)). A cancel only blocks. A re-run reads the live PR state. Deleting a newer run on a head SHA can expose an older pass on that SHA (V6 f). | | Repo maintainer | Same as a repo writer under the org ruleset. On a pilot repo, can merge past a failed run with the pilot bypass, also on their own PR, which GitHub records. | -| Repo admin | Cannot remove or change the rule. Same run powers as a writer. Can change repo-level Actions settings (V12). On a pilot repo, can merge past a failed run with the pilot bypass. | +| Repo admin | Cannot remove or change the rule. Same run powers as a writer. Can change repo-level Actions settings, which do not affect the required run (V12, phase 3: with Actions disabled, and with an allowed-actions list that excludes the actions the workflow uses, the required run still started and ran). On a pilot repo, can merge past a failed run with the pilot bypass. | | Org admin | Bypasses the ruleset through a pull request, and changes it. | - **Why a same-name job cannot stand in for the gate.** In [claude-review-test](https://github.com/OperationCode/claude-review-test), a repo ruleset that required a check by name was satisfied by a passing job from a fork PR's own workflow (the positive control). Under the workflows rule, the same fake job did not satisfy the rule when it passed, and did not block it when it failed. This held on fork and same-repo PRs, when the fake finished after the required run, and when it finished 179 seconds before it (T6, T7). In these tests the scratch repo was both the workflow source and the target, and the pin was the tip of main. Not yet tested: a fake job that starts before the required run, and a fork from a non-member account (V8); a separate source repo with a pin behind main (V22); a same-path workflow in the target (V23). @@ -172,7 +172,7 @@ Facts about ruleset-required workflows ([troubleshooting rules](https://docs.git + 1 human approval (+ code owner where CODEOWNERS applies) ``` -The required run succeeds only when every job succeeds, and that run result is the gate. Only the gate job may fail because of policy. The other jobs fail only on an infrastructure error, or, for dependency-review, on a vulnerable dependency. Any failed, timed-out or cancelled job fails the run, whatever the gate job does. The gate job still runs (`always()`) to explain the failure in the summary, and treats it as not waivable. No path makes the run `success` without an exit 0 from the gate job. +The required run succeeds only when every job succeeds, and that run result is the gate. Only the gate job may fail because of policy. The other jobs fail only on an infrastructure error, or, for dependency-review, on a vulnerable dependency. Any failed, timed-out or cancelled job fails the run, whatever the gate job does. The gate job still runs (`always()`) to explain the failure in the summary, and treats it as not waivable. A normal run cancel does not stop it: live (phase 3, run 40565538379), the run was `cancelled` right after analyze and dependency-review finished, and the gate job ran to success and wrote a `pass` summary. The run conclusion stayed `cancelled` and the box blocked, so the gate holds. Only a force-cancel stops the gate job. No path makes the run `success` without an exit 0 from the gate job. Only the gate job writes to GitHub, and it writes only comments. It has no Anthropic credential and no `id-token`. Its `actions: read` scope only lists runs for the final check. Its `contents: read` scope is for the re-validation reads (the base ref tip and the compare endpoint), which need it on private repos. No job writes a check run through the Checks API, so no job has `checks: write`. The workflow sets `permissions: {}` at the top, and each job lists its own scopes, so every scope a job does not list is `none`. @@ -203,7 +203,7 @@ Only the gate job writes to GitHub, and it writes only comments. It has no Anthr ## Job handoff and provenance -Jobs pass data only through artifacts from the same run and the same run attempt. Artifact names carry the attempt (`oc-bundle-a`, `oc-pass--a`), because names are unique in a run and the artifacts of earlier attempts stay. Every upload has `if-no-files-found: error` and `retention-days: 3`, and every download has `digest-mismatch: error`. +Jobs pass data only through artifacts from the same run and the same run attempt. Artifact names carry the attempt (`oc-bundle-a`, `oc-pass--a`), because names are unique in a run. The artifacts of earlier attempts do not stay: after a re-run the API lists only the new attempt's artifacts (phase 3: runs 40567379756 and 40568067319 showed only `-a2` artifacts, and the `-a1` ones were gone). No job reads an earlier attempt's artifact, so the gate does not depend on them and still fails closed. The residual is audit only: an earlier attempt's pass records cannot be read back later, so keep that evidence from the job log and the summary. Every upload has `if-no-files-found: error` and `retention-days: 3`, and every download has `digest-mismatch: error`. - Each producer writes one envelope file. Trusted Python code (the producer step, not a `sha256sum` step) computes the SHA-256 of the file bytes. Then it uploads the file as one artifact with `actions/upload-artifact`. It sets two job outputs: the `artifact-id` and that file digest. The `artifact-digest` of the upload step is the digest of the zip. A consumer cannot recompute it from the extracted file. A workflow step may check it as an extra control. The consumer gate does not depend on it. - Each consumer downloads with `actions/download-artifact` by exact `artifact-ids`, never by name or pattern, and never from another run. It checks the SHA-256 of the extracted file bytes against the file digest output of the producer before it parses anything. A mismatch fails the run. When a producer failed, its artifact id output is empty, and `actions/download-artifact` with an empty `artifact-ids` would download every artifact of the run. So each download step is guarded by an explicit check that its id is not empty, and the gate job never downloads without one. In the gate job the guard is the step `if:` above. In analyze, a step before the download fails the job when the bundle id is empty. A failed download step ends the gate job before the publish step, so that run fails on the job result. The publisher treats a missing directory or an empty digest as the `artifact` reason (not waivable) and still posts the summary. @@ -500,19 +500,21 @@ An override is a PR comment from an authorized person: ### Which run counts -The gate record is a run of the central workflow on the PR head SHA. GitHub creates the run on the PR event and sets its result from its jobs. The rule reads only runs of the pinned central workflow. +The gate record is a run of the central workflow on the PR head SHA. GitHub creates the run on the PR event and sets its result from its jobs. The rule reads only runs of the central workflow at the pinned SHA. A run of the same workflow at another SHA is ignored (phase 3, V22 b). ``` head SHA H run R1 of the central workflow success run R2 of the central workflow queued | in_progress | failure | success - any other workflow run, any job ignored by the rule, whatever its name + any run at another SHA of the workflow ignored by the rule, newer or not + any other workflow run, any job ignored by the rule, whatever its name ``` - **Fail closed.** A run is `success` only when every job succeeds. A failed, timed-out or cancelled job fails the run. On a new head SHA the box blocked while the required run was pending (T7). A new head SHA with no run blocks. -- **Newest run decides (build prerequisite).** A reopen, a re-run, or two PRs with one head make several runs on one SHA. The docs do not say which run the rule uses. The design requires that the rule use the newest run on the head SHA, and that run's newest attempt: a queued, in-progress, cancelled, timed-out or failed newest run blocks, whatever an older run shows. V6 tests this first. If V6 shows that an older run or the last completed run can decide, building stops and the gate design changes. No extra read in the gate job can fix that order. +- **Newest run decides (build prerequisite), among the runs at the pinned SHA.** A reopen, a re-run, or two PRs with one head make several runs on one SHA. The docs do not say which run the rule uses. The design requires that the rule use the newest run on the head SHA, and that run's newest attempt: a queued, in-progress, cancelled, timed-out or failed newest run blocks, whatever an older run shows. V6 tests this first. If V6 shows that an older run or the last completed run can decide, building stops and the gate design changes. No extra read in the gate job can fix that order. - **Final check (defense in depth).** As its last step before exit 0, the gate job reads the snapshot and the override authority again, and lists the runs for the head SHA (`GET /repos/{owner}/{repo}/actions/runs?head_sha=...`). It exits non-zero if the snapshot changed, if the grant no longer holds, or if a run with the central workflow's path and a higher run ID exists (`superseded`). A same-path workflow in the target repo also matches; that can only make a run fail. A run list that was not read completely, or a run with no `path`, also counts as `superseded`. This shortens the time between the authority read and the run result. It is not atomic with GitHub's record of the run's completion, and a new run can be missing from the list for a short time (V24). So safety rests on newest-run selection, not on this check. The list is not filtered by PR, so it matches the box (V6 g2); do not add a PR filter. If the final check turns the result to fail, the job updates the summary before it exits, if it can. - **Re-runs.** "Re-run all jobs" makes a new attempt with the same run ID. Each attempt reads the live PR, comments, timeline and permissions when its gate job runs. A re-run of the newest run can pass; a re-run of an older run exits `superseded` when it sees the newer run (V6 e). +- **Pinned SHA only (phase 3).** The box counts only runs of the workflow at the SHA the ruleset pins. Observed: after the pin moved from S1 to a probe SHA and back to S1, PR 28 read mergeable from 18:34:53Z on its older S1 pass, although the newest run on its head, at the probe SHA, had failed. After a pin move M1 to M2 and back to M1, PRs 28 and 37 read blocked at once on the move to M2 (the M1 passes did not count), then clear again from 21:38:44Z on the move back (the M1 passes counted, and the newer M2 runs did not decide). Why it still fails closed on a forward move: every open PR blocks until a run at the new pin passes. Residual on a move back to an earlier SHA: every earlier pass at that SHA counts again, also on a PR whose newer run at the newer release failed. The merged code then passed a review by a release the owner pinned, not by the current one. Rule: after any pin move, including a move back, start a new run on each open PR and merge only on that run; never use a rollback to clear a block. An org owner can start a rollback only by editing the ruleset, which is a logged org-owner action. - **Per head SHA, not per PR.** The box reads runs per head SHA. A run of any PR with that head decides every PR with that head, in both directions (V6 g2). **Residual risk: an earlier pass on the same SHA.** A run's result is fixed when the run ends. Under newest-run selection, a newer run on head SHA H replaces an earlier pass as soon as it exists. Nothing else does: @@ -528,13 +530,14 @@ The gate record is a run of the central workflow on the PR head SHA. GitHub crea +-- no new run: the old pass stays usable, with no time limit ``` +- **Pin rollback (V22 b).** A move of the pin back to an earlier release SHA revives the passes at that SHA (see "Pinned SHA only"). This is a residual of the same kind as the list above. - **Exact residual.** Invalidating an override (revoke, or the grant author's role removal) takes effect only when a newer run on that head exists. If nobody starts one, it never takes effect. The same holds for an H to B to H return pushed with a `GITHUB_TOKEN`, a branch restore that starts no run, and a deleted newer run if V6 (f) shows the older pass decides. Between a reopen and the creation of its run, the old pass can also count (V6 a records the gap). This is a reduction of the gate guarantee. The owner accepts it in writing before org-wide enforcement (rollout step 7). Strict invalidation needs another gate design: a completed run cannot react to an event that starts no run. - The ruleset blocks force pushes on the base branch only, not on the head branch, so H to B to H is possible. - In each case the code that can merge is code that an earlier run of the pinned workflow passed. Human approval is a separate control. The push of B dismissed every earlier approval ("dismiss stale approvals on push"), so a merge needs a fresh approval. Role removal on an unchanged head is not a push and dismisses nothing. A new approval does not re-check the old override. - **Cross-PR effect (V6 g2).** Two PRs with one head H share the runs on H. A failing or pending run on PR 2 blocks PR 1. A pass on PR 2 clears PR 1, also when PR 1 has no override. An override covers the code pair (head and merge base), not one PR, so an override on PR 2 can clear PR 1. Anyone who can open a PR with head H (a fork can fetch the commit) can start a run on H, so they can block PR 1 or start a new run on it. - For an immediate stop, a maintainer closes the PR. Reopen then starts a new run; it is not an atomic swap of the old result. -**Summary writes.** One sticky comment, best effort. Each gate job writes its own result into it, so the last writer wins, whichever run that is. The summary says "result for run R, attempt N", names its head SHA and merge base, and links to its run. A job that fails after its summary write can leave a `pass` summary next to a failed run, and a repo writer can post a bot comment with the marker. Neither changes the gate. The summary says: "The result that counts is the required workflow in the merge box." +**Summary writes.** One sticky comment, best effort. Each gate job writes its own result into it, so the last writer wins, whichever run that is. The summary says "result for run R, attempt N", names its head SHA and merge base, and links to its run. A job that fails after its summary write can leave a `pass` summary next to a failed run, a normal cancel of the run can leave a `pass` summary next to a `cancelled` run (the gate job keeps running), and a repo writer can post a bot comment with the marker. Neither changes the gate. The summary says: "The result that counts is the required workflow in the merge box." ### Pilot and enforcement @@ -613,8 +616,11 @@ token lifetime: 10 minutes 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), then set R2 to it; if the trees - differ, the merge SHA is a new candidate: go back to step 3 -> release SHA + 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) @@ -622,6 +628,8 @@ token lifetime: 10 minutes wait until each has completed at the release SHA, then delete R1 (V22) ``` + 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"). + 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. 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. @@ -707,7 +715,7 @@ token lifetime: 10 minutes - revoke: pass by override, delete the comment, close and reopen: the box blocks from the new run's creation (V6) - recovery: fail on a hard block or registry outage, then pass on a later run at the same head - pilot bypass: under a ruleset shaped like the pilot ruleset, a failed run shows the red X; a user with Maintain and a user with Admin can merge with the bypass, a user with Write cannot, and GitHub records the bypass (V25) - - gate job failure at each point: cancel it before it starts, after inline comments, after the summary; summary write refused; each must leave the run failed or cancelled and the box blocked + - gate job failure at each point: a normal run cancel before the gate job starts does not stop it (the run is `cancelled`, the gate job still runs, the box blocks); a force-cancel stops the gate job, and is the way to cancel it after inline comments and after the summary; summary write refused; each must leave the run failed or cancelled and the box blocked - whole run cancelled during collect or analyze: the run fails and the box blocks - two PRs to the default branch with the same head SHA (from two branches): record which run decides each (V6 g) - analyze job cancelled; artifact upload removed; dependency-review failing @@ -722,6 +730,50 @@ token lifetime: 10 minutes - invalid federation rule (outage process): a denied exchange, run live. An exhausted budget is covered by V9 and the unit tests, with no Console limit edit (phase 3 design change, P3-1) - **Live smoke test:** the scratch repo above, before any real repo. +## Phase 3 results (smoke test on claude-review-test, 2026-10-27) + +Target: [claude-review-test](https://github.com/OperationCode/claude-review-test), test ruleset TR (org ruleset 25835887) pinned to the candidate or release SHA, bypass list empty. S1 = `3aa7ae9ec072ca5d77b895dac12665f2370983de`. M1 = `590a387333c999a5d30dad8022a00383251ce16a`, the squash merge of S1 (PR 9). Times are UTC. The V items above hold the detail for V6 f, V8, V11, V12, V22, V23, V25 and V26. Cases that need a non-member fork author (account X, being arranged by the owner) are marked NOT YET RUN. + +| Item | Result | Evidence | +|---|---|---| +| Release eval at S1 | Pass: 13 of 13 matched, 0 missed | [`.github` run 40528717338](https://github.com/OperationCode/.github/actions/runs/40528717338), approved 2026-10-27 15:13:14Z; tokens: input 39399, output 19901, cache write 56524, cache read 113048 | +| M1 tree check | Pass: S1 and M1 have tree `6ace36e90f776f1fd9e821851b42dfc79dad13c0`; `git diff S1 M1` empty | `origin/main` = M1 | +| V26 a, b, c | Pass (see V26) | R1 and R2 accepted at the placeholder; runs [40560632129](https://github.com/OperationCode/claude-review-test/actions/runs/40560632129) (accept), [40561015054](https://github.com/OperationCode/claude-review-test/actions/runs/40561015054) (deny), [40585724589](https://github.com/OperationCode/claude-review-test/actions/runs/40585724589) (S2 via R2), [40587964524](https://github.com/OperationCode/claude-review-test/actions/runs/40587964524) (M2 via R2) | +| Deny at the merge SHA, before the second edit | Pass: run [40583290922](https://github.com/OperationCode/claude-review-test/actions/runs/40583290922) at M1, PR 28 reopened 21:02:02 to 21:02:08Z; 3 passes `failed (authentication failed)`; gate fail; box blocked from 21:02:01Z | Exchange window 21:02:36 to 21:02:57Z, R1 then at S1. The owner reports every other R1 row in the test windows was `Success`; the reports do not say whether that covers this window, so its Console reason is not confirmed | +| Accept at M1 after the second edit | Pass: run [40585413003](https://github.com/OperationCode/claude-review-test/actions/runs/40585413003), PR 28 reopened 21:16:44Z, 3 `ok`, box clear 21:18:12Z; PR 37: run [40586089942](https://github.com/OperationCode/claude-review-test/actions/runs/40586089942), clear 21:22:46Z | R1 at M1; ready for phase 4 | +| Simulation (R2, S2, M2) | Pass: see V26 c | S2 `9752ba7c04d796822c64c670415d1046607c3c03`, M2 `1cefe4259bc26197ec4b063e8399ef856efdd4f2`, PR 10 into `p3-sim-base`, never main | +| V22 a, V22 b | Pass, with a rollback residual (see "Which run counts") | V22 | +| V11 | Pass | V11 | +| V12 | Pass; Actions settings do not affect the required run | V12 | +| V6 f | Residual confirmed as an org admin; NOT YET RUN as a Write user | V6 | +| V8, V23 | NOT YET RUN (pending account X); partial same-repo evidence | V8, V23 | +| V25 | NOT YET RUN (pending account X) | V25 | +| Web UI "Re-run all jobs" | NOT YET RUN (pending account X) | "Verify later" | +| Outage: invalid federation rule | Pass, live: two denied exchanges | runs 40561015054 and 40583290922 | +| Outage: exhausted budget | Covered by V9 and the unit tests; no Console limit edit (ruling Q7, P3-1) | V9 | + +Acceptance list, run without account X (V6 gives 33 run attempts and 6,153 observer readings on 10 PRs; none showed a clear box on a failed, cancelled or missing run of the workflow at the pin, except the documented residuals: the reopen-to-run gap, and an older pass exposed after run deletion). Secret scan of 33 run attempts for `eyJ`, `sk-ant-` and bearer strings: 0 hits. + +| Case | Result | Evidence | +|---|---|---| +| Dry run at `f808dd0`; old PRs open before the ruleset | Pass: 3 passes `failed (federation not configured)`, no exchange, box blocked; the 12 old PRs started no run and read blocked | run [40560225901](https://github.com/OperationCode/claude-review-test/actions/runs/40560225901) (PR 28, 18:27:58Z) | +| Push during a run; two quick pushes | Pass: stale runs wrote nothing and made no model calls; the newest run decided | [40563944420](https://github.com/OperationCode/claude-review-test/actions/runs/40563944420) and [40564019086](https://github.com/OperationCode/claude-review-test/actions/runs/40564019086); [40564287037](https://github.com/OperationCode/claude-review-test/actions/runs/40564287037) and [40564294168](https://github.com/OperationCode/claude-review-test/actions/runs/40564294168) | +| Override: re-run by REST after fail; reopen then override; override then reopen (rejected); deleted before publish; push invalidates; non-waivable reason ignores it | Pass | [40568067319](https://github.com/OperationCode/claude-review-test/actions/runs/40568067319), [40568865128](https://github.com/OperationCode/claude-review-test/actions/runs/40568865128), [40571071071](https://github.com/OperationCode/claude-review-test/actions/runs/40571071071), [40571699907](https://github.com/OperationCode/claude-review-test/actions/runs/40571699907) | +| Revoke, then reopen | Pass, residual as stated: the box stayed clear until the new run was created (7 s after the reopen; one clear reading in the gap) | PR 23, comment deleted 19:24:36Z, run created 19:24:47Z | +| Recovery after a hard block | Pass: a fresh-publish hard block, then a pass on the re-run after 72 hours | PR 31, runs [40563612731](https://github.com/OperationCode/claude-review-test/actions/runs/40563612731) (a1 fail, a2 pass) | +| Whole run cancelled in collect; in analyze | Pass: run `cancelled`, gate job ran and failed, box blocked | runs 40564970467 and 40565149498 | +| Normal cancel before the gate job starts | Pass for the box; the gate job was not stopped and wrote a `pass` summary (spec correction 3) | run 40565538379 | +| Force-cancel after the summary | Pass: run `cancelled`, box blocked | run 40565910355 | +| Gate cancelled after inline comments; summary write refused | NOT RUN: covered by unit tests; a refused write cannot be caused from outside | none | +| Two PRs, one head (V6 g2) | Recorded: the newest run from either PR decides both | PRs 28 and 30, run [40564621800](https://github.com/OperationCode/claude-review-test/actions/runs/40564621800) | +| Dependabot PR | Pass: the exchange works on a Dependabot-triggered run; box clear 19:17:27Z | run [40567489483](https://github.com/OperationCode/claude-review-test/actions/runs/40567489483) | +| Repeated close and reopen: usage per run | Pass: each run's token totals are in the summary edit history | PR 32 | +| Dependency review failing | Pass: fail, not waivable | PR 18, run [40564629610](https://github.com/OperationCode/claude-review-test/actions/runs/40564629610) | +| Re-run posts no duplicate inline comment | Pass on PR 34 (0 new). PR 23 got 3 new comments on the re-run, because the model re-worded the quote: recorded | runs [40567379756](https://github.com/OperationCode/claude-review-test/actions/runs/40567379756) and [40568067319](https://github.com/OperationCode/claude-review-test/actions/runs/40568067319) | +| Egress audit mode | Pass: `egress_policy: audit` and the V10 lists in every job log | run logs | +| Approver role removed; edited override; artifact upload removed | NOT RUN: need a second account or a workflow change at S1; unit tests cover them | none | +| Fork PR, draft then ready (V8 b); V8 a and V23 from a fork; V6 f as Write; V25 | NOT YET RUN (pending account X) | none | + ## Rollout The tasks, owner actions and exit criteria for each step are in [docs/build-plan.md](build-plan.md). @@ -732,7 +784,7 @@ The tasks, owner actions and exit criteria for each step are in [docs/build-plan 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, then creates the pilot ruleset (see "Pilot and enforcement") with the release SHA and the pilot repos (first `back-end`), reads it back, and enables it. Then list open PRs and start a run on each (push, or close and reopen), because the rule does not start on PRs that were already open. 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). -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, and run deletion if V6 f did not pass). 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. +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 if V6 f did not pass). 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. ## Admin checklist (Irving) @@ -761,7 +813,7 @@ These are assumptions I could not confirm from docs. Each has a concrete test. I 3. **Answered: federation rule (build).** 2026-10-26 (UTC), pass after one fix. The final rule text is in "Auth and cost". The test rule on the test workspace used pattern mode, the subject prefix `repo:OperationCode@10479364/*`, and exact claims `event_name`, `repository_owner`, `repository_owner_id` and `job_workflow_ref` (the scratch `p0-required.yml` at one SHA). The first rule had the prefix `repo:OperationCode/*` and denied every token with reason `match_subject_prefix`, because `sub` uses the ID form. CEL mode refused the planned compound condition, so the rule has no `startsWith` and no SHA list. With the fixed prefix, the ruleset-required run of the pinned workflow got HTTP 200: a token with scope `workspace:developer` for the test workspace, valid for 10 minutes. Four denies got HTTP 401 with the generic `authentication_error`; the Console reason for each was `match_claim_value_mismatch`: another repo and workflow on `pull_request_target`; a `push` run; a `workflow_dispatch` run of the pinned file (its `job_workflow_ref` uses `@refs/heads/main`); and the same workflow at an SHA that is not in the rule. `pull_request_target` ran the file from the default branch of the target repo, not from the PR base branch (see "Workflow details"). Evidence: accept [40258763277](https://github.com/OperationCode/claude-review-test/actions/runs/40258763277) (PR [14](https://github.com/OperationCode/claude-review-test/pull/14)); other repo [40258766911](https://github.com/OperationCode/claude-review-test/actions/runs/40258766911) (PR [24](https://github.com/OperationCode/claude-review-test/pull/24)); push [40258769681](https://github.com/OperationCode/claude-review-test/actions/runs/40258769681); dispatch [40259047420](https://github.com/OperationCode/claude-review-p0-source/actions/runs/40259047420) in [claude-review-p0-source](https://github.com/OperationCode/claude-review-p0-source); unapproved SHA [40259088062](https://github.com/OperationCode/claude-review-test/actions/runs/40259088062) (PR [14](https://github.com/OperationCode/claude-review-test/pull/14)). No log held a JWT or an access token. 4. **Answered: Dependabot PRs (build).** 2026-10-25, pass. Dependabot PR [23](https://github.com/OperationCode/claude-review-test/pull/23) got the ruleset-required run as `dependabot[bot]` (event `pull_request_target`). Analyze minted an OIDC token; no secret was needed. `job_workflow_sha` equaled the pinned SHA. The gate failed as set, and the box blocked (read once with `gh pr view`, not with the observer). An override and a reopen gave a passing run, and the box cleared. `sub` does not name the actor. The federation exchange on a Dependabot run is not tested. V3 passed on other PRs, and the rule matches no actor claim. Evidence: runs [40243816279](https://github.com/OperationCode/claude-review-test/actions/runs/40243816279) (fail) and [40243930238](https://github.com/OperationCode/claude-review-test/actions/runs/40243930238) (pass after override). The docs say nothing about `id-token` for Dependabot ([troubleshooting Dependabot](https://docs.github.com/en/code-security/dependabot/troubleshooting-dependabot/troubleshooting-dependabot-on-github-actions)). 5. **Answered: mid-conversation system message plus structured output plus server-side fallback** in one request (build). 2026-10-26 (UTC), pass. One request through the federated token: `claude-opus-5-5`, top-level `system` with ``, the nonce-delimited bundle with `cache_control`, a mid-conversation `system` message as the last entry, `output_config.format` with a stripped findings schema, `fallbacks: "default"` with the beta header, effort `medium`. HTTP 200, `stop_reason` `end_turn`, top-level `model` `claude-opus-5-5`. The output parsed, and it matched both the sent schema and the full schema in a client check. The model followed the mid-conversation message. It returned one correct, grounded finding (the `if (user.role = "admin")` assignment). The cache write covered the system and the bundle, so the prefix before the pass message can be shared. The fallback did not trigger, so a real fallback attempt is not tested. The request left out the schema keywords that structured outputs do not support (see "Request shape"). The fallback in this item (pass instructions in the top-level `system`) is not needed. Evidence: run [40261749299](https://github.com/OperationCode/claude-review-test/actions/runs/40261749299) (PR [26](https://github.com/OperationCode/claude-review-test/pull/26)). -6. **Which run counts (build, first).** The docs do not say which run decides when a head SHA has several runs of the required workflow. The design requires newest-run selection (see "Which run counts"). Run on fork and same-repo PRs, with a separate source and target repo. A scratch-only build of the gate job may pause for a set time after its final check, before it exits. Criteria: with no earlier pass on the head, the box blocks unless the newest run completed with `success`; with an earlier pass, the box blocks from the moment a newer run exists, in every state but `success`. +6. **Which run counts (build, first).** The docs do not say which run decides when a head SHA has several runs of the required workflow. The design requires newest-run selection (see "Which run counts"). Run on fork and same-repo PRs, with a separate source and target repo. A scratch-only build of the gate job may pause for a set time after its final check, before it exits. Criteria: with no earlier pass on the head, the box blocks unless the newest run completed with `success`; with an earlier pass, the box blocks from the moment a newer run exists, in every state but `success`. **Phase 3 (2026-10-27), (f) as an org admin, not a Write user: residual confirmed. NOT YET RUN as a Write user (pending account X).** PR 32 had an older pass (run 40564294168) and 4 newer cancelled runs. Deleting them newest first (19:49:16 to 19:50:06Z) gave blocked, blocked, blocked, then clear after the last delete, and still clear 20 s later. This is the residual the spec states; a Write user can delete runs and is expected to see the same. - (a) Replacement states: H passes by override; revoke; a reopen starts a new run, slowed on purpose. Pass when the box blocks while the new run is queued and in progress, after it is cancelled, after it times out and after it fails, and clears when it passes. Record the gap between the reopen and the run's creation. With no reopen (absent replacement), record that the old pass stays usable (the stated residual). - (b) Completion order: run A passes its final check and pauses; the override is revoked; run B starts and fails; A then exits 0 and finishes last. Pass when the box blocks after A completes. Reverse: B passes; A, which saw B, exits `superseded` and finishes last. Record whether the box follows B (liveness). - (c) H to B to H: after H passed by override, force-push the head to B and back to H. Pass when the box blocks while the new run on H is pending and after it fails. Repeat with the return push made with a `GITHUB_TOKEN`: record that no run starts and that the box shows the old pass. Check that the push of B dismissed approvals. @@ -776,11 +828,11 @@ These are assumptions I could not confirm from docs. Each has a concrete test. I - **Still open:** the queued cells and n1 (need an owner repo variable); (f) run deletion by a writer account; the approval-dismissal part of (c) (needs a second account); (g1) with the real Restore button, and on the same-repo PR; the default-branch check (`~DEFAULT_BRANCH`). "Which run counts" now has the cross-PR result (g2). It changes again when (f) has a result. - Evidence, same-repo run then fork run: (a) in progress and failed [40235495107](https://github.com/OperationCode/claude-review-test/actions/runs/40235495107) and [40235493548](https://github.com/OperationCode/claude-review-test/actions/runs/40235493548); (a) cancelled [40235689574](https://github.com/OperationCode/claude-review-test/actions/runs/40235689574) and [40235689417](https://github.com/OperationCode/claude-review-test/actions/runs/40235689417); (a) timed out [40235846948](https://github.com/OperationCode/claude-review-test/actions/runs/40235846948) and [40235918049](https://github.com/OperationCode/claude-review-test/actions/runs/40235918049); (a) cleared [40236150113](https://github.com/OperationCode/claude-review-test/actions/runs/40236150113) and [40236222191](https://github.com/OperationCode/claude-review-test/actions/runs/40236222191); (b1) A [40236289458](https://github.com/OperationCode/claude-review-test/actions/runs/40236289458) and [40236352621](https://github.com/OperationCode/claude-review-test/actions/runs/40236352621), B [40236386756](https://github.com/OperationCode/claude-review-test/actions/runs/40236386756) and [40236446523](https://github.com/OperationCode/claude-review-test/actions/runs/40236446523); (b2) A [40236783304](https://github.com/OperationCode/claude-review-test/actions/runs/40236783304) and [40236844122](https://github.com/OperationCode/claude-review-test/actions/runs/40236844122), B [40236801503](https://github.com/OperationCode/claude-review-test/actions/runs/40236801503) and [40236861667](https://github.com/OperationCode/claude-review-test/actions/runs/40236861667); (c) user push [40239816420](https://github.com/OperationCode/claude-review-test/actions/runs/40239816420) and [40239867888](https://github.com/OperationCode/claude-review-test/actions/runs/40239867888); (c) token push [40240144869](https://github.com/OperationCode/claude-review-test/actions/runs/40240144869) (same-repo only); (d) [40241159046](https://github.com/OperationCode/claude-review-test/actions/runs/40241159046) and [40243374015](https://github.com/OperationCode/claude-review-test/actions/runs/40243374015); (e) re-run newest [40237033260](https://github.com/OperationCode/claude-review-test/actions/runs/40237033260) and [40237091065](https://github.com/OperationCode/claude-review-test/actions/runs/40237091065); (e) re-run older [40237142925](https://github.com/OperationCode/claude-review-test/actions/runs/40237142925) and [40237192544](https://github.com/OperationCode/claude-review-test/actions/runs/40237192544); (g1) fork only [40242670876](https://github.com/OperationCode/claude-review-test/actions/runs/40242670876); (g2) second PRs [21](https://github.com/OperationCode/claude-review-test/pull/21) run [40240449144](https://github.com/OperationCode/claude-review-test/actions/runs/40240449144) and [20](https://github.com/OperationCode/claude-review-test/pull/20) run [40240149876](https://github.com/OperationCode/claude-review-test/actions/runs/40240149876). 7. **Answered: head SHA.** Job checks and required runs of a `pull_request_target` workflow land on the PR head SHA, on fork and same-repo PRs, although `GITHUB_SHA` is the default branch commit (T4, T6). -8. **Forging (blocking).** Answered in part: a rule bound to a check name was satisfied by a fake job from a fork PR's own workflow; the workflows rule ignored the same fake job when it passed or failed, when it finished after the required run, and when it finished 179 seconds before it (T6, T7). The fake always started after the required run, the forks belonged to org admins, and the scratch repo was both workflow source and target. Use a source repo separate from the target. Still to test with an account that is not an org member, and with an org member without write access: (a) a fork PR whose own workflow has a same-name job that starts before the required run; pass when the box follows only the required run; (b) the real `pull_request_target` review starts with no approval click; if it waits, the goal "no maintainer step" fails for outside contributors and the owner decides. +8. **Forging (blocking).** Answered in part: a rule bound to a check name was satisfied by a fake job from a fork PR's own workflow; the workflows rule ignored the same fake job when it passed or failed, when it finished after the required run, and when it finished 179 seconds before it (T6, T7). The fake always started after the required run, the forks belonged to org admins, and the scratch repo was both workflow source and target. Use a source repo separate from the target. Still to test with an account that is not an org member, and with an org member without write access: (a) a fork PR whose own workflow has a same-name job that starts before the required run; pass when the box follows only the required run; (b) the real `pull_request_target` review starts with no approval click; if it waits, the goal "no maintainer step" fails for outside contributors and the owner decides. **Phase 3 (2026-10-27): NOT YET RUN (pending account X).** The fork cases need a non-member author. Partial evidence only, from a same-repo PR by an org admin (PR 36, V8 a): a fake job `oc-claude-review` in `.github/workflows/p3-fake-gate.yml` (`on: pull_request`) started first (run 40572853203, lower ID than the central run 40572853562) and passed at 19:51:35Z; the central run failed at 19:53:01Z; the box stayed blocked throughout. 9. **Answered: spend limit behavior (build).** 2026-10-26 (UTC), pass. Test workspace with a deliberately tiny hard monthly limit, sequential calls of about 61k input tokens each, through the federated token. Calls 1 to 7 got HTTP 200. Call 8 got HTTP 400 `invalid_request_error` "You have reached your specified workspace API usage limits. You will regain access on 2026-11-01 at 00:00 UTC." A fresh token exchange after that still got HTTP 200, and a call with the new token got the same 400. Analyze failed, the gate job failed with `needs.analyze=failure` (not waivable) even with a forced pass, and the merge box was `BLOCKED`. Overshoot: the estimate from `usage` passed the limit after call 4, and the first deny came about 39 s and 3 calls later. The overshoot equals the cost of the calls that started during the lag: spend reached 87 percent over the limit. The owner confirmed that the Console spend matched the estimate. This repo records no currency amounts (P0-17). Billing of cancelled calls is not tested. The design changes are in "Auth and cost". Evidence: run [40261904117](https://github.com/OperationCode/claude-review-test/actions/runs/40261904117) (PR [26](https://github.com/OperationCode/claude-review-test/pull/26)). 10. **Answered: egress allowlist (build).** 2026-10-27 (UTC), pass, with a harden-runner hang. Kits: the built workflow and `review/` staged in the scratch source repo as K1 `5a7a569` (audit), K2 `7399e8b` (block, `disable-telemetry: true`, `disable-sudo-and-containers: true`, per-job lists) and K3 `df8ff51` (K2 without `disable-sudo-and-containers`), with `setup-uv` v10.3.2 from K2 on. Each kit ran on same-repo PR 27 (clean), fork PR 19 (clean) and fork PR 18 (vulnerable dependencies). The audit run gave the endpoints per job (see "Workflow details"). In block mode no job needed a host off its list, every analyze job did the token exchange, and all 27 Claude passes were `ok`. A clean PR passed the gate in block mode (PR 19 in K2 and K3, PR 27 in K3 after a re-run), and PR 18 failed it through dependency review in every round. Hang: 2 of 24 block-mode jobs (one in K2, one in K3, both on PR 27; 2 of 6 block-mode runs) ran every step, then hung in "Complete job" until GitHub cancelled them about 15 minutes later. The gate failed closed. The K3 run passed on a REST re-run; the K2 run was not re-run. Only those 2 of the 36 jobs logged dropped IPs: Azure DNS and IPs of allowlisted hosts, after the agent restarted systemd-resolved, and then an Azure IP that the runner likely needs to finish the job. The symptoms match the open harden-runner [issue 708](https://github.com/step-security/harden-runner/issues/708), which reports that in block mode on GitHub-hosted runners with a GitHub DNS change ([runner-images issue 14925](https://github.com/actions/runner-images/issues/14925)), the agent drops Azure DNS `168.63.129.16` and the job hangs in cleanup. The runner region of the two hung jobs was not recorded, so the cause is not confirmed. v2.22.2, the latest release, has no fix, and audit mode is the known workaround. It is not a missing list entry. `disable-sudo-and-containers` is not the cause (the K3 hang had it off), but it adds a slow docker purge. Ruling: audit mode with the measured lists (K-14). Evidence: runs in claude-review-test; audit [40433731499](https://github.com/OperationCode/claude-review-test/actions/runs/40433731499), [40433735510](https://github.com/OperationCode/claude-review-test/actions/runs/40433735510), [40433740291](https://github.com/OperationCode/claude-review-test/actions/runs/40433740291); K2 [40434470537](https://github.com/OperationCode/claude-review-test/actions/runs/40434470537) (hang), [40434476879](https://github.com/OperationCode/claude-review-test/actions/runs/40434476879), [40434482049](https://github.com/OperationCode/claude-review-test/actions/runs/40434482049); K3 [40436332281](https://github.com/OperationCode/claude-review-test/actions/runs/40436332281) (hang, attempt 2 is the re-run), [40436337518](https://github.com/OperationCode/claude-review-test/actions/runs/40436337518), [40436343610](https://github.com/OperationCode/claude-review-test/actions/runs/40436343610). The endpoints are in each job's harden-runner post-step log; the kit diffs are in the owner's V10 test notes, outside this repo. -11. **Retarget (blocking).** The ruleset targets only the default branch, so a PR into another branch should start no required run. Test (a): open a PR to a staging branch that already holds a change not on the default branch; confirm that no required run starts; retarget to the default branch, and record whether the retarget starts a required run. Pass when the merge stays blocked until a review against the default branch passes (from a run that the retarget starts, or from a push or reopen), and that review shows the staging change in its diff. (b): the same, but the head SHA already has a passing required run from a PR to the default branch; record whether that pass counts after the retarget. -12. **Repo-level Actions settings (blocking).** Test: in the scratch repo, disable Actions or restrict allowed actions. Pass when merges block rather than skip the rule. +11. **Retarget (blocking).** The ruleset targets only the default branch, so a PR into another branch should start no required run. Test (a): open a PR to a staging branch that already holds a change not on the default branch; confirm that no required run starts; retarget to the default branch, and record whether the retarget starts a required run. Pass when the merge stays blocked until a review against the default branch passes (from a run that the retarget starts, or from a push or reopen), and that review shows the staging change in its diff. (b): the same, but the head SHA already has a passing required run from a PR to the default branch; record whether that pass counts after the retarget. **Result (phase 3, 2026-10-27): pass.** (a) PR 29 to a staging branch (staging = main plus one file) started no run in 60 s. The retarget to main at 18:37:27Z started no run and the box stayed blocked. A reopen at 18:38:43Z started run [40561903948](https://github.com/OperationCode/claude-review-test/actions/runs/40561903948): coverage "files in comparison 2" (the staging file and the PR file), 3 passes `ok`, pass; the box cleared only after it. (b) PR 30 (same head `1c432d5` as PR 28, which had an S1 pass) retargeted at 18:37:29Z started no run, and the box read mergeable: the pass from the other PR counted (per head SHA, V6 g2). +12. **Repo-level Actions settings (blocking).** Test: in the scratch repo, disable Actions or restrict allowed actions. Pass when merges block rather than skip the rule. **Result (phase 3, 2026-10-27): pass, and the settings do not affect the required run.** Actions disabled (18:41:26Z): a push at 18:41:50Z still started the required run [40562372022](https://github.com/OperationCode/claude-review-test/actions/runs/40562372022); all jobs ran, 3 `ok`, pass. Allowed actions "selected", GitHub-owned only (18:43Z, which excludes `step-security/harden-runner` and `astral-sh/setup-uv`): run [40562688524](https://github.com/OperationCode/claude-review-test/actions/runs/40562688524) downloaded both and passed. `local_only` was not run (same mechanism). The settings were restored at 18:45:44Z; the permissions, workflow and selected-actions responses equal the before responses byte for byte. 13. **Answered: dependency review on fork PRs (build).** 2026-10-25, pass. `actions/dependency-review-action` on a `pull_request_target` fork PR with no checkout, `base-ref` and `head-ref` set to the merge base and head SHAs, failed the job for a vulnerable version (minimist 1.2.5 critical, lodash 4.17.20 high) and passed for a safe version. The compare API and the action agreed in both cases. The dependency graph must be on in the target repo. Tested on a public repo only. On a private repo the action needs GitHub Advanced Security, which the Team plan does not include by default (not tested; open). Evidence: vulnerable PR [18](https://github.com/OperationCode/claude-review-test/pull/18), run [40233919632](https://github.com/OperationCode/claude-review-test/actions/runs/40233919632); safe PR [19](https://github.com/OperationCode/claude-review-test/pull/19), run [40233923908](https://github.com/OperationCode/claude-review-test/actions/runs/40233923908). 14. **Moot: which same-name check the merge box uses.** The gate no longer uses a check name. For a name-bound rule, the newest check suite decided (T2 to T4). 15. **Removed.** Publish concurrency is gone, so there is nothing to test. @@ -790,16 +842,16 @@ These are assumptions I could not confirm from docs. Each has a concrete test. I 19. **Moot: called-workflow job check.** v1 has no caller. A required workflow can call a reusable workflow and pass it the PR number, and the call's result gated the merge (T6). v1 does not need this. 20. **Merged into V6.** On a new head SHA, the box blocked while the required run was pending (T7). 21. **Merged into V6.** -22. **Pin after `.github` main moves (blocking).** The rule accepted a SHA pin, and the run used it (T6), but the pin was still the head of main, and the scratch repo was both source and target. Use a source repo separate from the target. Before each step, read the ruleset back and check `repository_id`, path and `sha`, with no `ref`. (a) Pin S1, push a changed workflow to the source main, open a PR in the target; pass when the run uses S1 (`job.workflow_sha`). (b) Move the pin to a new SHA with the release steps in "Auth and cost" (a new federation rule for the new SHA); record whether runs from the old pin still satisfy the rule on open PRs. (c) Removed (K-2): there is no advisory release. -23. **Same-path workflow in the target repo (blocking).** T6 was inconclusive: the PR's copy never ran, and the scratch repo was both source and target. Use a source repo separate from the target. Test: a fork PR adds `.github/workflows/claude-review.yml` to the target repo with a passing job `oc-claude-review`, while the central run fails. Pass when the box blocks. +22. **Pin after `.github` main moves (blocking).** The rule accepted a SHA pin, and the run used it (T6), but the pin was still the head of main, and the scratch repo was both source and target. Use a source repo separate from the target. Before each step, read the ruleset back and check `repository_id`, path and `sha`, with no `ref`. (a) Pin S1, push a changed workflow to the source main, open a PR in the target; pass when the run uses S1 (`job.workflow_sha`). (b) Move the pin to a new SHA with the release steps in "Auth and cost" (a new federation rule for the new SHA); record whether runs from the old pin still satisfy the rule on open PRs. (c) Removed (K-2): there is no advisory release. **Result (phase 3, 2026-10-27): (a) pass.** `job.workflow_sha` equaled the TR pin at creation in all 33 run attempts (1 at `f808dd0`, 1 at the probe `13a1e48`, 31 at S1 `3aa7ae9`), never `.github` main (`f808dd0` until the merge), including re-run attempts. **(b) pass, with a rollback residual.** Forward move M1 `590a387` to M2 `1cefe42` at 21:32:15Z: PRs 28 and 37, clear on their M1 passes at 21:32:04Z, read blocked at 21:32:17Z and 21:34:18Z (the M1 passes did not satisfy the rule). New runs at M2 ([40587964524](https://github.com/OperationCode/claude-review-test/actions/runs/40587964524), [40587977659](https://github.com/OperationCode/claude-review-test/actions/runs/40587977659); `job.workflow_sha` M2, 3 `ok`) cleared both at 21:38:23Z. After the pin moved back to M1 at 21:38:43Z both read clear again at 21:38:44Z, 21:39:15Z and 21:40:16Z on the older M1 passes. The first answer came from S1 to the probe SHA and back (18:32:57Z to 18:34:53Z). See "Pinned SHA only" in "Which run counts". +23. **Same-path workflow in the target repo (blocking).** T6 was inconclusive: the PR's copy never ran, and the scratch repo was both source and target. Use a source repo separate from the target. Test: a fork PR adds `.github/workflows/claude-review.yml` to the target repo with a passing job `oc-claude-review`, while the central run fails. Pass when the box blocks. **Phase 3 (2026-10-27): NOT YET RUN (pending account X).** Partial evidence only, from a same-repo PR by an org admin (PR 35): the PR added `.github/workflows/claude-review.yml` with a passing job `oc-claude-review` (run 40572849151, higher ID, success at 19:51:34Z) and a bidi file. The central run [40572849014](https://github.com/OperationCode/claude-review-test/actions/runs/40572849014) failed (blocking-finding, prefilter-hard-block, `superseded`: the final check counted the same-path run). The box stayed blocked throughout and after. 24. **Answered: run list for the final check (build).** 2026-10-25, pass. The job's `GITHUB_TOKEN` with `actions: read` listed runs by head SHA on same-repo `pull_request` runs and on `pull_request_target` runs of fork and same-repo PRs, including ruleset-started runs, with the correct `path`. A new run first appeared in the list within 5 s of its creation in every measurement (poll step about 3 s). The final check stays defense in depth. Evidence: [40227607202](https://github.com/OperationCode/claude-review-test/actions/runs/40227607202) and [40227659578](https://github.com/OperationCode/claude-review-test/actions/runs/40227659578) (same-repo); [40230848821](https://github.com/OperationCode/claude-review-test/actions/runs/40230848821) (ruleset-started runs, PR [17](https://github.com/OperationCode/claude-review-test/pull/17)); b1 run A [40236352621](https://github.com/OperationCode/claude-review-test/actions/runs/40236352621) listed run B (fork PR [15](https://github.com/OperationCode/claude-review-test/pull/15)). -25. **Pilot bypass (blocking).** The docs list the `RepositoryRole` actor type but not the `actor_id` values of the built-in roles. Test: in the scratch repo, an org ruleset with only the workflows rule and a bypass for the Maintain and Admin repository roles in `pull_request` mode. Read it back and record the `actor_id` values. Pass when, on a failed run, a user with Maintain and a user with Admin can merge with the bypass, a user with Write cannot, and GitHub records each bypass. Also record whether a direct push to the default branch is blocked; before a repo joins the pilot, check it for people or bots that push to the default branch directly. -26. **Placeholder SHA in a federation rule (verify in phase 3; part a done 2026-10-27).** The setup and every release create a rule at the placeholder SHA `0000000000000000000000000000000000000000` and later edit it to a real SHA ("Each release" in "Auth and cost"). V5 and V9 edited a rule from one real SHA to another; no test created a rule with a SHA that matches no commit. Test: (a) at setup, create the production rule at the placeholder and read it back. If the Console rejects it, stop, and resolve the order of rule ID and SHA before the production setup goes on. Result of part a (2026-10-27): the owner created R1 and the Console accepted it; no read-back output is recorded, so read R1 back before part b. R1 `claude-review-prod-r1` (`fdrl_01N2ZktTiVKqUvvXwywBFfys`) was created with `job_workflow_ref` at the placeholder SHA. The scratch rule R2 `claude-review-p3-r2` (`fdrl_01AfACzupW3U4Bx361ZUbsyR`; service account `claude-review-p0`, workspace `claude-review-p0-test`) was created the same way and stays at the placeholder until the release simulation of part c. (b) In phase 3, before the acceptance tests, edit the rule to the candidate SHA and read it back. Pass when a run under the scratch test ruleset at the candidate SHA gets a token, and a run at another SHA is denied. (c) Before the first release (V22 b), do steps 1 to 6 of "Each release" with a scratch rule R2 on the test workspace, including the re-edit of R2 to the merge SHA and the second exchange. Do not change R1 during this check. Pass when R2 reads back at the placeholder, then at the candidate SHA, then at the release SHA, and the exchange succeeds at both real SHAs. The scratch ruleset is pinned to a SHA that is on a non-default branch of `.github` at step 4 (a fork branch counts if the fork is the test path); pass only if the workflows rule accepts and runs it. If it does not, validate after the merge, at the main SHA, and the order of the steps changes. +25. **Pilot bypass (blocking).** The docs list the `RepositoryRole` actor type but not the `actor_id` values of the built-in roles. Test: in the scratch repo, an org ruleset with only the workflows rule and a bypass for the Maintain and Admin repository roles in `pull_request` mode. Read it back and record the `actor_id` values. Pass when, on a failed run, a user with Maintain and a user with Admin can merge with the bypass, a user with Write cannot, and GitHub records each bypass. Also record whether a direct push to the default branch is blocked; before a repo joins the pilot, check it for people or bots that push to the default branch directly. **Phase 3 (2026-10-27): NOT YET RUN (pending account X).** It needs accounts with Write, Maintain and Admin on claude-review-test, and the bypass list on the test ruleset, which stays empty until then. The role `actor_id` values are not recorded yet. +26. **Placeholder SHA in a federation rule (verify in phase 3; part a done 2026-10-27).** The setup and every release create a rule at the placeholder SHA `0000000000000000000000000000000000000000` and later edit it to a real SHA ("Each release" in "Auth and cost"). V5 and V9 edited a rule from one real SHA to another; no test created a rule with a SHA that matches no commit. Test: (a) at setup, create the production rule at the placeholder and read it back. If the Console rejects it, stop, and resolve the order of rule ID and SHA before the production setup goes on. Result of part a (2026-10-27): the owner created R1 and the Console accepted it; no read-back output is recorded, so read R1 back before part b. R1 `claude-review-prod-r1` (`fdrl_01N2ZktTiVKqUvvXwywBFfys`) was created with `job_workflow_ref` at the placeholder SHA. The scratch rule R2 `claude-review-p3-r2` (`fdrl_01AfACzupW3U4Bx361ZUbsyR`; service account `claude-review-p0`, workspace `claude-review-p0-test`) was created the same way and stays at the placeholder until the release simulation of part c. (b) In phase 3, before the acceptance tests, edit the rule to the candidate SHA and read it back. Pass when a run under the scratch test ruleset at the candidate SHA gets a token, and a run at another SHA is denied. (c) Before the first release (V22 b), do steps 1 to 6 of "Each release" with a scratch rule R2 on the test workspace, including the re-edit of R2 to the merge SHA and the second exchange. Do not change R1 during this check. Pass when R2 reads back at the placeholder, then at the candidate SHA, then at the release SHA, and the exchange succeeds at both real SHAs. The scratch ruleset is pinned to a SHA that is on a non-default branch of `.github` at step 4 (a fork branch counts if the fork is the test path); pass only if the workflows rule accepts and runs it. If it does not, validate after the merge, at the main SHA, and the order of the steps changes. **Results (phase 3, 2026-10-27).** (b) pass: with R1 at S1 `3aa7ae9` the run [40560632129](https://github.com/OperationCode/claude-review-test/actions/runs/40560632129) (PR 28, reopened 18:30:32Z) got a token, 3 passes `ok`, gate pass, box clear at 18:32:12Z (exchange window 18:31:01 to 18:31:40Z). The run [40561015054](https://github.com/OperationCode/claude-review-test/actions/runs/40561015054) at the probe `13a1e48` (S1 plus one empty commit, on branch `p3-deny-probe`; TR pinned to it at 18:32:57.9Z) was denied: 3 passes `failed (authentication failed)`, gate fail, box blocked (window 18:33:32 to 18:33:56Z). Console, read by the owner (2026-10-27): the deny run at 18:33:49Z shows 6 rows `Failure`, reason `match_claim_value_mismatch`, rule `claude-review-prod-r1`, service account `claude-review`. The owner reports that every other R1 row in the test windows was `Success`; there is no screenshot, and the token scope and workspace are not separately confirmed. The read-back output of R1 at the placeholder is still not recorded. (c) pass: S2 `9752ba7` (M1 plus R2 and the test service account; three assertions in `test_evals.py` adjusted, simulation only; unit tests 2563 passed, `-m release` 2 passed) on branch `p3-sim`, PR 10 into `p3-sim-base` (branched from M1), never main. TR accepted a SHA that is only on a non-default branch (S1, and S2 and the probe) and started runs at it. Step 4: TR pinned to S2 at 21:18:49Z, run [40585724589](https://github.com/OperationCode/claude-review-test/actions/runs/40585724589) at S2: 3 `ok`, box clear 21:20:18Z (window 21:19:19 to 21:19:45Z). Step 5: PR 10 squash-merged into `p3-sim-base` at 21:20:59Z gave M2 `1cefe4259bc26197ec4b063e8399ef856efdd4f2`; `git diff S2 M2` is empty. Step 6: runs [40587964524](https://github.com/OperationCode/claude-review-test/actions/runs/40587964524) and [40587977659](https://github.com/OperationCode/claude-review-test/actions/runs/40587977659) at M2, 3 `ok` each (window 21:35:22 to 21:35:54Z). R2 rows were not read in the Console, so the token through R2 is inferred: R1 names M1 and cannot match a run at M2. The reports record no read-back output for R2 at the placeholder, S2 or M2. R1 was not changed during the simulation, and not deleted. 27. **Eval federation rule (verify in phase 2, before the first eval).** The rule cannot name `eval.yml`, because a dispatch run's `job_workflow_ref` ends in the branch ref (V3). It binds `repository_id`, `event_name` and `environment: eval` instead ("Fixture eval credential"). Passed 2026-10-27 (UTC) with one substitute, one case not run, and the P1 scope not confirmed. The other-repo case ran as `push` in claude-review-test with no environment (N4, 401), so `repository_id` is not tested separately. The "another environment" case was not run; only the no-environment case ran (N1). The Console reason for each deny is recorded in "Fixture eval credential" (`match_claim_absent` for N1 and the no-environment job of the rejected-approval run; `match_claim_value_mismatch` for the no-environment job of the push run, N3 and N4); claim name and P1 scope not shown in the list view, so the P1 scope `workspace:developer` is not confirmed. See "Fixture eval credential". Test on the test workspace `claude-review-p0-test`, and record the Console reason for each deny. Pass when an approved `eval.yml` dispatch run in `.github` gets a token with scope `workspace:developer` for that workspace only, and each of these gets no token: a run whose approval is rejected (the job never starts, and the Console shows no exchange); a dispatch run in `.github` with no environment or another environment; a dispatch run with an `eval` environment in another repo (claude-review-test, with its own `eval` environment; this case is required, P2-2); a run in `.github` with the `eval` environment on another event (for example `push`). Also pass when the workflow unit test (see "Testing") fails on a fixture workflow other than `eval.yml` that uses `eval`, and on a job that uses `eval` with a GitHub write permission. The residual stays: an approver who approves without reading. Verify later (phase 3): -- The web UI "Re-run all jobs" button on a ruleset-started run is untested. Until it is tested, the documented way to apply an override is the REST re-run call (see "Override"). +- **NOT YET RUN (pending account X):** the web UI "Re-run all jobs" button on a ruleset-started run. Phase 3 asked for the owner to click it on run 40568865128 and got no click, so the override cases ran with the REST re-run call. Until the button is tested, the documented way to apply an override is the REST re-run call (see "Override"). Answered since the last version: @@ -904,3 +956,4 @@ P2: phase 2 rulings, 2026-10-27 (UTC). Where a P2- line changes an earlier line, - P2-5. Expectation format v2 and the fixture builder ("Testing"). An exact severity failed a "high" case on a `critical` answer, and there was no way to state "no high findings", a citation side, verification, alternative categories or a redacted summary. Now severity means "or higher", and `max_severity`, `file`, `side`, `verified`, category lists and `summary_excludes` exist. A hand-written bundle made "hard block regardless of Claude" true by construction, so every fixture bundle is built by the real `collect()` and a unit test checks it against a rebuild. - P3-1. Phase 3 rulings (owner and coordinator, 2026-10-27). Q1: a new production service account `claude-review` (`svac_01PiMgfb6qJgNpLZUV8hprCC`), separate from the test account, so test teardown cannot break production. Q2: the coordinator's approval of `eval` runs extends through phase 4; "prevent self-review" stays off until phase 5. Q3: test work in claude-review-test and `p3-*` branches in `.github` is approved; the non-member fork author (account X) is pending, and such an account becomes an outside collaborator, not an org member, when given repo access. Q4: `test.yml` runs `pytest tests/unit -m release` on every PR, in the `unit-tests` job. Q5: no workflow change on main for V22 a; the evidence is `job.workflow_sha` against the pin. Q6: one eval, at the candidate commit only. Q7 (design change): no Console edit lowers the test workspace spend limit; the exhausted-budget outage case is covered by V9 (phase 0) and the unit tests; the invalid federation rule case still runs live (denied exchange, V26 b). V26 a: R1 was created at the placeholder SHA and the Console accepted it (2026-10-27); R2 was created the same way. - P3-2. Over-long strings (design change). Release eval [run 40517202830](https://github.com/OperationCode/.github/actions/runs/40517202830) at the candidate S1 (ccbae51) matched 12 of 13 cases: in `obfuscated-test-payload` the supply-chain pass failed with "response does not match the findings schema: 'findings'/1/'quote': [maxLength]". Phase 2 run 40445885087 matched the same case with the same prompts, so this is model variance. Cause: structured outputs cannot enforce `maxLength` (P0-12), so the API accepts a string of any length, and one long string made the whole pass `failed` and the gate fail on a real PR. Fix (coordinator ruling, 2026-10-27): before the full schema check, analyze cuts every string over its schema `maxLength` to that limit and keeps the finding. This covers `summary` (1500), `file` (400), `quote` (300), `title` (120) and `explanation` (1200), and any limit added to the schema later; a unit test checks that the cut reaches every `maxLength`. A file path is an identifier, not prose, so its cut differs: the first 399 characters and a final "/" (fix round 1, review r-p3-cut Important 1). A plain prefix of a 401-character path can be the path of another changed file, which could then verify the finding and place its inline comment on the wrong file. No file path ends in "/", and the file lookup never matches one, so a cut path names no file. The finding stays and still counts toward the gate. The system prompt also tells the model to copy the part of a longer line that shows the problem. Why it does not lower risk: a finding is never dropped, and its category, severity, confidence, side and line never change. Blocking never depends on the text, the quote or verification ("Citation check"). Whitespace normalization keeps a prefix of a quote inside the normalized text wherever the whole quote is, so a quote that verified still verifies. A cut quote that is not at that place, a cut quote that is only whitespace or holds a redacted credential, or a cut file path, stays unverified and still blocks. A cut can make two findings that differed past the cut share file, title and quote (review r-p3-cut Important 2). A redaction before the cut can do the same far below the limit: two titles that differ only in a credential both become `Token [REDACTED:jwt]` (fix round 2, review rr-p3-cut-fix1 Important A). Two findings below every limit can also share file, side, title and quote and differ in explanation or severity. So each inline comment carries two keys ("Publishing (code)", fix round 3, review rr-p3-cut-fix2): the D14 key (file, side, title and quote) and a full key of every field that the reader sees, both of the shown, redacted text. In one run, findings are deduped by full key, so each distinct finding gets its own inline comment. Across runs, a finding whose D14 key is already on the PR is not posted again; when its full key matches none of the earlier comments with that D14 key, the summary shows it in full. So none hides the explanation of another, and a re-run or a push does not post every finding again (fix round 2 keyed only on every shown field, which re-posted almost every finding on each run, because the explanation and the line change). Fix round 4 (review r-p3-cut-final, coordinator rulings 2026-10-27): the marker alone is not proof that a comment shows a finding, because any Actions workflow in the repo writes as `github-actions[bot]` and the keys are public hashes. The D14 skip needs the inline layout, path, side and title, and "see the earlier comment" needs the exact body at the same line; a forged comment can at most move a finding to its full summary block. A cut file path makes a finding unverified, and an unverified high or critical finding now keeps its never-cut rank in the summary, so the summary shows every such explanation or the run fails closed. The residual of one summary comment (a low or medium explanation can be cut on a changed-key re-run at the largest sizes; capacity can fail a run closed) is stated in "Publishing (code)"; continuation comments are planned for phase 4. Findings that are equal in every shown field after the cut show one inline comment; the publisher cannot know the text past the cut, and that comment holds all the text of each. A credential-like string in an over-long string is redacted before the cut, so the cut cannot leave part of a secret that the publisher would then not recognize. The publisher escapes every character of a cut string, so a cut inside Markdown, HTML or an escape still shows as text (unit test). A cut title, explanation or summary has no truncation mark, because the publisher shows one only for its own cuts. A new candidate with this change needs its own eval (P0-15). +- P3-3. Phase 3 results and spec corrections (2026-10-27). Four spec claims did not survive the live tests, and each is now corrected in place. (1) The merge box counts only runs at the pinned SHA, not the newest run on the head SHA; a pin move back revives older passes at that SHA (PR 28 on 2026-10-27 at 18:34:53Z; PRs 28 and 37 at 21:38:44Z). A forward move fails closed (the box blocks at once). The rollback is a residual, covered by the rule to start a new run on each open PR after any pin move; it joins the residuals in rollout step 7. (2) Repo Actions settings do not affect the required run (V12 pass); the role table no longer lists them as an admin power against the gate. (3) A normal cancel does not stop the `always()` gate job, which then writes a `pass` summary next to a `cancelled` run; the run conclusion still blocks the box, and only a force-cancel stops the gate job. (4) The artifacts of earlier attempts are gone after a re-run; the gate reads only its own attempt, so it still fails closed, and the residual is that earlier pass records cannot be read back. Release procedure: two Console edits per release (candidate SHA, then merge SHA), a tree check of the merge SHA, and a denied exchange at the merge SHA before the second edit. NOT YET RUN, pending account X: V8, V23, V6 f as a Write user, V25 and the web UI "Re-run all jobs" case. The exhausted-budget outage case stays covered by V9 and the unit tests (Q7). From cbfd3f4752914ea71ade32b11769545c69d12f42 Mon Sep 17 00:00:00 2001 From: Irving Popovetsky Date: Tue, 27 Oct 2026 14:51:15 -0700 Subject: [PATCH 2/5] Phase 3 results: fix review round 1 (Console scope, V26 c and simulation partial, minor corrections) Co-Authored-By: Claude Opus 5.5 --- docs/build-plan.md | 10 +++++----- docs/claude-review-design.md | 22 ++++++++++++---------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/docs/build-plan.md b/docs/build-plan.md index 64cbeec..cd0ab03 100644 --- a/docs/build-plan.md +++ b/docs/build-plan.md @@ -64,7 +64,7 @@ Owner: Exit criteria: -- [x] V6 shows that the newest run on the head SHA decides. If not, building stops. (The queued cell is still open.) +- [x] V6 shows that the newest run on the head SHA (among runs at the pinned SHA, see spec P3-3) decides. If not, building stops. (The queued cell is still open.) - [x] V2 and V3 pass. If not, building stops. - [x] V4, V5, V9, V13, V16, V18 and V24 have results in the spec. A fallback in the spec covers each one that failed. @@ -154,10 +154,10 @@ Run the spec's workflow acceptance tests under a test org ruleset that requires - [x] V26 a: R1 created at the placeholder SHA and accepted by the Console (2026-10-27); R2 `claude-review-p3-r2` created the same way, inert until the release simulation. - [x] P3-2: an over-long string no longer fails a pass. Release eval run 40517202830 at S1 matched 12 of 13: one supply-chain quote went over 300 characters, which the API cannot enforce. Analyze now cuts every string over its schema `maxLength` (summary, file, quote, title, explanation) to that limit and keeps the finding (spec P3-2). Fix round 1: a cut file path ends in "/", so it names no changed file; when the file, title or quote is at its limit, the inline comment key covers every shown field, so a cut never hides a distinct explanation. Fix round 2: a redaction can make two titles equal far below the limit, so the key of every finding covers every shown field, at any length. Fix round 3: each inline comment carries the D14 key and the full key, both of the redacted shown text; one run dedupes by full key, a later run skips a D14 key already on the PR and shows the finding in full in the summary when its full key is new. Fix round 4: an earlier comment counts only when its text checks out (the D14 skip needs the inline layout, path, side and title; "see the earlier comment" needs the exact body at the same line), and an unverified high or critical finding is never cut from the summary. A candidate with this change needs its own eval. - [x] V26 b, before the acceptance tests: after the owner sets the production rule to the candidate SHA, a run at that SHA gets a token and a run at another SHA is denied. Run 40560632129 at S1 got a token (2026-10-27, 18:31Z); run 40561015054 at the probe SHA was denied, and the Console shows 6 rows `Failure` `match_claim_value_mismatch` on R1 at 18:33:49Z (owner). -- [x] V26 c, before V22 b: release steps 1 to 6 with a scratch rule R2 on the test workspace, R1 unchanged. Include the second exchange at the release SHA, and check that the workflows rule accepts a scratch ruleset pinned to a SHA that is only on a non-default branch of `.github`. S2 `9752ba7` (PR 10 into `p3-sim-base`) and M2 `1cefe42` both got a token (runs 40585724589 and 40587964524); the rule accepted SHAs that are only on non-default branches. R2 Console rows were not read, and no R2 read-back output is recorded. +- [ ] V26 c, before V22 b: release steps 1 to 6 with a scratch rule R2 on the test workspace, R1 unchanged. Include the second exchange at the release SHA, and check that the workflows rule accepts a scratch ruleset pinned to a SHA that is only on a non-default branch of `.github`. S2 `9752ba7` (PR 10 into `p3-sim-base`) and M2 `1cefe42` both got a token (runs 40585724589 and 40587964524); the rule accepted SHAs that are only on non-default branches. Partial: accepted at S2 and M2 (token inferred through R2); R2 Console rows were not read, no R2 read-back output is recorded, and step 5a was not run for R2/M2. Remaining: R2 read-backs at the placeholder, S2 and M2, and a denied exchange at M2 before the R2 edit. - [x] Release eval at S1 (`3aa7ae9`): run 40528717338, 13 of 13 matched. - [x] M1 tree check: M1 `590a387` and S1 have the same tree, and the R1 exchange works at M1 (run 40585413003) after the denied run at M1 (run 40583290922). -- [x] Release simulation (R2, S2, M2): steps 1 to 6 of "Each release", never merged to main. +- [ ] Release simulation (R2, S2, M2): steps 1 to 6 of "Each release", never merged to main. Partial: steps ran and S2 and M2 got tokens; no R2 read-backs and no step 5a deny for R2/M2. Remaining: those two parts. - [ ] A fork PR from an account outside the org, including a draft that later becomes ready. The review starts with no approval click (V8 b). NOT YET RUN, pending account X. - [ ] V8 a: a fork PR whose own workflow has a job named `oc-claude-review` that starts before the required run. NOT YET RUN, pending account X. A same-repo version by an org admin passed (PR 36, run 40572853203 started first; the box stayed blocked). - [x] V11: retarget from a non-default branch. The retarget starts no run and the box stays blocked until a reopen run passes (PR 29, run 40561903948); an old pass on the same head counts (PR 30). @@ -165,12 +165,12 @@ Run the spec's workflow acceptance tests under a test org ruleset that requires - [x] V22 a and b: pin behind main, then a pin move with PRs open. `job.workflow_sha` equaled the pin in 33 of 33 attempts. After a forward pin move the old passes do not count; after a move back they count again (spec "Pinned SHA only"; PRs 28 and 37, 21:32 to 21:40Z). - [ ] V23: a same-path workflow file in the target repo. NOT YET RUN, pending account X. A same-repo version by an org admin passed (PR 35, run 40572849014). - [ ] V25 (NOT YET RUN, pending account X): the pilot bypass. Maintain and Admin can merge past a failed run, Write cannot, and GitHub records each bypass. Record the role `actor_id` values, and whether a direct push to the default branch is blocked. -- [x] The rest of the acceptance list in the spec's "Testing" section that needs no account X, including the outage process (spec "Phase 3 results"). Design change (spec P3-1): the invalid federation rule case ran live with two denied exchanges (runs 40561015054 and 40583290922); the exhausted budget case is covered by V9 (phase 0) and the unit tests, with no Console limit edit (ruling Q7). +- [x] The rest of the acceptance list in the spec's "Testing" section that needs no account X, except the four unit-test-only cases listed in the next item (approver removed, edited override, artifact removed, gate cancel after inline comments), including the outage process (spec "Phase 3 results"). Design change (spec P3-1): the invalid federation rule case ran live with two denied exchanges (runs 40561015054 and 40583290922); the exhausted budget case is covered by V9 (phase 0) and the unit tests, with no Console limit edit (ruling Q7). - [ ] The acceptance cases that need account X: V6 f as a Write user, and the web UI "Re-run all jobs" button on a ruleset-started run (NOT YET RUN; the override cases ran with the REST re-run). Also not run, covered by unit tests: gate cancelled after inline comments with a refused summary write, approver role removed, edited override, artifact upload removed. Owner: -- [x] Owner: set `job_workflow_ref` of the production federation rule to the candidate release SHA. Done 2026-10-27 (S1, then M1 `590a387`; the run at M1 got a token). The candidate's code must name that rule ID (spec "Auth and cost", "Each release"). +- [x] Owner: set `job_workflow_ref` of the production federation rule to the candidate release SHA. Done 2026-10-27 (S1, then M1 `590a387`; the run at M1 got a token; no read-back recorded). The candidate's code must name that rule ID (spec "Auth and cost", "Each release"). - [ ] Owner: an account outside the org opens the fork PRs. - [ ] Owner: test users with Write, Maintain and Admin on claude-review-test for V25. diff --git a/docs/claude-review-design.md b/docs/claude-review-design.md index 2436fef..054bcbf 100644 --- a/docs/claude-review-design.md +++ b/docs/claude-review-design.md @@ -172,7 +172,7 @@ Facts about ruleset-required workflows ([troubleshooting rules](https://docs.git + 1 human approval (+ code owner where CODEOWNERS applies) ``` -The required run succeeds only when every job succeeds, and that run result is the gate. Only the gate job may fail because of policy. The other jobs fail only on an infrastructure error, or, for dependency-review, on a vulnerable dependency. Any failed, timed-out or cancelled job fails the run, whatever the gate job does. The gate job still runs (`always()`) to explain the failure in the summary, and treats it as not waivable. A normal run cancel does not stop it: live (phase 3, run 40565538379), the run was `cancelled` right after analyze and dependency-review finished, and the gate job ran to success and wrote a `pass` summary. The run conclusion stayed `cancelled` and the box blocked, so the gate holds. Only a force-cancel stops the gate job. No path makes the run `success` without an exit 0 from the gate job. +The required run succeeds only when every job succeeds, and that run result is the gate. Only the gate job may fail because of policy. The other jobs fail only on an infrastructure error, or, for dependency-review, on a vulnerable dependency. Any failed, timed-out or cancelled job fails the run, whatever the gate job does. The gate job still runs (`always()`) to explain the failure in the summary, and treats it as not waivable. A normal run cancel before the gate job starts does not stop it (a normal cancel while the gate job runs was not tested): live (phase 3, run 40565538379), the run was `cancelled` right after analyze and dependency-review finished, and the gate job ran to success and wrote a `pass` summary. The run conclusion stayed `cancelled` and the box blocked, so the gate holds. Only a force-cancel stops the gate job. No path makes the run `success` without an exit 0 from the gate job. Only the gate job writes to GitHub, and it writes only comments. It has no Anthropic credential and no `id-token`. Its `actions: read` scope only lists runs for the final check. Its `contents: read` scope is for the re-validation reads (the base ref tip and the compare endpoint), which need it on private repos. No job writes a check run through the Checks API, so no job has `checks: write`. The workflow sets `permissions: {}` at the top, and each job lists its own scopes, so every scope a job does not list is `none`. @@ -514,7 +514,7 @@ The gate record is a run of the central workflow on the PR head SHA. GitHub crea - **Newest run decides (build prerequisite), among the runs at the pinned SHA.** A reopen, a re-run, or two PRs with one head make several runs on one SHA. The docs do not say which run the rule uses. The design requires that the rule use the newest run on the head SHA, and that run's newest attempt: a queued, in-progress, cancelled, timed-out or failed newest run blocks, whatever an older run shows. V6 tests this first. If V6 shows that an older run or the last completed run can decide, building stops and the gate design changes. No extra read in the gate job can fix that order. - **Final check (defense in depth).** As its last step before exit 0, the gate job reads the snapshot and the override authority again, and lists the runs for the head SHA (`GET /repos/{owner}/{repo}/actions/runs?head_sha=...`). It exits non-zero if the snapshot changed, if the grant no longer holds, or if a run with the central workflow's path and a higher run ID exists (`superseded`). A same-path workflow in the target repo also matches; that can only make a run fail. A run list that was not read completely, or a run with no `path`, also counts as `superseded`. This shortens the time between the authority read and the run result. It is not atomic with GitHub's record of the run's completion, and a new run can be missing from the list for a short time (V24). So safety rests on newest-run selection, not on this check. The list is not filtered by PR, so it matches the box (V6 g2); do not add a PR filter. If the final check turns the result to fail, the job updates the summary before it exits, if it can. - **Re-runs.** "Re-run all jobs" makes a new attempt with the same run ID. Each attempt reads the live PR, comments, timeline and permissions when its gate job runs. A re-run of the newest run can pass; a re-run of an older run exits `superseded` when it sees the newer run (V6 e). -- **Pinned SHA only (phase 3).** The box counts only runs of the workflow at the SHA the ruleset pins. Observed: after the pin moved from S1 to a probe SHA and back to S1, PR 28 read mergeable from 18:34:53Z on its older S1 pass, although the newest run on its head, at the probe SHA, had failed. After a pin move M1 to M2 and back to M1, PRs 28 and 37 read blocked at once on the move to M2 (the M1 passes did not count), then clear again from 21:38:44Z on the move back (the M1 passes counted, and the newer M2 runs did not decide). Why it still fails closed on a forward move: every open PR blocks until a run at the new pin passes. Residual on a move back to an earlier SHA: every earlier pass at that SHA counts again, also on a PR whose newer run at the newer release failed. The merged code then passed a review by a release the owner pinned, not by the current one. Rule: after any pin move, including a move back, start a new run on each open PR and merge only on that run; never use a rollback to clear a block. An org owner can start a rollback only by editing the ruleset, which is a logged org-owner action. +- **Pinned SHA only (phase 3).** The box counts only runs of the workflow at the SHA the ruleset pins. Observed: after the pin moved from S1 to a probe SHA and back to S1, PR 28 read mergeable from 18:34:53Z on its older S1 pass, although the newest run on its head, at the probe SHA, had failed. After a pin move M1 to M2 and back to M1, PRs 28 and 37 read blocked at once on the move to M2 (the M1 passes did not count), then clear again from 21:38:44Z on the move back (the M1 passes counted; whether the newer M2 runs, which passed, were ignored is not shown by this case). Why it still fails closed on a forward move: every open PR blocks until a run at the new pin passes. Residual on a move back to an earlier SHA: every earlier pass at that SHA counts again, also on a PR whose newer run at the newer release failed. The merged code then passed a review by a release the owner pinned, not by the current one. Rule: after any pin move, including a move back, start a new run on each open PR and merge only on that run; never use a rollback to clear a block. An org owner can start a rollback only by editing the ruleset, which is a logged org-owner action. - **Per head SHA, not per PR.** The box reads runs per head SHA. A run of any PR with that head decides every PR with that head, in both directions (V6 g2). **Residual risk: an earlier pass on the same SHA.** A run's result is fixed when the run ends. Under newest-run selection, a newer run on head SHA H replaces an earlier pass as soon as it exists. Nothing else does: @@ -738,10 +738,10 @@ Target: [claude-review-test](https://github.com/OperationCode/claude-review-test |---|---|---| | Release eval at S1 | Pass: 13 of 13 matched, 0 missed | [`.github` run 40528717338](https://github.com/OperationCode/.github/actions/runs/40528717338), approved 2026-10-27 15:13:14Z; tokens: input 39399, output 19901, cache write 56524, cache read 113048 | | M1 tree check | Pass: S1 and M1 have tree `6ace36e90f776f1fd9e821851b42dfc79dad13c0`; `git diff S1 M1` empty | `origin/main` = M1 | -| V26 a, b, c | Pass (see V26) | R1 and R2 accepted at the placeholder; runs [40560632129](https://github.com/OperationCode/claude-review-test/actions/runs/40560632129) (accept), [40561015054](https://github.com/OperationCode/claude-review-test/actions/runs/40561015054) (deny), [40585724589](https://github.com/OperationCode/claude-review-test/actions/runs/40585724589) (S2 via R2), [40587964524](https://github.com/OperationCode/claude-review-test/actions/runs/40587964524) (M2 via R2) | -| Deny at the merge SHA, before the second edit | Pass: run [40583290922](https://github.com/OperationCode/claude-review-test/actions/runs/40583290922) at M1, PR 28 reopened 21:02:02 to 21:02:08Z; 3 passes `failed (authentication failed)`; gate fail; box blocked from 21:02:01Z | Exchange window 21:02:36 to 21:02:57Z, R1 then at S1. The owner reports every other R1 row in the test windows was `Success`; the reports do not say whether that covers this window, so its Console reason is not confirmed | -| Accept at M1 after the second edit | Pass: run [40585413003](https://github.com/OperationCode/claude-review-test/actions/runs/40585413003), PR 28 reopened 21:16:44Z, 3 `ok`, box clear 21:18:12Z; PR 37: run [40586089942](https://github.com/OperationCode/claude-review-test/actions/runs/40586089942), clear 21:22:46Z | R1 at M1; ready for phase 4 | -| Simulation (R2, S2, M2) | Pass: see V26 c | S2 `9752ba7c04d796822c64c670415d1046607c3c03`, M2 `1cefe4259bc26197ec4b063e8399ef856efdd4f2`, PR 10 into `p3-sim-base`, never main | +| V26 a, b, c | a and b pass; c partial (see V26 c: no R2 read-backs, step 5a not run for R2 and M2) | R1 and R2 accepted at the placeholder; runs [40560632129](https://github.com/OperationCode/claude-review-test/actions/runs/40560632129) (accept), [40561015054](https://github.com/OperationCode/claude-review-test/actions/runs/40561015054) (deny), [40585724589](https://github.com/OperationCode/claude-review-test/actions/runs/40585724589) (S2 via R2), [40587964524](https://github.com/OperationCode/claude-review-test/actions/runs/40587964524) (M2 via R2) | +| Deny at the merge SHA, before the second edit | Pass: run [40583290922](https://github.com/OperationCode/claude-review-test/actions/runs/40583290922) at M1, PR 28 reopened 21:02:02 to 21:02:08Z; 3 passes `failed (authentication failed)`; gate fail; box blocked from 21:02:01Z | Exchange window 21:02:36 to 21:02:57Z, R1 then at S1. Console reason not read | +| Accept at M1 after the second edit | Pass: run [40585413003](https://github.com/OperationCode/claude-review-test/actions/runs/40585413003), PR 28 reopened 21:16:44Z, 3 `ok`, box clear 21:18:12Z; PR 37: run [40586089942](https://github.com/OperationCode/claude-review-test/actions/runs/40586089942), clear 21:22:46Z | R1 at M1 (no read-back or Console evidence recorded, only the working token); ready for phase 4 | +| Simulation (R2, S2, M2) | Partial pass: see V26 c. Remaining: R2 read-backs at the placeholder, S2 and M2; a denied exchange at M2 before the R2 edit to M2 | S2 `9752ba7c04d796822c64c670415d1046607c3c03`, M2 `1cefe4259bc26197ec4b063e8399ef856efdd4f2`, PR 10 into `p3-sim-base`, never main | | V22 a, V22 b | Pass, with a rollback residual (see "Which run counts") | V22 | | V11 | Pass | V11 | | V12 | Pass; Actions settings do not affect the required run | V12 | @@ -752,6 +752,8 @@ Target: [claude-review-test](https://github.com/OperationCode/claude-review-test | Outage: invalid federation rule | Pass, live: two denied exchanges | runs 40561015054 and 40583290922 | | Outage: exhausted budget | Covered by V9 and the unit tests; no Console limit edit (ruling Q7, P3-1) | V9 | +PR 9 `collect` and `oc-claude-review` failed with HTTP 422 seconds after the push and were never re-run. The 32 MiB packument cap hard-blocks `renovate` (K-11). + Acceptance list, run without account X (V6 gives 33 run attempts and 6,153 observer readings on 10 PRs; none showed a clear box on a failed, cancelled or missing run of the workflow at the pin, except the documented residuals: the reopen-to-run gap, and an older pass exposed after run deletion). Secret scan of 33 run attempts for `eyJ`, `sk-ant-` and bearer strings: 0 hits. | Case | Result | Evidence | @@ -842,11 +844,11 @@ These are assumptions I could not confirm from docs. Each has a concrete test. I 19. **Moot: called-workflow job check.** v1 has no caller. A required workflow can call a reusable workflow and pass it the PR number, and the call's result gated the merge (T6). v1 does not need this. 20. **Merged into V6.** On a new head SHA, the box blocked while the required run was pending (T7). 21. **Merged into V6.** -22. **Pin after `.github` main moves (blocking).** The rule accepted a SHA pin, and the run used it (T6), but the pin was still the head of main, and the scratch repo was both source and target. Use a source repo separate from the target. Before each step, read the ruleset back and check `repository_id`, path and `sha`, with no `ref`. (a) Pin S1, push a changed workflow to the source main, open a PR in the target; pass when the run uses S1 (`job.workflow_sha`). (b) Move the pin to a new SHA with the release steps in "Auth and cost" (a new federation rule for the new SHA); record whether runs from the old pin still satisfy the rule on open PRs. (c) Removed (K-2): there is no advisory release. **Result (phase 3, 2026-10-27): (a) pass.** `job.workflow_sha` equaled the TR pin at creation in all 33 run attempts (1 at `f808dd0`, 1 at the probe `13a1e48`, 31 at S1 `3aa7ae9`), never `.github` main (`f808dd0` until the merge), including re-run attempts. **(b) pass, with a rollback residual.** Forward move M1 `590a387` to M2 `1cefe42` at 21:32:15Z: PRs 28 and 37, clear on their M1 passes at 21:32:04Z, read blocked at 21:32:17Z and 21:34:18Z (the M1 passes did not satisfy the rule). New runs at M2 ([40587964524](https://github.com/OperationCode/claude-review-test/actions/runs/40587964524), [40587977659](https://github.com/OperationCode/claude-review-test/actions/runs/40587977659); `job.workflow_sha` M2, 3 `ok`) cleared both at 21:38:23Z. After the pin moved back to M1 at 21:38:43Z both read clear again at 21:38:44Z, 21:39:15Z and 21:40:16Z on the older M1 passes. The first answer came from S1 to the probe SHA and back (18:32:57Z to 18:34:53Z). See "Pinned SHA only" in "Which run counts". +22. **Pin after `.github` main moves (blocking).** The rule accepted a SHA pin, and the run used it (T6), but the pin was still the head of main, and the scratch repo was both source and target. Use a source repo separate from the target. Before each step, read the ruleset back and check `repository_id`, path and `sha`, with no `ref`. (a) Pin S1, push a changed workflow to the source main, open a PR in the target; pass when the run uses S1 (`job.workflow_sha`). (b) Move the pin to a new SHA with the release steps in "Auth and cost" (a new federation rule for the new SHA); record whether runs from the old pin still satisfy the rule on open PRs. (c) Removed (K-2): there is no advisory release. **Result (phase 3, 2026-10-27): (a) pass.** `job.workflow_sha` equaled the TR pin at creation in all 33 run attempts (1 at `f808dd0`, 1 at the probe `13a1e48`, 31 at S1 `3aa7ae9`), including re-run attempts. The run at `f808dd0` equaled main then; the 31 at S1 ran while main was still `f808dd0`. **(b) pass, with a rollback residual.** Forward move M1 `590a387` to M2 `1cefe42` at 21:32:15Z: PRs 28 and 37, clear on their M1 passes at 21:32:04Z, read blocked at 21:32:17Z and 21:34:18Z (the M1 passes did not satisfy the rule). New runs at M2 ([40587964524](https://github.com/OperationCode/claude-review-test/actions/runs/40587964524), [40587977659](https://github.com/OperationCode/claude-review-test/actions/runs/40587977659); `job.workflow_sha` M2, 3 `ok`) cleared both at 21:38:23Z. In the probe case the newer run had failed and the box still read clear, so the newer run did not decide there. After the pin moved back to M1 at 21:38:43Z both read clear again at 21:38:44Z, 21:39:15Z and 21:40:16Z on the older M1 passes. The first answer came from S1 to the probe SHA and back (18:32:57Z to 18:34:53Z). See "Pinned SHA only" in "Which run counts". 23. **Same-path workflow in the target repo (blocking).** T6 was inconclusive: the PR's copy never ran, and the scratch repo was both source and target. Use a source repo separate from the target. Test: a fork PR adds `.github/workflows/claude-review.yml` to the target repo with a passing job `oc-claude-review`, while the central run fails. Pass when the box blocks. **Phase 3 (2026-10-27): NOT YET RUN (pending account X).** Partial evidence only, from a same-repo PR by an org admin (PR 35): the PR added `.github/workflows/claude-review.yml` with a passing job `oc-claude-review` (run 40572849151, higher ID, success at 19:51:34Z) and a bidi file. The central run [40572849014](https://github.com/OperationCode/claude-review-test/actions/runs/40572849014) failed (blocking-finding, prefilter-hard-block, `superseded`: the final check counted the same-path run). The box stayed blocked throughout and after. 24. **Answered: run list for the final check (build).** 2026-10-25, pass. The job's `GITHUB_TOKEN` with `actions: read` listed runs by head SHA on same-repo `pull_request` runs and on `pull_request_target` runs of fork and same-repo PRs, including ruleset-started runs, with the correct `path`. A new run first appeared in the list within 5 s of its creation in every measurement (poll step about 3 s). The final check stays defense in depth. Evidence: [40227607202](https://github.com/OperationCode/claude-review-test/actions/runs/40227607202) and [40227659578](https://github.com/OperationCode/claude-review-test/actions/runs/40227659578) (same-repo); [40230848821](https://github.com/OperationCode/claude-review-test/actions/runs/40230848821) (ruleset-started runs, PR [17](https://github.com/OperationCode/claude-review-test/pull/17)); b1 run A [40236352621](https://github.com/OperationCode/claude-review-test/actions/runs/40236352621) listed run B (fork PR [15](https://github.com/OperationCode/claude-review-test/pull/15)). 25. **Pilot bypass (blocking).** The docs list the `RepositoryRole` actor type but not the `actor_id` values of the built-in roles. Test: in the scratch repo, an org ruleset with only the workflows rule and a bypass for the Maintain and Admin repository roles in `pull_request` mode. Read it back and record the `actor_id` values. Pass when, on a failed run, a user with Maintain and a user with Admin can merge with the bypass, a user with Write cannot, and GitHub records each bypass. Also record whether a direct push to the default branch is blocked; before a repo joins the pilot, check it for people or bots that push to the default branch directly. **Phase 3 (2026-10-27): NOT YET RUN (pending account X).** It needs accounts with Write, Maintain and Admin on claude-review-test, and the bypass list on the test ruleset, which stays empty until then. The role `actor_id` values are not recorded yet. -26. **Placeholder SHA in a federation rule (verify in phase 3; part a done 2026-10-27).** The setup and every release create a rule at the placeholder SHA `0000000000000000000000000000000000000000` and later edit it to a real SHA ("Each release" in "Auth and cost"). V5 and V9 edited a rule from one real SHA to another; no test created a rule with a SHA that matches no commit. Test: (a) at setup, create the production rule at the placeholder and read it back. If the Console rejects it, stop, and resolve the order of rule ID and SHA before the production setup goes on. Result of part a (2026-10-27): the owner created R1 and the Console accepted it; no read-back output is recorded, so read R1 back before part b. R1 `claude-review-prod-r1` (`fdrl_01N2ZktTiVKqUvvXwywBFfys`) was created with `job_workflow_ref` at the placeholder SHA. The scratch rule R2 `claude-review-p3-r2` (`fdrl_01AfACzupW3U4Bx361ZUbsyR`; service account `claude-review-p0`, workspace `claude-review-p0-test`) was created the same way and stays at the placeholder until the release simulation of part c. (b) In phase 3, before the acceptance tests, edit the rule to the candidate SHA and read it back. Pass when a run under the scratch test ruleset at the candidate SHA gets a token, and a run at another SHA is denied. (c) Before the first release (V22 b), do steps 1 to 6 of "Each release" with a scratch rule R2 on the test workspace, including the re-edit of R2 to the merge SHA and the second exchange. Do not change R1 during this check. Pass when R2 reads back at the placeholder, then at the candidate SHA, then at the release SHA, and the exchange succeeds at both real SHAs. The scratch ruleset is pinned to a SHA that is on a non-default branch of `.github` at step 4 (a fork branch counts if the fork is the test path); pass only if the workflows rule accepts and runs it. If it does not, validate after the merge, at the main SHA, and the order of the steps changes. **Results (phase 3, 2026-10-27).** (b) pass: with R1 at S1 `3aa7ae9` the run [40560632129](https://github.com/OperationCode/claude-review-test/actions/runs/40560632129) (PR 28, reopened 18:30:32Z) got a token, 3 passes `ok`, gate pass, box clear at 18:32:12Z (exchange window 18:31:01 to 18:31:40Z). The run [40561015054](https://github.com/OperationCode/claude-review-test/actions/runs/40561015054) at the probe `13a1e48` (S1 plus one empty commit, on branch `p3-deny-probe`; TR pinned to it at 18:32:57.9Z) was denied: 3 passes `failed (authentication failed)`, gate fail, box blocked (window 18:33:32 to 18:33:56Z). Console, read by the owner (2026-10-27): the deny run at 18:33:49Z shows 6 rows `Failure`, reason `match_claim_value_mismatch`, rule `claude-review-prod-r1`, service account `claude-review`. The owner reports that every other R1 row in the test windows was `Success`; there is no screenshot, and the token scope and workspace are not separately confirmed. The read-back output of R1 at the placeholder is still not recorded. (c) pass: S2 `9752ba7` (M1 plus R2 and the test service account; three assertions in `test_evals.py` adjusted, simulation only; unit tests 2563 passed, `-m release` 2 passed) on branch `p3-sim`, PR 10 into `p3-sim-base` (branched from M1), never main. TR accepted a SHA that is only on a non-default branch (S1, and S2 and the probe) and started runs at it. Step 4: TR pinned to S2 at 21:18:49Z, run [40585724589](https://github.com/OperationCode/claude-review-test/actions/runs/40585724589) at S2: 3 `ok`, box clear 21:20:18Z (window 21:19:19 to 21:19:45Z). Step 5: PR 10 squash-merged into `p3-sim-base` at 21:20:59Z gave M2 `1cefe4259bc26197ec4b063e8399ef856efdd4f2`; `git diff S2 M2` is empty. Step 6: runs [40587964524](https://github.com/OperationCode/claude-review-test/actions/runs/40587964524) and [40587977659](https://github.com/OperationCode/claude-review-test/actions/runs/40587977659) at M2, 3 `ok` each (window 21:35:22 to 21:35:54Z). R2 rows were not read in the Console, so the token through R2 is inferred: R1 names M1 and cannot match a run at M2. The reports record no read-back output for R2 at the placeholder, S2 or M2. R1 was not changed during the simulation, and not deleted. +26. **Placeholder SHA in a federation rule (verify in phase 3; part a done 2026-10-27).** The setup and every release create a rule at the placeholder SHA `0000000000000000000000000000000000000000` and later edit it to a real SHA ("Each release" in "Auth and cost"). V5 and V9 edited a rule from one real SHA to another; no test created a rule with a SHA that matches no commit. Test: (a) at setup, create the production rule at the placeholder and read it back. If the Console rejects it, stop, and resolve the order of rule ID and SHA before the production setup goes on. Result of part a (2026-10-27): the owner created R1 and the Console accepted it; no read-back output is recorded, so read R1 back before part b. R1 `claude-review-prod-r1` (`fdrl_01N2ZktTiVKqUvvXwywBFfys`) was created with `job_workflow_ref` at the placeholder SHA. The scratch rule R2 `claude-review-p3-r2` (`fdrl_01AfACzupW3U4Bx361ZUbsyR`; service account `claude-review-p0`, workspace `claude-review-p0-test`) was created the same way and stays at the placeholder until the release simulation of part c. (b) In phase 3, before the acceptance tests, edit the rule to the candidate SHA and read it back. Pass when a run under the scratch test ruleset at the candidate SHA gets a token, and a run at another SHA is denied. (c) Before the first release (V22 b), do steps 1 to 6 of "Each release" with a scratch rule R2 on the test workspace, including the re-edit of R2 to the merge SHA and the second exchange. Do not change R1 during this check. Pass when R2 reads back at the placeholder, then at the candidate SHA, then at the release SHA, and the exchange succeeds at both real SHAs. The scratch ruleset is pinned to a SHA that is on a non-default branch of `.github` at step 4 (a fork branch counts if the fork is the test path); pass only if the workflows rule accepts and runs it. If it does not, validate after the merge, at the main SHA, and the order of the steps changes. **Results (phase 3, 2026-10-27).** (b) pass: with R1 at S1 `3aa7ae9` the run [40560632129](https://github.com/OperationCode/claude-review-test/actions/runs/40560632129) (PR 28, reopened 18:30:32Z) got a token, 3 passes `ok`, gate pass, box clear at 18:32:12Z (exchange window 18:31:01 to 18:31:40Z). The run [40561015054](https://github.com/OperationCode/claude-review-test/actions/runs/40561015054) at the probe `13a1e48` (S1 plus one empty commit, on branch `p3-deny-probe`; TR pinned to it at 18:32:57.9Z) was denied: 3 passes `failed (authentication failed)`, gate fail, box blocked (window 18:33:32 to 18:33:56Z). Console, read by the owner (2026-10-27): the deny run at 18:33:49Z shows 6 rows `Failure`, reason `match_claim_value_mismatch`, rule `claude-review-prod-r1`, service account `claude-review`. Before the merge, the owner reported `Success` for the R1 rows in the windows 18:31:01 to 18:31:40Z and 19:16:09 to 19:16:46Z. The windows from 21:02Z on (M1 deny, M1 accept, PR 37) and all R2 rows were not checked. There is no screenshot, and the token scope and workspace are not separately confirmed. The read-back output of R1 at the placeholder is still not recorded. (c) partial pass: accepted at S2 and M2, token through R2 inferred; no R2 read-backs, and step 5a (a denied exchange at the merge SHA before the second edit) was not run for R2 and M2 (the deny at the merge SHA ran only for R1 at M1). Not done, and still to do: R2 read-backs at the placeholder, S2 and M2, and a denied run at M2 before the R2 edit. S2 `9752ba7` (M1 plus R2 and the test service account; three assertions in `test_evals.py` adjusted, simulation only; unit tests 2563 passed, `-m release` 2 passed) on branch `p3-sim`, PR 10 into `p3-sim-base` (branched from M1), never main. TR accepted a SHA that is only on a non-default branch (S1, and S2 and the probe) and started runs at it. Step 4: TR pinned to S2 at 21:18:49Z, run [40585724589](https://github.com/OperationCode/claude-review-test/actions/runs/40585724589) at S2: 3 `ok`, box clear 21:20:18Z (window 21:19:19 to 21:19:45Z). Step 5: PR 10 squash-merged into `p3-sim-base` at 21:20:59Z gave M2 `1cefe4259bc26197ec4b063e8399ef856efdd4f2`; `git diff S2 M2` is empty. Step 6: runs [40587964524](https://github.com/OperationCode/claude-review-test/actions/runs/40587964524) and [40587977659](https://github.com/OperationCode/claude-review-test/actions/runs/40587977659) at M2, 3 `ok` each (window 21:35:22 to 21:35:54Z). R2 rows were not read in the Console, so the token through R2 is inferred: R1 names M1 and cannot match a run at M2. The reports record no read-back output for R2 at the placeholder, S2 or M2. R1 was not changed during the simulation, and not deleted. 27. **Eval federation rule (verify in phase 2, before the first eval).** The rule cannot name `eval.yml`, because a dispatch run's `job_workflow_ref` ends in the branch ref (V3). It binds `repository_id`, `event_name` and `environment: eval` instead ("Fixture eval credential"). Passed 2026-10-27 (UTC) with one substitute, one case not run, and the P1 scope not confirmed. The other-repo case ran as `push` in claude-review-test with no environment (N4, 401), so `repository_id` is not tested separately. The "another environment" case was not run; only the no-environment case ran (N1). The Console reason for each deny is recorded in "Fixture eval credential" (`match_claim_absent` for N1 and the no-environment job of the rejected-approval run; `match_claim_value_mismatch` for the no-environment job of the push run, N3 and N4); claim name and P1 scope not shown in the list view, so the P1 scope `workspace:developer` is not confirmed. See "Fixture eval credential". Test on the test workspace `claude-review-p0-test`, and record the Console reason for each deny. Pass when an approved `eval.yml` dispatch run in `.github` gets a token with scope `workspace:developer` for that workspace only, and each of these gets no token: a run whose approval is rejected (the job never starts, and the Console shows no exchange); a dispatch run in `.github` with no environment or another environment; a dispatch run with an `eval` environment in another repo (claude-review-test, with its own `eval` environment; this case is required, P2-2); a run in `.github` with the `eval` environment on another event (for example `push`). Also pass when the workflow unit test (see "Testing") fails on a fixture workflow other than `eval.yml` that uses `eval`, and on a job that uses `eval` with a GitHub write permission. The residual stays: an approver who approves without reading. Verify later (phase 3): @@ -894,7 +896,7 @@ T2: decisions from the workflows-rule tests T6 and T7 in [claude-review-test](ht T3: decisions on review J2 of the T2 revision, 2026-10-25. Where a T3- line changes a T2- line, the T3- line wins. -- T3-1. Completion order (review J2 Important 1): **resolved by a prerequisite**. The rule must use the newest run on the head SHA and its newest attempt; a pending, cancelled or failed newest run blocks. The claim that the design holds under last-completed selection is removed. If V6 shows another selection, building stops for a design change. The final check stays as defense in depth only, because it is not atomic with run completion. V6 (b) pauses a run after its final check and tests both outcomes of the newer run; V24 records list delay instead of requiring it to be short. +- T3-1. Completion order (review J2 Important 1): **resolved by a prerequisite**. The rule must use the newest run on the head SHA (among runs at the pinned SHA, see P3-3) and its newest attempt; a pending, cancelled or failed newest run blocks. The claim that the design holds under last-completed selection is removed. If V6 shows another selection, building stops for a design change. The final check stays as defense in depth only, because it is not atomic with run completion. V6 (b) pauses a run after its final check and tests both outcomes of the newer run; V24 records list delay instead of requiring it to be short. - T3-2. Stale-pass boundary (Important 2): **exact residual stated**. Invalidating an override on an unchanged head takes effect only when a newer run on that head exists, and never if none starts (revoke, role removal, `GITHUB_TOKEN` return push, branch restore, deleted run). Human approval is a separate control and does not re-check the override. The owner accepts the residual in writing before org-wide enforcement (K-2); strict invalidation needs another gate design. V6 has separate criteria with and without an earlier pass, and tests absent, queued, in-progress, cancelled, timed-out and completed replacements, H to B to H, branch restore and a second PR with the same head. - T3-3. Run history control (Important 3): **residual stated, test added**. The role table lists cancel, re-run and delete of runs for repo writers and admins. V6 (f) deletes a newer run with an older pass present and checks the box and the run list. If the older pass decides, the owner accepts the writer trust boundary before org-wide enforcement (K-2), or the design changes. - T3-4. Advisory pilot (Important 4): **replaced by K-2**. Ruleset Evaluate mode is Enterprise only and not available on Team. @@ -924,7 +926,7 @@ K: owner decisions, 2026-10-25. Where a K- line changes an earlier line, the K- P0: results of the phase 0 platform tests, 2026-10-25 and 2026-10-26 (UTC), in [claude-review-test](https://github.com/OperationCode/claude-review-test) with the scratch source repo [claude-review-p0-source](https://github.com/OperationCode/claude-review-p0-source). V2, V3, V4, V5, V9, V13, V16, V18 and V24 are answered and V6 has no stop; the results are in "Verify before building". Where a P0- line changes an earlier line, the P0- line wins. -- P0-1. V6: **the newest run on the head SHA decides the merge box. The stop rule did not trigger.** Still open: the queued cells and n1, (f), the approval part of (c), (g1) with the real Restore button, and the default-branch check. "Which run counts" now states the cross-PR effect (g2). +- P0-1. V6: **the newest run on the head SHA (among runs at the pinned SHA, see P3-3) decides the merge box. The stop rule did not trigger.** Still open: the queued cells and n1, (f), the approval part of (c), (g1) with the real Restore button, and the default-branch check. "Which run counts" now states the cross-PR effect (g2). - P0-2. `author_association` is not authority. For the one private org member tested, a workflow token read `CONTRIBUTOR`; after the membership was made public, it read `MEMBER`. The design does not rely on it for authority: overrides use the collaborator permission API ("Override"). It stays a context signal only ("Prefilter checks"). - P0-3. Run controls and the merge box. `gh run cancel` and `gh run rerun` fail on ruleset-started runs (404, because the workflow lives in the source repo). The REST endpoints `POST /repos/{owner}/{repo}/actions/runs/{id}/cancel` and `/rerun` work. The "Re-run all jobs" button is not tested on a ruleset-started run. A job that hits `timeout-minutes` ends as `cancelled`; the timeout is only an annotation. So the run is `cancelled`, not `timed_out`. The merge box reads runs per head SHA across PRs: a failing run of a second PR with the same head blocked the first PR, and a passing run on one PR cleared the other. - P0-4. V16: the epoch events `head_ref_force_pushed`, `head_ref_deleted`, `head_ref_restored` and `base_ref_changed` all appear with server times. A force push of the head branch while the PR is closed adds no event, and GitHub refused the reopen, so no `reopened` event and no new run followed. `reopened` did appear on a normal reopen. By the V16 rule, `reopened` must join the epoch events. That change also breaks the close-and-reopen step in "To apply" (an override posted before the reopen would be older than the epoch). The owner ruled in K-12: `reopened` moves the epoch, and an override is applied by a REST re-run ("Override"). From 919495fde2d8a7f3d09bf61285e48a3b7c2cc9c2 Mon Sep 17 00:00:00 2001 From: Irving Popovetsky Date: Tue, 27 Oct 2026 14:44:23 -0700 Subject: [PATCH 3/5] Make the model a per-pass setting; Sonnet 5.5 candidate Co-Authored-By: Claude Sonnet 5.5 --- docs/build-plan.md | 2 +- docs/claude-review-design.md | 11 ++++++----- review/src/oc_review/analyze.py | 6 ++++-- review/tests/unit/test_analyze.py | 10 +++++++++- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/docs/build-plan.md b/docs/build-plan.md index cd0ab03..57d35a4 100644 --- a/docs/build-plan.md +++ b/docs/build-plan.md @@ -92,7 +92,7 @@ Depends on: none for most tasks; V18 for `collect.py`, V16 for the push epoch in ### 1b. Analyze and publish -- [x] `analyze.py`: one Messages API call per pass, no tools, `claude-opus-5-5`, explicit `effort` and `max_tokens`, structured output with the stripped schema and a full schema check in code (V5), fallback, at most 2 retries, a request deadline, stale check before the calls. The spend limit error (HTTP 400 `invalid_request_error`, "workspace API usage limits") is an outage with its own clear reason, not a request bug (V9). Always writes exactly three pass records, and exits 0 when it wrote them. The pass record has the optional `meta` field (reason, model, stop reason, schema digest, token usage). Federation goes through the SDK's `WorkloadIdentityCredentials`; placeholder IDs make it refuse to call, and the `release`-marked test (`pytest -m release`) fails on them; phase 3 set the production IDs. Tests use a fake client. +- [x] `analyze.py`: one Messages API call per pass, no tools, the model set per pass in code (`claude-sonnet-5-5` is the candidate, see P3-3), explicit `effort` and `max_tokens`, structured output with the stripped schema and a full schema check in code (V5), fallback, at most 2 retries, a request deadline, stale check before the calls. The spend limit error (HTTP 400 `invalid_request_error`, "workspace API usage limits") is an outage with its own clear reason, not a request bug (V9). Always writes exactly three pass records, and exits 0 when it wrote them. The pass record has the optional `meta` field (reason, model, stop reason, schema digest, token usage). Federation goes through the SDK's `WorkloadIdentityCredentials`; placeholder IDs make it refuse to call, and the `release`-marked test (`pytest -m release`) fails on them; phase 3 set the production IDs. Tests use a fake client. - [x] `publish.py`: escaping, inline `COMMENT` review, sticky summary, final check, exit code (0 only for `pass`). It never approves, requests changes, merges, closes, labels or edits the PR (spec decision K-1). A summary over the comment limit is cut in the spec's order, or fails closed with the full record in the gate job log. The summary shows token counts, no currency. Depends on: V3, V5, V9 (analyze); V6, V16, V24 (publish). diff --git a/docs/claude-review-design.md b/docs/claude-review-design.md index 054bcbf..e77829d 100644 --- a/docs/claude-review-design.md +++ b/docs/claude-review-design.md @@ -348,7 +348,7 @@ Three passes, each one call to the Messages API with no tools. Do not use `claud ### Request shape -- **Model:** `claude-opus-5-5`. Adaptive thinking is always on for this model ([what's new](https://platform.claude.com/docs/en/models/opus-5-5/whats-new-opus-5-5)). Set `output_config.effort` explicitly per pass. +- **Model:** per pass, set in code (`MODEL` in `analyze.py`, like effort). Every pass uses `claude-sonnet-5-5` (decision log P3-4). `claude-opus-5-5` was the model until then and is the reference of the Sonnet eval. The switch takes effect at the next release: the production rule pins a SHA, so merging does not change production. Both support adaptive thinking, structured outputs, mid-conversation `system` messages and `fallbacks: "default"`. Adaptive thinking is always on for Opus 5.5 ([what's new](https://platform.claude.com/docs/en/models/opus-5-5/whats-new-opus-5-5)), and `thinking: {type: "disabled"}` is a 400 on both. Set `output_config.effort` explicitly per pass. Sonnet 5.5 recalibrates the effort levels; the effort values carried over and the Sonnet eval matched 13 of 13 with them (P3-4). - **Output limit:** set `max_tokens` explicitly per pass, in code. Thinking tokens count toward `max_tokens` ([Messages API](https://platform.claude.com/docs/en/api/messages)), so the value bounds thinking plus output. The values are 16,000 for `review` and `security` and 32,000 for `supply-chain` (tuned in the phase 2 evals). A `max_tokens` stop makes the pass `failed`. - **Structured output:** `output_config.format` with the findings JSON schema below. Structured outputs do not support some JSON Schema keywords: `minLength`, `maxLength`, `minimum`, `maximum`, `exclusiveMinimum`, `exclusiveMaximum`, `multipleOf`, `minItems`, `maxItems` and `uniqueItems`. So analyze sends a stripped copy of the schema without these keywords, removed only where they are schema keywords (a property named `maximum` stays), and records the digest of the schema it sent in `meta.request_schema_sha256` on every pass record. The full schema stays the contract. Analyze checks every response against it, and a response that fails it makes the pass `failed`. The publisher checks it again when it reads the pass envelope (see "Job handoff and provenance"). Code never drops a finding to fit, and it cuts text in one case only: before the check, analyze cuts every string over its schema `maxLength` (`summary`, `file`, `quote`, `title`, `explanation`) to that limit, and the finding stays (P3-2). A cut file path ends in "/" so that it names no file (P3-2). The cut is driven by the full schema, not by a list of fields. When the string holds a credential-like string, found as the publisher finds it, the cut is taken from the redacted string, so no part of a secret survives the cut (the publisher redacts before it cuts, see "Publishing (code)"). The cut never changes category, severity, confidence, side or line, and blocking never reads the cut strings, so it cannot lower risk. Every other schema error (type, `minLength`, enum, a missing or extra key) still fails the pass. The prompts state the limits, so the model knows them (V5). A pass with more findings than its limit (10, 10, 15) that is valid against the full schema stays `ok`; the gate fails it in one place, not waivable (see "Reasons"), and its findings still show. - **Refusals:** security content can trip safety classifiers. Enable the server-side fallback (`fallbacks: "default"` with beta `server-side-fallback-2026-07-01`). Every attempt that produced output is billed separately ([refusals and fallback](https://platform.claude.com/docs/en/build-with-claude/refusals-and-fallback)), so the cost bound counts each fallback attempt. Record the top-level `model` field. Always check `stop_reason` before reading content: only `end_turn` is accepted, and `max_tokens`, `refusal`, `pause_turn` or any other value makes the pass `failed`, even when the text looks like valid JSON. Analyze reads the last `text` block, so a fallback block before it is ignored. Any API error also makes the pass `failed`, which makes the gate fail. Each failed pass records a clear reason in `meta.reason`: "workspace spend limit reached" (see "Auth and cost"), "request rejected" (any other 4xx, none retried), "authentication failed" (401 or 403, or a token that cannot be read), "API unavailable" (408, 409, 429, 5xx, a connection error or a timeout, after the retries), "deadline exceeded", "render failed", "PR read failed", "federation not configured", "OIDC token unavailable", "OIDC claim mismatch", "static credential refused", "API host not allowed", "missing environment variable: ", a response with no text block, that is not valid JSON or that does not match the findings schema (the reason names the JSON path and the keyword, never the value), and "internal error". @@ -368,7 +368,7 @@ messages: system: pass-specific instructions ("You are running the supply-chain pass. ...") ``` - Mid-conversation `system` messages are supported on Claude Opus 5.5 with no beta header. They must follow a user message and be the last entry. + Mid-conversation `system` messages are supported on Claude Opus 5.5 and Claude Sonnet 5.5 with no beta header. They must follow a user message and be the last entry. - **Cache order:** a cache entry is available only after the first response starts ([prompt caching](https://platform.claude.com/docs/en/build-with-claude/prompt-caching)). Start the supply-chain pass first, and start the other two when its response begins to stream, or when it ends without a stream (for example an API error), so each pass gets its own result. The cost bound does not depend on cache hits. - **Delimiters:** `{nonce}` is 16 random hex characters per run. Code checks that the rendered bundle does not contain the string `untrusted_pr_{nonce}` before sending. Before that, render replaces any text that reads as the delimiter name (in any case, with lookalike letters or invisible characters inside) with `[NEUTRALIZED:delimiter]`, and shows control, format and other invisible characters as ``. If the check still fails, the pass fails; render does not retry with a new nonce. @@ -537,7 +537,7 @@ The gate record is a run of the central workflow on the PR head SHA. GitHub crea - **Cross-PR effect (V6 g2).** Two PRs with one head H share the runs on H. A failing or pending run on PR 2 blocks PR 1. A pass on PR 2 clears PR 1, also when PR 1 has no override. An override covers the code pair (head and merge base), not one PR, so an override on PR 2 can clear PR 1. Anyone who can open a PR with head H (a fork can fetch the commit) can start a run on H, so they can block PR 1 or start a new run on it. - For an immediate stop, a maintainer closes the PR. Reopen then starts a new run; it is not an atomic swap of the old result. -**Summary writes.** One sticky comment, best effort. Each gate job writes its own result into it, so the last writer wins, whichever run that is. The summary says "result for run R, attempt N", names its head SHA and merge base, and links to its run. A job that fails after its summary write can leave a `pass` summary next to a failed run, a normal cancel of the run can leave a `pass` summary next to a `cancelled` run (the gate job keeps running), and a repo writer can post a bot comment with the marker. Neither changes the gate. The summary says: "The result that counts is the required workflow in the merge box." +**Summary writes.** One sticky comment, best effort. Each gate job writes its own result into it, so the last writer wins, whichever run that is. The summary says "result for run R, attempt N", names its head SHA and merge base, and links to its run. A job that fails after its summary write can leave a `pass` summary next to a failed run, a normal cancel of the run before the gate job starts can leave a `pass` summary next to a `cancelled` run (the gate job still runs; a normal cancel while it runs was not tested), and a repo writer can post a bot comment with the marker. Neither changes the gate. The summary says: "The result that counts is the required workflow in the merge box." ### Pilot and enforcement @@ -752,7 +752,7 @@ Target: [claude-review-test](https://github.com/OperationCode/claude-review-test | Outage: invalid federation rule | Pass, live: two denied exchanges | runs 40561015054 and 40583290922 | | Outage: exhausted budget | Covered by V9 and the unit tests; no Console limit edit (ruling Q7, P3-1) | V9 | -PR 9 `collect` and `oc-claude-review` failed with HTTP 422 seconds after the push and were never re-run. The 32 MiB packument cap hard-blocks `renovate` (K-11). +PR 9 `collect` and `oc-claude-review` failed with HTTP 422 seconds after the push and were never re-run. The 32 MiB packument cap ("Registry lookups") hard-blocks `renovate`; K-11 covers lookup count and time, not this cap. Acceptance list, run without account X (V6 gives 33 run attempts and 6,153 observer readings on 10 PRs; none showed a clear box on a failed, cancelled or missing run of the workflow at the pin, except the documented residuals: the reopen-to-run gap, and an older pass exposed after run deletion). Secret scan of 33 run attempts for `eyJ`, `sk-ant-` and bearer strings: 0 hits. @@ -958,4 +958,5 @@ P2: phase 2 rulings, 2026-10-27 (UTC). Where a P2- line changes an earlier line, - P2-5. Expectation format v2 and the fixture builder ("Testing"). An exact severity failed a "high" case on a `critical` answer, and there was no way to state "no high findings", a citation side, verification, alternative categories or a redacted summary. Now severity means "or higher", and `max_severity`, `file`, `side`, `verified`, category lists and `summary_excludes` exist. A hand-written bundle made "hard block regardless of Claude" true by construction, so every fixture bundle is built by the real `collect()` and a unit test checks it against a rebuild. - P3-1. Phase 3 rulings (owner and coordinator, 2026-10-27). Q1: a new production service account `claude-review` (`svac_01PiMgfb6qJgNpLZUV8hprCC`), separate from the test account, so test teardown cannot break production. Q2: the coordinator's approval of `eval` runs extends through phase 4; "prevent self-review" stays off until phase 5. Q3: test work in claude-review-test and `p3-*` branches in `.github` is approved; the non-member fork author (account X) is pending, and such an account becomes an outside collaborator, not an org member, when given repo access. Q4: `test.yml` runs `pytest tests/unit -m release` on every PR, in the `unit-tests` job. Q5: no workflow change on main for V22 a; the evidence is `job.workflow_sha` against the pin. Q6: one eval, at the candidate commit only. Q7 (design change): no Console edit lowers the test workspace spend limit; the exhausted-budget outage case is covered by V9 (phase 0) and the unit tests; the invalid federation rule case still runs live (denied exchange, V26 b). V26 a: R1 was created at the placeholder SHA and the Console accepted it (2026-10-27); R2 was created the same way. - P3-2. Over-long strings (design change). Release eval [run 40517202830](https://github.com/OperationCode/.github/actions/runs/40517202830) at the candidate S1 (ccbae51) matched 12 of 13 cases: in `obfuscated-test-payload` the supply-chain pass failed with "response does not match the findings schema: 'findings'/1/'quote': [maxLength]". Phase 2 run 40445885087 matched the same case with the same prompts, so this is model variance. Cause: structured outputs cannot enforce `maxLength` (P0-12), so the API accepts a string of any length, and one long string made the whole pass `failed` and the gate fail on a real PR. Fix (coordinator ruling, 2026-10-27): before the full schema check, analyze cuts every string over its schema `maxLength` to that limit and keeps the finding. This covers `summary` (1500), `file` (400), `quote` (300), `title` (120) and `explanation` (1200), and any limit added to the schema later; a unit test checks that the cut reaches every `maxLength`. A file path is an identifier, not prose, so its cut differs: the first 399 characters and a final "/" (fix round 1, review r-p3-cut Important 1). A plain prefix of a 401-character path can be the path of another changed file, which could then verify the finding and place its inline comment on the wrong file. No file path ends in "/", and the file lookup never matches one, so a cut path names no file. The finding stays and still counts toward the gate. The system prompt also tells the model to copy the part of a longer line that shows the problem. Why it does not lower risk: a finding is never dropped, and its category, severity, confidence, side and line never change. Blocking never depends on the text, the quote or verification ("Citation check"). Whitespace normalization keeps a prefix of a quote inside the normalized text wherever the whole quote is, so a quote that verified still verifies. A cut quote that is not at that place, a cut quote that is only whitespace or holds a redacted credential, or a cut file path, stays unverified and still blocks. A cut can make two findings that differed past the cut share file, title and quote (review r-p3-cut Important 2). A redaction before the cut can do the same far below the limit: two titles that differ only in a credential both become `Token [REDACTED:jwt]` (fix round 2, review rr-p3-cut-fix1 Important A). Two findings below every limit can also share file, side, title and quote and differ in explanation or severity. So each inline comment carries two keys ("Publishing (code)", fix round 3, review rr-p3-cut-fix2): the D14 key (file, side, title and quote) and a full key of every field that the reader sees, both of the shown, redacted text. In one run, findings are deduped by full key, so each distinct finding gets its own inline comment. Across runs, a finding whose D14 key is already on the PR is not posted again; when its full key matches none of the earlier comments with that D14 key, the summary shows it in full. So none hides the explanation of another, and a re-run or a push does not post every finding again (fix round 2 keyed only on every shown field, which re-posted almost every finding on each run, because the explanation and the line change). Fix round 4 (review r-p3-cut-final, coordinator rulings 2026-10-27): the marker alone is not proof that a comment shows a finding, because any Actions workflow in the repo writes as `github-actions[bot]` and the keys are public hashes. The D14 skip needs the inline layout, path, side and title, and "see the earlier comment" needs the exact body at the same line; a forged comment can at most move a finding to its full summary block. A cut file path makes a finding unverified, and an unverified high or critical finding now keeps its never-cut rank in the summary, so the summary shows every such explanation or the run fails closed. The residual of one summary comment (a low or medium explanation can be cut on a changed-key re-run at the largest sizes; capacity can fail a run closed) is stated in "Publishing (code)"; continuation comments are planned for phase 4. Findings that are equal in every shown field after the cut show one inline comment; the publisher cannot know the text past the cut, and that comment holds all the text of each. A credential-like string in an over-long string is redacted before the cut, so the cut cannot leave part of a secret that the publisher would then not recognize. The publisher escapes every character of a cut string, so a cut inside Markdown, HTML or an escape still shows as text (unit test). A cut title, explanation or summary has no truncation mark, because the publisher shows one only for its own cuts. A new candidate with this change needs its own eval (P0-15). -- P3-3. Phase 3 results and spec corrections (2026-10-27). Four spec claims did not survive the live tests, and each is now corrected in place. (1) The merge box counts only runs at the pinned SHA, not the newest run on the head SHA; a pin move back revives older passes at that SHA (PR 28 on 2026-10-27 at 18:34:53Z; PRs 28 and 37 at 21:38:44Z). A forward move fails closed (the box blocks at once). The rollback is a residual, covered by the rule to start a new run on each open PR after any pin move; it joins the residuals in rollout step 7. (2) Repo Actions settings do not affect the required run (V12 pass); the role table no longer lists them as an admin power against the gate. (3) A normal cancel does not stop the `always()` gate job, which then writes a `pass` summary next to a `cancelled` run; the run conclusion still blocks the box, and only a force-cancel stops the gate job. (4) The artifacts of earlier attempts are gone after a re-run; the gate reads only its own attempt, so it still fails closed, and the residual is that earlier pass records cannot be read back. Release procedure: two Console edits per release (candidate SHA, then merge SHA), a tree check of the merge SHA, and a denied exchange at the merge SHA before the second edit. NOT YET RUN, pending account X: V8, V23, V6 f as a Write user, V25 and the web UI "Re-run all jobs" case. The exhausted-budget outage case stays covered by V9 and the unit tests (Q7). +- P3-3. Phase 3 results and spec corrections (2026-10-27). Four spec claims did not survive the live tests, and each is now corrected in place. (1) The merge box counts only runs at the pinned SHA, not the newest run on the head SHA; a pin move back revives older passes at that SHA (PR 28 on 2026-10-27 at 18:34:53Z; PRs 28 and 37 at 21:38:44Z). A forward move fails closed (the box blocks at once). The rollback is a residual, covered by the rule to start a new run on each open PR after any pin move; it joins the residuals in rollout step 7. (2) Repo Actions settings do not affect the required run (V12 pass); the role table no longer lists them as an admin power against the gate. (3) A normal cancel before the gate job starts does not stop the `always()` gate job (run 40565538379), which then writes a `pass` summary next to a `cancelled` run; a normal cancel while the gate job runs was not tested; the run conclusion still blocks the box, and a force-cancel stopped the gate job after the summary (run 40565910355). (4) The artifacts of earlier attempts are gone after a re-run; the gate reads only its own attempt, so it still fails closed, and the residual is that earlier pass records cannot be read back. Release procedure: two Console edits per release (candidate SHA, then merge SHA), a tree check of the merge SHA, and a denied exchange at the merge SHA before the second edit. NOT YET RUN, pending account X: V8, V23, V6 f as a Write user, V25 and the web UI "Re-run all jobs" case. The exhausted-budget outage case stays covered by V9 and the unit tests (Q7). +- P3-4. Model per pass, Sonnet 5.5 (2026-10-27). The owner proposed `claude-sonnet-5-5` for general use. The model is now a per-pass setting, and every pass uses Sonnet 5.5. The switch needed a full match of the 13 cases (P2-4 bar); Opus 5.5 would stay on any pass where Sonnet missed. Result: two eval runs at 302882f, [40589761270](https://github.com/OperationCode/.github/actions/runs/40589761270) and [40590079545](https://github.com/OperationCode/.github/actions/runs/40590079545), each matched 13 of 13 cases with no failed pass (Opus reference [run 40528717338](https://github.com/OperationCode/.github/actions/runs/40528717338), also 13 of 13). Against Opus, total tokens were 0.97x and 0.98x, output tokens 0.71x and 0.76x, and eval wall time 84 s and 82 s against 138 s. No currency figures were recorded. The switch takes effect at the next release: the production rule pins a SHA, so merging does not change production. diff --git a/review/src/oc_review/analyze.py b/review/src/oc_review/analyze.py index 5c92b6c..587fd40 100644 --- a/review/src/oc_review/analyze.py +++ b/review/src/oc_review/analyze.py @@ -50,7 +50,9 @@ # so they can read the cache entry it writes (spec "Cache order"). FIRST_PASS = "supply-chain" -MODEL = "claude-opus-5-5" +# Per pass, so a pass that Sonnet misses in the release eval can stay on Opus +# (spec "Request shape"). `claude-sonnet-5-5` is the candidate. +MODEL = {name: "claude-sonnet-5-5" for name in PASS_NAMES} EFFORT = {"review": "medium", "security": "medium", "supply-chain": "high"} # Thinking tokens count toward `max_tokens`, so this bounds thinking plus output. MAX_TOKENS = {"review": 16_000, "security": 16_000, "supply-chain": 32_000} @@ -233,7 +235,7 @@ def build_request(pass_name: str, *, system_text: str, user_text: str, pass_text so they share one cache entry. The pass message is a mid-conversation system message, after the untrusted data, as the last entry.""" return { - "model": MODEL, + "model": MODEL[pass_name], "max_tokens": MAX_TOKENS[pass_name], "system": system_text, "messages": [ diff --git a/review/tests/unit/test_analyze.py b/review/tests/unit/test_analyze.py index 2a6aa74..9667925 100644 --- a/review/tests/unit/test_analyze.py +++ b/review/tests/unit/test_analyze.py @@ -354,7 +354,7 @@ def test_request_kwargs_per_pass(tmp_path, configured): users = {json.dumps(c["messages"][0]) for c in calls.values()} assert len(systems) == 1 and len(users) == 1 for name, kw in calls.items(): - assert kw["model"] == "claude-opus-5-5" + assert kw["model"] == "claude-sonnet-5-5" assert "" in kw["system"] user, last = kw["messages"] assert user["role"] == "user" @@ -1446,3 +1446,11 @@ def api(request): except Exception: # noqa: BLE001 - the Messages call is not the point pass assert seen["federation_rule_id"] == other.rule_id + + +def test_every_pass_has_a_model_and_an_effort(): + for name in analyze.PASS_NAMES: + assert analyze.MODEL[name] + assert analyze.EFFORT[name] + assert analyze.MAX_TOKENS[name] + assert set(analyze.MODEL) == set(analyze.EFFORT) == set(analyze.PASS_NAMES) From f17278ea4ec2651e6181637bb4a6dc8c5c7aa882 Mon Sep 17 00:00:00 2001 From: Irving Popovetsky Date: Tue, 27 Oct 2026 14:54:48 -0700 Subject: [PATCH 4/5] Record the Sonnet 5.5 eval; fix three spec statements Decision P3-4 and the model text record two Sonnet runs at 302882f (13 of 13 each) against the Opus reference. The switch takes effect at the next release because the production rule pins a SHA. A normal cancel does not stop the gate job only before it starts (design 540, 961 now match 175). The 32 MiB packument cap is not K-11. Co-Authored-By: Claude Opus 5.5 --- docs/build-plan.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/build-plan.md b/docs/build-plan.md index 57d35a4..a8bc9bb 100644 --- a/docs/build-plan.md +++ b/docs/build-plan.md @@ -92,7 +92,7 @@ Depends on: none for most tasks; V18 for `collect.py`, V16 for the push epoch in ### 1b. Analyze and publish -- [x] `analyze.py`: one Messages API call per pass, no tools, the model set per pass in code (`claude-sonnet-5-5` is the candidate, see P3-3), explicit `effort` and `max_tokens`, structured output with the stripped schema and a full schema check in code (V5), fallback, at most 2 retries, a request deadline, stale check before the calls. The spend limit error (HTTP 400 `invalid_request_error`, "workspace API usage limits") is an outage with its own clear reason, not a request bug (V9). Always writes exactly three pass records, and exits 0 when it wrote them. The pass record has the optional `meta` field (reason, model, stop reason, schema digest, token usage). Federation goes through the SDK's `WorkloadIdentityCredentials`; placeholder IDs make it refuse to call, and the `release`-marked test (`pytest -m release`) fails on them; phase 3 set the production IDs. Tests use a fake client. +- [x] `analyze.py`: one Messages API call per pass, no tools, the model set per pass in code (`claude-sonnet-5-5` on every pass, see spec P3-4), explicit `effort` and `max_tokens`, structured output with the stripped schema and a full schema check in code (V5), fallback, at most 2 retries, a request deadline, stale check before the calls. The spend limit error (HTTP 400 `invalid_request_error`, "workspace API usage limits") is an outage with its own clear reason, not a request bug (V9). Always writes exactly three pass records, and exits 0 when it wrote them. The pass record has the optional `meta` field (reason, model, stop reason, schema digest, token usage). Federation goes through the SDK's `WorkloadIdentityCredentials`; placeholder IDs make it refuse to call, and the `release`-marked test (`pytest -m release`) fails on them; phase 3 set the production IDs. Tests use a fake client. - [x] `publish.py`: escaping, inline `COMMENT` review, sticky summary, final check, exit code (0 only for `pass`). It never approves, requests changes, merges, closes, labels or edits the PR (spec decision K-1). A summary over the comment limit is cut in the spec's order, or fails closed with the full record in the gate job log. The summary shows token counts, no currency. Depends on: V3, V5, V9 (analyze); V6, V16, V24 (publish). @@ -167,6 +167,7 @@ Run the spec's workflow acceptance tests under a test org ruleset that requires - [ ] V25 (NOT YET RUN, pending account X): the pilot bypass. Maintain and Admin can merge past a failed run, Write cannot, and GitHub records each bypass. Record the role `actor_id` values, and whether a direct push to the default branch is blocked. - [x] The rest of the acceptance list in the spec's "Testing" section that needs no account X, except the four unit-test-only cases listed in the next item (approver removed, edited override, artifact removed, gate cancel after inline comments), including the outage process (spec "Phase 3 results"). Design change (spec P3-1): the invalid federation rule case ran live with two denied exchanges (runs 40561015054 and 40583290922); the exhausted budget case is covered by V9 (phase 0) and the unit tests, with no Console limit edit (ruling Q7). - [ ] The acceptance cases that need account X: V6 f as a Write user, and the web UI "Re-run all jobs" button on a ruleset-started run (NOT YET RUN; the override cases ran with the REST re-run). Also not run, covered by unit tests: gate cancelled after inline comments with a refused summary write, approver role removed, edited override, artifact upload removed. +- [x] Sonnet 5.5 on every pass (spec P3-4). Eval at 302882f: runs 40589761270 and 40590079545 each matched 13 of 13 with no failed pass; the Opus reference run 40528717338 also matched 13 of 13. Total tokens 0.97x and 0.98x of Opus, output tokens 0.71x and 0.76x, eval wall time 84 s and 82 s against 138 s. The switch takes effect at the next release: the production rule pins a SHA, so merging does not change production. Owner: From 44cb4ff69dd5f788292f2c887b28647971b05ba5 Mon Sep 17 00:00:00 2001 From: Irving Popovetsky Date: Tue, 27 Oct 2026 14:59:35 -0700 Subject: [PATCH 5/5] Docs: V26 a partial, M1 row wording, cancel statement Co-Authored-By: Claude Opus 5.5 --- docs/build-plan.md | 4 ++-- docs/claude-review-design.md | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/build-plan.md b/docs/build-plan.md index a8bc9bb..35ae69a 100644 --- a/docs/build-plan.md +++ b/docs/build-plan.md @@ -55,7 +55,7 @@ Tasks: Owner: - [x] Owner: create a scratch source repo in the org for the V tests, separate from claude-review-test ([claude-review-p0-source](https://github.com/OperationCode/claude-review-p0-source)). -- [x] Owner: Anthropic Console: a dedicated workspace with a hard monthly spend limit and a service account. Create the production federation rule as in spec "Auth and cost": pattern mode, subject prefix `repo:OperationCode@10479364/*`, scope `workspace:developer`, the review workspace only, and the placeholder SHA `0000000000000000000000000000000000000000` in `job_workflow_ref`. Read it back; if the Console rejects the placeholder, stop and resolve the order of rule ID and SHA first (spec V26 a). Its ID goes into the central code; phase 3 sets the candidate SHA. No API key. Done 2026-10-27: workspace `claude-review`, service account `claude-review`, rule R1 `claude-review-prod-r1` created; the Console accepted the placeholder SHA (V26 a). The read-back output is still not recorded; V26 b passed at the candidate SHA (2026-10-27), which shows R1 held it. +- [ ] Owner: Anthropic Console: a dedicated workspace with a hard monthly spend limit and a service account. Create the production federation rule as in spec "Auth and cost": pattern mode, subject prefix `repo:OperationCode@10479364/*`, scope `workspace:developer`, the review workspace only, and the placeholder SHA `0000000000000000000000000000000000000000` in `job_workflow_ref`. Read it back; if the Console rejects the placeholder, stop and resolve the order of rule ID and SHA first (spec V26 a). Its ID goes into the central code; phase 3 sets the candidate SHA. No API key. Done 2026-10-27: workspace `claude-review`, service account `claude-review`, rule R1 `claude-review-prod-r1` created; the Console accepted the placeholder SHA (V26 a partial). The read-back output is not recorded, so this item stays open; the later V26 b exchange does not replace it. - [ ] Owner: the test workspace (reused for evals, spec P2-1), and a federation rule for `workflow_dispatch` runs in `OperationCode/.github` whose job uses the environment `eval`: pattern mode, the same subject prefix, exact claims `repository_owner_id`, `repository_id`, `event_name` and `environment`, scope `workspace:developer`, that workspace only (spec "Auth and cost"). Its IDs are in `evals.py`. The rule cannot name `eval.yml`: any `.github` workflow that uses `eval` can match, so the owner's approval and the workflow unit test (1c) hold it to `eval.yml`. Test it as in spec V27 before phase 2. - [ ] Owner: on `.github`, create the GitHub environment `eval`: the owner is a required reviewer for every run, and any branch may deploy to it. "Prevent self-review" is off temporarily (owner, 2026-10-26), so that the coordinator, which runs with the owner's token, can approve eval runs it started from reviewed branches; the owner turns it on again in phase 5. Admin bypass stays on. During phase 2 the coordinator's approval stands in for the owner's under the owner's delegation. Before approving a run, check the run that waits for approval: the workflow file and commit it runs, every job in it that uses `eval`, and their effective permissions (no GitHub write). Also check that the PR does not change `eval.yml`, any other workflow file, the eval code, the prompts, the fixtures or the federation setup in a harmful way, and that `test.yml` passed. The approval is the control; the residual risk is an approver who approves without reading. Exception to the no-execute rule (K-4): evals of PRs to this repo may run that PR's code, only after an owner approves the `eval` environment run. Needed before phase 2. - [x] Owner: a test workspace with a very small spend limit for V9. @@ -151,7 +151,7 @@ Exit criteria: Run the spec's workflow acceptance tests under a test org ruleset that requires the central workflow from `OperationCode/.github` at the candidate release SHA and targets only claude-review-test. Only one ruleset requires the workflow there at a time. - [x] Production IDs in `analyze.py` and the release tests in CI (the candidate S1). R1 `claude-review-prod-r1` (`fdrl_01N2ZktTiVKqUvvXwywBFfys`) and the production service account are set; `test.yml` runs `pytest tests/unit -m release`. -- [x] V26 a: R1 created at the placeholder SHA and accepted by the Console (2026-10-27); R2 `claude-review-p3-r2` created the same way, inert until the release simulation. +- [ ] V26 a (partial): the owner created R1 at the placeholder SHA and the Console accepted it (2026-10-27); R2 `claude-review-p3-r2` was created the same way, inert until the release simulation. No read-back output is recorded for R1 or R2. Remaining: read R1 and R2 back. - [x] P3-2: an over-long string no longer fails a pass. Release eval run 40517202830 at S1 matched 12 of 13: one supply-chain quote went over 300 characters, which the API cannot enforce. Analyze now cuts every string over its schema `maxLength` (summary, file, quote, title, explanation) to that limit and keeps the finding (spec P3-2). Fix round 1: a cut file path ends in "/", so it names no changed file; when the file, title or quote is at its limit, the inline comment key covers every shown field, so a cut never hides a distinct explanation. Fix round 2: a redaction can make two titles equal far below the limit, so the key of every finding covers every shown field, at any length. Fix round 3: each inline comment carries the D14 key and the full key, both of the redacted shown text; one run dedupes by full key, a later run skips a D14 key already on the PR and shows the finding in full in the summary when its full key is new. Fix round 4: an earlier comment counts only when its text checks out (the D14 skip needs the inline layout, path, side and title; "see the earlier comment" needs the exact body at the same line), and an unverified high or critical finding is never cut from the summary. A candidate with this change needs its own eval. - [x] V26 b, before the acceptance tests: after the owner sets the production rule to the candidate SHA, a run at that SHA gets a token and a run at another SHA is denied. Run 40560632129 at S1 got a token (2026-10-27, 18:31Z); run 40561015054 at the probe SHA was denied, and the Console shows 6 rows `Failure` `match_claim_value_mismatch` on R1 at 18:33:49Z (owner). - [ ] V26 c, before V22 b: release steps 1 to 6 with a scratch rule R2 on the test workspace, R1 unchanged. Include the second exchange at the release SHA, and check that the workflows rule accepts a scratch ruleset pinned to a SHA that is only on a non-default branch of `.github`. S2 `9752ba7` (PR 10 into `p3-sim-base`) and M2 `1cefe42` both got a token (runs 40585724589 and 40587964524); the rule accepted SHAs that are only on non-default branches. Partial: accepted at S2 and M2 (token inferred through R2); R2 Console rows were not read, no R2 read-back output is recorded, and step 5a was not run for R2/M2. Remaining: R2 read-backs at the placeholder, S2 and M2, and a denied exchange at M2 before the R2 edit. diff --git a/docs/claude-review-design.md b/docs/claude-review-design.md index e77829d..ca538a8 100644 --- a/docs/claude-review-design.md +++ b/docs/claude-review-design.md @@ -172,7 +172,7 @@ Facts about ruleset-required workflows ([troubleshooting rules](https://docs.git + 1 human approval (+ code owner where CODEOWNERS applies) ``` -The required run succeeds only when every job succeeds, and that run result is the gate. Only the gate job may fail because of policy. The other jobs fail only on an infrastructure error, or, for dependency-review, on a vulnerable dependency. Any failed, timed-out or cancelled job fails the run, whatever the gate job does. The gate job still runs (`always()`) to explain the failure in the summary, and treats it as not waivable. A normal run cancel before the gate job starts does not stop it (a normal cancel while the gate job runs was not tested): live (phase 3, run 40565538379), the run was `cancelled` right after analyze and dependency-review finished, and the gate job ran to success and wrote a `pass` summary. The run conclusion stayed `cancelled` and the box blocked, so the gate holds. Only a force-cancel stops the gate job. No path makes the run `success` without an exit 0 from the gate job. +The required run succeeds only when every job succeeds, and that run result is the gate. Only the gate job may fail because of policy. The other jobs fail only on an infrastructure error, or, for dependency-review, on a vulnerable dependency. Any failed, timed-out or cancelled job fails the run, whatever the gate job does. The gate job still runs (`always()`) to explain the failure in the summary, and treats it as not waivable. A normal run cancel before the gate job starts does not stop it (a normal cancel while the gate job runs was not tested): live (phase 3, run 40565538379), the run was `cancelled` right after analyze and dependency-review finished, and the gate job ran to success and wrote a `pass` summary. The run conclusion stayed `cancelled` and the box blocked, so the gate holds. Force-cancel stopped the gate job after the summary in run 40565910355; normal cancel while it runs was not tested. No path makes the run `success` without an exit 0 from the gate job. Only the gate job writes to GitHub, and it writes only comments. It has no Anthropic credential and no `id-token`. Its `actions: read` scope only lists runs for the final check. Its `contents: read` scope is for the re-validation reads (the base ref tip and the compare endpoint), which need it on private repos. No job writes a check run through the Checks API, so no job has `checks: write`. The workflow sets `permissions: {}` at the top, and each job lists its own scopes, so every scope a job does not list is `none`. @@ -738,9 +738,9 @@ Target: [claude-review-test](https://github.com/OperationCode/claude-review-test |---|---|---| | Release eval at S1 | Pass: 13 of 13 matched, 0 missed | [`.github` run 40528717338](https://github.com/OperationCode/.github/actions/runs/40528717338), approved 2026-10-27 15:13:14Z; tokens: input 39399, output 19901, cache write 56524, cache read 113048 | | M1 tree check | Pass: S1 and M1 have tree `6ace36e90f776f1fd9e821851b42dfc79dad13c0`; `git diff S1 M1` empty | `origin/main` = M1 | -| V26 a, b, c | a and b pass; c partial (see V26 c: no R2 read-backs, step 5a not run for R2 and M2) | R1 and R2 accepted at the placeholder; runs [40560632129](https://github.com/OperationCode/claude-review-test/actions/runs/40560632129) (accept), [40561015054](https://github.com/OperationCode/claude-review-test/actions/runs/40561015054) (deny), [40585724589](https://github.com/OperationCode/claude-review-test/actions/runs/40585724589) (S2 via R2), [40587964524](https://github.com/OperationCode/claude-review-test/actions/runs/40587964524) (M2 via R2) | +| V26 a, b, c | a partial (R1 and R2 created at the placeholder and accepted by the Console; no read-back recorded); b pass; c partial (see V26 c: no R2 read-backs, step 5a not run for R2 and M2) | The owner created R1 and R2 at the placeholder and the Console accepted them; runs [40560632129](https://github.com/OperationCode/claude-review-test/actions/runs/40560632129) (accept), [40561015054](https://github.com/OperationCode/claude-review-test/actions/runs/40561015054) (deny), [40585724589](https://github.com/OperationCode/claude-review-test/actions/runs/40585724589) (S2 via R2), [40587964524](https://github.com/OperationCode/claude-review-test/actions/runs/40587964524) (M2 via R2) | | Deny at the merge SHA, before the second edit | Pass: run [40583290922](https://github.com/OperationCode/claude-review-test/actions/runs/40583290922) at M1, PR 28 reopened 21:02:02 to 21:02:08Z; 3 passes `failed (authentication failed)`; gate fail; box blocked from 21:02:01Z | Exchange window 21:02:36 to 21:02:57Z, R1 then at S1. Console reason not read | -| Accept at M1 after the second edit | Pass: run [40585413003](https://github.com/OperationCode/claude-review-test/actions/runs/40585413003), PR 28 reopened 21:16:44Z, 3 `ok`, box clear 21:18:12Z; PR 37: run [40586089942](https://github.com/OperationCode/claude-review-test/actions/runs/40586089942), clear 21:22:46Z | R1 at M1 (no read-back or Console evidence recorded, only the working token); ready for phase 4 | +| Accept at M1 after the second edit | Pass: run [40585413003](https://github.com/OperationCode/claude-review-test/actions/runs/40585413003), PR 28 reopened 21:16:44Z, 3 `ok`, box clear 21:18:12Z; PR 37: run [40586089942](https://github.com/OperationCode/claude-review-test/actions/runs/40586089942), clear 21:22:46Z | R1 at M1 (no read-back or Console evidence recorded, only the working token); M1 exchange passed; phase 3 exit criteria remain open | | Simulation (R2, S2, M2) | Partial pass: see V26 c. Remaining: R2 read-backs at the placeholder, S2 and M2; a denied exchange at M2 before the R2 edit to M2 | S2 `9752ba7c04d796822c64c670415d1046607c3c03`, M2 `1cefe4259bc26197ec4b063e8399ef856efdd4f2`, PR 10 into `p3-sim-base`, never main | | V22 a, V22 b | Pass, with a rollback residual (see "Which run counts") | V22 | | V11 | Pass | V11 | @@ -794,7 +794,7 @@ The tasks, owner actions and exit criteria for each step are in [docs/build-plan - [ ] Create the public repo `OperationCode/.github` and push this local repo. - [ ] Protect `.github` main: CODEOWNERS review, 1 approval, no force push, no deletion. CODEOWNERS is the Contributors team, which needs write access to the repo. Org admins may bypass through a pull request only. - [ ] Also on `.github` main: dismiss stale approvals on push, and require the `test.yml` check (the job `unit-tests`). -- [x] Anthropic Console: dedicated workspace with a hard monthly spend limit, service account, federation rule as in "Auth and cost" (pattern mode, ID-form subject prefix, scope `workspace:developer`, the review workspace only, the placeholder SHA until phase 3; V26). Done 2026-10-27: workspace `claude-review`, service account `claude-review`, rule R1 `claude-review-prod-r1` created at the placeholder SHA and accepted by the Console (V26 a); no read-back is recorded yet. +- [ ] Anthropic Console: dedicated workspace with a hard monthly spend limit, service account, federation rule as in "Auth and cost" (pattern mode, ID-form subject prefix, scope `workspace:developer`, the review workspace only, the placeholder SHA until phase 3; V26). Done 2026-10-27: workspace `claude-review`, service account `claude-review`, rule R1 `claude-review-prod-r1` created at the placeholder SHA and accepted by the Console (V26 a partial); no read-back is recorded, so this item stays open. - [ ] Anthropic Console, eval part: the eval federation rule (V27) on the test workspace `claude-review-p0-test`, separate from the review workspace (P2-1). A GitHub environment `eval` on `.github` that requires the owner's approval for every run, allows any branch, and has "prevent self-review" off during phase 2 (owner, 2026-10-26; back on in phase 5). No API key. - [ ] Org Actions setting: default `GITHUB_TOKEN` permissions read-only. - [ ] Org setting: require 2FA for all members. A stolen maintainer account bypasses everything in this design. @@ -848,7 +848,7 @@ These are assumptions I could not confirm from docs. Each has a concrete test. I 23. **Same-path workflow in the target repo (blocking).** T6 was inconclusive: the PR's copy never ran, and the scratch repo was both source and target. Use a source repo separate from the target. Test: a fork PR adds `.github/workflows/claude-review.yml` to the target repo with a passing job `oc-claude-review`, while the central run fails. Pass when the box blocks. **Phase 3 (2026-10-27): NOT YET RUN (pending account X).** Partial evidence only, from a same-repo PR by an org admin (PR 35): the PR added `.github/workflows/claude-review.yml` with a passing job `oc-claude-review` (run 40572849151, higher ID, success at 19:51:34Z) and a bidi file. The central run [40572849014](https://github.com/OperationCode/claude-review-test/actions/runs/40572849014) failed (blocking-finding, prefilter-hard-block, `superseded`: the final check counted the same-path run). The box stayed blocked throughout and after. 24. **Answered: run list for the final check (build).** 2026-10-25, pass. The job's `GITHUB_TOKEN` with `actions: read` listed runs by head SHA on same-repo `pull_request` runs and on `pull_request_target` runs of fork and same-repo PRs, including ruleset-started runs, with the correct `path`. A new run first appeared in the list within 5 s of its creation in every measurement (poll step about 3 s). The final check stays defense in depth. Evidence: [40227607202](https://github.com/OperationCode/claude-review-test/actions/runs/40227607202) and [40227659578](https://github.com/OperationCode/claude-review-test/actions/runs/40227659578) (same-repo); [40230848821](https://github.com/OperationCode/claude-review-test/actions/runs/40230848821) (ruleset-started runs, PR [17](https://github.com/OperationCode/claude-review-test/pull/17)); b1 run A [40236352621](https://github.com/OperationCode/claude-review-test/actions/runs/40236352621) listed run B (fork PR [15](https://github.com/OperationCode/claude-review-test/pull/15)). 25. **Pilot bypass (blocking).** The docs list the `RepositoryRole` actor type but not the `actor_id` values of the built-in roles. Test: in the scratch repo, an org ruleset with only the workflows rule and a bypass for the Maintain and Admin repository roles in `pull_request` mode. Read it back and record the `actor_id` values. Pass when, on a failed run, a user with Maintain and a user with Admin can merge with the bypass, a user with Write cannot, and GitHub records each bypass. Also record whether a direct push to the default branch is blocked; before a repo joins the pilot, check it for people or bots that push to the default branch directly. **Phase 3 (2026-10-27): NOT YET RUN (pending account X).** It needs accounts with Write, Maintain and Admin on claude-review-test, and the bypass list on the test ruleset, which stays empty until then. The role `actor_id` values are not recorded yet. -26. **Placeholder SHA in a federation rule (verify in phase 3; part a done 2026-10-27).** The setup and every release create a rule at the placeholder SHA `0000000000000000000000000000000000000000` and later edit it to a real SHA ("Each release" in "Auth and cost"). V5 and V9 edited a rule from one real SHA to another; no test created a rule with a SHA that matches no commit. Test: (a) at setup, create the production rule at the placeholder and read it back. If the Console rejects it, stop, and resolve the order of rule ID and SHA before the production setup goes on. Result of part a (2026-10-27): the owner created R1 and the Console accepted it; no read-back output is recorded, so read R1 back before part b. R1 `claude-review-prod-r1` (`fdrl_01N2ZktTiVKqUvvXwywBFfys`) was created with `job_workflow_ref` at the placeholder SHA. The scratch rule R2 `claude-review-p3-r2` (`fdrl_01AfACzupW3U4Bx361ZUbsyR`; service account `claude-review-p0`, workspace `claude-review-p0-test`) was created the same way and stays at the placeholder until the release simulation of part c. (b) In phase 3, before the acceptance tests, edit the rule to the candidate SHA and read it back. Pass when a run under the scratch test ruleset at the candidate SHA gets a token, and a run at another SHA is denied. (c) Before the first release (V22 b), do steps 1 to 6 of "Each release" with a scratch rule R2 on the test workspace, including the re-edit of R2 to the merge SHA and the second exchange. Do not change R1 during this check. Pass when R2 reads back at the placeholder, then at the candidate SHA, then at the release SHA, and the exchange succeeds at both real SHAs. The scratch ruleset is pinned to a SHA that is on a non-default branch of `.github` at step 4 (a fork branch counts if the fork is the test path); pass only if the workflows rule accepts and runs it. If it does not, validate after the merge, at the main SHA, and the order of the steps changes. **Results (phase 3, 2026-10-27).** (b) pass: with R1 at S1 `3aa7ae9` the run [40560632129](https://github.com/OperationCode/claude-review-test/actions/runs/40560632129) (PR 28, reopened 18:30:32Z) got a token, 3 passes `ok`, gate pass, box clear at 18:32:12Z (exchange window 18:31:01 to 18:31:40Z). The run [40561015054](https://github.com/OperationCode/claude-review-test/actions/runs/40561015054) at the probe `13a1e48` (S1 plus one empty commit, on branch `p3-deny-probe`; TR pinned to it at 18:32:57.9Z) was denied: 3 passes `failed (authentication failed)`, gate fail, box blocked (window 18:33:32 to 18:33:56Z). Console, read by the owner (2026-10-27): the deny run at 18:33:49Z shows 6 rows `Failure`, reason `match_claim_value_mismatch`, rule `claude-review-prod-r1`, service account `claude-review`. Before the merge, the owner reported `Success` for the R1 rows in the windows 18:31:01 to 18:31:40Z and 19:16:09 to 19:16:46Z. The windows from 21:02Z on (M1 deny, M1 accept, PR 37) and all R2 rows were not checked. There is no screenshot, and the token scope and workspace are not separately confirmed. The read-back output of R1 at the placeholder is still not recorded. (c) partial pass: accepted at S2 and M2, token through R2 inferred; no R2 read-backs, and step 5a (a denied exchange at the merge SHA before the second edit) was not run for R2 and M2 (the deny at the merge SHA ran only for R1 at M1). Not done, and still to do: R2 read-backs at the placeholder, S2 and M2, and a denied run at M2 before the R2 edit. S2 `9752ba7` (M1 plus R2 and the test service account; three assertions in `test_evals.py` adjusted, simulation only; unit tests 2563 passed, `-m release` 2 passed) on branch `p3-sim`, PR 10 into `p3-sim-base` (branched from M1), never main. TR accepted a SHA that is only on a non-default branch (S1, and S2 and the probe) and started runs at it. Step 4: TR pinned to S2 at 21:18:49Z, run [40585724589](https://github.com/OperationCode/claude-review-test/actions/runs/40585724589) at S2: 3 `ok`, box clear 21:20:18Z (window 21:19:19 to 21:19:45Z). Step 5: PR 10 squash-merged into `p3-sim-base` at 21:20:59Z gave M2 `1cefe4259bc26197ec4b063e8399ef856efdd4f2`; `git diff S2 M2` is empty. Step 6: runs [40587964524](https://github.com/OperationCode/claude-review-test/actions/runs/40587964524) and [40587977659](https://github.com/OperationCode/claude-review-test/actions/runs/40587977659) at M2, 3 `ok` each (window 21:35:22 to 21:35:54Z). R2 rows were not read in the Console, so the token through R2 is inferred: R1 names M1 and cannot match a run at M2. The reports record no read-back output for R2 at the placeholder, S2 or M2. R1 was not changed during the simulation, and not deleted. +26. **Placeholder SHA in a federation rule (verify in phase 3; part a partial 2026-10-27: creation accepted, read-back not recorded).** The setup and every release create a rule at the placeholder SHA `0000000000000000000000000000000000000000` and later edit it to a real SHA ("Each release" in "Auth and cost"). V5 and V9 edited a rule from one real SHA to another; no test created a rule with a SHA that matches no commit. Test: (a) at setup, create the production rule at the placeholder and read it back. If the Console rejects it, stop, and resolve the order of rule ID and SHA before the production setup goes on. Result of part a (2026-10-27): the owner created R1 and the Console accepted it; no read-back output is recorded, so part a is partial: placeholder creation accepted, read-back not recorded. Read R1 back to close it. R1 `claude-review-prod-r1` (`fdrl_01N2ZktTiVKqUvvXwywBFfys`) was created with `job_workflow_ref` at the placeholder SHA. The scratch rule R2 `claude-review-p3-r2` (`fdrl_01AfACzupW3U4Bx361ZUbsyR`; service account `claude-review-p0`, workspace `claude-review-p0-test`) was created the same way and stays at the placeholder until the release simulation of part c. (b) In phase 3, before the acceptance tests, edit the rule to the candidate SHA and read it back. Pass when a run under the scratch test ruleset at the candidate SHA gets a token, and a run at another SHA is denied. (c) Before the first release (V22 b), do steps 1 to 6 of "Each release" with a scratch rule R2 on the test workspace, including the re-edit of R2 to the merge SHA and the second exchange. Do not change R1 during this check. Pass when R2 reads back at the placeholder, then at the candidate SHA, then at the release SHA, and the exchange succeeds at both real SHAs. The scratch ruleset is pinned to a SHA that is on a non-default branch of `.github` at step 4 (a fork branch counts if the fork is the test path); pass only if the workflows rule accepts and runs it. If it does not, validate after the merge, at the main SHA, and the order of the steps changes. **Results (phase 3, 2026-10-27).** (b) pass: with R1 at S1 `3aa7ae9` the run [40560632129](https://github.com/OperationCode/claude-review-test/actions/runs/40560632129) (PR 28, reopened 18:30:32Z) got a token, 3 passes `ok`, gate pass, box clear at 18:32:12Z (exchange window 18:31:01 to 18:31:40Z). The run [40561015054](https://github.com/OperationCode/claude-review-test/actions/runs/40561015054) at the probe `13a1e48` (S1 plus one empty commit, on branch `p3-deny-probe`; TR pinned to it at 18:32:57.9Z) was denied: 3 passes `failed (authentication failed)`, gate fail, box blocked (window 18:33:32 to 18:33:56Z). Console, read by the owner (2026-10-27): the deny run at 18:33:49Z shows 6 rows `Failure`, reason `match_claim_value_mismatch`, rule `claude-review-prod-r1`, service account `claude-review`. Before the merge, the owner reported `Success` for the R1 rows in the windows 18:31:01 to 18:31:40Z and 19:16:09 to 19:16:46Z. The windows from 21:02Z on (M1 deny, M1 accept, PR 37) and all R2 rows were not checked. There is no screenshot, and the token scope and workspace are not separately confirmed. The read-back output of R1 at the placeholder is still not recorded. (c) partial pass: accepted at S2 and M2, token through R2 inferred; no R2 read-backs, and step 5a (a denied exchange at the merge SHA before the second edit) was not run for R2 and M2 (the deny at the merge SHA ran only for R1 at M1). Not done, and still to do: R2 read-backs at the placeholder, S2 and M2, and a denied run at M2 before the R2 edit. S2 `9752ba7` (M1 plus R2 and the test service account; three assertions in `test_evals.py` adjusted, simulation only; unit tests 2563 passed, `-m release` 2 passed) on branch `p3-sim`, PR 10 into `p3-sim-base` (branched from M1), never main. TR accepted a SHA that is only on a non-default branch (S1, and S2 and the probe) and started runs at it. Step 4: TR pinned to S2 at 21:18:49Z, run [40585724589](https://github.com/OperationCode/claude-review-test/actions/runs/40585724589) at S2: 3 `ok`, box clear 21:20:18Z (window 21:19:19 to 21:19:45Z). Step 5: PR 10 squash-merged into `p3-sim-base` at 21:20:59Z gave M2 `1cefe4259bc26197ec4b063e8399ef856efdd4f2`; `git diff S2 M2` is empty. Step 6: runs [40587964524](https://github.com/OperationCode/claude-review-test/actions/runs/40587964524) and [40587977659](https://github.com/OperationCode/claude-review-test/actions/runs/40587977659) at M2, 3 `ok` each (window 21:35:22 to 21:35:54Z). R2 rows were not read in the Console, so the token through R2 is inferred: R1 names M1 and cannot match a run at M2. The reports record no read-back output for R2 at the placeholder, S2 or M2. R1 was not changed during the simulation, and not deleted. 27. **Eval federation rule (verify in phase 2, before the first eval).** The rule cannot name `eval.yml`, because a dispatch run's `job_workflow_ref` ends in the branch ref (V3). It binds `repository_id`, `event_name` and `environment: eval` instead ("Fixture eval credential"). Passed 2026-10-27 (UTC) with one substitute, one case not run, and the P1 scope not confirmed. The other-repo case ran as `push` in claude-review-test with no environment (N4, 401), so `repository_id` is not tested separately. The "another environment" case was not run; only the no-environment case ran (N1). The Console reason for each deny is recorded in "Fixture eval credential" (`match_claim_absent` for N1 and the no-environment job of the rejected-approval run; `match_claim_value_mismatch` for the no-environment job of the push run, N3 and N4); claim name and P1 scope not shown in the list view, so the P1 scope `workspace:developer` is not confirmed. See "Fixture eval credential". Test on the test workspace `claude-review-p0-test`, and record the Console reason for each deny. Pass when an approved `eval.yml` dispatch run in `.github` gets a token with scope `workspace:developer` for that workspace only, and each of these gets no token: a run whose approval is rejected (the job never starts, and the Console shows no exchange); a dispatch run in `.github` with no environment or another environment; a dispatch run with an `eval` environment in another repo (claude-review-test, with its own `eval` environment; this case is required, P2-2); a run in `.github` with the `eval` environment on another event (for example `push`). Also pass when the workflow unit test (see "Testing") fails on a fixture workflow other than `eval.yml` that uses `eval`, and on a job that uses `eval` with a GitHub write permission. The residual stays: an approver who approves without reading. Verify later (phase 3):