Make a halt record itself as a halt, not as a completion

[Antawari]

Four records reported a failure as a completion, or gave a reason that was not the reason that occurred. All four were reported second-hand; each was verified against the tree before it was touched. One did not reproduce as reported — the correction is below and is worth more than that fix.

1. Per defect — reproduced or not

Verified by running the real code before any edit (imports of the worktree source, no billed dispatch, nothing written to the tree).

1. Cost ledger — REPRODUCED, worse than reported. A PipelineFailed and a one-stage PipelineCompleted produced rows differing in timestamp only:

success: {'type':'pipeline','session_id':'s','total_cost_usd':1.5,'duration_seconds':10.0,'stages_completed':1}
failure: {'type':'pipeline','session_id':'s','total_cost_usd':1.5,'duration_seconds':10.0,'stages_completed':1}

PipelineFailed already carried failed_stage and error_message; _on_pipeline_failed discarded both. The operator's real ledger already holds such a row.

2. XP consumer — the reported defect does NOT reproduce. The synthesised PipelineCompleted is a local wrapper, never emitted on the bus, and success=False reaches the tracker. The store on disk was already honest: {'xp_total': 25, 'success': False, 'respawn': False}. A consumer can tell failure from success.

What is actually broken is the reason. From a failure carrying failed_stage="builder", error_message="builder raised RuntimeError: disk full", the emitted event was XPPenalty(reason='Pipeline failed with 1 stage failures') — the real cause dropped, a count invented.

I also reported earlier that stages_failed=1 was hardcoded so respawn could never fire. That was wrong and I did not "fix" it. Every PipelineFailed emit site in engine/pipeline.py halts on the first failing stage, so exactly one stage failed and 1 is the measured truth. Changing it would have substituted one fabricated number for another.

3. Onboard orchestrator — REPRODUCED. A crashed scanner and one that found nothing emitted byte-identical frames.

4. Onboard flow — REPRODUCED, all three causes. A genuinely oversize payload, a frame with no text field at all, and text: 12345 all returned message_too_long.

2. The ledger: what distinguishes a halt, and what happens to rows on disk

Field: outcome: Literal["completed","failed","unknown"], set explicitly to "completed" on the success path and "failed" on the halt path, alongside failed_stage / error_message copied from the event rather than inferred.

Default is "unknown", not "completed". Rows written before the field existed genuinely do not record how the run ended; defaulting them to success would fabricate exactly the history this defect corrupted.

Rows already on disk keep working. The sole reader is cost/analyzer.py, whose _PIPELINE_REQUIRED_FIELDS is (timestamp, session_id, total_cost_usd, duration_seconds, stages_completed) — the new field is not required, so legacy rows keep validating and keep aggregating. Two tests pin this: one asserts a pre-field row loads as "unknown", one asserts it still aggregates through CostAnalyzer.all_sessions().

How a genuine success still records as a success: the completion path sets "completed" explicitly, never by omission — and a negative control asserts outcome == "completed", failed_stage is None, error_message is None, with cost and stage count intact.

3. Control rods — both halves, per defect

Each fix was broken, confirmed red for its own reason, restored, confirmed green.

Rod 1a — halt path stops carrying outcome/stage/error:

AssertionError: assert None == 'builder'
AssertionError: assert 'unknown' == 'failed'

Rod 1b — origin/main exactly, both paths write the identical record:

AssertionError: halt row and success row differ only by timestamp — the ledger cannot tell a
crash from a completion: {...'stages_completed': 1, 'outcome': 'unknown', 'failed_stage': None,
'error_message': None} vs {...'stages_completed': 1, 'outcome': 'unknown', 'failed_stage': None,
'error_message': None}
assert set()

Restored: 5 passed.

Rod 2 — drop halt_reason, fall back to the invented count:

