feat(chat): add reusable durable plan and interaction workflows

[vutuanlinh2k2]

Summary

• add a reusable ./durable-chat server workflow for scoped plan projections, CAS decisions, stable follow-up receipts, retryable product effects, interaction tombstones, and crash-recoverable answer settlement
• project durable plan/question lifecycle events through both arbitrary chat producers and the shared Sandbox producer, with revision-aware stale-event protection
• add browser clients, persistent interaction attempt keys, durable plan/question controllers, and canonical React cards directly consumable by ChatMessages
• preserve accepted interaction values and authoritative outstanding IDs across live updates, retries, reconnects, and transcript reload

This expands the original persistence fix into the reusable application-shell design tracked by #212. Products now provide authorization, production storage, Sandbox/sidecar connections, idempotent effects, and follow-up attachment; they no longer need to reimplement the lifecycle state machines. GTM PR tangle-network/gtm-agent#591 remains the reference consumer for the follow-up migration.

Correctness details

• plan decisions use stable scope/plan/revision/decision keys and reconcile ambiguous authority commits
• repeated or restored decisions return the same deterministic receipt and retry incomplete product effects
• stale plan revisions, stale terminal events, delimiter-bearing IDs, and same-revision content mutation cannot corrupt durable state
• interaction answers use caller-stable attempt keys and prepare → authority acknowledgement → finalize ordering
• unresolved answer reconciliation stays retryable; cancellation tombstones and accepted values remain monotonic
• duplicate-question dedupe ends with the pending lifecycle, and authoritative restore replaces obsolete duplicate IDs
• legacy plan cards are suppressed only by explicit planId + revision, never by content equality

Validation

• pnpm typecheck
• focused durable chat/interaction/stream/React suite: 17 files, 194 tests passed
• NODE_OPTIONS=--max-old-space-size=8192 pnpm build (ESM and declarations)
• full pnpm test: 2,596 passed; 3 pre-existing macOS deferred-profile retry tests exceeded the 5-second timeout, matching the branch baseline
• git diff --check

Closes #212

[tangletools]

🟡 Value Audit — sound-with-nits

Verdict	sound-with-nits
Concerns	2 (2 weak-concern)
Heuristic	0.0s
Duplication	0.0s
Interrogation	203.9s (2 bridge agents)
Total	203.9s

💰 Value — sound

Adds a browser-safe /plans chat projection + durable interaction-answer codecs so plan cards and accepted answers survive reload; mirrors the existing /interactions codec and /chat-routes beforeTurn seam — fits the codebase grain.

• What it does: Introduces a new additive /plans subpath (src/plans/index.ts:1-148) that parses the peer sandbox SDK's plan.submitted events into a ChatPlan union, persists them as type:'plan' transcript parts, and exposes key/turn-id/monotonic-status helpers. Wires the sandbox chat producer to capture plan.submitted events durably (src/chat-routes/sandbox-producer.ts:236-244), teaches chat-store parts to project
• Goals it achieves: Plan cards and accepted interaction answers were lost on page reload — the shell persisted only the ask, not the answer, and dropped plan.submitted events entirely. After merge: (1) plan cards render and restore from persisted parts just like interaction parts; (2) accepted interaction selections are strictly validated, persisted, and replayed; (3) products get a single pre-unblock persistence hoo
• Assessment: Good change, in the grain of the codebase. It hits every shell invariant in AGENTS.md: (a) additive subpath wired in all four places — tsup.config.ts, package.json exports, src/index.ts barrel, AGENTS.md module map row 32; (b) substrate-free — /plans imports nothing from @tangle-network/sandbox, structurally byte-matching the SDK's plan shape (the same approach /interactions takes per AGENTS.md mo
• Better / existing approach: none — this is the right approach. I checked: (1) src/missions/plan-parse.ts is a different thing — it parses the agent-authored :::mission text block for the missions orchestration feature, not the SDK's durable plan.submitted events; not reusable here. (2) @tangle-network/sandbox owns plan lifecycle/decisions and is the authoritative source, but importing it into /plans would violate the substra
• Model: opencode/zai-coding-plan/glm-5.2
• Bridge attempts: 2
• Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound-with-nits

A textbook contribute-down of gtm-agent's working plan + interaction-answers persistence into the shared chat shell, with the imminent consumer (gtm-agent PR #591) already running on a local byte-equivalent copy.

• Integration: Fully wired and reachable. createSandboxChatProducer consumes plan.submitted events at src/chat-routes/sandbox-producer.ts:236, persists them via planToPersistedPart, and yields them on the live stream. createInteractionAnswerRoute exposes the new beforeAnswer seam at src/interactions/route.ts:151 — a product opts in by passing the option, no other change. ChatPlanPart is added to the
• Fit with existing patterns: Matches the codebase's grain precisely. The shell's /plans module is a structural mirror of the consumer's chat-plans.ts (ChatPlan union, parsePlanSubmittedEvent, planToPersistedPart, persistedPartToPlan, planPartKey, canTransitionPlanStatus — all one-for-one). It composes cleanly into the existing /stream normalizer (plan key + merge with monotonic status, parallel to the interaction-status
• Real-world viability: Robust on the non-happy paths. parsePlanSubmittedEvent accepts both the direct (data.plan) and session-enveloped (properties.plan) shapes and fails loud with a typed error on malformed payloads (the producer drops + logs). canTransitionPlanStatus is monotonic (only pending → terminal), and mergePersistedPart at src/stream/stream-normalizer.ts:315 enforces it on every overlay so a stale r
• Model: opencode/zai-coding-plan/glm-5.2
• Bridge attempts: 1

💰 Value Audit

🟡 stampInteractionAnswers is exported with no in-shell caller [maintenance] ``

