fix(orchestrator): sessions are orchestrator-scoped — one agent across all tabs and workflows (#884)

[artokun]

Closes #884

Closes #902

This was a P0 bug, not an enhancement

#884 was filed (by the triage side of this system, not the reporter) as an enhancement with a section titled "Not a bug — but a real gap", framing per-workflow sessions as "the design working correctly". That framing was wrong and is corrected here publicly: the owner's stated invariant is that agents are session-bound, with knowledge of all open workflows — one session spans all panels, all tabs, and all workflows, stored and managed by the orchestrator and persisted on disk, not the browser. A workflow-scoped, tab-scoped or panel-scoped agent is a bug, never the design. The reporter (seanmcmagic, Discord #help) hit exactly that bug: every Workflow → New lobotomized the agent mid-task.

Root cause

agentKeyFor(panelTabId) = panelTabId + "::" + backendForTab(panelTabId) (introduced as collateral of 0e09dbd, the single-port multi-provider work). The panel's tab_id is the workflow identity (wf:<path> / tmp:<uuid>), so the agent key — and with it the conversation — was workflow-scoped.

And this did regress recently, which is why the reports are arriving now. The invariant was actually shipped in July (panel 0.9.0 + the orchestrator same-socket rebind; see docs/blog/panel-owned-sessions.mdx — "the conversation is the unit of continuity … the orchestrator rebinds the live agent across tabs"). The per-workflow key was still underneath, but the rebind papered over it. Then #570's wrong-resume hardening made the rebind fail closed: a same-socket re-hello without proven workflow-uuid continuity is treated as a switch — the old agent is retired and the new tab starts fresh. Workflow → New mints a different uuid by construction, so it can never prove continuity: the reporter's exact case ("every time the agent does Workflow_New it loses all of its context"). The panel's default chat scope has been panel-wide all along and re-sends its one session id for every workflow — but the #570 ownership gate refused that resume too, annotating it as "a separate, separately-authorized feature". The two layers disagreed about who owns the conversation; the orchestrator was wrong. This PR removes the disagreement at the root instead of re-papering it: the key itself stops being workflow-scoped.

Design: session identity and routing target are now two different things

tab_id was doing two jobs. The fix separates them instead of deleting either:

(a) Session identity — orchestrator::<backend> (SHARED_SESSION_SCOPE in src/services/session-scope.ts). The backend half survives because switching provider deliberately restarts the agent (the panel replays the transcript to seed the new provider) — the workflow half is gone. One agent per backend per orchestrator, persisted in ~/.comfyui-mcp/sessions/panel-sessions-<port>.json (port-keyed so two ComfyUI instances never cross-resume).

(b) Routing target — the real per-tab ids, at three chokepoints:

• Tool calls out: the panel MCP servers (in-process Claude server and the Codex/Gemini loopback HTTP MCP URL) bind the backend-qualified scope address (the agent key, orchestrator::<backend>). While a turn is in flight, its tool calls are PINNED to the tab the turn was issued from — "the active tab" is ambiguous by construction mid-turn (a queued message from another tab moves it immediately), so the pin is set at the same batch-close where the issue-time stamp lands, follows the tab's own same-socket migration (a workflow switch on the turn's OWN tab), and refuses loudly (the standard no-connected-tab error, parked/retried by the existing reconnect machinery) when the pinned tab is gone or the batch had mixed origins — never a silent fallback to another tab, and the scope ctx is never silently rebound onto a real tab id. Idle-time scope resolution (capability probes, between turns) falls back to the last-user-message tab, else the most recent interactive connection. The Session resume keyed by ephemeral tmp:<uuid> tab id — unsaved workflows always lose conversation across an orchestrator restart #570 per-command workflow stamp/fence keeps its issue-time semantics: the workflow a turn was issued for is captured per conversation and rides the message to its dequeue (lastTurnUuidByKey), so a mid-turn switch makes late edits fail the panel's fence loudly instead of silently re-aiming; the agent's own validated panel_open_workflow / re-pin refreshes stamp + pin via the Stale workflow UUID after reconnect/open/re-pin rejects panel_run #716 path, so deliberate canvas moves keep working. Run/ask journal tickets key the real routed tab, captured at dispatch. (This routing model went through five adversarial rounds — the pin itself is the confirming-gate P0 fix.)
• Frames back: agent output (say/stream/turn/thinking/action/status/session/turn_anchor/seen-acks) fans out to every connected tab on the agent's backend — the same conversation is visible from every tab. Fanout goes through bridge.push per tab, so the mirror allowlist (MIRROR_SAFE_FRAME_TYPES), pinned socket kind, and canonical-id fanout keep their invariants untouched. With no participating tab connected, frames park per agent key (backend-qualified — a claude turn finishing while only a codex tab is open can never paint into the codex conversation) and flush to the next hello / set_backend join on that backend; bridge-level scope buffering additionally covers the zero-tabs mailbox path.
• Knowledge of open workflows: when a user message's origin workflow differs from the previous message's, a one-line note is prepended ("you are now operating on …; panel tools target THIS workflow now; the conversation itself continues"). Plus the existing panel_list_workflows / workflow_open surface, and the run/ask journals stay keyed by the real originating tab.