AssertionError: assert 'builder' in 'Pipeline failed with 1 stage failures'
AssertionError: assert 'Budget exceeded' in 'Pipeline failed with 1 stage failures'
AssertionError: assert 'sage' in 'Pipeline failed with 1 stage failures'

Restored: 6 passed.

Rod 3 — crash reported as clean again:

AssertionError: a dead scanner and an empty one emit identical frames — the browser cannot tell
'the scan died' from 'we found nothing'
assert {'type': 'scan_complete', 'panel': 'git_state', 'item_count': 0, 'failed': False, ...}
    != {'type': 'scan_complete', 'panel': 'git_state', 'item_count': 0, 'failed': False, ...}

AssertionError: assert 0 == 1
 +  where 0 = AllScansComplete(type='all_scans_complete', total_items=0, failed_panels=0).failed_panels

Restored: 4 passed.

Rod 4 — every validation error called message_too_long again:

AssertionError: missing text: reported as a length problem over a message that was never long —
the user is sent to fix something that is not broken
assert 'message_too_long' != 'message_too_long'

AssertionError: assert 'text' in 'Message too long; please keep under 8 KiB.'

Restored: 4 passed.

Every defect also carries a negative control asserting a genuine success still records as a success, so none of this is satisfiable by code that reports everything as broken.

4. Budget

Measured with the gate's own logic, after the code was written:

package	was	now	raise
tests/unit	73410 (measured 73409)	73777	+367
src/bonfire/onboard	4416 (measured 4416)	4460	+44

One line of headroom and none at all. Zero headroom in onboard was verified by a one-line probe producing package src/bonfire/onboard is 4417 lines. Both raises are the measurement, not an estimate and not padded; the test file was tightened first (shared sink helper, shared legacy-row constant, two parametrisations), taking it from 389 to 368. The raise is a separate commit with full reasons and rejected alternatives in package_raises.

This is the second consecutive lane to hit a ratchet sitting on its own measurement — the previous raise moved it 73199 → 73410 and landed it right back on the number. That is a design question about the ceiling, not something a lane can settle.

5. Verification

• pytest tests/ -x --tb=short (the exact CI invocation): 5766 passed, 3 skipped, 37 xfailed, 20 xpassed. Baseline before the change was 5673 on tests/unit tests/integration; after, 5693 — +20, the new tests.
• ruff check src/ tests/ → All checks passed. ruff format --check src/ tests/ → 375 files already formatted.
• cf-file-budget check → clean.
• Ledger safety: ~/.bonfire/cost/cost_ledger.jsonl mtime 1785248222 and 23 lines, identical before and after every full suite run. The conftest.py fixture holds and was not defeated.

6. Needs a follow-up from the docs lane

protocol.py shifted, so citations in docs/scan-front-door-protocol.md need updating. I did not edit docs/ — another lane owns it. The citation gate still exits 0, but 2 citations now land outside any symbol, so this is real drift the gate cannot fail on:

• old line ≤ 92 → unchanged
• old 93–97 → +6
• old 98–101 → +8
• old 102–104 → +12
• old ≥ 105 → +13

Concretely: protocol.py:100-104 → 108-116, 107-110 → 120-123, 113-118 → 126-131, 121-126 → 134-139, 147-157 → 160-170, 164-173 → 177-186, 175-177 → 188-190, 195-197 → 208-210, 200-202 → 213-215, and the 60-202 span → 60-215.

7. What I did NOT prove

• No browser run. ui.html was changed to render a failed panel distinctly; that is verified by reading, not by driving a browser. The Python side of the contract is tested; the rendering is not.
• No box run and no billed dispatch — none was needed.
• The _halt_reason bounce-target and __outer__ sentinel branches are tested against synthesised PipelineFailed events, not against a real engine bounce.
• AllScansComplete.failed_panels is slightly beyond the four defects as briefed. It is the same defect one level up — without it an all-crashed run still summarises as total_items=0 — and it is called out in the budget raise rather than slipped in.
• The three ledger rows that appeared mid-session came from another tree (they lack the outcome field this branch now writes), not from my runs.

