fix(server): deliver a gap-landing elicitation verdict to the codex re-park (tombstone zombie-waiter resolves, fingerprint tombstones)

[omni-resolve-agent]

Related issue

Closes #2055

Resolves OMNI-1233 (Linear).

Summary

A codex-native approval gate long-polls POST /v1/sessions/{sid}/hooks/codex-elicitation-request in chunks with the same JSON-RPC envelope, so the server-side elicitation id is stable across re-parks. When a proxy severs a chunk client-side while the backend connection stays open, the abandoned chunk's waiter stays parked as a zombie. If the operator answers the approval card in that gap, _resolve_elicitation set the verdict on the zombie waiter's Future (written to a connection nobody reads) and — because a Future was registered — wrote no tombstone. The harness's next chunk re-parked the same stable id, found nothing, and re-published the gate: the operator's answer was lost and they had to answer twice.

Fix:

• Always tombstone a verdict that settles a registered waiter, so a re-park of the same stable id adopts it instead of re-asking.
• Fingerprint tombstones (sha256 of the elicitation request params, taken from the parked waiter or, on the nothing-parked path, the pending-index prompt) and drop a mismatched tombstone at consume time — so a later, different question that reuses the same JSON-RPC id within the 300s TTL can never inherit a stale approval.

ELI5: the server used to hand your "Approve" to a phone line that had already gone dead, and then shredded the note. Now it also pins the note to the door (with a description of exactly which question it answers), so when the caller redials the same question, they get your answer — and a different question can't steal it.

before:  chunk1 parks ──sever (undetected)──▶ zombie waiter
         operator approves ──▶ verdict → zombie Future (lost), no tombstone
         chunk2 re-parks same id ──▶ nothing found ──▶ gate re-asked ✗

after:   chunk1 parks ──sever (undetected)──▶ zombie waiter
         operator approves ──▶ verdict → zombie Future AND fingerprinted tombstone
         chunk2 re-parks same id ──▶ tombstone fingerprint matches ──▶ verdict delivered ✓

Test Plan

Fail→pass proof (all three fail on the unfixed tree for the behavioral reason, pass with the fix):

• tests/e2e_ui/approvals/test_codex_gap_verdict_zombie_repark.py — real SPA journey: park chunk 1 (undetected sever), approve the card in the browser, re-POST the same envelope. Before: chunk 2 hits ReadTimeout and the card re-appears. After: the re-park returns {"decision": "accept"} and no pending card remains.
• tests/server/integration/test_codex_gap_verdict_zombie_poll.py — same journey at the HTTP layer. Before: fails with "gap-landing verdict LOST". After: passes.
• tests/server/test_resolve_tombstones_zombie_waiter.py — unit tests at the changed layer: resolve of a live future also tombstones the verdict; a mismatched-fingerprint tombstone is dropped at consume; invalid payloads tombstone nothing.

Also ran the surrounding elicitation suites (91 passed) to catch regressions. Independent cross-vendor (codex) review ran pre-PR, 2 rounds: its blocking finding (stale-verdict replay when a JSON-RPC id is reused within the TTL) drove the fingerprinting; its residual note (identical-content re-ask within the TTL adopts the verdict) is the pre-existing, intended retry semantics of the tombstone mechanism, unchanged here.

Demo

• Non-visual evidence provided below or in Test Plan

Before/after recordings (web UI journey and terminal test runs) are preserved in the resolve workflow's artifact bundle (before-gap-verdict-lost.mp4, after-gap-verdict-delivered.mp4, before-resolve-2xx.mp4, after-resolve-delivered.mp4) and attached to the Linear ticket OMNI-1233.

Type of change

• Bug fix
• Feature
• UI / frontend change
• Refactor / chore
• Docs
• Test / CI
• Breaking change

Test coverage

• Unit tests added / updated
• Integration tests added / updated
• E2E tests added / updated
• Manual verification completed
• Existing tests cover this change
• Not applicable

Coverage notes

Unit, integration, and E2E-UI coverage all assert the same fail→pass transition at their own layer; no manual-only verification.

Changelog

Approving a codex gate while its poll was silently severed no longer loses the answer — the harness's next poll receives it instead of re-asking.

[github-actions]

UI Preview is being deployed for this PR ⏳

Commit	7484cb0
Run	https://github.com/omnigent-ai/omnigent/actions/runs/33487262724

Building and deploying... This comment will be updated with the preview URL.

[omnigent-ci]

Polly AI Review

Review: gap-landing elicitation verdict to the codex re-park