src/interactions/contract.ts:325 adds stampInteractionAnswers, which is referenced only by tests/interactions-contract.test.ts:50 inside this repo — no internal call site. It is a pure-mechanism helper for the consumer (gtm-agent#591) to rehydrate a transcript from its own store. Justified by the PR's explicit goal of moving that mechanism out of the GTM repo, but it is speculative API surface until the reference consumer lands — worth flagging only so a reviewer knows why an apparently-unused e

🎯 Usefulness Audit

🟡 InteractionAnswers narrows against the wire validator and the in-flight consumer [ergonomics] ``

src/interactions/route.ts:69 validates POST data fields as InteractionData (string|number|boolean|string[]), but src/interactions/contract.ts:73 declares InteractionAnswers = Record<string, string[]> for the PERSISTED shape, and interactionToPersistedPart/stampInteractionAnswers enforce the strict shape via parseInteractionAnswers. The in-flight consumer (gtm-agent#591 src/lib/chat-interactions.ts:26) deliberately keeps a looser parseInteractionAnswers that accepts primitives, and

---

What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass	What it asks
Heuristic	Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication	Do added function/class names already exist elsewhere in the repo?
Value Audit	What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit	Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

_{value-audit · 20260721T060243Z}

[tangletools]

✅ No Blockers — 3929aed8

Review health 100/100 · Reviewer score 25/100 · Confidence 95/100 · 21 findings (1 medium, 20 low)

glm: Correctness 25 · Security 25 · Testing 25 · Architecture 25

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 8/8 planned shots over 18 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM stream module breaks L0 substrate-free invariant — src/stream/stream-normalizer.ts

ARCHITECTURE.md states L0 Foundation modules 'Import nothing from the package; safe anywhere' and explicitly lists stream (SSE normalize + resumable turn buffer) as L0. Before this PR stream-normalizer.ts had zero imports (verified via git show 973c96e:src/stream/stream-normalizer.ts). The diff adds import { ... } from '../interactions/contract' and from '../plans/index'. interactions/contract.ts itself imports from peer @tangle-network/agent-interface, so stream's transitive surface widens. Functionally safe today (peer dep is browser-clean, all 25 browser-safe tests pass), but the doc invariant is no longer true. Fix: either (a) move the plan/interaction normalize+merge branches into chat-store/parts.ts (which already owns canonical validation per its inline comments) and let s

🟡 LOW No producer-level test for malformed plan.submitted drop path — src/chat-routes/sandbox-producer.ts

The new plan.submitted branch logs and continues on parse failure (L238-241), but tests/chat-routes/sandbox-producer.test.ts only covers the happy path (L141-175). The existing interaction branch has a paired malformed-drop test at L177-194 ('drops malformed interactions') that asserts log was called and the event was not forwarded. Add an equivalent test feeding e.g. {type:'plan.submitted', data:{plan:{id:'plan-1'}}} (missing required fields) and assert: (1) yielded stream is empty, (2) log called once with the parse error, (3) assistantParts() returns []. The parser-level coverage in tests/plans.test.ts L44-49 protects the contract, but the producer's plumbing (log invocation, no-yield, no-persist) is unverified at this layer.

🟡 LOW Plan revisions with same planId overlay silently in persisted projection — src/chat-routes/sandbox-producer.ts

recordPersistedPart(planToPersistedPart(parsed.value), undefined) keys solely on planId (via planPartKey at stream-normalizer.ts L226), so a second plan.submitted for the same planId with revision: 2 overlays revision 1 — only the latest revision is retained in assistantParts(). mergePersistedPart's status-transition guard (stream-normalizer.ts L315-328) prevents pending→approved rollback from a stale submission, so this is safe for the common case. But there's no test for the revision-overlay case and no explicit decision documented that 'only latest revision persists' is the contract. If the SDK ever emits incremental revisions of the same planId within one turn, history is lost silently. Likely correct behavior (matches supersession semantics in /plans), but worth ei

🟡 LOW No regression test for valid answers round-trip on an interaction part — src/chat-store/parts.ts

answers?: InteractionAnswers was just added to the interface (line 170) and is the field the new persistedPartToInteraction validator checks via parseInteractionAnswers. parts-projection.test.ts:16 exercises the no-answers case and :38 exercises the invalid-answers case ({ q0: 'invalid' } — string, not string[]), but no case exercises the happy path of a stored interaction part with answers: { q0: ['a1'] } round-tripping through toChatMessageParts and surviving the new validator. Since this is the exact surface the new validation logic was added for, add one positive case so a future regression of parseInteractionAnswers integration is caught here, not

🟡 LOW Stricter interaction validation silently drops previously-persisted rows with off-enum statuses — src/chat-store/parts.ts

Old code: str(part.id) && str(part.kind) && str(part.title) && str(part.status) && part.answerSpec && typeof part.answerSpec === 'object' accepted ANY status string and ANY object-shaped answerSpec. New code delegates to persistedPartToInteraction, which requires status ∈ {pending,answered,declined,cancelled,expired} (contract.ts:350) AND answerSpec.fields to be an array (contract.ts:348) AND answers (if present) to parse. Old rows in stores with a stray status (e.g. an uppercased 'PENDING', a future status, or answerSpec without a fields array) will now be silently dropped on read at turn-routes.ts:692, disappearing from the transcript. The tightening is desirable, but it is an undocumented backward-compat break in the read path — either call it out in the PR body or s

🟡 LOW plan branch merge can leak non-canonical fields (e.g. SDK id) at rest — src/chat-store/parts.ts

return plan ? ({ ...part, ...planToPersistedPart(plan) } as ChatPlanPart) : null keeps every original key and overlays the canonical form. parsePlan reads planId ?? id, so an SDK-style row carrying id (not planId) gets planId written but id is retained on the runtime object even though ChatPlanPersistedPart/ChatPlanPart have no id field — the as ChatPlanPart cast hides it. The interaction branch one line above does NOT merge (it just casts part), so the two storable-kind branches behave inconsistently. Persisted junk keys survive the typed boundary. Fix: either return planToPersistedPart(plan) directly (drop the merge, matching the intent in plans/index.ts:14-16 that planId exists precisely to avoid colliding with a part id), or apply the same merge normalize-

🟡 LOW New barrel line lacks an inline rationale comment — src/index.ts

The added export * from './plans/index' sits between ./interactions and ./missions with no comment, while surrounding lines (e.g. lines 21-23 for chat-store, 37-39 for theme-contract, 41-49 for app-auth/chat-routes/web-react) each explain WHY they are or are not re-exported (peer-dep boundaries, browser-clean vs node-touching). The project rule is explicit in AGENTS.md ('Additive subpaths … new capability = new ./subpath … Never a breaking change to an existing export') and /plans qualifies as browser-safe pure mechanism (import-free, listed in BROWSER_NONREACT). A one-line comment such as `// /plans is browser-safe pure projection (import-free); re-exported for ergon

🟡 LOW parseInteractionAnswers error strings interpolate user-controlled key names — src/interactions/contract.ts

Errors include interaction answers contain an unsafe field key: ${key} (line 87) and interaction answer ${key} must be a string array (line 90). The regex rejects keys with non-[A-Za-z0-9_-] chars, but unsafe keys reaching the unsafe-key branch include __proto__, constructor, prototype (the explicit denylist) — all safe substrings, but the pattern of interpolating raw user input into error strings establishes a log-injection vector if a product logs the error verbatim. Bounded impact here because the regex + denylist constrain what

🟡 LOW stampInteractionAnswers/interactionToPersistedPart throw TypeError on bad answers with no internal guard — src/interactions/contract.ts

Both interactionToPersistedPart (line 309) and stampInteractionAnswers (line 335) throw TypeError when parseInteractionAnswers fails. No internal src/ callers (grep confirms exports only); products are expected to pre-validate. The throw is the documented fail-loud contract, but a product passing unvalidated persisted data (e.g. reading a legacy row) would see the error propagate unhandled up a render or persist path. Fix: document the throw contract in JSDoc, or have callers wrap. Not blocking since InteractionPersistedPart.answers is newl

🟡 LOW duplicateRequests passed to beforeAnswer can diverge from duplicates actually POSTed — src/interactions/route.ts

duplicateRequests is computed from the BEFORE snapshot (route.ts:223-227) and handed to beforeAnswer as 'Content-identical questions that the shared route will also answer' (contract JSDoc route.ts:140). But the duplicates the route actually POSTs come from a SEPARATE remaining snapshot taken AFTER the first respondToSessionInteraction (route.ts:252-257). If the sidecar state changes between the two GETs (a duplicate gets cancelled, a new duplicate is emitted, the first POST itself removes one), the product's beforeAnswer persisted an answer for a duplicate that was never POSTed, or missed one that was. Impact: minor at-most-one-row inconsistency in the product's persisted answers; reconciled on next list. Fix: either compute duplicates once and reuse, or rename the field to 'duplicateRequ

🟡 LOW Timestamp fields are string-only; SDK emitting Date/number drops the plan — src/plans/index.ts

submittedAt/decidedAt/supersededAt/withdrawnAt all flow through requiredString, which rejects non-strings (typeof value !== 'string'). If the SDK emits numeric epoch millis or Date objects (both common in JS SDKs), parsePlan returns null and the producer drops the event with the same 'malformed plan' log. No SDK type to verify against. Low severity because the shell owns the contract until the SDK exports a Plan type, and ISO-8601 strings are a reasonable default — but worth a defensive coercion (typeof === 'number' → new Date(value).toISOString()) or a documented invariant.

🟡 LOW mergePersistedPart can pollute a plan part with fields from a conflicting terminal state — src/plans/index.ts

canTransitionPlanStatus enforces monotonic status (pending → non-pending only). In stream/stream-normalizer.ts:315-328 the merge guard uses overlayDefined(existing, incoming) THEN conditionally resets merged.status — but the overlay already copied incoming's terminal-specific fields onto the record. Example: existing={status:'approved', decidedAt:'X'}, incoming={status:'rejected', decidedAt:'Y', feedback:'no'} → merged.status is forced back to 'approved', but merged now also carries feedback:'no', producing a record that matches no ChatPlan variant. The persisted row would be type-incorrect under the ChatPlanPersistedPart union. Not exercised by tests (stream-normalizer.test.ts:217-228 only replays same-status or pending→terminal). In practice the producer only emits pending via plan.submi

🟡 LOW planFollowUpTurnId outcome enum excludes 'superseded' and 'withdrawn' — src/plans/index.ts

Signature is (planId, outcome: 'approved' | 'rejected'). The ChatPlanStatus union also has 'superseded' and 'withdrawn' as terminal outcomes. If a product ever wires a follow-up turn for a superseded or withdrawn plan, the type forbids it and forces an ad-hoc id. Not a bug today — the comment in interactions/contract.ts:224-227 explains approval is intentionally an explicit card click — but the asymmetry is unexplained here. Either widen the outcome type or add a one-line comment that follow-up turns are only meaningful for approved/rejected by design.

🟡 LOW revision >= 1 contract is asserted but not provably aligned to the (absent) SDK Plan type — src/plans/index.ts

parsePlan rejects revision < 1 (test at tests/plans.test.ts implicitly via stream-normalizer.test.ts:145 expects revision:0 → null). AGENTS.md:32 claims /plans 'structurally byte-matches the durable plan returned by peer @tangle-network/sandbox's SandboxSession.plan()', but the installed peer (@tangle-network/sandbox@0.10.5) exports no Plan type and no plan() method (grep'd dist/*.d.ts — zero hits for durable plan shapes). If the SDK ever ships revisions starting at 0, every plan.submitted event silently drops with log 'plan.submitted event carried a malformed plan' (sandbox-producer.ts:239) — a data-loss path with no alarm. Fix: when the SDK lands Plan types, add a compile-time MutuallyAssignable check (mirrors chat-store/parts.ts:193-194) and a byte-equality test against a fixture the SD

🟡 LOW asymmetric normalization: interaction validates-only, plan normalizes-and-validates — src/stream/stream-normalizer.ts

Interaction branch returns rawPart verbatim after validation (return persistedPartToInteraction(rawPart) ? rawPart : null); plan branch rebuilds via { ...rawPart, ...planToPersistedPart(plan) }. The plan path canonicalizes fields (drops stray status values, syncs planId from id fallback) while interaction leaves any extra fields untouched. Minor inconsistency; both are valid choices but the rationale isn't documented. Also: the previous behavior passed interaction through unvalidated — malformed interaction parts that previously rendered will now silently disappear (return null). The code comment explains this tightening for notice but not for interaction; a one-liner stating 'invalid persisted interaction parts are dropped by design' would make the behavior change explicit.

🟡 LOW interaction merge has a redundant answers preservation check — src/stream/stream-normalizer.ts

if (incoming.answers === undefined && existing.answers !== undefined) merged.answers = existing.answers is a no-op: overlayDefined (line 236) already skips undefined patch values, so merged.answers is already existing.answers when incoming.answers is undefined. Either the check defends against a future overlayDefined change worth documenting in a comment, or it should be removed as dead code. Not a bug — flagging because the asymmetry with the plan branch (which has no equivalent guard for its correlated fields) suggests one of the two is wrong.

🟡 LOW plan merge leaves inconsistent related fields when status regression is blocked — src/stream/stream-normalizer.ts

In the plan branch, when canTransitionPlanStatus(existingStatus, incomingStatus) returns false, only merged.status is reset to existingStatus. But overlayDefined already applied any other defined incoming fields. Example: existing {status:'approved', decidedAt:'2026-01-01'}, incoming {status:'rejected', decidedAt:'2026-02-02', feedback:'no'} → merged becomes {status:'approved', decidedAt:'2026-02-02', feedback:'no'} — an approved plan carrying rejection feedback. The test at line 217 sidesteps this by explicitly setting incoming fields to undefined. Same shape applies to the interaction branch ([line 297](https://github.com/tangle-network/agent-app/blob/

🟡 LOW No assertion that malformed plan.submitted events are dropped by the producer — tests/chat-routes/sandbox-producer.test.ts

The producer's plan.submitted branch (src/chat-routes/sandbox-producer.ts:238-240) explicitly logs and drops malformed plans via parsePlanSubmittedEvent's succeeded:false path — directly parallel to the malformed-interaction drop asserted 36 lines below at line 177 ('forwards error and cancel events verbatim and drops malformed interactions'). The new test only covers the happy path. Adding a sibling case feeding { type:'plan.submitted', data:{ plan: { id:'x' } } } (missing revision/body/submittedAt) and asserting expect(await drain(producer.stream)).toEqual([]) + expect(producer.assistantParts?.()).toEqual([]) would close the symmetric covera

🟡 LOW turn-routes plan test does not verify the plan.submitted event is streamed to NDJSON output — tests/chat-routes/turn-routes.test.ts

The test feeds { type: 'plan.submitted', data: { plan } } as a producer stream event and asserts only on rows[assistant].parts. The readLines(...) return value is discarded. The engine framing is expected to forward plan.submitted through to the client (so /web-react can render a plan card); a one-liner expect(lines.find((l) => l.type === 'plan.submitted')).toBeTruthy() would lock that contract. The existing 'streams the turn' test at line 95 sets the precedent of asserting producer events appear in NDJSON. Persistence-only assertions risk silently regressing the stream lane.

🟡 LOW Duplicate-list coverage is only the with-duplicates shape — tests/interactions-route.test.ts

Every beforeAnswer test that asserts duplicateRequests sets up ask-2 as a content-identical duplicate (args.duplicateRequests → ['ask-2']). No test asserts args.duplicateRequests is [] when the snapshot has only the answered ask — the empty-array branch at route.ts:223-227 is exercised only implicitly via the 'does not answer or unblock when beforeAnswer fails' test (which uses a single-ask sidecar but never reads duplicateRequests). Add a one-line expect(args.duplicateRequests).toEqual([]) assertion to a single-ask beforeAnswer variant to lock the empty-path contract.

🟡 LOW beforeAnswer happy-path test does not assert args.request is the original Request — tests/interactions-route.test.ts

BeforeInteractionAnswerArgs.request (route.ts:131) is part of the public seam contract and is forwarded at route.ts:232, but the test asserts args.body/answer/outstanding/answeredRequest/duplicateRequests/connection — never args.request. A regression that dropped the request field would not fail this suite. Add expect(args.request).toBe(request) (capturing the request passed to route.answer) or assert identity against a module-level Request constant.

---

_{tangletools · 2026-07-21T06:39:45Z · trace}

[tangletools]

✅ Approved — 21 non-blocking findings — 3929aed8

Full multi-shot audit completed 8/8 planned shots over 18 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary

---

_{tangletools · 2026-07-21T06:39:45Z · immutable trace}

[tangletools]

🟡 Value Audit — sound-with-nits

Verdict	sound-with-nits
Concerns	4 (4 weak-concern)
Heuristic	0.0s
Duplication	0.0s
Interrogation	547.2s (2 bridge agents)
Total	547.2s

💰 Value — sound-with-nits

Adds a reusable server workflow + browser cards that make plan decisions and interaction answers survive crashes/reloads; well-seamed and grain-aligned, with two minor architectural nits worth a human glance.

• What it does: Introduces three things. (1) A new ./durable-chat subpath: a server-side DurablePlanStore port with CAS claims for plan commands, plan effects, and answer intents; createDurablePlanRoutes() (GET current + POST decide with authority reconciliation and effect retry); createDurableInteractionSettlement() (prepare/ack/finalize/abort lifecycle); an InMemoryDurableChatStateStore reference adapter; and b
• Goals it achieves: Make plan decisions and accepted interaction answers survive worker restarts, browser reloads, ambiguous retries, and reconnects WITHOUT each product re-implementing the state machine. The product supplies storage + auth + sidecar connection + idempotent afterDecision effect; the shell owns the protocol (idempotency keys, CAS, receipt shapes, prepare→ack→finalize ordering, tombstones). Eliminates
• Assessment: Fits the codebase grain on every invariant that matters: new capability = new additive subpath (rule 5 ✓), structural port with no product imports (rule 2 ✓), /plans is substrate-free (rule 4 ✓), server-only logic stays out of browser bundles, and the /interactions seam is extended additively (DurableInteractionRoutePersistence is opt-in). The CAS-claim pattern echoes /missions but uses typed per-
• Better / existing approach: No materially better architecture found; this is the right shape. Searched for: (a) overlap with /missions' MissionStorePort — same CAS idea, different and well-typed-for-its-domain port, not a true duplicate; (b) overlap with /billing's KeyProvisioner/WorkspaceKeyStore/KeyCrypto — different concern (no crypto/storage seams here); (c) overlap with the sandbox SDK's durable primitives — AGENTS rule
• Model: opencode/zai-coding-plan/glm-5.2
• Bridge attempts: 2
• Bridge warning: opencode/kimi-for-coding/k2p7: bridge stream ended without value-audit content

🎯 Usefulness — sound-with-nits

A coherent, additive durable plan/interaction workflow layer that consolidates three fleet forks (gtm/legal/tax), fits the structural-port grain, and is reachable through every documented seam — with one docs gap where the canonical example wires interaction projection but not plan projection.

• Integration: Fully reachable. /durable-chat and /plans are in package.json exports and re-exported from src/index.ts:30-31. The sandbox producer accepts an optional interactionProjection (src/chat-routes/sandbox-producer.ts:60,233,272,307), /interactions accepts an optional durable persistence seam (src/interactions/route.ts:183), and ChatMessages.durableCards renders the new cards from persi
• Fit with existing patterns: Matches the AGENTS.md 'engine=peerDep, app-shell=compose' rule exactly. DurableChatScope is a branded opaque type created only after auth (src/durable-chat/types.ts:13-18), the store is a structural port with CAS-shaped claim/finalize verbs, and InMemoryDurableChatStateStore is explicitly marked non-production (src/durable-chat/memory.ts:17-21). This is the same shape as /billing, `/tang
• Real-world viability: Handles the realistic edge cases: tombstone-before-ask so a late duplicate ask cannot reopen a cancelled card (src/durable-chat/interactions.ts:61-68), competing-decision detection across the scope (src/durable-chat/memory.ts:111-118), stale plan revision rejection (src/durable-chat/plan-routes.ts:243-245), reconcile-on-ambiguous-retry when the sidecar says the ask is gone (`src/interactions
• Model: opencode/zai-coding-plan/glm-5.2
• Bridge attempts: 1

💰 Value Audit

🟡 Two parallel CAS-store ports now exist (/missions + /durable-chat) with no shared primitive [duplication] ``

/missions/service.ts:145 (MissionStorePort with guarded update + conflict) and /durable-chat/types.ts:116 (DurablePlanStore with claimPlanCommand/claimAnswerIntent/claimPlanEffect + in-memory reference at memory.ts:22) implement the same CAS-with-conflict-detection idea twice. The per-resource typed claims in /durable-chat are more ergonomic for plan/answer semantics than a generic update would be, so this is not a refactor-now situation — but if a fourth or fifth such port appears, the typed-cl

🟡 1500+ LOC of new shell ships with no in-repo consumer; verification deferred to GTM PR #591 [maintenance] ``

grep shows createDurablePlanRoutes / createDurableInteractionRoutePersistence / DurablePlanStore / withDurableChatProjection / createDurableChatEventProjection appear only in their own module + tests, never consumed inside this repo. AGENTS.md 'When you add a module' step 5 says 'prove it on a reference consumer — it stays green.' examples/chat-app.md:177-210 is a code snippet in markdown, not a working consumer. Shell-first is an accepted pattern in this codebase and the prior fix (3929aed) alr

🟡 Two overlapping projection paths in /chat-routes (inline adapter vs generic wrapper) [better-architecture] ``

createSandboxChatProducer accepts options.interactionProjection (src/chat-routes/sandbox-producer.ts:57-63, fires only for interaction events) AND withDurableChatProjection wraps any producer to observe+materialize arbitrary events (src/chat-routes/durable-projection.ts:13-40, what createDurableChatEventProjection feeds). A consumer that wants durable plans AND interaction projection today must wire BOTH, and the example at examples/chat-app.md:188-191 only shows the inline adapter (no plan proj

🎯 Usefulness Audit

🟡 Canonical example wires interaction projection but not plan projection — plan decide will 503 on a literal follower [ergonomics] ``

examples/chat-app.md:186-191 wires createDurableInteractionProjectionAdapter into createSandboxChatProducer({ interactionProjection }). But the sandbox producer's interactionProjection option only projects interaction events — src/chat-routes/sandbox-producer.ts:277-286 shows plan.submitted only calls recordPersistedPart(planToPersistedPart(...)) (chat-store parts), with no call to seed the durable plan store. So createDurablePlanRoutes.decide() will hit its own guard at `src/dur

---

What this audit checks

It judges the change on its merits — not whether it was tasked out in an issue. Unticketed, fast-moving work is fine; the question is whether the change is good and whether a better or existing approach should be used instead.

Pass	What it asks
Heuristic	Vague title? Whitespace-only or cruft-bearing diff? (content signals only)
Duplication	Do added function/class names already exist elsewhere in the repo?
Value Audit	What does it do? What goal does it achieve? Is it good? Better architecture or already-exists?
Usefulness Audit	Does it integrate and fit? Will it hold up in real use and actually get used?

Findings are concerns, not blocks — the human reviewer decides what to do with them.

_{value-audit · 20260721T074814Z}

[vutuanlinh2k2]

Addressed the actionable findings from review 4742392773: removed the overlapping interaction-only projection path; plans and interactions now share the generic durable projection wrapper; updated the canonical sandbox chat example to install both projections; added an executable producer-to-projection-to-plan-decision composition test; coalesced same-tick plan decisions; and normalized replayed responses as idempotent. The CAS helper duplication remains intentionally deferred, as suggested in the review, until another production store port establishes the right shared primitive. Verification: focused durable-chat tests, typecheck, package build, and CI build all pass. The unrelated pre-existing shell-profile/base64 failures in src/sandbox/index.test.ts remain unchanged.

[tangletools]

✅ No Blockers — 5f1b8613

Review health 100/100 · Reviewer score 0/100 · Confidence 95/100 · 41 findings (5 medium, 36 low)

glm: Correctness 0 · Security 0 · Testing 0 · Architecture 0

Reviewer score is advisory once the run is complete and the verdict has no blockers.

Full multi-shot audit completed 8/8 planned shots over 46 changed files. Global verifier still owns final merge decision.

🟠 MEDIUM Projection failures kill the live chat stream (no try/catch around interactionProjection and withDurableChatProjection.observe) — src/chat-routes/sandbox-producer.ts

Lines 233, 272, 307 call await options.interactionProjection?.{upsertAsk,cancel,materialize}() without any try/catch. Line 233 in particular sits between recordPersistedPart and yield: if the durable store throws (transient D1/Postgres outage, conflict error that isn't DurableChatConflictError, network blip), the generator re-throws, the chat turn dies, and the user sees a broken stream even though the model is still running. This is asymmetric with declineInteraction ([lines 239-247](https://github.com/tangle-network/agen

🟠 MEDIUM decide masks a conflicting authority terminal as 503 instead of 409 — src/durable-chat/plan-routes.ts

When options.authority.decide throws and options.authority.current returns a plan whose terminal decision DIFFERS from parsed.decision (or has no terminal), the route returns 503 'DURABLE_CHAT_UNAVAILABLE / plan authority unavailable'. The user can retry the same approval indefinitely, always seeing 503, never learning the authority has already rejected (or otherwise moved on). The local projection was never put (we threw before putPlanProjection), so the next attempt re-enters the same branch. Fix: when reconciled.plan has terminal !== parsed.decision, return 409 'DURABLE_CHAT_CONFLICT' with the actual state; only return 503 when reconciled.plan is null/pending (genuine retry-worthiness). Test coverage for this branch is also absent.

🟠 MEDIUM localStorage QuotaExceededError escapes the submitter silently — src/web-react/durable-interaction-submit.ts

Line 98 options.attempts.set(submission.id, signature, attemptKey) runs BEFORE the try block on line 104. createSessionInteractionAttemptStore.set calls storage.setItem (durable-interaction-submit.ts:41), which throws QuotaExceededError when localStorage is full. That exception propagates out of the submitter entirely; the catch on line 124 never runs. The card's `

🟠 MEDIUM restoreChatInteractions behavior change can leave phantom pending cards for existing consumers — src/web-react/use-chat-interactions.ts

Pre-PR: restoreChatInteractions marked every previously-pending ask NOT in the sidecar's outstanding list as answered. Post-PR: those asks stay pending until a durable projection arrives via the new hydrate() (line 145). The new behavior is documented as deliberate ('absence cannot distinguish answered, withdrawn, expired, or a temporarily unavailable registry'), and is correct in principle — but any existing consumer of useChatInteractions that relied on auto-settle will see 'phantom' pending cards after reload until they also wire hydrate. The PR adds the mechanism but, within this shot's 16 files, no consumer integration. The behavior chang

🟠 MEDIUM No test covers combined beforeAnswer + durable configuration — tests/interactions-route.test.ts

route.ts:275 gates beforeAnswer on options.beforeAnswer && answeredRequest; route.ts:250 short-circuits to 410 only when beforeAnswer && !durable && !answeredRequest. With BOTH configured and the ask absent from the sidecar snapshot, beforeAnswer is silently skipped and the durable reconcile path runs instead (route.ts:301-314). The existing suite tests beforeAnswer-alone (line 278, 315, 437, 454) and durable-alone (335, 361, 378, 398, 417) but never the combined case. The interaction is non-obvious — a product wiring beforeAnswer for audit + durable for crash-safety would reasonably expect beforeAnswer to fire on every successful answer, and would not noti

🟡 LOW ChatRouteDurableProjection.observe types events as unknown, losing type safety — src/chat-routes/durable-projection.ts

observe(event: unknown) discards the StreamEvent type the wrapper actually passes (durable-projection.ts:20 yields events from producer.stream which is typed AsyncIterable<StreamEvent> via ChatTurnRouteProducer). The durable-chat adapter (src/durable-chat/adapters.ts:111) re-validates with eventRecord() because it cannot trust the shape. Typing observe as (event: StreamEvent) => ... would let implementations skip re-validation. Minor — no runtime impact.

🟡 LOW withDurableChatProjection does not run materialize on producer-stream throw — src/chat-routes/durable-projection.ts

If producer.stream throws (or the consumer abandons it via return()), projected = await projection.materialize() at line 23 is never executed, so wrapped.assistantParts() only reflects the producer's parts (no terminal interaction/plan states). Probably acceptable on the throw path (the turn failed anyway), but on early-return (consumer disconnect) the product may want terminal parts persisted via the projection's store independently — which they are (the store writes happened in observe); only the assistant-parts merge is skipped. Worth a one-line code comment confirming this is intentional.

🟡 LOW attemptKey length validation lives only at the settlement seam, producing a generic 503 at the route boundary — src/durable-chat/interactions.ts

ensureAttemptKey throws TypeError when attemptKey.length > 512. The /interactions route (src/interactions/route.ts:223) only checks typeof body.attemptKey === 'string' && attemptKey.trim(), so a 10 KB attemptKey passes the route and fails inside settlement.prepare, surfacing as 503 'INTERACTION_PREPARE_FAILED' rather than 400 with the specific reason. Move/repeat the length+non-empty check at the route boundary (or in validateInteractionAnswerBody) so malformed attempts get a clear 400.

🟡 LOW recordPlanAuthorityResult signature in memory.ts is wider than the types.ts port — src/durable-chat/memory.ts

types.ts declares recordPlanAuthorityResult(scope, commandKey, result, receipt: DurableFollowUpReceipt): Promise<void> (receipt required). memory.ts declares the same method with receipt: DurablePlanCommandRecord['receipt'] which is DurableFollowUpReceipt | undefined. The reference impl is wider than the port contract; a production adapter copying this signature could silently persist a command without a receipt. Tighten memory.ts to match the port. (Caller in plan-routes.ts:313 always passes authorityResult.receipt which is guaranteed by the intersection type, so this is a signature drift only, not a runtime bug.)

🟡 LOW GET /plans/:id has side effects (runs afterDecision) on every terminal-decision read — src/durable-chat/plan-routes.ts

current() calls runEffect whenever the plan has a terminal decision and a stored/synthetic receipt, so a GET can trigger afterDecision (e.g., notifications, billing tier assignment) on every retry until it succeeds. This is intentional ('heal a pending decision effect from local state' test #204 covers it) and the contract documents afterDecision must be idempotent by effectKey, but it is a GET with side effects that surprises REST norms. Note for product adopters: a flooded GET retry against a failing downstream can amplify load. No code change required if the contract is honored; worth a one-line note in the JSDoc for current().

🟡 LOW parseDecisionBody accepts 'status' as an alias for 'decision', inviting accidental approvals — src/durable-chat/plan-routes.ts

normalizePlanDecision(record.decision ?? record.outcome ?? record.status) lets a client send {planId, revision, status: 'approved'} and have it interpreted as a decision. 'status' is also the field name on the plan projection itself, so a payload accidentally echoed back (e.g., a cached projection) silently becomes a decision. outcome and decision are reasonable aliases; status should be rejected to keep the surface tight.

🟡 LOW Dead duplicate JSDoc above InteractionAnswerValue — src/interactions/contract.ts

contract.ts:72-75 has two adjacent JSDoc blocks: /** Accepted field selections keyed by answer-spec field name. */ followed immediately by /** Accepted answer values. ... */. Only the second binds to the type; the first is leftover from a rename and is misleading (the type is values, not selections). Delete the stale line.

🟡 LOW parseInteractionAnswers imposes no per-value size bound — src/interactions/contract.ts

parseInteractionAnswers accepts arbitrarily long string values and arbitrarily large string arrays. Combined with interactionToPersistedPart this is the path by which those values land in messages.parts. A malicious or buggy client could POST a multi-MB string in data., have it accepted by validateInteractionAnswerBody (which also has no size limit), persisted as an answer, and stamped onto every matching transcript part. The 700KiB / 1MiB gateway cap in /chat-routes upload route is unrelated — the answer channel has no equivalent. Worth at least documenting that products must enforce their own cap, or adding a conservative default (e.g. 64KiB per value, 256 entries per array).

🟡 LOW persistedPartToInteraction drops whole card when part.answers is invalid — src/interactions/contract.ts

contract.ts:358 adds (parsedAnswers && !parsedAnswers.succeeded) to the null-return guard, so a persisted part whose answers field fails parseInteractionAnswers causes the entire interaction card to disappear from the transcript (null is the documented 'not one of ours' signal and callers log+skip). This is asymmetric with interactionToPersistedPart (contract.ts:315), which throws TypeError on the same input. The codec prevents bad writes in normal operation, but the failure mode matters for schema migrations, cross-version reads, or direct DB writes: the user silently loses the card. Either drop just the answers field (return the part without answers) or surface a dedicated log so the silent drop is observable. Matches the file's 'never a half-rendered card' philosophy only if the c

🟡 LOW Misleading log message when durable.acknowledge throws — src/interactions/route.ts

route.ts:377-393 wraps both acknowledge and finalize in one try-block and logs [interactions] durable finalize failed: for either failure. When acknowledge is the thrower, the message wrongly names finalize. The client-facing 503 INTERACTION_FINALIZE_FAILED is reasonable for both (the user just retries), but the log should distinguish the phase so an operator debugging a stuck intent knows which call to investigate. Split the try-blocks or include a phase field in the logged error.

🟡 LOW Silent swallow of durable.fail errors can leak prepared intents — src/interactions/route.ts

route.ts:333-338 catches and discards any throw from options.durable.fail?.(). The comment justifies this (upstream failure stays authoritative), but the cost is that a prepared intent record can be left dangling in the store when fail throws. For a reconciled guarantee that is acceptable — a future retry with the same attemptKey will hit reconcile and settle — but for a best-effort durable the leak is permanent. At minimum log the swallowed error so an operator can reconcile manually; consider surfacing it as a header on the response.

🟡 LOW attemptKey is trimmed but unbounded at the route layer — src/interactions/route.ts

route.ts:223 only checks truthiness after trim. The reference durable impl enforces length <= 512 in durable-chat/interactions.ts:32, but a product supplying its own DurableInteractionRoutePersistence gets no route-layer guard — a 10MB attemptKey would be passed through to prepare/reconcile/acknowledge/finalize and persisted into the intentKey (durable-chat/interactions.ts:28 builds a string containing the raw attemptKey). The web-react client generates UUIDs so this is bounded in practice, but the contract is the route's to defend. Either bound length here (e.g. 512 to match the reference impl) or document that custom durable impls must enforce their own limit.

🟡 LOW Metadata object is shared by reference, not cloned — src/plans/index.ts

metadata: record.metadata as Record<string, unknown> stores the source event's metadata object by reference. A downstream consumer that mutates parsed.metadata.foo = bar silently mutates the original event record, which is shared across other projections in the same stream (see src/stream/stream-normalizer.ts:315-333 mergePersistedPart and src/chat-store/parts.ts:264). The doc says metadata is 'deliberately opaque,' but opacity does not imply immutability. Suggest a shallow clone ({ ...record.metadata }) or freeze to prevent cross-event aliasing.

🟡 LOW Stable key namespace can collide for adversarial planId values — src/plans/index.ts

Keys produced: plan:{id}, plan:{id}:revision:{n}, plan:{id}:approved|rejected. A planId literally containing :revision:2 or :approved would produce a colliding string. UUIDs/snowflake ids (the normal case) avoid this, but there is no validation that planId is collision-safe. Low risk in practice; worth either a comment asserting the id format contract or a separator that cannot appear in the id.

🟡 LOW Untested allowed transition branches in canTransitionPlanStatus — src/plans/index.ts

tests/plans.test.ts:79-88 covers same→same, pending→approved, approved→pending (false), approved→rejected (false), pending→preparing (false), preparing→pending (true), but NOT preparing→superseded or preparing→withdrawn, both of which are allowed by line 138. If those branches ever regress, no test catches it. Add two asserts to close the gap.

🟡 LOW canTransitionPlanStatus doc comment contradicts implementation — src/plans/index.ts

Comment says 'Plan status is monotonic: only a pending plan can settle,' but the body (lines 138-139) ALSO allows preparing→pending, preparing→superseded, and preparing→withdrawn. Either tighten the comment to describe the full rule or split into 'preparing may progress to pending or be retired via superseded/withdrawn; pending may settle to approved/rejected or be retired via superseded/withdrawn; terminal states are sticky.' Misleading doc on a status-machine helper risks callers misusing it (e.g., assuming preparing→superseded is invalid and adding a redundant guard).

🟡 LOW revision<1 rejects zero without a cited SDK contract — src/plans/index.ts

revision < 1 rejects 0. The doc string does not state the SDK guarantees revisions start at 1. If any SDK path ever emits revision 0 (e.g., a draft or pre-submission placeholder), the event is silently dropped as malformed via the parsePlan null return, surfaced only as 'plan.submitted event carried a malformed plan.' Either cite the SDK contract in the comment or loosen to revision < 0/!Number.isInteger(revision) if zero is a legitimate sentinel.

🟡 LOW DurablePlanCard and DurableChatCards React components are exercised only by one integration test — src/web-react/durable-plan-card.tsx

DurablePlanCard's local validation (if (decision === 'rejected' && !trimmed) → setLocalError) at line 49-52 is not unit-tested. The expand/collapse behavior, the local-error-vs-prop-error precedence (localError ?? error), and the action button disabled/label states for deciding='approved'|'rejected' have no direct tests. DurableChatCards is only tested via durableChatCardsFromParts (pure function) plus the single render in chat-messages-durable.test.tsx. Adequate core-mechanism coverage; gap on UI affordances.

🟡 LOW createDurablePlanDecisionClient.current only tested for absolute URLs — src/web-react/durable-plan-flow.ts

The path-only branch (${url.pathname}${url.search}) is untested. new URL(rawUrl, globalThis.location?.origin ?? 'http://localhost') resolves relative paths against the base — for rawUrl = 'api/plan' (no leading slash) on a host whose real path is nested, the resolved pathname may surprise consumers. Absolute paths (/api/plan) and full URLs work. The test at durable-plan-flow.test.tsx:50-62 only exercises the absolute URL path. Either document that consumers MUST pass an absolute path or full URL, or add a path-only test.

🟡 LOW useDurablePlanFlow error and idempotency paths untested — src/web-react/durable-plan-flow.ts

DurablePlanFlow's error paths are uncovered: (1) decide() returning null when deciding is already set (line 201); (2) the DurablePlanClientError branch at line 214-217 where cause.currentPlan triggers setPlan + onUpdated (intended recovery); (3) clearError(). The two existing tests cover only happy-path attach + concurrent coalescing. The currentPlan fallback is a meaningful behavior — a 409 from the server with the authoritative plan should update local state — and should have a test.

🟡 LOW useDurablePlanFlow useEffect can overwrite a fresh server plan with a stale prop — src/web-react/durable-plan-flow.ts

useEffect(() => setPlan(options.plan), [options.plan]) unconditionally resets local state whenever the parent's plan reference changes. After a successful decide, apply() calls setPlan(result.plan) with the authoritative server plan; if the parent then re-renders with a stale options.plan (same planId, older status — e.g., before the parent's own onUpdated state propagates), the optimistic/authoritative update is clobbered. Standard React sync footgun. Fix: gate the effect by an identity field (e.g., compare ${planId}:${revision}:${status} and skip when local is at least as new).

🟡 LOW Terminal-merge spread in upsertChatInteraction can clobber existing defined fields with undefined — src/web-react/use-chat-interactions.ts

Line 62 next[index] = { ...existing, ...interaction } spreads interaction over existing unconditionally. If a future caller constructs interaction with an own-property set to undefined (e.g., body: undefined), it would overwrite existing.body. Today's persistedPartToInteraction (contract.ts:348-369) conditionally omits unset fields, so the bug is not triggered — but the merge is brittle. Defensive fix: filter to defined own-properties, or {...existing, ...pickDefined(interaction, ['answers','cancelReason'])} to limit the merge surface to the two fields the guard actually intends to enrich.

🟡 LOW Negative arrayContaining assertion is correct but hard to read — tests/chat-routes/durable-projection.test.ts

expect(wrapped.assistantParts?.()).not.toEqual(expect.arrayContaining([expect.objectContaining({ id: 'ask-old' })])) is a double-negative through asymmetric matcher composition. It works (verified: the projection's observed set excludes 'ask-old' so materialize never returns it), but a reader must unpack three levels of matcher semantics to confirm the intent. Prefer: const ids = wrapped.assistantParts?.().map(p => p.id); expect(ids).not.toContain('ask-old') — same guarantee, one level of negation.

🟡 LOW No test for non-DurableChatConflictError thrown from projection.observe — tests/chat-routes/durable-projection.test.ts

The third test proves DurableChatConflictError is swallowed (cancel-on-answered, pending-after-approved), which is the common-case resilience. But adapters.ts:126 and :142 re-throw any non-DurableChatConflictError, which would abort the wrapped stream mid-drain in withDurableChatProjection (the await projection.observe(event) sits inside the for-await before yield). This fail-loud behavior for unexpected store errors is correct by design, but it is untested — a regression that accidentally broadens the catch (e.g., to Error) would silently hide real failures and no test would catch it. Consider one test asserting a synthetic store error propagates out of the drained stream.

🟡 LOW interactionProjection.cancel callback never tested positively — tests/chat-routes/sandbox-producer.test.ts

The new test 'lets a durable projection materialize acknowledged answers' asserts expect(projection.cancel).not.toHaveBeenCalled() but no test exercises the positive cancel path — i.e., feed an interaction.cancel event with a projection wired, then assert projection.cancel was invoked with the parsed InteractionCancelData. The cancel handler at sandbox-producer.ts:272 is reachable and works (verified by trace), but test coverage is one-sided. Add a small test mirroring the upsertAsk one but with a cancel event.

🟡 LOW turn-routes plan-persistence test is single-scenario and structurally circular — tests/chat-routes/turn-routes.test.ts

The test uses planToPersistedPart(plan) both as the producer's assistantParts() return AND as the expected persisted shape. If planToPersistedPart had a field-mapping bug, both sides would carry it and the test would still pass. The assertion really verifies the route plumbing (producer.assistantParts → toChatMessageParts → appendMessage.parts), not the plan codec itself. Acceptable since plans.test.ts covers the codec, but the test name 'durable plan part' implies more than it proves. A second case feeding a raw record (not pre-coded) through the producer would close the gap.

🟡 LOW Persistence seam never receives non-empty duplicateIds and never exercises reconcile/abort — tests/durable-chat.test.ts

Test 12 calls prepare/acknowledge/finalize with duplicateIds: []. The adapter at adapters.ts:222-235 IGNORES duplicateIds entirely (acknowledge/finalize only use prepared), so duplicate answered asks are not reflected in the durable projection — but no test asserts that gap, and no test exercises a non-empty duplicateIds path. Additionally, persistence.reconcile(...) (the ambiguous-retry path used by route.ts:304 and route.ts:322 when sidecar says 404) is never called, persistence.fail(...) (route.ts:334) is never called, and settlement.abort(...) is never called. The reconciled-guarantee happy path is the only one covered.

🟡 LOW Plan-route error paths (401/400/410/503-unavailable) are not exercised in this file — tests/durable-chat.test.ts

Every test supplies authorize: async () => scope, a valid body, and a reachable authority. The branches at plan-routes.ts:83 (401 UNAUTHORIZED), plan-routes.ts:173 (400 MISSING planId), plan-routes.ts:199 (410 GONE when authority.plan is null), plan-routes.ts:235 (400 BAD_REQUEST body validation), plan-routes.ts:241 (503 missing local projection), and plan-routes.ts:305-308 (503 when current() also throws after decide() throws) have no test in this file. Likely covered in sibling suites but the durable-chat-specific surface is bare here. Add at minimum: missing-planId GET, unauthorized authorize, and authority-current-throws-only (no local terminal) to lock the contract.

🟡 LOW Recovery after putPlanProjection failure is not asserted — tests/durable-chat.test.ts

FailingProjectionStore.fail=true also prevents finalizePlanCommand from running (it sits in the same try as putPlanProjection at plan-routes.ts:320-326), leaving the command in state 'claimed' with no stored receipt and the local projection still 'pending'. The test asserts the immediate 200 + projectionPending response but does not issue a follow-up GET/POST to prove the route re-converges (the authority's idempotent decide should heal it). Without that follow-up, a regression that returns 503 on the next call (because local is still 'pending' but the command is half-claimed) would not be caught here.

🟡 LOW Test 1 omits the logger mock, leaking the expected afterDecision error to stderr — tests/durable-chat.test.ts

Tests 4 and 10 pass logger: { warn: () => {}, error: () => {} } to suppress expected warnings; Test 1 does not, so vitest prints [durable-chat] afterDecision failed: Error: effect down to stderr on every run (visible in the reporter output). Cosmetic, but it adds noise to CI logs and is inconsistent with the other retry tests in the same describe block.

🟡 LOW Test 13 retry assertion only checks intentKey, not record identity or state — tests/durable-chat.test.ts

const retry = await settlement.prepare(...); expect(retry.intentKey).toBe(prepared.intentKey). The memory store returns the existing record (return { status: 'existing', record: existing }) on idempotent re-prepare, but the test does not assert retry === prepared or retry.state === 'prepared'. A conflicting payload (different outcome/data) must throw DurableChatConflictError — that branch is also untested. Tighten to: expect(retry).toBe(prepared) plus a await expect(settlement.prepare(scope, 'ask-3', 'declined', { other: true })).rejects.toThrow() to lock the conflict path.

🟡 LOW Test gaps for new durable-chat seams: abort path, best-effort guarantee, conflicting-terminal recovery — tests/durable-chat.test.ts

12 tests cover happy paths and key idempotency, but no test exercises: (a) the persistence.fail/abort callback path on createDurableInteractionRoutePersistence (a sidecar failure after prepare), (b) the 'best-effort' guarantee variant of createDurableInteractionRoutePersistence (only 'reconciled' is tested at line 273), (c) the decide-route conflicting-terminal recovery branch (plan-routes.ts:304-309) which is the medium-severity finding above, (d) recoverReceipt's synthetic turnId behavior when no command journal entry exists. None of these are blockers — the contract types force them to compile — but they are exactly the failure modes a regression would silently br

🟡 LOW best-effort guarantee variant is untested — tests/durable-chat.test.ts

CreateDurableInteractionRoutePersistence and createDurableInteractionSettlement both branch on guarantee ('reconciled' | 'best-effort'), and the type union at adapters.ts:169-185 makes reconcileAuthority optional for best-effort. Only 'reconciled' appears in this file. A test that constructs the best-effort variant (and verifies finalize still records the projection with guarantee='best-effort' on the intent) would lock the second arm of the contract.

🟡 LOW Durable POST-failure reconciliation path is untested — tests/interactions-route.test.ts

src/interactions/route.ts:318-340 has a distinct branch: when respondToSessionInteraction fails AFTER durable.prepare succeeded, the route calls durable.reconcile again, returns {ok:true,idempotent:true} on settled, returns 503 INTERACTION_RECONCILIATION_PENDING on a 404 result, and invokes durable.fail?.() for non-recoverable errors. None of the 9 new tests exercise this branch — every durable test either has the sidecar POST succeed or never reaches it (empty sidecar, answeredRequest undefined → reconcile-and-return at route.ts:309). This is the branch that earns the 'crash-recoverable' label on DurableInteractionRoutePersistence; shipping it untested is the kind of gap the durable-chat consumer will hit first.

🟡 LOW durable + beforeAnswer combination untested — tests/interactions-route.test.ts

beforeAnswer is tested only without durable (lines 278-333); durable is tested only without beforeAnswer (lines 335-435). Production runs both when configured: beforeAnswer fires at route.ts:275-285 inside the same answeredRequest guard that durable.prepare later depends on. The interaction (e.g., does beforeAnswer's lifecycleArgs get reused for durableArgs? what happens if beforeAnswer throws when durable is configured — does prepare run?) is asserted by no test. Add one combined test to pin the ordering: beforeAnswer →

🟡 LOW durable.fail callback never exercised — tests/interactions-route.test.ts

DurableInteractionRoutePersistence.fail? (route.ts:170, invoked at route.ts:334) is the cleanup hook for non-recoverable POST failures. It is never asserted on, never configured on any fake. Combined with the finding above, a regression that drops the fail?.() call would not be caught. Add a test where prepare succeeds, the POST returns 500, reconcile returns {settled:false}, and assert fail was called with {prepared, error:result.error}.

---

_{tangletools · 2026-07-21T08:31:19Z · trace}

[tangletools]

✅ Approved — 41 non-blocking findings — 5f1b8613

Full multi-shot audit completed 8/8 planned shots over 46 changed files. Global verifier still owns final merge decision.

Full immutable report for this review: trace

Summary comment for this run: full summary

---

_{tangletools · 2026-07-21T08:31:19Z · immutable trace}