Skip to content

fix(chat reliability): execution authority, atomic admission, failure classification, durable runtime tools - #16

Merged
thatswiftdev merged 10 commits into
mainfrom
wt/chat-reliability
Oct 22, 2026
Merged

thatswiftdev merged 10 commits into
mainfrom
wt/chat-reliability

Conversation

@thatswiftdev

@thatswiftdev thatswiftdev commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Closes #15.

What's in this PR (all generic — no consumer vocabulary)

1. Cause-based failure classification (src/loop.ts) — LoopError carries an explicit cause (budget-hop|wall|token, caller-abort, provider, tool-invalid) and a widened code union. Provider failures keep their own codes (PROVIDER_BAD_REQUEST etc. pass through; unattributed timeouts → PROVIDER_TIMEOUT; unknown → PROVIDER_FAILED) instead of being relabeled BUDGET_EXCEEDED, which the transport treats as a resumable pause. The loop-owned wall is a separate signal; explicit Stop beats a concurrent wall expiry. Only genuine budget outcomes create a resume stash.

2. Execution authority (src/orchestrate/execution-authority.ts, new) — one executor per canonical agent DB, elected by a lifetime BEGIN IMMEDIATE on a separate <agent>.execution-control.db. Atomic durable admission (epoch check, session-active check, request idempotence via stored fingerprint, turn.request events + receipt + ledger row in one sink-connection commit, requiring the intended turn.started — queue decisions and storage failures roll back entirely). Stash peek + exact CAS consume at the execution boundary. Truthful dead-owner recovery (ONE EXECUTION_INTERRUPTED, idempotent re-recovery, ledger-only reconcile when already terminal). Durable worker rows with operator-evidenced quarantine for unproven survivors. readSession common read: legacy active folds are uncertain/legacy_owner, never silently idle.

3. Transport wiring (src/transport/conversation.ts) — additive ConversationDeps.execution: authority-driven message POST (409 busy / REQUEST_ALREADY_ADMITTED with original turnId / REQUEST_ID_REUSE / 503 quarantine-drain-recovery with no acceptance; optional clientRequestId body field), attempt-fenced durable writes, execution-boundary stash consumption, Stop-before-start as honest ABORTED, /abort with optional expectedTurnId (stale → 409), session GET with the common execution snapshot (liveTurn only for positively-known local ownership). Legacy no-authority hosts keep the process-local path unchanged. Ownership release is once and identity-guarded; the disconnect hook can no longer abort a released turn on either path.

4. Runtime tool durability + per-invocation identity (src/loop.ts) — ordinary registry tools emit per-invocation onTool call/result callbacks carrying the provider-NATIVE call id; the transport keys durable calls by invocation (tc_<turn>_i_<nativeId>), so repeated same-name invocations are distinct durable calls with their own args/results (previously: zero durable events; name-derived identity collapsed repeats). Failures record isError, never silent success.

5. Text block lifecycle durability (NEW src/provider/activity.ts + loop/turn-engine/transport) — the glued-narration fix: streamed text persists with block identity. createBlockTracker emits text.start/delta/end per assistant block; deltas forward bytes exactly (de+velop=develop, never whitespace injection); text.end carries the full streamed block text. The turn.chunk command/event gains optional additive block {id, phase, native?, text?} metadata (scalar delta always kept); legacy readers are unaffected. Multiple narration blocks in one turn stay distinct — a renderer can finally recover the boundaries the flat stream destroyed.

6. Claude CLI parser fidelity (src/provider/claude-cli.ts) — completed snapshots replace-or-complete the streamed block (no duplicated partial+full text; a cut stream gets exactly the missing tail), native tool_use ids preserved into invocationId, full results (no 160-char/first-line reduction), empty results emitted honestly.

7. Owned atomic finalization (finalizeOwned) — terminal event, projections, session token accounting, budget-stash write and ledger settle commit in ONE sink-connection transaction (Storage gains optional sync in-transaction hooks applyAccountingSync/stashPartialSync). Duplicate finalize is a no-op; a storage failure rolls back everything, leaves the attempt reserved for an idempotent retry, and the transport reports 503 FINALIZATION_PENDING — success is never reported for an uncommitted terminal. All three terminal paths route through it under an authority.

8. Drainable host tails — afterTurn may return a Promise: the POST never awaits it, but under an authority it registers as a tracked worker; the provider root run is itself tracked, so normal shutdown (drain) awaits real provider work AND host tails before releasing the control lock.

9. EventSink / Storage (additive) — passive constructors ({ passive: true } skips the boot sweep; recovery is authority-gated), handleInTransaction (caller ledger writes join the event commit; rollback discards the poisoned state cache), withTransaction (ledger writes on the sink's own connection — two connections to one file must never nest transactions), readSnapshot (one consistent state+events+lastSeq boundary), Storage.peekPartial/consumePartialExact/sync hooks.

Verification

  • pnpm run check: tsc clean, 47 files / 275 passed / 5 skipped / 0 failed at head 4cb897c (49 new tests across classification matrix, admission race, authority groups on real disk SQLite, authority-transport integration over real HTTP, tool durability/identity, finalizeOwned atomicity, drainable tails, block lifecycle, claude parser fidelity).
  • Key behaviors proven over real HTTP + real disk SQLite: concurrent same-session sends → one 200/one 409, single provider run, no phantom user row; provider 400 → envelope PROVIDER_BAD_REQUEST, no budget stash, session error; duplicate clientRequestId → 409 without re-execution; mid-turn Stop → ABORTED and session back to ready; second executor fails election; died owner → exactly one interruption terminal, re-recovery no-op; same-name double invocation → two distinct durable calls; duplicate finalize → no second terminal; budget stash commits inside the finalization transaction; afterTurn tail returns after the envelope and is awaited by drain; two-hop stream → distinct block ids with byte-exact per-block concatenation; claude fake-binary → no duplicate snapshot text, native ids preserved, empty results recorded, long results untruncated.
  • Loop wall test updated to the corrected cause/message contract; no other existing test changed.

All planned work for this PR is in. Consumer pin (Lionidas) follows this PR's exact merged SHA — merge stays operator-owned.

…ion admission

F3: LoopError carries an explicit cause (budget-hop/wall/token, caller-abort,
provider, tool-invalid) and a widened code union. Provider failures keep their
own codes (PROVIDER_BAD_REQUEST etc. pass through; unattributed timeouts ->
PROVIDER_TIMEOUT; unknown -> PROVIDER_FAILED) instead of being relabeled
BUDGET_EXCEEDED, which the transport treats as a resumable budget pause. The
loop-owned wall stays a separate signal so classification reads observed
signal state; explicit Stop wins over a concurrent wall expiry.

F3 consumers: only genuine BUDGET_EXCEEDED creates a resume stash.

F1: the transport's session admission check-and-acquire is now atomic (no
await between has() and set()) and precedes every durable write, history/stash
preparation and host hook. A preparation failure releases the reservation
instead of stranding the session; the race loser gets 409 before any effect.

Tests: 14 new (classification matrix on the real loop; concurrent admission,
follow-up-after-409, prep-failure release over real HTTP); loop.test.ts wall
case updated to the corrected cause/message contract.
…stash CAS, recovery, quarantine (F1/F2 core)

New generic src/orchestrate/execution-authority.ts: one executor per canonical
agent DB, elected by a lifetime BEGIN IMMEDIATE on a separate tiny control DB
(<agent>.execution-control.db). Competing executors fail election before
serving; the OS releases the lock on process death.

- Atomic durable admission: tryReserve installs an in-process attempt with no
  intervening await; admit() commits epoch check, session-active check,
  request idempotence (REQUEST_ALREADY_ADMITTED / REQUEST_ID_REUSE via stored
  request fingerprint), turn.request events + projections + receipt and the
  execution-ledger row in ONE sink-connection transaction, and requires the
  intended turn.started event — queue decisions and storage failures roll
  back entirely (no phantom execution, receipt or user row).
- Execution boundary: continuation stash is PEEKED during preparation and
  consumed only via exact CAS delete at beginExecution, with the original
  bytes snapshotted into the ledger; Stop-before-start never touches it.
- Truthful recovery: only the lock-holding authority reconciles known
  previous-epoch executions with ONE turn.failed EXECUTION_INTERRUPTED
  (already-terminal turns reconcile the ledger only); re-recovery is a no-op.
- Worker safety: durable butler_execution_workers tracked before start,
  settled only on actual promise settlement; unproven previous-epoch workers
  put the authority in global quarantine (fresh execution blocked, 503-shaped
  reads) until operator-evidenced resolveUncertainWorker.
- Common read: readSession returns idle / preparing / running / stopping /
  uncertain(+reason) from one snapshot; legacy active folds without ledger
  rows are uncertain (legacy_owner), never silently idle. No sweep in reads.

EventSink: constructors can be passive (no boot-time orphan sweep — recovery
is authority-gated); handleInTransaction composes caller ledger writes into
the event commit; withTransaction runs authority ledger writes on the sink's
own connection (two connections to one file must never nest transactions);
rollback discards the poisoned state cache; readSnapshot gives one consistent
state+events+lastSeq boundary.

Storage contract: optional peekPartial + consumePartialExact (non-destructive
peek; exact CAS consume) implemented by the SQLite driver.

Tests: 12 groups on real disk SQLite — election/busy takeover, passive sink,
atomic admission + rollback, request-id idempotence, queue-decision rollback,
stash CAS + stop-before-start, died-owner single interruption + idempotent
re-recovery, quarantine + operator resolution, read walk, stale Stop targeting,
drain-before-release. Full suite 256 passed.
…(F1/F2 wiring)

ConversationDeps.execution?: ExecutionAuthority (additive). When set:

- Message POST: tryReserve + admit drive admission — cross-process atomic via
  the control-DB election; turn.request events, receipt, user message and the
  ledger row land in admit()'s single transaction. Mapped rejections: 409
  busy / REQUEST_ALREADY_ADMITTED (with original turnId) / REQUEST_ID_REUSE /
  SESSION_ACTIVE / QUEUE_DECISION; 503 unavailable (quarantine, drain,
  recovery) with no acceptance. Legacy hosts without an authority keep the
  process-local reservation path unchanged.
- Execution boundary: beginExecution() snapshot-CAS-consumes the continuation
  stash only at the provider boundary; Stop-before-start is an honest ABORTED
  outcome with no provider run. All durable writes (chunks, tools, usage,
  terminal) go through the attempt-fenced seam; ownership release is once and
  identity-guarded (ownsLiveTurn) so the disconnect hook cannot abort a
  released turn on either path.
- /abort: requestStop with optional expectedTurnId — stale targeting 409,
  non-local 503.
- Session GET: readSession common snapshot — liveTurn ONLY for positively
  known local preparing/running/stopping; additive execution field exposes
  idle/blocked/uncertainty explicitly.

Integration tests (real HTTP + real disk SQLite): concurrent sends one-200/
one-409 with a single provider run and no phantom user row; provider 400 →
envelope PROVIDER_BAD_REQUEST with no budget stash and honest session error;
duplicate clientRequestId → 409 REQUEST_ALREADY_ADMITTED without re-execution;
explicit Stop → ABORTED and the session returns to ready; a second transport
instance cannot elect an authority on the same agent DB.
…s (F1 tool durability)

Ordinary provider-requested tools previously produced ZERO durable tool
events: the loop invoked deps.onTool only for streamed harness tools (the
provider's own onTool callback), so the transport's durable seam never saw
registry executions — replayed history showed no tool activity for real work
(the 'tool calls not showing' incident class).

The loop now emits {phase:'call', name, arg: rawArguments} before execution
and {phase:'result', name, result, isError} after; handler throws stay ReAct
observations but are recorded honestly as isError. Transport durability picks
these up through the existing onTool seam.

Remaining (activity schema v2 workstream): per-invocation durable identity —
the transport still derives call IDs from turn+tool NAME, so repeated
same-name invocations share one durable call row with multiple updates.

Tests: registry tool → durable tool_call + tool_call_update; failing handler
→ isError update, never silent success.
…ic finalization, drainable host tails

Per-invocation durable tool identity (F1): onTool events carry the
provider-NATIVE call id; the transport keys durable calls by invocation
(tc_<turn>_i_<nativeId>) — repeated same-name invocations are now distinct
durable calls with their own args/results. The name-derived key remains only
for legacy harness events without a native id.

finalizeOwned (§3.5): terminal event, projections, session token accounting,
budget-stash write and the execution-ledger settle commit in ONE sink-
connection transaction. Storage gains optional sync in-transaction hooks
(applyAccountingSync, stashPartialSync — better-sqlite3 forbids await inside
a transaction, so the async methods cannot join the commit). Duplicate
finalize is a no-op (engine terminality guard → already-terminal, ledger
reconciles); a storage failure rolls back everything, leaves the attempt
reserved for an idempotent retry, and the transport reports 503
FINALIZATION_PENDING — success is never reported for an uncommitted terminal.
All three transport terminal paths (success, empty-after-work, failure incl.
budget stash) route through it under an authority; legacy hosts keep the
existing sequence.

Drainable host tails: afterTurn may return a Promise — the POST never awaits
it, but under an authority it registers as a tracked worker; the provider
root run is itself a tracked worker, so normal shutdown (drain) awaits real
provider work AND host tails before releasing the control lock.

Tests: finalize atomic + duplicate-no-op + in-commit stash; same-name double
invocation → two distinct durable calls with separate args; afterTurn tail
returns after the envelope and is awaited by drain. Full suite green.
…de parser fidelity

The glued-narration root cause: streamed text was persisted as one flat
per-turn chunk stream with no block identity, so separate narration phases
concatenated into each other and no renderer could recover the boundaries.

- NEW src/provider/activity.ts: TextBlockActivity contract (text.start/
  delta/end per block) + createBlockTracker — first delta of a hop opens a
  block, deltas forward bytes exactly (de+velop=develop, no whitespace
  injection), message completion closes it with the full streamed text
  (streamed accumulation wins over provider content when both exist); a
  no-delta hop emits nothing.
- loop.ts: additive LoopDeps.onActivity; tracker feeds BEFORE the existing
  onDelta/onEvent paths (block map populated before chunks persist).
- turn-engine.ts: turn.chunk command + chunk event detail gain optional
  block {id, phase:start|delta|end, native?, text?}; fold propagates it
  additively (durability requires the decided EVENT to carry it) and ignores
  all other unknown fields. Legacy scalar delta kept on every chunk.
- transport: blockRail() helper — text.start/end persist as their own chunk
  commands (delta:''), per-delta chunks carry block{id,phase:'delta'}; wired
  into both runLoop paths (authority + legacy).
- claude-cli.ts parser fidelity: completed snapshots REPLACE-OR-COMPLETE the
  streamed block (snapshot-only emits once; cut stream gets exactly the
  missing tail; fully-streamed emits nothing — no duplicated text), native
  tool_use ids preserved into onTool.invocationId (call + result), full
  results (no 160-char/first-line reduction), empty results emitted honestly
  (result:'', isError:false).

Tests (8 new): two-hop stream → distinct block ids, byte-exact per-block
concatenation, scalar delta on every chunk; text.end prefers streamed text;
no-delta hop → no block; claude fake-binary: no duplicate snapshot text,
cut-stream tail completion, native id preserved, empty result recorded,
300-char result untruncated. Full suite 275 passed / 0 failed; tsc clean.
@thatswiftdev
thatswiftdev marked this pull request as ready for review October 22, 2026 00:32
The selected identity contract says native id where provided, ALLOCATED at
first observation otherwise — never keyed by tool name. Runtime registry
tools already carry the native call id; harness-style provider onTool events
(previously claude-cli, third-party harness providers) arrived without one
and the transport fell back to a name-derived durable id, collapsing
repeated same-name invocations into one durable call.

The loop now allocates an id per observed call event and pairs the result
FIFO per name WITHIN the same provider call only (a deterministic window —
no cross-call guessing); an unpaired result stays uncorrelated.

Test: harness same-name double invocation → two distinct durable calls with
allocated ids (tc_*_i_alloc_*) and correct per-invocation args.
…tail)

The block rail was durable-only (sink chunk commands); live consumers had no
way to key bubbles per narration block, so a tool hop still glued into one
live buffer. createBlockTracker.onDelta now returns the block id it appended
to and the loop includes it as detail.blockId on every turn.delta — absent
for hosts without onActivity, additive for everyone else.
…n authority API

- LoopError.cause collides with ES2022 Error.cause — stricter consumer
  tsconfigs (noImplicitOverride) reject the parameter property; add the
  override modifier (type narrows LoopErrorCause, still assignable).
- Barrel-export createSqliteExecutionAuthority / ExecutionAuthority /
  ExecutionAuthorityBusyError + types: hosts wire the authority through the
  package surface, not deep module paths.
@thatswiftdev
thatswiftdev merged commit 12f9dc8 into main Oct 22, 2026
1 check passed
@thatswiftdev
thatswiftdev deleted the wt/chat-reliability branch October 22, 2026 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chat reliability: admission race, provider-error misclassification, silent owner-death recovery, zero durable tool events

1 participant