docs: Claude lane contract close-out (Phase 5)

CLAUDE.md

@@ -4,46 +4,71 @@ Guidance for working in this repo. The [README](README.md) covers what each
 building block does and the consumer contract; this file captures the rules a
 change must not violate.
 
-## Security ground rules — `claude-review` reusable workflow
+## Security ground rules — the Claude lane reusable workflows
 
-The PR-review workflow runs an AI agent with an org credential on a **public**
-repo. These rules are load-bearing; changing any of them needs explicit review.
+`claude-review.yml`, `claude-security-review.yml`, and `claude-e2e-verify.yml`
+each run an AI agent with an org credential on a **public** repo, and each
+workflow's `SECURITY MODEL` header block cites this file for the rationale.
+These rules are load-bearing; changing any of them needs explicit review.
 
 - **`pull_request` only; never `pull_request_target` or `workflow_run` with
   secrets.** Those triggers run in a privileged context (base-repo secrets +
   write token) over potentially untrusted fork code — the "pwn request" class,
-  i.e. token exfiltration. The workflow keeps a tripwire step that hard-fails on
-  those two events. Do not whitelist a single event (that would block a
+  i.e. token exfiltration. Each workflow keeps a tripwire step that hard-fails
+  on those two events. Do not whitelist a single event (that would block a
   consumer's legitimate `workflow_dispatch` / `schedule`); reject only the
   dangerous two.
 - **Fork PRs are intentionally not reviewed.** GitHub passes no secrets and a
-  read-only token to fork-triggered `pull_request` runs, so the review degrades
-  to a warning. That is the safety guarantee, not a bug — never "fix" it by
-  reaching for `pull_request_target`.
+  read-only token to fork-triggered `pull_request` runs, so the agent cannot
+  run. `claude-review` and `claude-e2e-verify` let it degrade to a warning;
+  `claude-security-review` skips the job at job level instead, because its
+  fail-closed mapping would otherwise pin every fork PR red for a cause no push
+  can fix — and because that skip reads as success, its required check does not
+  prove a fork PR was reviewed, so fork changes to security-sensitive surfaces
+  need a human. The no-secrets guarantee is the safety property, not a bug —
+  never "fix" it by reaching for `pull_request_target`.
 - **SHA-pin both layers.** Consumers pin this reusable workflow `@<40-char-sha>`;
   this workflow pins `anthropics/claude-code-action@<sha>` (mutable tags were the
   tj-actions/CVE-2025-30066 vector). Dependabot bumps the inner pin.
 - **Least privilege, owned by the caller.** A called workflow can only *downgrade*
   the caller's `GITHUB_TOKEN` grant, so the consumer's caller job must grant
   exactly `contents: read` + `pull-requests: write` + `id-token: write`
   (`id-token` mints the Claude GitHub App token and is required even with an
-  OAuth/API-key credential). The review job re-declares that minimal set so an
+  OAuth/API-key credential). Each lane's job re-declares that minimal set so an
   over-granting caller is still narrowed here.
 - **Never touch the org secret.** `CLAUDE_CODE_OAUTH_TOKEN` is an org secret
   with visibility *all repositories* — the live, deliberate value. Never
   revoke, edit, or re-scope it from automation, and do not "correct" it to
   selected-repositories. Forced-failure testing uses a repo-level same-name
   override in a sandbox repo, never a change to the org secret.
 - **Public-repo log hygiene.** Keep `display_report` and `show_full_output` off
-  (defaults) — both can surface model-authored content or secrets in publicly
-  visible logs. Never enable Actions debug (`ACTIONS_STEP_DEBUG`) here. Never
-  echo the token.
-- **No untrusted checkout before the action.** Do not check out a PR head ref
-  into the workspace root ahead of the action step.
+  — both can surface model-authored content or secrets in publicly visible
+  logs. No lane sets `show_full_output`, so it stays at the action's off
+  default. `display_report` is enforced unevenly, and deliberately so:
+  `claude-security-review` hardcodes it `false` so a consumer cannot turn it
+  on, `claude-e2e-verify` never sets it, and `claude-review` exposes it as an
+  input for private consumers — **a public caller must never set
+  `display-report: true`**. Never enable Actions debug (`ACTIONS_STEP_DEBUG`)
+  here. Never echo the token.
+- **Never execute PR-authored code in a review job.** All three lanes check out
+  before the action step, and on `pull_request` the default checkout resolves to
+  the PR merge ref, so PR-authored file content is already in the workspace root
+  — presence is not the hazard, execution is. Never point a checkout at the PR
+  head ref, and never run a build, install script, or test command against that
+  tree in a review lane. `claude-e2e-verify` is the
+  deliberate exception, because building, serving, and browser-driving the PR
+  head *is* that lane: it runs PR-authored code on the runner alongside the
+  persisted `GITHUB_TOKEN` and the agent's credential. That residual is
+  accepted, not mitigated in-job; the reasoning, its bound (the
+  `pull_request`-only tripwire plus the fork-PR no-secrets guarantee), and the
+  hardening direction if the lane's scope widens are recorded in that
+  workflow's own header. Do not widen it, and do not carry the pattern into a
+  review lane.
 
 The `secrets:`/`vars:` context is unavailable in composite actions; that, plus
-the need for job-level `permissions` and the `secrets:` interface, is why review
-is a reusable workflow and the tool-runner lanes are composite actions.
+the need for job-level `permissions` and the `secrets:` interface, is why each
+Claude lane is a reusable workflow and the tool-runner lanes are composite
+actions.
 
 ## Pin everything by SHA
 

README.md

@@ -636,27 +636,38 @@ GitHub continues the normal weekly patching of each hosted image generation.
   repo, so the lane scopes itself to security-sensitive surfaces — but the
   caller must NOT express that scope with a workflow-level `on.pull_request.paths`
   filter, because a path miss leaves a required check Pending forever and wedges
-  every prose PR. Instead the caller triggers on all PR events and passes the
-  `paths` input (security-sensitive surfaces in Actions `paths:` syntax:
-  workflow files, permission/settings configs, hook and shell scripts,
-  auth/token-touching code, network-call sites); the workflow's `changes` job
-  evaluates it and a not-applicable PR yields a name-stable skipped
-  `security-review` check. A consumer's ruleset may make that EXECUTION check
-  required (check context `<caller job> / security-review`); the VERDICT stays
-  advisory. All inputs have public-safe defaults documented inline in the
-  workflow header (the authoritative list). Consume it per the [Claude
+  every prose PR. Instead the caller triggers on all PR events and supplies that
+  scope as a pattern list of root-anchored globs (workflow files,
+  permission/settings configs, hook and shell scripts, auth/token-touching code,
+  network-call sites); the workflow's `changes` job evaluates it and a
+  not-applicable PR yields a name-stable skipped `security-review` check. A
+  consumer's ruleset may make that EXECUTION check required (check context
+  `<caller job> / security-review`); the VERDICT stays advisory.
+
+  **Where that pattern list lives** is the caller's choice between two inputs.
+  The conventional shape is `paths-file`, pointing at a repo-owned file
+  (`.github/claude-security-paths`) so each repo keeps its own
+  security-sensitive-surface list in its own tree; the inline `paths` input
+  takes the same content directly and, when non-empty, wins over the file. The
+  file is read from the PR's **base** branch, never the head, so a PR cannot
+  edit its content to skip its own security review — repointing the input is
+  still possible, but only as a visible caller diff, which is the pre-existing
+  trust boundary. An absent or unreadable file **fails open** (every PR
+  reviewed, with a warning), matching the `changes` job's fail-open discipline
+  throughout; both inputs empty means no filtering, so a consumer that passes
+  nothing is unaffected. Patterns are matched as root-anchored **gitignore**
+  patterns rather than Actions `paths:` patterns — identical for the ordinary
+  `*` / `**` globs worth writing here, but `!` negation cannot be honored as
+  Actions defines it and is rejected outright, so express an exclusion by
+  narrowing the positive patterns. All inputs have public-safe defaults
+  documented inline
+  in the workflow header (the authoritative list). Consume it per the [Claude
   lanes — shared consumption contract](#claude-lanes--shared-consumption-contract)
-  below, triggering on all PR events (no workflow-level `paths:`) and passing
-  the `paths` input in the job:
+  below, triggering on all PR events (no workflow-level `paths:`):
 
   ```yaml
   with:
-    paths: |
-      .github/workflows/**
-      .github/actions/**
-      **/*.sh
-      **/*.ps1
-      # plus the caller's own auth/token and network-call source paths
+    paths-file: .github/claude-security-paths
   ```
 
 - `.github/workflows/claude-e2e-verify.yml` — Claude-powered end-to-end
@@ -878,26 +889,19 @@ GitHub continues the normal weekly patching of each hosted image generation.
 `claude-review.yml`, `claude-security-review.yml`, and `claude-e2e-verify.yml`
 share one consumption shape. Each is **advisory**: it posts PR comments and
 never gates `ci-status`. (The advisory verdict is separate from execution
-evidence: `claude-security-review.yml` additionally takes a `paths` input and
-its name-stable `security-review` check may be made a required status check —
-see its entry above.) Each is a whole-job concern (job `permissions:` plus a
-`secrets:` interface), which is why each is a reusable workflow rather than a
-composite action — the caller owns the triggers and the permission grant, and
-the workflow owns the SHA-pinned `anthropics/claude-code-action` and the safe
-handling. Security rules live in [CLAUDE.md](CLAUDE.md).
+evidence: `claude-security-review.yml` scopes itself to security-sensitive
+paths, and its name-stable `security-review` check may be made a required
+status check — see its entry above.) Each is a whole-job concern (job
+`permissions:` plus a `secrets:` interface), which is why each is a reusable
+workflow rather than a composite action — the caller owns the triggers and the
+permission grant, and the workflow owns the SHA-pinned
+`anthropics/claude-code-action` and the safe handling. Security rules live in
+[CLAUDE.md](CLAUDE.md).
 
 ```yaml
 on:
   pull_request:
-    # Trigger types are PER LANE — copy the canonical caller from the lane's
-    # own workflow header, not this line:
-    #   claude-review:          [opened, ready_for_review, reopened]
-    #     (no `synchronize` — pushes do not re-trigger the code review;
-    #      re-run the job for a fresh pass)
-    #   claude-security-review: [opened, synchronize, ready_for_review, reopened]
-    #     (its check certifies execution at the merge head)
-    #   claude-e2e-verify:      [opened, synchronize, ready_for_review, reopened]
-    types: [opened, synchronize, ready_for_review, reopened]
+    types: [<per lane — see below>]
 jobs:
   <lane>:
     permissions:
@@ -913,7 +917,79 @@ The caller's job must grant those three permissions (a called workflow can only
 downgrade, not elevate); the `CLAUDE_CODE_OAUTH_TOKEN` org secret has
 visibility "all repositories", so every org repo receives it. Pass that one
 named secret explicitly rather than `secrets: inherit`, which forwards every
-parent secret. Fork PRs receive no secrets by design and are not reviewed.
+parent secret. Fork PRs receive no secrets by design and are not reviewed. On
+both review lanes the caller can name actors whose comments are withheld from
+the agent's context — prompt-injection hygiene, not a trigger gate.
+
+**What the security lane's required check proves — and does not.** Only
+`claude-security-review`'s check is designed to be required: it reports under
+its own context (`<caller job> / security-review`), never through `ci-status`.
+Its claim is narrow: a security pass RAN at this head, or the PR was judged not
+applicable. The verdict stays advisory — findings never fail the job. Execution
+is what **fails closed**: when the PR is in scope and the review could not run
+at all (usage limit, dead credential, SDK crash), the check reports FAILURE,
+because `success`, `neutral` and `skipped` all satisfy a required check, so
+failure is the only conclusion that does not silently authorize a merge on
+absent evidence. That mapping is scoped to `pull_request` runs; a non-PR event
+cannot run the review at all and keeps the historical pass-through, and a fork
+PR skips the job outright — neither green is execution evidence.
+
+The corollary is what the check does not prove. Every legitimate non-run reads
+as success to a ruleset. Four are name-stable job-level skips: a fork PR, an
+out-of-scope PR, a skip-listed actor, a kill-switched lane. A fifth is not a
+skip at all — a run whose head was superseded while it queued retires itself
+step by step and reports a **green** job having reviewed nothing, on the
+premise that the newer run for the current head reports the same context. So a
+green required check is not by itself proof that this head was reviewed, and it
+never establishes that a fork PR was: review fork changes to security-sensitive
+surfaces by hand. Availability is bought back by the bounded retry below, and a
+sustained outage is meant to be handled by an explicit, logged, attributable
+break-glass on the consumer's ruleset — never by weakening the check.
+
+**Trigger cadence is per lane, deliberately.** `claude-review` runs on
+`opened` / `ready_for_review` / `reopened` and **not** on `synchronize`: a push
+does not re-trigger the code review, so re-run the job for a fresh pass. That
+caps per-PR spend on active branches, and it is safe precisely because the
+lane's verdict gates nothing. It also skips draft PRs at job level, so an
+`opened` event on a draft costs nothing and `ready_for_review` is what buys the
+review. `claude-security-review` keeps `synchronize`, because its check
+certifies that a security pass ran at the head being merged — a review of an
+earlier head is not that evidence, and it reviews drafts. `claude-e2e-verify`
+keeps `synchronize` too, and gates on nothing but its kill-switches — no draft
+skip, no `skip-actors` input — so the most expensive lane has the loosest gate.
+Scope it with the caller's own trigger types. Take each lane's canonical caller
+from its own workflow header.
+
+**Bounded retry.** Every lane makes at most **two** agent attempts — one
+automatic retry, never a loop. The retry is deliberately narrow, because a
+second attempt after the agent has already spoken duplicates its comments: it
+fires only on **zero assistant turns** in the first attempt's execution file.
+Nor does an **auth-class** failure retry — HTTP 401/402/403, or an
+`authentication_error` / `billing_error` / `permission_error` in the error
+payload — because the credential needs an operator and no retry can clear it.
+The gate also honors the same guards the first attempt does, so a superseded
+run (and, on the code-review lane, a capped one) never spends a retry. Between
+the attempts the lane backs off `retry-delay-seconds` plus a 0–29 second
+jitter, so lanes retrying against the same contended seat do not re-collide in
+lockstep. What this buys is availability against the sporadic-429 class without
+weakening the security lane's fail-closed execution claim.
+
+One divergence is worth knowing. The two review lanes demand **proof** of zero
+turns: a missing or unparsable execution file is not proof — a hard kill can
+lose the file after turns were already spent — so they do not retry on one.
+`claude-e2e-verify` reads an unreadable file as recording no assistant turn and
+does retry. It also sets no `track_progress` tracking comment, so it has no
+orphan comment to clean up between attempts, which the review lanes do.
+
+**Review-count cap (code-review lane only).** `claude-review.yml` stops
+reviewing a PR after `max-reviews-per-pr` successful reviews, capping spend on
+long-lived PRs. The counter is a **visible** per-PR status comment upserted
+after each successful review — failed and skipped runs never inflate it — which
+doubles as the human "was this reviewed" signal;
+deleting it resets the count, which is fail-open by design. A capped run is a
+name-stable skip, not a red check. Treat it as a soft cap: concurrent runs for
+different heads read the counter before either writes it, so a burst can
+briefly exceed it by the number of concurrent heads.
 
 **Kill-switches.** Every lane honors two Actions variables at job level:
 `CLAUDE_LANES_DISABLED` (all lanes) and a per-lane switch
@@ -928,6 +1004,22 @@ disabled NO lane reports security findings — REVIEW.md's code-review
 exclusion keys on the security workflow file existing, and the file remains —
 so re-enable promptly and treat the outage window as security-unreviewed.
 
+All four organization variables carry **all-repositories** visibility. That is
+a deliberate deviation from the org's selected-visibility convention for
+Actions variables, not an oversight: a switch scoped to a selection is invisible
+to every repo outside it, so flipping it during an incident would silently
+no-op exactly where nobody is looking. A kill-switch is only worth having if it
+reaches the whole fleet. Do not "correct" the visibility to selected.
+
+**Adoption.** Each lane's own workflow header carries its canonical caller;
+copy it from there. Callers are additionally being brought under the org's
+sync-managed component distribution in
+[`melodic-software/standards`](https://github.com/melodic-software/standards),
+which materializes a canonical per-lane caller into every target repo and keeps
+it current through the ordinary sync PR. Where a repo's caller is
+sync-managed, change it at the component source and let the sync carry it to
+every target — never edit the materialized caller in the target repo.
+
 ## Standalone gate checks — shared adoption contract
 
 `semantic-pr.yml`, `pr-issue-linkage.yml`, and `do-not-merge-gate.yml` are