Active/foreground workflow when a call omits a target: the tab the user last talked from; the panel_set_workflow_target pin still overrides per call, and is now conversation-scoped (stored under the agent key) rather than per-tab.

Two further codex round-1 findings fixed alongside: download rows now stamp the owning agent key instead of a tab id (a tab switching backends mid-download can no longer send download_done into the wrong conversation), and the session store treats an existing-but-corrupt home file as empty rather than falling back to the stale pre-migration tmpdir store (missing ≠ corrupt; flushes are atomic via temp+rename).

What the hello handler does now

A same-socket re-hello under a new tab id (workflow switch, save/rename, Workflow → New, id-scheme change) never touches the agent. It carries socket-scoped prefs (backend, headless, blind), moves the run-completion/ask-answer journals to the new id (a render finishing after a workflow switch now reaches the conversation that queued it — previously it was dropped on an unproven switch), and re-stamps the command fence from the hello's trusted identity. The whole #570 per-workflow machinery — migration discriminator, destination-collision reset, stable resume keys + the poisoned stable index, sibling-ownership guards, the in-place-replacement teardown, hello.resume ownership gating — enforced the isolation this invariant abolishes, and is deleted rather than routed around. hello.resume is now a last-resort hint honored only when the orchestrator's own store has nothing (wiped disk).

Provider switches keep #570's retire-not-reset semantics (A→B→A resumes), with one new guard: the shared agent is retired only when no other connected tab still runs on that backend — one tab's switch can never stop an agent other tabs are using (shouldRetireSharedAgent). Per-tab backend selection itself is unchanged: a tab switching to Codex does not flip anyone else's backend; it changes which shared conversation that tab participates in.

Conversation boundaries (new_session / resume_session / rewind) now apply conversation-wide: journals close for every participating tab, and the session-cleared frame fans out.

The session store (second discrepancy in the issue)

Moved from join(tmpdir(), ...) (world-writable on some systems, routinely wiped) to ~/.comfyui-mcp/sessions/, per the owner's words. Migrations, both one-shot:

