Build the hook fail-open promotion-and-surfacing relay (#391)#431
Merged
Conversation
A crashed hook or a missing tool-runtime used to leave only a transient stderr line nothing consumed — no tracked finding opened, boot never surfaced it, and the one message an operator might glimpse falsely claimed "this was recorded as a problem to fix" when nothing recorded anything. This makes the blind safety net honest and operator-visible. - hooks.py: each fail-open branch now promotes the finding to a tracked engine Issue via telemetry.promote_finding (the close.py "log it" precedent) — best-effort, with telemetry/boot imported LAZILY off every hook's hot path, TRUST_CRITICAL, source-keyed hooks/fail-open/<event>/<kind> so recurrences collapse to one Issue. FAIL-SAFE: any error in the promote path is swallowed, so recording the crash can never re-break the fail-open path into a block (the #390-class fail-closed stranding). A test-harness backstop refuses to touch live GitHub from any test run. - The "recorded" copy is now STRICTLY conditional on the promotion landing: promoted -> "recorded as a tracked item you'll see at your next start"; offline -> "not durably recorded until the engine next reaches GitHub". The old false copy is gone. - hook-runner.sh: when the venv interpreter never appears, name the absent runtime in plain language on stderr (non-blocking) instead of exiting silently. - build-orchestration.md: the PR Validation section surfaces any open fail-open finding as a named line at submit (non-blocking; informs consent, never a new gate). Boot orientation already carries any promoted finding via open_findings — no new boot code. The live triage LOOP (production run() driver, ambient capture, refused-cursor and broken-runtime durable routing, auto-close) stays telemetry's — #403/#412. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…oted one Deliverable-gate finding (usability): the operator-runnable demo showed only the promoted 'recorded' copy, leaving the load-bearing offline honesty case — the state the old code lied about — visible only in tests the operator cannot read. Add a demo case that injects an offline promoter and renders the honest 'not durably recorded' line, and make the demo self-check falsify on its absence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #391
Purpose
Make the local safety net honest: when a hook crashes or the tool-runtime is missing, tell the operator instead of going blind and lying.
.engine/.venv) leaves only a transient stderr line nothing consumes — no tracked finding opens, boot never surfaces it, and the one message an operator might glimpse falsely says "this was recorded as a problem to fix" when nothing recorded anything.Impact: a safety gate that couldn't run now becomes a durable, tracked, operator-visible finding — and the copy never claims a record that doesn't exist.
Scope
Wire the emit → promote → surface relay, name the absent runtime, and correct the false copy.
hooks.py: each fail-open branch promotes the finding to a tracked engine Issue via the existingtelemetry.promote_finding(theclose.pyprecedent) — best-effort,telemetry/bootimported lazily off every hook's hot path,TRUST_CRITICAL, source-keyedhooks/fail-open/<event>/<kind>so recurrences collapse to one Issue.hook-runner.sh: when the interpreter never appears, name the absent runtime in plain language on stderr (non-blocking) instead of exiting silently.build-orchestration.md: the PR Validation section surfaces any open fail-open finding as a named line at submit.Impact: all three operator touchpoints — boot orientation, the merge-time Validation line, and the in-session copy — now actually reach the operator.
Out of scope
The general telemetry triage loop — deliberately left to its lane-siblings.
run()/reconcile()driver, the ambient-capture writer, de-dup-at-scale, auto-close, and the refused-cursor / broken-runtime durable routing are [conformance] Wire telemetry end-to-end: consume the three signals-of-record, drive the triage loop live, and render honestly #403 / [conformance] Wire the telemetry self-healing loop into production so fail-open and degraded conditions become durable, auto-closing tracked findings #412. This PR wires the immediate promotion of these fail-open findings, not the loop.boot.open_findingsalready surfaces any promoted engine finding regardless of blocking status.Impact: #391 is the root emit-side relay; #412 will need light re-scoping once this lands (it claimed some of the same relay).
Risk
It edits two protected hook-wiring files, so it needs your deliberate
guardrail-ack. The new write is a hard fail-safe addition..engine/tools/hooks.pyand.engine/tools/hook-runner.share guarded →engine-guardgoes RED and holds the merge until you applyguardrail-ack. This strengthens the fail-open safety net; the ack is still yours. I have not applied it.boot.py/telemetry.pyare only read (lazy import), not edited, so they add no ack.gh auth tokenresolves even locally). The promoter is injectable (tests/demo use a fake), and the real one hard-refuses live GitHub under a test harness. (An early build-verification probe that bypassed this backstop did open one spurious Issue, Engine health: msg #430, since closed.)Impact: the merge stays your conscious decision; a crashed gate can never brick the session.
Validation
Full suite green; the fail-safe, honest-copy, and readout behaviors are all pinned.
Ran 2987 tests … OK— including new tests for the promoted vs offline copy, the fail-safety of a throwing promoter, the coarse marker-safesource_id, the real wiring against a mocked GitHub, the test-harness backstop, and the missing-runtime readout through the realhook-runner.sh.validate.py --suite CI: green exceptdisposition-issue-resolution(environmental — needs aGITHUB_TOKENCI has; unrelated to hooks). Knowledge graph regenerated.hooks.py demoself-check was extended to fail unless the crash finding is promoted with the honest copy and the missing-runtime case names its runtime.Impact: an approver can rely on the automated floor being green in CI; correctness rests on Review + your merge.
Review
Thorough depth: four cold plan lenses reshaped the design before building; four deliverable lenses after. The plan gate caught a real spec violation and half-a-double-build.
promote_finding); (2) the consumer/driver/refused-cursor routing is [conformance] Wire telemetry end-to-end: consume the three signals-of-record, drive the triage loop live, and render honestly #403/[conformance] Wire the telemetry self-healing loop into production so fail-open and degraded conditions become durable, auto-closing tracked findings #412's and would double-build; (3) the boot carry is already free; (4) the PR-Validation line has no mechanical renderer — it's a submit-time authoring duty. You approved the tightened scope (emit-side relay + copy + readout + a submit-time guidance line) and the deferrals.BaseException, telemetry raising) and confirmed the gate can never go fail-closed; content-injection closed by construction; dedup prevents spam; the test backstop holds and Engine health: msg #430 is closed with no other spurious Issues. Technical-integrity: healthy — hot path imports nothing new, no unbounded hang (bounded timeouts), fail-safety double-wrapped, tests are real falsifications. Usability: the honesty fix lands in all states, but the serious finding was that the operator-runnable demo showed only the promoted copy — fixed by adding a demo case that renders the honest offline "not durably recorded" line (validated; not re-reviewed).close.py's immediate-promote);boot.read_state's "owes → telemetry writer" comment is left as-is — the refused-cursor emission it names is genuinely still [conformance] Wire the telemetry self-healing loop into production so fail-open and degraded conditions become durable, auto-closing tracked findings #412's; the block-misuse/decide-misuse copy sharing the crash voice is spec-consistent; the readout's raw.venvpath is acceptable (plain language carries the meaning); the localdisposition-issue-resolutionFAIL is environmental (pre-existing onorigin/main, needs a token CI has).spec_referent.py review-stepsdoes not apply — [conformance] Build the hook fail-open promotion-and-surfacing relay #391 is an engine conformance fix with no settled productdocs/specdescription.uv run --directory .engine -- python tools/hooks.py demo. It drives the real harness: a crashing gate proceeds (non-blocking) and its finding is promoted (against a faked GitHub) with the honest "recorded" copy; a block on a non-eligible event does the same; and the launcher, on an absent interpreter, names the runtime on stderr instead of exiting silently. To see the un-recorded copy, note the offline path says "not durably recorded until the engine next reaches GitHub."Impact: the engine's own account of the review — your merge is the binding gate.
Files of interest
One harness, one launcher, one runbook line.
.engine/tools/hooks.py—_promote_fail_open/_do_promote_fail_open(the relay + backstop),_emit_finding(honest conditional copy), the four fail-open sites, and the demo..engine/tools/hook-runner.sh— the missing-runtime readout..engine/operations/build-orchestration.md— the submit-time Validation-line guidance..engine/tools/test_hooks.py— the promotion/copy/fail-safety/readout regression tests.Impact: the promotion seam and the honest copy are where the operator's trust is won or lost.
Claude involvement
A design the plan gate materially corrected; the wiring follows an existing precedent.
close.py's establishedpromote_findingseam; the fail-safety and test-safety backstops are the new safety-critical logic.Impact: AI judgment is load-bearing on the spec-conformant scope; the relay is a precedent-following wire, test-pinned.