plans/machine-followup.md
Proposed follow-up to plans/better-state-machine.md. That plan refactors the
version-preview machine; this one addresses what PRs #3968 (plan handoff),
#3969 (app run), and #3970 (connection flow) revealed: the codebase now has
four hand-rolled state machines in the same pattern, with verbatim-level
mechanical duplication and early signs of drift.
Decision recorded here (from review): extract only the invariant micro-kernel and codify the convention in a doc. Do not build a generic controller, and do not adopt XState. Four machines produced four distinct, load-bearing concurrency models — FIFO queue (plan handoff), runId epochs (app run), flowId correlation (connection flow), per-command-class rules (version preview). A generic controller would be a policy framework larger than the ~100–200-line controllers it replaces. The convention and the lifecycle plumbing are generic; the controllers are not.
connection_flow's main-process registry. It is already the
right shape (injected timers/ids/broadcast, no module globals, no React).
Only its shared types participate here.| version_preview | plan_handoff (#3968) | app_run (#3969) | connection_flow (#3970) | |
|---|---|---|---|---|
| Process | renderer | renderer | renderer | main |
| Keyed by | appId | chatId | appId, per Jotai store | provider |
| Execution model | parallel dispatch, per-command rules | strict FIFO drain loop | serial per app, settlement-as-events | registry derives effects from transitions |
| Staleness | epoch on read command; mutations never dropped | none (matrix handles re-entry) | runId epoch, stale events dropped pre-transition | flowId correlation in domain events |
| Registry | module globals (replaced by main plan) | per-chat map in hook | module-global WeakMap<store, Map<appId>> | injected-dependency class in main |
Identical in all four: pure transition(state, event) with ignore() and
exhaustive never checks; listener-Set + getSnapshot +
notify-on-reference-change (~15–20 lines each); a lazy keyed registry
(~30–50 lines each); a useSyncExternalStore binding; totality-style
transition tests.
Drift already visible:
WeakMap keyed by Jotai store; the
version-preview plan solves the identical problem with a provider-owned
host. Two competing lifecycle patterns landing the same week.dispose() and live forever per chat;
version preview treats disposal as a first-class invariant.onIgnoredEvent telemetry; the other
three machines have nothing equivalent.Add rules/state-machines.md (follow the existing rules/ organization; if
none fits, place under docs/ and link from AGENTS.md). Keep it short
enough to be read in review. Contents:
state.ts (types only), transition.ts (pure),
controller.ts (or a main-process registry), commands.ts (adapter),
plus a hook binding for renderer machines.state.ts/transition.ts are pure and dependency-free: no React,
Electron, Jotai, TanStack Query, zod, timers, Date, or randomness. This
is what lets types travel between renderer and main.switch + never checks and explicit ignore(state, reason) so
deliberate no-ops are distinguishable from omissions.state.ts or controller.ts header:
what executes serially vs in parallel, what can be dropped as stale, and
what must never be dropped.src/state_machines/Only what is identical across all four machines. Hard constraint, enforced
by review and a lint boundary if available: no imports from any domain,
Jotai, TanStack Query, IPC, or toast code. react.ts is the only file
that may import React.
types.ts — TransitionResult<State, Command>,
ignore(state, reason?), an IgnoreReason-style tag type (adopting
#3970's refinement), and a TransitionObserver telemetry interface
(generalizing version_preview/debug.ts and #3970's onIgnoredEvent):
hooks for applied transitions and ignored events.snapshot_store.ts — SnapshotStore<S>: listener set, getSnapshot,
setState with notify-on-reference-change, subscribe, dispose.
Controllers embed it (composition); it is not a base class and carries no
transition or command semantics.keyed_host.ts — KeyedControllerHost<K, C> exactly as specified in
plans/better-state-machine.md: lazy creation, per-key and any-key
subscriptions, disposal of one key or the whole host.react.ts — useKeyedController(host, key, selectSnapshot) over
useSyncExternalStore with stable snapshot identity, plus the smaller
useControllerSnapshot(controller) binding for non-keyed use (usable by
useConnectionFlow's renderer projection if trivial).testing.ts — totality driver (run every event against every reachable
state, assert a result), reference-stability assertion helper, and a
recording fake command-runner harness.Before exporting the host as the shared primitive, validate its contract against all four machines — on paper, not by migrating them first:
number (appId, chatId) and string (provider).{ getSnapshot, subscribe, dispose }.
The host must not constrain domain surfaces beyond that: app_run's
dispatch()-returns-promise and onStateChange projection, plan
handoff's send, and version preview's event API all remain
controller-owned.dispose (#3968) get one added during migration;
the host requires it.Each migration is its own PR, after the corresponding feature PR lands, and is behavior-preserving under the tests those PRs already added.
Covered by plans/better-state-machine.md Phase 1. Coordination rule: the
kernel files above are the canonical versions; that plan's Phase 1 consumes
them rather than defining a parallel copy. If this plan lands second, move
its extracted pieces here without API change.
usePlanHandoff.ts with a
provider- or module-explicit KeyedControllerHost<number, HandoffController>.dispose() to the handoff controller; chat deletion disposes its
controller, mirroring app deletion in version preview.SnapshotStore in place of the hand-rolled listener set.TransitionResult/ignore types; behavior unchanged.WeakMap<store, Map<appId, controller>> in
src/app_run/registry.ts with a provider-owned host constructed with the
Jotai store — the WeakMap exists only to isolate test stores, which
provider ownership solves directly.useRunApp's public API and the onStateChange atom projection
exactly as landed; only lifecycle ownership moves.SnapshotStore; adopt shared transition types.ignore/IgnoreReason and
TransitionObserver types from types.ts (it is the origin of the
pattern), and optionally bind its renderer projection through
useControllerSnapshot. Its main-process registry stays as is.TransitionObserver wire-up to the
version_preview, plan_handoff, and app_run transitions. Mechanical:
ignore(state) → ignore(state, "reason"), observer plumbed where
debug.ts logging exists today. No transition semantics change.getAppRunController(store, appId)'s WeakMap-by-store is the
module-global lifecycle pattern being eliminated and a provider-owned
host migration is expected; on #3968, that chat deletion should dispose
its controller.rules/state-machines.md. Does not wait on any code; this is the
cheapest drift-stopper and informs review of everything below.src/state_machines/ files with their tests, validated against the
interface-freeze checklist.plans/better-state-machine.md Phase 1 so exactly one
canonical copy of the host exists.The checklist above is validated against all four machines before the host is exported. If a fifth machine appears mid-plan, it validates the contract too — it does not expand it.
Migrations touch only lifecycle/registry code, not transitions, commands, or public hook APIs. The feature PRs' own tests are the characterization suite; a migration PR that has to modify a transition test is out of scope by definition.
Keep it to invariants and decisions, not tutorials. Link it from the repo rules/AGENTS index so review agents and humans load it. New-machine PRs are expected to cite deviations against it explicitly.
plans/better-state-machine.md specs KeyedControllerHost and the React
adapter; this plan adds SnapshotStore, shared types, and the test kit
around the same files. Whichever lands first creates src/state_machines/;
the other consumes it unchanged. Any API disagreement is resolved in favor
of the interface-freeze checklist here, since it is validated against all
four machines rather than one.
Per phase, narrowest first, then the standard pre-commit checks:
npm test -- src/state_machines/
npm test -- src/plan_handoff/
npm test -- src/app_run/
npm test -- src/connection_flow/
npm test -- src/version_preview/
npm run fmt
npm run lint
npm run ts
npm run build
Migration PRs must show git diff --stat free of transition/command file
changes (types-import lines excepted) and pass the feature's existing test
suites unmodified except for added disposal coverage.
rules/state-machines.md exists, records the invariants and the
per-machine degrees of freedom, and is linked from the rules index.src/state_machines/ contains the host, snapshot store, shared types
with ignore reasons and transition observer, React bindings, and test
kit — with zero domain imports.