• Location: if the new file is absent, the old tmpdir file's sessions are imported and re-persisted at the new location (the old file is left for any still-running older build).
• Keying: on the first lookup of a shared key with no entry, the most recently used legacy per-workflow entry for that backend is adopted as the shared conversation — an upgrading user's newest conversation keeps its memory. Adoption consumes the legacy entries for that backend, so a later "New chat" can never resurrect them (older conversations remain reachable through the panel's history picker via explicit resume_session, which carries the session id itself). Test/spike keys (e2e-*, spike-*) are never adopted.
• The stable index (per-workflow resume fallback for unsaved workflows, including its poison mechanism) is obsolete under a shared key that never churns; it is dropped on load and never written. Nothing can trip the poison because the index no longer exists.
• Per Third instance: tests write to real ~/.comfyui-mcp state. Guard at the WRITE, not per-test #866 ("guard at the WRITE, not per-test"): the store refuses its real-home default when constructed under the test runner without an explicit { dir }, using remote ComfyUI and secrets: HTML where JSON was promised, and a secret that reports success without reaching the child #837's runner-detection (vitest worker globals; uncertain → allow).

Companion panel PR (lands together)

artokun/comfyui-mcp-panel#680 — hard-wires chatScopeMode() to "panel" and removes the "Chat conversation scope" setting (the legacy workflow / ask modes were per-workflow sessions by another name; the owner ruled the shared session is the default and the only behavior). The panel's default mode needs no panel change for this fix to work — the panel already treats the conversation as panel-owned; the orchestrator was the layer breaking it. Either PR is safe to land first; together they close the loop.

Deliberate behavior consequences (not regressions)

