feat(daemon)!: replace tmux with Bun-native daemon and JSON-RPC UI server

[lavaman131]

Summary

Replaces the tmux-based session and workflow attachment model with a Bun-native daemon process that exposes a TCP loopback JSON-RPC UI server (vscode-jsonrpc over LSP Content-Length framing). All workflow control — discovery, dispatch, lifecycle, panel state, and PTY I/O — now flows through JSON-RPC 2.0 methods and notifications on the daemon's protocol surface.

Key Changes

Runtime / Daemon

• New Daemon singleton (daemon.ts): per-user singleton enforced via ~/.atomic/daemon.endpoint.json; binds UIServer on 127.0.0.1:0, traps SIGTERM/SIGINT/SIGHUP, manages daemon.log
• UIServer (ui-server.ts): TCP loopback JSON-RPC server over vscode-jsonrpc with Content-Length framing; auth gate enforces connect before any non-version method
• DaemonWorkflowContext (daemon-workflow-context.ts): builds real provider SDK client/session per stage (Copilot send-until-idle wrap, env/flag plumbing), persists saved records, feeds initial PTY cols/rows
• DaemonSupervisorAdapter (daemon-supervisor-adapter.ts): bridges ISupervisor to Supervisor, carries cwd
• RunManager (run-manager.ts): tracks active runs, stage PIDs, and cancellation; stop() sends SIGKILL for immediate cancellation (replaces SIGTERM + delayed kill)
• WorkflowRegistry (registry.ts): discovers and imports workflow files; awaited before endpoint file is written
• RunState (run-state.ts): canonical in-memory run state with pruning, versioning, and AgentType
• Supervisor (supervisor.ts): process supervisor with DI PTY spawner (bun-pty), 4 MiB ring-buffer scrollback, pane/output and pane/exit notifications; no tmux dependency
• Protocol versioning: protocol-version.ts + sdk-protocol-version.json for client/server compatibility checks

CLI

• New atomic daemon restart command for graceful terminate and respawn
• session and workflow commands now dispatch through JSON-RPC (ensureStarted()) instead of tmux -L atomic
• New --ui-server flag on atomic workflow for detached daemon-backed starts
• New atomic workflow attach <runId> for client reconnection

SDK Components

• PanelClient (panel-client.tsx): drives chat-session-panel, pty-pane, and workflow-picker-panel via ui-protocol notifications
• PtyPane (pty-pane.tsx): terminal emulator pane backed by daemon PTY supervisor
• ChatSessionPanel (chat-session-panel.tsx): session-level chat panel with history and streaming
• panel-footer and terminal-mouse: new utility components
• workflow-picker-panel model and theme split into discrete modules (workflow-picker-model.ts, workflow-picker-theme.ts)

Infrastructure

• Export classification registry (export-registry.json + Zod schema + validate-export-registry.ts script)
• bunfig.toml: daemon I/O-heavy modules (daemon.ts, ui-server.ts) exempted from 85% coverage gate — consistent with existing executor.ts / providers/** policy
• All tmux code removed: tmux.ts, self-exec.ts, orchestrator-entry.ts, attached-footer.ts, cc-debounce.ts
• File discovery helper (helpers/file-discovery.ts) scrubs git env vars so cwd controls the repo
• sdk-protocol-version.js bootstrap shim for cross-package version negotiation

Docs & Examples

• packages/atomic-sdk/docs/ui-server.md: full JSON-RPC UI server reference (806 lines)
• packages/atomic-sdk/docs/migration-1x-to-2.md: migration guide from 1.x runWorkflow() / tmux to 2.x daemon / PanelClient
• examples/ui-server-client/: standalone UI server client example with annotated code
• Research: research/docs/2026-05-09-ui-server-architecture.md + specs/2026-05-09-ui-server-bun-native.md

Breaking Changes

• runWorkflow() no longer drives the subprocess directly from the CLI — all importable-workflow runs route through the daemon
• Session/panel lifecycle requires the daemon; ensureStarted() auto-spawns on first use
• RunManager.stop() sends SIGKILL immediately (previously SIGTERM + delayed kill)
• hostLocalWorkflows([wf]) removed — replace with export default workflow in workflow source files
• Hidden subcommands removed: _orchestrator-entry, _emit-workflow-meta, _atomic-run, _cc-debounce
• 1.x tmux sessions and on-disk artifacts (~/.atomic/sessions/<runId>/) are not migrated

Migration Notes

See packages/atomic-sdk/docs/migration-1x-to-2.md for the full upgrade guide.

Quick summary:

1. Replace hostLocalWorkflows([wf]) with export default wf in workflow source files
2. Terminate any running 1.x tmux sessions before upgrading: tmux kill-server -L atomic
3. Remove stale 1.x artifacts: rm -rf ~/.atomic/sessions/
4. Use atomic workflow attach <runId> instead of tmux-layer detach/reattach

Test Plan

• bun typecheck and bun lint clean (pre-commit hooks pass)
• bun test — 2799 pass, 0 fail
• bun run test:coverage — gate passes at 85%
• Manual: atomic chat session list and atomic chat session connect <id> route through the daemon
• Manual: atomic workflow -n <name> runs a multi-stage workflow with provider SDK stages end-to-end
• Manual: atomic daemon restart gracefully terminates and respawns the daemon
• Manual: atomic workflow attach <runId> reconnects a detached panel client

[claude]

PR Review Part 1/3: Atomic 2 daemon refactor — Overview & Blocker

Substantial well-structured rewrite. JSON-RPC architecture, schema-validated dispatcher, and Zod validation are clean and testable. Tests in daemon.test.ts, run-manager.test.ts, and supervisor RPC tests exercise real behavior through fakes, not coverage padding.

== Blocker ==

1. Daemon shutdown never disposes the Supervisor — PTY children orphan on SIGTERM/SIGINT

Daemon.stop in packages/atomic-sdk/src/runtime/daemon.ts:426-440 only calls this.server.stop and never reaps PTY children. ISupervisor has no dispose member. The owning Supervisor (whose dispose performs killPtyProcessTree on stage.pty with SIGKILL) is created in packages/atomic/src/commands/cli/ui-server.ts:30 and held only by DaemonSupervisorAdapter, so the daemon has no path to dispose it on signal. Combined with Bun.spawn using detached:true and all-ignore stdio in ensureStarted (line 582), spawned agent CLIs (Copilot/OpenCode UI servers, Claude subprocesses) orphan to PID 1 when the daemon receives SIGTERM. Add a dispose to ISupervisor, or pass the concrete Supervisor to Daemon and call supervisor.dispose before process.exit in signalHandler, unhandledRejectionHandler, and uncaughtExceptionHandler.

[claude]

PR Review Part 2/3: High & Medium severity findings

== High ==

2. UIServer connection close does not clean up RunState/Supervisor subscriptions

UIServer.handleConnection (ui-server.ts:184-186) on socket close only removes the entry from its local connections set. It does NOT propagate close to RunState.subscribers (run-state.ts:65) or Supervisor.outputSubs (supervisor.ts:164). When a panel client disconnects without calling panel/unsubscribe or pane/unsubscribeOutput (crash, network drop, force-quit), the daemon keeps the dead MessageConnection in those maps and continues fanning out notifications on every state change. Pruning happens lazily only when sendNotification rejects, but vscode-jsonrpc buffers writes silently (see the EPIPE/ECONNRESET ignore in Daemon.unhandledRejectionHandler at line 465). Fix: register a conn.onClose callback that walks RunState.subscribers for every active run and Supervisor.outputSubs and removes entries matching the closed connection.

3. RunManager.getTranscript joins user-controlled strings into a filesystem path

run-manager.ts:300-308 builds messagesPath via join(home, .atomic, sessions, runId, sessionName, messages.json). runId and sessionName arrive over JSON-RPC validated only as z.string (schemas.ts:236-243). path.join does not strip parent-directory refs (..), so attacker input produces traversal. Impact bounded (filename hardcoded to messages.json, parse failure returns []), but on a host where attackers can land messages.json in arbitrary locations the daemon will read+parse on demand. Either reject runId/sessionName containing path separators or parent-dir refs, or look up the run in this.runs first and refuse if runId is not registered.

4. RunManager.stop SIGKILLs the PTY but cancellation does not unwind into running run(ctx)

stop (run-manager.ts:252-277) iterates runPids, calls supervisor.kill with SIGKILL, then markRunCancelled. For _runVisibleProviderStage in daemon-workflow-context.ts:454-468 the registered PID is the bun-pty child (e.g. copilot --ui-server). The provider SDK clients (CopilotClient, OpencodeClient) created at lines 585/626 keep their own HTTP connections; when stop SIGKILLs the PTY out from under them, cleanup in the finally (line 522) tries provider.cleanup then session.disconnect against a dead server. The promise from await run(ctx) is never given a chance to reject — markRunCancelled happens, but cancellation does not propagate into the user run(ctx) callback. Nothing inside run(ctx) polls state.isCancelled, so user code can hang on a stale provider session for up to 1s (Bun.sleep at line 532) before completing the finally. Consider exposing an AbortSignal on the context or having stop reject the in-flight stage promise.

== Medium ==

5. chat/start schema validates cols/rows but the dispatch cast strips them

ui-protocol/methods.ts:355-363 casts to agent/args/env/cwd — omitting cols and rows even though ChatStartParamsSchema (schemas.ts:196-203) and handleChatStart (line 496-506) both accept them. Runtime validation preserves the fields so data flows through (cast is type-only), but a future reader will assume cols/rows are dropped. Add cols and rows to the cast.

6. connect token comparison rejects mismatched lengths before timingSafeEqual — leaks length via timing

methods.ts:449-453 checks if a.length !== b.length OR not timingSafeEqual(a, b). The length early-out defeats the constant-time guarantee — an attacker measuring response latency on connect failures can probe token lengths. For loopback-only tokens this is largely theoretical, but the comment at ui-server.ts:107 hints --host override is in v1 only; on non-loopback this becomes a real timing oracle. Hash both sides via crypto.createHash sha256 and timingSafeEqual the digests; digest length is constant regardless of input.

7. probeLiveness raw socket parser assumes a single response in one TCP segment

daemon.ts:222-247 accumulates bytes into a string and parses Content-Length framing once. If the daemon ever sends a JSON-RPC notification before the response (early server/closing race during graceful restart), lenMatch parses the first frame and JSON.parse of body may not contain result.protocolVersion, returning null — marking the live daemon as stale. Also chunk.toString utf8 at arbitrary byte boundaries can split multi-byte UTF-8 across TCP segments. Concatenate Buffer and decode once after framing is found.

8. RunState.broadcast does not snapshot subscribers before iteration

run-state.ts:282-301 iterates this.subscribers and calls this.subscribers.delete from inside the async catch handler. The catch is asynchronous so iteration is safe against itself, but cancel/dispose can run synchronously between loop completion and catch firing, mutating the map under in-flight rejection handlers. Practically benign after dispose clears the map. Iterate a copy or document the assumption.

9. pty-pane.tsx DirectPtyPane writes mouse-reporting toggles to process.stdout directly

pty-pane.tsx lines 242, 247, 320: process.stdout.write of TERMINAL_MOUSE_REPORTING_DISABLE_SEQUENCE and mouseFilter.finish run inside the useEffect cleanup. If useRenderer throws during render (ErrorBoundary activates), the cleanup never runs, leaving the user terminal with mouse reporting still enabled after detach. Move the disable into the renderer config or wrap in try/finally at mountWorkflowPane in panel-client.tsx.

[claude]

PR Review Part 3/3: Low severity, tests, and overall

== Low ==

10. Supervisor.RingBuffer is unbounded for a single append

supervisor.ts:42-51: this.buffer += data then trims. If a stage emits a single 100MB chunk, that chunk is held in memory before slicing. Bun strings are concat-cheap but the intermediate allocation is real. Fine for terminal output (typically less than 1 KiB per frame) — worth a docstring note.

11. bunfig.toml exempts daemon-workflow-context.ts and supervisor.ts from coverage

Justification (provider SDK + bun-pty integration) is reasonable, but daemon-workflow-context.ts (863 lines) contains pure logic in _runStage, _runVisibleProviderStage exit-promise wiring, wrapCopilotSendUntilIdle, and the cancellation path — all unit-testable with the same fake ISupervisor already used in run-manager.test.ts. Consider lifting the exemption and adding targeted tests for cancellation/exit ordering; the orphan-process fix in #1 will need them anyway.

12. daemon.ts unhandled-rejection handler races signal handler

Both signalHandler and unhandledRejectionHandler are async and call process.exit after await this.stop. If SIGTERM arrives while an unhandled rejection is mid-flight (or vice versa), stop runs twice and process.exit codes race (last wins). stop is server-idempotent but signal-handler-deregister happens at the top. Track a stopping flag.

== Tests ==

daemon.test.ts, run-manager.test.ts, supervisor.test.ts, methods.test.ts exercise real behavior through public APIs with fake ISupervisor/MessageConnection doubles — not internal-implementation testing. methods.supervisor-rpc.test.ts drives a real Supervisor with a fake IPtySpawner, which is the right level of integration. The protocol-version test is trivial but appropriate.

== Overall ==

Architecture is solid, but #1 (orphan PTYs on daemon shutdown) and #2 (subscription leaks on client disconnect) should land before merge — the daemon is now the single owner of long-lived processes and connections, so cleanup paths must be airtight.

[claude]

Code Review — daemon-driven sessions, panels, and SDK stages

This is a major architectural change (~150 files, +22K/-11K) that swaps a tmux-attached orchestration model for a Bun-native daemon with a JSON-RPC TCP loopback UI server. Overall the design is clean: clear interface boundaries (IRunManager / ISupervisor), Zod-validated wire schemas, dependency-injected supervisors, and thoughtful lifecycle/shutdown handling. Test coverage on the new daemon layer is meaningful (220+ new tests across daemon.test.ts, ui-server.test.ts, methods.test.ts, run-manager.test.ts, supervisor.test.ts, daemon-workflow-context.test.ts, etc.).

Issues are ordered by severity.

Security / correctness

1. Path traversal in RunManager.getTranscript() — run-manager.ts:300-309

const messagesPath = join(home, ".atomic", "sessions", runId, sessionName, "messages.json");

sessionName arrives from the JSON-RPC client as z.string() with no validation (schemas.ts:236-240). An authenticated client could pass sessionName: "../../.." to read arbitrary messages.json files under $HOME. The daemon is loopback-only with token auth, so the blast radius is bounded, but it's still a bug — any local process able to read the endpoint file (mode 0o600, fine) and token could traverse. Tighten the schema (e.g. z.string().regex(/^[A-Za-z0-9._-]+$/)) and the path-join, or resolve() + verify the result is contained under the run directory.

2. Arbitrary file import via workflow/start.source — methods.ts:484-494 → run-manager.ts:212

runs.start({ source, ... }) calls import(source) directly. source is z.string() with no check against the loaded WorkflowRegistry. Anything an authenticated caller asks the daemon to load gets evaluated as a TypeScript module in-process. The original tmux model already had similar exposure, but the new daemon centralises it under a long-running, multi-client process. Consider validating that source resolves to a path the WorkflowRegistry has already classified as a workflow source (registry.bySource.has(...)).

3. Subscriber/outputSubs desync in Supervisor.fanOutNotification — supervisor.ts:389-406

Async-rejecting connections are dropped from stage.outputSubscribers but the corresponding record in this.outputSubs (keyed by subscriptionId) is left dangling — it'll only be cleaned up if the client explicitly calls pane/unsubscribeOutput. For long-running daemons with flaky clients this leaks subscription records. Either index outputSubs reverse-lookup (connection → subscriptionId[]) or scan outputSubs and prune entries pointing at the dead connection in the same code path.

4. RunManager.list("completed") excludes cancelled / errored runs — run-manager.ts:279-290

list("completed") returns only status === "complete". Cancelled and errored runs aren't returned by either "active" or "completed" — they only show up under "all". atomic session list callers asking for "what's finished" will silently miss failed runs. Either widen "completed" to include complete | error | cancelled, or add a third explicit scope (e.g. "terminal").

Code quality

5. PanelClient.destroy() and the instance constructor are unreachable — panel-client.tsx:237-263, 631-664

The PanelClient class has a private constructor that's never called — the only entry point is the static mount() method, which manages its own renderer/connection lifecycle and never instantiates PanelClient. destroy(), foregroundStage, subscriptionId, and the instance fields all appear to be dead code. Either wire mount() to construct an instance and let it own teardown, or delete the unused class shape.

6. probeLiveness open-codes JSON-RPC framing — daemon.ts:199-266

The comment says "Uses a raw Content-Length framed JSON-RPC exchange to avoid vscode-jsonrpc's Bun-specific socket write timing issues." Worth linking to the upstream issue tracker (or a local repro doc) so the workaround can be removed when fixed; otherwise this will silently become dead complexity nobody dares touch.

7. process.env as Record<string, string> — daemon.ts:585, supervisor.ts:191, etc.

NodeJS.ProcessEnv is { [key: string]: string | undefined }. The cast happens to be safe in practice on Linux/macOS, but it's the kind of cast the project's CLAUDE.md flags ("avoid ambiguous types"). A small helper that strips undefined keys before passing to Bun.spawn/PTY would remove the casts.

8. Bun.spawnSync(["ps", "-eo", "pid=,ppid="]) blocks the event loop per kill — supervisor.ts:471-501

Called from killPtyProcessTree, which fires on every stage kill and on RunManager.stop. For an interactive daemon this is fine, but if many runs are stopped in burst (e.g. shutdown), the cumulative blocking time on ps could be noticeable. Async Bun.spawn + a small await would be safer; alternatively cache the snapshot for the duration of a single shutdown pass.

9. Unauthenticated mode silently accepts any token — methods.ts:443-456 / ui-server.ts:92-98

When opts.token is undefined, the warning is printed but every connection still succeeds with any (or no) token. That's documented as "loopback-only permissive mode," and the binding is hard-coded to 127.0.0.1 in Daemon.start(), so the surface is limited. Still worth either (a) hard-rejecting connections when token is absent, or (b) auto-generating a random token when none is provided rather than going fully permissive.

10. Daemon.signalHandler calls process.exit(0) after await stop() — daemon.ts:454-458

If stop() throws (rare, but possible — netServer.close callback can error), process.exit(0) never runs and the daemon hangs on whatever signal triggered it. Wrap in try/finally so exit always fires, or escalate to exit(1) on stop failures.

Tests / coverage

• Three runtime modules are excluded from the 85% coverage gate via bunfig.toml:53-60 (supervisor.ts, daemon-supervisor-adapter.ts, daemon-workflow-context.ts). The rationale is documented (require live bun-pty / agent CLIs), but these are the daemon's most security- and lifecycle-critical files. Worth tracking integration test coverage explicitly so the exclusions don't become permanent debt.
• daemon-workflow-context.test.ts is the lightest of the new suites at 18 tests for an 863-line file. Provider-SDK branches (createVisibleProvider, createHeadlessProvider, copilot/opencode/claude paths) appear largely uncovered.
• panel-client.test.ts covers pure helpers (castSnapshot, mapSnapshotSessions, applyForegroundStage) but none of the static mount() flow — understandable given OpenTUI rendering, but a no-network mock could exercise at least the connect/subscribe/teardown sequence.

Nits

• supervisor.ts:309 uses bare crypto.randomUUID() (globalThis) while run-manager.ts:8 and daemon-workflow-context.ts:20 import from node:crypto. Pick one for consistency.
• daemon-workflow-context.ts:528 swallows the kill error silently — at least log it via the daemon log writer so orphan-PID debugging is possible.
• RunState.broadcast (run-state.ts:282-301) iterates this.subscribers while async handlers can mutate the same map. JS Map iteration is safe under concurrent deletion, but the order of panel/update vs run/ended could surprise — a single test asserting "no notification arrives after run/ended" would lock the contract down.

Strengths worth calling out

• MethodDispatcher cleanly separates protocol validation from business logic and is fully testable with stub dependencies — methods.test.ts reflects this nicely (50 tests).
• RingBuffer design and pane/output offset semantics (appendScrollback / sliceNewPaneOutput) are well thought-out and have explicit unit tests for the boundary cases.
• Zod schemas + runtime result validation in dispatch() catches handler regressions at the wire boundary — a strong invariant for a protocol that multiple SDKs depend on.
• DaemonWorkflowContext overload that splits headless vs visible provider stages (and gates Claude to headless until the daemon-native pane lands) is a sensible incremental migration path.
• Token comparison uses timingSafeEqual after explicit length check (methods.ts:449-453) — correct constant-time auth.

Overall a well-engineered refactor. Items 1–4 are worth addressing before merge; the rest can land as follow-ups.

[claude]

PR Review: daemon-driven sessions, panels, and SDK stages

Big-picture: this is an impressive rearchitecture. The JSON-RPC surface is well-scoped, the schemas-as-source-of-truth pattern is clean, tests are extensive (~3.4k lines across daemon/run-manager/supervisor/panel-client), and the docs (ui-server.md, migration-1x-to-2.md) make the new world approachable. Below are the items worth attention before merge.

Security

1. Unauthenticated mode + workflow/start source = local RCE (medium-high)

packages/atomic-sdk/src/runtime/ui-protocol/schemas.ts:178 lets workflow/start accept any source: z.string(), and RunManager.executeRun() (run-manager.ts:212) calls await import(source) on it. When ATOMIC_UI_SERVER_TOKEN is unset the daemon falls into permissive mode (daemon.ts:330-332, methods.ts:455) where any token is accepted. On a shared host any local user can connect to 127.0.0.1, send a workflow/start with a hand-crafted source, and execute code as the daemon owner. The only safeguard today is the loopback bind plus the console.warn at startup — both easy to miss.

Suggested mitigations (any one is sufficient):

• In permissive mode, validate source against workflows.getDescriptor(workflowName).source and only import the registered path.
• Auto-generate a random token at daemon start when none is provided and pass it to clients through the endpoint file (which is already mode 0o600).
• At minimum, gate workflow/start's source to a path under ~/.atomic, process.cwd(), or XDG_CONFIG_HOME.

The same threat applies to pane/sendInput and agent/spawn, but workflow/start is the most direct path to RCE.

2. readEndpointFile accepts any JSON shape (low)

daemon.ts:158-165 does JSON.parse(raw) as DaemonEndpoint with no runtime validation. A corrupt or maliciously-crafted endpoint file (mode 0o600 limits this, but the file is in ~/.atomic which other tools may touch) could redirect clients to an arbitrary host — probeLiveness and openConnection honor whatever ep.host says. Consider a DaemonEndpointSchema (z.object({ host: z.literal("127.0.0.1"), port: z.number().int().positive(), ... })) and treat parse failures as "stale, unlink, respawn".

Correctness / robustness

3. /tmp fallback for HOME is the wrong default (medium)

packages/atomic-sdk/src/runtime/run-state.ts:81 and packages/atomic-sdk/src/runtime/run-manager.ts:301 use process.env.HOME ?? process.env.USERPROFILE ?? "/tmp". Elsewhere (daemon.ts, daemon-workflow-context.ts) the codebase consistently uses os.homedir(). On systems where HOME is unset, session data ends up in world-writable /tmp — a privacy and integrity issue. Use os.homedir() and let it throw if there is genuinely no home directory.

4. Hung shutdown if stop() rejects (low)

daemon.ts:454-458 and :460-471: signalHandler and unhandledRejectionHandler both await this.stop(...) and then process.exit. If stop() throws (e.g., netServer.close() callback returns an error), process.exit is never reached. Suggest wrapping in try { ... } finally { process.exit(code); } plus a hard timeout (e.g. setTimeout(() => process.exit(code), 2_000).unref()).

5. ESM cache after workflow/refresh (low)

RunManager.executeRun() and WorkflowRegistry._importAll() both call await import(source). refresh() only clears the registry's own maps — Node/Bun's ESM module cache still returns the old module on subsequent imports. If the design intends true hot-reload, you'll want a cache-busting query string (?t=${Date.now()}). If not, document that workflows are pinned to first-import and refresh() only discovers new entries.

6. Inconsistent kill semantics (low)

RunManager.stop() (run-manager.ts:259) sends SIGKILL immediately — the PR notes this as a behavior change. DaemonWorkflowContext._runVisibleProviderStage() (daemon-workflow-context.ts:528) sends SIGTERM on cleanup with a 1s grace, then proceeds. These two paths can race when a visible stage is cancelled mid-callback. Probably fine in practice (best-effort) but worth a comment so future readers know the intent.

Performance

7. RingBuffer.append is O(buffer length) (low)

supervisor.ts:42-51 does this.buffer += data followed by this.buffer.slice(excess). For a 4 MiB capacity with steady output that exceeds capacity each tick, this is an O(n) copy on every PTY data chunk. Probably negligible for human-readable stage output, but a long-running agent dumping JSON streams could spike CPU. Consider a chunked/segment-list storage if you see CPU regressions in production. (Not a merge blocker.)

8. Fan-out has no backpressure (low)

Supervisor.fanOutNotification (supervisor.ts:389-406) fires conn.sendNotification and lets vscode-jsonrpc queue indefinitely on slow consumers. A subscriber that has stopped reading will accumulate memory in the writer. Worth tracking, especially because the daemon is per-user and long-lived.

Code quality / style

9. bun-pty via require() (nit) — supervisor.ts:87 uses require("bun-pty") with an eslint-disable. The comment says "Dynamic import so the module is not evaluated at load time in tests" — Bun's ESM import() would achieve the same lazy evaluation. Worth aligning with the rest of the codebase, which doesn't use CJS require.

10. Unused field on CompletedStageRecord (nit) — daemon-workflow-context.ts:38 carries sessionId but only record.sessionDir is ever read by transcript() / getMessages(). Drop it or hand it back through DaemonSessionHandle.id.

11. _sessionOpts underscore parameter (nit) — daemon-workflow-context.ts:257 keeps the leading underscore even though the value is consumed when forwarded to provider stages. Rename to sessionOpts to match the actual usage.

12. Telemetry handler is a documented no-op (nit) — methods.ts:461-470 is a deliberate stub. Fine as-is, but a TODO comment with a tracking issue would help future contributors not assume it's wired.

Test coverage

Extensive and well-targeted. A few small gaps worth thinking about:

• daemon.ts's exception handlers are exercised but the "stop rejects, process hangs" failure mode isn't (matches item 4).
• No test exercises permissive-mode workflow/start from a third-party client perspective (matches item 1).
• bunfig.toml exempts daemon-workflow-context.ts (864 lines) from the 85% gate — justified by provider-SDK dependencies, but a comment in the file pointing to the integration coverage would help reviewers.

Bottom line

Architecture is sound, the migration story is clear, and the tests are confidence-inspiring. Items 1 and 3 are worth fixing before merge — the rest are follow-ups that can land in a polish PR.

[claude]

Review — Daemon + JSON-RPC UI server (PR #917)

Overall a well-structured refactor with strong test coverage (Zod schemas, DI-friendly interfaces, fixture-based daemon tests). A few concerns worth addressing before merge.

Bugs / Correctness

1. Socket leak in sessions.ts default deps (packages/atomic-sdk/src/primitives/sessions.ts:81, repeated for every primitive: listRuns/getRun/stopRun/getRunStatus/getRunTranscript/getAttachInfo/setForeground). Each call does ensureStarted() → request → conn.dispose(). dispose() tears down the MessageConnection but StreamMessageReader/StreamMessageWriter do not close the underlying net.Socket. The PR already exports closeDaemonConnection() (daemon.ts:515) precisely for this and uses it in panel-client.tsx. Every primitive call currently leaks a file descriptor. Replace conn.dispose() with closeDaemonConnection(conn).

2. Path traversal in RunManager.getTranscript (run-manager.ts:300-309). The sessionName param is concatenated into <home>/.atomic/sessions/<runId>/<sessionName>/messages.json with no sanitization. An authenticated client (or a workflow that names a stage "../../../etc/something") can read arbitrary messages.json files within reach. Result validation (RunTranscriptResultSchema) limits damage to JSON-arrays-of-records but it should still be guarded — e.g. reject any sessionName containing /, \, or .., or resolve+verify the final path stays under the run's session dir.

3. RunManager.subscriptions never cleaned up on disconnect. subscribe() stores { connection, runId, stateSubscriptions } keyed by subscriptionId. Nothing prunes the entry if the JSON-RPC connection drops without an explicit panel/unsubscribe. The inner RunState.broadcast lazy-prunes dead subscribers, but the outer RunManager.subscriptions map grows unbounded. Register an onClose listener on the connection inside subscribe() to call unsubscribe(subscriptionId).

4. Supervisor retains exited stages forever. pty.onExit records endedAt/exitCode on the SupervisedStage but never removes the entry from this.stages / this.pidIndex / its outputSubscribers. The only cleanup is dispose() at daemon shutdown. For a long-lived daemon with many runs, every completed stage stays in memory (including its 4 MiB ring buffer by default). Either prune on exit (with a small grace window for late scrollback fetches) or expose forgetStage(runId, stageName) that RunManager calls when a run reaches terminal state.

5. Daemon.start() race window. Between readEndpointFile (daemon.ts:365) and writeEndpointFile (:414), two daemons starting concurrently can both observe "no endpoint" and both bind a kernel-assigned port. Only the last writeEndpointFile wins; the loser stays bound and accepts connections that no client will ever find. Consider fs.openSync(endpointFile, "wx") to claim the slot before binding, or a sidecar lockfile (flock on ~/.atomic/daemon.lock).

6. Supervisor.pidIndex collisions after PID reuse. pidIndex.set(pty.pid, key) (supervisor.ts:230) blindly overwrites if the OS recycles a PID across stage spawns inside one daemon. The old stage record stays in this.stages but its pid lookup is hijacked — killByPid(oldPid) would kill the new stage. With Flora131/feat/add skills #4 unfixed this becomes increasingly likely the longer the daemon runs.

7. mode: 0o600 is silently ignored on Windows (daemon.ts:174). The endpoint file doesn't contain the token but it discloses the loopback port — combined with the unauthenticated/permissive default (no token), that's enough to attach from another local user. Consider an ACL'd path on Windows or at least document the loopback-trust assumption explicitly.

Design / API

8. connectToDaemon() skips protocol version negotiation. protocol/getVersion is exposed pre-auth and returns protocolVersion, but connectToDaemon/ensureStarted only call connect. A client talking to a stale daemon of a different protocol version will get cryptic method-not-found / shape-mismatch errors instead of a clean IncompatibleSdkError. Fetch protocol/getVersion first and compare against getProtocolVersion() before issuing connect.

9. Inconsistent paneId between visible and headless stages. Visible: String(pid) (daemon-workflow-context.ts:508). Headless: "headless-${name}-${sessionId}" (:410). Anyone keying off paneId (logs, scrollback lookups, telemetry) has to special-case both shapes. Pick one stable identifier (e.g. runId:stageName).

10. CommonJS require in BunPtySpawner (supervisor.ts:87). Everything else here is ESM; the eslint-disable acknowledges it. Use await import("bun-pty") with a one-time module-level cache instead — preserves Bun's normal resolution path.

11. RunState.broadcast uses console.warn directly (run-state.ts:285) bypassing the injected onLog/onWarn. Tests can't observe these and they aren't routed to daemon.log. Plumb a logger into the RunState constructor.

12. handleRunStatus / handlePanelGet cast snapshots to Record<string, unknown> (methods.ts:521, 626). The result is then validated against WorkflowStatusSnapshotSchema, so the cast is purely to silence TS. Mark the dispatcher's handler return type as unknown and let Zod be the source of truth.

Style / CLAUDE.md compliance

• The if (process.platform !== "win32") guard is duplicated in killPtyProcessTree (supervisor.ts:438 and :442) — collapse to one.
• (this as unknown as { emit(): void }).emit() in DaemonPanelStore.applySnapshot (panel-client.tsx:85) is a private-escape from a subclass. Make emit protected in PanelStore and drop the cast.
• tmuxSession: "" is still threaded through buildSnapshot (run-state.ts:307) and the wire schema requires it (schemas.ts:57). The PR removes all tmux usage — drop the field or at least make it optional with a deprecation comment, otherwise every snapshot carries the dead field forever.

Tests

Coverage is strong (daemon, run-manager, supervisor, registry, ui-protocol/*, daemon-workflow-context — all substantial). Worth adding tests for:

• The socket leak in sessions.ts (assert socket destroyed after each call).
• Disconnect-without-unsubscribe cleanup in RunManager.
• Path-traversal rejection in getTranscript.
• The startup race in Daemon.start() (two parallel start() calls against the same endpoint file).
• PID reuse collision in Supervisor.

Migration / Release

• Migration doc (migration-1x-to-2.md) is clear and the tmux kill-server -L atomic step is the right call.
• The bunfig.toml coverage carve-out for daemon.ts / ui-server.ts mirrors the existing executor/providers pattern — reasonable. Worth checking whether daemon-workflow-context.ts and run-manager.ts also need it explicitly rather than letting the 85% gate drift.

Summary

Architecture is sound and the test surface is the right shape. The leaks (#1, #3, #4) and the path traversal (#2) are the must-fix items; the race in #5 and PID reuse in #6 are edge cases but easy to harden now. Nice work overall — the daemon model is a much cleaner foundation than the tmux orchestrator.

[claude]

Review: atomic 2.0 daemon refactor

Thanks for the thorough write-up — the migration doc, the spec under specs/, and the panel-client/run-manager/run-state test suites make this very reviewable for a 224-file PR. Overall direction looks good: a Bun-native daemon + JSON-RPC surface is a clear win over the tmux shell-out approach, and the Zod-validated MethodDispatcher is a strong protocol-enforcement layer.

Comments below are grouped by severity. Most are nits or follow-ups; a couple are worth addressing before merge.

Critical / should fix before merge

1. packages/atomic-sdk/sdk-protocol-version.js looks like an accidentally-committed build artifact

import"./atomic-sdk/index-37x76zdn.js";

// sdk-protocol-version.json
var protocolVersion = \"1.0.0\";
...

• The import references ./atomic-sdk/index-37x76zdn.js, a content-hashed bundle path that does not exist in source (packages/atomic-sdk/atomic-sdk/ is not a directory in this PR).
• The file is not listed in package.json exports or files, and grep shows no *.ts/*.tsx/*.json references to it.
• The PR description calls it a "bootstrap shim for cross-package version negotiation," but the only thing it does is re-export the JSON via a phantom bundle import.

Either delete this file or replace it with a hand-written shim that imports the JSON directly (import data from './sdk-protocol-version.json' with { type: 'json' }; or equivalent). As-is, anything that tries to import it at runtime will throw ERR_MODULE_NOT_FOUND.

2. Subscriber maps leak when sockets disconnect without explicit unsubscribe

UIServer.handleConnection registers socket.on(\"close\", () => this.connections.delete(entry)), which cleans up the per-server connection set. But:

• RunState.subscribers (run-state.ts:65) keyed by subscriptionId
• RunManager.subscriptions (run-manager.ts:37) keyed by subscriptionId
• Supervisor.outputSubs (supervisor.ts:164) keyed by subscriptionId

are not cleaned up on socket close. They're only reaped lazily inside fanOutNotification / broadcast when a sendNotification rejects — but a socket can close cleanly between fan-outs without ever failing a send.

For long-lived daemons with many client reconnect cycles, this accumulates dead entries indefinitely. Worse, every broadcast still iterates them and tries to call conn.sendNotification(...), which then errors and only then prunes — at scale that's an O(stale) cost per notification.

Fix idea: have UIServer.handleConnection track per-connection subscriptions (or expose a teardown hook on MethodDispatcher), and on socket.close walk those and call the right unsubscribe paths on runs / supervisor. The WeakMap for auth state is fine; the strong-keyed subscription maps are the leak.

3. DaemonSupervisorAdapter.kill violates the documented ISupervisor.kill contract

ISupervisor.kill is documented (methods.ts:197) as:

No-op if pid is unknown (process may have already exited).

But the adapter (daemon-supervisor-adapter.ts:129) does:

kill(pid: number, signal: ... = \"SIGTERM\"): void {
  this.supervisor.killByPid(pid, signal);
}

…and Supervisor.killByPid (supervisor.ts:242) throws STAGE_NOT_FOUND if the pid isn't tracked. A client calling agent/kill on a stale pid will get a JSON-RPC error instead of the documented no-op. RunManager.stop happens to wrap it in try/catch (run-manager.ts:253-259), but the wire-level contract is the one users see.

Either: (a) make the adapter swallow STAGE_NOT_FOUND, or (b) update the interface doc and add a process_not_found error code so callers can distinguish.

4. PR description disagrees with bunfig.toml diff

The description says:

bunfig.toml: daemon I/O-heavy modules (daemon.ts, ui-server.ts) exempted from 85% coverage gate

But the actual diff adds supervisor.ts, daemon-supervisor-adapter.ts, and daemon-workflow-context.ts to the ignore list — not daemon.ts or ui-server.ts. Worth aligning them (and IMO daemon.ts + ui-server.ts have enough direct test coverage that they shouldn't be exempted — the description seems to be the wrong one).

High priority

5. Version drift between SDK and pinned platform binaries

packages/atomic-sdk/package.json is at 0.7.16, but every @bastani/atomic-${platform}-${arch} entry under optionalDependencies is pinned to 0.7.13. resolvePackagedAtomicBinary() walks getPlatformPackageSuffixes() and require.resolves these — if they drift further apart (or aren't re-published in lockstep), users hit the Bun.which(\"atomic\") fallback or MissingDependencyError. The release script should sync these versions automatically.

6. /tmp fallback when $HOME is unset is unsafe

RunState constructor (run-state.ts:81) and RunManager.getTranscript (run-manager.ts:295) both do:

const home = process.env.HOME ?? process.env.USERPROFILE ?? \"/tmp\";

If HOME is unset (sandboxed containers, certain CI runners, daemonized systemd units), this writes status snapshots and reads transcripts under /tmp/.atomic/sessions/, which is world-writable. UUID run-ids make collisions improbable but other users can still inspect or pre-create paths. Prefer throwing on missing HOME here — it's a daemon, not a CLI flag, so the cost of strictness is low.

7. Token comparison short-circuits on length

MethodDispatcher.handleConnect (methods.ts:449-453):

const a = Buffer.from(token);
const b = Buffer.from(envToken);
if (a.length !== b.length || !timingSafeEqual(a, b)) {
  throw authenticationRequired();
}

For the default randomly-generated 32-byte hex token, both strings have fixed length, so the length leak is moot. But if a user sets a custom ATOMIC_UI_SERVER_TOKEN of arbitrary length, this leaks length via timing. A constant-time wrapper that pads-then-compares would close it, or just add a comment that the lengths must match by construction.

Medium / nits

8. Daemon.signalAbort is allocated but never observed

registerSignalHandlers sets this.signalAbort = new AbortController() (daemon.ts:480), but nothing passes the signal to any awaiter. Either wire it to in-flight work (e.g., cancel ongoing workflows.load() if a shutdown signal arrives mid-startup) or remove it.

9. RunState.broadcast warns via console.warn instead of the configured onWarn

run-state.ts:299-302 writes directly to console.warn when a subscriber drops, while the rest of the daemon routes warnings through the injected onWarn callback. Worth unifying.

10. probeLiveness re-implements Content-Length framing by hand

daemon.ts:199-265 hand-parses Content-Length framing instead of using vscode-jsonrpc. The comment explains it's a Bun socket-write-timing workaround — fair, but it's worth a follow-up to upstream the issue (Bun or vscode-jsonrpc) so this duplication can eventually go away.

11. Inconsistent fetch/subscribe order between DirectPtyPane and PtyPane

DirectPtyPane (pty-pane.tsx:280) subscribes first, then fetches scrollback, and uses pendingLiveOutput to merge — correct. PtyPane (pty-pane.tsx:412-444) fetches scrollback first, then subscribes — can miss output between the two calls. Worth bringing PtyPane to the same model (or document why the legacy log view doesn't need it).

12. Supervisor.killByPid reports unknown pids as STAGE_NOT_FOUND with placeholder runId

supervisor.ts:243-246 throws stageNotFound(\"(unknown)\", \pid ${pid}`)` when the pid isn't indexed. Either swallow at the adapter (see #3) or introduce a distinct error code so callers can tell "pid not tracked" from "named stage not found."

13. No max-stages / max-connections cap

Supervisor.spawn doesn't enforce a concurrent-PTY limit. A misconfigured loop workflow could exhaust the file-descriptor budget. Similarly, UIServer doesn't cap connections.size — loopback-only mitigates blast radius, but a defensive cap (with a clear error code) is cheap insurance.

14. runWorkflow foreground path uses three round-trips

workflow/start → run/getAttachInfo → run/get is fine for correctness but adds latency before each foreground run. Consider returning the run's initial status from workflow/start (or piggybacking getAttachInfo into the start result) to drop one round-trip on the hot path.

Tests / coverage

Test suite is impressively thorough — panel-client.test.ts (668 lines), run.test.ts (414 lines), daemon.test.ts, supervisor.test.ts, ui-server.test.ts, plus the focused ui-protocol/schemas.test.ts to lock the wire schema. PR claims 2799 pass / 0 fail and 85% coverage gate green.

Gaps worth a follow-up:

• A regression test for the subscriber-cleanup-on-socket-close path (item updates to readme and instructions #2) would be valuable.
• DaemonSupervisorAdapter.kill should have a unit test asserting the no-op contract from ISupervisor.kill (item update readme and mcp servers #3) once decided.
• The PR adds no integration coverage I can find for the --ui-server flag path (packages/atomic/src/commands/cli/ui-server.ts) — it's a small file but it's the production entry point for the daemon and is excluded from unit coverage by cli.ts.

Style / CLAUDE.md alignment

Skimming the new files: TypeScript, Bun-native, no any/unknown slop, Zod schemas where appropriate, dependency-injected supervisors and pty spawners. All consistent with the repo's CLAUDE.md guidance. The examples/ui-server-client/ and docs/ui-server.md (806 lines) are genuinely helpful for SDK consumers.

---

Nothing here is a hard blocker except #1 (the apparently-broken sdk-protocol-version.js) — easy fix or delete. #2 (subscriber leak) and #3 (kill contract violation) are worth resolving before this becomes 2.0 because both will show up as real user bug reports.

[claude]

test

[claude]

PR 917 review (part 1 of 2) — tmux to Bun-native daemon with JSON-RPC

Great architectural direction and the test suite is substantial (DI through IPtySpawner is especially clean). Most of the feedback below is about hardening edges that emerge from running a long-lived per-user daemon — the very thing that distinguishes this from the prior tmux-shell-out model. Items marked must-fix would block merge in my view; the rest are sequenced by impact.

---

Must-fix

1. NUL byte in packages/atomic/src/commands/cli/workflow-list.ts:55

The file is detected as binary by file(1) and shows up as Bin 6756 to 6623 bytes in git diff --stat. Confirmed: counting NUL bytes returns 1. Line 55 (Map-key separator) embeds a literal NUL between the workflow name and description in the template literal forming the bucket key.

Fix: declare an explicit constant and use it in the template literal — e.g. ASCII Unit Separator (hex 1F) as an escape, not a literal byte. Many editors, linters, and tools choke on NULs in source — git diff already does (hence the Bin annotation).

2. Daemon singleton: TOCTOU plus non-atomic endpoint write (runtime/daemon.ts:362-419, :171-177)

• The read-probe-unlink-bind-write sequence has no flock or O_EXCL. Two concurrent atomic invocations on a cold start can both observe a missing/stale endpoint, both bind a (different) kernel-assigned port, and both write daemon.endpoint.json — last writer wins, the loser orphans a fully-functional but unreachable daemon. ensureStarted() makes this worse on first use.
• Endpoint write at :171-177 is direct-overwrite. Use writeFile(tmp) then rename(tmp, dest) so partial reads can never be observed.
• Stale-recovery at :373 does not verify ownership before unlink. Consider process.kill(pid, 0) as a cheap second-line check before assuming death.

3. SIGKILL with no SIGTERM grace (runtime/run-manager.ts:246-271)

stop() sends SIGKILL immediately. Claude/Copilot CLIs spawn MCP servers, language servers, and child workers; transcripts and scratch files get truncated mid-write. getTranscript (:294-303) JSON.parses the result and swallows the throw — so corruption is silent. Recommend SIGTERM, 200-500 ms grace, then SIGKILL. Let IRunManager.stop accept an optional signal so programmatic callers can opt out. Compare Supervisor.killStage which already takes a signal.

4. Uncaught-exception handler leaks the supervisor (runtime/daemon.ts:473-477)

Daemon.stop calls server.stop() but never supervisor.dispose(). On uncaughtException we exit with all bun-pty children orphaned. Either hold a supervisor ref on Daemon and call dispose() in stop(), or wire it into the existing fatal path.

5. Registry hot-reload is a no-op for code edits (runtime/registry.ts:124)

await import(sourcePath) hits the ESM module cache. refresh() clears the internal maps but import() returns the cached module — the user's edits to a workflow file never propagate until daemon restart. Either bust with a query string (?v=timestamp) or document explicitly that refresh is metadata-only and code requires daemon restart.

6. Run state and subscriber sets leak monotonically

• RunManager.runs and .states (run-manager.ts:35-36) are never pruned. stop() deletes from runPids only (:261). Long-lived daemons (the explicit design goal!) accumulate runs forever.
• Subscriber sets in supervisor.ts:308, run-state.ts:65, run-manager.ts:37 only get pruned when a send fails. There is no MessageConnection.onClose wiring, so silent-but-alive clients (or paused subscribers) stay registered as long as the daemon lives. Add per-connection subscription caps and dispose-on-close.
• run-state.ts:225-231, 262-276: on dispose() the persist timer is not clearTimeout'd, so the last pending write is silently dropped. Either cancel the timer or flush synchronously in dispose. Also no serialization on writeSnapshot — back-to-back debounce fires can run concurrent fs.writeFile calls on the same path.

7. Export registry validator is not wired into CI

packages/atomic-sdk/script/validate-export-registry.ts is well-designed (enforces enum, parity with package.json exports, surfaces uncertain entries) — but grep for validate-export-registry in .github/ returns nothing, and the SDK package.json has no validate:exports script. The registry will rot the moment someone adds a new export. Add a bun run script/validate-export-registry.ts step to .github/workflows/ci.yml and ideally to prepublishOnly.

---

High-impact issues

Protocol and security

• No real version gating. INCOMPATIBLE_SDK error code exists in ui-protocol/errors.ts:9 and a helper at :74-80, but is never thrown. ConnectParams (schemas.ts:140-143) has no clientProtocolVersion — version is advisory-only and clients carry the burden. Recommend: add clientProtocolVersion to connect, reject on major-mismatch with INCOMPATIBLE_SDK.
• UIServer.start(port, host) accepts arbitrary host (ui-server.ts:109) despite the docstring promising loopback-only. Drop the parameter or assert it starts with 127.
• No size caps on pane/getScrollback (schemas.ts:319-325) and no per-connection subscription cap (methods.ts:629-641). A misbehaving local client can OOM the daemon. Also no payload-size limit on the vscode-jsonrpc Content-Length framing — a Content-Length: 2147483647 from a local attacker is uncapped.
• tmuxSession field still in WorkflowStatusSnapshotSchema (schemas.ts:57, written empty at run-state.ts:325). Dead/legacy — remove before this ships.
• Outbound notifications bypass NotificationSchemas (schemas.ts:542). Inbound is fully Zod-validated; outbound notifications like server/closing are hand-built. Validate the send path too.
• Permissive (no-token) mode still authenticates any clientName (methods.ts:457). Defensible because of loopback-only, but worth logging a loud warning on every startup, not just once.

Cross-platform

• Windows process-tree kill is a no-op for descendants (supervisor.ts:438-460). The collectDescendantPids path is Unix-only (ps -eo pid,ppid). On Windows only the PTY direct child is killed; agent-spawned MCP / language servers leak. CLAUDE.md documents Windows config paths, so this is a real gap, not theoretical.
• Bun.spawnSync in the fan-in path (supervisor.ts:473) blocks the event loop during shutdown — every active connection notification dispatch stalls for the duration of ps. Use async Bun.spawn.

(continued in next comment...)

[claude]

PR 917 review (part 2 of 2)

High-impact issues (continued)

PanelClient and PtyPane

• No reconnect on daemon death (panel-client.tsx:321-353). One socket, no onClose or onError. If the daemon restarts, the panel silently freezes. The migration doc tells users to use atomic workflow attach runId but the SDK panel itself does not try.
• panel/get before panel/subscribe (panel-client.tsx:365-374) is the inverse of the careful subscribe-first-then-fetch ordering used in DirectPtyPane (pty-pane.tsx:286-317) and ChatSessionPanel. Updates between the two RPCs are lost. Apply the same ordering uniformly.
• panel/foregroundChange initial race (methods.ts:633-637): subscribe-then-read returns a value that may be stale by the time the first notification arrives.
• Resize signaling is unbatched (pty-pane.tsx:333-342). Dragging the corner emits one RPC per row. Debounce ~50 ms.

Daemon lifecycle

• Signal handler has no escalation (daemon.ts:454-458). If stop() hangs, the daemon will never exit even on a second SIGINT. Wire a force-exit timer (e.g. 5 s) on the second signal.
• SIGHUP is conflated with shutdown (:483). Conventional Unix daemons treat SIGHUP as "reload config" — a natural fit for WorkflowRegistry.refresh. At minimum, comment why this is deliberate.
• Unhandled rejection bias is "any error is fatal" (daemon.ts:460-471 plus isTransportError at :294-298). For a daemon meant to outlive the CLI, this is hostile. Filter or rate-limit non-transport rejections.

Coverage gaps and exemptions

• bunfig.toml:36-79 exempts supervisor.ts and the adapter from the 85 percent gate citing "branch coverage requires running bun-pty processes." But IPtySpawner DI is already in place and exercised in supervisor.test.ts:60-77. Re-enable coverage now that the seam exists.
• Integration scenarios not covered: daemon restart mid-run, client reconnect to a running workflow, two clients on one runId with consistent offsets, protocol-version mismatch end-to-end, malformed JSON-RPC at the TCP layer (truncated Content-Length, invalid JSON body, missing jsonrpc field).
• Flake risk: supervisor.test.ts:397 150 ms sleep, ui-server.test.ts:411 50 ms sleep for broadcast, run-manager.test.ts:50 flushAsync = 10 ms. Replace with explicit event-awaits where possible.

---

CLAUDE.md violations (any/unknown)

The codebase says "avoid any and unknown," but the new dispatcher uses them heavily:

• methods.ts:335-416: every case does a params-as-typecast even though Zod already parsed the value at :301. Use z.infer of the params schema — the dispatcher should return a fully-typed params. This is the single largest type-safety regression in the PR.
• methods.ts:518-522, 623: Record-of-unknown plus an as-unknown-as Record-of-unknown re-cast. Define the actual snapshot type.
• schemas.ts:443: MethodSchemas typed loosely on a string key — should be keyed on a MethodName string-literal union.
• methods.ts:577-606: supervisor cast to ISupervisor with extras — the methods are already on ISupervisor. Dead cast.
• panel-client.tsx:76: this cast to unknown then to an emit-bearing object punches through private. Expose protected emit() on PanelStore.
• panel-client.tsx:92: double-cast opaque to WorkflowStatusSnapshot — schema type and local type are not identical; unify or validate.
• run-manager.ts:376: match.run(ctx as never) hides a real mismatch between DaemonWorkflowContext and a workflow declared context type.

---

Lower-priority

• Schema tightening (schemas.ts): fromOffset (:319), headOffset (:324), pid (:365) should be .int() (and offsets .nonnegative()).
• protocol-version.ts: readFileSync at first-call is lazy plus sync; the first protocol/getVersion request blocks the event loop. Cache the result at module load and Zod-validate the JSON file shape; handle missing/malformed gracefully (no error handling around JSON.parse).
• Dead code: signalAbort (daemon.ts:325, 480, 494), bySource fallback (registry.ts:256), handleProtocolSendTelemetry no-op (methods.ts:461-470), legacy PtyPane (pty-pane.tsx:412-552), redundant catch on executeRun (run-manager.ts:85).
• RingBuffer capacityBytes is character-count (supervisor.ts:25-26, 42-50). Doc says "characters", parameter is named capacityBytes, code uses string.length (UTF-16 code units). The slice(excess) at :48 can split a surrogate pair, producing invalid UTF-16 that surfaces when re-emitted as JSON-RPC.
• daemon restart CLI (commands/cli/daemon.ts:33-150) hardcodes DEFAULT_RESTART_TIMEOUT_MS=2000 and DEFAULT_RESTART_POLL_MS=50 and exposes neither as CLI flags. Worst-case wait is silently 2x timeout. Add --timeout and --poll-interval.
• cli.ts:501-506 uses process.argv.slice(2).includes("--ui-server") to short-circuit. Could mis-match if a positional arg ever contains --ui-server. Use Commander parsed flag.
• chat-session-panel.tsx:310-313: process.on("SIGINT", forwardSigint) adds without dedup; multiple panels stack handlers. Track and dedup.
• PANEL_EXIT_SIGNALS duplicated inline at panel-client.tsx:181-189, 464. Extract.
• Comment hygiene (CLAUDE.md: WHY only): pty-pane.tsx:407-410 describes mechanics; either delete or rewrite as WHY. Some runtime/registry.ts JSDoc ("first one") is non-deterministic and misleading for multi-agent files.

---

Migration doc

packages/atomic-sdk/docs/migration-1x-to-2.md is concise and well-written but skips the failure-mode section a long-lived daemon really needs:

• Stale endpoint file recovery story: behaviour is implemented and tested but the doc never tells users what to do if ~/.atomic/daemon.endpoint.json is orphaned.
• Port conflict / EADDRINUSE: kernel-assigned port avoids the obvious case, but TIME_WAIT exhaustion on rapid-restart cycles, or any other loopback service binding the cached port, is undocumented.
• Auth-token rotation across daemon restarts: ui-server.md:216 notes tokens are per-daemon-lifetime; clients caching the env var will see auth failures on restart. Surface this.
• Daemon log rotation (~/.atomic/daemon.log) not mentioned.

Add a short "When something goes wrong" section before merge.

---

Summary

The architecture is right; the tests catch a lot. The recurring theme across findings is that this is now a long-lived per-user process with shared state — but several subsystems still behave like the short-lived tmux helper they replaced (no subscription/run pruning, hot-reload that does not actually reload, SIGKILL with no grace, no daemon-side connection-close hook). Address the must-fix list (especially the singleton race, the NUL byte, and the missing export-registry CI wiring), and most of the rest can land in incremental follow-ups.