I traced the fix through both tombstone-writing paths (_resolve_elicitation), the consume path (_consume_pre_resolved_harness_elicitation), and the fingerprint helper, and independently corroborated with a cross-vendor pass.

Blocking issues

None. I examined the candidates below and downgraded each after checking the actual diff and surrounding code:

• Cross-session tombstone overwrite (nothing-parked branch). The no-waiter branch writes _harness_pre_resolved_elicitations[elicitation_id] = … without an owner check, so in principle session B's resolve could clobber session A's tombstone under a shared id. But (a) this is pre-existing behavior, unchanged by this PR; (b) consume is session-scoped, so a foreign verdict can never be inherited — the worst case is a lost verdict (re-ask), not a wrong approval; and (c) codex elicitation ids are derived from (session, method, rpc_id), so a cross-session id collision isn't realistically reachable. Not blocking.

Security notes

• Delivered (non-zombie) verdicts now leave a 300s tombstone that an identical re-request can adopt without re-prompting. This is the one genuine behavior widening in the diff. Before this PR, a verdict delivered to a live Future left no tombstone; now every delivered approval plants one. If the harness later issues a genuinely new approval request that reuses the same JSON-RPC id and has byte-identical params (same fingerprint) within the 300s TTL, the re-park consumes the tombstone and auto-approves a second execution with no operator prompt. The fingerprint guards the "different question, reused id" case (good), but cannot distinguish "harness retrying the same still-open question" (desired adoption) from "operator being asked to approve the same command a second time" (should re-prompt) — by id+content alone they are indistinguishable. The PR description acknowledges this as intended retry semantics. It's a defensible tradeoff given the harness re-POSTs an identical envelope on retry, but it does relax strict per-execution approval, so it's worth an explicit callout for the human's threat model. If per-execution consent matters, consider one-shot consume of the delivered-path tombstone, or a generation/nonce keyed to the waiter rather than content alone. Non-blocking, but the most important thing to weigh before merge.
• None-fingerprint fallback adopts by id. On the gap path, if the pending entry is missing/invalid, gap_fingerprint is None, and consume accepts a None-fingerprint tombstone for any re-park fingerprint (legacy adopt-by-id). This is the pre-existing semantics preserved as a fallback, not a new hole — the common path is now fingerprinted, which strictly improves safety. Tightening the fallback (drop rather than adopt when no fingerprint is available) would close the residual window, but it's not a regression.

Non-blocking notes

• Fingerprint parity across the two paths is sound. The parked path fingerprints params.model_dump(mode="json") directly; the gap path round-trips through the pending index (model_dump() → model_validate → model_dump(mode="json")). Both canonicalize with sort_keys=True and consistent None inclusion, and ElicitationRequestParams uses extra="allow" so passthrough fields survive both round-trips identically. A legitimate same-question re-park will match. Good.
• Bounded storage. Tombstones stay bounded by the existing 300s TTL and the max-entries cap; _prune_* is called on both write paths. No unbounded growth.
• Consume is independent of registry cleanup, so the zombie-finally-pops-registry vs. chunk-2-re-park race doesn't defeat adoption — verified against _publish_and_wait_for_harness_elicitation's finally.
• Ownership + validation guards hold on the live-Future path (verdict and tombstone are both gated by the owner check and ValidationError skip), and the tests (test_resolve_from_wrong_session_tombstones_nothing, test_resolve_with_invalid_payload_tombstones_nothing) pin exactly that.
• Test coverage is strong: three fail→pass tests spanning unit, HTTP-integration, and real-SPA layers, each asserting the specific behavioral symptom rather than incidental state.

Summary

Well-scoped, well-tested fix for a real data-loss bug: an operator approval landing in the undetected-sever gap was silently swallowed by a zombie waiter, and the fingerprinting sensibly prevents a reused JSON-RPC id from inheriting a stale approval for a different question. No blocking defects. The one thing to weigh consciously before merge is that the diff now writes a tombstone for every delivered verdict (not just the nothing-parked path), which lets a byte-identical re-request within the 300s TTL adopt an approval without re-prompting — an accepted retry-semantics tradeoff the author called out, but a genuine relaxation of per-execution approval worth a human decision. Recommend merge once that tradeoff is acknowledged (optionally embedding the referenced before/after clips directly in the PR body, since they currently live only on the Linear ticket).The inbox is empty — that was the reviewer completion I already collected and incorporated. The review is complete and posted above. No further action needed.

---

_{Automated review by Polly · workflow run}