Improvement controller lane 3: control journal, fenced dispatch, and the valor-improve CLI

[tomcounsell]

Closes #3215

Plan: docs/plans/improvement-controller-lane-3-control-journal-fenced-dispatch.md

Summary

Ships the control substrate the improvement controller's dispatch decisions run on: a control journal with a fencing lease, durable dispatch intents, an admitted session status, a reconcile reflection, a unit-2 paid-inference meter, a vault writer, and the valor-improve CLI — the only door through which a research session proposes anything.

• Control journal (tools/improvement_control/journal.py): one Lua EVAL per transition, checking schema, pause, generation >= highest_accepted, expected_revision == revision, and — for a session-bound write — the intent binding, all before recording anything.
• Lease (lease.py): LeaseProtocol matches Session execution lease: fence worker ownership with a renewable Redis lease #3220's declared interface; CaseLease is the interim implementation, deletable in one commit the day models/redis_lease.py exists (hand-off posted on #3220).
• Dispatch intents (intents.py): the six-state machine (Decision 13), one script per effect, the lane-slot reservation, and finalize_session step 7's terminal hook.
• admitted session status: added to NON_TERMINAL_STATUSES, RECOVERY_OWNERSHIP, and ACTIVE_STATUSES in one commit; inert to the worker, the health check, startup recovery, and the resume drip.
• Scheduler adapter (scheduler_adapter.py, the improvement-controller-tick reflection): admits, materializes through the create-or-bind seam, and activates once a worker is alive — Race 7's four branches (admitted/pending/terminal/missing row) all handled on a retry tick.
• Reconcile pass (recovery.py, improvement-intent-reconcile, 300s): stale-sweep budget for admitted/materialized intents, first-sweep action for a running intent whose row is gone, forced-terminal via the existing finalize_session.
• Unit-2 meter (tools/paid_inference_meter.py): mirrors unit 3's shape; settles exact from usage.cost or estimated from tokens; the cross-vendor judge's review spend is receipted, never gated.
• Vault writer (tools/vault_write.py): the one sanctioned op item create path; no credential byte on any path.
• valor-improve CLI (tools/improvement.py, 12 subcommands) and the research skill (.claude/skills/improve-research/SKILL.md).
• Dashboard control panel (ui/data/improvement.py::get_control_status).

Fault injection (the issue's acceptance criteria)

• Stale-generation rejection, split by writer kind: test_stale_controller_generation_is_refused (Race 4a) and test_stale_session_intent_is_refused (Race 4b, unit + integration).
• Crash between admission and session creation: Race 2, retry yields one row via the create-or-bind seam.
• Unreleased lane slot on restart: Race 3, freed on the first qualifying reconcile sweep.
• Journal unavailability: UNAVAILABLE reason code, doctor reports it, namespace state is untouched and reads correctly once restored.

Mutation review (Task 12)

12/12 mutations caught (generation compare, revision compare, from_state compare, and compare-and-delete across journal.transition, intents.admit/record_materialized/record_running/cancel/on_session_terminal, and lease.renew/release) — no blockers. Hand-off comment posted on #3220.

Deviations from the plan (noted for the record)

• dead_letter_stage value is improve_intent (the stage already reserved in bridge/dead_letters.py's STAGES), not the plan's literal "improvement-intent" string — fixed a real bug in models/session_lifecycle.py::_record_terminal_dead_letter along the way (it silently coerced any non-session_init_hang stage to "session_recovery_cap", ignoring the caller's own stage).
• ImprovementCase.query.filter() does exact-match on an IndexedField, not IN — every "iterate open cases" site loops per-state, matching ui/data/improvement.py's existing pattern (this was a real bug caught by the scheduler-adapter tests before commit).
• _move()'s from_state is always explicit, never auto-derived from _ALLOWED, because reconciliation_required has three valid predecessors (another real bug caught before commit).
• The Verification table has 6 malformed rows (unescaped |, or a basic grep needing -E for alternation) that are plan-authoring artifacts, not code issues — verified by hand instead; see the Task 14 commit message for the full accounting.
• tools/paid_inference_meter.py's reserve/release scripts are not generation/revision-fenced by the case lease — unit 2 is a project-level daily pool, independent of any one case's state, so Task 12's mutation matrix for "reserve_unit2/settle_unit2" didn't apply the same way; the pool's own reserve-then-check atomicity is still one Lua EVAL.

Review rounds

• Round 1 (e8dcaf8ce): head state seeded on the first accepted write with apply/replay refusing an empty clobber; propose stores the payload through VerifyingArtifactStore before the lease and journals the $CF: reference; doctor guards every namespace read and the outage break-glass drill is a test; action_type journaled; slot release and reason moved inside the move script; dead_letter_exhausted on every exhaustion branch; adapter passes a real working_dir, both digests, and a /improve-research message; publish-count, override-key, and isolation assertions; unit2 and ns_pause round-trip on import; sweep honors the injected clock. The plan's Risk 5 --acknowledge-unknown gate is reframed as accepted residual scope (charter §8 receipt retained; the budget unknown-receipt block below is its visible surface).
• Round 2 (93def1ecb): the artifact path is asserted on the accepted, refused, and export paths; propose tests write to a tmp content root; projection.apply runs after every accepted CLI transition; a state_changed journal event is the writer of head state, re-seeded when empty; import refuses foreign keys before writing and --force replaces history; budget lists metering="unknown" receipts with their window; settle is one Lua script; ARTIFACT_WRITE_FAILED reason code; brief_ref carries the loadable reference.
• Round 3 (f657d12ea): tools/vault_write.py defaults to the op enum category API_CREDENTIAL (the display name "API Credential" was refused by op item create --template), pinned at unit level by TestTemplateCategoryIsTheEnumSpelling; _probe_vault_write abstains with unknown instead of certifying a write from an existence check; three nits: the unused _project_key helper deleted, the control panel's <h4>Control</h4> title dropped so the heading hierarchy matches its sibling panels, and import's expire routed through assert_control_key.
• Round 4 (8f8c22f22): import --force deletes the namespace slot and pause hashes before restoring, so a slot admitted after the export is dropped with its intent instead of stranded with no release path (test_import_force_drops_a_post_export_slot, red under mutation); doctor reads the unit-1 slot hash and the open unit-2 window for real and prints them under reservations, naming each slot's holder case, with the clean line only when all three views are empty (unit and end-to-end tests assert the seeded slot); five nits: pause carries reason and refuses CASE_BUSY on a held case lease instead of presenting generation 0; a break-glass propose without --action-id mints one so the proposal is admittable; release is one Lua script under a CAS on state == 'reserved' (a double release decrements exactly once); _probe_vault_write carries a docstring stating the invariant; this body refreshed.

Testing

• scripts/pytest-clean.sh on all 21 files the plan's own "Lane tests pass" Verification row names: 732 passed at 8f8c22f22.
• Integration test_improvement_control_cli.py: 5 passed at 8f8c22f22, run against the real .venv/bin/valor-improve binary in a claimed test db. test_vault_write_integration.py is guarded by OP_CACHE=false op whoami and skips where the valor-local service account is absent (both round-4 judges reported it skipped on their machine); where op authenticates non-interactively it runs, creates one test-lane3-<uuid> item in m-valor, and deletes it in a finally (verified at round 3: no residue, 57 items before and after). The round-3 category fix is pinned by tests/unit/test_vault_write.py::TestTemplateCategoryIsTheEnumSpelling, which needs no op auth and was proven red against the display-name value.
• ruff check . / ruff format --check .: clean at 8f8c22f22 (50 changed Python files in the diff).
• Docs gate (scripts/validate_docs_changed.py): passed, 7 docs changed.

[tomcounsell]

Review: Changes Requested

Three blockers, all verified by reading the code and reproducing the behavior in the review worktree (detached at bd3e49e): the projection writes an empty state over ImprovementCase.state because no script ever writes state onto the head; propose never stores the proposal payload it hashes, so an accepted proposal's content exists nowhere; and the journal-unavailability break-glass drill (one of the issue's four acceptance criteria) has no test, with doctor raising a traceback instead of reporting namespace unreachable on that path. The substrate itself is sound: 710 unit tests and 5 integration tests reproduced green here, the fences held against three additional mutations I ran myself, the scope guard is clean, and the five disclosed deviations all check out.

Mode: sequential lenses (Agent tool unavailable: not in tool list). The repo declares a two-judge roster (code-quality, risk); the Agent tool is absent from this stage-runner context, so both lenses were applied in sequence by one reviewer. compute_consensus(expected_judges=2) over the one reporting judge returns quorum_shortfall: true (n=1, expected_n=2); this run is recorded as that shortfall, never as agreement between judges. The verdict below is driven by the blockers and would be CHANGES REQUESTED under a full roster too.

Rubric

• 1. Plan vs. implementation match — fail — Blockers 1-3: head state never written (Data Flow steps 4-5, Decision 2's head schema), payload never stored (Data Flow step 2, Race 4b), break-glass drill absent (Success Criterion 2). Tech debt 4 and 7 are further Data Flow gaps (action_type dropped; adapter passes empty digests and a relative working_dir).
• 2. New code quality — pass — Type hints, dataclasses, reason-code vocabulary, one Lua script per effect, private Redis alias. Tech-debt items 5 and 6 are design deviations, not quality problems.
• 3. Test coverage — fail — 710/710 reproduced. Gaps: the outage drill (blocker 3); publish-exactly-once and zero-on-no-worker never asserted; recorded extra_context_overrides keys never asserted; test_tick_isolates_one_bad_case absent (tech debt 8).
• 4. Regression risk to existing callers — pass — finalize_session step 7 is gated on action_id and exception-isolated (three tests). _record_terminal_dead_letter rewrite is behavior-neutral for its two existing callers (session_init_hang stays non-replayable, session_recovery_cap stays replayable); 112-test lifecycle file green. admitted is additive and asserted invisible to worker/health/drip.
• 5. Data integrity — fail — Blocker 1: projection.apply/replay overwrite ImprovementCase.state with "", removing the case from every OPEN_CASE_STATES reader. No migration needed otherwise (non-Popoto namespace).
• 6. Security — pass — Vault writer never carries the value in argv/logs/result/evidence; template file in a mkdtemp dir, unlinked in finally. No eval/exec/shell interpolation; the only subprocess is the fixed op item create argv.
• 7. Documentation accuracy — fail — docs/features/improvement-controller.md:349 says "Payloads are written to the content store and hashed before propose references their digest"; the code hashes and never writes (blocker 2). Everything else in the doc cascade matches the shipped shapes.
• 8. PR body accuracy — fail — "Journal unavailability: ... doctor reports it" is untested and, on the control-namespace path, false (blocker 3). Test counts, deviation list, ruff, and scope claims all reproduced.
• 9. Disclosed deferrals — pass — All five deviations are explicit, verified, and sound (see Acknowledged Deferrals).
• 10. Follow-up claims verified — pass — Session execution lease: fence worker ownership with a renewable Redis lease #3220 is OPEN; hand-off comment 5665248148 exists and names LeaseProtocol, default_lease(), the retirement test, and the Verification row.

Pre-Verdict Checklist

• 1. All plan acceptance/success criteria validated against diff — FAIL — Success Criterion 2's outage drill and the head-state/projection contract unmet; see blockers.
• 2. No-Gos from plan — none violated — PASS — git diff --stat origin/main -- agent/agent_session_queue.py agent/session_executor.py models/agent_session.py scripts/update/migrations.py tools/infrastructure_budget.py tools/improvement_eval/ is empty; no parent_agent_session_id=/VALOR_ALLOW_CHILD_SESSIONS on the research path; cross-vendor judge keeps its provider; models/redis_lease.py absent.
• 3. New except Exception blocks — each has logger/raise/swallow-ok — PASS — Every new broad except logs (scheduler_adapter.tick, recovery.reconcile, finalize_session step 7, vault_write, get_control_status, _sweep_unit2).
• 4. New integration tests — exercise serialization boundary — PASS — test_improvement_control_cli.py runs the real .venv/bin/valor-improve subprocess against the claimed test db with AGENT_SESSION_ID set; 5 passed, 1 skipped (op auth absent, by design).
• 5. Plan internal consistency — spike findings match task steps — PASS — Decision 3 named reserve_unit2/settle_unit2 as generation-fenced while Decision 7's signature carries no generation; the builder followed Decision 7 and disclosed it (acknowledged).
• 6. No hardcoded secrets or debug artifacts — PASS — No prints, TODOs, or secrets in the diff.
• 7. New public APIs — docstrings present — PASS.
• 8. Breaking changes — migration path documented — N/A — Additive status value, constants, and a separate Redis namespace; Update System section confirms no migration.
• 9. Tests added for new behavior — PASS — 21 unit files plus 2 integration files; Race 1/2/3/4a/4b/6/7 each have a named test.
• 10. Tests cover the failure path — FAIL — Outage drill missing (blocker 3); publish-count and kwargs assertions missing (tech debt 8).
• 11. UI changes (if any) — screenshot captured — PASS — 1 screenshot of the running dashboard from this branch on port 8517 (see Screenshots).
• 12. Docs updated for user-facing changes — FAIL — Docs are updated, but one claim is false (rubric 7).

Blockers

• File: tools/improvement_control/projection.py:59 (verified: read this file)
  Code: case.state = head.state
  Issue: No script ever writes state onto the head hash. journal._LUA_TRANSITION (journal.py:99-105) and every intent script (intents.py:115-116, 170-171, 210-211, 245-246) HSET only revision, highest_accepted, epoch, updated_at; read_head therefore returns state="" (journal.py:172). apply then saves "" into ImprovementCase.state, and replay (exposed as valor-improve replay-projection) does the same. A case whose state is "" is dropped by every OPEN_CASE_STATES per-state loop (scheduler_adapter._open_case_rows, recovery.reconcile, cmd_doctor, get_control_status), so the operator's own reconciliation command makes the case vanish from the controller. Reproduced: a probe test creating an investigating case, running one accepted transition, then apply, fails with apply clobbered state to ''. test_apply_writes_state_and_revision_from_the_head asserts only revision, and test_replay_corrects_a_direct_save_with_a_wrong_state corrupts revision, not state, which is why the suite is green.
  Severity: blocker
  Fix: Give the head a real state: seed it on the first accepted write (e.g. the transition/admit scripts HSET state from an ARGV the Python layer fills from the case's current state, or ensure_head(project_key, case_id, state) called before the first transition), and have apply/replay refuse to write an empty state (log and keep the projection's value). Add a test that apply after one action_proposed leaves state == "investigating", and make the replay test corrupt state, as Task 5 specifies.

• File: tools/improvement.py:94-136 (verified: read this file)
  Code: payload_digest = "sha256:" + hashlib.sha256(payload_text.encode()).hexdigest() ... transition(... payload_digest=payload_digest, ...)
  Issue: Data Flow step 2 ("Artifact first: the payload is written to VerifyingArtifactStore under POPOTO_IMPROVEMENT_CONTENT_PATH and hashed; the digest is what the journal will reference") is not implemented. cmd_propose reads the file, hashes it, journals the digest, and discards the text. On the accepted path the proposal's content exists nowhere the controller, lane 5, or an operator can read it back from; on the refused path (Race 4b) the evidence row carries only the digest, so "the artifact it tried to submit is kept as evidence rather than lost" is not true either. export.py:76-78 writes artifacts.json with a hardcoded "digests": [] for the same reason. models/verifying_artifact_store.py exists on main and is unused by this lane. docs/features/improvement-controller.md:349 claims the write happens.
  Severity: blocker
  Fix: Store the payload through VerifyingArtifactStore before the lease is taken, journal the store reference alongside (or as) the digest, keep the reference on the refusal evidence row, populate artifacts.json from the stored references, and correct the doc if the shape changes. Add a test that an accepted propose leaves the payload loadable by digest and a refused one leaves it as evidence.

• File: tools/improvement.py:292-308 (verified: read this file)
  Code: for case in cases: head = read_head(PROJECT_KEY, case.id) ... for intent in list_intents(PROJECT_KEY, case.id):
  Issue: The issue's fourth acceptance criterion and the plan's Success Criterion 2 require "journal unavailability with the break-glass recovery exercised end to end (doctor reports unreachable, namespace restored, doctor reads the pre-outage heads)". No test exercises it: test_connection_error_becomes_unavailable_reason_code covers the transition reason code only, and neither test_improvement_cli.py nor the integration file contains "unreachable". The code path is also wrong: cmd_doctor's try guards only the imports and the ORM case query; read_head/list_intents run outside it, so a redis.exceptions.ConnectionError from the control namespace propagates as a traceback with exit 1 instead of namespace unreachable: <error> with exit 2. Reproduced with a probe that monkeypatches journal._control_redis/intents._control_redis to raise: main(["doctor"]) raised ConnectionError: simulated outage, stdout empty.
  Severity: blocker
  Fix: Move the per-case reads inside the guard (catch redis.exceptions.ConnectionError/TimeoutError around them, print namespace unreachable: <error>, return 2), and add the end-to-end drill test: seed a head, make the namespace client raise, assert doctor prints unreachable with exit 2 and wrote nothing, restore the client, assert doctor reads the same head.

Tech Debt

• File: tools/improvement.py:481 and tools/improvement_control/scheduler_adapter.py:88 (verified: read both)
  Code: p.add_argument("--action-type", default="investigate") / action_type = entry.get("action_type") or "investigate"
  Issue: --action-type is parsed but cmd_propose never reads args.action_type; transition has no parameter for it and the journal entry (journal.py:115-122) carries no action_type, so the adapter's _unadmitted_proposal defaults every proposal to investigate and the intent hash records that. A propose --action-type experiment is dispatched and journaled as an investigation.
  Severity: tech_debt
  Fix: Carry action_type through transition into the journal entry (validated against ALLOWED_ACTION_TYPES at the CLI) and read it in _unadmitted_proposal; assert it end to end in test_happy_path_admits_materializes_and_activates.

• File: tools/improvement_control/intents.py:503-533 (verified: read this file)
  Code: result = _move(...) then _control_redis().hdel(keys.slots_key(project_key), action_id)
  Issue: Decision 3 makes every effect one script that checks and records in the same call, and Task 4 lists mark_reconciliation_required as "from_state check, slot release, journal event". Here the slot release is a second, unconditional HDEL after the CAS script returns, so a crash between the two leaves a reconciliation_required intent still holding its slot until cancel runs. The reason keyword is accepted and never written to the intent hash, so case explain's "why is this case where it is" cannot show why an intent was wedged (DispatchIntent.reason stays "").
  Severity: tech_debt
  Fix: Pass the slots key into _LUA_MOVE_INTENT and HDEL inside the script when to_state == "reconciliation_required"; HSET reason in the same call.

• File: tools/improvement_control/recovery.py:199-221 (verified: read this file)
  Code: if row is not None and row.status not in terminal_statuses: finalize_session(row, "abandoned", reason=reason, dead_letter_stage="improve_intent")
  Issue: intents.dead_letter_exhausted (intents.py:643-667) is defined and unit-tested but has no production caller. An exhausted intent with no bound row (an admitted intent that never materialized, which is exactly the Race 2 exhaustion shape) writes no DeadLetter at all. When a row does exist, the rewritten _record_terminal_dead_letter (models/session_lifecycle.py:280-284) records it replayable=True, while Task 4 and dead_letter_exhausted say replayable=False; the same exhaustion is recorded two different ways depending on whether the row survived.
  Severity: tech_debt
  Fix: Call dead_letter_exhausted on the no-row branch (and on the has-row branch instead of relying on finalize_session's generic write, or pass replayable through), and pin one replayable value in a test.

• File: tools/improvement_control/scheduler_adapter.py:164-201 (verified: read this file)
  Code: admit(..., action_type=action_type, max_concurrent=...) / push(project_key=project_key, session_id=str(uuid.uuid4()), working_dir=".", message_text=(f"Improvement research: case={case_id} action={action_id} type={action_type}"), ...)
  Issue: Three Data Flow deviations in the dispatch call. (a) working_dir="." is stored on the AgentSession row; the worker resolves it against its own cwd, and the precedent this plan cites (agent/reflection_scheduler.py:813) passes str(project_root). (b) admit is called with request_digest="" and charter_digest="", so the intent hash never records the digest it was admitted under and the adapter performs no "charter digest pinned" check (Data Flow step 6 lists four checks; the CLI's propose-time check is the only one that runs). (c) message_text names neither the improve-research skill nor a brief reference (Data Flow step 9: "the session's prompt carries the case brief and the research skill"; Rabbit Holes: "this lane's adapter takes a brief_ref digest and passes it through").
  Severity: tech_debt
  Fix: Resolve the project root the way the reflection scheduler does; pass the case's charter_digest and the journal entry's payload_digest to admit and compare the charter digest against ImprovementCharter.pinned(project_key).digest; have the message invoke /improve-research and carry brief_ref when present.

• File: tests/unit/test_improvement_control_dispatch.py:128-150 (verified: read this file)
  Code: result = adapter.tick(PK, lease=fake_lease(), push=push) / assert result.skipped.get(case.id) == "no_live_worker"
  Issue: Success Criterion 7 requires "publishes publish_session_notify(session) exactly once per activation, after the row is pending, and never on the no-live-worker path (asserted on the recorded call)"; neither the happy-path nor the no-live-worker test records publish_session_notify (only the two Race 7 retry tests do). Success Criterion 5 requires the recorded extra_context_overrides to carry exactly research_case_id, experiment_id, action_id, idempotency_key and no generation, and Task 6 requires asserting parent_agent_session_id is never passed; CountingPush records neither. test_tick_isolates_one_bad_case (Failure Path) does not exist.
  Severity: tech_debt
  Fix: Record the publish calls and the push kwargs in CountingPush; add the three assertions and the isolation test.

• File: tools/improvement_control/export.py:60,102-124 (verified: read this file)
  Code: "unit2": {k: r.hgetall(k) for k in r.scan_iter(...)} (exported) with no matching write in import_namespace
  Issue: unit2 window and reservation hashes are exported but never restored by import_namespace; the namespace pause hash (_ns:pause) is neither exported nor imported. A restore silently drops the day's spend accounting and any operator-wide pause.
  Severity: tech_debt
  Fix: Restore unit2 keys and round-trip _ns:pause; extend the round-trip test.

• File: tools/paid_inference_meter.py:349-357 (verified: read this file)
  Code: def sweep_unsettled_reservations(project_key: str, *, now: float | None = None) ... today_key, _, _ = current_day(datetime.now(UTC), "UTC")
  Issue: The now argument is accepted (and passed by recovery._sweep_unit2) but ignored; today_key always comes from the wall clock, so the deterministic-clock seam the reconcile pass exposes does nothing for this branch.
  Severity: tech_debt
  Fix: current_day(datetime.fromtimestamp(now, UTC), "UTC").

Nits

• File: tools/improvement_control/keys.py:90-98 (verified: read this file)
  Code: def intent_scan_pattern(project_key: str) -> str:
  Issue: No caller. recovery.reconcile enumerates open cases through the ORM and list_intents (a cleaner choice than the plan's scan_iter), so the "one sanctioned scan pattern" is dead code and its docstring describes a reader that does not exist.
  Severity: nit
  Fix: Delete it, or use it in reconcile if intents on non-open cases must also be swept.

• File: tools/improvement_control/journal.py:66 (verified: read this file)
  Code: if ns_pause_reason and ns_pause_reason ~= "" and event ~= "ns_resumed" then
  Issue: ns_resumed is not in KNOWN_EVENTS and no caller emits it (namespace resume is a direct delete in resume()), so the exemption is unreachable.
  Severity: nit
  Fix: Drop the event ~= "ns_resumed" clause.

• File: tools/improvement_control/intents.py:55-59,185-187 (verified: read this file)
  Code: #: Concatenated into each script body so a mutation to one script's copy cannot silently spare another's / #: ARGV: [schema_version, expected_revision, generation, action_id, from_state, journal_max_entries]
  Issue: There is one _LUA_FENCE_PRELUDE constant shared by four scripts, so a single mutation hits all of them at once; the comment claims per-script copies. The _LUA_MOVE_INTENT ARGV comment lists six arguments; the script reads eight (to_state, event, then journal_max_entries at ARGV[8]).
  Severity: nit
  Fix: Correct both comments.

• File: tools/improvement_control/__init__.py:22 (verified: read this file)
  Code: POPOTO_REDIS_DB`` or anything from ``popoto.redis_db`` directly
  Issue: The docstring spells out both names, so the plan's own "Anti-criterion: no Popoto client in the control package" grep reports __init__.py:1, and Risk 4's mitigation says "no file in it names POPOTO_REDIS_DB or popoto".
  Severity: nit
  Fix: Reword the docstring without the literal identifiers (the Task 14 commit already did this for recovery.py's HINCRBY.*attempts).

• File: tools/vault_write.py:26-27 (verified: read this file)
  Code: return subprocess.run(argv, capture_output=True, text=True, timeout=30)
  Issue: Decision 8 and CLAUDE.md's sanctioned shape run OP_CACHE=false op item create ...; the default runner inherits the ambient environment and sets nothing.
  Severity: nit
  Fix: env={**os.environ, "OP_CACHE": "false"} in _default_runner.

• File: ui/templates/improvement/control.html:9-16 (verified: read this file)
  Code: {% elif control.empty %} <p class="improvement-meta">Nothing yet, written by lane 3 when a case is admitted.</p>
  Issue: The empty and unavailable states render with no <h4> heading, unlike every sibling panel (goals.html heads each section), so on the dashboard the sentence floats under the corrections banner with nothing naming it as the control panel (visible in the screenshot).
  Severity: nit
  Fix: Add an <h4>Control</h4> (or similar) above all three states.

• File: tools/paid_inference_meter.py:137-183 (verified: read this file)
  Code: accepted = _redis().eval(_LUA_RESERVE, 1, window_key, cents, cap_cents, KEY_EXPIRY_SECONDS)
  Issue: Risk 5's mitigation ("a reservation with no settlement is receipted metering="unknown" ... and pauses further purpose="rsi" admission until an operator runs budget --acknowledge-unknown") has no implementation: no --acknowledge-unknown subcommand exists and an unknown receipt does not gate reserve. Decision 7 says only purpose="rsi" counts against the pool; reserve counts every purpose (documented in its docstring as deliberate). Neither is in the Success Criteria; recorded so the residual is visible.
  Severity: nit
  Fix: Either implement the acknowledge gate or strike the sentence from Risk 5 in the plan and doc.

Miscellaneous

• None

Acknowledged Deferrals (verified)

• dead_letter_stage is improve_intent, the already-reserved stage, not the plan's literal "improvement-intent" — explicit deviation with sound rationale (bridge/dead_letters.py STAGES reserves it; a new literal would fail validation); the _record_terminal_dead_letter fix is behavior-neutral for both existing callers. The replayable flip it introduces is tech debt 3 above, not a reason to reject the deviation.
• ImprovementCase.query.filter() is exact-match; every open-case site loops per state — verified against ui/data/improvement.py's existing pattern; consistent across adapter, recovery, doctor, and dashboard.
• _move() takes from_state explicitly rather than deriving it from _ALLOWED — sound: reconciliation_required has three predecessors; _move still refuses any pair outside _ALLOWED (ValueError), and test_transition_table_is_enforced covers the script side.
• 6 malformed Verification rows verified by hand — confirmed: all six pass when run with the intended escaping (grep -cE "def (acquire|renew|release)\(" = 6; no KEYS/SCAN in Lua; seam diff empty; _ALLOWED/INTENT_STATES = 2; no OpenRouter URL or HTTP import; no op outside the vault writer).
• Unit-2 reserve/release scripts are not generation/revision-fenced by the case lease — deliberate scope boundary, accepted: unit 2 is a project-level daily pool with no case revision to fence against, and Decision 7's own signature carries no generation; the pool's reserve-then-check is one EVAL. Decision 3's inclusion of reserve_unit2/settle_unit2 in the fenced list was the inconsistent line in the plan.
• Session execution lease: fence worker ownership with a renewable Redis lease #3220 hand-off — tracked by Session execution lease: fence worker ownership with a renewable Redis lease #3220 (OPEN); comment 5665248148 posted 2026-09-14 names the exact edits that retire CaseLease.

Verification Results

• Lane tests (the plan's own 21-file row, run via scripts/pytest-clean.sh from a detached review worktree at bd3e49e with PYTHONPATH pinned): 710 passed (2m59s). Reproduces the builder's count.
• Integration (test_improvement_control_cli.py, test_vault_write_integration.py): 5 passed, 1 skipped (op auth absent, by design).
• ruff check . and ruff format --check . on the whole tree: clean.
• Scope guard: git diff --stat origin/main...HEAD -- agent/agent_session_queue.py agent/session_executor.py models/agent_session.py scripts/update/migrations.py tools/infrastructure_budget.py tools/improvement_eval/ = 0 lines.
• Verification table (agent.verification_parser): 28 parsed rows, 6 malformed (plan-authoring, verified by hand as above). All pass except "Judge receipt pinned to the pool's project" (single-line grep defeated by ruff's multi-line formatting; the call is at tools/cross_vendor_judge.py:275-282 with project_key="valor", correct) and "no Popoto client in the control package" (docstring mention only; nit above).
• Reviewer-run mutations (beyond the builder's 12): drop the agent_session_id compare in transition's intent binding → 1 failure (test_improvement_control_journal.py); make on_session_terminal's slot compare-and-delete unconditional → 1 failure (test_improvement_control_intents.py); remove mark_reconciliation_required's slot release → 2 failures (test_improvement_control_recovery.py). All caught.
• Decision 12 fencing rule confirmed in code: the lease generation is compared in every controller script (generation >= highest_accepted, journal.py:78 and the shared prelude at intents.py:74); sessions are fenced by intent.state == "running" and intent.agent_session_id == ARGV inside transition (journal.py:86-93), and scheduler_adapter.py contains no "generation" in extra_context_overrides.
• Bridge/worker impact: models/session_lifecycle.py and agent/session_health.py changed, plus two new reflection registrations; a worker restart on every machine is required after deploy.

Screenshots

• generated_images/pr-3315/01_dashboard_control_panel.png — the dashboard from this branch (python -m ui.app, port 8517) scrolled to the new #improvement-control-panel, rendering the empty state "Nothing yet, written by lane 3 when a case is admitted." against the production namespace (no open cases). Captured via BYOB in the operator's Chrome.

[tomcounsell]

Review (Judge code-quality): Changes Requested

Independent judge, dispatched as its own subagent with no view of the risk judge's output. Reviewed at head e8dcaf8ce (rebased onto main b13dc8ad3). Verdict: CHANGES REQUESTED, 0 blockers, 5 tech debt, 6 nits. The three round-1 blockers are substantively closed and mutation-verified; what remains is coverage and hygiene of the fix commit.

Round-1 disposition (located by symbol at e8dcaf8ce)

• B1 closed: tools/improvement_control/journal.py:109 (HSETNX head state), :205-218 (_current_case_state), tools/improvement_control/projection.py:59-73 (empty-state guard). Removing the seed → test_replay_corrects_a_direct_save_with_a_wrong_state red. Removing the apply guard alone → green (guard untested).
• B2 partial: tools/improvement.py:134-140 writes through VerifyingArtifactStore before the lease, :159 journals artifact_ref, :177 keeps it on the evidence row, export.py populates artifacts.json. Replacing store.save with artifact_ref = "" → test_improvement_cli.py + integration all green (19 passed). The test round 1 asked for was not added, and the tests now write into the production store.
• B3 closed: tools/improvement.py:160-176 guards read_head/list_intents; drill at tests/unit/test_improvement_cli.py:187-227. Moving the loop outside the guard → drill red.
• TD1 closed (journal.py:132, scheduler_adapter.py:102-104, CLI validation improvement.py:83-89; mutation red). TD2 partial (HDEL and reason HSET moved into the CAS script at intents.py:224-229; HDEL mutation red, reason mutation green across 76 tests). TD3 partial (recovery.py:230 calls dead_letter_exhausted on every branch, replayable=False pinned in the helper's test; deleting the recovery call → suite green). TD4 partial (_project_root :59-67, charter check :179-194, digests into admit :205-206, /improve-research message :220-222; all four mutations green). TD5 closed (publish exactly once, never on no-worker, exact override keys, isolation test; three mutations red). TD6 closed (unit2 and ns_pause round-trip; both mutations red). TD7 closed (injected clock; mutation red).
• N1, N2, N4, N5, N6 closed. N3 partial: prelude and ARGV comments corrected, but intents.py:192-193 still says from_state is derived from _ALLOWED, contradicting _move's docstring at :477-480. N7 closed as explicit exclusion: plan :372 reframes the --acknowledge-unknown gate as accepted residual scope; the feature doc never claimed it; judged acceptable.

Blockers

• None

Tech Debt

• File: tests/unit/test_improvement_cli.py:266 and tests/integration/test_improvement_control_cli.py:140 (verified: read both; ran both)
  Code: def test_propose_break_glass_without_agent_session_id(self, capsys, monkeypatch, tmp_path): / the integration propose runs
  Issue: cmd_propose now writes every payload through VerifyingArtifactStore() with the default root and no propose test sets POPOTO_IMPROVEMENT_CONTENT_PATH. Every suite run leaves files in the production store: ~/.popoto/improvement_content/ImprovementProposal/ holds 25 <case-uuid>-a1-<digest16>.txt files from today's builder, reviewer, and judge runs. The existing store tests (test_improvement_eval_calibration.py:151) redirect to tmp_path; these do not.
  Severity: tech_debt
  Fix: monkeypatch.setenv("POPOTO_IMPROVEMENT_CONTENT_PATH", str(tmp_path)) in every test that reaches cmd_propose (pass it through the subprocess env in the integration file), and add the test round 1 asked for: after an accepted propose, VerifyingArtifactStore().load(result["artifact_ref"]) returns the payload bytes; after a refused one, the evidence row's detail is that same loadable reference. Delete the 25 leaked files.

• File: tools/improvement_control/scheduler_adapter.py:179-222 (verified: read this file; mutated it three ways)
  Code: if charter_digest: ... result.skipped[case_id] = "charter_digest_stale" / working_dir=_project_root(), / message_text = f"/improve-research case=...
  Issue: The TD4 fix added four behaviors and no test reaches any of them: disabling the charter-drift check, reverting working_dir to ".", restoring the old message text, and blanking request_digest/charter_digest into admit each leave test_improvement_control_dispatch.py fully green (9 passed each). The same gap covers intents.py:225 (reason never asserted after mark_reconciliation_required) and recovery.py:230 (deleting the dead_letter_exhausted call leaves the recovery suite green). CountingPush.push_kwargs already records working_dir and message_text; nothing reads them.
  Severity: tech_debt
  Fix: In the happy-path dispatch test assert push.push_kwargs[0]["working_dir"] is an absolute existing path and message_text.startswith("/improve-research "); add a charter_digest_stale skip test; assert list_intents(...)[0].reason after the Race 3 sweep and that a DeadLetter(stage="improve_intent") row exists after the third sweep on the no-row branch.

• File: tools/improvement_control/export.py:134-136 (verified: read this file)
  Code: for key, mapping in data.get("unit2", {}).items(): if mapping: r.hset(key, mapping=mapping)
  Issue: The unit-2 restore is the only write in the package whose key name comes from the archive file rather than a keys.* constructor, so it bypasses keys.assert_control_key (keys.py:35-43). An import of a hand-edited or foreign archive can HSET any Redis key through the package's private alias; the package docstring's namespace guarantee no longer holds on this path.
  Severity: tech_debt
  Fix: Build the prefix from project_key, refuse (ImportRefusal("FOREIGN_KEY")) any key outside it, wrap with keys.assert_control_key(key); add a round-trip test with one foreign key in the archive.

• File: tools/improvement_control/projection.py:38 and docs/features/improvement-controller.md:42 (verified: read both; grepped callers)
  Code: def apply(project_key: str, case_id: str) -> None: / "The flat Popoto records below are its queryable projection, updated after the journal commits."
  Issue: projection.apply has no production caller: cmd_propose, cmd_pause, cmd_resume, and cmd_propose_amendment never call it after an accepted transition; only replay() reaches it. Plan Data Flow step 5 is unimplemented and the doc sentence is false: ImprovementCase.revision stays 0 until an operator runs replay-projection.
  Severity: tech_debt
  Fix: Call projection.apply(PROJECT_KEY, case_id) after each accepted transition in the CLI and assert reloaded.revision == result.revision in the CLI propose test, or, if the projection write is deliberately deferred, say so in the doc and the plan's Data Flow.

• File: models/session_lifecycle.py:273-279 and docs/features/pipeline-dead-letters.md:49 (verified: read both)
  Code: # ... any caller outside this module that passes its own reserved stage (e.g. \improve_intent`, Improvement controller lane 3: control journal, fenced dispatch, and the valor-improve CLI #3215) -- is replayable/| `improve_intent` | Reserved for the improvement control plane (Recursive self-improvement controller: Valor owns the discover, investigate, experiment, evaluate, release loop #3177) | — |**Issue:** After the TD3 fix no caller passesdead_letter_stage="improve_intent", and the lane writes that stage replayable=Falseviadead_letter_exhausted. The lifecycle comment names a caller and a replayability that no longer exist; the dead-letters doc still lists the stage as reserved with no replayable value although this PR writes it. **Severity:** tech_debt **Fix:** Drop the improve_intent example from the lifecycle comment; update the doc row to "Exhausted improvement dispatch intent (recovery.reconcile)" / no; adjust docs/features/improvement-controller.md:505so the dead letter is attributed todead_letter_exhausted` on every branch.

Nits

• File: tools/improvement_control/intents.py:192-193 (verified: read this file)
  Code: #: ... \from_state` is derived by the Python wrapper from `_ALLOWED`, never chosen by the caller.**Issue:** Contradicts_move's docstring at :477-480 and the PR's disclosed deviation. **Severity:** nit **Fix:** "from_state` is passed explicitly by the wrapper and CAS-checked by the script."

• File: tools/improvement_control/scheduler_adapter.py:99-101 plus 14 sibling comment sites (git grep "#3315 review" = 15 hits in tools/)
  Code: # Tech debt fix (#3315 review): ... the fallback stays for any entry written before that change landed.
  Issue: Fifteen production comments narrate review history ("previously", "before the fix landed") for code that has never shipped; paid_inference_meter.py:351-352 says the module "may not yet exist in every deployment" though it lands in the same PR as its caller. CLAUDE.md principle 1: describe only the new status quo.
  Severity: nit
  Fix: Rewrite each comment to state the invariant; drop the review references and the never-existing-entry fallback language.

• File: tools/improvement_control/projection.py:111 (verified: read this file)
  Code: after = head.state
  Issue: On the empty-state branch apply leaves case.state unchanged, but replay reports projection_state_after="", so replay-projection --json misreports exactly when the guard fires. The guard itself has no test.
  Severity: nit
  Fix: after = head.state or (before or ""); add a test that HSETs a head without state and asserts apply keeps "investigating".

• File: tools/improvement.py:138-140 (verified: read this file)
  Code: artifact_ref = store.save(payload_bytes, key=artifact_key, model_class_name="ImprovementProposal")
  Issue: The one filesystem write in cmd_propose is unguarded; every other boundary in the command returns a reason code. A full disk or missing content root produces a traceback and exit 1 with no --json refusal.
  Severity: nit
  Fix: except OSError as e: _emit(args, f"refused: ARTIFACT_WRITE_FAILED ({e})", {...}); return 1.

• File: tools/improvement_control/scheduler_adapter.py:220-222 and .claude/skills/improve-research/SKILL.md:15-29 (verified: read both)
  Code: message_text += f" brief_ref={request_digest}"
  Issue: brief_ref carries the bare sha256: payload digest, which VerifyingArtifactStore.load cannot resolve (it needs the $CF: reference the journal entry also carries as artifact_ref), and the skill never mentions brief_ref.
  Severity: nit
  Fix: Pass the entry's artifact_ref and add one line to the skill's "Your brief" section saying how to load it, or drop the token until lane 5 supplies a real brief.

• File: PR body, "Testing" section
  Code: "710 passed"
  Issue: The body predates the fix commit; the 21-file row now yields 714 passed and the body says nothing about the round-1 fix commit or the Risk 5 reframe.
  Severity: nit
  Fix: Refresh the count and add a "Round-1 review fixes" line naming the reframed --acknowledge-unknown residual.

Rubric

• 1. Plan vs. implementation match — pass — every Success Criterion delivered; Data Flow step 5 gap recorded as tech debt.
• 2. New code quality — fail — unguarded archive-supplied key in import_namespace, unguarded store write, contradictory comment, 15 review-history comments.
• 3. Test coverage — fail — B2 artifact path, export digests, TD4 adapter behaviors, reason write, reconcile dead-letter all green under mutation; propose tests write the production store.
• 4. Regression risk to existing callers — pass — _record_terminal_dead_letter neutral for both session_health.py callers; admitted inert; 714 lane tests green.
• 5. Data integrity — pass — head state seeded, empty clobber refused (mutation red), unit-2 and pause round-trip.
• 6. Security — pass — fixed-argv subprocess only; OP_CACHE=false; no request-derived shell input.
• 7. Documentation accuracy — fail — improvement-controller.md:42, pipeline-dead-letters.md:49, session_lifecycle.py:273-279.
• 8. PR body accuracy — fail — test count stale; no mention of the fix commit.
• 9. Disclosed deferrals — pass — six verified.
• 10. Follow-up claims verified — pass — Session execution lease: fence worker ownership with a renewable Redis lease #3220 OPEN, hand-off comment present.

Pre-Verdict Checklist

• 1. All plan acceptance/success criteria validated against diff — PASS
• 2. No-Gos from plan — none violated — PASS — scope diff on the six guarded paths is 0 lines.
• 3. New except Exception blocks — PASS — _current_case_state documents its swallow; cmd_doctor's guard prints and returns 2.
• 4. New integration tests — exercise serialization boundary — PASS — real valor-improve subprocess; 5 passed, 1 skipped.
• 5. Plan internal consistency — PASS
• 6. No hardcoded secrets or debug artifacts — PASS
• 7. New public APIs — docstrings present — PASS
• 8. Breaking changes — migration path documented — N/A
• 9. Tests added for new behavior — FAIL — artifact store write, export digests, charter drift skip, working_dir, message text, reason write, reconcile dead-letter have none.
• 10. Tests cover the failure path — PASS — outage drill, isolation, no-worker publish all red under mutation.
• 11. UI changes — screenshot captured — N/A for judge (parent gate).
• 12. Docs updated for user-facing changes — FAIL — rubric 7.

Verification

• ruff check . exit 0; ruff format --check . exit 0 (1597 files) at e8dcaf8ce.
• 21-file lane row: 714 passed; integration: 5 passed, 1 skipped.
• Verification table: 28 parsed rows, 6 malformed, every row passes by hand.
• 17 mutations applied, run, restored; 9 red, 8 green (the green ones are the untested fix paths listed above, plus OP_CACHE which the recording-runner tests cannot reach by design).

[tomcounsell]

Review (Judge risk): Changes Requested

Independent judge, dispatched as its own subagent with no view of the code-quality judge's output. Reviewed at head e8dcaf8ce (rebased onto main b13dc8ad3). Verdict: CHANGES REQUESTED, 1 blocker, 6 tech debt, 5 nits. Every fence held under mutation; the blocker is a test gap on a fix whose behavior was verified working.

Round-1 disposition (located by symbol at e8dcaf8ce)

• B1 closed: journal.py:109 seeds state via HSETNX from _current_case_state (:205-218); projection.py:59-73 refuses the empty clobber. Removing the seed → 1 failure. Disabling apply's guard → 18/18 green (guard unreached; tech debt below). Residual: the head state is a one-time snapshot (tech debt below).
• B2 partial: tools/improvement.py:134-140 writes through VerifyingArtifactStore before the lease, :157-159 journals artifact_ref, :177 keeps it on the refusal evidence row, export.py:76-92 populates artifacts.json. Behavior verified with a probe on both paths (store.load(ref) returns the payload bytes; refusal evidence detail is a loadable $CF: ref). Replacing store.save with artifact_ref = "" → 24/24 green across test_improvement_cli.py, test_improvement_control_export.py, and the integration file. Blocker below.
• B3 closed: tools/improvement.py:328-344 one guard over the ORM query and every read_head/list_intents; drill at tests/unit/test_improvement_cli.py:187-227. Re-raise inside the guard → 1 failure.
• TD1 closed (action_type journaled and read; forcing "investigate" → 1 failure). TD2 partial (intents.py:224-227 HSETs reason and HDELs the slot inside _LUA_MOVE_INTENT; dropping the HDEL → 2 failures; dropping the reason write → 76/76 green). TD3 partial (recovery.py:229-233 calls dead_letter_exhausted on every branch, one writer, replayable=False; deleting the call → 62/62 green). TD4 partial (real working_dir, digests into admit, soft charter check, /improve-research message; disabling the charter check → 9/9 green; working_dir/message_text recorded and never asserted). TD5 closed (publish exactly once, never on no-worker, exact override keys, test_tick_isolates_one_bad_case). TD6 closed (ns_pause and unit2 round-trip; dropping the unit2 restore → 1 failure). TD7 closed (injected clock; wall clock → 1 failure).
• N1 through N6 closed. N7 closed as accepted residual: plan Risk 5 (:372) states the --acknowledge-unknown gate is not implemented in this lane; it was a mitigation sentence, never a Success Criterion; no doc claims it. Its visibility gap in budget is tech debt below.

Blockers

• File: tools/improvement.py:134-140 (verified: read this file; mutation-checked)
  Code: store = VerifyingArtifactStore() ... artifact_ref = store.save(payload_bytes, key=artifact_key, model_class_name="ImprovementProposal")
  Issue: Round 1's Blocker 2 Fix required "a test that an accepted propose leaves the payload loadable by digest and a refused one leaves it as evidence". No such test exists: replacing the store write with artifact_ref = "" leaves tests/unit/test_improvement_cli.py, tests/unit/test_improvement_control_export.py, and tests/integration/test_improvement_control_cli.py at 24 passed. The entire blocker fix (store write, journaled artifact_ref, evidence detail, artifacts.json digests) is unprotected; nothing asserts artifact_ref anywhere under tests/. The behavior itself is correct (probe loaded the bytes back on both paths and went red under the same mutation).
  Severity: blocker
  Fix: Add to TestPropose*: (a) break-glass accepted propose with POPOTO_IMPROVEMENT_CONTENT_PATH monkeypatched to tmp_path, assert payload["artifact_ref"].startswith("$CF:"), VerifyingArtifactStore().load(ref) == payload_bytes, and journal_tail(...)[-1]["artifact_ref"] == ref; (b) session-bound propose against a case with no running intent, assert reason INTENT_STATE and the propose-refused: evidence row's detail loads to the payload; (c) one export test asserting artifacts.json["digests"] carries that ref.

Tech Debt

• File: tools/improvement.py:146-192 and docs/features/improvement-controller.md:42 (verified: read both; no production caller of apply( under tools/, reflections/, ui/data/)
  Code: result = transition(...) ... _emit(args, f"accepted: revision={result.revision}", ...) with no projection.apply call
  Issue: Data Flow step 5 is not wired: apply is called only by replay. ImprovementCase.revision never advances in production and the doc's "queryable projection, updated after the journal commits" is not what ships. No decision reads the projection, so no fence is affected.
  Severity: tech_debt
  Fix: Call projection.apply(PROJECT_KEY, case_id) after an accepted transition in cmd_propose/cmd_propose_amendment, or correct the doc to say the projection is reconciled only by replay-projection.

• File: tools/improvement_control/journal.py:103-109 (verified: read this file)
  Code: redis.call("HSETNX", head_key, "state", case_state)
  Issue: The head's state is a one-time snapshot of ImprovementCase.state at the first transition and no journal event ever changes it (KNOWN_EVENTS has no lifecycle event). The doc and plan call the head "the authority for case state", but the only writer of lifecycle state will be a later lane writing the ORM row, after which replay-projection rolls the case back to the seeded value. A row missing at seed time pins state="" forever (HSETNX never re-seeds), so case show/case explain print an empty state for that case.
  Severity: tech_debt
  Fix: Either give the journal a lifecycle event so head state has a real writer, or make apply/replay write revision only and leave state to the lane that owns lifecycle; re-seed when the stored head state is empty. State the chosen contract in docs/features/improvement-controller.md.

• File: tools/improvement_control/export.py:134-136,140-141 (verified: read this file)
  Code: for key, mapping in data.get("unit2", {}).items(): ... r.hset(key, mapping=mapping) and r.rpush(keys.journal_key(project_key, case_id), entry)
  Issue: (a) unit-2 keys are written under the raw key string from the archive: no keys.assert_control_key, no check that the archive's project_key matches the argument, and the restored hashes carry no TTL (reserve sets KEY_EXPIRY_SECONDS), so they persist indefinitely. (b) import --force onto a non-empty namespace appends journal entries with RPUSH after overwriting the head, producing a journal with duplicated history whose fold can no longer reach the head.
  Severity: tech_debt
  Fix: Rebuild unit-2 keys from project_key + the archived suffix (or assert the archive's project_key and assert_control_key(key)), re-apply EXPIRE, and on force delete each case's journal/intents/intent hashes before restoring.

• File: tools/improvement.py:422,437 (verified: read this file)
  Code: unknown_receipts = [] ... "unit2_receipted_unknown": unknown_receipts
  Issue: Plan "Error State Rendering": "budget prints metering="unknown" receipts in their own block with the window they were charged to, so an unknown never reads as zero." The list is a hardcoded empty literal; budget --json always reports no unknown receipts. This is the visibility half of the accepted Risk 5 residual, so the accepted deferral currently has no operator-facing surface.
  Severity: tech_debt
  Fix: Query ImprovementEvidence(kind="spend_receipt") rows whose detail.metering == "unknown", print them in their own block with day_key, and assert one in TestBudget.

• File: tests/unit/test_improvement_cli.py:266 and tests/integration/test_improvement_control_cli.py:77 (verified: read both; observed on disk)
  Code: run_cli(["propose", ...]) / run([... "propose", ...], env={"AGENT_SESSION_ID": session_id}) with no POPOTO_IMPROVEMENT_CONTENT_PATH
  Issue: Since the B2 fix, every accepted propose in the suite writes a real artifact into the production retention root (~/.popoto/improvement_content/ImprovementProposal/, 25 test payloads today). Lane 7's export contract treats that root as durable evidence.
  Severity: tech_debt
  Fix: monkeypatch.setenv("POPOTO_IMPROVEMENT_CONTENT_PATH", str(tmp_path)) in the unit tests and pass it in the integration env=; consider a session-scoped conftest fixture so no improvement test can write the real root.

• File: tools/improvement_control/projection.py:59-73, tools/improvement_control/recovery.py:233, tools/improvement_control/scheduler_adapter.py:188-194, tools/improvement_control/intents.py:225 (verified: mutation-checked each)
  Code: if not head.state: / dead_letter_exhausted(...) / if charter_digest: ... "charter_digest_stale" / "reason", reason
  Issue: Four fix paths from this commit are unreached by any test: disabling the apply empty-state guard (18 green), deleting the recovery dead-letter call (62 green), disabling the adapter's charter check (9 green), dropping the reason write in the move script (76 green). working_dir and the /improve-research ... brief_ref= message are recorded by CountingPush.push_kwargs and never asserted.
  Severity: tech_debt
  Fix: One assertion each: apply on a head with state="" leaves the ORM state unchanged; exhaustion in reconcile writes one DeadLetter(stage="improve_intent", replayable=False) on the no-row branch; a case whose charter_digest differs from the pinned one is skipped charter_digest_stale; list_intents(...)[0].reason == "session_gone_or_terminal" after the Race 3 sweep; push.push_kwargs[0]["working_dir"] is the repo root and message_text starts with /improve-research.

Nits

• File: models/session_lifecycle.py:273-279 (verified: read this file)
  Code: # ... any caller outside this module that passes its own reserved stage (e.g. \improve_intent`, Improvement controller lane 3: control journal, fenced dispatch, and the valor-improve CLI #3215) -- is replayable**Issue:** No caller passesimprove_intenthere after the TD3 fix, andimprove_intentrows arereplayable=False, the opposite of the comment. **Severity:** nit **Fix:** Drop the improve_intent` example.

• File: tools/improvement_control/projection.py:109-111 (verified: read this file)
  Code: apply(project_key, case_id) / after = head.state
  Issue: When apply takes the empty-state branch, ReplayResult.projection_state_after still reports head.state ("") rather than the value actually left on the row.
  Severity: nit
  Fix: Re-read the case after apply or return before on that branch.

• File: tools/paid_inference_meter.py:216-219 (verified: read this file)
  Code: _redis().eval(_LUA_RELEASE_RESERVED, ...) / _redis().hincrby(window_key, "settled_cents", cents) / _redis().hset(res_key, "state", "settled")
  Issue: settle is three calls; a crash after the release and before state=settled leaves the reservation reserved, so the next sweep releases and settles it again (settled_cents double-counts). Over-counting is the conservative direction and the window is small.
  Severity: nit
  Fix: Fold the three writes into one Lua script.

• File: tests/unit/test_improvement_control_dispatch.py:143-158 (verified: read this file)
  Code: lambda s: published.append(s.session_id) ... assert len(published) == 1
  Issue: Success Criterion 7 says "after the row is pending"; the recorder captures only the id, so ordering is unasserted.
  Severity: nit
  Fix: Record s.status alongside and assert published == [(sid, "pending")].

• File: PR body, "Testing" section
  Code: **710 passed**
  Issue: The count predates the fix commit, which added tests.
  Severity: nit
  Fix: Refresh the 21-file count at e8dcaf8ce.

Rubric

• 1. Plan vs. implementation match — pass — every Success Criterion delivered or acknowledged; Data Flow step 5 gap filed as tech debt.
• 2. New code quality — pass — one script per effect, reason codes, private alias; comments explain each change.
• 3. Test coverage — fail — B2's store path unreached (24 green after deleting the write); four further fix paths unreached.
• 4. Regression risk to existing callers — pass — step 7 gated on action_id and exception-isolated; _record_terminal_dead_letter neutral; admitted additive; 243 lane tests green.
• 5. Data integrity — pass — no Popoto schema change; namespace separate; snapshot and import gaps filed as tech debt.
• 6. Security — pass — vault writer never carries the value in argv/log/result/evidence, OP_CACHE=false, template in mkdtemp unlinked in finally; no shell interpolation.
• 7. Documentation accuracy — fail — improvement-controller.md:42.
• 8. PR body accuracy — pass — claims reproduced; test count stale (nit).
• 9. Disclosed deferrals — pass.
• 10. Follow-up claims verified — pass — Session execution lease: fence worker ownership with a renewable Redis lease #3220 OPEN.

Pre-Verdict Checklist

• 1. All plan acceptance/success criteria validated against diff — PASS
• 2. No-Gos from plan — none violated — PASS — scope guard 0 lines.
• 3. New except Exception blocks — PASS
• 4. New integration tests — exercise serialization boundary — PASS — 5 passed, 1 skipped.
• 5. Plan internal consistency — PASS
• 6. No hardcoded secrets or debug artifacts — PASS
• 7. New public APIs — docstrings present — PASS
• 8. Breaking changes — migration path documented — N/A
• 9. Tests added for new behavior — FAIL — none for the B2 artifact path.
• 10. Tests cover the failure path — FAIL — apply empty-state branch, recovery dead-letter, charter-stale skip, wedge reason untested.
• 11. UI changes — screenshot captured — N/A for judge (parent gate).
• 12. Docs updated for user-facing changes — FAIL — rubric 7.

Verification

• Scope guard: git diff --stat origin/main...e8dcaf8ce -- agent/agent_session_queue.py agent/session_executor.py models/agent_session.py scripts/update/migrations.py tools/infrastructure_budget.py tools/improvement_eval/ → 0 lines.
• Integration: 5 passed, 1 skipped. Lane unit baseline (12 files): 243 passed.
• 15 mutations: M1 seed → 1 fail; M2 apply guard → 18 green; M3 doctor → 1 fail; M4 store write → 24 green; M5 move HDEL → 2 fail; M6 reason → 76 green; M7 binding ignores session id → 1 fail; M8 on_session_terminal without created_ts compare → 1 fail; M9 admit reconciliation guard → 2 fail; M10 transition generation fence → 1 fail; M11 recovery dead-letter → 62 green; M12 charter check → 9 green; M13 action_type → 1 fail; M14 wall clock → 1 fail; M15 unit2 restore → 1 fail. Every fence holds; the green ones are untested fix paths.
• Restart impact: models/session_lifecycle.py and agent/session_health.py are imported by bridge and worker; two new reflections register on /update. A worker restart on every machine is required after deploy.

[tomcounsell]

Review: Changes Requested

Mode: independent roster (2 judges): code-quality, risk. Both dispatched as separate subagents with the same brief and no view of each other's output, each mutation-checking in its own throwaway worktree. Aggregation: agent.sdlc_review_consensus.compute_consensus, rule any-blocker-wins, expected_judges=2, n=2, quorum_shortfall=false, tied=false. Per-judge comments precede this one.

Verdict: CHANGES REQUESTED. Round 2 at head e8dcaf8ce (rebased onto main b13dc8ad3; conflicts in two docs files resolved keeping both lanes' statements). All three round-1 blockers are substantively closed and mutation-verified by both judges independently (seed removal, doctor re-raise, and the action_type/HDEL/round-trip/clock fixes all go red). Two blockers remain: one code blocker both judges found independently (risk rated it a blocker, code-quality tech debt; any-blocker-wins takes the blocker), and one visual-proof gate blocker that is an environment condition, not a code defect. The deduplicated list below is what /do-patch should work from; the per-judge comments carry full reasoning and reproduction.

Parent-reproduced gates (Hard Rule 10): ruff check . clean; ruff format --check . clean (1597 files); plan ## Verification table 28 parsed rows, 6 parser-FAIL rows each verified passing by hand (grep -c exit-1-on-zero for rows 2 and 4; -E alternation gives 2 for the vocabulary row; record_receipt( at tools/cross_vendor_judge.py:275-276 carries project_key="valor"; result_digest has one HSET writer at journal.py:96; models/redis_lease.py absent with #3220 OPEN); touched-test selection plus adjacent suites 803 passed, 1 skipped at -n 6 after the rebase. Judges reproduced: 21-file lane row 714 passed; integration 5 passed, 1 skipped (op auth absent by design); scope guard on the six No-Go paths 0 lines.

Blockers

• File: tools/improvement.py:134-140 (verified by both judges; mutation-checked by both)
  Code: store = VerifyingArtifactStore() ... artifact_ref = store.save(payload_bytes, key=artifact_key, model_class_name="ImprovementProposal")
  Issue: Round 1's Blocker 2 Fix required a test that an accepted propose leaves the payload loadable and a refused one leaves it as evidence. None was added: replacing store.save with artifact_ref = "" leaves tests/unit/test_improvement_cli.py, tests/unit/test_improvement_control_export.py, and tests/integration/test_improvement_control_cli.py fully green (24 passed under risk, 19 under code-quality); disabling the export digest walk is also green. Nothing under tests/ asserts artifact_ref. The behavior itself works (risk's probe loaded the bytes back on both paths and went red under the same mutation).
  Severity: blocker
  Fix: In TestPropose*: (a) break-glass accepted propose with POPOTO_IMPROVEMENT_CONTENT_PATH monkeypatched to tmp_path; assert payload["artifact_ref"].startswith("$CF:"), VerifyingArtifactStore().load(ref) == payload_bytes, and journal_tail(...)[-1]["artifact_ref"] == ref. (b) Session-bound propose against a case with no running intent; assert reason INTENT_STATE and that the propose-refused: evidence row's detail loads to the payload. (c) One export test asserting artifacts.json["digests"] carries that ref.

• File: ui/templates/improvement/control.html:9 (UI file in the diff without visual proof this round)
  Code: <h4>Control</h4>
  Issue: UI files changed and no browser-MCP screenshot was captured in this round: the BYOB MCP server failed to connect in the reviewing session (Hard Rule 7 / visual proof gate). Round 1 captured generated_images/pr-3315/01_dashboard_control_panel.png at bd3e49e46; the only template change since is this heading. This is an environment condition, not a code defect, and /do-patch cannot close it.
  Severity: blocker
  Fix: The next review round must run from a session whose BYOB MCP is connected (a human reload of the extension at chrome://extensions revives it), start the dashboard on a spare port from the PR branch, capture the #improvement-control-panel with the heading, and stop it by PID.

Tech Debt

• File: tests/unit/test_improvement_cli.py:266 and tests/integration/test_improvement_control_cli.py:77,140 (both judges)
  Code: run_cli(["propose", ...]) / run([... "propose", ...], env={"AGENT_SESSION_ID": session_id}) with no POPOTO_IMPROVEMENT_CONTENT_PATH
  Issue: Every accepted propose in the suite writes a real artifact into the production retention root; ~/.popoto/improvement_content/ImprovementProposal/ holds 25 test payloads from today's builder, reviewer, and judge runs. Lane 7's export contract treats that root as durable evidence.
  Severity: tech_debt
  Fix: monkeypatch.setenv("POPOTO_IMPROVEMENT_CONTENT_PATH", str(tmp_path)) in every test reaching cmd_propose, pass it through the integration subprocess env=, consider a session-scoped conftest fixture, and delete the 25 leaked *-a1-*.txt files.

• File: tools/improvement.py:146-192, tools/improvement_control/projection.py:38, docs/features/improvement-controller.md:42 (both judges)
  Code: result = transition(...) with no projection.apply call / "queryable projection, updated after the journal commits"
  Issue: projection.apply has no production caller (cmd_propose, cmd_pause, cmd_resume, cmd_propose_amendment never call it; only replay() does). Plan Data Flow step 5 is unimplemented; ImprovementCase.revision stays 0 until an operator runs replay-projection; the doc sentence is false.
  Severity: tech_debt
  Fix: Call projection.apply(PROJECT_KEY, case_id) after each accepted transition in the CLI and assert reloaded.revision == result.revision in the propose test, or state in the doc and the plan's Data Flow that the projection is reconciled only by replay-projection.

• File: tools/improvement_control/scheduler_adapter.py:179-222, tools/improvement_control/intents.py:225, tools/improvement_control/recovery.py:230-233, tools/improvement_control/projection.py:59-73 (both judges; every path mutation-checked green)
  Code: if charter_digest: ... "charter_digest_stale" / working_dir=_project_root() / message_text = f"/improve-research ..." / admit(..., request_digest, charter_digest) / "reason", reason / dead_letter_exhausted(...) / if not head.state:
  Issue: Seven fix paths from the round-1 commit are unreached by any test: disabling the charter-drift check (9 green), reverting working_dir to "." plus the old message text (green), blanking both digests into admit (green), dropping the reason HSET in the move script (76 green), deleting the reconcile dead_letter_exhausted call (62 green), and disabling apply's empty-state guard (18 green). CountingPush.push_kwargs records working_dir and message_text and nothing reads them.
  Severity: tech_debt
  Fix: One assertion each: happy-path dispatch asserts push.push_kwargs[0]["working_dir"] is an absolute existing path and message_text.startswith("/improve-research "), and that admit received the case's charter_digest and the entry's payload_digest; a charter_digest_stale skip test (pin a charter, seed a case with another digest); list_intents(...)[0].reason == "session_gone_or_terminal" after the Race 3 sweep; a DeadLetter(stage="improve_intent", replayable=False) row after the third sweep on the no-row branch; apply on a head with state="" leaves the ORM state unchanged.

• File: tools/improvement_control/export.py:134-136,140-141 (both judges)
  Code: for key, mapping in data.get("unit2", {}).items(): ... r.hset(key, mapping=mapping) / r.rpush(keys.journal_key(project_key, case_id), entry)
  Issue: (a) The unit-2 restore writes the raw key string from the archive: no keys.assert_control_key (keys.py:35-43), no check that the archive's project_key matches the argument, and no TTL re-applied (reserve sets KEY_EXPIRY_SECONDS), so a hand-edited or foreign archive can HSET any key through the package's private alias and restored hashes persist indefinitely. (b) import --force onto a non-empty namespace RPUSHes journal entries after overwriting the head, duplicating history so the fold can no longer reach the head.
  Severity: tech_debt
  Fix: Build the unit-2 prefix from project_key, refuse (ImportRefusal("FOREIGN_KEY")) any key outside it, wrap with keys.assert_control_key, re-apply EXPIRE; on force, delete each case's journal/intents/intent hashes before restoring. Round-trip test with one foreign key in the archive.

• File: tools/improvement_control/journal.py:103-109 (risk)
  Code: redis.call("HSETNX", head_key, "state", case_state)
  Issue: The head's state is a one-time snapshot at the first transition; no journal event changes it (KNOWN_EVENTS has no lifecycle event). The doc and plan call the head "the authority for case state", but the only lifecycle writer will be a later lane writing the ORM row, after which replay-projection rolls the case back to the seeded value. A row missing at seed time pins state="" forever (HSETNX never re-seeds), so case show/case explain print an empty state.
  Severity: tech_debt
  Fix: Either give the journal a lifecycle event so head state has a real writer, or make apply/replay write revision only and leave state to the lane that owns lifecycle; re-seed when the stored head state is empty. State the chosen contract in docs/features/improvement-controller.md.

• File: tools/improvement.py:422,437 (risk)
  Code: unknown_receipts = [] ... "unit2_receipted_unknown": unknown_receipts
  Issue: Plan "Error State Rendering" says budget prints metering="unknown" receipts in their own block so an unknown never reads as zero. The list is a hardcoded empty literal; budget --json always reports none. This is the visibility half of the accepted Risk 5 residual, which therefore has no operator-facing surface.
  Severity: tech_debt
  Fix: Query ImprovementEvidence(kind="spend_receipt") rows with detail.metering == "unknown" for the project, print them with day_key, assert one in TestBudget.

• File: models/session_lifecycle.py:273-279, docs/features/pipeline-dead-letters.md:49, docs/features/improvement-controller.md:505 (code-quality tech debt; risk nit)
  Code: # ... any caller outside this module that passes its own reserved stage (e.g. \improve_intent`, Improvement controller lane 3: control journal, fenced dispatch, and the valor-improve CLI #3215) -- is replayable/| `improve_intent` | Reserved for the improvement control plane (Recursive self-improvement controller: Valor owns the discover, investigate, experiment, evaluate, release loop #3177) | — |**Issue:** After the TD3 fix no caller passesdead_letter_stage="improve_intent"and the lane writes that stagereplayable=Falseviadead_letter_exhausted. The lifecycle comment names a caller and a replayability that no longer exist; the dead-letters doc still lists the stage as reserved with no replayable value; improvement-controller.md:505attributes the dead letter tofinalize_session. **Severity:** tech_debt **Fix:** Drop the improve_intent example from the lifecycle comment; update the doc row to "Exhausted improvement dispatch intent (recovery.reconcile)" / no; attribute the dead letter to dead_letter_exhausted` on every branch.

Nits

• File: tools/improvement_control/intents.py:192-193 (code-quality)
  Code: #: ... \from_state` is derived by the Python wrapper from `_ALLOWED`, never chosen by the caller.**Issue:** Contradicts_move's docstring at :477-480 and the PR's disclosed deviation. **Severity:** nit **Fix:** "from_state` is passed explicitly by the wrapper and CAS-checked by the script."

• File: tools/improvement_control/scheduler_adapter.py:99-101 plus 14 sibling sites (git grep "#3315 review" = 15 hits in tools/), and tools/paid_inference_meter.py:351-352 (code-quality)
  Code: # Tech debt fix (#3315 review): ... the fallback stays for any entry written before that change landed.
  Issue: Fifteen production comments narrate review history for code that has never shipped; the meter says a module "may not yet exist in every deployment" though it lands in this PR. CLAUDE.md principle 1: describe only the new status quo.
  Severity: nit
  Fix: Rewrite each comment to state the invariant; drop the review references and the never-existing-entry fallback language.

• File: tools/improvement_control/projection.py:109-111 (both judges)
  Code: after = head.state
  Issue: On the empty-state branch apply leaves case.state unchanged but replay reports projection_state_after="", so replay-projection --json misreports exactly when the guard fires.
  Severity: nit
  Fix: after = head.state or (before or ""), or re-read the case after apply.

• File: tools/improvement.py:138-140 (code-quality)
  Code: artifact_ref = store.save(payload_bytes, key=artifact_key, model_class_name="ImprovementProposal")
  Issue: The one filesystem write in cmd_propose is unguarded while every other boundary returns a reason code; a full disk or missing content root tracebacks with exit 1 and no --json refusal.
  Severity: nit
  Fix: except OSError as e: _emit(args, f"refused: ARTIFACT_WRITE_FAILED ({e})", {"accepted": False, "reason": "ARTIFACT_WRITE_FAILED"}); return 1.

• File: tools/improvement_control/scheduler_adapter.py:220-222 and .claude/skills/improve-research/SKILL.md:15-29 (code-quality)
  Code: message_text += f" brief_ref={request_digest}"
  Issue: brief_ref carries the bare sha256: payload digest, which VerifyingArtifactStore.load cannot resolve (it needs the $CF: artifact_ref the journal entry also carries), and the skill never mentions brief_ref, so the token has no consumer.
  Severity: nit
  Fix: Pass the entry's artifact_ref and add one line to the skill's "Your brief" section on loading it, or drop the token until lane 5 supplies a real brief.

• File: tools/paid_inference_meter.py:216-219 (risk)
  Code: _redis().eval(_LUA_RELEASE_RESERVED, ...) / _redis().hincrby(window_key, "settled_cents", cents) / _redis().hset(res_key, "state", "settled")
  Issue: settle is three calls; a crash after the release and before state=settled lets the next sweep release and settle again, double-counting settled_cents. Over-counting is the conservative direction and the window is small.
  Severity: nit
  Fix: Fold the three writes into one Lua script.

• File: tests/unit/test_improvement_control_dispatch.py:143-158 (risk)
  Code: lambda s: published.append(s.session_id) ... assert len(published) == 1
  Issue: Success Criterion 7 says "after the row is pending"; the recorder captures only the id, so ordering is unasserted.
  Severity: nit
  Fix: Record s.status alongside and assert published == [(sid, "pending")].

• File: PR body, "Testing" section (both judges)
  Code: **710 passed**
  Issue: The count predates the fix commit (the 21-file row now yields 714) and the body says nothing about the round-1 fix commit or the Risk 5 reframe.
  Severity: nit
  Fix: Refresh the count; add a "Round-1 review fixes" line naming the reframed --acknowledge-unknown residual.

Miscellaneous

• None

Acknowledged Deferrals (verified)

• --acknowledge-unknown gate — plan Risk 5 (docs/plans/improvement-controller-lane-3-control-journal-fenced-dispatch.md:372) reframes it as accepted residual scope with the charter §8 receipt retained; it was never a Success Criterion and no doc claims the gate exists. Both judges accepted the reframe; the budget visibility gap is filed above as tech debt.
• dead_letter_stage is improve_intent — explicit deviation, still sound; now written by dead_letter_exhausted.
• Exact-match ImprovementCase.query.filter() with per-state loops — unchanged and consistent across adapter, recovery, doctor, dashboard.
• Explicit from_state in _move() — sound; the stale comment is nit 1.
• 6 malformed Verification rows — re-verified by hand at e8dcaf8ce.
• Unit-2 reserve/release unfenced by the case lease — deliberate scope boundary, unchanged.
• Session execution lease: fence worker ownership with a renewable Redis lease #3220 hand-off — Session execution lease: fence worker ownership with a renewable Redis lease #3220 OPEN, comment 5665248148 present.

Review Delta (vs prior review on HEAD bd3e49e)

• Resolved: B1 (head state seeded; empty clobber refused), B3 (doctor guard; drill test), TD1, TD5, TD6, TD7, N1, N2, N4, N5, N6; N7 by explicit plan reframe.
• Partially resolved, now tech debt: B2 (behavior implemented, untested; promoted to the sole code blocker), TD2 (reason write untested), TD3 (reconcile dead-letter call untested), TD4 (all four adapter behaviors untested), N3 (one stale from_state comment remains).
• New: production content-store leak from propose tests; projection.apply has no production caller; head state one-time snapshot; import raw-key / TTL / --force journal append; budget unknown-receipt block hardcoded empty; stale dead-letter comment and doc row; review-history comments; unguarded store write; brief_ref not loadable; non-atomic settle; publish-ordering unasserted; PR body count stale.
• Unchanged: none carried verbatim.

Verification Results

• Rebase: 16 commits replayed onto b13dc8ad3; conflicts in docs/features/improvement-controller.md (two hunks) and docs/plans/critiques/recursive-self-improvement-capability-matrix.md resolved keeping lanes 3 and 4 both as shipped. Pushed with --force-with-lease; mergeable=MERGEABLE, mergeStateStatus=CLEAN.
• Parent: ruff check . clean; ruff format --check . clean; Verification table 28 rows, 6 parser-FAIL rows all verified passing by hand (values above); touched-test selection plus adjacent suites 803 passed, 1 skipped.
• Judges: 21-file lane row 714 passed; 12-file lane baseline 243 passed; integration 5 passed, 1 skipped; scope guard 0 lines. 32 mutations across both judges: every fence and every round-1 closed fix goes red; the green mutations are exactly the untested fix paths listed under Blockers and Tech Debt.
• Bridge/worker impact: models/session_lifecycle.py and agent/session_health.py are imported by both bridge and worker; two new reflections register on /update. A worker restart on every machine is required after deploy.

Screenshots

• None captured this round: BYOB MCP failed to connect in the reviewing session (see blocker 2). Round 1's generated_images/pr-3315/01_dashboard_control_panel.png at bd3e49e46 predates the <h4> heading.

[valorengels]

Review: Changes Requested

Idempotent: prior review on HEAD e8dcaf8 / body hash a0a9030 is still valid — returning the prior verdict without regenerating findings.

Mode: inherited from the round-2 independent roster (2 judges) run (code-quality, risk; compute_consensus, any-blocker-wins, expected_judges=2, n=2, quorum_shortfall=false, tied=false). No judges were re-dispatched this round: the Prior Review Context idempotency check (code-review.md § 2.6 Step C) fired on an exact match of both inputs, so re-running the roster could only reproduce the same findings at cost.

Idempotency evidence

• Current PR head: e8dcaf8ce0d2313d972cce61824fc9d855783d97
• Prior review's embedded head_sha: e8dcaf8ce0d2313d972cce61824fc9d855783d97 — match
• Current PR body SHA-256: a0a90307ca563800edb87e1319397664879e2ac967f17cd167112d643379ad64
• Prior review's embedded pr_body_hash: a0a90307ca563800edb87e1319397664879e2ac967f17cd167112d643379ad64 — match
• Mergeability preflight re-run this round: state=OPEN, mergeable=MERGEABLE, mergeStateStatus=CLEAN — unchanged, no short-circuit.
• Base main is still b13dc8ad3, the commit the round-2 rebase targeted, so the merge context the judges reviewed against is also unchanged.

Carried-forward verdict: CHANGES REQUESTED — 2 blockers, 7 tech debt, 9 nits, exactly as enumerated in the round-2 aggregate review (comment 5667717695) and its two per-judge comments (code-quality, risk). That comment is the authoritative finding list for /do-patch; it is not restated here, because restating it would fork the list /do-patch works from.

The two blockers, in one line each for routing purposes:

1. tools/improvement.py:134-140 — the artifact_ref round-trip has no test; store.save can be stubbed to "" and the whole suite stays green. A code defect /do-patch can close.
2. ui/templates/improvement/control.html:9 — visual-proof gate (Hard Rule 7): UI files are in the diff and no browser-MCP screenshot was captured in round 2 because BYOB failed to connect. This is an environment condition, not a code defect, and /do-patch cannot close it. It needs a review round run from a session with a live BYOB MCP connection.

Because blocker 2 is not patchable, the next round should pair the /do-patch pass on blocker 1 and the tech debt with a reviewing session whose BYOB MCP is connected; otherwise the same non-code blocker will carry forward again on the next idempotency-busting head.

Blockers

• See the round-2 aggregate review, comment 5667717695, § Blockers (2). Carried forward unchanged.

Tech Debt

• See the round-2 aggregate review, § Tech Debt (7). Carried forward unchanged.

Nits

• See the round-2 aggregate review, § Nits (9). Carried forward unchanged.

Miscellaneous

• None

Acknowledged Deferrals (verified)

• Carried forward unchanged from the round-2 aggregate review, § Acknowledged Deferrals (7 entries, including the Session execution lease: fence worker ownership with a renewable Redis lease #3220 hand-off verified OPEN).

Review Delta (vs prior review on HEAD e8dcaf8)

• Resolved: none — the head SHA has not moved since the round-2 review.
• New: none.
• Unchanged: all 18 findings (2 blockers, 7 tech debt, 9 nits).

Verification Results

• Not re-run. Re-running the plan's ## Verification table against an identical tree and an identical base would reproduce the round-2 results by construction; those results stand as recorded in the round-2 aggregate review.

Screenshots

• None captured this round. The round-2 visual-proof blocker is carried forward unchanged, not re-cleared.

[valorengels]

Review: Changes Requested

Mode: sequential lenses (Agent tool unavailable: not in tool list). This stage runs inside a supervisor-dispatched Agent and the Agent tool is absent from this context's tool list, so the declared two-judge roster (code-quality, risk) could not be dispatched. Both lenses were applied by one reviewer in sequence. Recorded as a quorum shortfall (judges_run=1, expected_judges=2), not as agreement between judges — per the roster contract in docs/sdlc/do-pr-review.md and #3198.

Head judged: 93def1ecbc715bc74318c90df6e4f57816153aca (resolved via tools/pr_head_resolver.py, not a bare gh read). Base main b13dc8ad3. Preflight: state=OPEN, mergeable=MERGEABLE, mergeStateStatus=CLEAN — no short-circuit.

Round 3 summary. All 18 round-2 findings are closed and independently mutation-verified. The round-2 visual-proof blocker is discharged this round — BYOB was live, the dashboard was started from this worktree, and the control panel was captured in both its empty and its populated branch. It is no longer a carry-forward obstacle.

One new blocker surfaced, and it surfaced precisely because this machine has what the two prior rounds lacked: a working non-interactive op service account. tests/integration/test_vault_write_integration.py skipped in rounds 1 and 2 ("op auth absent by design"), so the vault writer's single contract with the external tool had never actually been exercised. Run here, it fails — and the defect is that the writer cannot create an item at all.

Blockers

• File: tools/vault_write.py:51 (verified: read the file, reproduced the failure and the fix against the real op CLI)
  Code: category: str = "API Credential",
  Issue: The default category is 1Password's display name, but op item create --template requires the template's enum form. op item template get "API Credential" returns "category": "API_CREDENTIAL". With the shipped default, op rejects every call:
  [ERROR] "API Credential" is an unknown item type: "API Credential" isn't a recognized item type.
  so write_credential — described in the PR as "the one sanctioned op item create path" and the whole of plan Task 9 — returns state="refused" on every invocation of its default (and only production) path. Reproduced both directions on this machine: the shipped default refuses; category="API_CREDENTIAL" returns state="created" and the item was created in m-valor and deleted again. Integration result at this head: tests/integration/test_vault_write_integration.py 1 failed (assert 'refused' == 'created'), not "1 skipped".
  Nothing caught this because every unit test in tests/unit/test_vault_write.py injects a fake runner, so no test ever compares the category against op's real vocabulary; the integration test is the only real-boundary check and it had never run.
  Severity: blocker
  Fix: Default to the enum form (category: str = "API_CREDENTIAL"), and pin it with a unit test asserting the template dict's category is the enum spelling so a display-name regression goes red without needing op. Confirm the integration test passes on a machine with the valor-local service account before calling Task 9 done.

Tech Debt

• File: tools/improvement_resources.py:198-201 (verified: read the file)
  Code: if _VAULT_WRITER.exists(): return _entry("verified", "the sanctioned vault writer is present")
  Issue: The charter §8 resource probe reports verified on file existence alone. With the blocker above live, doctor/probe reports the vault-write capability as verified while every write refuses — the exact "counts a capability the system does not have" failure the improvement charter's own framing warns against. A probe that cannot distinguish "present" from "works" is the surface that let the blocker stay invisible for two rounds.
  Severity: tech_debt
  Fix: Either downgrade the wording to present (the honest claim for an existence check), or make the probe assert the template category against op item template get when op is authenticated and return unknown otherwise.

• File: tests/integration/test_vault_write_integration.py:33-38 (verified: read the file, ran it)
  Code: pytestmark = pytest.mark.skipif(not _op_available(), reason="op CLI not authenticated non-interactively ...")
  Issue: The guard is correct, but nothing in the suite or the PR distinguishes "this test was skipped" from "this test would have failed". The PR's Testing section reports the skip as an expected, benign outcome, which is how a hard failure in the one real-boundary test travelled two review rounds as a green line. This is the op-dependent counterpart of Hard Rule 10: a skip is not evidence.
  Severity: tech_debt
  Fix: Add a unit-level assertion on the template's category value (no op needed) so the contract is pinned unconditionally, and state in the PR body when the integration row was skipped rather than passed.

Nits

• File: tools/improvement.py:20-21 (verified: grepped every call site)
  Code: def _project_key(args) -> str: return getattr(args, "project_key", None) or PROJECT_KEY
  Issue: Dead code. No caller anywhere in tools/ or tests/, and no --project-key argument is registered on any subparser — every command hardcodes the module-level PROJECT_KEY. CLAUDE.md principle 1 (no legacy code tolerance).
  Severity: nit
  Fix: Delete it, or register the flag and route the commands through it.

• File: ui/templates/improvement/control.html:9 (verified: read the template, compared siblings, confirmed in the captured screenshot)
  Code: <h4>Control</h4>
  Issue: The panel title is the same heading level as its own five subsections (Lane slots, Intents by state, Paused heads, Reconciliation required, Unit 2: paid inference), so in the rendered page "Control" reads as a peer of "Lane slots" rather than their parent. The sibling panels (goals.html, intervention_burden.html) carry no panel-title heading at all — their first <h4> is already a section heading. Visible in the screenshot below.
  Severity: nit
  Fix: Drop the title (matching the siblings) or promote it to <h3> so the hierarchy reads.

• File: tools/improvement_control/export.py:151 (verified: read the file)
  Code: r.expire(key, KEY_EXPIRY_SECONDS) immediately after r.hset(keys.assert_control_key(key), mapping=mapping)
  Issue: The line above routes through the package's stated guard idiom; this one uses the raw key. Functionally identical here (the prefix was validated earlier), but the inconsistency invites a future edit that drops the guard entirely.
  Severity: nit
  Fix: r.expire(keys.assert_control_key(key), KEY_EXPIRY_SECONDS).

Miscellaneous

• None

Acknowledged Deferrals (verified)

• --acknowledge-unknown gate — plan Risk 5 reframe as accepted residual scope; the budget unknown-receipt block is now its real operator surface (round-2 TD6 closed).
• dead_letter_stage is improve_intent — explicit deviation, sound; dead_letter_exhausted is the one writer on every branch.
• Exact-match ImprovementCase.query.filter() with per-state loops — consistent across adapter, recovery, doctor, dashboard.
• Explicit from_state in _move() — sound; the contradicting comment was corrected this round.
• 6 malformed Verification rows — re-verified by hand at this head (values below).
• Unit-2 reserve/release unfenced by the case lease — deliberate scope boundary.
• #3220 hand-off — verified OPEN (Session execution lease: fence worker ownership with a renewable Redis lease).

Review Delta (vs prior review on HEAD e8dcaf8)

• Resolved (all 18 round-2 findings):
  • B1 artifact_ref round-trip — closed and mutation-verified. Blanking artifact_ref after store.save now goes red on three tests (test_propose_break_glass_without_agent_session_id, test_propose_refused_by_intent_state_keeps_the_payload_as_evidence, test_export_artifacts_index_carries_the_proposal_reference); previously green. File restored, tree clean.
  • B2 visual-proof gate — discharged this round (screenshots below).
  • TD1 propose tests write to a tmp content root (POPOTO_IMPROVEMENT_CONTENT_PATH set in both the unit and the integration fixture).
  • TD2 projection.apply now has four production callers (tools/improvement.py:188,240,291,337 via _project).
  • TD3 all seven previously-unreached adapter/recovery/projection fix paths are now asserted (working_dir absolute+existing, message_text prefix and brief_ref suffix, charter_digest_stale skip, session_gone_or_terminal reason, improve_intent DeadLetter, empty-head-state guard).
  • TD4 import hardening — FOREIGN_KEY refusal on project_key mismatch and on any unit-2 key outside the project prefix, assert_control_key on restore, TTL re-applied, and --force deletes journal/intents/intent hashes before restoring instead of appending.
  • TD5 head state contract — state_changed is a real writer (journal.set_state), empty heads re-seed instead of pinning to "" forever, and the contract is documented at journal.py:43-51 and docs/features/improvement-controller.md:318-327.
  • TD6 budget now lists metering="unknown" receipts via paid_inference_meter.unknown_receipts, asserted in TestBudget.
  • TD7 dead-letter comment and both doc rows corrected.
  • N1-N9 all closed: from_state comment corrected; git grep "#3315 review" -- tools/ is now 0 (was 15); replay's after no longer misreports on the empty-state branch; ARTIFACT_WRITE_FAILED reason code added; brief_ref carries the loadable $CF: ref and the skill documents it; settle is one Lua script; publish ordering asserted as (session_id, status); PR body count refreshed to 726 and reproduced.
• New: the vault_write category blocker and its two tech-debt companions; three nits.
• Unchanged: none.

Verification Results (Hard Rule 10 — reproduced in this environment)

• ruff check . — clean (exit 0). ruff format --check . — clean, 1597 files.
• Plan ## Verification table: 28 rows parsed, 22 PASS, 6 parser-FAIL rows all verified passing by hand at this head:
  • Status trio — session_lifecycle.py=3 (≥2), ui/data/sdlc.py=1 (≥1). Pass; the expectation is prose, not machine-comparable.
  • No second general lease — output 0, #3220 OPEN. Pass.
  • Session carries no generation — 0 matches; grep -c exits 1 on zero matches.
  • result_digest one writer — runner reported 2; by hand grep -rl 'HSET.*result_digest' tools/improvement_control/ | wc -l = 1 (journal.py:107 is the sole HSET; intents.py only HGETs). The runner's extra match is a stale __pycache__ .pyc carrying the embedded Lua source, not a second writer. Pass.
  • Two counters, two owners — first=7 (>0), second=0. Pass.
  • Judge receipt pinned — record_receipt( at tools/cross_vendor_judge.py:275-276 carries project_key="valor" across a line break, so the single-line grep cannot see it. Pass.
  • Investigation vocabulary — grep -cE (alternation needs -E) returns 2. Pass.
• Lane tests, the plan's own 21-file row: 726 passed in 51.88s — the PR's claim reproduces exactly.
• Integration: test_improvement_control_cli.py 5 passed. (It first skipped all 5 because this worktree's venv had no valor-improve console script; after uv pip install -e . --no-deps the binary materialized and the tests ran. That is a local venv-sync condition, not a code defect.) test_vault_write_integration.py 1 failed — the blocker above.
• Documentation gate: all 7 files named in the plan's ## Documentation section exist and are in the diff.
• Plan section compliance: ## Test Impact, ## No-Gos, ## Update System, ## Agent Integration, ## Documentation, ## Verification all present.
• Bridge/worker impact: models/session_lifecycle.py and agent/session_health.py are imported by both bridge and worker, and two new reflections register on /update. A worker restart on every machine is required after deploy.

Screenshots

Visual proof gate: PASSED (round-2 blocker 2 discharged). BYOB MCP was live; the dashboard was started from this worktree (UI_PORT=8521/8522 python -m ui.app) and both rendering branches of the changed template were captured, then the servers were stopped and the scratch data removed through the ORM.

• Empty branch, against live data — #improvement-control renders the <h4>Control</h4> heading and "Nothing yet, written by lane 3 when a case is admitted."
• Populated branch, against an isolated test Redis db (db 14, asserted before any write; never production) seeded with two open cases and four intents — generated_images/pr-3315/01_control_panel_populated.jpg. Renders: Lane slots: 1 slot(s) in use; Intents by state table with admitted/admitted/running/reconciliation_required rows carrying case id, action id and action type; Paused heads: No paused cases; Reconciliation required with the correct valor-improve resume --case <id> --force break-glass line; Unit 2: paid inference — Window 2026-09-14: reserved $0.00, settled $0.00. All five subsections render correctly. The only visual issue is the heading-level nit above.

Pre-Verdict Checklist

• 1. All plan acceptance/success criteria validated against diff — FAIL — Task 9's vault writer cannot create an item on its default path.
• 2. No-Gos from plan — none violated — PASS — scope guard clean on the six No-Go paths.
• 3. New except Exception blocks — each has logger/raise/swallow-ok — PASS — vault_write.py:88, :114 and the ui/data/improvement.py reader all log; swallow is intentional and documented.
• 4. New integration tests — exercise serialization boundary (not in-memory only) — FAIL — the one external-boundary integration test fails at this head.
• 5. Plan internal consistency — spike findings match task steps — PASS.
• 6. No hardcoded secrets or debug artifacts — PASS — write_credential never puts the value in argv; it goes through a 0600 temp file that is unlinked in finally, and only a sha256: fingerprint is returned.
• 7. New public APIs — docstrings present — PASS.
• 8. Breaking changes — migration path documented — N/A — no schema migration; admitted is an additive status.
• 9. Tests added for new behavior — PASS — 726 lane tests; all round-2 mutation gaps now closed.
• 10. Tests cover the failure path (not just happy path) — FAIL — vault_write's refusal paths are tested only against a fake runner, never the real op vocabulary.
• 11. UI changes (if any) — screenshot captured — PASS — both template branches captured this round.
• 12. Docs updated for user-facing changes — PASS — all 7 documentation targets updated.

Rubric

• 1. Plan vs. implementation match — fail — Task 9's vault writer is non-functional on its default path.
• 2. New code quality — pass — three nits, no structural issues.
• 3. Test coverage — fail — the op template-category contract has no passing test at any level.
• 4. Regression risk to existing callers — pass — admitted is additive; _record_terminal_dead_letter's stage coercion fix is covered.
• 5. Data integrity — pass — no schema migration needed; import now refuses foreign keys and replaces history under --force.
• 6. Security — pass — no credential in argv, logs, or the returned result; import refuses out-of-namespace keys.
• 7. Documentation accuracy — pass — the head-state contract and dead-letter rows now match the code.
• 8. PR body accuracy — fail — "Integration suite: 5 passed, 1 skipped" does not reproduce; the vault row fails where op is authenticated. The 726 test count and the ruff claims do reproduce exactly.
• 9. Disclosed deferrals — pass — all 7 sound.
• 10. Follow-up claims verified — pass — #3220 OPEN.

Verdict derivation: Rule 1 — critical items 1 and 3 fail with no matching acknowledgement → CHANGES REQUESTED — Blocker. The quorum shortfall independently forbids APPROVED this round regardless.

Is the carry-forward the sole remaining obstacle? No — and it is no longer an obstacle at all. Round-2 blocker 2 (ui/templates/improvement/control.html:9, the visual-proof gate) is discharged. The only blocker standing between this PR and approval is the new tools/vault_write.py:51 category defect, which /do-patch can close in one line plus a pinning test.

[tomcounsell]

Review (Judge code-quality): Changes Requested

Independent judge, dispatched as its own subagent (no name, foreground, own throwaway worktree) with no view of the risk judge's output. Reviewed at head f657d12eaddb83e3966fdeb6332a731e06c10b34. Lens: correctness, test honesty, code quality, plan and docs accuracy.

Verdict: CHANGES REQUESTED (0 blockers, 0 tech debt, 2 nits). Every round-2 and round-3 finding is closed, located by symbol at this head, and the five mutations run all went red.

Round-2 dispositions (16, located by symbol at f657d12)

• B1 artifact_ref round-trip: closed. tests/unit/test_improvement_cli.py::test_propose_break_glass_without_agent_session_id, ::test_propose_refused_by_intent_state_keeps_the_payload_as_evidence, ::TestExportImport::test_export_artifacts_index_carries_the_proposal_reference (the third lives in test_improvement_cli.py, not test_improvement_control_export.py). Mutation-verified below.
• B2 visual-proof gate: discharged (parent captured both branches this round; see aggregate).
• TD1 content-root leak: closed. Autouse _content_root fixture tests/unit/test_improvement_cli.py:21-26; tests/integration/test_improvement_control_cli.py:42.
• TD2 projection.apply caller: closed. tools/improvement.py::_project called from cmd_propose, cmd_propose_amendment, cmd_pause, cmd_resume; reloaded.revision == payload["revision"] asserted.
• TD3 seven unreached paths: closed. test_improvement_control_dispatch.py:194-203,222; test_improvement_control_recovery.py:110-119,155; test_improvement_control_projection.py::test_apply_on_a_head_with_empty_state_leaves_the_row_state_alone.
• TD4 import hardening: closed. export.py::import_namespace FOREIGN_KEY on project_key and unit-2 prefix, assert_control_key + expire, --force deletes journal/intents/intent hashes; test_import_refuses_a_foreign_key_before_writing_anything, test_import_force_replaces_history_rather_than_appending.
• TD5 head-state contract: closed. journal.py::KNOWN_EVENTS has state_changed; _LUA_TRANSITION writes on it and re-seeds while empty; journal.set_state; contract at journal.py:43-51, docs/features/improvement-controller.md:318-327.
• TD6 budget unknown receipts: closed. cmd_budget calls paid_inference_meter.unknown_receipts; test_budget_lists_unknown_metered_receipts_with_their_window.
• TD7 dead-letter comment/docs: closed. models/session_lifecycle.py::_record_terminal_dead_letter names no improve_intent; pipeline-dead-letters.md:49 row no; improvement-controller.md:530 attributes to dead_letter_exhausted.
• N1 from_state comment: closed (intents.py:192-194). N2 review-history comments: closed (git grep -n "#3315" -- tools/ models/ agent/ ui/ = 0). N3 replay after: closed (projection.py::replay). N4 unguarded store.save: closed (except OSError -> ARTIFACT_WRITE_FAILED, test_propose_refuses_when_the_store_write_fails). N5 brief_ref: closed (scheduler_adapter.py:231-232 sends proposal.artifact_ref; skill documents it). N6 settle single Lua: closed (_LUA_SETTLE, test_settle_is_one_script_and_a_second_settle_counts_nothing). N7 publish ordering: closed (published == [(session_id, "pending")]). N8 PR body count: closed at 93def1e, re-staled at this head (nit 2).

Round-3 dispositions (6)

• B1 category display name: closed. tools/vault_write.py::write_credential default category: str = "API_CREDENTIAL"; tests/unit/test_vault_write.py::TestTemplateCategoryIsTheEnumSpelling (2 tests), mutation-verified red.
• TD1 probe verified on existence: closed. tools/improvement_resources.py::_probe_vault_write returns unknown with "not checked" detail; test_vault_write_probe_abstains_rather_than_certifying_a_write, mutation-verified red.
• TD2 skip-vs-fail invisibility: closed. Unit pin needs no op; PR body Testing section states the skip condition.
• N1 dead _project_key: closed (0 hits). N2 <h4>Control</h4>: closed (template opens straight into the {% if %}). N3 raw key in expire: closed (export.py:152 routes through keys.assert_control_key).

Mutation log (each restored, git status --short empty before the next)

• (a) cmd_propose store.save(...) -> artifact_ref = "": RED on all three B1 tests plus test_propose_refuses_when_the_store_write_fails (4 failed, 21 passed).
• (b) refused-path detail blanked: RED on test_propose_refused_by_intent_state_keeps_the_payload_as_evidence.
• (c) export digest walk disabled: RED on test_export_artifacts_index_carries_the_proposal_reference.
• (d) vault_write.py default -> "API Credential": RED on test_default_category_is_the_enum_form_not_the_display_name.
• (e) _probe_vault_write -> verified: RED on test_vault_write_probe_abstains_rather_than_certifying_a_write.

Blockers

• None

Tech Debt

• None

Nits

• File: tools/improvement_resources.py:200-201 (verified: read this file)
  Code: # can be present while every \op item create` it makes is refused. `verified`/# was therefore a certain answer to an uncertain question, the exact harm the **Issue:** The comment narrates the superseded behavior ("verifiedwas therefore...") rather than the status quo (CLAUDE.md principle 1, the same category as round-2 N2). The sibling_probe_cloudflare_clicarries a docstring; this function carries a comment block. **Severity:** nit **Fix:** Convert to a docstring stating the invariant only: an existence check establishes that the writer is on disk and nothing more, so the probe reportsunknownwith what it established and spawns noop` call.

• File: PR Improvement controller lane 3: control journal, fenced dispatch, and the valor-improve CLI #3315 body, "Testing" section (verified: gh pr view 3315 --json body)
  Code: **726 passed** at \93def1e`/on all 49 changed Python files/ "Review rounds" list ends at93def1e**Issue:** The body describes the prior head. Atf657d12the plan's 21-file lane row is 728 passed (twoTestTemplateCategoryIsTheEnumSpellingtests added), the diff has 50 changed.pyfiles (29 added, 21 modified), and the "Review rounds" list has no round-3 entry for the vault-category fix. **Severity:** nit **Fix:** Refresh to "728 passed atf657d12", "50 changed Python files", and add a "Round 3 (f657d12`)" line naming the category enum, the probe abstention, and the three nits.

Miscellaneous

• None

Acknowledged Deferrals (verified)

• --acknowledge-unknown gate: plan Risk 5 reframe; budget's unknown-receipt block is the operator surface. Sound.
• dead_letter_stage is improve_intent: intents.dead_letter_exhausted is the one writer. Docs match.
• Exact-match ImprovementCase.query.filter() per-state loops: consistent across adapter, recovery, doctor, budget, dashboard.
• Explicit from_state in _move(): sound.
• 6 malformed Verification rows: re-verified by hand at this head (values below).
• Unit-2 reserve/release unfenced by the case lease: deliberate scope boundary.
• Session execution lease: fence worker ownership with a renewable Redis lease #3220 hand-off: gh issue view 3220 = OPEN; models/redis_lease.py absent.
• journal.set_state has no production caller by design (the lifecycle owner's one door for a later lane, journal.py:367-372); exercised by two projection tests.

Verification results (reproduced in the judge's worktree, PYTHONPATH pinned)

• ruff check . clean; ruff format --check . clean (1597 files).
• Verification table: 34 rows. 6 MALFORMED (unescaped |), all six pass by hand: no KEYS/SCAN in any Lua; lease protocol def acquire|renew|release( = 6; queue-seam diff 0 lines; intent vocab 22 / 2; OpenRouter greps 0 / 0; only-vault-writer-touches-op 0. Of 28 executed rows, 20 PASS and 8 parser-FAIL all passing by hand: status trio 3 / 1; no second general lease 0 with Session execution lease: fence worker ownership with a renewable Redis lease #3220 OPEN; session carries no generation 0 (grep -c exits 1 on zero); result_digest one writer 1; two counters 7 / 0; judge receipt project_key="valor" at tools/cross_vendor_judge.py:275-276 across a line break; investigation vocabulary grep -cE = 2; the lane-tests row timed out at the parser's 120s cap (133s here) and passed by hand.
• Lane row (21 files, plan line 714): 728 passed, 0 failed, 0 skipped, 133s, exit 0.
• test_vault_write.py + test_improvement_resources.py + test_ui_improvement_data.py: 37 passed.
• Integration: test_improvement_control_cli.py 5 passed; test_vault_write_integration.py 1 skipped (OP_CACHE=false op whoami exits 1 on this machine). Reported as skipped, never as passed.
• Docs gate: all 7 files named in the plan's ## Documentation exist and are in the diff. ## Test Impact, ## No-Gos, ## Update System, ## Agent Integration, ## Documentation, ## Verification present. No .env.example or migrations.py change, matching the plan.

Pre-Verdict Checklist

• 1. All plan acceptance/success criteria validated against diff — PASS — vault writer default path now creates; every other criterion carries its named test.
• 2. No-Gos from plan — none violated — PASS — queue-seam diff 0 lines; no child-gate bypass; judge keeps its provider.
• 3. New except Exception blocks — each has logger/raise/swallow-ok — PASS — vault_write.py:92,118, scheduler_adapter.py:140, recovery.py:121,242, ui/data/improvement.py:276,290 all log.
• 4. New integration tests — exercise serialization boundary (not in-memory only) — PASS — CLI suite runs the console binary in a subprocess against a claimed db; the op boundary test skipped here and is pinned at unit level.
• 5. Plan internal consistency — spike findings match task steps — PASS
• 6. No hardcoded secrets or debug artifacts — PASS — credential goes through a 0600 temp file unlinked in finally; only a sha256: fingerprint returns.
• 7. New public APIs — docstrings present — PASS
• 8. Breaking changes — migration path documented — N/A — admitted is additive; no schema migration.
• 9. Tests added for new behavior — PASS — 728 lane tests; 5 mutations all red.
• 10. Tests cover the failure path (not just happy path) — PASS — refused propose, store OSError, foreign-key import, stale generation, display-name category all asserted.
• 11. UI changes (if any) — screenshot captured — N/A — parent owns the visual gate.
• 12. Docs updated for user-facing changes — PASS — 7/7 documentation targets in the diff.

Rubric

• 1. Plan vs. implementation match — pass
• 2. New code quality — pass — one history-narrating comment, filed as a nit.
• 3. Test coverage — pass — round-2 and round-3 mutation gaps all closed and re-verified.
• 4. Regression risk to existing callers — pass
• 5. Data integrity — pass — no migration needed; import refuses foreign keys and replaces case history under --force.
• 6. Security — pass
• 7. Documentation accuracy — pass
• 8. PR body accuracy — fail — test count, Python-file count, and review-rounds list describe 93def1ecb (nit 2).
• 9. Disclosed deferrals — pass
• 10. Follow-up claims verified — pass — Session execution lease: fence worker ownership with a renewable Redis lease #3220 OPEN.

Verdict derivation: no critical-item fail; rubric 8 (non-critical) fails on a nit, Rule 2 yields CHANGES REQUESTED.

[tomcounsell]

Review (Judge risk): Changes Requested

Independent judge, dispatched as its own subagent (no name, foreground, own throwaway worktree) with no view of the code-quality judge's output. Reviewed at head f657d12eaddb83e3966fdeb6332a731e06c10b34. Lens: regression risk to bridge/worker/lifecycle callers, data integrity (namespaces, TTLs, Lua atomicity, CAS fencing, import/export), security, failure paths, mutation evidence.

Verdict: CHANGES REQUESTED. The judge classified its two substantive findings as tech debt; the parent promoted the first to a blocker under rubric item 5 (critical) after verifying the reproduction, so the roster-level counts are 1 blocker, 1 tech debt, 3 nits. Every round-2 and round-3 finding is closed with mutation evidence (19 of 20 mutations red; the one green is an equivalent mutant).

Round-2 dispositions (16, located by symbol at f657d12)

• B1: closed. cmd_propose writes artifact_ref on transition and on the propose-refused: detail; three named tests go red under mutation. (Third test lives in tests/unit/test_improvement_cli.py::TestExportImport.)
• B2: parent's gate (the <h4> left the template in f657d12).
• TD1: closed. Autouse _content_root fixtures in both suites; ~/.popoto/improvement_content/ImprovementProposal/ 0 files before and after every run.
• TD2: closed. _project called at four sites. TD3: closed, each of the seven paths mutation-checked red. TD4: closed, FOREIGN_KEY on project_key and unit-2 prefix, assert_control_key on hset and expire, --force deletes journal/intents/intent hashes. TD5: closed, set_state + state_changed branch + empty-state re-seed, mutations red. TD6: closed, mutation red. TD7: closed, session_lifecycle.py:273-281, pipeline-dead-letters.md:49, improvement-controller.md:530.
• N1 through N7: closed (intents.py:192-193; git grep "#3315 review" -- tools/ = 0; projection.py::replay; except OSError; scheduler_adapter.py:231-232; _LUA_SETTLE CAS; published == [(session_id, "pending")]). N8: closed as stated for 93def1e; this head yields 728.

Round-3 dispositions (6)

• B1: closed, tools/vault_write.py:55 "API_CREDENTIAL", TestTemplateCategoryIsTheEnumSpelling red under the display-name mutation. TD1: closed, _probe_vault_write returns unknown, test_vault_write_probe_abstains_rather_than_certifying_a_write red under mutation. TD2: closed. N1, N2, N3: closed (export.py:152 r.expire(keys.assert_control_key(key), ...)).

Mutation log (20; every mutation restored, git status --short empty after each)

1. cmd_propose artifact_ref = "": red on the three B1 tests (3 failed, 22 passed).
2. refused-row detail="": red on test_propose_refused_by_intent_state_keeps_the_payload_as_evidence.
3. export digest walk disabled: red on test_export_artifacts_index_carries_the_proposal_reference.
4. both FOREIGN_KEY refusals dropped: red on test_import_refuses_a_foreign_key_before_writing_anything.
5. raw key in hset + expire dropped: red on test_unit2_and_ns_pause_round_trip (TTL assertion).
6. only the assert_control_key wrap on hset dropped: ALL GREEN, equivalent mutant (the upstream FOREIGN_KEY prefix check already excludes every key the wrap would reject); defense in depth, not a gap.
7. --force pre-delete disabled: red on test_import_force_replaces_history_rather_than_appending.
8. state_changed no longer writes head state: red on test_state_changed_is_the_writer_of_head_state_and_apply_projects_it.
9. empty head state no longer re-seeded: red on test_next_accepted_write_reseeds_an_empty_head_state.
10. _LUA_SETTLE state CAS removed: red on test_settle_is_one_script_and_a_second_settle_counts_nothing.
11. unknown_receipts = []: red on test_budget_lists_unknown_metered_receipts_with_their_window.
12. charter-drift check disabled: red on test_charter_drift_between_propose_and_admit_skips_the_case.
13. working_dir=".": red on test_happy_path_admits_materializes_and_activates. 14. message_text prefix changed: red on the same. 15. both digests blanked into admit: red on the same.
14. reason HSET dropped from _LUA_MOVE_INTENT: red on test_swept_only_once_stale_sweeps_reaches_max_dispatch_attempts, test_unreleased_slot_is_freed_after_one_pass.
15. dead_letter_exhausted call removed: red on test_swept_only_once_stale_sweeps_reaches_max_dispatch_attempts.
16. apply empty-state guard disabled: red on test_apply_on_a_head_with_empty_state_leaves_the_row_state_alone.
17. vault_write.py default "API Credential": red on test_default_category_is_the_enum_form_not_the_display_name.
18. probe back to verified: red on test_vault_write_probe_abstains_rather_than_certifying_a_write.

Blockers

• File: tools/improvement_control/export.py:146 (verified: judge read the file and reproduced with a throwaway test in a claimed test db, then deleted it; parent re-read import_namespace at this head)
  Code: r.hset(keys.slots_key(project_key), mapping=data["slots"])
  Issue: import --force merges the archive's _ns:slots hash into the live one instead of replacing it, while it deletes each archived case's intent hashes (:156-160). A slot admitted after the export survives with its intent hash gone, and no release path can reach it: on_session_terminal reads foreign_holder, mark_reconciliation_required/cancel refuse INTENT_STATE, and the reconcile pass walks intents and sees nothing. Reproduced: export, admit("a1"), import --force, then on_session_terminal -> foreign_holder, hlen(slots)==1, next admit("a2") -> SLOT_EXHAUSTED. With max_concurrent_research_sessions defaulting to 1 (config/settings.py:611), dispatch stalls for the whole project and doctor/budget never show it (budget counts slots from intents, not the hash). docs/features/improvement-controller.md:372-373 says a forced restore "replaces history rather than appending to it"; the slot hash is the exception. Classified tech debt by the judge; the parent holds it at blocker because it is a reproduced, unrecoverable-without-raw-Redis wedge on a shipped operator command (rubric item 5, critical).
  Severity: blocker
  Fix: Under force, r.delete(keys.slots_key(project_key)) (and keys.pause_key) before restoring, so the archive's slots are the whole truth; add the export test "force restore drops a post-export slot" (admit after export, force import, assert hlen(slots) == len(archive slots) and a fresh admit is accepted).

Tech Debt

• File: tools/improvement.py:375 (verified: read this file; docs read at docs/features/improvement-controller.md:493-494 and docs/tools-reference.md:363)
  Code: "no paused heads, no stale intents, no outstanding reservations",
  Issue: cmd_doctor never reads _ns:slots or any unit-2 reservation hash (its only reads are read_head and list_intents), yet its clean line asserts "no outstanding reservations", both docs promise doctor prints outstanding reservations, and plan Success Criterion :490 ("doctor on a seeded paused case prints the paused head and its outstanding reservation") is half delivered: test_doctor_on_a_seeded_paused_case_prints_it asserts only case.id in out["paused"]. An operator on the break-glass path reads a certain "none" for a value the tool did not check, the same shape as the round-3 probe finding.
  Severity: tech_debt
  Fix: Read keys.slots_key (HGETALL) and the open-window unit-2 reservations and print them under a reservations key, asserted in the integration doctor test against the seeded slot; or drop the "no outstanding reservations" clause, the two doc claims, and the SC phrase.

Nits

• File: tools/improvement.py:288 (verified: read this file; journal.py::_LUA_TRANSITION for the fence)
  Code: _emit(args, f"paused: {result.accepted}", {"accepted": result.accepted})
  Issue: cmd_pause drops result.reason. On the per-case path a busy lease yields generation = generation or 0 (:274), and the transition script refuses 0 < highest_accepted as STALE_GENERATION for any case with one accepted write, so pause --case X during a controller tick prints paused: False, exits 1, and gives the operator no reason (same for PAUSED, UNAVAILABLE).
  Severity: nit
  Fix: Include "reason": result.reason in the payload and the human line; on the per-case path refuse CASE_BUSY when generation is None instead of presenting generation 0.

• File: tools/improvement.py:105 (verified: read this file and tools/improvement_control/scheduler_adapter.py:103-105)
  Code: action_id = args.action_id
  Issue: A break-glass propose (no AGENT_SESSION_ID) without --action-id is accepted (exit 0, accepted: true) and journaled with action_id="", which _unadmitted_proposal skips forever (if not action_id: return None). The proposal is silently inert; docs/tools-reference.md:357 and the skill never mention --action-id.
  Severity: nit
  Fix: On the break-glass path mint action_id = args.action_id or uuid.uuid4().hex (or refuse MISSING_ACTION_ID), and name --action-id in the tools-reference line.

• File: tools/paid_inference_meter.py:218-220 (verified: read this file)
  Code: _redis().eval(_LUA_RELEASE_RESERVED, 1, window_key, int(row["cents"])) ... _redis().hset(res_key, "state", "released")
  Issue: release is still the two-call shape round 2 folded for settle: a crash between the window decrement and state=released leaves the row reserved, so the next release or the day-close sweep subtracts the cents a second time (floored at 0). That direction under-counts reserved_cents and lets the pool admit past the cap, the opposite of settle's conservative over-count.
  Severity: nit
  Fix: Move the state == 'reserved' CAS, the decrement, and state=released into one script (_LUA_RELEASE, KEYS [window, reservation]), and assert in test_release_is_idempotent that a second release after a simulated half-crash counts nothing.

Miscellaneous

• None (advisory notes, no finding): _LUA_TRANSITION performs one idempotent SET schema_key before its fences; _LUA_ADMIT builds intent keys inside Lua, fine on single-node Redis. _record_terminal_dead_letter is a behavior-preserving pass-through for both agent/session_health.py callers; no test names it. admitted readers audited: _push_agent_session publishes a notify the worker's status="pending" scan ignores; expectation_reconciler._pm_target and sdlc_progress treat any live eng row alike, pre-existing. Worker restart per machine is needed for slot release to fire; reflection registration is idempotent (register_reflection returns noop on rerun).

Acknowledged Deferrals (verified)

• --acknowledge-unknown gate reframed as accepted residual (plan Risk 5). Sound.
• dead_letter_stage is improve_intent, replayable=False on every branch (intents.py:664-688); bridge/dead_letters.py::HANDLERS has no improve_intent entry, so replay never picks it up.
• Exact-match ImprovementCase.query.filter() per-state loops. Explicit from_state in _move(). Unit-2 reserve/release unfenced by the case lease. All sound.
• 6 malformed Verification rows verified by hand (queue seam 0; judge receipt project_key="valor" at cross_vendor_judge.py:275-276).
• Session execution lease: fence worker ownership with a renewable Redis lease #3220 hand-off: OPEN; models/redis_lease.py absent.

Verification results

• Plan "Lane tests pass" 21-file row: 728 passed. Item-6 unit set (7 files): 199 passed. CLI + export unit baseline: 25 passed.
• Integration test_improvement_control_cli.py: 5 passed (binary at .venv/bin/valor-improve). test_vault_write_integration.py: 1 skipped (op not authenticated non-interactively here). Reported as skipped, never as passed.
• Scope guards on the six No-Go paths: queue-seam diff 0 lines; models/redis_lease.py, agent/session_executor.py, tools/improvement_eval/, models/improvement_release.py, reflections/improvement_assumption_digest.py untouched; cross_vendor_judge.py base_url 0; child-gate / op / Lua-scan / Popoto-client / .env-write greps all 0; unit-3 key stays improvement:budget:unit3:.
• ~/.popoto/improvement_content/ImprovementProposal/: 0 files before, 0 after every run.
• vault_write.py: value only in the 0600 template inside a 0700 mkdtemp dir, unlinked in finally; argv carries only the path; OP_CACHE=false; no op signin; return carries only a sha256: fingerprint.
• New except Exception blocks all log or are a documented swallow (journal._current_case_state); cmd_doctor prints and exits 2.

Pre-Verdict Checklist

• 1. All plan acceptance/success criteria validated against diff — FAIL — doctor "outstanding reservation" half of plan :490 is not implemented; everything else delivered.
• 2. No-Gos from plan — none violated — PASS — all six scope guards 0.
• 3. New except Exception blocks — each has logger/raise/swallow-ok — PASS
• 4. New integration tests — exercise serialization boundary (not in-memory only) — PASS — CLI test spawns the real binary against the claimed db; vault test skipped here and reported as such.
• 5. Plan internal consistency — spike findings match task steps — PASS
• 6. No hardcoded secrets or debug artifacts — PASS
• 7. New public APIs — docstrings present — PASS
• 8. Breaking changes — migration path documented — N/A
• 9. Tests added for new behavior — PASS — 19 of 20 mutations red; the one green is an equivalent mutant.
• 10. Tests cover the failure path (not just happy path) — FAIL — import --force over a live namespace strands a slot; no test reaches it.
• 11. UI changes (if any) — screenshot captured — N/A — parent owns the visual gate.
• 12. Docs updated for user-facing changes — FAIL — improvement-controller.md:493-494 and tools-reference.md:363 claim doctor prints reservations it never reads.

Rubric

• 1. Plan vs. implementation match — fail — doctor reservations half of SC :490 (tech debt above).
• 2. New code quality — pass — three nits, no structural issues.
• 3. Test coverage — pass — all round-2/3 fixes mutation-verified; the stranded-slot path is filed under 5.
• 4. Regression risk to existing callers — pass — admitted inert to worker/health/drip; _record_terminal_dead_letter behavior-preserving; queue seam untouched.
• 5. Data integrity — fail — force-import merges the slot hash and strands post-export slots (reproduced; blocker above).
• 6. Security — pass
• 7. Documentation accuracy — fail — doctor reservation claims at two doc sites.
• 8. PR body accuracy — pass — 726 correctly attributed to 93def1e; 728 reproduces here; integration line accurate about the skip.
• 9. Disclosed deferrals — pass
• 10. Follow-up claims verified — pass — Session execution lease: fence worker ownership with a renewable Redis lease #3220 OPEN.

Verdict derivation: rubric items 1 and 5 (critical) fail with no matching acknowledgement, Rule 1 yields CHANGES REQUESTED.

[tomcounsell]

Review: Changes Requested

Mode: independent roster (2 judges): code-quality, risk. Both dispatched as separate foreground subagents (no name, awaited in-turn) with the same brief and no view of each other's output, each mutation-checking in its own throwaway worktree with the lane venv symlinked whole. Aggregation: agent.sdlc_review_consensus.compute_consensus, rule any-blocker-wins, expected_judges=2, n=2, quorum_shortfall=false, tied=false. Per-judge comments precede this one.

Head judged: f657d12eaddb83e3966fdeb6332a731e06c10b34 (resolved via tools/pr_head_resolver.py::resolve_pr_head_sha). Base main b13dc8ad3. Preflight: state=OPEN, mergeable=MERGEABLE, mergeStateStatus=CLEAN, no short-circuit. This is review round 4: the round-3 review at 93def1ecb (single sequential reviewer, quorum shortfall) was answered by f657d12ea, and this round's roster re-verified both the 16 round-2 dispositions and the 6 round-3 dispositions by symbol at the new head.

Verdict: CHANGES REQUESTED, 1 blocker, 1 tech debt, 5 nits. Every prior-round finding is closed with mutation evidence from both judges independently (code-quality: 5 mutations, all red; risk: 20 mutations, 19 red, 1 equivalent mutant). The visual-proof gate passed this round. The one blocker is new surface the risk judge reproduced in a claimed test db; the aggregator re-read export.py::import_namespace and holds it at blocker under rubric item 5 (the judge filed it as tech debt; the mechanical rule maps a critical-item fail to a blocker, and the reproduced outcome is a project-wide dispatch stall with no ORM-level recovery path). The deduplicated list below is what /do-patch should work from.

Parent-reproduced gates (Hard Rule 10): both judges reproduced ruff check . clean, ruff format --check . clean (1597 files), the plan's 21-file lane row at 728 passed (the PR body's 726 is pinned to 93def1ecb; two TestTemplateCategoryIsTheEnumSpelling tests were added since), integration test_improvement_control_cli.py 5 passed, test_vault_write_integration.py 1 skipped (OP_CACHE=false op whoami exits 1 on this machine; reported as skipped, not passed; the round-3 category fix is pinned at unit level and mutation-verified red by both judges). Verification table: 34 rows, 6 malformed by the parser and 8 parser-FAIL, all 14 verified passing by hand (values in the per-judge comments). Scope guard on the six No-Go paths 0 lines. Content root ~/.popoto/improvement_content/ImprovementProposal/ 0 files before and after every test run.

Blockers

• File: tools/improvement_control/export.py:146 (verified: risk reproduced in a claimed test db; parent re-read import_namespace at this head)
  Code: r.hset(keys.slots_key(project_key), mapping=data["slots"])
  Issue: import --force merges the archive's _ns:slots hash into the live one instead of replacing it, while :156-160 delete each archived case's intent hashes. A slot admitted after the export survives with its intent hash gone and no release path can reach it (on_session_terminal -> foreign_holder; mark_reconciliation_required/cancel refuse INTENT_STATE; the reconcile pass walks intents and sees nothing). Reproduced: export, admit("a1"), import --force, on_session_terminal -> foreign_holder, hlen(slots)==1, next admit("a2") -> SLOT_EXHAUSTED. With max_concurrent_research_sessions defaulting to 1 the whole project's dispatch stalls and doctor/budget never show it. docs/features/improvement-controller.md:372-373 says a forced restore "replaces history rather than appending to it"; the slot hash is the exception. Related: the pause_key hash at :148 has the same merge shape.
  Severity: blocker
  Fix: Under force, r.delete(keys.slots_key(project_key), keys.pause_key(project_key)) before restoring so the archive is the whole truth; add an export test "force restore drops a post-export slot" (admit after export, force import, assert hlen(slots) == len(archive slots) and a fresh admit is accepted).

Tech Debt

• File: tools/improvement.py:375 (verified: parent read cmd_doctor; docs at docs/features/improvement-controller.md:493-494, docs/tools-reference.md:363; plan :321, :490)
  Code: "no paused heads, no stale intents, no outstanding reservations",
  Issue: cmd_doctor reads only read_head and list_intents, never _ns:slots or a unit-2 reservation hash, yet its clean line asserts "no outstanding reservations", both docs promise doctor prints outstanding reservations, and plan Success Criterion :490 ("prints the paused head and its outstanding reservation") is half delivered: test_doctor_on_a_seeded_paused_case_prints_it asserts only case.id in out["paused"]. An operator on the break-glass path reads a certain "none" for a value the tool did not check, the same shape as the round-3 probe finding.
  Severity: tech_debt
  Fix: Read keys.slots_key (HGETALL) and the open-window unit-2 reservations and print them under a reservations key, asserted in the integration doctor test against the seeded slot; or drop the "no outstanding reservations" clause, the two doc claims, and the SC phrase.

Nits

• File: tools/improvement_resources.py:200-201 (code-quality)
  Code: # can be present while every \op item create` it makes is refused. `verified`/# was therefore a certain answer to an uncertain question, the exact harm the**Issue:** The comment narrates the superseded behavior rather than the status quo (CLAUDE.md principle 1); the sibling_probe_cloudflare_clicarries a docstring. **Severity:** nit **Fix:** Convert to a docstring stating the invariant only: an existence check establishes that the writer is on disk and nothing more, so the probe reportsunknownand spawns noop` call.

• File: tools/improvement.py:288 (risk)
  Code: _emit(args, f"paused: {result.accepted}", {"accepted": result.accepted})
  Issue: cmd_pause drops result.reason. On the per-case path a busy lease yields generation = generation or 0 (:274) and the transition script refuses STALE_GENERATION for any case with one accepted write, so pause --case X during a controller tick prints paused: False, exits 1, with no reason (same for PAUSED, UNAVAILABLE).
  Severity: nit
  Fix: Include "reason": result.reason in the payload and the human line; on the per-case path refuse CASE_BUSY when generation is None instead of presenting generation 0.

• File: tools/improvement.py:105 (risk)
  Code: action_id = args.action_id
  Issue: A break-glass propose (no AGENT_SESSION_ID) without --action-id is accepted and journaled with action_id="", which scheduler_adapter._unadmitted_proposal skips forever (if not action_id: return None); the proposal is silently inert and docs/tools-reference.md:357 never mentions --action-id.
  Severity: nit
  Fix: On the break-glass path mint action_id = args.action_id or uuid.uuid4().hex (or refuse MISSING_ACTION_ID), and name --action-id in the tools-reference line.

• File: tools/paid_inference_meter.py:218-220 (risk)
  Code: _redis().eval(_LUA_RELEASE_RESERVED, 1, window_key, int(row["cents"])) ... _redis().hset(res_key, "state", "released")
  Issue: release is still the two-call shape round 2 folded for settle: a crash between the window decrement and state=released lets the next release or day-close sweep subtract the cents again, under-counting reserved_cents and admitting past the cap (the opposite direction from settle's conservative over-count).
  Severity: nit
  Fix: One _LUA_RELEASE script (CAS on state == 'reserved', decrement, state=released); assert in test_release_is_idempotent that a second release after a simulated half-crash counts nothing.

• File: PR Improvement controller lane 3: control journal, fenced dispatch, and the valor-improve CLI #3315 body, "Testing" section (code-quality)
  Code: **726 passed** at \93def1e`/on all 49 changed Python files/ "Review rounds" ends at93def1e**Issue:** The body describes the prior head: this head's lane row is 728 passed, the diff has 50 changed.pyfiles, and there is no round-3 entry for the vault-category fix. **Severity:** nit **Fix:** Refresh to "728 passed atf657d12", "50 changed Python files", and add a "Round 3 (f657d12`)" line naming the category enum, the probe abstention, and the three nits.

Miscellaneous

• None

Acknowledged Deferrals (verified)

• --acknowledge-unknown gate — plan Risk 5 reframe as accepted residual scope; budget's unknown-receipt block is its operator surface. Both judges accepted.
• dead_letter_stage is improve_intent — dead_letter_exhausted is the one writer, replayable=False on every branch; bridge/dead_letters.py::HANDLERS has no improve_intent entry.
• Exact-match ImprovementCase.query.filter() with per-state loops — consistent across adapter, recovery, doctor, budget, dashboard.
• Explicit from_state in _move() — sound; comment corrected.
• 6 malformed Verification rows — re-verified by hand at this head by both judges.
• Unit-2 reserve/release unfenced by the case lease — deliberate scope boundary.
• Session execution lease: fence worker ownership with a renewable Redis lease #3220 hand-off — gh issue view 3220 = OPEN; models/redis_lease.py absent.
• journal.set_state has no production caller — by design, the lifecycle owner's one door for a later lane (journal.py:367-372); exercised by two projection tests.

Review Delta (vs prior review on HEAD 93def1e)

• Resolved (all 6 round-3 findings): B1 category enum (tools/vault_write.py::write_credential default "API_CREDENTIAL", unit pin red under mutation by both judges); TD1 probe abstains with unknown (mutation red, both judges); TD2 unit-level pin plus PR-body skip disclosure; N1 _project_key deleted; N2 <h4>Control</h4> dropped (confirmed in the screenshots below); N3 expire routed through assert_control_key.
• Resolved (all 16 round-2 dispositions re-verified at this head): B1 three tests red under three mutations (both judges); TD1 through TD7 and N1 through N7 each located by symbol and, where a fix path exists, mutation-checked red by risk (mutations 4 through 18).
• New: import --force slot-hash merge (blocker); doctor reservation claim (tech debt); cmd_pause reason, break-glass propose without --action-id, two-call release, history-narrating probe comment, stale PR body (nits).
• Unchanged: none.

Verification Results

• Preflight MERGEABLE/CLEAN; head resolved through resolve_pr_head_sha = f657d12ea.
• ruff check . clean; ruff format --check . clean (1597 files). Lane row 728 passed (both judges). Integration 5 passed + 1 skipped (op unauthenticated here). Verification table 34 rows: 20 parser PASS, 14 by hand PASS. Scope guards 0. Docs gate 7/7. Plan sections ## Documentation, ## Update System, ## Agent Integration, ## Test Impact, ## No-Gos, ## Verification present. No new env var, no new migration. git grep -n "#3315" -- tools/ models/ agent/ ui/ = 0.
• Bridge/worker impact: models/session_lifecycle.py and agent/session_health.py are imported by both bridge and worker, and two reflections register on /update (idempotent). A worker restart on every machine is required after deploy.

Screenshots

Visual proof gate: PASSED. BYOB MCP (real Chrome) was live; the dashboard was started from this worktree at the PR head on spare ports (8531 against live data, 8532 against a claimed test db seeded through the ORM and cleaned through the ORM afterward), both stopped by PID.

• generated_images/pr-3315/r4_01_control_panel_empty.png — empty branch against live data: #improvement-control-panel renders "Nothing yet, written by lane 3 when a case is admitted." with no panel-title heading.
• generated_images/pr-3315/r4_02_control_panel_populated.png — populated branch: five <h4> sections Lane slots (1 slot(s) in use), Intents by state (rows admitted / reconciliation_required with case id, action id, type), Paused heads (No paused cases), Reconciliation required with the valor-improve resume --case <id> --force break-glass line, Unit 2: paid inference (Window 2026-09-15: reserved $0.00, settled $0.00). With the round-3 title dropped, the panel's heading hierarchy now matches its sibling panels.

Pre-Verdict Checklist (aggregate, any judge FAIL = FAIL)

• 1. All plan acceptance/success criteria validated against diff — FAIL — doctor reservation half of SC :490 (tech debt).
• 2. No-Gos from plan — none violated — PASS
• 3. New except Exception blocks — each has logger/raise/swallow-ok — PASS
• 4. New integration tests — exercise serialization boundary (not in-memory only) — PASS — CLI suite runs the real binary against a claimed db; the op boundary test skipped here and is reported as such.
• 5. Plan internal consistency — spike findings match task steps — PASS
• 6. No hardcoded secrets or debug artifacts — PASS
• 7. New public APIs — docstrings present — PASS
• 8. Breaking changes — migration path documented — N/A
• 9. Tests added for new behavior — PASS — 25 mutations across both judges, 24 red, 1 equivalent mutant.
• 10. Tests cover the failure path (not just happy path) — FAIL — import --force over a live namespace strands a slot; no test reaches it (blocker).
• 11. UI changes (if any) — screenshot captured — PASS — both template branches captured this round.
• 12. Docs updated for user-facing changes — FAIL — two doc sites claim doctor prints reservations it never reads (tech debt).

Rubric (aggregate, any judge fail = fail)

• 1. Plan vs. implementation match — fail — doctor reservations half of SC :490.
• 2. New code quality — pass — nits only.
• 3. Test coverage — pass — every round-2 and round-3 fix mutation-verified.
• 4. Regression risk to existing callers — pass — admitted inert to worker/health/drip; queue seam untouched.
• 5. Data integrity — fail — force-import merges the slot hash and strands post-export slots (blocker).
• 6. Security — pass
• 7. Documentation accuracy — fail — doctor reservation claims.
• 8. PR body accuracy — fail — describes 93def1ecb.
• 9. Disclosed deferrals — pass
• 10. Follow-up claims verified — pass — Session execution lease: fence worker ownership with a renewable Redis lease #3220 OPEN.

Verdict derivation: Rule 1, critical items 1 and 5 fail with no matching acknowledgement, CHANGES REQUESTED. Consensus: any-blocker-wins over n=2 of expected_n=2, blockers max = 1.

[tomcounsell]

Review (Judge code-quality): Approved

Independent judge, continued from round 4 (same subagent, own throwaway worktree, no view of the risk judge's output). Reviewed at head 8f8c22f22ee887a60e60ffe785645e5884789599. Lens: correctness, test honesty, code quality, plan and docs accuracy.

Verdict: APPROVED, zero findings. All seven round-4 dispositions closed, located by symbol; the new export test mutation-verified red.

Round-4 dispositions (7, at 8f8c22f)

• B1 import --force merges _ns:slots / pause hash: closed. tools/improvement_control/export.py::import_namespace if force: r.delete(keys.slots_key(project_key), keys.pause_key(project_key)) before the hset restores; docstring states it; test_import_force_drops_a_post_export_slot (export, admit a1 with max_concurrent=1, force import, hlen(slots) == len(archive_slots), fresh admit("a2") accepted).
• TD1 cmd_doctor reservations: closed. tools/improvement.py::cmd_doctor reads text_redis().hgetall(keys.slots_key(PROJECT_KEY)) and paid_inference_meter.status_dict inside the same try as the per-case reads (exit-2 guard); holders maps admitted/materialized/running intents to their case; reservations payload (slots with action_id/case_id/since, unit2 with day_key/reserved_usd); clean line only when not paused and not wedged and not outstanding; human line names each slot's holder or "none, no live intent holds it". Tests: TestDoctor::test_doctor_prints_an_outstanding_slot_with_its_holder, test_doctor_reports_clean_when_nothing_seeded asserts reservations keys; integration test_doctor_on_a_seeded_paused_case_prints_it admits the intent and asserts the seeded slot in both formats. Docs: improvement-controller.md doctor prose and tools-reference.md:363 match the code; plan SC :490 ticked.
• N1 _probe_vault_write comment: closed, docstring stating the invariant only.
• N2 cmd_pause reason / generation 0: closed. Refuses CASE_BUSY when generation is None and args.case (namespace pause still proceeds unfenced); human paused: False (<reason>), JSON carries reason; TestPause::test_pause_surfaces_its_reason_and_refuses_a_busy_case_lease.
• N3 break-glass propose action_id="": closed. import uuid at module level; elif not action_id: action_id = uuid.uuid4().hex; or 'noaction' / or "" fallbacks gone (git grep noaction = 0); tools-reference.md:357 names [--action-id ID]; test_propose_break_glass_without_action_id_mints_one.
• N4 release two-call shape: closed. _LUA_RELEASE single script (CAS state ~= 'reserved' -> 0, floor-decrement, HSET state released); release() one eval; _LUA_RELEASE_RESERVED gone; test_release_is_idempotent proves a double release decrements exactly once and a settled row releases nothing.
• N5 PR body: closed. Testing says 732 passed at 8f8c22f22 (reproduced exactly), 50 changed Python files (reproduced), Round 3 and Round 4 entries present, vault integration described as skipped where op is unauthenticated.

Mutation log

• export.py::import_namespace force r.delete(...) -> pass: RED on TestRoundTrip::test_import_force_drops_a_post_export_slot (assert 1 == 0; 1 failed, 7 passed); restored, git status --short empty.

Blockers

• None

Tech Debt

• None

Nits

• None

Miscellaneous

• None

Acknowledged Deferrals (verified)

• Session execution lease: fence worker ownership with a renewable Redis lease #3220 hand-off: gh issue view 3220 --json state = OPEN; models/redis_lease.py absent.
• --acknowledge-unknown reframe, dead_letter_stage=improve_intent, exact-match per-state ImprovementCase.query.filter() loops (cmd_doctor keeps the pattern), explicit from_state in _move(), 6 malformed Verification rows (targets untouched by 8f8c22f: no Lua in intents.py/journal.py, no lease file, no queue-seam file changed), unit-2 reserve/release unfenced by the case lease, journal.set_state without a production caller yet. All unchanged and sound.

Verification results (reproduced in the judge's worktree, PYTHONPATH pinned)

• ruff check . clean; ruff format --check . clean (1597 files).
• Plan's 21-file lane row: 732 passed, 0 failed, 0 skipped (41.6s), exit 0. +4 over round 4's 728 = the four new tests in the delta.
• test_improvement_cli.py + test_paid_inference_meter.py: 40 passed. Integration test_improvement_control_cli.py 5 passed; test_vault_write_integration.py 1 skipped (op unauthenticated here; reported as skipped, not passed).
• git grep -n "#3315" -- tools/ models/ agent/ ui/ = 0; no _LUA_RELEASE_RESERVED or noaction left in tools/, tests/, docs/features/.
• Docs accuracy on the delta: improvement-controller.md "no reader ever runs KEYS or SCAN" still holds (doctor adds one HGETALL on the slots hash and the unit-2 window hash); doctor prose matches the three views, the guard placement, and the clean-line condition.
• Fresh look at the delta: every new or changed function carries a docstring (cmd_doctor, _probe_vault_write, release, import_namespace); _LUA_RELEASE KEYS/ARGV header matches its one call site; new tests assert real state (window amounts, slot hash lengths, journal tail, read_head().paused), not exit codes alone. Nothing new to raise.

Pre-Verdict Checklist

• 1. All plan acceptance/success criteria validated against diff — PASS — doctor SC :490 now delivered in full.
• 2. No-Gos from plan — none violated — PASS
• 3. New except Exception blocks — each has logger/raise/swallow-ok — PASS — doctor's guards print namespace unreachable and exit 2 by design.
• 4. New integration tests — exercise serialization boundary (not in-memory only) — PASS — extended doctor e2e runs the console binary and parses JSON and human output.
• 5. Plan internal consistency — spike findings match task steps — PASS
• 6. No hardcoded secrets or debug artifacts — PASS
• 7. New public APIs — docstrings present — PASS
• 8. Breaking changes — migration path documented — N/A — doctor --json gains reservations, pause --json gains reason; both additive.
• 9. Tests added for new behavior — PASS — 4 new tests, 1 extended e2e, 1 rewritten idempotency test; the blocker's test mutation-verified.
• 10. Tests cover the failure path (not just happy path) — PASS — CASE_BUSY refusal, settled-row release no-op, post-export slot drop.
• 11. UI changes (if any) — screenshot captured — N/A — no UI file in the delta.
• 12. Docs updated for user-facing changes — PASS

Rubric

• 1. Plan vs. implementation match — pass
• 2. New code quality — pass
• 3. Test coverage — pass
• 4. Regression risk to existing callers — pass — release() callers are tests and the meter's own sweep; doctor/pause JSON changes are additive.
• 5. Data integrity — pass — force import replaces the slot/pause hashes wholesale; release decrements exactly once under CAS.
• 6. Security — pass
• 7. Documentation accuracy — pass
• 8. PR body accuracy — pass — 732 / 50 / rounds 3 and 4 reproduce.
• 9. Disclosed deferrals — pass
• 10. Follow-up claims verified — pass — Session execution lease: fence worker ownership with a renewable Redis lease #3220 OPEN.

Verdict derivation: every rubric item pass or n/a, Miscellaneous empty, no checklist FAIL: APPROVED.

[tomcounsell]

Review (Judge risk): Approved

Independent judge, continued from round 4 (same subagent, own throwaway worktree, no view of the code-quality judge's output). Reviewed at head 8f8c22f22ee887a60e60ffe785645e5884789599. Lens: regression risk, data integrity, security, failure paths, mutation evidence.

Verdict: APPROVED, zero findings. All seven round-4 dispositions closed by symbol and by mutation; the judge's own round-4 reproduction of the stranded slot no longer reproduces.

Round-4 dispositions (7, at 8f8c22f)

• B1 force-import slot merge: closed. export.py::import_namespace deletes the slot and pause hashes under force before restoring. Round-4 reproduction re-run in a throwaway test (export, admit a1, import --force, on_session_terminal, fresh admit): now hlen(slots)==0, on_session_terminal -> absent, admit("a2") accepted.
• TD1 doctor reservations: closed. cmd_doctor reads text_redis().hgetall(keys.slots_key(PROJECT_KEY)) and paid_inference_meter.status_dict, builds holders over admitted/materialized/running intents, all inside the namespace unreachable exit-2 guard; clean line only when not paused and not wedged and not outstanding; both doc sites match; plan :490 ticked.
• N1 pause reason / CASE_BUSY: closed. if generation is None and args.case: -> CASE_BUSY exit 1; namespace path unchanged (generation 0, direct hash write); reason in every emit.
• N2 break-glass propose action_id: closed. uuid.uuid4().hex minted under the session is None branch only; session path still takes ec["action_id"]; noaction fallback removed from artifact_key.
• N3 two-call release: closed. _LUA_RELEASE KEYS [window, reservation], CAS state ~= 'reserved' -> 0, decrement floored at 0, state='released'; release() early-returns only on a missing row; no stale _LUA_RELEASE_RESERVED reference.
• N4 (code-quality) _probe_vault_write comment: closed, docstring.
• N5 (code-quality) PR body: closed. Round 3 and Round 4 lines present; "732 passed at 8f8c22f" reproduces; vault integration described as skipped where op is unauthenticated.

Mutation log (all restored, git status --short empty after each)

• (a) export.py force r.delete(...) -> pass: red on test_import_force_drops_a_post_export_slot.
• (b) _LUA_RELEASE state CAS removed: red on test_release_is_idempotent.
• (c) cmd_doctor slots read -> {}: red on test_doctor_prints_an_outstanding_slot_with_its_holder (unit) and test_doctor_on_a_seeded_paused_case_prints_it (integration).
• (d) cmd_pause CASE_BUSY branch -> if False: red on test_pause_surfaces_its_reason_and_refuses_a_busy_case_lease.
• (e) cmd_propose mint -> "": red on test_propose_break_glass_without_action_id_mints_one.

Blockers

• None

Tech Debt

• None

Nits

• None

Miscellaneous

• None (advisory notes, no finding): cmd_doctor is the one new direct Redis read outside tools/improvement_control/ (text_redis().hgetall on keys.slots_key), through the package key builder and the same private alias the package uses; the plan's anti-criteria scope (tools/improvement_control/ and the meter) both still 0. import --force now also deletes _ns:pause, so a namespace pause set after the export is cleared by a forced restore of an older archive; the docstring and improvement-controller.md state it as "the archive's whole truth", consistent with replace semantics. release() has no production caller yet (lane 4 will call it); import_namespace and cmd_doctor are CLI-only, so no caller regression surface.

Acknowledged Deferrals (verified)

• Session execution lease: fence worker ownership with a renewable Redis lease #3220 OPEN (gh issue view 3220), models/redis_lease.py absent. All seven prior deferrals unchanged and sound.

Verification results

• 21-file lane row: 732 passed. Integration test_improvement_control_cli.py: 5 passed. test_vault_write_integration.py: 1 skipped ("op CLI not authenticated non-interactively"), reported as skipped.
• test_improvement_control_export.py + test_paid_inference_meter.py + test_improvement_cli.py + the throwaway probe: 49 passed (probe deleted afterward).
• ~/.popoto/improvement_content/ImprovementProposal/: 0 files before and after.
• Six No-Go scope guards: queue-seam diff 0; Session execution lease: fence worker ownership with a renewable Redis lease #3220/Improvement controller lane 5: the first complete research cycle #3217/Improvement controller lane 4: frozen evaluation inputs, blinding, and corrected statistics #3216/Improvement controller lane 6: production promotion, rollback drills, and the recursive comparison #3218/Improvement controller lane 7: cloud execution capacity #3274 paths untouched; judge base_url 0; child-gate / op / Popoto-client / .env greps 0.
• PR head re-resolved at the end: 8f8c22f22, mergeable=MERGEABLE, mergeStateStatus=CLEAN.

Pre-Verdict Checklist

• 1. All plan acceptance/success criteria validated against diff — PASS — doctor criterion :490 delivered and asserted end to end.
• 2. No-Gos from plan — none violated — PASS — all six guards 0.
• 3. New except Exception blocks — each has logger/raise/swallow-ok — PASS — delta adds none; doctor's reads sit inside the existing exit-2 guard.
• 4. New integration tests — exercise serialization boundary (not in-memory only) — PASS — doctor e2e runs the real binary and asserts the seeded slot line.
• 5. Plan internal consistency — spike findings match task steps — PASS
• 6. No hardcoded secrets or debug artifacts — PASS
• 7. New public APIs — docstrings present — PASS
• 8. Breaking changes — migration path documented — N/A
• 9. Tests added for new behavior — PASS — 5/5 delta mutations red.
• 10. Tests cover the failure path (not just happy path) — PASS — CASE_BUSY, double release, post-export slot drop all asserted.
• 11. UI changes (if any) — screenshot captured — N/A — no UI file in the delta.
• 12. Docs updated for user-facing changes — PASS

Rubric

• 1. Plan vs. implementation match — pass
• 2. New code quality — pass
• 3. Test coverage — pass — every round-4 fix mutation-verified.
• 4. Regression risk to existing callers — pass — no production caller of release(); import_namespace/cmd_doctor CLI-only; namespace pause path unchanged.
• 5. Data integrity — pass — stranded-slot reproduction no longer reproduces; release is one CAS-guarded script.
• 6. Security — pass
• 7. Documentation accuracy — pass
• 8. PR body accuracy — pass
• 9. Disclosed deferrals — pass
• 10. Follow-up claims verified — pass — Session execution lease: fence worker ownership with a renewable Redis lease #3220 OPEN.

Verdict derivation: zero findings, no rubric fail: APPROVED.

[tomcounsell]

Review: Approved

Mode: independent roster (2 judges): code-quality, risk. Both continued from their round-4 subagents (foreground, no name, awaited in-turn, each in its own throwaway worktree with the lane venv symlinked whole) with the same brief and no view of each other's output. Aggregation: agent.sdlc_review_consensus.compute_consensus, rule any-blocker-wins, expected_judges=2, n=2, quorum_shortfall=false, tied=false, mean confidence 0.95. Per-judge comments precede this one.

Head judged: 8f8c22f22ee887a60e60ffe785645e5884789599 by both judges. Head at posting: a912acdc6751a9d7de3b6f6eb65f4b866a2e5fd6 (resolved via tools/pr_head_resolver.py::resolve_pr_head_sha), whose only delta over the judged head is the APPROVED-path plan-checkbox sync commit (docs(#3215): sync plan checkboxes with review verdict): one file, docs/plans/improvement-controller-lane-3-control-journal-fenced-dispatch.md, 16 - [ ] -> - [x] lines in ## Success Criteria, no code. Base main b13dc8ad3. Preflight: state=OPEN, mergeable=MERGEABLE, mergeStateStatus=CLEAN.

Round 5 summary. Round 4 at f657d12ea carried 1 blocker, 1 tech debt, and 5 nits; commit 8f8c22f22 answers all seven, and both judges independently located each disposition by symbol and mutation-checked every fix path in the delta (code-quality 1 mutation, risk 5 mutations, all red). The risk judge re-ran its own round-4 reproduction of the stranded slot (export, admit, import --force, on_session_terminal, fresh admit) and it no longer reproduces. Zero findings from either judge. This is a zero-finding approval, not a nits-only stop.

Parent-reproduced gates (Hard Rule 10): the parent measured the plan's 21-file lane row at 732 passed and ruff check . / ruff format --check . clean at 8f8c22f22 before refreshing the PR body; both judges reproduced 732 independently. Integration test_improvement_control_cli.py 5 passed (both judges); test_vault_write_integration.py 1 skipped (OP_CACHE=false op whoami exits 1 on this machine; reported as skipped, never as passed; the round-3 category fix is pinned at unit level and was proven red under mutation in round 4). Six No-Go scope guards 0. Content root ~/.popoto/improvement_content/ImprovementProposal/ 0 files before and after every run. git grep -n "#3315" -- tools/ models/ agent/ ui/ = 0.

Verified

• Code correctness
• Test coverage
• Security (no vulnerabilities found)
• Plan requirements met

Blockers

• None

Tech Debt

• None

Nits

• None

Miscellaneous

• None (advisory, no finding): cmd_doctor adds one direct HGETALL on the slots hash outside tools/improvement_control/, through the package key builder, mirroring ui/data/improvement.py; import --force now also replaces _ns:pause, documented as "the archive's whole truth"; release() has no production caller yet (lane 4).

Acknowledged Deferrals (verified)

• --acknowledge-unknown gate — plan Risk 5 reframe as accepted residual scope; budget's unknown-receipt block is its operator surface.
• dead_letter_stage is improve_intent — dead_letter_exhausted is the one writer, replayable=False; bridge/dead_letters.py::HANDLERS has no improve_intent entry.
• Exact-match ImprovementCase.query.filter() with per-state loops — consistent across adapter, recovery, doctor, budget, dashboard.
• Explicit from_state in _move() — sound.
• 6 malformed Verification rows — verified by hand in round 4; their targets are untouched by 8f8c22f22 (no Lua in intents.py/journal.py, no lease file, no queue-seam file changed).
• Unit-2 reserve/release unfenced by the case lease — deliberate scope boundary.
• Session execution lease: fence worker ownership with a renewable Redis lease #3220 hand-off — gh issue view 3220 = OPEN (both judges); models/redis_lease.py absent.
• journal.set_state has no production caller — by design, the lifecycle owner's one door for a later lane.

Review Delta (vs prior review on HEAD f657d12)

• Resolved (all 7 round-4 findings): B1 import --force deletes slots_key and pause_key before restoring (test_import_force_drops_a_post_export_slot, red under mutation by both judges); TD1 cmd_doctor reads the slot hash and the unit-2 window inside the exit-2 guard and prints them under reservations with each slot's holder, clean line only when all three views are empty, both doc sites corrected, plan SC ticked; N1 _probe_vault_write docstring; N2 cmd_pause carries reason and refuses CASE_BUSY on a held case lease; N3 break-glass propose mints a uuid action_id; N4 release is one _LUA_RELEASE script under a CAS; N5 PR body refreshed at 8f8c22f22 (732 / 50 / rounds 3 and 4 all reproduce).
• New: none.
• Unchanged: none.

Verification Results

• Preflight MERGEABLE/CLEAN; head at posting a912acdc6 (plan-only tick commit over the judged 8f8c22f22).
• ruff check . clean; ruff format --check . clean (1597 files). Lane row 732 passed (parent and both judges). Integration 5 passed + 1 skipped. Scope guards 0. Docs gate 7/7 from the plan's ## Documentation section. Plan sections ## Documentation, ## Update System, ## Agent Integration, ## Test Impact, ## No-Gos, ## Verification present. No new env var, no new migration.
• Plan checkbox sync (APPROVED path): 16 Success Criteria ticked via tools.plan_checkbox_writer, 0 match failures; the two pipeline rows (Tests pass (/do-test), Documentation updated (/do-docs)) are left for their stages.
• Bridge/worker impact: models/session_lifecycle.py and agent/session_health.py are imported by both bridge and worker, and two reflections register on /update (idempotent). A worker restart on every machine is required after deploy.

Screenshots

Visual proof gate: PASSED, carried from round 4. The 8f8c22f22 and a912acdc6 deltas touch no ui/ file (tools, tests, docs, and the plan only), so the round-4 BYOB captures at f657d12ea remain the proof of the shipped template:

• generated_images/pr-3315/r4_01_control_panel_empty.png — empty branch against live data.
• generated_images/pr-3315/r4_02_control_panel_populated.png — populated branch (five <h4> sections, intents table, reconciliation break-glass line, unit-2 window).

Pre-Verdict Checklist (aggregate, any judge FAIL = FAIL)

• 1. All plan acceptance/success criteria validated against diff — PASS — doctor SC :490 delivered in full; 16 criteria ticked.
• 2. No-Gos from plan — none violated — PASS
• 3. New except Exception blocks — each has logger/raise/swallow-ok — PASS
• 4. New integration tests — exercise serialization boundary (not in-memory only) — PASS — doctor e2e runs the console binary in a subprocess against a claimed db; the op boundary test skipped here and is reported as such.
• 5. Plan internal consistency — spike findings match task steps — PASS
• 6. No hardcoded secrets or debug artifacts — PASS
• 7. New public APIs — docstrings present — PASS
• 8. Breaking changes — migration path documented — N/A — doctor --json gains reservations, pause --json gains reason; additive.
• 9. Tests added for new behavior — PASS — 6 delta mutations across both judges, all red.
• 10. Tests cover the failure path (not just happy path) — PASS
• 11. UI changes (if any) — screenshot captured — PASS — round-4 captures; no UI file in the delta.
• 12. Docs updated for user-facing changes — PASS

Rubric (aggregate, any judge fail = fail)

• 1. Plan vs. implementation match — pass
• 2. New code quality — pass
• 3. Test coverage — pass
• 4. Regression risk to existing callers — pass
• 5. Data integrity — pass — force import replaces the slot/pause hashes wholesale; release decrements exactly once under CAS.
• 6. Security — pass
• 7. Documentation accuracy — pass
• 8. PR body accuracy — pass — 732 / 50 / rounds 3 and 4 reproduce.
• 9. Disclosed deferrals — pass
• 10. Follow-up claims verified — pass — Session execution lease: fence worker ownership with a renewable Redis lease #3220 OPEN.

Verdict derivation: Rule 4, every rubric item pass or n/a, Miscellaneous empty, no checklist FAIL: APPROVED. Consensus: any-blocker-wins over n=2 of expected_n=2, blockers max = 0.

[tomcounsell]

Review: Approved

Re-verification of the round-5 verdict at head d527bd697befdc18c4d0600f41f452fa991bb2a7 (mode: independent roster (2 judges), code-quality and risk, both continued from round 5, foreground, awaited in-turn; consensus any-blocker-wins, n=2, expected_n=2, no shortfall). Two things moved the head since the judged code head 8f8c22f22 and the posted a912acdc6:

1. eca0347a0, the /do-docs cascade (docs-only, 5 files, +24/-17 in docs/features/README.md, improvement-controller.md, improvement-evaluation.md, reflections.md, plus the lane-3 plan's Documentation checklist ticked and status: docs_complete). Both judges re-checked out that head, confirmed git diff 8f8c22f22..eca0347a0 -- ':!docs' is empty, and verified every edited sentence against the code (cmd_propose_amendment's investigation kind/state and single page; recovery.reconcile reclaiming intents and slots and never touching ImprovementExperiment; CaseLease as the dispatch fence; the three reflection bullets against reflection_register.py, config/settings.py, and the callables; the archived lane-4 plan path on disk; all three anchors; the docs gate reproducing "7 doc(s) changed as expected"). Zero findings from either judge.
2. Rebase onto origin/main 205344717 (main had moved by one docs/plans/ file for Voice interview channel: harvest open questions into a pre-recorded pruning tree, ask them as voice notes, record answers back into plan docs #3330, +1570, no overlap). Both judges and the parent independently confirmed git diff origin/main...eca0347a0 and git diff origin/main..d527bd697 are byte-identical (9159 lines each) and that git diff --stat eca0347a0 d527bd697 is exactly main's own plan file.

The round-5 code verdict (732 lane tests, ruff clean, 6 delta mutations red, all 7 round-4 dispositions closed) therefore carries to this head unchanged. Preflight at posting: state=OPEN, mergeable=MERGEABLE, mergeStateStatus=CLEAN. #3220 still OPEN. Issue #3336 was filed by the cascade for a pre-existing project-key split (reflections on DEFAULT_PROJECT_KEY vs the CLI and dashboard on valor), inherited from lane 2 and outside this PR's scope.

Blockers

• None

Tech Debt

• None

Nits

• None

Miscellaneous

• None