From 69f89ab49be842d9826fb0b1621cc8c8dea5f14c Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Wed, 24 Jun 2026 03:26:11 +0900 Subject: workspace: conversation.open/statusChanged carry workspaceId (1405 vitest) - @dispatch/transport-contract 0.18.0 -> 0.19.0: add workspaceId: string to ConversationOpenMessage and ConversationStatusChangedMessage - session-orchestrator: include persisted workspaceId in conversationOpened/ conversationStatusChanged payloads - transport-ws: forward workspaceId in WS broadcasts - transport-http: POST /conversations/:id/open resolves workspaceId before emit - FE handoff to 29ae: frontend-workspace-open-handoff.md --- frontend-workspace-open-handoff.md | 47 +++++++ notes/assumptions-log.md | 21 +++ notes/pending-issues.md | 85 ++++++++++++ .../session-orchestrator/src/orchestrator.test.ts | 153 ++++++++++++++++++--- packages/session-orchestrator/src/orchestrator.ts | 45 +++++- packages/transport-contract/package.json | 2 +- packages/transport-contract/src/index.ts | 12 ++ packages/transport-http/src/app.test.ts | 12 +- packages/transport-http/src/app.ts | 11 +- packages/transport-ws/src/extension.ts | 23 +++- packages/transport-ws/src/server.bun.test.ts | 118 ++++++++++++++-- tasks.md | 23 +++- 12 files changed, 505 insertions(+), 47 deletions(-) create mode 100644 frontend-workspace-open-handoff.md create mode 100644 notes/assumptions-log.md create mode 100644 notes/pending-issues.md diff --git a/frontend-workspace-open-handoff.md b/frontend-workspace-open-handoff.md new file mode 100644 index 0000000..8005a2f --- /dev/null +++ b/frontend-workspace-open-handoff.md @@ -0,0 +1,47 @@ +# Frontend handoff — workspace id on conversation.open / statusChanged + +## What changed + +The backend now resolves the conversation's actual persisted workspace id and +includes it on the WS broadcast for both `conversation.open` and +`conversation.statusChanged`. + +- `@dispatch/transport-contract` `0.18.0 → 0.19.0` — additive `workspaceId: string` + on both `ConversationOpenMessage` and `ConversationStatusChangedMessage`. +- The backend uses the conversation's stored workspace (`"default"` fallback), + not the per-turn start option. + +## What the frontend must do + +1. **Re-pin the `file:` dep** on `@dispatch/transport-contract` from the backend + repo once this commit lands. +2. **Re-mirror `.dispatch/transport-contract.reference.md`** to match the `0.19.0` + contract. +3. **Parser update** (`src/adapters/ws/logic.ts:116-123`): parse `workspaceId` + from the incoming `conversation.open` and `conversation.statusChanged` messages. +4. **`openConversation()`** (`src/app/store.svelte.ts:588-600`): use the message's + `workspaceId` to stamp/focus the tab instead of `activeWorkspaceId` (the + viewer's current workspace). This fixes the bug where a tab opened via the + CLI `--open --workspace my-ws` was appearing in every workspace. +5. **`onConversationStatusChanged()`** (`src/app/store.svelte.ts:703-718`): same + fix when the FE calls `openConversation(conversationId)` on a status change and + has no existing tab — use the `workspaceId` from the message. +6. **Tests** (`logic.test.ts`, `index.test.ts`, `App.test.ts`, `conformance.test.ts`): + update fixtures/assertions to carry `workspaceId`. + +## Backend status + +- `@dispatch/transport-contract`: `0.19.0` with additive `workspaceId`. +- `session-orchestrator`: payload types widened; status-change emits resolve + workspace id from store. +- `transport-ws`: broadcasts include `workspaceId`. +- `transport-http`: `POST /conversations/:id/open` resolves workspace id and emits + it. +- Verified: `tsc -b` EXIT 0, biome clean, **1405 vitest** green. + +## Note on assumptions + +I'm working autonomously while the user is away. Every assumption I make is +recorded in `notes/assumptions-log.md` in this repo. Please record any +assumptions you make while implementing this handoff in your own assumptions log +so we can raise them when the user returns. diff --git a/notes/assumptions-log.md b/notes/assumptions-log.md new file mode 100644 index 0000000..a04467b --- /dev/null +++ b/notes/assumptions-log.md @@ -0,0 +1,21 @@ +# Assumptions Log + +> Single place where every assumption made while the user is away is recorded. +> These will be raised when the user returns; nothing here is final. + +## Task 1 — Workspace tab issue (conversation.open drops workspaceId) + +1. **workspaceId is always available.** `ConversationStore.getWorkspaceId(id)` returns `"default"` for unknown/unassigned conversations, so there is no `null` case to handle on the `ConversationOpenMessage`/`ConversationStatusChangedMessage` contract — a plain `string` field is sufficient. No migration or legacy row edge case changes the "default" fallback. +2. **No transport-contract version-bump type.** Because there is no external load-time semver machinery yet (restructure-plan §2.9: "the type system IS the version check" for internal consumers), I will bump `@dispatch/transport-contract` with a **minor (additive)** version increase. The exact number is for changelog communications, not mechanics; my target is `0.19.0` based on the current `0.18.0` plus an additive surface change. +3. **No conversation-store change required.** The store surface already exports `getWorkspaceId`; we can look it up from transport-http / session-orchestrator as needed. +4. **Both `conversation.open` and `conversation.statusChanged` get the same additive `workspaceId: string` field**, and both broadcast paths are fixed in this task. The handoff says "same bug on conversation.statusChanged path." +5. **Frontend tests are the frontend's problem.** I will send a handoff with the BE contract changes and the exact `29ae` deliverables; I do not edit the frontend repo. + + +## Task 2 — System context builder not loading referenced files + +*Assumptions will be appended here as the work progresses.* + +## Task 3 — Persistent provider + model selection per chat + +*Assumptions will be appended here as the work progresses.* diff --git a/notes/pending-issues.md b/notes/pending-issues.md new file mode 100644 index 0000000..7dc1022 --- /dev/null +++ b/notes/pending-issues.md @@ -0,0 +1,85 @@ +# Pending Issues — upcoming work + +> Three issues recorded for future waves. Each is marked DONE here as it ships. +> Do NOT start any of these until the prerequisites noted per-item are met. + +--- + +## 1. Workspace tab issue + +**Status:** PENDING — awaiting a handoff file from the user before starting. + +> A handoff file will be provided when we are ready to begin. Do not plan or +> summon until it arrives — the scope + root cause are expected to come from it. + +--- + +## 2. System context builder — referenced files not loaded + +**Status:** PENDING. + +**Symptom:** The system context builder does not load referenced files that live +in the conversation's working directory. Concrete reproduction: the orchestrator +is in a workspace pointing at `/home/tradam/projects/dispatch` and the +conversation's working directory (cwd) is `arch-rewrite`, which contains an +`AGENTS.md` file at its root — yet that file was NOT loaded into the system +context. The context builder appears to miss files it should be discovering and +injecting from the cwd. + +**Scope (initial hypothesis, to confirm on investigation):** +- Which unit owns the system-context / system-prompt assembly (likely + `session-orchestrator` context-assembly filter chain, §3.2 of the restructure + plan; see `notes/system-prompt-design.md`). +- How referenced files are discovered (cwd-aware?) and whether the discovery is + wired through the per-conversation cwd at all. +- Whether this is a discovery gap (files not found) or an injection gap (found + but not threaded into the prompt). + +**Notes:** +- This is a live-found bug — reproduce against the dev stack before assuming a + fix is correct. +- The cwd resolution pipeline was recently hardened (workspace defaultCwd + fallthrough, relative resolution, per-turn override) — check whether the + context builder uses `getEffectiveCwd` or a separate/older code path. + +--- + +## 3. Persistent provider + model selection per chat + +**Status:** PENDING. + +**Symptom:** A chat's selected provider + model is NOT persisted per +conversation. Opening the same chat in a new browser session defaults to a +single hardcoded/default model + provider rather than recalling the one +originally chosen for that conversation. + +**Desired behavior:** The provider + model chosen for a conversation is +persisted (per-conversation, like `cwd` and `reasoningEffort` already are) and +restored when the conversation is reopened — including from a fresh browser +session. New conversations still default to the global default until the user +selects. + +**Scope (initial hypothesis, to confirm):** +- Storage: `conversation-store` likely needs a new per-conversation key space + (mirror the `getCwd/setCwd` and `getReasoningEffort/setReasoningEffort` + precedents) for the selected model name (`/` form per + GLOSSARY) — and possibly the credential/provider if model name alone is not + enough to re-resolve. +- Transport: `transport-http` + `transport-ws` need GET/PUT (or WS op) endpoints + for the selection, mirroring the cwd/reasoning-effort endpoints. +- Session: `session-orchestrator` resolves the per-turn model from the persisted + value when the request omits an explicit override (same precedence pattern as + reasoning effort: per-turn override → persisted → default). +- Wire/contract: `@dispatch/transport-contract` + possibly `@dispatch/wire` get + additive types (version bumps). +- FE courier: the web repo needs to send + restore the selection (couriered via + the user per ORCHESTRATOR §7). + +**Notes:** +- Glossary check first (§1 step 2 of the golden workflow): "model name" is the + canonical term (`/` form); "credential" binds a name to + a provider. Confirm whether we persist the model name alone (re-resolvable via + credential-store) or also the credential/provider. +- Boundary decision is the USER's (§1 step 3): is this one unit + (`conversation-store` + transport + orchestrator) or split? Surface before + planning. diff --git a/packages/session-orchestrator/src/orchestrator.test.ts b/packages/session-orchestrator/src/orchestrator.test.ts index 18a4a62..3b79bd5 100644 --- a/packages/session-orchestrator/src/orchestrator.test.ts +++ b/packages/session-orchestrator/src/orchestrator.test.ts @@ -19,6 +19,8 @@ import { runTurn } from "@dispatch/kernel"; import type { SystemPromptService } from "@dispatch/system-prompt"; import { describe, expect, it } from "vitest"; import { + type ConversationOpenedPayload, + type ConversationStatusChangedPayload, createCompactionService, createSessionOrchestrator, createWarmService, @@ -1228,23 +1230,48 @@ describe("lifecycle event hooks", () => { modelName: "mymodel", }); - expect(emitted).toHaveLength(4); - expect(emitted[0]?.hook).toBe("session-orchestrator/turn-started"); - expect(emitted[0]?.payload.conversationId).toBe("conv-lifecycle"); - expect(emitted[0]?.payload.cwd).toBe("/work"); - expect(emitted[0]?.payload.modelName).toBe("mymodel"); - expect(emitted[0]?.order).toBe(0); + // The status-changed emits resolve the persisted workspace id async + // (getWorkspaceId) before firing, so they may land after turn-settled + // in microtask order. Flush all pending microtasks so every emit has + // landed, then assert by hook identity rather than strict index order. + await new Promise((resolve) => setImmediate(resolve)); - expect(emitted[1]?.hook).toBe("session-orchestrator/conversation-status-changed"); - expect((emitted[1]?.payload as unknown as { status: string }).status).toBe("active"); + expect(emitted).toHaveLength(4); - expect(emitted[2]?.hook).toBe("session-orchestrator/turn-settled"); - expect(emitted[2]?.payload.conversationId).toBe("conv-lifecycle"); - expect(emitted[2]?.payload.cwd).toBe("/work"); - expect(emitted[2]?.payload.modelName).toBe("mymodel"); + const started = emitted.find((e) => e.hook === "session-orchestrator/turn-started"); + const settled = emitted.find((e) => e.hook === "session-orchestrator/turn-settled"); + const statusChanges = emitted.filter( + (e) => e.hook === "session-orchestrator/conversation-status-changed", + ); - expect(emitted[3]?.hook).toBe("session-orchestrator/conversation-status-changed"); - expect((emitted[3]?.payload as unknown as { status: string }).status).toBe("idle"); + expect(started).toBeDefined(); + expect(started?.payload.conversationId).toBe("conv-lifecycle"); + expect(started?.payload.cwd).toBe("/work"); + expect(started?.payload.modelName).toBe("mymodel"); + // turn-started is the FIRST emit (synchronous, before any async deferral). + expect(started?.order).toBe(0); + + expect(settled).toBeDefined(); + expect(settled?.payload.conversationId).toBe("conv-lifecycle"); + expect(settled?.payload.cwd).toBe("/work"); + expect(settled?.payload.modelName).toBe("mymodel"); + // turn-started precedes turn-settled. + expect(started?.order).toBeLessThan(settled?.order ?? Infinity); + + expect(statusChanges).toHaveLength(2); + const activeChange = statusChanges.find( + (e) => (e.payload as unknown as { status: string }).status === "active", + ); + const idleChange = statusChanges.find( + (e) => (e.payload as unknown as { status: string }).status === "idle", + ); + expect(activeChange).toBeDefined(); + expect(idleChange).toBeDefined(); + // Both status-changed payloads now carry the persisted workspace id. + expect((activeChange?.payload as unknown as { workspaceId: string }).workspaceId).toBe( + "default", + ); + expect((idleChange?.payload as unknown as { workspaceId: string }).workspaceId).toBe("default"); }); }); @@ -2402,7 +2429,7 @@ describe("closeConversation (CR-4c)", () => { expect(persisted[0]?.role).toBe("user"); }); - it("is idempotent on an idle/unknown conversation: abortedTurn false, hook still emitted", () => { + it("is idempotent on an idle/unknown conversation: abortedTurn false, hook still emitted", async () => { const store = createInMemoryStore(); const emitted: Array<{ hook: string; payload: unknown }> = []; const { orchestrator } = createSessionOrchestrator({ @@ -2418,22 +2445,110 @@ describe("closeConversation (CR-4c)", () => { const result = orchestrator.closeConversation("conv-never-seen"); expect(result.abortedTurn).toBe(false); + // The conversation-closed hook is emitted synchronously; the + // status-changed hook resolves the workspace id async before emitting. expect(emitted).toEqual([ { hook: "session-orchestrator/conversation-closed", payload: { conversationId: "conv-never-seen" }, }, - { - hook: "session-orchestrator/conversation-status-changed", - payload: { conversationId: "conv-never-seen", status: "closed" }, - }, ]); + // Flush the async getWorkspaceId resolution so the status-changed emit lands. + await new Promise((resolve) => setImmediate(resolve)); + expect(emitted).toContainEqual({ + hook: "session-orchestrator/conversation-status-changed", + payload: { conversationId: "conv-never-seen", status: "closed", workspaceId: "default" }, + }); // Closing again is still safe. expect(orchestrator.closeConversation("conv-never-seen").abortedTurn).toBe(false); }); }); +// --- workspace id on conversationOpened / conversationStatusChanged payloads --- + +describe("workspace id broadcast payloads", () => { + it("conversationStatusChanged payload carries the workspace id from the store", async () => { + const base = createInMemoryStore(); + // Pre-assign a non-default workspace so we can assert it's threaded + // through (not the per-turn start option, which differs). + await base.setWorkspaceId("conv-ws-broadcast", "team-workspace"); + + const emitted: Array<{ hook: string; payload: ConversationStatusChangedPayload }> = []; + const provider = createFakeProvider([ + [ + { type: "text-delta", delta: "ok" }, + { type: "finish", reason: "stop" }, + ], + ]); + + const { orchestrator } = createSessionOrchestrator({ + conversationStore: base, + resolveProvider: () => provider, + resolveTools: () => [], + applyToolsFilter: identityApplyToolsFilter, + runTurn, + emit: (hook, payload) => { + if (hook.id === "session-orchestrator/conversation-status-changed") { + emitted.push({ + hook: hook.id, + payload: payload as ConversationStatusChangedPayload, + }); + } + }, + }); + + // Pass a DIFFERENT per-turn workspaceId to prove the payload uses the + // persisted store value, not the start option. + await orchestrator.handleMessage({ + conversationId: "conv-ws-broadcast", + text: "hi", + onEvent: () => {}, + workspaceId: "should-not-appear", + }); + // Flush the async getWorkspaceId resolutions. + await new Promise((resolve) => setImmediate(resolve)); + + expect(emitted.length).toBeGreaterThanOrEqual(2); + for (const e of emitted) { + expect(e.payload.workspaceId).toBe("team-workspace"); + } + + // closeConversation also threads the persisted workspace id. + const closeEmitted: ConversationStatusChangedPayload[] = []; + const { orchestrator: orchestrator2 } = createSessionOrchestrator({ + conversationStore: base, + resolveProvider: () => provider, + resolveTools: () => [], + applyToolsFilter: identityApplyToolsFilter, + runTurn, + emit: (hook, payload) => { + if (hook.id === "session-orchestrator/conversation-status-changed") { + closeEmitted.push(payload as ConversationStatusChangedPayload); + } + }, + }); + orchestrator2.closeConversation("conv-ws-broadcast"); + await new Promise((resolve) => setImmediate(resolve)); + const closed = closeEmitted.find((p) => p.status === "closed"); + expect(closed).toBeDefined(); + expect(closed?.workspaceId).toBe("team-workspace"); + }); + + it("conversationOpened payload carries the workspace id (type-level construct)", () => { + // conversationOpened is emitted by a sibling transport unit, so this + // package only owns the payload TYPE. This regression test pins the + // type to require workspaceId (a missing field would fail to compile) + // and verifies the persisted value flows through at construction time. + const payload: ConversationOpenedPayload = { + conversationId: "conv-open", + workspaceId: "open-workspace", + }; + expect(payload.workspaceId).toBe("open-workspace"); + expect(payload.conversationId).toBe("conv-open"); + }); +}); + describe("reasoning effort resolution", () => { it("override wins over stored → provider receives the override level", async () => { const store = createInMemoryStore(); diff --git a/packages/session-orchestrator/src/orchestrator.ts b/packages/session-orchestrator/src/orchestrator.ts index 7599a0c..1403288 100644 --- a/packages/session-orchestrator/src/orchestrator.ts +++ b/packages/session-orchestrator/src/orchestrator.ts @@ -113,6 +113,12 @@ export const conversationClosed: EventHookDescriptor /** Payload for the conversationOpened bus event. */ export interface ConversationOpenedPayload { readonly conversationId: string; + /** + * The conversation's actual persisted workspace id (resolved from the + * store, not the per-turn start option), so a frontend can open/focus the + * tab in the correct workspace. Falls back to `"default"`. + */ + readonly workspaceId: string; } /** @@ -128,6 +134,12 @@ export const conversationOpened: EventHookDescriptor export interface ConversationStatusChangedPayload { readonly conversationId: string; readonly status: ConversationStatus; + /** + * The conversation's actual persisted workspace id (resolved from the + * store, not the per-turn start option), so a frontend can sync the tab + * in the correct workspace. Falls back to `"default"`. + */ + readonly workspaceId: string; } /** @@ -410,7 +422,15 @@ export function createSessionOrchestrator( payloadPromise.then((payload) => { deps.emit?.(turnStarted, payload); - deps.emit?.(conversationStatusChanged, { conversationId, status: "active" }); + // Resolve the persisted workspace id (not the per-turn start option) + // before emitting so the broadcast carries the correct workspace. + void deps.conversationStore.getWorkspaceId(conversationId).then((workspaceId) => { + deps.emit?.(conversationStatusChanged, { + conversationId, + status: "active", + workspaceId, + }); + }); void deps.conversationStore.setConversationStatus(conversationId, "active"); }); @@ -595,9 +615,14 @@ export function createSessionOrchestrator( void payloadPromise.then((payload) => { deps.emit?.(turnSettled, payload); if (!carried) { - deps.emit?.(conversationStatusChanged, { - conversationId, - status: "idle", + // Resolve the persisted workspace id before emitting so the + // broadcast carries the correct workspace. + void deps.conversationStore.getWorkspaceId(conversationId).then((workspaceId) => { + deps.emit?.(conversationStatusChanged, { + conversationId, + status: "idle", + workspaceId, + }); }); void deps.conversationStore.setConversationStatus(conversationId, "idle"); // Fire-and-forget auto-compaction: check threshold and @@ -691,7 +716,17 @@ export function createSessionOrchestrator( turn.controller.abort(); } deps.emit?.(conversationClosed, { conversationId }); - deps.emit?.(conversationStatusChanged, { conversationId, status: "closed" }); + // Resolve the persisted workspace id before emitting so the + // broadcast carries the correct workspace. The hook is + // fire-and-forget; closeConversation stays synchronous (returns + // immediately) while the status-changed emit resolves async. + void deps.conversationStore.getWorkspaceId(conversationId).then((workspaceId) => { + deps.emit?.(conversationStatusChanged, { + conversationId, + status: "closed", + workspaceId, + }); + }); void deps.conversationStore.setConversationStatus(conversationId, "closed"); return { abortedTurn }; }, diff --git a/packages/transport-contract/package.json b/packages/transport-contract/package.json index 542ed90..71baa43 100644 --- a/packages/transport-contract/package.json +++ b/packages/transport-contract/package.json @@ -1,6 +1,6 @@ { "name": "@dispatch/transport-contract", - "version": "0.18.0", + "version": "0.19.0", "type": "module", "private": true, "main": "dist/index.js", diff --git a/packages/transport-contract/src/index.ts b/packages/transport-contract/src/index.ts index 2a3bb9f..fcbd1b1 100644 --- a/packages/transport-contract/src/index.ts +++ b/packages/transport-contract/src/index.ts @@ -604,6 +604,12 @@ export type WsServerMessage = export interface ConversationOpenMessage { readonly type: "conversation.open"; readonly conversationId: string; + /** + * The conversation's actual workspace id, so a frontend can open/focus it + * in the correct workspace instead of stamping it with the viewer's current + * workspace. + */ + readonly workspaceId: string; } /** @@ -615,6 +621,12 @@ export interface ConversationStatusChangedMessage { readonly type: "conversation.statusChanged"; readonly conversationId: string; readonly status: ConversationStatus; + /** + * The conversation's actual workspace id, so a frontend can open/focus it + * in the correct workspace instead of stamping it with the viewer's current + * workspace. + */ + readonly workspaceId: string; } /** diff --git a/packages/transport-http/src/app.test.ts b/packages/transport-http/src/app.test.ts index 2a4b451..7887695 100644 --- a/packages/transport-http/src/app.test.ts +++ b/packages/transport-http/src/app.test.ts @@ -2814,8 +2814,13 @@ describe("POST /conversations/:id/open", () => { const emit: HostAPI["emit"] = (hook, payload) => { emitCalls.push({ hook, payload }); }; + // A store whose getWorkspaceId returns a non-default id, so the test + // proves the handler resolves and forwards the PERSISTED workspace id + // (not a hard-coded "default"). + const store = createFakeConversationStore(); + store.getWorkspaceId = async () => "open-workspace"; const app = createApp({ - conversationStore: createFakeConversationStore(), + conversationStore: store, orchestrator: createFakeOrchestrator([]), credentialStore: createFakeCredentialStore([]), emit, @@ -2825,7 +2830,10 @@ describe("POST /conversations/:id/open", () => { expect(res.status).toBe(200); expect(emitCalls).toHaveLength(1); expect(emitCalls[0]?.hook).toBe(conversationOpened); - expect(emitCalls[0]?.payload).toEqual({ conversationId: "conv1" }); + expect(emitCalls[0]?.payload).toEqual({ + conversationId: "conv1", + workspaceId: "open-workspace", + }); }); it("returns 500 when emit is absent", async () => { diff --git a/packages/transport-http/src/app.ts b/packages/transport-http/src/app.ts index 41b583c..bc8b9de 100644 --- a/packages/transport-http/src/app.ts +++ b/packages/transport-http/src/app.ts @@ -767,7 +767,7 @@ export function createApp(opts: CreateServerOptions): Hono { return c.json(body, 200); }); - app.post("/conversations/:id/open", (c) => { + app.post("/conversations/:id/open", async (c) => { const conversationId = c.req.param("id"); if (opts.emit === undefined) { log.warn("conversations: open requested but emit is not available", { @@ -775,8 +775,13 @@ export function createApp(opts: CreateServerOptions): Hono { }); return c.json({ error: "not available" }, 500); } - opts.emit(conversationOpened, { conversationId }); - log.info("conversations: opened", { conversationId }); + // Resolve the conversation's persisted workspace id so the frontend can + // open/focus the tab in the correct workspace. The store falls back to + // `"default"` when no workspaceId is persisted (or the conversation is + // unknown), so this never throws for a missing conversation. + const workspaceId = await opts.conversationStore.getWorkspaceId(conversationId); + opts.emit(conversationOpened, { conversationId, workspaceId }); + log.info("conversations: opened", { conversationId, workspaceId }); const body: OpenConversationResponse = { conversationId }; return c.json(body, 200); }); diff --git a/packages/transport-ws/src/extension.ts b/packages/transport-ws/src/extension.ts index 899dabb..1e3da27 100644 --- a/packages/transport-ws/src/extension.ts +++ b/packages/transport-ws/src/extension.ts @@ -133,18 +133,29 @@ export function createTransportWsExtension(): Extension { // whenever the orchestrator signals a conversation was opened (e.g. the // CLI `--open` flag). The frontend decides whether to open/focus a tab — // the backend just signals. This is a GLOBAL fan-out (like the catalog), - // NOT a per-conversation chat broadcast. + // NOT a per-conversation chat broadcast. The payload's `workspaceId` + // is the conversation's actual persisted workspace (resolved by the + // orchestrator from the store), so a frontend opens/focuses the tab in + // the correct workspace. disposers.push( - host.on(conversationOpened, ({ conversationId }) => { - broadcast({ type: "conversation.open", conversationId }); + host.on(conversationOpened, ({ conversationId, workspaceId }) => { + broadcast({ type: "conversation.open", conversationId, workspaceId }); }), ); // Broadcast `conversation.statusChanged` to all connected clients so - // tabs sync across devices in real time. + // tabs sync across devices in real time. `workspaceId` is the + // conversation's actual persisted workspace (resolved by the + // orchestrator from the store), forwarded so a frontend syncs the tab + // in the correct workspace. disposers.push( - host.on(conversationStatusChanged, ({ conversationId, status }) => { - broadcast({ type: "conversation.statusChanged", conversationId, status }); + host.on(conversationStatusChanged, ({ conversationId, status, workspaceId }) => { + broadcast({ + type: "conversation.statusChanged", + conversationId, + status, + workspaceId, + }); }), ); diff --git a/packages/transport-ws/src/server.bun.test.ts b/packages/transport-ws/src/server.bun.test.ts index 3a1bf03..e24aa6b 100644 --- a/packages/transport-ws/src/server.bun.test.ts +++ b/packages/transport-ws/src/server.bun.test.ts @@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import type { AgentEvent, Attributes, ErrorAttributes, Logger } from "@dispatch/kernel"; import type { SessionOrchestrator, TurnEventListener } from "@dispatch/session-orchestrator"; import type { SurfaceContext, SurfaceProvider, SurfaceRegistry } from "@dispatch/surface-registry"; -import type { WsServerMessage } from "@dispatch/transport-contract"; +import type { ConversationStatus, WsServerMessage } from "@dispatch/transport-contract"; import type { SurfaceCatalogEntry, SurfaceClientMessage, SurfaceSpec } from "@dispatch/ui-contract"; import { catalogMessage, routeClientMessage, subKey } from "./router.js"; @@ -445,11 +445,30 @@ function startServer( /** * Simulate the `conversationOpened` hook firing — mirrors the * `host.on(conversationOpened, ...)` subscription in extension.ts, which - * broadcasts a `conversation.open` WS message to every connected client. + * broadcasts a `conversation.open` WS message (carrying the conversation's + * persisted `workspaceId`) to every connected client. */ return Object.assign(server, { - triggerConversationOpen(conversationId: string): void { - broadcast({ type: "conversation.open", conversationId }); + triggerConversationOpen(conversationId: string, workspaceId: string): void { + broadcast({ type: "conversation.open", conversationId, workspaceId }); + }, + /** + * Simulate the `conversationStatusChanged` hook firing — mirrors the + * `host.on(conversationStatusChanged, ...)` subscription in extension.ts, + * which broadcasts a `conversation.statusChanged` WS message (carrying the + * conversation's persisted `workspaceId`) to every connected client. + */ + triggerConversationStatusChanged( + conversationId: string, + status: ConversationStatus, + workspaceId: string, + ): void { + broadcast({ + type: "conversation.statusChanged", + conversationId, + status, + workspaceId, + }); }, }); } @@ -1079,10 +1098,14 @@ describe("conversation.open broadcast (conversationOpened hook)", () => { // Simulate the conversationOpened hook firing (extension.ts's // `host.on(conversationOpened, ...)` handler runs and broadcasts). - server.triggerConversationOpen("conv-42"); + server.triggerConversationOpen("conv-42", "ws-7"); const msg = await waitForMessage(ws); - expect(msg).toEqual({ type: "conversation.open", conversationId: "conv-42" }); + expect(msg).toEqual({ + type: "conversation.open", + conversationId: "conv-42", + workspaceId: "ws-7", + }); ws.close(); }); @@ -1099,12 +1122,87 @@ describe("conversation.open broadcast (conversationOpened hook)", () => { await waitForMessage(ws2); // drain catalog // Global fan-out: BOTH connected clients receive the broadcast, - // regardless of any per-conversation subscription state. - server.triggerConversationOpen("shared-conv"); + // regardless of any per-conversation subscription state. The forwarded + // `workspaceId` is identical on both. + server.triggerConversationOpen("shared-conv", "ws-shared"); + + const [msg1, msg2] = await Promise.all([waitForMessage(ws1), waitForMessage(ws2)]); + expect(msg1).toEqual({ + type: "conversation.open", + conversationId: "shared-conv", + workspaceId: "ws-shared", + }); + expect(msg2).toEqual({ + type: "conversation.open", + conversationId: "shared-conv", + workspaceId: "ws-shared", + }); + + ws1.close(); + ws2.close(); + }); +}); + +describe("conversation.statusChanged broadcast (conversationStatusChanged hook)", () => { + let server: ReturnType; + let port: number; + + afterEach(() => { + server.stop(); + }); + + test("conversation.statusChanged broadcast forwards workspaceId", async () => { + const orch = fakeOrchestrator(); + const registry = fakeRegistry([fakeProvider("demo", "Demo Surface")]); + server = startServer(registry, orch); + port = server.port as number; + + const ws = new WebSocket(`ws://localhost:${port}`); + await waitForMessage(ws); // drain catalog + + // Simulate the conversationStatusChanged hook firing (extension.ts's + // `host.on(conversationStatusChanged, ...)` handler runs and broadcasts). + server.triggerConversationStatusChanged("conv-9", "active", "ws-9"); + + const msg = await waitForMessage(ws); + expect(msg).toEqual({ + type: "conversation.statusChanged", + conversationId: "conv-9", + status: "active", + workspaceId: "ws-9", + }); + + ws.close(); + }); + + test("conversation.statusChanged sent to all connected clients with the same workspaceId", async () => { + const orch = fakeOrchestrator(); + const registry = fakeRegistry([fakeProvider("demo", "Demo Surface")]); + server = startServer(registry, orch); + port = server.port as number; + + const ws1 = new WebSocket(`ws://localhost:${port}`); + await waitForMessage(ws1); // drain catalog + const ws2 = new WebSocket(`ws://localhost:${port}`); + await waitForMessage(ws2); // drain catalog + + // Global fan-out: BOTH connected clients receive the broadcast with the + // conversation's persisted workspaceId forwarded unchanged. + server.triggerConversationStatusChanged("shared-conv", "idle", "ws-shared"); const [msg1, msg2] = await Promise.all([waitForMessage(ws1), waitForMessage(ws2)]); - expect(msg1).toEqual({ type: "conversation.open", conversationId: "shared-conv" }); - expect(msg2).toEqual({ type: "conversation.open", conversationId: "shared-conv" }); + expect(msg1).toEqual({ + type: "conversation.statusChanged", + conversationId: "shared-conv", + status: "idle", + workspaceId: "ws-shared", + }); + expect(msg2).toEqual({ + type: "conversation.statusChanged", + conversationId: "shared-conv", + status: "idle", + workspaceId: "ws-shared", + }); ws1.close(); ws2.close(); diff --git a/tasks.md b/tasks.md index 911b018..1edba70 100644 --- a/tasks.md +++ b/tasks.md @@ -5,7 +5,28 @@ > Keep this lean and current; do not let it re-accrete a step-by-step changelog. ## Status (current) -`tsc -b` EXIT 0 · biome clean · **1396 vitest** green. +`tsc -b` EXIT 0 · biome clean · **1405 vitest** green. + +## Workspace tab issue — conversation.open drops workspaceId (DONE) +Cross-repo additive fix: `conversation.open` / `conversation.statusChanged` WS +broadcasts now carry the conversation's persisted workspace id, so a frontend +opens/focuses a tab in the correct workspace instead of the viewer's current +workspace (`activeWorkspaceId`). CLI `dispatch --open --workspace my-ws` +now opens only in `my-ws`. +- **Wave 0 (orchestrator, contracts):** `@dispatch/transport-contract` + `0.18.0→0.19.0` — additive `readonly workspaceId: string` on + `ConversationOpenMessage` and `ConversationStatusChangedMessage`. +- **Wave 1 (parallel):** `session-orchestrator` (add `workspaceId` to + `ConversationOpenedPayload`/`ConversationStatusChangedPayload`; resolve from + `conversationStore.getWorkspaceId` at all status-change emit sites) + + `transport-ws` (thread `workspaceId` from hook payload into WS broadcasts) — + disjoint packages. +- **Wave 2:** `transport-http` — `POST /conversations/:id/open` now awaits + `getWorkspaceId(conversationId)` and emits `conversationOpened` with it. +- [x] Verified: `tsc -b` EXIT 0, biome clean, **1405 vitest** green; all agents in-lane. +- [x] **FE courier** to `29ae`: `frontend-workspace-open-handoff.md` — parse/use + `workspaceId` from `conversation.open` and `conversation.statusChanged`; + re-pin `@dispatch/transport-contract` `0.19.0`; re-mirror reference.md. ## LSP cwd resolution — server-default fallthrough + workspace assignment (DONE) Bug: `GET /conversations/:id/lsp` called `getEffectiveCwd` directly, which falls through -- cgit v1.2.3