Skip to content

Delete terminals:dirty channel; collapse session autosave to direct write #803

@srid

Description

@srid

Symptom

packages/server/src/session.ts:89-108 runs an initSessionAutoSave loop that subscribes to the terminals:dirty channel, leading-edge-throttles at 500ms, and persists the session blob.

The channel fires at high frequency: packages/server/src/meta/state.ts:63 publishes terminals:dirty on every metadata publish, including live-only fields. Per the comment at session.ts:80-84, the Claude transcript watcher fires terminals:dirty every 150ms while an agent is streaming.

But SavedTerminal (packages/common/src/index.ts:152-176) persists only cwd, git, themeName, parentId, canvasLayout, subPanel, lastAgentCommand. The high-frequency live fields (agent, pr, foreground, declared in LiveTerminalFieldsSchema at index.ts:184-191) are not persisted.

Result: during agent streaming, terminals:dirty fires ~6×/sec, the throttle schedules a save every 500ms, and that save writes a SavedTerminal blob whose persisted bytes are identical to the previous one. Most saves during streaming are no-op identical writes that still publish to every connected client via session:changed.

Why this matters

  • session.ts carries the only autosave-with-throttle in the codebase. preferences.ts (packages/server/src/preferences.ts:28-40) writes directly at the mutation site. Session is the odd one — because the signal is overgenerous, not because the content churns.
  • The throttle subscription owns a module-level saveTimer and a documented test-ordering race (session.ts:55-65) that exists only because the throttle exists. Both vanish under the fix.
  • Disk and publishSystem("session:changed", …) traffic during agent streaming is wasted.

The fix is simpler than first sketched: delete the channel

Initial sketch was "split terminals:dirty into live vs. persisted-dirty channels." Investigation showed this is overcomplicated. Grep for terminals:dirty returns:

  • 2 publishers: terminals.ts:78 (create/kill), meta/state.ts:63 (every metadata publish).
  • 1 subscriber: session.ts:97 (the autosave loop).
  • Zero other consumers.

The live UI does not use terminals:dirty. Live terminal.list rides on the terminal-list channel (publisher.ts:46-47); per-terminal metadata streams ride on metadata:<id> channels (publisher.ts:21-23, published at meta/state.ts:62). terminals:dirty is a 1:1 pipe whose entire purpose is to be the throttle's input. There's nothing to split.

Fix shape

session.ts collapses to the preferences pattern: saveSession(snapshot()) is called directly at the small set of sites that mutate persisted fields:

  • onCwd callback in terminals.ts:135 (writes cwd via updateServerMetadata).
  • The git provider when git resolves/changes (packages/server/src/meta/git.ts).
  • Client-driven RPCs that write themeName / canvasLayout / subPanel / parentId.
  • The preexec hook that writes lastAgentCommand (packages/server/src/meta/agent-command.ts).
  • createTerminal (terminals.ts:92-172) — initial seeding.
  • killTerminal and the onExit callback (terminals.ts:108-122, terminals.ts:175+) — removal.

Once those direct calls land, terminals:dirty has zero consumers. Delete:

  • The channel typing in publisher.ts:45 ("terminals:dirty": Record<string, never>).
  • Both publishers: terminals.ts:78, meta/state.ts:63.
  • The subscription loop and saveTimer in session.ts:89-108.
  • The cancellation logic and the test-ordering race comment in session.ts:55-65 (setSavedSession's clearTimeout block).

Result: three persistence modules (preferences.ts, session.ts, future tasks.ts) share one shape — direct store.set at the mutation site + a single :changed publish — and the codebase loses one channel, one timer, and one documented race.

Risk

store.set is sync and cheap, but publishSystem("session:changed", …) reaches every connected client. Worst case under the fix is N writes for N rapid persisted-field mutations (e.g. session-restore replay) — vs. one write per 500ms today. Audit the batched-mutation sites before merging; if a same-tick burst is meaningful, a small runWithSave(() => { … saveSession() }) wrapper at each call site is sufficient (no need for a publisher-throttled loop).

Out of scope

tasks.ts (the new persisted domain landing under #760's refinement) does not depend on this fix; it joins the cluster directly with the preferences pattern. This issue is about cleaning up the fourth module to match.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions