plans/claude-state-machines.md
Proposed. Synthesized from the review threads and fix-commit iterations of ~25 merged state-machine PRs: kernel/infra (#4014, #4015, #4024, #4026, #4027, #4045, #4038), chat stream (#4008, #4019, #4023, #4025), app run and plan handoff (#3968, #3969), sagas (#4040, #4060), and the domain ports (#4036, #4058, #4059, #4061, #3967, #3970, #4005, #4029, #4032, #4033, #4047, #4048).
The finding: the kernel's core bet held. Pure transition(),
commands-as-data, keyed controller hosts, and per-machine concurrency policy
(the plans/machine-followup.md decision) produced almost no bugs in the
kernel itself. But reviewers caught the same five bug classes over and over
in the code each machine had to hand-roll — which is exactly the layer the
kernel deliberately does not own. The conclusion is not to reverse the
no-generic-controller decision or adopt XState; it is to promote a small
number of patterns from "convention in rules/state-machines.md, enforced
by reviewers citing line numbers" to "primitive in src/state_machines/,
enforced by construction."
Ranked by severity × recurrence. Each class lists the concrete findings that motivated it; every one of these was a real review comment that produced a fix commit.
The single worst finding of the migration. Any state whose only exit is a
timer event is one missed schedule-* command away from being permanently
stuck.
waitingSelectorReady during an in-flight
capture did not re-emit schedule-settle (one path emitted
cancel-settle), so SETTLE_ELAPSED could never fire — "the machine is
permanently stuck... reintroduces exactly the stale-thumbnail regression
the PR set out to fix." An HMR reload during the 3s settle window is
common.pending had no fallback for untagged/error pages
that never send SELECTOR_READY; awaitingResponse had no bounded wait
if the iframe never replied.cancelling had no timeout — a never-settling IPC promise means
the job is never terminal, so pruneTerminalJobs can never reclaim it.checkingProviders could wedge the first-prompt overlay if the
providers query never resolved; fixed with a watchdog plus a
timeout-origin state with late recovery.Staleness-by-generation only works if the generation source outlives the controller. Three machines independently reinvented per-key identity retention.
lastStreamIdByChatId) and seeding replacements from it.Four PRs had dispose-ordering bugs. The contract that every fix converged on lives only in fix-commit folklore.
starting/streaming/cancelling released transport
but never synced a terminal snapshot, so the legacy isStreamingByIdAtom
projection stayed true and blocked queue dispatch forever.finalizing cleared the command queue — dropping
run-end-side-effects — while skipping releaseTransport; with
autoRelease:false the renderer stream entry and turn context leaked.dispose() called stop() first, releasing the projection writer
before the controller's final idle syncProjection; the write was
dropped and the chat looked like it was streaming forever after remount.selectedAppId before disposing controllers;
the manager's synchronous atom subscription sent APP_CHANGED and
started a return checkout against an already-deleted app.stopping forever.Converged contract: dispose must (a) settle all outstanding waiters unsuccessfully, (b) synchronously emit a terminal snapshot/projection, (c) release owned resources even when release normally lives in a queued command, (d) make late events and settlements inert, (e) be idempotent.
The largest cluster by count. Total transition matrices with
ignore(state, reason) are correct — but no projection derived "which
events does this state accept," so legacy imperative buttons became enabled
no-ops. Every fix had the same shape: hand-add a canRequestX flag.
CLOSE, which the closed
state ignores; per-message restore buttons enabled during
recovery-required, events "silently dropped with no navigation or
toast."send() re-entered process(),
executing the inner event's commands before the outer's. Fixed with a
processing flag + pending-event buffer, and enqueue-before-notify.onEnd; the old stream's post-callback cleanup then deleted the
new stream's just-installed callbacks. Fixed with generation-aware
callback removal (clean up only if not superseded).command.files and relied
on a React closure cleared in the same synchronous dispatch — "works only
because command dispatch is synchronous... fragile, easy-to-break
coupling."watch-stream-idle awaited inside the serial command drain — a
never-idle stream permanently wedged the FIFO and leaked the
subscription. Rule that emerged: never await an unbounded external
condition inside the drain loop; convert to disposable, supersedable
watchers or watchdog-bounded states.getUserMedia throwing synchronously stranded voice-to-text in
acquiring (#4029); the controller only logs runner throws, so every
adapter must remember its own try/catch.never-checks prove totality of handling, not reachability
or producibility: the never-produced superseded state (#4036), the
unreachable successBanner("rebase") (#4059), the missing
conflicted → switch-blocked cell discovered only when the consumer PR
needed it (#4061), and unreachableState returning garbage instead of
throwing so unknown events were silently swallowed (#3970).chatStream at first construction, during
render, before the root effect injected the facade — reload-safe
continuation silently never ran, and tests missed it because each test
constructed the adapter correctly. #3970's cold-start unsolicited-return
drop (listener installed lazily) is the same defect.claimReturn
claimed whichever same-provider flow was awaiting-return, so a stale
poll or old browser callback could advance a newer flow — "connect the
wrong account." Correlation is only as strong as its weakest claim site;
where the ID physically cannot round-trip (Supabase/Neon proxy accepts no
state parameter), the invariant must be structural and documented — and
#4038's doc review showed the documentation of such invariants is itself
correctness-critical ("teaches future contributors the wrong invariant").cancelling. The adopted
rule: always finalize on any non-stale terminal event in a cancelling
state; reject staleness structurally (by generation), never by inferring
event provenance from ordering. #4040's creation registry with
commit/cancel tombstones solves the same shape in main.clearTodosOnCancel before the
persisted snapshot was loaded, deleting the chat's on-disk todos on
Stop-during-initial-compaction.isStreamingByIdAtom
caused a P1 in #4008 (machine idle-write clobbering an external stream's
true), the #4019 dispose bug, and defensive guards that only became
deletable when #4025 made the machine the single writer. Same theme in
derived-value form in #4040: three PROVIDER_CONFIGURED emitters resolved
chat mode differently, so whichever event won the race decided the mode.Promote the five hand-rolled patterns with the worst review record into kernel primitives, and close the flagged-but-deferred observability gaps. Everything here stays within the micro-kernel philosophy: invariant plumbing, no policy framework.
A machine declares timeouts alongside its states:
timeouts: {
waitingSelectorReady: { after: 3_000, event: { type: "SETTLE_ELAPSED" } },
}
The controller arms the timer on every entry path into the state
(including self-re-entry via a state that returns a new reference), cancels
it on exit, and uses the injected Clock. Manual schedule-*/cancel-*
command pairs remain available for timers that are not entry-scoped.
Companion test-kit assertion: any state whose only outgoing transitions are
timer-delivered events must have a declared timeout
(assertTimerStatesBounded(transition, timeouts)).
KeyedControllerHost (class 2)The host already owns key lifecycle; give it identity that outlives the controller:
host.nextGeneration(key) — monotonic per key, surviving
disposeKey/re-ensure. Deletes the hand-rolled lastStreamIdByChatId
pattern and prevents the #4023 counter-reset class structurally.stale-generation ignore reason in types.ts so trace logs
and tests spell it identically across machines (#4023 mapped onto
stale-stream-id by hand).EntityDisposalRegistry, as
#4023's retention map already established (documented as deliberate).Two pieces:
createDisposalSequence(controller) — a kernel helper encoding the
converged order: settle waiters unsuccessfully → synchronously emit
terminal snapshot and final projection sync → release owned resources,
including those whose release normally lives in a queued command → mark
inert so late settlements are dropped → idempotent on re-entry. Writer
release happens after the final projection sync (#4045's bug, by
construction).assertDisposalContract(makeController) in testing.ts — drives
dispose() from every reachable non-terminal state and asserts:
projections cleared, recorded commands include every release the state
owned, second dispose is a no-op, post-dispose events are ignored with a
stable reason. Required in the standard machine test suite.transition is pure and total, so acceptance is computable by probing:
const caps = deriveCapabilities(transition, state, [
"SYNC_REQUESTED",
"SWITCH_BRANCH",
]);
// caps.SYNC_REQUESTED === false when the matrix would ignore() it
Implementation: an event is "accepted" iff transition(state, event) does
not return an ignored result (kernel already distinguishes this —
ignore() returns the same reference with a reason). Projections spread
these flags; UI disables on them. Enabled-no-op buttons become impossible
by construction instead of being caught one button per review. Events whose
construction needs a payload probe with a declared minimal probe payload.
The serial drain loop is already verbatim-duplicated across controllers; extract the ~40 lines with the fixes baked in:
onThrow event (or a standard
command-threw event) instead of relying on each commands.ts adapter's
discipline (#4029).This is not the declined generic controller: no staleness policy, no concurrency model, no command scheduling semantics — those stay per-machine. It is the mechanical loop all thirteen machines already share, with the three review-caught bugs fixed once.
exploreReachableStates exists in testing.ts; add and require:
assertAllStatesReachable(transition, initial, eventCorpus) — every
declared state identity is producible (catches #4036's dead
superseded).assertAllCommandsProducible(...) — every command constructor is emitted
on some reachable path (catches #4059's unreachable banner).unreachableState and its siblings must throw, never return a value
(#3970's silent swallow).createLateBinding<T>() with get(), configure(value), and
onConfigured(cb) (fires queued work immediately if already configured).
Replaces ad-hoc configureChatStream-style retrofits; #4047's HIGH and
#3970's cold-start listener drop are the identical defect and both reduce
to "dependency arrives after first construction."
All flagged during review and consciously deferred; close them here:
window.__dyadMachines, and make defaultDescription refuse to
retain raw untagged objects (#4026 — retention/exposure hazard).result.state eagerly (#4027).registerAtomWriter production-throw question (#4045,
Dyadbot MEDIUM): the design doc scoped single-writer enforcement to a
dev-mode assertion, but the guard throws unconditionally — a transient
double-mount during an overlapping route transition would crash in prod.
Either downgrade to dev-assert + prod-warn, or record the throw as a
deliberate decision.plans/machine-followup.md decision. Still no generic
controller policy (staleness, concurrency, scheduling), still no
XState. Item 5 extracts the mechanical drain loop, not a policy engine.screenshot, image_generation;
generations → chat_stream, app_run); the rest migrate opportunistically.rules/state-machines.md already encodes several of these as conventions
(bots cite it by line). Once the primitives exist, the corresponding rules
change from "remember to do X" to "use kernel primitive Y"; add rules that
remain convention-only:
[domain, appId, ...] so
invalidation scopes per app; no sibling keys for data invalidated
together (#4059/#4061 hand-tuned this twice).screenshot and image_generation migrated —
the confirmed HIGH class.chat_stream and app_run
migrated.github_ops and version_preview as the motivating consumers.Each step is one PR-sized unit in the established pattern: kernel change + motivating machine migration + named regression tests mirroring the original review findings (the A→B→A→B dispose test and co-sim bound-drain test set the precedent).