feat(claude-lane-incident): route the incident issue to the attended queue - #385
Conversation
…queue Implement the lane-routing half of the ci-workflows#238 Contract that was never built (ci-workflows#364): the incident issue now wears the human-gated role label and carries a machine escalation-marker comment, so it lists as [escalated] in the attended queue instead of waiting to be noticed. Both writes live in the byte-pinned write job, so this is the designed lockstep edit: the workflow's write-job region, its pinned literal, and every fixture that mirrors that region change together in this commit. The nine fixtures that deliberately diverge from the pin (bad-pin and write-gate negative tests) are untouched; the mirror/diverge split was computed from the old pin's bytes, not judged by eye. - The upsert step's labels input adds needs-human. create-issue-from-file runs addLabels on the update path as well as the create path (verified in the action's source at the pinned SHA), so the label is re-asserted fail-closed while the incident is open; the workflow comment claiming labels applied "on creation only" was wrong and is corrected. - A new write-job step posts the escalation-marker comment (work-items:escalation grammar, kind=routed-advisory, lane=incident-aggregator) once, on action == 'open' only: a relapsed incident is superseded by a fresh issue, never reopened, so one comment covers an incident's whole life. The comment body is fixed text; nothing derived from annotations is rendered into it. - No marker suppression branch is needed for transient-only incidents: an incident only ever opens on an escalating class (auth, runner), and both are defined as requiring a human. - Tests: the write job is now four steps; new assertions pin the label pair, the marker step's gate and wiring, the marker grammar as the comment's first line (executed against a mock Octokit), and the no-number guard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HCJfVqDNYt92YRyvKUMgYW
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Claude finished @kyle-sexton's task in 4m 50s —— View job Code Review
SummaryReviewed against The change itself is well-scoped and matches its own description: One finding, posted inline on 🔴 Important — the label write and the marker-comment write aren't atomic, and the design forecloses retry. If the new marker-comment step fails after the upsert step has already re-asserted Everything else — the label re-assertion rationale, the "no suppression branch" argument, the marker-grammar test asserting the comment's first line via a mock-Octokit Note: I did not independently execute |
|
Claude finished @kyle-sexton's task in 2m 28s —— View job Security review complete
No security issues found. This PR adds a label (
One unrelated observation, not a finding: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4eb27a3780
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
…, not open-gated Review rework on the lane-routing change: gating the marker post on action == 'open' alone was not idempotent — if the upsert creates the issue and the marker post then fails (rate limit, 5xx, timeout, cancelled run), every later poll emits 'update' and the marker is never retried, leaving the incident permanently invisible to the attended queue: exactly the parked state the change exists to eliminate. The "re-run failed jobs" duplicate variant was recorded in the same review. The step now runs on open AND update and owns at-most-once itself: it lists the issue's comments and posts only when no bot-authored comment already opens with the marker. The ordinary path still posts exactly once at open and every later update is a read that writes nothing — transition-edge writes preserved — while an update after a failed post repairs the missing marker. Bot-authored only, mirroring the issue lookup's selector filter, so a human-planted decoy cannot suppress the real marker. Full byte-pin lockstep again in this one commit: workflow write-job region + pinned literal + the 40 fixtures that mirror the region. Also re-derives the four negative fixtures the previous commit left stale — write-job-extra-step, write-job-bare-hyphen-step, write-job-duplicate-step-name, write-job-script-edited — onto the new pin with exactly their single named mutation re-applied, restoring the corpus's one-property-differs invariant (they had gone vacuous: failing on staleness alone rather than on their mutation). Tests: the marker-step gate assertion now pins open-or-update; the executed-script suite gains the idempotence case (the existing comment is the one the script itself posts, so check and write cannot drift apart), the update-repair case, and the human-decoy case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HCJfVqDNYt92YRyvKUMgYW
|
Claude finished @kyle-sexton's task in 2m 0s —— View job Security review complete
No security issues found. This PR was already reviewed at
Supply-chain / pin-format concerns are left to zizmor's advisory lane per scope, as before. |
… crediting it (#387) Addresses findings 1 and 3 of #363, observed live during the Phase 4 acceptance test on claude-lane-sandbox (skip: run 31077349229; synchronize disproof: run 31083096934). This is the **A1 half** of the repo's documented A1/A2 delivery shape for composite changes (PLAN.md's convention): the composite's new surface lands here unreferenced by the caller's pinned invocation; the caller repoint and the finding-2 gating flip follow in the A2 PR once this change's merge SHA exists to pin. ## 1. The silent no-review is now machine-visible (`class=skipped-validation`) `claude-code-action`'s workflow-validation guard exits **0** with only a warning annotation, so the lane concluded green, emitted no `class=` token, and the incident aggregator — whose entire detection surface is that token — could not see it by construction. **Detection chosen** (the issue thread recorded no decision, so this was researched against the action's source at both the pre- and post-bump pins, v1.0.185 `9db594c…` and v1.0.187 `1623c36…`; the delta between them does not touch any load-bearing path): - The `WorkflowValidationSkipError` path in `src/entrypoints/run.ts` returns *before* Claude runs and never sets the `execution_file` output; the internal `skipped_due_to_workflow_validation_mismatch` output is not declared in the action's `action.yml` and is therefore invisible to callers. - The no-trigger early return cannot fire in this lane — not because of agent mode, but because `track_progress` (default `true` here) forces **tag** mode on `pull_request` events, and tag mode's `checkContainsTrigger` returns true immediately when a prompt is provided ("If prompt is provided, always trigger"); this lane's `prompt` input has a non-empty default. - The remaining pre-run exits (actor-permission gate, env validation) throw and redden the step. So within these lanes, **step outcome `success` + no execution file ⇔ the action skipped itself** — the least-invasive caller-side detector, requiring no new secrets, no new API reads, and no fork of upstream. - The `claude-lane-outcome` composite now splits the green path: success **with** an execution file → `review-ran=true`; success **without** evidence → a `class=skipped-validation` **warning** annotation (the aggregator reads warnings and errors alike), `review-ran=false`, `review-failed=false` (the check stays green by the advisory contract — the skip is expected on exactly the PRs that edit the caller workflow, which upstream's own skip message calls normal). - The aggregator's `RECOGNIZED_CLASSES` gains `skipped-validation` as a counted, **deliberately non-escalating** class: it self-corrects on merge, and the poll cannot see which files a PR touches, so it cannot distinguish the benign caller-edit shape from a real silent no-review on an unrelated PR. It now renders in any open incident's class table instead of being invisible. The synthetic canary deferred in #228 remains the only mechanism that could *prove* a review happened; this PR removes the "invisible by construction" part. ## 2. Finding 2 (false review count) — deferred to A2, deliberately The caller's count-upsert and clear-stale steps keep their `review-failed == 'false'` gates **in this PR**. `claude-review.yml` invokes the outcome composite at a pinned pre-merge SHA (`b5d54bf`), which does not produce `review-ran`; a gate reading it here would compare against an output that pinned composite never sets, evaluate false on every run, and silently skip the count upsert and stale-comment clearing on genuine reviews — defeating `max-reviews-per-pr` fleet-wide (empirically confirmed by the verifier on this PR's own self-review run, job 93087360214, where both steps skipped). The A2 follow-up repoints the caller's composite pin at this change's merge SHA and flips both gates in the same commit. ## 3. The marker copy no longer asserts what the caller owns "A new push does **not** re-trigger this lane" is false for any `synchronize`-wired caller (disproved directly on the sandbox). New copy states the condition: "A new push re-triggers this lane only if the caller's `pull_request` triggers include `synchronize` (the canonical caller omits it)." The step comment carrying the same overbroad claim is corrected alongside. ## Constraint compliance - The byte-pinned incident write-job region, its pin file, and the write-gate fixtures are untouched on this branch. - The equality-pinned step-4 counting-rule string in `claude-lane-incident.test.cjs`: not applicable — no `renderIssueBody` copy changed. - Files are disjoint from PR #385 (#364's lane-routing change); branch rebased onto current `origin/main` (includes the v1.0.187 action bump), not stacked. ## Deliberately unchanged — needs an operator decision The **security lane** fails closed only on `review-failed`, so a validation skip still turns that *required* check green with nothing reviewed — the same hole in a check that "certifies execution". Extending its fail-closed posture to the skip shape would block every PR that edits the security caller until merge, which is a policy call, not a drive-by change. Flagged with options and a recommendation in #363's thread; tracked with the fail-closed work. ## Tests - New `outcome-step.test.cjs` executes the composite's shipped script text against mock cores: evidence/no-evidence success shapes, the genuine-failure path, the failure-with-no-file path (stays `other`, never relabeled as a benign skip), and a cross-module contract check that the aggregator's own `extractSignals` recovers `skipped-validation` from the emitted annotation text. - Incident-module suite pins recognition + non-escalation of the new token, following the existing `runner` listed-ahead-of-emitter pattern. - Full suites after the rebase: `node --test .github/scripts/*.test.cjs` — 516 pass, 0 fail; `node --test .github/actions/claude-lane-outcome/*.test.cjs` — 14 pass, 0 fail. `actionlint` clean on `claude-review.yml`; comment-hygiene scan clean at CI's scope. ## Related No linked issue. For reference: #363 (findings 1 and 3 addressed here; finding 2 lands in the A2 repoint), #228 (the harm class), #238 (the aggregator contract), #349 (the runbook whose finding 3 corrected the same claim in prose). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01HCJfVqDNYt92YRyvKUMgYW Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…ion ledger (#390) Advances the Phase 4 tag `[DOING]` → `[DONE]` and appends a dated close-out amendment dispositioning every item in the 2026-08-06 REMAINING list — kept verbatim above it as the historical record. Dispositions recorded (each with its authority): - #364 shipped via #385 (`bcf48a0`) — routing label + idempotent escalation marker with repair path - #363 shipped via #387 (`7415d4e`, A1) + #389 (`9f9757e`, A2) — skipped-validation visibility, count/stale-clear gating, marker copy, wiring tripwire; security-lane variant tracked in #388 - Both #238 spec conflicts adjudicated (operator-delegated, 2026-08-07): supersede-not-reopen; three-clean-cycles close - Multi-repo shape waived; caller-side `class=runner` emission deferred with provisioning#215's trigger (operator-authorized recommendations) - #1327 comment-closed; #228/#238 closed with pointers — SC4 met - Canary deferral rehomed into the ledger (re-evaluate 2026-11-06 + three triggers), per its own the-moment-#228-closes rule - Secret-visibility hardening item retired: operator keeps `CLAUDE_CODE_OAUTH_TOKEN` at visibility "all"; matches the github-iac record (github-iac#269/github-iac#270 landed, github-iac#266 closed) - Fleet repin v0.9.1→v0.10.2 done (standards #337, claude-code-plugins #1990/#1992, sandbox #3) The deliberately-untouched verbatim run-30571900637 quote (no `coverage=` field) remains untouched; the byte-pinned regions are not in this diff. ## Related No linked issue. For reference: #238, #228, #363, #364, #385, #387, #389, #388. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01HCJfVqDNYt92YRyvKUMgYW --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Implements the lane-routing bullet of the #238 Contract, recorded as #364: the incident issue now wears the human-gated role label (
needs-human) and carries a machine escalation-marker comment (kind=routed-advisory), so it surfaces as[escalated]in the attended queue instead of opening correctly and waiting to be noticed.What changed
labels: claude-lane-incident, needs-human.create-issue-from-fileat the pinned SHA runsaddLabelsafter both its create and update branches (verified in the action's source,src/main.ts), so the label is re-asserted fail-closed for as long as the incident stays open — matching the org's fail-closed precedent (a missingneeds-humanon ci(claude-review): dead-credential canary + error-class surfacing — a revoked OAuth token was invisible for 19h #228 was restored, not respected). The workflow's prior comment claiming labels are applied "on creation only" was factually wrong and is corrected in the same region.action == 'open' || action == 'update', it lists the issue's comments (github.paginate) and posts only when no bot-authored comment already starts with the marker. First line is the org marker grammar:<!-- work-items:escalation lane=incident-aggregator kind=routed-advisory -->. Ordinary update cycles are reads that write nothing; the only update-path write is the repair of a marker whose original post failed (transient API error, timeout, cancelled run) — without it, a failed post after the upsert would leave the incident permanently parked (needs-human, never[escalated]), since later polls emitupdateforever. The same presence check covers the re-run duplicate variant. A singleconstfeeds both the presence check and the posted body, so check and write cannot drift. The body is fixed text; nothing derived from annotation or API strings is rendered.auth,runner), andESCALATING_CLASSESdefines both as dead-until-a-human-acts. Transient classes never open an incident.Byte-pin lockstep (the designed seam)
Both writes land inside the region
claude-lane-incident-write-gate.cjspins byte-for-byte, so each commit here is a full lockstep edit:.github/workflows/claude-lane-incident-aggregator.yml— write-job region (plus the out-of-region header prose: three steps → four).github/scripts/claude-lane-incident-write-job.pinned.yml— the pin, updated to the new region.github/scripts/fixtures/claude-lane-incident-write-gate/whose tails byte-match the pin, re-synced by script (matched on the previous pin's exact bytes, not judged by eye)write-job-extra-step,write-job-bare-hyphen-step,write-job-duplicate-step-name,write-job-script-edited) were re-derived onto the new pin with exactly their single named mutation re-applied, restoring the suite's one-property-differs invariant; the other five (no-write-scoped-job,workflow-not-a-mapping,write-gate-inverted,write-gate-removed,write-job-pin-missing) assert parse-level violations independent of pin staleness and are untouched.Decisions the thread had not settled
lane=incident-aggregatorin the marker — the attended queue discriminates on the marker prefix +kind, and thelane=token names the emitter (work-loop,babysit-loopfor the session lanes); the aggregator is a workflow emitter, so it names itself. The loop-lane convention's local escalation-record file binds only the session lanes, and the Contract says "No new lane wiring", so label + marker is the complete contract shape here.Tests
upsertid wiring, the marker grammar as the comment's first line (executed against a mock Octokit, not just grepped), the missing-number guard, and the idempotence contract: a marker the bot already posted is never posted twice (the test replays the script's own posted body as the existing comment), a missing marker is repaired onupdate, and a human-planted decoy marker does not suppress the real post.node --test .github/scripts/*.test.cjs— 522 pass, 0 fail;node --test .github/actions/claude-lane-outcome/*.test.cjs— 10 pass.actionlintclean on the workflow; comment-hygiene scan clean at CI's scope; zizmor reports only the pre-existing advisory finding on the untouched mint step.Related
No linked issue. For reference: #364 (this implements it), #238 (the Contract), #228 (the harm class), #361 (the acceptance-test incident demonstrating the gap).
🤖 Generated with Claude Code
https://claude.ai/code/session_01HCJfVqDNYt92YRyvKUMgYW