diff options
| author | Adam Malczewski <[email protected]> | 2026-06-23 21:07:53 +0900 |
|---|---|---|
| committer | Adam Malczewski <[email protected]> | 2026-06-23 21:07:53 +0900 |
| commit | 4158e699e3c8ff556684fe2fc7a39ffab040623e (patch) | |
| tree | 1ca7b3069f6b1cfcb119c4b1fae614baf20f6e69 /packages | |
| parent | 961f564f6de6c6c63970b0aeaa48f730cc028d92 (diff) | |
| download | dispatch-4158e699e3c8ff556684fe2fc7a39ffab040623e.tar.gz dispatch-4158e699e3c8ff556684fe2fc7a39ffab040623e.zip | |
fix(lsp): gate LSP endpoint on persisted cwd; accept workspaceId on PUT cwd
GET /conversations/:id/lsp was calling getEffectiveCwd directly, which falls
through to serverDefaultCwd (process.cwd()) when no conversation cwd is set.
Now gates on getCwd first: returns {cwd:null, servers:[]} when no cwd
persisted; only resolves via getEffectiveCwd + calls lspService.status when
a persisted cwd exists.
PUT /conversations/:id/cwd now accepts optional workspaceId — validates with
isValidWorkspaceSlug, then ensureWorkspace → setWorkspaceId → setCwd (assigns
the workspace before persisting cwd, so getEffectiveCwd resolves relative
cwds against the workspace defaultCwd, not the server default).
transport-contract 0.16.0→0.17.0 (additive SetCwdRequest.workspaceId;
LspStatusResponse.cwd comment updated).
1332 vitest pass.
Diffstat (limited to 'packages')
| -rw-r--r-- | packages/transport-contract/package.json | 2 | ||||
| -rw-r--r-- | packages/transport-contract/src/index.ts | 27 | ||||
| -rw-r--r-- | packages/transport-http/src/app.test.ts | 278 | ||||
| -rw-r--r-- | packages/transport-http/src/app.ts | 107 |
4 files changed, 389 insertions, 25 deletions
diff --git a/packages/transport-contract/package.json b/packages/transport-contract/package.json index 1923e9f..95e1aef 100644 --- a/packages/transport-contract/package.json +++ b/packages/transport-contract/package.json @@ -1,6 +1,6 @@ { "name": "@dispatch/transport-contract", - "version": "0.16.0", + "version": "0.17.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 583f986..b195171 100644 --- a/packages/transport-contract/src/index.ts +++ b/packages/transport-contract/src/index.ts @@ -181,6 +181,14 @@ export interface ConversationMetricsResponse { readonly turns: readonly TurnMetrics[]; } +export interface ConversationStatusResponse { + readonly conversationId: string; + /** True if the orchestrator has an in-memory active turn for this conversation. */ + readonly isActive: boolean; + /** The persisted lifecycle status from the conversation store. */ + readonly status: ConversationStatus; +} + /** The aggregation window for `GET /metrics/throughput`. */ export type ThroughputPeriod = "day" | "week" | "month"; @@ -232,9 +240,19 @@ export interface CwdResponse { readonly cwd: string | null; } -/** Body of `PUT /conversations/:id/cwd`. */ +/** + * Body of `PUT /conversations/:id/cwd`. + * + * When `workspaceId` is provided, the conversation is assigned to that + * workspace BEFORE the cwd is persisted — so a subsequent + * `GET /conversations/:id/lsp` resolves a relative cwd against the + * workspace's `defaultCwd` (not the server default). Omit for unchanged + * workspace assignment (the conversation keeps its current workspace, or + * `"default"` if none). + */ export interface SetCwdRequest { readonly cwd: string; + readonly workspaceId?: string; } // ─── Per-conversation reasoning effort ──────────────────────────────────────── @@ -345,7 +363,12 @@ export interface LspServerInfo { /** Response of `GET /conversations/:id/lsp`. */ export interface LspStatusResponse { readonly conversationId: string; - /** The conversation's persisted cwd, or null if unset (then `servers` is empty). */ + /** + * The resolved working directory the LSP connects on, or `null` when no + * cwd has been set for the conversation (then `servers` is empty). When + * non-null, this is the effective cwd — a relative persisted cwd resolved + * against the conversation's workspace `defaultCwd`. + */ readonly cwd: string | null; /** The language servers configured for `cwd` and their live state. */ readonly servers: readonly LspServerInfo[]; diff --git a/packages/transport-http/src/app.test.ts b/packages/transport-http/src/app.test.ts index 1390199..0b840db 100644 --- a/packages/transport-http/src/app.test.ts +++ b/packages/transport-http/src/app.test.ts @@ -125,6 +125,9 @@ function createFakeConversationStore( async setCwd(conversationId, cwd) { cwdStore.set(conversationId, cwd); }, + async clearCwd(conversationId) { + cwdStore.delete(conversationId); + }, async getReasoningEffort(conversationId) { return reasoningEffortStore.get(conversationId) ?? null; }, @@ -177,6 +180,35 @@ function createFakeConversationStore( }; } +/** + * Wraps a ConversationStore to record the ORDER of mutating calls + * (ensureWorkspace, setWorkspaceId, setCwd) as a labeled list — so tests can + * assert that workspace assignment happens BEFORE setCwd. + */ +function createCallTrackingStore( + base: ConversationStore, +): ConversationStore & { readonly calls: readonly string[] } { + const calls: string[] = []; + return { + ...base, + get calls() { + return calls; + }, + async ensureWorkspace(id, opts) { + calls.push(`ensureWorkspace:${id}`); + return base.ensureWorkspace(id, opts); + }, + async setWorkspaceId(conversationId, workspaceId) { + calls.push(`setWorkspaceId:${workspaceId}`); + await base.setWorkspaceId(conversationId, workspaceId); + }, + async setCwd(conversationId, cwd) { + calls.push(`setCwd:${cwd}`); + await base.setCwd(conversationId, cwd); + }, + }; +} + function createFakeOrchestrator(events: AgentEvent[]): SessionOrchestrator { return { startTurn() { @@ -327,6 +359,28 @@ function createFakeLspService( }; } +function createCapturingLspService( + statuses: readonly { + readonly id: string; + readonly name: string; + readonly root: string; + readonly extensions: readonly string[]; + readonly state: "connected" | "starting" | "error" | "not-started"; + readonly error?: string; + }[] = [], +): LspService & { readonly statusCalls: readonly string[] } { + const calls: string[] = []; + return { + get statusCalls() { + return calls; + }, + async status(cwd) { + calls.push(cwd); + return statuses; + }, + }; +} + const noopLogger = createFakeLogger(); describe("GET /health", () => { @@ -895,6 +949,7 @@ describe("GET /conversations/:id", () => { return null; }, async setCwd() {}, + async clearCwd() {}, async getReasoningEffort() { return null; }, @@ -1013,6 +1068,7 @@ describe("GET /conversations/:id", () => { return null; }, async setCwd() {}, + async clearCwd() {}, async getReasoningEffort() { return null; }, @@ -1200,6 +1256,7 @@ describe("GET /conversations/:id/metrics", () => { return null; }, async setCwd() {}, + async clearCwd() {}, async getReasoningEffort() { return null; }, @@ -1888,6 +1945,78 @@ describe("PUT then GET /conversations/:id/cwd", () => { }); }); +describe("DELETE /conversations/:id/cwd", () => { + it("after a PUT cwd → returns { cwd: null } and a subsequent GET returns cwd: null", async () => { + const store = createFakeConversationStore(); + const app = createApp({ + conversationStore: store, + orchestrator: createFakeOrchestrator([]), + credentialStore: createFakeCredentialStore([]), + logger: noopLogger, + }); + + const putRes = await app.request("/conversations/conv1/cwd", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ cwd: "/home/user/project" }), + }); + expect(putRes.status).toBe(200); + + const deleteRes = await app.request("/conversations/conv1/cwd", { method: "DELETE" }); + expect(deleteRes.status).toBe(200); + const deleteBody = (await deleteRes.json()) as { conversationId: string; cwd: string | null }; + expect(deleteBody.conversationId).toBe("conv1"); + expect(deleteBody.cwd).toBeNull(); + + const getRes = await app.request("/conversations/conv1/cwd"); + expect(getRes.status).toBe(200); + const getBody = (await getRes.json()) as { conversationId: string; cwd: string | null }; + expect(getBody.cwd).toBeNull(); + }); + + it("on a conversation that never had a cwd set → returns { cwd: null }, no error (idempotent)", async () => { + const app = createApp({ + conversationStore: createFakeConversationStore(), + orchestrator: createFakeOrchestrator([]), + credentialStore: createFakeCredentialStore([]), + logger: noopLogger, + }); + + const deleteRes = await app.request("/conversations/conv1/cwd", { method: "DELETE" }); + expect(deleteRes.status).toBe(200); + const deleteBody = (await deleteRes.json()) as { conversationId: string; cwd: string | null }; + expect(deleteBody.conversationId).toBe("conv1"); + expect(deleteBody.cwd).toBeNull(); + }); + + it("does NOT affect other conversations' cwds (isolation)", async () => { + const cwdStore = new Map<string, string>([ + ["conv1", "/home/user/project"], + ["conv2", "/other/path"], + ]); + const store = createFakeConversationStore(new Map(), new Map(), cwdStore); + const app = createApp({ + conversationStore: store, + orchestrator: createFakeOrchestrator([]), + credentialStore: createFakeCredentialStore([]), + logger: noopLogger, + }); + + const deleteRes = await app.request("/conversations/conv1/cwd", { method: "DELETE" }); + expect(deleteRes.status).toBe(200); + + const get1Res = await app.request("/conversations/conv1/cwd"); + expect(get1Res.status).toBe(200); + const get1Body = (await get1Res.json()) as { conversationId: string; cwd: string | null }; + expect(get1Body.cwd).toBeNull(); + + const get2Res = await app.request("/conversations/conv2/cwd"); + expect(get2Res.status).toBe(200); + const get2Body = (await get2Res.json()) as { conversationId: string; cwd: string | null }; + expect(get2Body.cwd).toBe("/other/path"); + }); +}); + describe("PUT /conversations/:id/cwd", () => { it("with missing cwd returns 400", async () => { const app = createApp({ @@ -1920,6 +2049,80 @@ describe("PUT /conversations/:id/cwd", () => { }); expect(res.status).toBe(400); }); + + it("PUT cwd with workspaceId: assigns workspace before setCwd", async () => { + const base = createFakeConversationStore(); + const store = createCallTrackingStore(base); + const app = createApp({ + conversationStore: store, + orchestrator: createFakeOrchestrator([]), + credentialStore: createFakeCredentialStore([]), + logger: noopLogger, + }); + const res = await app.request("/conversations/conv1/cwd", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ cwd: "/home/user/project", workspaceId: "my-team" }), + }); + expect(res.status).toBe(200); + const body = (await res.json()) as { conversationId: string; cwd: string }; + expect(body.cwd).toBe("/home/user/project"); + // ensureWorkspace + setWorkspaceId called (in that order) BEFORE setCwd. + expect(store.calls).toEqual([ + "ensureWorkspace:my-team", + "setWorkspaceId:my-team", + "setCwd:/home/user/project", + ]); + }); + + it("PUT cwd without workspaceId: only setCwd", async () => { + const base = createFakeConversationStore(); + const store = createCallTrackingStore(base); + const app = createApp({ + conversationStore: store, + orchestrator: createFakeOrchestrator([]), + credentialStore: createFakeCredentialStore([]), + logger: noopLogger, + }); + const res = await app.request("/conversations/conv1/cwd", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ cwd: "/home/user/project" }), + }); + expect(res.status).toBe(200); + // ensureWorkspace and setWorkspaceId NOT called. + expect(store.calls).toEqual(["setCwd:/home/user/project"]); + }); + + it("PUT cwd with invalid workspaceId: returns 400", async () => { + const base = createFakeConversationStore(); + const store = createCallTrackingStore(base); + const app = createApp({ + conversationStore: store, + orchestrator: createFakeOrchestrator([]), + credentialStore: createFakeCredentialStore([]), + logger: noopLogger, + }); + // Uppercase is not a valid workspace slug. + const res = await app.request("/conversations/conv1/cwd", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ cwd: "/home/user/project", workspaceId: "UPPER" }), + }); + expect(res.status).toBe(400); + const body = (await res.json()) as { error: string }; + expect(body.error).toBe("Invalid workspaceId"); + // No mutating calls should have been made. + expect(store.calls).toEqual([]); + + // Empty string is also invalid. + const res2 = await app.request("/conversations/conv1/cwd", { + method: "PUT", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ cwd: "/home/user/project", workspaceId: "" }), + }); + expect(res2.status).toBe(400); + }); }); describe("GET /conversations/:id/lsp", () => { @@ -1994,6 +2197,79 @@ describe("GET /conversations/:id/lsp", () => { expect(body.servers[1]?.state).toBe("error"); expect(body.servers[1]?.error).toBe("spawn failed"); }); + + it("LSP: returns null+empty when no persisted cwd — lspService.status NOT called", async () => { + const cwdStore = new Map<string, string>(); // no persisted cwd + const store = createFakeConversationStore(new Map(), new Map(), cwdStore); + const lsp = createCapturingLspService([ + { + id: "typescript", + name: "TypeScript", + root: "/irrelevant", + extensions: [".ts"], + state: "connected" as const, + }, + ]); + const app = createApp({ + conversationStore: store, + orchestrator: createFakeOrchestrator([]), + credentialStore: createFakeCredentialStore([]), + lspService: lsp, + logger: noopLogger, + }); + const res = await app.request("/conversations/conv1/lsp"); + expect(res.status).toBe(200); + const body = (await res.json()) as { + conversationId: string; + cwd: string | null; + servers: readonly unknown[]; + }; + expect(body.conversationId).toBe("conv1"); + expect(body.cwd).toBeNull(); + expect(body.servers).toEqual([]); + expect(lsp.statusCalls).toEqual([]); // status NOT called + }); + + it("LSP: uses effectiveCwd when persisted cwd is set — status called with resolved cwd", async () => { + // Persisted (relative) cwd differs from the resolved effective cwd. + const cwdStore = new Map<string, string>([["conv1", "subdir"]]); + const store = createFakeConversationStore(new Map(), new Map(), cwdStore); + // Override getEffectiveCwd to return the resolved (absolute) value. + const resolvedStore: ConversationStore = { + ...store, + async getEffectiveCwd() { + return "/workspace/subdir"; + }, + }; + const lsp = createCapturingLspService([ + { + id: "typescript", + name: "TypeScript", + root: "/workspace/subdir", + extensions: [".ts", ".tsx"], + state: "connected" as const, + }, + ]); + const app = createApp({ + conversationStore: resolvedStore, + orchestrator: createFakeOrchestrator([]), + credentialStore: createFakeCredentialStore([]), + lspService: lsp, + logger: noopLogger, + }); + const res = await app.request("/conversations/conv1/lsp"); + expect(res.status).toBe(200); + const body = (await res.json()) as { + conversationId: string; + cwd: string | null; + servers: readonly { readonly id: string }[]; + }; + expect(body.conversationId).toBe("conv1"); + expect(body.cwd).toBe("/workspace/subdir"); // effective, not persisted + expect(lsp.statusCalls).toEqual(["/workspace/subdir"]); + expect(body.servers).toHaveLength(1); + expect(body.servers[0]?.id).toBe("typescript"); + }); }); describe("POST /chat reasoningEffort", () => { @@ -2964,7 +3240,7 @@ it("GET /conversations/:id/lsp uses effective cwd", async () => { const res = await app.request("/conversations/conv1/lsp"); expect(res.status).toBe(200); expect(effectiveCwdCalled).toBe(true); - expect(getCwdCalled).toBe(false); + expect(getCwdCalled).toBe(true); // gated on persisted cwd first expect(lspCwd).toBe("/effective"); const body = (await res.json()) as { conversationId: string; diff --git a/packages/transport-http/src/app.ts b/packages/transport-http/src/app.ts index b4ab513..eba5c1a 100644 --- a/packages/transport-http/src/app.ts +++ b/packages/transport-http/src/app.ts @@ -6,6 +6,7 @@ import type { ConversationHistoryResponse, ConversationListResponse, ConversationMetricsResponse, + ConversationStatusResponse, CwdResponse, DeleteWorkspaceResponse, LastMessageResponse, @@ -234,6 +235,17 @@ export function createApp(opts: CreateServerOptions): Hono { } }); + app.get("/conversations/:id/status", async (c) => { + const conversationId = c.req.param("id"); + const isActive = opts.orchestrator.isActive(conversationId); + const status = await opts.conversationStore.getConversationStatus(conversationId); + if (status === null) { + return c.json({ error: "Conversation not found" }, 404); + } + const body: ConversationStatusResponse = { conversationId, isActive, status }; + return c.json(body, 200); + }); + app.get("/models", async (c) => { try { const models = await opts.credentialStore.listCatalog(); @@ -281,6 +293,7 @@ export function createApp(opts: CreateServerOptions): Hono { const events: AgentEvent[] = []; let controllerRef: ReadableStreamDefaultController<Uint8Array> | undefined; + let streamClosed = false; const stream = new ReadableStream<Uint8Array>({ start(controller) { @@ -288,12 +301,38 @@ export function createApp(opts: CreateServerOptions): Hono { }, }); + function safeEnqueue(data: Uint8Array): void { + if (streamClosed) return; + try { + controllerRef?.enqueue(data); + } catch (err) { + streamClosed = true; + log.warn("chat: stream enqueue failed", { + conversationId, + error: err instanceof Error ? err.message : String(err), + }); + } + } + + function safeClose(): void { + if (streamClosed) return; + streamClosed = true; + try { + controllerRef?.close(); + } catch (err) { + log.warn("chat: stream close failed", { + conversationId, + error: err instanceof Error ? err.message : String(err), + }); + } + } + const orchestratorInput: Parameters<SessionOrchestrator["handleMessage"]>[0] = { conversationId, text: message, onEvent: (event) => { events.push(event); - controllerRef?.enqueue(new TextEncoder().encode(serializeEventLine(event))); + safeEnqueue(new TextEncoder().encode(serializeEventLine(event))); }, ...(model !== undefined ? { modelName: model } : {}), ...(cwd !== undefined ? { cwd } : {}), @@ -304,7 +343,7 @@ export function createApp(opts: CreateServerOptions): Hono { opts.orchestrator .handleMessage(orchestratorInput) .then(async () => { - controllerRef?.close(); + safeClose(); await recordThroughput(events, model); }) .catch((err) => { @@ -315,8 +354,8 @@ export function createApp(opts: CreateServerOptions): Hono { turnId: "", message: err instanceof Error ? err.message : String(err), }; - controllerRef?.enqueue(new TextEncoder().encode(serializeEventLine(errorEvent))); - controllerRef?.close(); + safeEnqueue(new TextEncoder().encode(serializeEventLine(errorEvent))); + safeClose(); }); return new Response(stream, { @@ -485,7 +524,22 @@ export function createApp(opts: CreateServerOptions): Hono { return c.json({ error: "Field 'cwd' is required and must be a non-empty string" }, 400); } + // When a workspaceId is provided, assign the conversation to that + // workspace BEFORE persisting the cwd — so a subsequent + // GET /conversations/:id/lsp resolves a relative cwd against the + // workspace's defaultCwd (not the server default). Omit for unchanged + // workspace assignment (backward compatible). + if (obj.workspaceId !== undefined) { + if (typeof obj.workspaceId !== "string" || !isValidWorkspaceSlug(obj.workspaceId)) { + return c.json({ error: "Invalid workspaceId" }, 400); + } + } + try { + if (typeof obj.workspaceId === "string") { + await opts.conversationStore.ensureWorkspace(obj.workspaceId); + await opts.conversationStore.setWorkspaceId(conversationId, obj.workspaceId); + } await opts.conversationStore.setCwd(conversationId, obj.cwd); log.info("conversations: cwd set", { conversationId }); const response: CwdResponse = { conversationId, cwd: obj.cwd }; @@ -496,19 +550,17 @@ export function createApp(opts: CreateServerOptions): Hono { } }); - app.delete("/conversations/:id/cwd", (c) => { + app.delete("/conversations/:id/cwd", async (c) => { const conversationId = c.req.param("id"); - // CR: The ConversationStore interface currently has no `clearCwd` method. - // `setCwd(id, "")` would persist an empty string (not the same as "no - // cwd key"), and `getEffectiveCwd` treats any non-null explicit cwd as - // set — so an empty string would shadow the workspace defaultCwd instead - // of clearing. A `clearCwd` method (wrapping `storage.delete(cwdKey(id))`) - // is needed on the store interface to truly clear the persisted key. - // For now the route returns the contract shape (cwd: null); the actual - // clearing is a no-op until the CR is implemented. - log.info("conversations: cwd cleared", { conversationId }); - const response: CwdResponse = { conversationId, cwd: null }; - return c.json(response, 200); + try { + await opts.conversationStore.clearCwd(conversationId); + log.info("conversations: cwd cleared", { conversationId }); + const response: CwdResponse = { conversationId, cwd: null }; + return c.json(response, 200); + } catch (err) { + log.error("conversations: cwd clear failure", { err }); + return c.json({ error: "Failed to clear conversation cwd" }, 500); + } }); app.get("/conversations/:id/reasoning-effort", async (c) => { @@ -557,19 +609,32 @@ export function createApp(opts: CreateServerOptions): Hono { app.get("/conversations/:id/lsp", async (c) => { const conversationId = c.req.param("id"); try { - const cwd = await opts.conversationStore.getEffectiveCwd(conversationId); - if (cwd === null) { + // Gate on the PERSISTED cwd first: when no cwd has been set for the + // conversation, the LSP does NOT connect (return null + empty servers) + // rather than falling through to the server default (process.cwd()). + const persistedCwd = await opts.conversationStore.getCwd(conversationId); + if (persistedCwd === null) { log.info("conversations: lsp status read (no cwd)", { conversationId }); const body: LspStatusResponse = { conversationId, cwd: null, servers: [] }; return c.json(body, 200); } + // A persisted cwd exists → resolve the EFFECTIVE cwd (relative cwd + // resolved against the workspace defaultCwd; absolute → as-is). + const effectiveCwd = await opts.conversationStore.getEffectiveCwd(conversationId); + if (effectiveCwd === null) { + // Edge case: persisted cwd exists but resolution returned null. + log.info("conversations: lsp status read (no effective cwd)", { conversationId }); + const body: LspStatusResponse = { conversationId, cwd: null, servers: [] }; + return c.json(body, 200); + } + if (opts.lspService === undefined) { log.warn("conversations: lsp service not available", { conversationId }); return c.json({ error: "LSP service not available" }, 503); } - const statuses = await opts.lspService.status(cwd); + const statuses = await opts.lspService.status(effectiveCwd); const servers: LspServerInfo[] = statuses.map((s: LspServerStatus) => { const info: LspServerInfo = { id: s.id, @@ -583,10 +648,10 @@ export function createApp(opts: CreateServerOptions): Hono { }); log.info("conversations: lsp status read", { conversationId, - cwd, + cwd: effectiveCwd, serverCount: servers.length, }); - const body: LspStatusResponse = { conversationId, cwd, servers }; + const body: LspStatusResponse = { conversationId, cwd: effectiveCwd, servers }; return c.json(body, 200); } catch (err) { log.error("conversations: lsp status failure", { err }); |
