fix(kanban): fail closed on missing worker handoff

[jayleaton]

Summary

• fail closed in one-shot Kanban workers when an rc=0 return lacks a durable complete, block, or review handoff
• fence finalization to the exact task run, retry transient database failures, and verify the terminal run via a fresh connection
• atomically record protocol violations with existing bounded circuit-breaker accounting while preserving completed, blocked, reviewed, crashed, and stale runs
• reject missing or malformed worker run identity and finalize before the cleanup watchdog can force a successful process exit

Verification

• scripts/run_tests.sh tests/hermes_cli/test_kanban*.py tests/plugins/test_kanban*.py tests/cli/test_single_query_session_finalize.py tests/cli/test_oneshot_resumed_session_persist.py tests/tools/test_oneshot_completion_linger.py -q — 453 passed, 2 OS-specific skipped
• ruff check cli.py hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_worker_finalization.py — passed
• python -m compileall -q cli.py hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_worker_finalization.py — passed
• git diff --cached --check — passed
• independent staged-diff review — passed with no security concerns or logic errors

Safety

• no Discord gateway was started
• no model/provider spend configuration was changed
• this PR must not be merged by the implementing agent

[andrexibiza]

Reviewed exact head de20c1676fee3fec629c781db4416eb34869af6c against current main 63279301bcbdc185c1b07b98a9312eb0c862f26d. I inspected the two-commit history, all three changed files, the new lifecycle tests, current hosted workflow state, open review/comments (none), and the adjacent Kanban ownership/liveness/acceptance work.

The core direction is good. In particular, exact-run fencing on current_run_id, atomic run-finalization + breaker accounting, a fresh-connection read-back before allowing nominal success, and performing lifecycle verification before cleanup can reach its _exit(0) fail-safe are all the right kinds of invariants for this boundary. I would not merge this head yet, though: there are four merge blockers, two of them correctness/authority defects in the current implementation.

1. Merge blocker: inherited Kanban identity is being treated as process ownership

_finalize_single_query() decides that it owns a Kanban lifecycle solely from HERMES_KANBAN_TASK / HERMES_KANBAN_RUN_ID in os.environ, then calls _ensure_kanban_worker_lifecycle(). That is not a valid ownership predicate in this repository.

We already have live evidence for this exact defect class. #102560 documents a fresh Hermes/cron subprocess inheriting the parent worker's HERMES_KANBAN_* environment even though the child is not dispatcher-owned, and current main already has the canonical is_dispatcher_owned_worker_context() boundary used by the Kanban tool/skill surfaces. #98750 is the other side of the same shape: stale/nested processes can retain a task/run identity after authority has moved.

With this PR as written, a nested one-shot Hermes process can do the following deterministically:

1. parent worker owns task T, current run R;
2. parent starts a child Hermes one-shot process and the child inherits HERMES_KANBAN_TASK=T and HERMES_KANBAN_RUN_ID=R;
3. child completes its own unrelated work normally without calling a parent Kanban terminal tool;
4. child _finalize_single_query() sees the inherited env and calls finalize_clean_worker_exit(T, R);
5. the CAS succeeds because the parent's task is still running on run R;
6. the child closes the parent's run as crashed, releases/requeues (or eventually blocks) the task, while the legitimate parent process is still alive and working.

That converts an identity carrier into mutation authority. It is worse than a false diagnostic: the nested child can terminate somebody else's exact live run.

Please gate this path on the canonical dispatcher-owned worker predicate, not environment presence. The malformed/missing run-id failure should likewise apply only after establishing that this process actually owns a dispatcher run. Add a regression that proves both sides of the boundary: a dispatcher-owned worker clean-exiting without a handoff fails closed, while a delegated/nested/non-dispatcher child carrying the same inherited task/run env cannot mutate the parent's task or run.

Landing #102560 first would not fix this by itself because this new finalizer bypasses that predicate and reads os.environ directly. The two patches need to compose explicitly.

2. Merge blocker: PRODUCT_SIGNOFF is an untrusted prose backdoor around the breaker

The second commit adds _PRODUCT_SIGNOFF_MARKER = "PRODUCT_SIGNOFF" and makes _account_protocol_violation() return before breaker accounting whenever that substring appears in the task title or body. The accompanying regression intentionally drives more than _PROTOCOL_VIOLATION_FAILURE_LIMIT clean-exit violations and asserts the task remains ready forever with no gave_up event.