---

8. Third commit: five drifted exemptions re-anchored

CI's gate / gate went red on five UNREGISTERED_SUPPRESSION findings. This branch added no suppressions. The complete diff of noqa lines across the whole change is one character-level edit:

-    except Exception:  # noqa: BLE001
+    except Exception as exc:  # noqa: BLE001

— binding exc so the scanner's error could be reported instead of erased, which is the point of the PR.

All five suppressions exist unchanged on 7bddd71 and are all already registered in exemptions.json:

flow.py:147          async def run_front_door(  # noqa: C901,PLR0915      -> entries [38][39] anchor "147"
flow.py:239          except (asyncio.CancelledError, Exception):  # noqa: BLE001,S110 -> [40][41] anchor "239"
orchestrator.py:106  except Exception:  # noqa: BLE001                     -> entry  [44] anchor "106"

_matches() resolves an entry by symbol_or_line == str(suppression.line) or symbol_or_line == suppression.symbol. These were anchored by line number. Inserting lines above them moved the code to 166 / 258 / 116 while the anchors stayed put, silently un-registering all five.

The gate's message is actively misleading: "a self-issued suppression is not an exemption" is exactly wrong here — it is a registered exemption with a named approver whose anchor drifted. A reader who trusts the message goes hunting for a suppression somebody sneaked in.

This is a defect in the registry's design, not in this branch. The sibling lane hit the identical failure in the same hour on engine/pipeline.py (entries [15] and [16], lines 154→158 and 589→599 — the top-level pipeline barrier and the per-stage boundary). Two lanes, two files, one root cause. It will keep firing on every future PR that inserts a line above a line-anchored suppression.

Fix: re-anchored by symbol, not by corrected line number — a corrected number drifts again on the very next insertion, which is precisely what just happened. 52 entries in, 52 entries out; frozen_count untouched at 52. Nothing added, nothing raised. Every reason and approver carried across verbatim. cf-exemptions: OK.

Rejected: the two fixes first prescribed to me. Splitting run_front_door is collateral scope on a function this branch never touched, carrying a v1.0.1 grandfather. Narrowing the flow.py shutdown drain contradicts its own exemption text — "the asyncio.CancelledError arm is load-bearing (deliberately swallows the cancel during drain)" — and orchestrator.py's boundary is registered precisely because scanners are a pluggable open set that must not abort onboarding. Also rejected: an in-file line-number fix, which would mean deleting the docstring explaining the fix to hold orchestrator.py:106, and padding flow.py with five blank lines because the run_front_door anchor breaks upward too (net −5 there).

9. Local did not equal CI, and that is the bigger lesson

I verified against ci.yml — 4 checks (ruff, ruff-format, the citation script, pytest) — plus cf-file-budget, and reported green. The failing check was gate / gate, from quality.yml, which calls the shared kit workflow and runs 11 gates. I ran 4 of 11 and assumed I had covered the gate. I verified against the workflow I found instead of the one that was failing.

cf-gate now runs locally. Same command, same venv, both trees:

	bare origin/main	this branch
cf-exemptions	PASS	PASS (was FAIL before the re-anchor)
cf-import-contract, mypy, complexipy, pytest	FAIL	FAIL

Those four are red on bare main too in this local environment — pre-existing or environmental, not introduced here (pytest exits 2 = a usage/collection error under cf-gate's invocation, while pytest tests/ -x passes 5766). After the re-anchor this branch's gate profile is identical to main's. Whether those four were also red in CI was left unproven when this was written; CI has since answered it. gate / gate runs the same 11-gate kit and now passes on this branch, so cf-import-contract, mypy, complexipy and pytest are green in CI and red only in my local environment — local-environment artifacts, confirmed, not repo state. The one gate that genuinely differed was cf-exemptions, and it is fixed.

8b. Fourth commit: the re-anchor's encoding, redone surgically

The re-anchor above was semantically right but written the wrong way: the edit went through a json.dumps round-trip, which escaped every em-dash in the file to — and turned a 5-row change into 71 changed lines, 66 of them pure encoding noise.

Two concrete costs, not aesthetics:

1. It guaranteed a textual conflict with the sibling lane, which is re-anchoring two rows in this same file (engine/pipeline.py, entries [15][16]) with a surgical edit. A 71-line rewrite against a 2-line edit conflicts in a file where both sets of re-anchors must survive. Redone surgically, the two changes touch different rows and merge cleanly in either order.
2. A 5-row change shown as 71 changed lines is not reviewable — you have to parse the file to see what actually moved, which defeats the point of a fix whose whole story is "a registry lost its anchors."

The fourth commit restores exemptions.json to origin/main byte-for-byte and reapplies only the five symbol_or_line values as an in-place text edit. Verified:

git diff --numstat origin/main -- exemptions.json   ->  5  5  exemptions.json
grep -c '—' exemptions.json                    ->  0
entries: 52   frozen_count: 52
cf-exemptions: OK
non-symbol_or_line lines changed                    ->  0

History is append-only (four commits, no force-push) because force-push is denied by policy in this environment; the PR diff against origin/main — what GitHub shows and what merges — is the clean +5/-5.

Rule worth keeping past this PR: never round-trip a hand-maintained JSON file through a serializer to change one field. It reformats escapes, key order, indentation and whitespace you did not intend, and buries the real change. Edit the text. Credit to the sibling lane, which hit this trap first and flagged it.

---

10. Rebased onto bb2f7c6 (#253 + #255 landed)

origin/main moved from 7bddd71 to bb2f7c6 while this was open. Merged in (merge commit, not a rebase — force-push is denied by policy here). exemptions.json auto-merged; file-budget.json conflicted and was resolved keeping both sides.

Resolved packages — every entry is one lane's number, none overwritten:

dispatch 2869 · engine 1981 (#255) · handlers 3250
onboard 4460 (mine) · tests/integration 1702 (#255) · tests/unit 73777 (mine)

package_raises: 8 entries — the 3 predating this round, #255's 3 (engine/pipeline.py 989→994, engine 1903→1981, tests/integration 1363→1702), and my 2 appended at the end. None dropped.

Re-measured against the merged tree rather than re-typed. Every package now lands exactly on its ceiling, and my two figures were unchanged by the merge:

package	measured	ceiling	delta
src/bonfire/dispatch	2869	2869	0
src/bonfire/engine	1981	1981	0
src/bonfire/handlers	3250	3250	0
src/bonfire/onboard	4460	4460	0
tests/integration	1702	1702	0
tests/unit	73777	73777	0

My from: values (73410, 4416) still match origin/main's ceilings exactly, so neither entry is a stale anchor — the thing I diagnosed this morning, checked against my own work.

exemptions.json after the merge: 52 entries / frozen_count 52, unchanged. #255's two symbol re-anchors survived alongside my five — [15] PipelineEngine.run, [16] PipelineEngine._execute_stage, [38-41] run_front_door, [44] _run_one. Seven symbol-anchored rows where there were none this morning, in two independent lanes.

Conflict resolution was done as a text operation on origin/main's bytes, not a serializer round-trip — so #255's and #253's entries are byte-identical to what they merged, and the diff against main is 18/2 on file-budget.json (2 ceiling values + my 2 appended entries) and 5/5 on exemptions.json. Escape count unchanged on both files: file-budget.json carries the same 2 pre-existing — lines main has, and I added none.

Verification against the new base: full suite 5774 passed, 3 skipped, 37 xfailed, 20 xpassed (up from 5766 — #255's 8 new checkpoint tests). cf-gate: cf-exemptions and cf-file-budget both PASS. CI green on all four checks against bb2f7c6.