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

src/__tests__/orchestrator/ask-answer-journal.test.ts

@@ -1002,7 +1002,11 @@ describe("ask-answer journal — bounds may LABEL, never silently lose (#486)",
     // and pushes it to whoever holds the tab, so a retired conversation's pick
     // must be counted there, not quoted. The durable LOG still gets everything —
     // two audiences, two buckets.
-    const exitAt2 = src.indexOf("AskAnswers.allOutstanding()");
+    // Anchor inside the EXIT-DISCLOSURE function: #884's boundary sweep and
+    // journal-flush helpers also call allOutstanding() earlier in the file.
+    const exitFnAt = src.indexOf("function reportLostCompletionsOnExit");
+    expect(exitFnAt, "exit-disclosure function not found").toBeGreaterThan(-1);
+    const exitAt2 = src.indexOf("AskAnswers.allOutstanding()", exitFnAt);
     expect(exitAt2, "exit disclosure not found").toBeGreaterThan(-1);
     const exitBlock = src.slice(exitAt2, exitAt2 + 900);
     expect(exitBlock, "the notice must gate content on the boundary").toContain(
@@ -1049,20 +1053,38 @@ describe("ask-answer journal — bounds may LABEL, never silently lose (#486)",
     // Pairing the two journals structurally is what stops the next boundary from
     // being added to one and forgotten in the other — which is exactly how a late
     // answer reached a conversation that never asked the question, twice.
-    const pairs: Array<[RegExp, string]> = [
-      [/RunCompletions\.closeRuns\((\w+)\)/g, "AskAnswers.closeAsks($1)"],
-      [/RunCompletions\.forget\((\w+)\)/g, "AskAnswers.forget($1)"],
+    const pairs: Array<[RegExp, string, boolean]> = [
+      [/RunCompletions\.closeRuns\((\w+)\)/g, "AskAnswers.closeAsks($1)", true],
+      // #884 removed every forget() site: an unproven workflow transition no
+      // longer purges the journals, because the conversation is orchestrator-
+      // scoped and deliberately CONTINUES across a workflow switch (pending
+      // deliveries follow the socket via moveKey instead). The pairing rule
+      // still stands for any future forget() site — it just may be absent.
+      [/RunCompletions\.forget\((\w+)\)/g, "AskAnswers.forget($1)", false],
     ];
-    for (const [re, template] of pairs) {
+    for (const [re, template, required] of pairs) {
       const found = [...src.matchAll(re)];
-      expect(found.length, `no ${re.source} call sites found`).toBeGreaterThan(0);
+      if (required) {
+        expect(found.length, `no ${re.source} call sites found`).toBeGreaterThan(0);
+      }
       for (const m of found) {
         const arg = m[1];
         const want = template.replace("$1", arg);
         const near = src.slice(m.index!, m.index! + 900);
         expect(near, `${m[0]} is not paired with ${want}`).toContain(want);
       }
     }
+    // …and the #884 boundary-follows-the-socket rule has the same two-journal
+    // pairing obligation: a moveKey in one journal must move the other too.
+    const moved = [...src.matchAll(/RunCompletions\.moveKey\((\w+), (\w+)\)/g)];
+    expect(moved.length, "no RunCompletions.moveKey call sites found").toBeGreaterThan(0);
+    for (const m of moved) {
+      const want = `AskAnswers.moveKey(${m[1]}, ${m[2]})`;
+      expect(
+        src.slice(m.index!, m.index! + 900),
+        `${m[0]} is not paired with ${want}`,
+      ).toContain(want);
+    }
   });
 
   // codex round 7, P1: an answer already QUEUED into an agent carries wording

src/__tests__/orchestrator/in-place-replace-reset.test.ts

@@ -1,12 +1,16 @@
-// #570 P0 — a SAVED workflow overwritten IN PLACE (same wf:<path> tab id, new uuid) must
-// start FRESH. The hello handler detects the durable identity mismatch and calls
-// manager.reset(key) — a FULL session boundary. This pins the manager-lifecycle half:
-// (1) the exact session is BOUND to the identity uuid (identityForKey → SessionStore.u),
-// and (2) reset() stops the LIVE agent AND clears the durable session + pending resume, so
-// the next message can't continue the replaced workflow's conversation.
+// Manager teardown mechanics, originally pinned for #570's in-place workflow
+// replacement. #884 (orchestrator-scoped sessions) removed the hello handler's
+// per-workflow identity boundary — a workflow switch/replacement no longer
+// resets anything — but the SAME manager primitives now back the explicit
+// conversation boundaries (new_session / resume_session), so what these pin is
+// still load-bearing: (1) the durable identity binding plumbing
+// (identityForKey → SessionStore.u) stays coherent, and (2) reset() stops the
+// LIVE agent AND clears the durable session + pending resume + held mail, so
+// the next message after an explicit boundary can't continue the cleared
+// conversation.
 
-import { describe, expect, it, beforeAll, afterEach } from "vitest";
-import { rmSync } from "node:fs";
+import { describe, expect, it, beforeAll, afterAll, afterEach } from "vitest";
+import { mkdtempSync, rmSync } from "node:fs";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
 import type {
@@ -24,13 +28,31 @@ beforeAll(async () => {
 });
 
 const PORT = 59244;
-const FILE = join(tmpdir(), `comfyui-mcp-panel-sessions-${PORT}.json`);
+// #884 — SessionStore lives in ~/.comfyui-mcp/sessions by default; tests pin it
+// to a scratch dir so they never touch the real home (and never leak state
+// between tests through the shared file).
+const DIR = mkdtempSync(join(tmpdir(), "cmcp-sessions-"));
+const FILE = join(DIR, `panel-sessions-${PORT}.json`);
 afterEach(() => {
   try {
     rmSync(FILE);
   } catch {
     /* already gone */
   }
+  // A stale pre-#884 tmpdir file would be silently MIGRATED into the next
+  // store — remove it so tests are hermetic.
+  try {
+    rmSync(join(tmpdir(), `comfyui-mcp-panel-sessions-${PORT}.json`));
+  } catch {
+    /* already gone */
+  }
+});
+afterAll(() => {
+  try {
+    rmSync(DIR, { recursive: true, force: true });
+  } catch {
+    /* best-effort */
+  }
 });
 
 class SessioningBackend implements AgentBackend {
@@ -65,7 +87,7 @@ const IDENTITY_A = `http://127.0.0.1:8188::${UUID_A}`;
 describe("in-place workflow replacement resets the live session (#570 P0)", () => {
   it("binds the exact session to its identity uuid and reset() clears the live agent + session", async () => {
     const backend = new SessioningBackend();
-    const store = new SessionStore(PORT);
+    const store = new SessionStore(PORT, { dir: DIR });
     const manager = new PanelAgentManager({
       mcpServers: {},
       systemAppend: "",
@@ -103,7 +125,7 @@ describe("in-place workflow replacement resets the live session (#570 P0)", () =
     // a save/rename changes the wf: tab id. The migration must move the dormant Claude session to
     // the new id too — not only the current (Codex) one — or switching back to Claude starts fresh.
     const backend = new SessioningBackend();
-    const store = new SessionStore(PORT);
+    const store = new SessionStore(PORT, { dir: DIR });
     const manager = new PanelAgentManager({
       mcpServers: {},
       systemAppend: "",
@@ -132,7 +154,7 @@ describe("in-place workflow replacement resets the live session (#570 P0)", () =
     expect(store.get("wf:old.json::claude")).toBeUndefined();
     expect(store.get("wf:old.json::codex")).toBeUndefined();
     // …and durably: switching back to Claude on the new id resumes its original conversation.
-    expect(new SessionStore(PORT).get("wf:new.json::claude")).toBe("sess-claude");
+    expect(new SessionStore(PORT, { dir: DIR }).get("wf:new.json::claude")).toBe("sess-claude");
 
     await manager.stopAll();
   });
@@ -142,7 +164,7 @@ describe("in-place workflow replacement resets the live session (#570 P0)", () =
     // bridge has already SUPERSEDED the destination's socket, so its still-mapped agent would
     // render into the incoming tab. Reset the superseded destination, THEN rebind the source into
     // the freed id so the incoming tab keeps its OWN conversation.
-    const store = new SessionStore(PORT);
+    const store = new SessionStore(PORT, { dir: DIR });
     const manager = new PanelAgentManager({
       mcpServers: {},
       systemAppend: "",
@@ -182,7 +204,7 @@ describe("in-place workflow replacement resets the live session (#570 P0)", () =
     // destination's session → the incoming tab resumes the OTHER tab's conversation on a later
     // switch. The collision handling must reset the destination for ANY state (incl. a dormant
     // durable session / held mail), not just a live agent.
-    const store = new SessionStore(PORT);
+    const store = new SessionStore(PORT, { dir: DIR });
     const manager = new PanelAgentManager({
       mcpServers: {},
       systemAppend: "",
@@ -207,7 +229,7 @@ describe("in-place workflow replacement resets the live session (#570 P0)", () =
 
     // B's dormant session is GONE — the incoming tab starts fresh on Claude, never inherits it.
     expect(store.get(newKey)).toBeUndefined();
-    expect(new SessionStore(PORT).get(newKey)).toBeUndefined(); // durable
+    expect(new SessionStore(PORT, { dir: DIR }).get(newKey)).toBeUndefined(); // durable
 
     await manager.stopAll();
   });
@@ -220,7 +242,7 @@ describe("in-place workflow replacement resets the live session (#570 P0)", () =
     // stops the live agent but keeps the identity-bound session, so a later switch-back
     // continues the original conversation (codex round-trip). This pins that shared operation.
     const backend = new SessioningBackend();
-    const store = new SessionStore(PORT);
+    const store = new SessionStore(PORT, { dir: DIR });
     const manager = new PanelAgentManager({
       mcpServers: {},
       systemAppend: "",
@@ -246,7 +268,7 @@ describe("in-place workflow replacement resets the live session (#570 P0)", () =
     expect(store.get(claudeKey)).toBe("sess-A");
     expect(store.identityOf(claudeKey)).toBe(IDENTITY_A);
     // Durable across a fresh process too (a restart between switches).
-    expect(new SessionStore(PORT).get(claudeKey)).toBe("sess-A");
+    expect(new SessionStore(PORT, { dir: DIR }).get(claudeKey)).toBe("sess-A");
 
     await manager.stopAll();
   });
@@ -279,7 +301,7 @@ describe("in-place workflow replacement resets the live session (#570 P0)", () =
       }
     }
     const backend = new NoSessionBackend();
-    const store = new SessionStore(PORT);
+    const store = new SessionStore(PORT, { dir: DIR });
     const manager = new PanelAgentManager({
       mcpServers: {},
       systemAppend: "",
@@ -321,7 +343,7 @@ describe("in-place workflow replacement resets the live session (#570 P0)", () =
         return [];
       }
     }
-    const store = new SessionStore(PORT);
+    const store = new SessionStore(PORT, { dir: DIR });
     const manager = new PanelAgentManager({
       mcpServers: {},
       systemAppend: "",

src/__tests__/orchestrator/panel-tools.test.ts

@@ -1480,6 +1480,84 @@ describe("panel-tools: post-reconnect retry-once (#278/#310/#332/#481)", () => {
     expect((res.content[0] as { text: string }).text).toMatch(/multiple|last active|pass tab_id/i);
   });
 
+  // #884 confirming gate 3, P0 — the scope-repin double gate. A SCOPE-bound ctx
+  // (shared conversation) is repinned ONLY by the explicit
+  // panel_set_workflow_target({mode:"current"}) consent AND only when the pin is
+  // provably dead; panel_reload is NOT a consent path. (The full path — real
+  // UiBridge, real tracker, real repin handler — is driven in ui-bridge.test.ts;
+  // these pin the panel-tools gate itself on a stub bridge.)
+  describe("scope-bound ctx: repin consent gate (#884 gate 3)", () => {
+    function scopeBridge(opts: { reachable: boolean }) {
+      const sent: Array<{ cmd: Record<string, unknown>; tabId?: string }> = [];
+      const repinCalls: string[] = [];
+      const bridge = {
+        send: async (cmd: Record<string, unknown>, sendOpts?: { tabId?: string }) => {
+          if (!opts.reachable) {
+            throw new Error(`no connected tab with id "${sendOpts?.tabId}". Connected: none`);
+          }
+          sent.push({ cmd, tabId: sendOpts?.tabId });
+          return { ok: true };
+        },
+        push: () => 1,
+        canReach: () => opts.reachable,
+        tabs: () => [{ tab_id: "live-tab", title: "wf", connected_at: 0 }],
+        isHeadless: () => false,
+        resolveActiveTabId: () => "live-tab",
+        repinScopeToActive: (scopeId: string) => {
+          repinCalls.push(scopeId);
+          return "live-tab";
+        },
+      } as unknown as PanelToolCtx["bridge"];
+      return { bridge, sent, repinCalls };
+    }
+
+    it("panel_reload NEVER repins a scope ctx — healthy binding forwards to the scope untouched", async () => {
+      const { bridge, sent, repinCalls } = scopeBridge({ reachable: true });
+      const ctx = makePanelToolCtx(bridge, "orchestrator::claude", new WorkflowTargetStore());
+      const res = await defByName("panel_reload").handler({ scope: "frontend" }, ctx);
+      expect(res.isError).toBeFalsy();
+      expect(repinCalls).toHaveLength(0); // the healthy turn was not re-aimed
+      expect(ctx.tabId).toBe("orchestrator::claude"); // never narrowed to a real tab
+      expect(sent.at(-1)).toMatchObject({
+        cmd: { cmd: "soft_reload", scope: "frontend" },
+        tabId: "orchestrator::claude",
+      });
+    });
+
+    it("panel_reload on a DEAD scope pin FAILS naming the explicit recovery — no silent repin", async () => {
+      const { bridge, repinCalls } = scopeBridge({ reachable: false });
+      const ctx = makePanelToolCtx(bridge, "orchestrator::claude", new WorkflowTargetStore());
+      const res = await defByName("panel_reload").handler({}, ctx);
+      expect(res.isError).toBe(true);
+      expect((res.content[0] as { text: string }).text).toMatch(
+        /panel_set_workflow_target\(\{mode:"current"\}\)/,
+      );
+      expect(repinCalls).toHaveLength(0);
+    });
+
+    it("rebindToActiveTab repins a scope ctx ONLY with consent AND a provably dead pin", () => {
+      // Healthy + consent → no repin (recovery only, never a re-target).
+      const healthy = scopeBridge({ reachable: true });
+      const healthyCtx = makePanelToolCtx(healthy.bridge, "orchestrator::claude", new WorkflowTargetStore());
+      expect(healthyCtx.rebindToActiveTab!({ scopeRecoveryConsent: true })).toMatchObject({
+        rebound: false,
+      });
+      expect(healthy.repinCalls).toHaveLength(0);
+      // Dead + NO consent → no repin (implicit callers can never re-aim a turn).
+      const dead = scopeBridge({ reachable: false });
+      const deadCtx = makePanelToolCtx(dead.bridge, "orchestrator::claude", new WorkflowTargetStore());
+      expect(deadCtx.rebindToActiveTab!()).toMatchObject({ rebound: false });
+      expect(dead.repinCalls).toHaveLength(0);
+      // Dead + consent → the one recovery path, and the ctx STAYS scope-bound.
+      expect(deadCtx.rebindToActiveTab!({ scopeRecoveryConsent: true })).toMatchObject({
+        rebound: true,
+        current: "orchestrator::claude",
+      });
+      expect(dead.repinCalls).toEqual(["orchestrator::claude"]);
+      expect(deadCtx.tabId).toBe("orchestrator::claude");
+    });
+  });
+
   it("a genuinely-gone tab (no reconnect) still fails clearly after the single retry", async () => {
     const store = new WorkflowTargetStore();
     // Nothing ever becomes live — retry can't rebind, so it must surface an error.

src/__tests__/orchestrator/run-completion-continuation.test.ts

@@ -31,7 +31,6 @@ import {
   type CompletionPayload,
 } from "../../orchestrator/run-completion-journal.js";
 import { AskAnswerJournalImpl } from "../../orchestrator/ask-answer-journal.js";
-import { destinationHasCollisionState } from "../../orchestrator/session-store.js";
 
 let PanelAgentManager: typeof import("../../orchestrator/panel-agent.js").PanelAgentManager;
 
@@ -1462,39 +1461,20 @@ describe("run completion journal correlation (#468)", () => {
     expect(seen).toEqual([PROMPT_A]);
   });
 
-  it("a tab holding ONLY a journal entry counts as OCCUPIED for a tab-id migration", () => {
-    // CRITICAL: the destination of a same-workflow tab-id migration can have had
-    // its agent AND durable session cleared (New chat) and still hold an
-    // undelivered completion. If the occupancy check ignores the journal, the
-    // destination reads as empty, the source is rebound onto its id, and the
-    // next flush hands the DESTINATION tab's render to the SOURCE tab's
-    // conversation — a cross-conversation delivery, worse than a lost one.
-    expect(
-      destinationHasCollisionState({
-        hasManagerState: false,
-        hasDurableSession: false,
-        renderHeldCount: 0,
-        journaledCompletionCount: 1,
-      }),
-    ).toBe(true);
-    expect(
-      destinationHasCollisionState({
-        hasManagerState: false,
-        hasDurableSession: false,
-        renderHeldCount: 0,
-        journaledCompletionCount: 0,
-      }),
-    ).toBe(false);
-    // …and the purge that follows leaves nothing for the incoming agent to
-    // inherit, for BOTH pending and already-handed-off entries.
+  it("moveKey MERGES pending entries onto the destination without losing either side (#884)", () => {
+    // #884 retired the tab-id-migration collision purge (destinationHasCollisionState):
+    // the conversation is orchestrator-scoped, so a same-socket re-hello MOVES the
+    // journals onto the new tab id — a render finishing after a workflow switch
+    // still reaches the one shared conversation that queued it. What must hold now
+    // is that moveKey merges without dropping entries on either key.
     journal.record("wf:dest", { kind: "executed", prompt_id: PROMPT_B });
-    journal.deliverPending("wf:dest", () => true);
-    journal.record("wf:dest", { kind: "executed", prompt_id: PROMPT_A });
+    journal.record("wf:src", { kind: "executed", prompt_id: PROMPT_A });
+    journal.moveKey("wf:src", "wf:dest");
+    expect(journal.outstanding("wf:src")).toHaveLength(0);
     expect(journal.outstanding("wf:dest")).toHaveLength(2);
+    // forget() (no orchestrator call sites since #884) still purges when called.
     journal.forget("wf:dest");
     expect(journal.outstanding("wf:dest")).toHaveLength(0);
-    journal.moveKey("wf:src", "wf:dest"); // then the source migrates in
-    expect(journal.outstanding("wf:dest")).toHaveLength(0);
   });
 
   it("an identical ID-LESS completion is NEVER merged — it gets its own entry, flagged", () => {