I cannot find a pre-existing PRODUCT_SIGNOFF contract in current main, nor an issue/PR defining that string as a trusted lifecycle/schema field. This PR is therefore creating a new authority decision out of arbitrary mutable task prose.

That undercuts the PR's own fail-closed invariant. A task that needs human/product sign-off still needs a durable handoff. Hermes already has explicit lifecycle states for that: kanban_block(..., kind="needs_input") and the review handoff. Clean rc=0 with only prose saying “I reported back for sign-off” is exactly the missing-terminal-transition condition this patch is meant to detect. Silently exempting it from bounded failure handling means a malformed worker can respawn indefinitely.

Please remove the title/body marker exemption from this PR. If product sign-off needs distinct kernel semantics, model it separately as structured/versioned state or metadata with an authoritative producer and an explicit terminal handoff. #102457/#102477 are useful precedent for putting acceptance policy in a structured contract rather than parsing task prose.

3. Merge blocker: this grows two known godfile owners and collides with the structural landing order

This patch modifies cli.py and hermes_cli/kanban_db.py. On this exact base they are still the large pre-decomposition owners (#102117 records the base sizes as ~22.4k and ~12.2k lines). The repository-wide residual tracker #78647 makes the landing rule explicit: modified source owners must finish at or below 2,000 physical lines, and follow-on behavior targeting old owners must be recomposed into bounded post-decomposition modules rather than regrowing the façade.

#102117 is therefore a structural predecessor/collision, not unrelated cleanup. It also touches both of these owners and extracts the CLI/Kanban responsibilities. If that work lands first, this lifecycle finalizer needs to be forward-ported into the correct bounded owner rather than pasted back into the old façade. If this behavior must land independently first, these touched responsibilities still need to be extracted so the final modified owners satisfy the 2K invariant.

There is additional FILE-LIST pressure in the same ownership area from #101911, #102390, #102477, #102524, and #102600. Those are not duplicates of this PR, but merge order must not let whichever patch lands last silently choose the lifecycle/ownership policy.

4. Merge blocker: no surviving commit currently has green hosted acceptance evidence

This PR has two commits:

• 546b1324b707dd63a6d8ba260fe6157344b67345
• de20c1676fee3fec629c781db4416eb34869af6c

For both, the hosted CI / Docker / Nix workflow runs currently resolve action_required, not green. The local 453 passed, 2 skipped targeted run is useful evidence and the new tests exercise important transaction/idempotency behavior, but it is not a substitute for the repository's exact-object acceptance gates. Every surviving commit needs its required checks green, and the final exact head needs the full hosted gates green before merge.

Interlocks / what this PR does and does not supersede

This patch is complementary to #102516: that work fixes the turn-end guard's understanding of review terminal actions, while this patch verifies the durable run state at process exit. It is also complementary to #98750: this patch catches a process that exits without a terminal handoff; it does not solve a stale process that stays alive and continues mutating after its run ended.

#102560 is the same ownership-boundary family and must compose with this patch as described above. #102390 and #102477 operate one layer later/differently: they bind completion/review acceptance to structured evidence; they are not replacements for clean-exit settlement, and this patch should not weaken their evidence semantics. #101911 owns worker lifetime/re-adoption/verified teardown and is another adjacent authority carrier, not a duplicate.

Contributor credit should stay with this work if the code is moved during decomposition. The transaction shape, exact-run CAS, idempotent replay behavior, and fresh read-back are valuable pieces worth preserving. The changes needed here are about putting those strong mechanics behind the correct ownership and policy boundaries, not discarding them.

Acceptance I would re-check on the next head

• dispatcher-owned worker + rc=0 + no durable terminal handoff => exact run is atomically recorded as a bounded protocol violation and process exits nonzero;
• nested/delegated/non-dispatcher child with inherited HERMES_KANBAN_* => cannot finalize, requeue, block, or otherwise mutate the parent's run;
• complete/block/request-review/request-changes/other already-terminal exact runs remain untouched and idempotent;
• no arbitrary title/body string can disable failure accounting;
• touched source owners satisfy the 2K landing invariant in the chosen merge topology;
• every surviving commit and the final exact head have green required CI/Docker/Nix/platform/attribution gates.

There is a solid fail-closed kernel inside this patch. Once the ownership carrier and prose-policy escape hatch are corrected, it will be much easier to trust the settlement boundary under adversarial composition.