• Blind mode (fix: generation tools — live-render widget fixes (generate_video, remove_background) #90) is now conversation-wide: pixels are withheld from the shared agent while any tab has Blind on (a per-tab gate would leak pixels to the shared agent through other tabs).
• New chat resets the shared conversation for that backend everywhere — that is what a shared session means.
• The mobile-mirror optimistic follow across a same-socket workflow switch is no longer undone (revokeTabMigration at the switch site) — under a shared session the mirror keeps seeing the same conversation either way; the bridge-side mirror security invariants (allowlist, pinned kind, owner-only send, canonical-id fanout) are untouched and their tests pass.
• A tab that connects mid-conversation shows the transcript from its join point (plus panel-side persisted history); backfilling the full shared transcript into a newly-joined panel is a panel-side follow-up, not part of the P0.
• #436's stamp-carry is gone because nothing binds a session to a per-workflow tab id anymore; a straggler command for a retired id now fails the fence closed (a retry through the scope heals it).

Adversarial gate (codex gpt-5.6-sol, four rounds)

• R1 (FAIL→fixed): turn-blind scope stamps (P0 — mutations re-aimed at whatever tab was active at dispatch; now issue-time per conversation), backend-blind output fallthrough (fixed one commit earlier via per-key frame parking), download wake mis-keyed after a backend switch, corrupt home store resurrecting the tmpdir store.
• R2 (FAIL→fixed): stamps apply at DEQUEUE (onSeen) not receipt; attached mirror viewers deduped from fanout; downloads never re-routed to a wrong sole-live agent + HTTP lane stamps its downloads; conversation boundaries sweep disconnected members.
• R3 (FAIL→fixed): mixed-origin dispatch batches fail closed instead of last-message-wins; mid-less messages can't flip an in-flight fence; run/ask journal tickets keyed by the real routed tab (a scope-keyed ticket made the agent's own render come back "foreign"); mid-map lifecycle hardened (cancel cleanup, consumed-ring, unknown→fail-closed); attached viewers still get their seen-acks directly.
• R4: scoped verification of the R3 fixes; its two findings (ticket-capture timing, cancel-guard) landed in d02c372/40c2d4b.
• R5 (confirming gate, FAIL→fixed): the big one — turn-target pinning (P0: a queued message from another tab re-aimed an in-flight turn's tool calls, and the Stale workflow UUID after reconnect/open/re-pin rejects panel_run #716 refresh laundered the fence; the turn now pins its target at dispatch and refuses loudly, with the scope ctx never silently rebound); the store's truncate-in-place fallback removed (P0: failing to persist is strictly better than destroying the store — .tmp recovery added); New chat's durable-clear outcome made observable (durable_cleared on the ack + chat disclosure — the false-success class); backend-qualified scope buffers drain only to matching-backend hellos with partial-send re-buffering; tests strengthened at the real seams (two tabs/two backends at the live bridge, store-durability, manager two-backend isolation).

Tests

• New: src/__tests__/orchestrator/shared-session-invariant.test.ts — the P0: agent sessions are workflow-scoped — context is lost on Workflow → New, on switching workflows, and across tabs #884 regression tests (fail on pre-P0: agent sessions are workflow-scoped — context is lost on Workflow → New, on switching workflows, and across tabs #884 code): the key composition; the migration block touches no agent and moves the journals; retires are guarded; resume-hint is last-resort; one key ⇒ one agent ⇒ one shared history across workflow changes on the real manager path; legacy adoption feeds the real spawn's resume.
• New: src/__tests__/services/session-scope.test.ts; scope-routing tests in ui-bridge.test.ts driving the real WS bridge (scope resolves to the tab the user last talked from; headless viewers never steal routing; scope frames buffer and replay; scope mutations stamp from the resolved conn).
• Rewritten: session-store.test.ts (the stable-index/poison/identity suites tested deleted machinery; core persistence/GC/clamp/migration coverage kept and extended). ui-bridge.test.ts release: 0.48.8 #436.3 now pins the P0: agent sessions are workflow-scoped — context is lost on Workflow → New, on switching workflows, and across tabs #884 stamp semantics. run-completion-continuation.test.ts's collision-occupancy test became a moveKey-merge test. ask-answer-journal.test.ts's journal-pairing rule now also pairs moveKey and tolerates zero forget sites (there are none left, deliberately).
• Kept intact (different concerns): tab-id-migration-staleness.test.ts and in-place-replace-reset.test.ts pin manager rebind/teardown primitives still used by explicit boundaries — re-pointed at scratch store dirs so they stop writing the real home (Third instance: tests write to real ~/.comfyui-mcp state. Guard at the WRITE, not per-test #866).
• Full suite: 287 files, 5997 passed / 1 expected fail / 2 skipped — on Windows (single-platform evidence; CI covers ubuntu/macOS).

#902 is the same bug wearing a scarier costume — chain verified in code, not assumed

#902 ("panel_open_workflow rebind guard fails AND every MCP server disconnects; 'whenever you want to build something on a new tab, the new tab disconnects you'") is a downstream symptom of the per-workflow keying, and this PR closes it. The verified chain, leg by leg:

1. panel_open_workflow(B) switches the canvas → the panel re-hellos on the same socket under B's tab id → pre-fix, B's uuid differs from A's by construction → the hello handler's switch branch fires.
2. That branch (main src/orchestrator/index.ts:3204) calls manager.retire(oldTab::backend) — stopping the calling agent mid-tool-call. PanelAgent.stop() disposes the backend/SDK session, and every MCP server rides that session (buildMcpServers() = the comfyui stdio child plus the user's inherited servers — civitai, blender-mcp, wangp in the report). One retire = all of them die; the respawn on the next message reconnects them. That is the reported "89 deferred tools are no longer available … reconnected" cycle, exactly.
3. The same branch calls bridge.revokeTabMigration(oldTab), and waitForOpenReceipt (the verify-after-timeout block panel_open_workflow: rebind guard fails AND correlates with full MCP orchestrator disconnect (0.49.8) #902 points at) probes via the tool ctx bound to the old tab id — now unroutable → "outcome undetermined" → the rebind-guard failure. The verify machinery itself was doing its job; the ground was pulled out from under it.

Under this PR none of the mechanisms exist: the re-hello never retires/resets/rebinds the agent and never revokes the migration alias (both pinned by source assertions in shared-session-invariant.test.ts), the tool ctx is scope-bound so the verify probes resolve the live (newly-opened) tab, and a successful open re-stamps the turn's workflow fence via the #716 path. Covered by a regression test that a workflow switch performs zero backend teardowns across the switch (backend.closes === 0, one run() — one SDK session, MCP children never churned). The canvas-revert leg of the report is panel-side fallout of the dead session and is expected to disappear with the cause; if any revert survives this fix, it is a separate panel bug to file fresh.

Related open issues (same root cause, listed for triage — not auto-closed here)

• feat(runpod): persist custom_nodes on the volume (survive restarts) #111 — seanmcmagic's community-request list includes the context-loss item this fixes.
• Fresh sidebar agent session loses workflow panel registration comfyui-mcp-panel#682 ("Fresh sidebar agent session loses workflow panel registration", related release: 0.48.7 #419/release: 0.48.8 #436/fix(tools): disambiguate panel_graph_outline vs visualize_workflow vs panel_query_graph (#557) #654 per its own triage) — the wf:<path>-bound session wedge class. Under scope routing an agent session is never bound to a dead per-workflow id (canReach(scope) is true whenever any tab is connected), so the wedge path shouldn't exist once this + panel fix(manager): route update_all tool through detectManagerApi (#656) #680 land; close manually after a re-test rather than auto-closing from here (it lives in the other repo and its report predates this fix).
• The "workflow instance mismatch → permanently wedged until browser refresh" class (e.g. the panel_set_workflow_target({mode:"current"}) no-repair reports): same reasoning as fix(tools): compact tool mode is the default; --full opts back in (#667) #682. Worth re-testing those reports after this lands.

🤖 Generated with Claude Code

---

Design decisions (confirming gate 4)

Two calls made explicitly during the gate-4 remediation, recorded here per the
gate's request rather than left implicit in the diff:

1. A requeued message keeps its origin. The consumed-mid bookkeeping used
to conflate "this mid's stamp was already applied" with "this mid has no
origin" — which let interrupt-with-requeue ("send now") launder a
mixed-workflow batch: message A's restored queue item contributed nothing, so
a merged A+B turn was pinned and stamped entirely to B, and A's requested edit
ran against B's graph. The rule is now: "already applied" means "contributes
no new stamp of its own", never "origin-less." A requeued item
re-contributes its recorded origin (tab + issue-time uuid + backend) at every
dequeue — alone, its re-run pins its own tab again; merged with another tab's
message, the batch is genuinely mixed and fails closed. Consequences accepted
deliberately: cross-tab send-now now refuses routing/mutations until the agent
explicitly targets a workflow (the mixed-batch doctrine — never guess), and a
requeued injected event (e.g. a run error) re-pins its own erroring tab
instead of inheriting.

2. Pin ownership is verified at resolution, not only at events. The gate's
P0-1 asked for invalidation "when a tab's backend changes". Event-driven
invalidation exists (tabChangedBackend, fired from both provider-switch
sites, judging pins by where the bridge routes them so path-compressed
migration aliases are covered) — but two gate rounds showed the event can be
unobservable: recurring wf:<hash> ids let a stale pin revive onto another
backend through a fresh socket with no switch ever seen. So the scope
resolver's pin lookup (resolvedPinOf) additionally verifies, at every use,
that the tab the pin routes to still belongs to the conversation's backend,
and fails closed persistently (null pin, dead stamp, loud warn) on the first
mismatch. Check-at-use closes the whole arrival-order class; the event layer
remains for early, precisely-attributed diagnostics.

[Copilot]

Pull request overview

Moves agent sessions from workflow scope to orchestrator scope while retaining backend-specific conversations and per-workflow command routing.

Changes:

• Adds shared session identity, fanout, routing, and workflow-origin context.
• Migrates durable sessions to ~/.comfyui-mcp/sessions.
• Reworks session, bridge, migration, and journal tests.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/services/ui-bridge.ts	Routes and replays shared-scope traffic.
src/services/session-scope.ts	Defines shared-session behavior.
src/orchestrator/session-store.ts	Migrates and persists shared sessions.
src/orchestrator/index.ts	Integrates shared agents and routing.
src/__tests__/services/ui-bridge.test.ts	Tests shared-scope bridge routing.
src/__tests__/services/session-scope.test.ts	Tests session-scope helpers.
src/__tests__/orchestrator/tab-id-migration-staleness.test.ts	Isolates migration tests from real state.
src/__tests__/orchestrator/shared-session-invariant.test.ts	Adds shared-session regression coverage.
src/__tests__/orchestrator/session-store.test.ts	Rewrites persistence and migration coverage.
src/__tests__/orchestrator/run-completion-continuation.test.ts	Tests journal key merging.
src/__tests__/orchestrator/in-place-replace-reset.test.ts	Updates manager lifecycle test isolation.
src/__tests__/orchestrator/ask-answer-journal.test.ts	Verifies paired journal migration operations.

Suppressed comments (4)

src/orchestrator/index.ts:1578

• This makes Blind global across all provider agents, but the toggle handlers restart only agentKeyFor(tabId). If another backend already has a live tool server, enabling Blind leaves that server running without COMFYUI_MCP_BLIND=1, so it can still return pixels despite anyTabBlind() now being true. Either make Blind backend-scoped or restart every affected live backend whenever this global predicate changes.

  // #884 — Blind mode (issue #90) is a promise that the AGENT never receives
  // pixels. The agent is now shared, so the promise is conversation-wide: pixels
  // are withheld while ANY tab has Blind on (a per-tab gate would leak pixels to
  // the shared agent through the other tabs).
  const anyTabBlind = (): boolean => blindTabs.size > 0;

src/orchestrator/session-store.ts:199

• The location migration currently runs after any read/parse failure, not only when the new file is absent. Because the old tmp file is intentionally retained, corruption or a transient permission error on the authoritative file can resurrect an obsolete pre-upgrade conversation. Check existsSync(this.path) first and start empty/report the error when an existing new store is unreadable; consult legacyPath only when the new path does not exist.

  private read(): { sessions: Record<string, Entry>; dirty: boolean } {
    try {
      return this.parse(readFileSync(this.path, "utf8"));
    } catch {
      // Missing or corrupt at the NEW location — try the pre-#884 tmpdir file so
      // an upgrade carries existing conversations over (location migration).
    }
    try {
      if (existsSync(this.legacyPath)) {
        const migrated = this.parse(readFileSync(this.legacyPath, "utf8"));

src/orchestrator/index.ts:3649

• The pin is stored under one literal scope for all backends, so a target selected by one provider changes every other provider's agent too. The PR defines one agent/session per backend and calls this state agent-scoped; key the target and sequence by the backend-qualified shared agent key so concurrent Claude/Codex conversations cannot overwrite each other's pin.

      // #884 — the pin belongs to the AGENT (whose tool ctx is bound to the
      // shared scope), not to one tab: store + sequence live under the scope so
      // the agent's command injection and this picker agree, and a newer
      // selection from ANY tab supersedes an in-flight async pin.
      const seq = (workflowTargetSeq.get(SHARED_SESSION_SCOPE) ?? 0) + 1;
      workflowTargetSeq.set(SHARED_SESSION_SCOPE, seq);
      const isCurrent = () => workflowTargetSeq.get(SHARED_SESSION_SCOPE) === seq;

src/orchestrator/index.ts:2124

• lastFocusTab is global even though managers are keyed per backend. After activity on backend B, a later respawn of backend A gets B's COMFYUI_MCP_TAB; download progress and completion wakeups are then attributed to B's shared agent rather than the agent that spawned the tool. Track focus per shared agent key and use the callback's key here.

    // Per-KEY factory — spawns must reflect live state (the Blind gate, the
    // focus-tab download stamp); the static set above stays as the fallback.
    makeMcpServers: () => buildMcpServers(lastFocusTab),

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.