Skip to content

P0: main CI red — Deploy MCP failure on b7a9363 #3123

Description

@github-actions

P0: main went red

Workflow: Deploy MCP concluded failure on main.

Failing run: https://github.com/edobry/minsky/actions/runs/32204510289
HEAD SHA: b7a9363168564cc0664c233e61bb0d43b1d1aef5
Head commit: fix(mt#3864): Measure why pre-narration's window misses, then fix what that supports

Summary

mt#3864's dominant-class remedy rested on a premise the source contradicts. The spec's 2026-08-13
amendment states the detector "requires the evidencing tool call in the SAME turn as the claim"
and derives its tune direction — widen the evidence window — from that. It does not.
TRAILING_WINDOW_TURNS = 12 and extractWindowToolUseNames have implemented cross-turn suppression
since mt#2671, and the constant's own docblock names this exact sequence as what it was sized for:

Large enough to cover a full convergence sequence (wait-for-review → fix → push → back-reference)

So the proposed fix would have widened something already wide. This PR replaces the hypothesis with
a measurement, then fixes only what the measurement supports.

The measurement

scripts/diagnose-pre-narration-window.ts (new) replays every injected calibration record against
its own session transcript, walking real-user-prompt boundaries exactly as the detector does, and
reports the boundary distance to the nearest evidence.

Zero beyond-window. Widening changes nothing. All 17 non-tool-backed matches are
tool-absent — the required tool appears nowhere before the fire — which splits into two causes
with opposite fixes.

Cause A — evidence via a READ rather than the ACT — FIXED, with identity correlation

The agent verified the outcome by reading PR state; the category listed only the tool that
performs it. Both measured cases had read tools in window and no merge tool, because another
actor did the merge.

R1 (BLOCKING) was correct, and the data proves it. The first version accepted any PR read by
NAME, with no correlation to the PR claimed — so a read of PR #100 would have silenced a false
"PR #200 merged", and reads are common enough that this would be the usual case rather than a
corner. Fixed by splitting the evidence sets:

  • requiredTools keeps name-match semantics for tools that PERFORM an outcome — rare, deliberate,
    and pre-existing behavior this PR does not change.
  • identityScopedTools (new) holds the read-shaped tools. They suppress only when the PR number
    the claim NAMES appears in that tool call's own input. A claim naming no PR number can never be
    identity-backed and deliberately falls through to firing — for a suppressor the safe degrade is
    MORE fires, never fewer (ADR-024's fail-to-Rung-1 invariant).

Applying correlation dropped the suppression from 2 to 1, which is the finding reproduced in the
recorded corpus:

"PR #3064 merged"  -> identity-backed. That agent did read #3064.
"PR #3033 merged"  -> STILL FIRES, despite pull_request_read AND session_pr_get
                      being in window — it read a DIFFERENT PR.

One of the two cases the name-match version "fixed" was a coincidence. The diagnostic gained the
same correlation, or its before/after would have misreported the fix it exists to measure.

List-shaped tools (list_pull_requests, session_pr_list) remain excluded: a listing establishes
no particular PR's state.

Cause B — the evidence is not in this transcript at all (13 of 17) — NOT tuned, on purpose

No PR-related tool appears in the window in any form; one session carries zero PR-evidence
calls across its entire 1,496-line transcript while reporting APPROVED. review-approved's
premise — that a verdict is always learned through one of five named tools — is false in a way no
phrase-list or window edit repairs. Widening until it stops firing is ADR-024 §Context's arms race
and would silence its true positives. Per mem#1067 §4 and ADR-032, a structural residual goes to
the calibration review as the input to flip / restrict / retire. Recorded in the spec under
§MEASURED CAUSE.

Class 6 (quotation) — FIXED

elideDoubleQuotedSpans composed onto the local elideMarkdownContexts, which covers
fences/code-spans/blockquotes but not double-quoted prose. Deviation from the spec, recorded:
it says apply elideQuotedAndCodeContexts; I composed instead of swapping because elision.ts's
own header scopes that consolidation to mt#2263 / the ADR-024 ladder. The 44 pre-existing tests
confirm no markdown behavior changed.

Spec verification — this PR is PARTIAL, and here is the boundary

Criterion Status
SC4 (quoting a stored artifact) Met — class-6 elision + differential test
SC5 (real classes still fire, by negative control) Met — six negative controls, below
SC6 (restated by the planning pass to a pre-merge replay measurement) Met — before/after below
SC7 (review-approved/merged past-tense reports) Partially metmerged fixed with identity correlation; review-approved is Cause B, deliberately untuned
SC8 (evidence never a tool call → subject-attribution, not a window change) Met as analysis — the two causes are separated and labelled rather than conflated

[sc1-deferred: mt#4256] [sc2-deferred: mt#4256] [sc3-deferred: mt#4256] [at1-deferred: mt#4256]

SC1 (domain literal), SC2 (subject not the agent) and SC3 (past-dated historical) are not
implemented here
and are tracked in mt#4256. They were authored against the 2026-08-08 window's
six classes; the fresh 2026-08-13→18 measurement shows the current injected population dominated by
Cause B instead. Implementing them now would narrow a matcher against a window that is no longer the
population the guard faces — the failure mem#1067 §1 records, where the same code measured 69.2%
against recent specs and 32.5% against the oldest.

R1 NON-BLOCKING (test set does not pin all six FP contexts) is the same boundary. Five of the
six belong to SC1–SC3; pinning them as must-not-fire would fail, because those classes are not
implemented. AT1 is therefore met for class 6 only and carries an at1-deferred marker rather than
prose — mt#4256 owns the rest.

Testing

All fixtures use verbatim recorded context from the calibration log, not invented shapes.

Execution evidence:

$ bun test --preload ./tests/setup.ts --timeout=15000 ./.minsky/hooks/pre-narration-detector.test.ts
 55 pass
 0 fail
 100 expect() calls
Ran 55 tests across 1 file. [167.00ms]

44 pre-existed and still pass. Eleven are new (AT1 class 6, AT4 fresh-window cases, R1 correlation).

AT3 — replay the corpus, before/after with the delta accounted class by class:

BEFORE (pre-fix):
   12  review-approved — tool-absent
    4  merged          — tool-absent
    3  build-test      — within-window
    1  pr-created      — tool-absent

AFTER (post-fix, with R1 correlation):
   12  review-approved — tool-absent
    3  build-test      — within-window
    3  merged          — tool-absent
    1  merged          — identity-backed
    1  pr-created      — tool-absent

Delta accounted: exactly one merged case flipped, the one whose claimed PR was actually read.
Nothing else moved. Note the intermediate name-match version flipped TWO — the extra one is the
false suppression R1 predicted, so the correlation requirement is visible in the numbers.

Negative controls — the class-6 elision could have passed vacuously against a matcher that stopped
detecting APPROVED at all, and the Cause-A widening could have disabled the category:

✓ APPROVED inside double quotes does not fire
✓ NEGATIVE CONTROL: the same phrase unquoted still fires
✓ BLOCKING R1: a read of a DIFFERENT PR does not suppress
✓ a claim naming NO PR number is never identity-backed
✓ NEGATIVE CONTROL: with no PR tool at all, the merge claim still fires
✓ a LIST-shaped tool is deliberately NOT evidence

Typecheck: 0 errors across 8 projects. Lint: 0 errors, 0 warnings, 3759 files.

Live verification

Not a deployed surface — a local UserPromptSubmit hook. The equivalent live exercise IS the replay
above: the diagnostic runs against real production calibration records and real session transcripts,
not fixtures. The generated .claude/hooks/pre-narration-detector.ts was regenerated by pre-commit
and is included.

Deploy verification: no deploy-surface file is touched (infra/**, services/*/Dockerfile,
services/*/railway.json, services/*/deploy.config.ts, services/*/railway.config.ts,
.github/workflows/deploy-*.yml all unchanged), so no post-merge deploy check applies.

Review rounds R2/R3 — the two non-blocking findings

R2 NON-BLOCKING — the .claude/hooks/ copy duplicates the detector logic. It is not
duplication to centralize: .claude/hooks/* is a BUILD ARTIFACT. .minsky/hooks/ is the single
source, and pre-commit regenerates the .claude mirror and re-stages it whenever a hooks source is
staged (CLAUDE.md §Hook Files; the generated-file-edit guard denies hand-edits to the mirror).
Both copies moving in one commit is the mechanism working, not a second maintained implementation —
and the R3 commit demonstrates it: only .minsky/hooks/pre-narration-detector.ts was edited by
hand, and .claude/hooks/pre-narration-detector.ts appears in the same commit because pre-commit
regenerated it.

R3 NON-BLOCKING — identity-scoped suppression reused the name-based reason strings. FIXED
(673b82b).
The finding is right, and against the file's own stated contract rather than a
preference: the constants' docblock says a calibration reviewer must be able to tell the suppression
sources apart from the record alone. An identity-backed suppression recorded same-turn-tool-call
or window-tool-call, both of which name a requiredTools call — and the identity path is reached
only when NO such call was in window, so the record named a call that never happened.

  • SUPPRESSION_IDENTITY_SCOPED_TOOL_CALL ("identity-scoped-tool-call"), selected by strength of
    evidence: same-turn > window > identity-scoped. The existing two keep their exact prior meaning —
    only the case previously mislabelled window-tool-call changes.
  • calibration-sweep.ts's conforming-producer table lists the third string; that table is the
    shared contract isSuppressedRecord consumers read, so a new reason that is not in it is a
    documentation gap by construction.
  • Two tests: the identity-backed reason, plus a negative control that a same-turn merge call with
    identity evidence present still records the stronger source (the ordering could otherwise regress
    silently in either direction).

Execution evidence for the R3 commit:

$ bun test --preload ./tests/setup.ts --timeout=15000 ./.minsky/hooks/pre-narration-detector.test.ts
 60 pass
 0 fail
 108 expect() calls

$ bun test --preload ./tests/setup.ts --timeout=15000 ./.minsky/hooks
 5522 pass
 0 fail
Ran 5522 tests across 154 files.

Typecheck: 0 errors across 8 projects. Lint: 0 errors, 0 warnings, 3759 files.

R4 — the identity key list (non-blocking) — ANSWERED BY MEASUREMENT (6f75421)

R4 asked whether every identity-scoped tool is safe to read a generic number key. Audited, then
measured.

Audit. Only two tools can reach this list. pull_request_read keys its PR number pullNumber;
session_pr_get resolves by task/sessionId and carries no PR number at all. Neither has ever
had a number or pr key — both were forward-compat catches.

Measurement. Replaying the 2026-08-13→18 corpus with and without the two generic keys produces
identical tallies — 1 identity-backed either way. They contributed no evidence, only the hazard.

$ bun scripts/diagnose-pre-narration-window.ts --since 2026-08-13 --log <main>/.minsky/pre-narration-calibration.jsonl
   12  review-approved — tool-absent      12  review-approved — tool-absent
    3  build-test      — within-window     3  build-test      — within-window
    3  merged          — tool-absent       3  merged          — tool-absent
    2  pr-created      — tool-absent       2  pr-created      — tool-absent
    1  merged          — identity-backed   1  merged          — identity-backed
    1  review-approved — within-window     1  review-approved — within-window
         (with "number","pr")                     (without)

So the keys are dropped rather than justified. A generic key is the wrong risk to carry here: a
spurious match MANUFACTURES identity evidence, which suppresses a fire — ADR-024's unsafe degrade,
and a silent one, since a suppressed claim is one the operator never sees. The inverse failure (a
future tool whose key is missing from the list) is a FIRE, which is the safe direction, and the
constant now says so. Two tests pin it, including a negative control that the same call keyed
pullNumber still is evidence — so the first measures the key, not a broken helper.

$ bun test --preload ./tests/setup.ts --timeout=15000 ./.minsky/hooks/pre-narration-detector.test.ts
 62 pass
 0 fail
 110 expect() calls

Typecheck: 0 errors across 8 projects. Lint: 0 errors, 0 warnings, 3759 files.

Co-Authored-By: minsky-ai[bot] <minsky-ai[bot]@users.noreply.github.com>

What this means

A push to main triggered CI and the workflow above did not conclude success. Per
CLAUDE.md user preference ("main must never be broken"), this is severity-1.

Diagnostic checklist

  1. Open the failing run URL above; identify which job/step failed.
  2. Check whether the offending PR was merged with a known-failing required check
    (operator-API bypass via gh api PUT /merge despite enforce_admins).
  3. Confirm enforce_admins is currently enabled:
    gh api repos/edobry/minsky/branches/main/protection --jq .enforce_admins.enabled
    
    Expected: true post-mt#1938. If false, that is itself a separate finding.

Recovery

  1. Open a hotfix branch off current main.
  2. Apply the smallest fix that turns CI green (often a formatter pass or a config
    flip).
  3. Land via the standard Minsky session flow:
    tasks_create → session_start → session_commit → session_pr_create → /review-pr → session_pr_merge.
  4. Verify the post-merge main build is green within ~5 minutes.
  5. Close this issue with a link to the hotfix PR.

Cross-references

Metadata

Metadata

Assignees

No one assigned

    Labels

    main-redAuto-filed by .github/workflows/main-watch.yml when main CI fails (mt#1938)p0Severity 0: production breakage requiring immediate attention

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions