Skip to content

fix(orchestration): threads imported while the server runs can't be deleted (or acted on at all) - #47

Open
DanielGGordon wants to merge 1 commit into
mainfrom
t3code/fix-thread-delete-external-writer
Open

fix(orchestration): threads imported while the server runs can't be deleted (or acted on at all)#47
DanielGGordon wants to merge 1 commit into
mainfrom
t3code/fix-thread-delete-external-writer

Conversation

@DanielGGordon

Copy link
Copy Markdown
Owner

Problem

Deleting a thread appears to do nothing for threads written by the t3 import CLI while the server is running ("zombie" threads). Reproduced live on prod and deterministically in an isolated dev environment.

Symptom chain (captured via Playwright + WebSocket instrumentation):

  1. Thread renders normally in the sidebar (the UI reads the SQL projections, which the import CLI keeps fully up to date).
  2. Right-click → Delete → confirm sends orchestration.dispatchCommand {"type":"thread.delete","threadId":"claude-import-…"}.
  3. The server rejects it:
    OrchestrationDispatchCommandError: Orchestration command invariant failed (thread.delete):
    Thread 'claude-import-…' does not exist for command 'thread.delete'.
    
  4. The row stays in the sidebar. Same failure for every other command against such a thread (the old thread.meta.update rejections in orchestration_command_receipts from 07-08 are the same bug).

Root cause

t3 import runs a second in-process OrchestrationEngine over the same SQLite file (cli/import.tsOrchestrationLayerLive + SqlitePersistenceLayerLive). Events, receipts, and projections all land correctly in the DB. But the long-running server's engine keeps its command read model in memory (commandReadModel in OrchestrationEngine.ts), hydrated once at startup from projectionSnapshotQuery.getCommandReadModel() and afterwards updated only with events the engine itself commits. Streams appended by the CLI process after server start are invisible to the decider, so requireThread fails.

The existing reconcileReadModelAfterDispatchFailure can't recover: it reads events above the in-memory watermark, and the engine's own later appends (SQLite AUTOINCREMENT sequences) leapfrog the externally appended sequences, leaving them permanently below the watermark. A server restart "heals" existing zombies (startup re-reads SQL), which is why the bug looks intermittent.

Fix

In processEnvelope, after the command-receipt dedupe and before decideOrchestrationCommand: if the command's aggregate (thread/project) is missing from the in-memory model and the command isn't thread.create/project.create (which legitimately target missing aggregates), refresh commandReadModel from the SQL snapshot — the same source startup hydration uses. The refresh is clamped so snapshotSequence/updatedAt never rewind (protects the ws latestSequence replay-gap contract and dispatchStartSequence reconcile semantics). Refresh failure degrades to a log warning and the stale model (command then fails exactly as before).

Deliberately no "is SQL newer than memory" sequence gate: the engine's own commits advance the projection bookkeeping past externally appended sequences, so that comparison reads "equal" in exactly the failing case.

commandToAggregateRef also gains a discriminated-union return type so the thread/project id narrows correctly.

The gate is a "must-exist aggregate" computation rather than a blanket create-command exclusion: project.create has no must-exist aggregate; thread.create's must-exist aggregate is its project (its decider calls requireProject, so creating a thread inside an imported project used to fail with "Project … does not exist" — caught in review); every other command checks its own thread/project.

Regression tests

OrchestrationEngine.test.ts:

  • "dispatches commands against threads appended by an external writer" — simulates the import CLI by appending a decider-produced thread.created directly via the event store + projection pipeline (bypassing engine.dispatch), advances the engine watermark with an unrelated dispatch (the leapfrog), then asserts a first-attempt thread.delete succeeds.
  • "dispatches thread creation into a project appended by an external writer" — same simulation for an imported project, then asserts thread.create into it succeeds.

Both are red without the engine change ("… does not exist") and green with it.

Verification

  • apps/server orchestration + persistence suites: 33 files / 231 tests pass; typecheck clean; no lint findings on changed files.
  • Reviewed by a Fable 5 agent (clamp/reconcile interplay, receipts, ws latestSequence contract, error-channel typing, layer-memoization in the test harness all verified; its one FIX-FIRST finding — thread.create into an imported project — is fixed and tested here). Note: fork CI never runs (no Blacksmith runners), so local verification is the gate.
  • Live end-to-end in an isolated dev env (node scripts/dev-runner.ts dev, worktree-local .t3): imported a transcript with t3 import claude … --base-dir <dev .t3> while the dev server was running, then deleted it through the UI with Playwright.
    • Before fix: delete dispatch rejected with the invariant error; thread stayed in the sidebar (error toast shows in Sidebar v2; v1 showed nothing).
    • After fix (fresh import against the fixed, already-running server): first-attempt delete succeeded, thread.deleted event appended, row gone from the sidebar and still gone after reload.

Follow-ups (out of scope, noted during diagnosis)

  • Externally appended events never reach the running server's eventPubSub, so connected clients don't see a new import until a reload/resnapshot. Routing t3 import through the live server (the t3 project tryResolveLiveProjectExecutionMode pattern) would fix that half and remove the two-writer exposure entirely.
  • No PRAGMA busy_timeout is set anywhere; concurrent writers risk immediate SQLITE_BUSY.
  • The new-thread draft page can wedge on "Thread already exists and cannot be created twice" after a failed worktree start (rollback-deleted thread id is reused by the draft; soft-deleted ids can never be recreated).

🤖 Generated with Claude Code

…ets an externally appended aggregate

The t3 import CLI runs a second in-process OrchestrationEngine over the
same SQLite file while the server is running. Its events and projections
land correctly, but the running server's in-memory command read model is
hydrated once at startup and only folds events the engine itself commits,
so imported threads render in the UI (SQL reads) while every command
against them - thread.delete included - is rejected by requireThread with
"Thread ... does not exist". The post-failure reconcile cannot recover:
it reads events above the in-memory watermark, and the engine's own later
appends leapfrog the externally appended sequences.

On an aggregate miss for non-create commands, re-read the command read
model from the SQL snapshot (the same source as startup hydration),
clamped so snapshotSequence/updatedAt never rewind. No sequence-based
gate: the engine's own commits advance the projection bookkeeping past
external sequences, so such a comparison reads "equal" in exactly the
failing case.

Regression test simulates the external writer by appending a
decider-produced thread.created via the event store + projection pipeline
directly, advances the watermark with an unrelated dispatch, and asserts
a first-attempt thread.delete succeeds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M labels Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant