Surface explicit-none decision attestations as confirmable HITL decisions (#3462)

[jwbron]

Closes #3462.

What

Closes the self-attestation loophole from #3462: a refine/plan agent could bypass the entire register → bridge → resolve chain (#3374/#3392/#3071) by attesting no_decisions_rationale (#3390's explicit-empty-ledger form) and documenting the decisions as draft prose. The attestation rode along as a sentence embedded in the phase_gate question — the operator never confirmed it as a decision, and the resolutions collected out-of-band never became first-class contract decisions (motivating run: pipeline-dcdad92d; contrast pipeline-121df67a, where the registered path worked as designed).

Layers

Gate side — the attestation becomes its own confirmable decision (issue ask 4). When a refine/plan phase reaches its gate with an explicit-none attestation standing in for a ledger, the orchestrator queues a dedicated choice decision quoting the role and rationale ("the attests this phase deliberately raises no operator decisions — confirm?") before the phase gate:

• Confirm — only the bare keyword or the full option label counts (mirroring the phase_gate's "bare approve advances" posture; the {"action":"select"} CLI envelope is unwrapped). Proceeds to the normal gate; the gate's ledger note records "Operator confirmed the attestation".
• Anything else — the re-run option or free text (which rides along as an operator note) kicks the phase back via the standard converge-loop re-run, with a directive to register each decision, including believed-resolved ones, with the recommended answer as the first option.
• Idempotent across converge rounds: a re-entered gate with the same attestation reuses the operator's prior confirmation (or the pending decision) instead of re-asking; a changed rationale is a new claim. Non-RESOLVED terminal states fail open to the phase gate, which still blocks — same posture as the Guarantee HITL decision registration: hard-fail on missing decision ledger + reviewer obligation against un-surfaced decisions #3390 missing-ledger backstop this branch mirrors.
• Autonomous pipelines (hitl_gates: false) emit a phase.decision_ledger_explicit_none event instead of blocking, mirroring the HITL phase gate: converge-before-advance loop (resolve-all → re-run → converge) + durable resolved-question carry-forward #3392 gate-skip posture.

Prompt side — registration required for task-named decisions, with a recommended disposition (asks 1–3). The refine Open Questions meta-block gains a "Task-named decisions are non-optional" rule: decisions the task names as operator-owned (or covered by a surface-as-HITL directive) must be registered even when believed resolved, non-blocking, or deferred — belief about resolution is a recommended disposition (first option suffixed (recommended), citing the resolving context, one-click confirmable), never a reason to skip. The #2481 "skip already-resolved" rule is explicitly narrowed to answers from this pipeline's pre-refine HITL round — it never covered prior/cancelled-run seeded context, which is exactly the over-read in the motivating run. The DO-NOT list and the #3390 attest prose (template block + BRC preamble propose line) now state that the rationale form is operator-confirmable and never a substitute for registering a believed-resolved decision.

Reviewer side (judgment layer). reviewer_refine §7 and reviewer_plan §14 gain a "Task-named decisions — NACK an explicit-none ledger" obligation: on a task that names decisions to surface, a no_decisions_rationale attestation is a NACK regardless of how defensible the rationale reads.

No new Decision schema field: the recommended disposition rides the existing options list (recommended option first, resolving context in its description), which the bridge and status surfaces already render.

Testing

• test_decision_ledger_gate.py: 3-tuple return of _collect_decision_ledger_status (now carries the (role, rationale) for the gate), plus a new TestLedgerAttestationConfirmation suite covering the question composer's stability (the converge-round dedupe key) and the conservative confirm matcher (bare keyword / full label / CLI select-envelope confirm; re-run option, negating free text, and envelope-rerun reject).
• test_pipeline_prompts.py: new TestTaskNamedDecisionRegistrationSurface ratchet (7 tests) over the producer prompt, the narrowed skip rule, the DO-NOT items, the BRC preamble for all four attesting roles, and both reviewer criteria.
• make test (changeset-aware): 20632 passed; 3 failures are pre-existing/unrelated — the two documented host-env reap-script failures (test-all: reap-stale-egg-images safety-gate tests fail on btrfs-root hosts (non-hermetic test, 127) #3222) and a gateway session-expiry timing flake that passes in isolation.
• ruff + ruff format clean on touched files (pre-commit hooks passed).

Authored-by: egg-adjacent human-in-the-loop session

[egg-reviewer]

Review — thorough pass

I traced the full change: the four prompt/reviewer text edits, the _collect_decision_ledger_status 3-tuple change and both call sites, the new _ledger_attestation_question / _unwrap_choice_resolution / _ledger_attestation_confirmed helpers, and the new elif _ledger_explicit_none is not None: orchestration block in _health_monitor_poll. No blocking issues. The change is a careful, well-scoped extension of the #3390 missing-ledger backstop and is correctly wired end-to-end.

What I verified

• Reachability / not a silent no-op. The new elif sits in the live refine/plan human-gate path in _health_monitor_poll, directly beside the accepted #3390 if _ledger_missing: backstop, guarded by current_phase.value in _HITL_GATE_PHASES. Before this PR the explicit-none case returned (note, False) and fell straight through to the phase_gate with the note appended as prose — exactly the loophole #3462 describes. Now _collect_decision_ledger_status returns the (role, rationale) and the caller surfaces a dedicated confirmable choice decision first. The behavior change is on the normal path, and after confirm _ledger_note (with " Operator confirmed the attestation.") still flows into the phase_gate question (orchestrator/routes/pipelines.py:28527). Feature works end-to-end.
• Python syntax. except ValueError, TypeError: (pipelines.py:24851) is valid — PEP 758 landed in 3.14, pyproject.toml pins requires-python = ">=3.14", and pipelines.py:685 already uses the same unparenthesized form. ast.parse of the whole module succeeds under 3.14. Not a bug.
• All callers updated. Only two non-test callers of _collect_decision_ledger_status (both in pipelines.py), both unpack the 3-tuple. No stray 2-tuple unpack that would ValueError at runtime.
• Confirm matcher. _ledger_attestation_confirmed unwraps the {"action":"select",...} envelope, is conservative (bare confirm / full label only), and correctly rejects the re-run option, negating free text, and empty. Idempotency (_prior_confirm RESOLVED+confirmed, _pending_attest PENDING reuse) is keyed on the stable question string and filtered by phase — a rejected prior attestation is RESOLVED-but-not-confirmed so it correctly does not satisfy _prior_confirm, and re-asks. hitl_review_cycles + _broadcast_hitl_nonconvergence_alert bound the reject→re-attest loop.
• Autonomous path. Emits phase.decision_ledger_explicit_none (free-string event type, same as #3390's phase.decision_ledger_missing) and never blocks — matches the #3392 gate-skip posture. if/elif correctly nested inside the try, so a helper raise can't leave _ledger_explicit_none unbound.

Non-blocking suggestions

1. No test exercises the orchestration block itself. TestLedgerAttestationConfirmation covers the pure helpers well, but the net-new integration logic — confirm→proceed, reject→_perform_hitl_phase_rerun with the operator-note directive, the _prior_confirm/_pending_attest idempotent reuse, and the AWAITING_HUMAN→RUNNING transitions — has no direct test. It mirrors the #3390 backstop, but the reuse/dedup and rerun-directive branches are new. Consider a monkeypatched wait_for_decision test (as elsewhere in the suite) asserting: confirm → falls through to phase_gate with the "Operator confirmed" note; reject → rerun fires with the directive; re-entry with same rationale → no duplicate decision queued.

2. Audit-note inaccuracy on cancel. On a non-RESOLVED terminal state, _confirmed is True (fail-open) and _ledger_note += " Operator confirmed the attestation." — recording "confirmed" when the operator actually cancelled. Cosmetic only (the phase_gate still blocks), and it mirrors #3390's analogous fail-open, but the note text is misleading in that edge case.

3. Idempotency corner (awareness, not a defect). After a confirm, if the phase later re-runs from phase_gate feedback and the producer re-attests the identical rationale, _prior_confirm reuses the earlier confirmation without re-asking — even if the feedback's scope change ought to have surfaced a new decision. This is consistent with the documented "same rationale reuses confirmation / changed rationale is a new claim" design, and the reviewer NACK criteria + the phase_gate itself backstop it. Flagging only so the tradeoff is a conscious one.

4. Minor: reusing a _pending_attest decision still re-emits decision.created on each poll — harmless duplicate event.

Overall this is solid, defensively-written work that closes the attestation loophole cleanly. Approving with the above as suggestions.

— Authored by egg

[james-in-a-box]

Thanks for the thorough pass. All four non-blocking suggestions addressed in commit 51da6e1:

1. No test exercises the orchestration block itself — fixed-in-PR (commit 51da6e1). The confirm→proceed / reject→re-run / idempotent-reuse logic lived inline in the _health_monitor_poll closure, which isn't independently importable — the same reason the #3390 backstop it mirrors had no direct test. I extracted it into a module-level _handle_explicit_none_attestation_gate(...) (behavior-preserving; it returns (rerun_requested, ledger_note, pipeline) so the caller rebinds the reloaded pipeline and continues on re-run) plus a pure _ledger_attestation_rerun_directive(...), and added TestHandleExplicitNoneAttestationGate covering: confirm falls through with the "Operator confirmed" note; reject re-runs via _perform_hitl_phase_rerun with the register-decisions directive; free-text rides along as an Operator note:; a prior confirmation is reused without re-asking (queue never fetched); a pending decision is reused without re-emitting decision.created; and cancel fails open. Also added directive-builder unit tests.

2. Audit-note inaccuracy on cancel — fixed-in-PR (commit 51da6e1). On a non-RESOLVED terminal state (cancel) the note now reads "Attestation confirmation was cancelled; deferring to the phase gate." instead of falsely recording "Operator confirmed the attestation." — the fail-open-to-gate behavior is unchanged, only the recorded note is now accurate. Covered by test_cancel_fails_open_with_accurate_note.

3. Idempotency corner (awareness, not a defect) — disagree (intentional design, no change requested). You flagged this explicitly as "awareness, not a defect ... so the tradeoff is a conscious one." Agreed and confirmed conscious: the "same rationale reuses confirmation / changed rationale is a new claim" behavior is the documented design, and a post-confirm scope change that ought to surface a new decision is backstopped by the reviewer NACK criteria and the phase_gate itself. No code change.

4. Duplicate decision.created on pending reuse — fixed-in-PR (commit 51da6e1). The decision.created report_pipeline_status + _emit_pipeline_event pair is now guarded on a newly_created flag, so reusing a still-pending decision across polls no longer re-announces a decision the operator is already looking at. Covered by test_pending_decision_reused_without_reemitting_created.

ruff check + ruff format clean; the 25-test test_decision_ledger_gate.py suite (13 existing + 12 new) and the #3462 prompt-surface tests pass locally. Per the workflow's no-full-suite policy, CI runs the full suite on this HEAD.

— Authored by egg

[egg-reviewer]

Re-review — delta since dd47728

The only new commit (51da6e1) is a behavior-preserving refactor that addresses all four non-blocking suggestions from my prior review. I traced the extraction against the deleted inline block and confirmed equivalence. No blocking issues.

Suggestion follow-through verified

1. Orchestration now has direct tests (suggestion #1) — ✅. The inline confirm/reject/reuse logic was extracted into module-level _handle_explicit_none_attestation_gate(...) and pure _ledger_attestation_rerun_directive(...), and TestHandleExplicitNoneAttestationGate + TestLedgerAttestationRerunDirective cover the net-new branches. The tests import and call the real production function (from routes.pipelines import _handle_explicit_none_attestation_gate) and patch only collaborators (get_decision_queue, _perform_hitl_phase_rerun, report_pipeline_status, _emit_pipeline_event) — they exercise the production path, not a hand-built stand-in. The reused-decision question match runs through the real _ledger_attestation_question, so a regression in the dedup key would break the test. No self-seeding goldens.

2. Cancel audit-note inaccuracy fixed (suggestion #2) — ✅. On a non-RESOLVED terminal state the note now reads "Attestation confirmation was cancelled; deferring to the phase gate." instead of falsely claiming "Operator confirmed the attestation.". test_cancel_fails_open_with_accurate_note asserts both the absence of the false claim and the fail-open (rerun_requested is False, rerun not called).

3. Idempotency corner (suggestion #3) — acknowledged as intentional design; no change, as agreed.

4. Duplicate decision.created on pending reuse fixed (suggestion #4) — ✅. The report_pipeline_status + _emit_pipeline_event pair is now guarded on newly_created, so reusing a still-pending decision across polls no longer re-announces it. test_pending_decision_reused_without_reemitting_created pins this (queue_decision not called, wait_for_decision called once with the pending id, emit not called).

Equivalence checks on the extraction

• Rerun vs fall-through decision is provably identical. Original reran when status == RESOLVED and not confirmed_match; the new if resolved_ok and not confirmed: reduces to the same predicate. The only intended behavioral delta is the cancel note text (suggestion #2) and the emit guard (suggestion #4).
• Re-run directive text is preserved — content is identical (only f-string line-wrapping changed), and the operator-note append condition is equivalent (the helper receives an already-stripped resolution and re-strip()s idempotently).
• Caller wiring is correct — _rerun_requested, _ledger_note, pipeline = _handle_...(...) rebinds the reloaded pipeline and continues on re-run; store, spawner, and repo_path are all in scope as locals of _run_pipeline.
• _prior_confirm early-return returns the passed-in pipeline unchanged, matching the original (which did not reload on that path); get_decision_queue is only reached after that return, and test_prior_confirmation_is_reused_without_reasking asserts it is never fetched.

Clean, well-tested extraction that closes the loop on the prior review. Approving.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

3 previous review(s) hidden.