From 9c89ec9db22d0a7226c36b62640addc00918029b Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Tue, 2 Jun 2026 15:30:42 +0900 Subject: fix(tabs): advertise send_to_tab/read_tab in the agent system prompt Granted tab-messaging tools were registered in the API tool payload but buildSystemPrompt built its 'You have access to the following tools' list by filtering toolNames through TOOL_DESCRIPTIONS, which had no entries for send_to_tab/read_tab. The model was therefore told it lacked those tools and refused to use them even when explicitly granted. Add the two missing TOOL_DESCRIPTIONS entries so the capability list matches the granted toolset. Add regression tests that capture the constructed Agent's systemPrompt and assert the tab-messaging tools are listed when granted (and omitted when not), locking the prompt list to the schema list so they can't drift again. --- packages/api/src/agent-manager.ts | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'packages/api/src') diff --git a/packages/api/src/agent-manager.ts b/packages/api/src/agent-manager.ts index 85dd160..36a26f8 100644 --- a/packages/api/src/agent-manager.ts +++ b/packages/api/src/agent-manager.ts @@ -83,6 +83,10 @@ const TOOL_DESCRIPTIONS: Record = { web_search: "Search the web and optionally scrape full page content from results.", youtube_transcribe: "Fetch the transcript/subtitles for a YouTube video. Set background=true to start in the background and get a job_id for later retrieval.", + send_to_tab: + "Send a message to another tab (agent) by its short ID, as shown in the tab bar. Fire-and-forget: it queues/wakes the target and returns immediately without waiting for a reply. Use read_tab later to read the target's response.", + read_tab: + "Read another tab (agent)'s most recent completed response by its short ID. Returns a non-blocking snapshot; if the target is still running you get its previous completed turn. Use after send_to_tab to collect a reply.", }; /** -- cgit v1.2.3 From e475e527cd768dc05368a0881a07a84ea140e13e Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Tue, 2 Jun 2026 15:42:00 +0900 Subject: fix(tabs): clearer send_to_tab context to stop busy-wait + wrong-recipient replies MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two behavioral problems observed once the tools were usable: 1. The SENDER busy-waited for a reply (ran 'sleep 20' / polled) instead of ending its turn. Tool description, the delivery result text, and the system-prompt one-liner now say plainly: do not sleep/poll/run commands to wait; a reply arrives on its own in a later turn (or via read_tab in a future turn); keep working if there's other work, else end your turn. 2. The RECIPIENT replied to its OWN user in plain text instead of routing the answer back through send_to_tab. The provenance wrapper now states the message is from another AGENT (not your user), and that to reply you must use send_to_tab addressed to the sender's handle — and only if asked, since it may just be context. A plain text answer reaches only your own user. Tests updated for the new wording. --- packages/api/src/agent-manager.ts | 2 +- packages/core/src/tools/send-to-tab.ts | 32 ++++++++++++++++++++++----- packages/core/tests/tools/send-to-tab.test.ts | 16 ++++++++++++-- 3 files changed, 42 insertions(+), 8 deletions(-) (limited to 'packages/api/src') diff --git a/packages/api/src/agent-manager.ts b/packages/api/src/agent-manager.ts index 36a26f8..4264884 100644 --- a/packages/api/src/agent-manager.ts +++ b/packages/api/src/agent-manager.ts @@ -84,7 +84,7 @@ const TOOL_DESCRIPTIONS: Record = { youtube_transcribe: "Fetch the transcript/subtitles for a YouTube video. Set background=true to start in the background and get a job_id for later retrieval.", send_to_tab: - "Send a message to another tab (agent) by its short ID, as shown in the tab bar. Fire-and-forget: it queues/wakes the target and returns immediately without waiting for a reply. Use read_tab later to read the target's response.", + "Send a message to another tab (agent) by its short ID, as shown in the tab bar. Fire-and-forget: it queues/wakes the target and returns immediately without waiting for a reply. Do NOT sleep, poll, or run commands to wait — a reply arrives on its own in a later turn (or use read_tab in a future turn); if you are only waiting, end your turn.", read_tab: "Read another tab (agent)'s most recent completed response by its short ID. Returns a non-blocking snapshot; if the target is still running you get its previous completed turn. Use after send_to_tab to collect a reply.", }; diff --git a/packages/core/src/tools/send-to-tab.ts b/packages/core/src/tools/send-to-tab.ts index eb86b7e..84e5f25 100644 --- a/packages/core/src/tools/send-to-tab.ts +++ b/packages/core/src/tools/send-to-tab.ts @@ -64,9 +64,14 @@ export function createSendToTabTool(callbacks: SendToTabCallbacks): ToolDefiniti " - If the target tab is idle, your message WAKES it and starts a new turn.", "", "This is fire-and-forget: it returns immediately and does NOT wait for a reply.", - "Use the 'read_tab' tool with the same ID later to read the target's latest response.", + "Do NOT sleep, poll, or run shell commands to wait for a reply — that wastes turns and", + "money. If the target replies it arrives on its own as a new message in a later turn; you", + "can also call 'read_tab' with the same ID in a FUTURE turn to check. If you have other", + "work to do, keep going; if you are ONLY waiting for the reply, end your turn now.", "", - "Your tab ID is auto-added to the top of the message so the recipient can reply to you.", + "Your tab ID is auto-added to the top of the message so the recipient knows who to reply", + "to. The recipient must use this same 'send_to_tab' tool (addressed to your ID) to answer;", + "a plain text response reaches only their own user, not you.", "IDs are git-style prefixes: pass any length that uniquely identifies the target (min 4 chars).", "If the ID is ambiguous you'll be asked to add a character.", ].join("\n"), @@ -117,8 +122,18 @@ export function createSendToTabTool(callbacks: SendToTabCallbacks): ToolDefiniti } // Stamp provenance so the recipient (and the watching user) can see - // which tab the message came from and reply back via its handle. - const delivered = `[message from tab ${callbacks.self.handle}]\n\n${message}`; + // which tab the message came from and how to reply. The header makes + // clear this is a PEER AGENT, not the recipient's own user, and the + // footer states the reply contract: a reply (only if warranted) must + // go back through `send_to_tab`, since a plain text answer reaches + // only the recipient's own user — not this sender. + const delivered = [ + `[message from tab ${callbacks.self.handle} — this is another agent, NOT your user]`, + "", + message, + "", + `[To reply to tab ${callbacks.self.handle}, use the send_to_tab tool with tab_id "${callbacks.self.handle}". Only reply if this message asks you to, or your user tells you to — it may just be context or instructions. A plain text response goes to your own user, not to this agent.]`, + ].join("\n"); try { const result = await callbacks.deliver(target.id, delivered); @@ -138,7 +153,14 @@ export function createSendToTabTool(callbacks: SendToTabCallbacks): ToolDefiniti result.status === "queued" ? "queued (target is busy; it will be picked up next turn)" : "delivered (target was idle; a new turn has started)"; - return `Message ${verb}. Target tab: ${target.handle} (${target.title}). Use read_tab with "${target.handle}" to read its reply later.`; + return [ + `Message ${verb}. Target tab: ${target.handle} (${target.title}).`, + "", + "Do NOT sleep, poll, or run commands to wait for a reply. If the target replies it", + `arrives on its own as a new message later; you can also call read_tab with "${target.handle}"`, + "in a FUTURE turn to check. Keep working if you have other tasks; if you are ONLY", + "waiting for this reply, end your turn now.", + ].join("\n"); } catch (err) { return `Error delivering message: ${err instanceof Error ? err.message : String(err)}`; } diff --git a/packages/core/tests/tools/send-to-tab.test.ts b/packages/core/tests/tools/send-to-tab.test.ts index 4450fc5..68f8fa0 100644 --- a/packages/core/tests/tools/send-to-tab.test.ts +++ b/packages/core/tests/tools/send-to-tab.test.ts @@ -24,6 +24,9 @@ describe("createSendToTabTool — schema & description", () => { expect(tool.name).toBe("send_to_tab"); expect(tool.description).toContain("fire-and-forget"); expect(tool.description.toLowerCase()).toContain("queued"); + // Description must steer the model away from busy-waiting for a reply. + expect(tool.description.toLowerCase()).toContain("do not sleep"); + expect(tool.description.toLowerCase()).toContain("end your turn"); }); }); @@ -35,11 +38,20 @@ describe("createSendToTabTool — execute()", () => { expect(deliver).toHaveBeenCalledTimes(1); const [targetId, delivered] = deliver.mock.calls[0] ?? []; expect(targetId).toBe("target-id"); - // Provenance prefix names the sending tab's handle. - expect(delivered).toContain("[message from tab self]"); + // Provenance header names the sending tab's handle and marks it as a + // peer agent (not the recipient's own user). + expect(delivered).toContain("[message from tab self"); + expect(delivered).toContain("another agent"); expect(delivered).toContain("hello there"); + // Reply contract: the recipient must answer via send_to_tab back to the + // sender's handle, not as a plain text reply to its own user. + expect(delivered).toContain('send_to_tab tool with tab_id "self"'); + expect(delivered.toLowerCase()).toContain("only reply if"); expect(out).toContain("idle"); expect(out).toContain("targ"); + // Sender is steered away from busy-waiting and told to end its turn. + expect(out.toLowerCase()).toContain("do not sleep"); + expect(out.toLowerCase()).toContain("end your turn"); }); it("reports the queued status when the target is busy", async () => { -- cgit v1.2.3 From aa295e82197ebc77d9466eee28380bc5bcc0863d Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Tue, 2 Jun 2026 15:53:15 +0900 Subject: fix(tabs): only mention read_tab when the sender actually has it; CAPS on ONLY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The send_to_tab guidance previously told the agent it could call read_tab to check for a reply, but the tab-messaging permissions are split — a tab can hold send_to_tab WITHOUT read_tab (the exact case in testing). Advertising a tool the agent wasn't granted is wrong. Thread a canReadTab flag from AgentManager.buildTabCommToolEntries into createSendToTabTool (true iff this tab is also granted read_tab). The tool description and the delivery-result text now only reference read_tab when canReadTab is true; otherwise they say a reply arrives on its own and to end the turn. Drop the read_tab phrasing from the static TOOL_DESCRIPTIONS one-liner (can't be conditional per-tab there). Also uppercase ONLY in the recipient reply-contract footer for emphasis. Tests: cover both canReadTab branches for description + result text; assert ONLY is uppercased. --- packages/api/src/agent-manager.ts | 13 ++++++-- packages/core/src/tools/send-to-tab.ts | 45 ++++++++++++++++++++++----- packages/core/tests/tools/send-to-tab.test.ts | 33 +++++++++++++++++++- 3 files changed, 79 insertions(+), 12 deletions(-) (limited to 'packages/api/src') diff --git a/packages/api/src/agent-manager.ts b/packages/api/src/agent-manager.ts index 4264884..3d233fc 100644 --- a/packages/api/src/agent-manager.ts +++ b/packages/api/src/agent-manager.ts @@ -84,7 +84,7 @@ const TOOL_DESCRIPTIONS: Record = { youtube_transcribe: "Fetch the transcript/subtitles for a YouTube video. Set background=true to start in the background and get a job_id for later retrieval.", send_to_tab: - "Send a message to another tab (agent) by its short ID, as shown in the tab bar. Fire-and-forget: it queues/wakes the target and returns immediately without waiting for a reply. Do NOT sleep, poll, or run commands to wait — a reply arrives on its own in a later turn (or use read_tab in a future turn); if you are only waiting, end your turn.", + "Send a message to another tab (agent) by its short ID, as shown in the tab bar. Fire-and-forget: it queues/wakes the target and returns immediately without waiting for a reply. Do NOT sleep, poll, or run commands to wait — a reply arrives on its own in a later turn; if you are only waiting, end your turn.", read_tab: "Read another tab (agent)'s most recent completed response by its short ID. Returns a non-blocking snapshot; if the target is still running you get its previous completed turn. Use after send_to_tab to collect a reply.", }; @@ -546,7 +546,7 @@ export class AgentManager { } // Tab-to-tab communication — gated on the child whitelist. if (allowed.has("send_to_tab") || allowed.has("read_tab")) { - for (const entry of this.buildTabCommToolEntries(tabId)) { + for (const entry of this.buildTabCommToolEntries(tabId, allowed.has("read_tab"))) { if (allowed.has(entry.name)) toolEntries.push(entry); } } @@ -631,7 +631,7 @@ export class AgentManager { const tabCommAllowed = new Set(); if (permSendToTab) tabCommAllowed.add("send_to_tab"); if (permReadTab) tabCommAllowed.add("read_tab"); - for (const entry of this.buildTabCommToolEntries(tabId)) { + for (const entry of this.buildTabCommToolEntries(tabId, permReadTab)) { if (tabCommAllowed.has(entry.name)) toolEntries.push(entry); } } @@ -1241,9 +1241,15 @@ export class AgentManager { * both tool-construction paths (child whitelist + permission-gated parent). * `selfHandle` is computed once so the calling tab can stamp provenance and * reject self-sends. + * + * `canReadTab` reflects whether THIS tab will also be granted `read_tab` + * (the permissions are split). It is forwarded into `send_to_tab` so the + * tool only points the agent at `read_tab` when it actually has it — never + * advertising a tool the agent wasn't granted. */ private buildTabCommToolEntries( tabId: string, + canReadTab: boolean, ): Array<{ name: string; tool: ReturnType }> { const selfHandle = shortestUniquePrefix(tabId); return [ @@ -1257,6 +1263,7 @@ export class AgentManager { this.deliverMessage(targetId, message, { origin: "agent" }), listOpenHandles: () => this.listOpenHandles(tabId), self: { id: tabId, handle: selfHandle }, + canReadTab, }), }, { diff --git a/packages/core/src/tools/send-to-tab.ts b/packages/core/src/tools/send-to-tab.ts index 84e5f25..50023a7 100644 --- a/packages/core/src/tools/send-to-tab.ts +++ b/packages/core/src/tools/send-to-tab.ts @@ -44,6 +44,13 @@ export interface SendToTabCallbacks { /** The calling tab's own id + handle — used to block self-sends and to * stamp provenance onto the delivered message. */ self: { id: string; handle: string }; + /** + * Whether THIS calling tab also has the `read_tab` tool granted. The + * tab-messaging permissions are split, so a tab can hold `send_to_tab` + * without `read_tab`. When false, the tool must NOT tell the agent to use + * `read_tab` (it doesn't have it) — replies only arrive on their own. + */ + canReadTab: boolean; } /** Render the "available tabs" hint shared by the none/ambiguous branches. */ @@ -54,6 +61,19 @@ function renderOpenHandles(handles: Array<{ handle: string; title: string }>): s } export function createSendToTabTool(callbacks: SendToTabCallbacks): ToolDefinition { + // The `read_tab` follow-up hint is only truthful when this tab actually + // holds the `read_tab` tool (the permissions are split). When it doesn't, + // the only honest guidance is that a reply arrives on its own — never tell + // the agent to call a tool it wasn't granted. + const waitLine = callbacks.canReadTab + ? "money. If the target replies it arrives on its own as a new message in a later turn; you" + : "money. If the target replies it arrives on its own as a new message in a later turn."; + const readTabLine = callbacks.canReadTab + ? ["can also call 'read_tab' with the same ID in a FUTURE turn to check. If you have other"] + : []; + const keepGoingLine = callbacks.canReadTab + ? "work to do, keep going; if you are ONLY waiting for the reply, end your turn now." + : "If you have other work to do, keep going; if you are ONLY waiting for the reply, end your turn now."; return { name: "send_to_tab", description: [ @@ -65,9 +85,9 @@ export function createSendToTabTool(callbacks: SendToTabCallbacks): ToolDefiniti "", "This is fire-and-forget: it returns immediately and does NOT wait for a reply.", "Do NOT sleep, poll, or run shell commands to wait for a reply — that wastes turns and", - "money. If the target replies it arrives on its own as a new message in a later turn; you", - "can also call 'read_tab' with the same ID in a FUTURE turn to check. If you have other", - "work to do, keep going; if you are ONLY waiting for the reply, end your turn now.", + waitLine, + ...readTabLine, + keepGoingLine, "", "Your tab ID is auto-added to the top of the message so the recipient knows who to reply", "to. The recipient must use this same 'send_to_tab' tool (addressed to your ID) to answer;", @@ -132,7 +152,7 @@ export function createSendToTabTool(callbacks: SendToTabCallbacks): ToolDefiniti "", message, "", - `[To reply to tab ${callbacks.self.handle}, use the send_to_tab tool with tab_id "${callbacks.self.handle}". Only reply if this message asks you to, or your user tells you to — it may just be context or instructions. A plain text response goes to your own user, not to this agent.]`, + `[To reply to tab ${callbacks.self.handle}, use the send_to_tab tool with tab_id "${callbacks.self.handle}". ONLY reply if this message asks you to, or your user tells you to — it may just be context or instructions. A plain text response goes to your own user, not to this agent.]`, ].join("\n"); try { @@ -153,13 +173,22 @@ export function createSendToTabTool(callbacks: SendToTabCallbacks): ToolDefiniti result.status === "queued" ? "queued (target is busy; it will be picked up next turn)" : "delivered (target was idle; a new turn has started)"; + const tail = callbacks.canReadTab + ? [ + "Do NOT sleep, poll, or run commands to wait for a reply. If the target replies it", + `arrives on its own as a new message later; you can also call read_tab with "${target.handle}"`, + "in a FUTURE turn to check. Keep working if you have other tasks; if you are ONLY", + "waiting for this reply, end your turn now.", + ] + : [ + "Do NOT sleep, poll, or run commands to wait for a reply. If the target replies it", + "arrives on its own as a new message later. Keep working if you have other tasks; if", + "you are ONLY waiting for this reply, end your turn now.", + ]; return [ `Message ${verb}. Target tab: ${target.handle} (${target.title}).`, "", - "Do NOT sleep, poll, or run commands to wait for a reply. If the target replies it", - `arrives on its own as a new message later; you can also call read_tab with "${target.handle}"`, - "in a FUTURE turn to check. Keep working if you have other tasks; if you are ONLY", - "waiting for this reply, end your turn now.", + ...tail, ].join("\n"); } catch (err) { return `Error delivering message: ${err instanceof Error ? err.message : String(err)}`; diff --git a/packages/core/tests/tools/send-to-tab.test.ts b/packages/core/tests/tools/send-to-tab.test.ts index 68f8fa0..48ff460 100644 --- a/packages/core/tests/tools/send-to-tab.test.ts +++ b/packages/core/tests/tools/send-to-tab.test.ts @@ -14,6 +14,7 @@ function makeCallbacks(overrides: Partial = {}): SendToTabCa deliver: () => ({ status: "started" }), listOpenHandles: () => [{ handle: "targ", title: "Target" }], self: { id: "self-id", handle: "self" }, + canReadTab: true, ...overrides, }; } @@ -28,6 +29,19 @@ describe("createSendToTabTool — schema & description", () => { expect(tool.description.toLowerCase()).toContain("do not sleep"); expect(tool.description.toLowerCase()).toContain("end your turn"); }); + + it("mentions read_tab in the description only when canReadTab is true", () => { + const tool = createSendToTabTool(makeCallbacks({ canReadTab: true })); + expect(tool.description).toContain("read_tab"); + }); + + it("never mentions read_tab in the description when canReadTab is false", () => { + const tool = createSendToTabTool(makeCallbacks({ canReadTab: false })); + expect(tool.description).not.toContain("read_tab"); + // Still tells the agent a reply arrives on its own + to end its turn. + expect(tool.description.toLowerCase()).toContain("arrives on its own"); + expect(tool.description.toLowerCase()).toContain("end your turn"); + }); }); describe("createSendToTabTool — execute()", () => { @@ -46,7 +60,7 @@ describe("createSendToTabTool — execute()", () => { // Reply contract: the recipient must answer via send_to_tab back to the // sender's handle, not as a plain text reply to its own user. expect(delivered).toContain('send_to_tab tool with tab_id "self"'); - expect(delivered.toLowerCase()).toContain("only reply if"); + expect(delivered).toContain("ONLY reply if"); expect(out).toContain("idle"); expect(out).toContain("targ"); // Sender is steered away from busy-waiting and told to end its turn. @@ -54,6 +68,23 @@ describe("createSendToTabTool — execute()", () => { expect(out.toLowerCase()).toContain("end your turn"); }); + it("points the sender at read_tab in the result only when canReadTab is true", async () => { + const deliver = vi.fn(() => ({ status: "started" as const })); + const tool = createSendToTabTool(makeCallbacks({ deliver, canReadTab: true })); + const out = await tool.execute({ tab_id: "targ", message: "hi" }); + expect(out).toContain("read_tab"); + }); + + it("omits read_tab from the result when canReadTab is false", async () => { + const deliver = vi.fn(() => ({ status: "started" as const })); + const tool = createSendToTabTool(makeCallbacks({ deliver, canReadTab: false })); + const out = await tool.execute({ tab_id: "targ", message: "hi" }); + expect(out).not.toContain("read_tab"); + // Still steers away from busy-waiting and toward ending the turn. + expect(out.toLowerCase()).toContain("do not sleep"); + expect(out.toLowerCase()).toContain("end your turn"); + }); + it("reports the queued status when the target is busy", async () => { const deliver = vi.fn(() => ({ status: "queued" as const })); const tool = createSendToTabTool(makeCallbacks({ deliver })); -- cgit v1.2.3 From 3ff2db698c2633023934d8477a9e995f78fa011e Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Tue, 2 Jun 2026 15:54:39 +0900 Subject: fix(perm): decouple perm_user_agent from perm_summon for spawning user agents Granting only the user-agent (top-level) permission without the subagent-summon permission left the agent unable to summon user agents: the whole summon tool was gated behind perm_summon, so perm_user_agent alone produced no summon tool. Register summon when EITHER perm_summon OR perm_user_agent is granted. createSummonTool now takes an independent subagentEnabled flag (mirrors perm_summon) alongside userAgentEnabled (mirrors perm_user_agent): - subagent-only -> ordinary subagents, no top_level - user-agent-only -> spawns ONLY top-level user agents (top_level forced, background/top_level params dropped, user-agent catalog only) - both -> unchanged full behavior retrieve stays bundled with perm_summon (user agents are fire-and-forget). Adds core summon tests (user-agent-only mode + legacy-default regression) and an agent-manager summon/user_agent permission-split suite. --- packages/api/src/agent-manager.ts | 36 ++++--- packages/api/tests/agent-manager.test.ts | 61 ++++++++++++ packages/core/src/tools/summon.ts | 163 ++++++++++++++++++++++--------- packages/core/tests/tools/summon.test.ts | 108 ++++++++++++++++++++ 4 files changed, 308 insertions(+), 60 deletions(-) (limited to 'packages/api/src') diff --git a/packages/api/src/agent-manager.ts b/packages/api/src/agent-manager.ts index 85dd160..9499ce5 100644 --- a/packages/api/src/agent-manager.ts +++ b/packages/api/src/agent-manager.ts @@ -575,7 +575,13 @@ export class AgentManager { }); } toolEntries.push({ name: "todo", tool: createTaskListTool(tabAgent.taskList) }); - if (permSummon) { + // The `summon` tool is registered when EITHER the subagent + // permission (`perm_summon`) OR the user-agent permission + // (`perm_user_agent`) is granted — the two are independent. + // `perm_summon` enables ordinary subagent spawning; granting + // only `perm_user_agent` exposes summon in user-agent-only mode + // (spawns top-level user agents exclusively). + if (permSummon || permUserAgent) { // Capture parent's allowed tool names for child permission enforcement const parentAllowedTools = new Set(toolEntries.map((e) => e.name)); const allAgentDefs = loadAgents(workingDirectory); @@ -609,19 +615,25 @@ export class AgentManager { availableUserAgents, agentDirPaths, permUserAgent, + permSummon, ), }); - toolEntries.push({ - name: "retrieve", - tool: createRetrieveTool({ - getResult: (id) => - tabAgent.shellStore.has(id) - ? tabAgent.shellStore.getResult(id) - : tabAgent.transcriptStore.has(id) - ? tabAgent.transcriptStore.getResult(id) - : this.getChildResult(id), - }), - }); + // `retrieve` collects subagent results. User agents are + // fire-and-forget, so it is bundled with the subagent + // permission only — a user-agent-only grant doesn't get it. + if (permSummon) { + toolEntries.push({ + name: "retrieve", + tool: createRetrieveTool({ + getResult: (id) => + tabAgent.shellStore.has(id) + ? tabAgent.shellStore.getResult(id) + : tabAgent.transcriptStore.has(id) + ? tabAgent.transcriptStore.getResult(id) + : this.getChildResult(id), + }), + }); + } } if (permSendToTab || permReadTab) { const tabCommAllowed = new Set(); diff --git a/packages/api/tests/agent-manager.test.ts b/packages/api/tests/agent-manager.test.ts index 014022a..f3ea207 100644 --- a/packages/api/tests/agent-manager.test.ts +++ b/packages/api/tests/agent-manager.test.ts @@ -319,6 +319,22 @@ vi.mock("@dispatch/core", () => ({ execute: async () => "mock", }; }, + // Summon parent-path dependencies. The real implementations load agent + // definitions from disk; tests only need the summon/retrieve tool entries + // to appear, so these return empty projections. + loadAgents() { + return []; + }, + toAvailableSubagents() { + return []; + }, + toAvailableUserAgents() { + return []; + }, + getAgentDirPaths() { + return []; + }, + GLOBAL_AGENTS_DIR: "/tmp/global-agents", createTab() {}, getTab(id: string) { return fakeTabs.get(id) ?? null; @@ -1441,6 +1457,51 @@ describe("AgentManager", () => { }); }); + describe("summon / user_agent permission split", () => { + // Drives the real parent-path tool construction in + // getOrCreateAgentForTab by toggling perm_summon and perm_user_agent + // independently, then inspecting which tools the constructed Agent + // received. The summon tool must be registered when EITHER permission + // is granted; `retrieve` rides with the subagent permission only + // (user agents are fire-and-forget). + async function toolsForPerms(tabId: string, perms: Record): Promise { + for (const [k, v] of Object.entries(perms)) setFakeSetting(k, v); + const manager = new AgentManager(); + await manager.processMessage(tabId, "go"); + return constructedAgents.at(-1)?.toolNames ?? []; + } + + it("grants summon + retrieve when only perm_summon is allowed", async () => { + const tools = await toolsForPerms("tab-summon-only", { perm_summon: "allow" }); + expect(tools).toContain("summon"); + expect(tools).toContain("retrieve"); + }); + + it("grants summon WITHOUT retrieve when only perm_user_agent is allowed", async () => { + // Regression: granting only the user-agent permission used to leave + // the agent unable to summon user agents because the whole summon + // tool was gated behind perm_summon. + const tools = await toolsForPerms("tab-user-agent-only", { perm_user_agent: "allow" }); + expect(tools).toContain("summon"); + expect(tools).not.toContain("retrieve"); + }); + + it("grants summon + retrieve when both permissions are allowed", async () => { + const tools = await toolsForPerms("tab-summon-both", { + perm_summon: "allow", + perm_user_agent: "allow", + }); + expect(tools).toContain("summon"); + expect(tools).toContain("retrieve"); + }); + + it("grants neither summon nor retrieve when both permissions are off", async () => { + const tools = await toolsForPerms("tab-summon-neither", {}); + expect(tools).not.toContain("summon"); + expect(tools).not.toContain("retrieve"); + }); + }); + // ─── Usage side-channel persistence ────────────────────────────── // // `usage` AgentEvents (one per LLM round-trip) are persisted as invisible diff --git a/packages/core/src/tools/summon.ts b/packages/core/src/tools/summon.ts index 4820e89..cfee8b8 100644 --- a/packages/core/src/tools/summon.ts +++ b/packages/core/src/tools/summon.ts @@ -60,10 +60,13 @@ function renderAgentGroup(label: string, agents: AvailableAgent[]): string[] { * the disk locations where they live, injected into the summon tool's * description. * - * When `userAgentEnabled` is false only subagents are shown (under the - * generic "Available agents" heading). When it is true, subagents and - * user agents are listed as two labelled groups so the LLM understands - * which slugs require `top_level=true`. + * `subagentEnabled` and `userAgentEnabled` independently control which + * groups are shown — they mirror the `perm_summon` and `perm_user_agent` + * permissions respectively: + * - subagents only → generic "Available agents" heading; + * - user agents only → a single user-agent group (top_level is implied); + * - both → two labelled groups so the LLM understands which slugs + * require `top_level=true`. * * Returns a compact "no agents defined" notice when nothing is visible. */ @@ -72,6 +75,7 @@ function buildAgentsCatalog( userAgents: AvailableAgent[], agentDirs: string[], userAgentEnabled: boolean, + subagentEnabled: boolean, ): string { const lines: string[] = []; lines.push(""); @@ -80,8 +84,9 @@ function buildAgentsCatalog( lines.push(` - ${d}`); } + const visibleSubagents = subagentEnabled ? subagents : []; const visibleUserAgents = userAgentEnabled ? userAgents : []; - if (subagents.length === 0 && visibleUserAgents.length === 0) { + if (visibleSubagents.length === 0 && visibleUserAgents.length === 0) { lines.push(""); lines.push("No agent definitions are currently defined."); return lines.join("\n"); @@ -93,12 +98,26 @@ function buildAgentsCatalog( lines.push("and working directory; the 'tools' parameter is ignored."); lines.push(""); + // User-agent-only mode: list just the user agents. top_level is implied + // (it is the only thing this grant can spawn), so the heading omits it. + if (!subagentEnabled && userAgentEnabled) { + lines.push( + ...renderAgentGroup( + "User agents (spawned as independent top-level tabs):", + visibleUserAgents, + ), + ); + return lines.join("\n"); + } + + // Subagent-only mode: single generic heading. if (!userAgentEnabled) { - lines.push(...renderAgentGroup("Available agents:", subagents)); + lines.push(...renderAgentGroup("Available agents:", visibleSubagents)); return lines.join("\n"); } - const subagentLines = renderAgentGroup("Subagents (spawned as child tabs):", subagents); + // Both enabled: two labelled groups. + const subagentLines = renderAgentGroup("Subagents (spawned as child tabs):", visibleSubagents); const userAgentLines = renderAgentGroup( "User agents (spawned as independent top-level tabs, requires top_level=true):", visibleUserAgents, @@ -122,9 +141,14 @@ function buildAgentsCatalog( * its description; this is information-only — the runtime resolves * slugs through `loadAgent` independently. * - * `userAgentEnabled` controls whether the `top_level` parameter and the - * user-agent catalog are surfaced to the LLM. It mirrors the - * `perm_user_agent` permission. + * `userAgentEnabled` mirrors the `perm_user_agent` permission and + * `subagentEnabled` mirrors the `perm_summon` permission. They are + * independent: the tool is registered whenever at least one is granted. + * - subagentEnabled only → spawn ordinary subagents (no `top_level`); + * - userAgentEnabled only → spawn ONLY top-level user agents + * (`top_level` is forced on, the `background` knob is dropped, and + * the catalog lists user agents only); + * - both → full behavior (subagents plus `top_level` user agents). */ export function createSummonTool( _defaultWorkingDirectory: string, @@ -133,39 +157,29 @@ export function createSummonTool( availableUserAgents: AvailableAgent[] = [], agentDirs: string[] = [], userAgentEnabled = false, + subagentEnabled = true, ): ToolDefinition { + // When only the user-agent permission is granted the tool spawns user + // agents exclusively: `top_level` is implied (and forced), subagent + // mechanics (background, retrieve, parallel work) are irrelevant. + const userAgentOnly = userAgentEnabled && !subagentEnabled; + const catalog = buildAgentsCatalog( availableSubagents, availableUserAgents, agentDirs, userAgentEnabled, + subagentEnabled, ); const subagentSlugs = availableSubagents.map((a) => a.slug); const userAgentSlugs = availableUserAgents.map((a) => a.slug); - const allSlugs = userAgentEnabled ? [...subagentSlugs, ...userAgentSlugs] : subagentSlugs; + const allSlugs = userAgentOnly + ? userAgentSlugs + : userAgentEnabled + ? [...subagentSlugs, ...userAgentSlugs] + : subagentSlugs; - const description = [ - "Spawn a new child agent to work on a task independently.", - "", - "By default, blocks until the child agent finishes and returns the result directly.", - "Set background=true to return immediately with an agent_id instead — use retrieve to collect the result later.", - "", - "The child agent runs in its own tab visible to the user. Use the 'retrieve' tool with the returned agent_id to get the result when needed.", - "", - "Pattern for parallel work:", - " 1. Call summon multiple times with background=true to start several agents", - " 2. Do your own work or wait", - " 3. Call retrieve for each agent_id to collect results", - ...(userAgentEnabled - ? [ - "", - "Set top_level=true to spawn an independent user agent — a first-class", - "top-level tab with no parent. User agents are fire-and-forget: you get", - "an agent_id back but cannot retrieve their result. top_level requires an", - "'agent' definition listed under 'User agents' below.", - ] - : []), - "", + const toolNamesList = [ "The 'tools' parameter controls what the child can do. Available tool names:", " - read_file: Read file contents", " - read_file_slice: Read a character-range slice of a single line", @@ -179,11 +193,50 @@ export function createSummonTool( " - youtube_transcribe: Fetch YouTube video transcripts", " - send_to_tab: Send a message to another tab/agent by its ID", " - read_tab: Read another tab/agent's latest response by its ID", - "", - "The 'agent' parameter is required — every spawned agent must use a definition.", - "Tools default to the agent definition's tools, intersected with your own tools (you can't grant capabilities you don't have).", - catalog, - ].join("\n"); + ]; + + const description = userAgentOnly + ? [ + "Spawn an independent top-level user agent to work on a task.", + "", + "User agents are first-class top-level tabs with no parent. They are", + "fire-and-forget: you get an agent_id back but cannot retrieve their result.", + "The user agent runs in its own tab visible to the user.", + "", + ...toolNamesList, + "", + "The 'agent' parameter is required — every spawned agent must use a definition.", + "Tools default to the agent definition's tools, intersected with your own tools (you can't grant capabilities you don't have).", + catalog, + ].join("\n") + : [ + "Spawn a new child agent to work on a task independently.", + "", + "By default, blocks until the child agent finishes and returns the result directly.", + "Set background=true to return immediately with an agent_id instead — use retrieve to collect the result later.", + "", + "The child agent runs in its own tab visible to the user. Use the 'retrieve' tool with the returned agent_id to get the result when needed.", + "", + "Pattern for parallel work:", + " 1. Call summon multiple times with background=true to start several agents", + " 2. Do your own work or wait", + " 3. Call retrieve for each agent_id to collect results", + ...(userAgentEnabled + ? [ + "", + "Set top_level=true to spawn an independent user agent — a first-class", + "top-level tab with no parent. User agents are fire-and-forget: you get", + "an agent_id back but cannot retrieve their result. top_level requires an", + "'agent' definition listed under 'User agents' below.", + ] + : []), + "", + ...toolNamesList, + "", + "The 'agent' parameter is required — every spawned agent must use a definition.", + "Tools default to the agent definition's tools, intersected with your own tools (you can't grant capabilities you don't have).", + catalog, + ].join("\n"); const parametersShape = { task: z @@ -205,7 +258,10 @@ export function createSummonTool( .filter(Boolean) .join(" "), ), - ...(userAgentEnabled + // `top_level` is only an explicit choice when BOTH subagents and user + // agents are available. In user-agent-only mode it is implied (forced + // on), so the knob is omitted entirely. + ...(userAgentEnabled && !userAgentOnly ? { top_level: z .boolean() @@ -248,12 +304,18 @@ export function createSummonTool( .describe( "Absolute path for the child to work in. Defaults to the agent definition's cwd (or the spawning agent's directory).", ), - background: z - .boolean() - .optional() - .describe( - "If true, returns immediately with an agent_id for later retrieval. If false (default), blocks until the child agent finishes and returns the result directly. Ignored when top_level is true.", - ), + // `background` is meaningless for fire-and-forget user agents, so the + // knob is omitted in user-agent-only mode. + ...(userAgentOnly + ? {} + : { + background: z + .boolean() + .optional() + .describe( + "If true, returns immediately with an agent_id for later retrieval. If false (default), blocks until the child agent finishes and returns the result directly. Ignored when top_level is true.", + ), + }), }; return { @@ -266,9 +328,14 @@ export function createSummonTool( const tools = args.tools as string[] | undefined; const workingDirectory = args.working_directory as string | undefined; const background = (args.background as boolean | undefined) ?? false; - const topLevel = userAgentEnabled - ? ((args.top_level as boolean | undefined) ?? false) - : false; + // User-agent-only mode always spawns top-level user agents. When both + // capabilities are present the caller chooses via `top_level`. When + // only subagents are available, top-level spawning is unavailable. + const topLevel = userAgentOnly + ? true + : userAgentEnabled + ? ((args.top_level as boolean | undefined) ?? false) + : false; try { const agentId = await callbacks.spawn({ diff --git a/packages/core/tests/tools/summon.test.ts b/packages/core/tests/tools/summon.test.ts index f59f345..4885a94 100644 --- a/packages/core/tests/tools/summon.test.ts +++ b/packages/core/tests/tools/summon.test.ts @@ -239,3 +239,111 @@ describe("createSummonTool — execute() argument forwarding", () => { expect(getResult).toHaveBeenCalled(); }); }); + +describe("createSummonTool — user-agent-only mode (perm_user_agent without perm_summon)", () => { + // userAgentEnabled=true, subagentEnabled=false → the tool spawns ONLY + // top-level user agents. `top_level` is implied (and forced), the + // subagent/parallel-work prose is dropped, and only the user-agent + // catalog group is shown. + const subagents: AvailableAgent[] = [ + { + slug: "programmer", + name: "Programmer", + description: "Codes things", + path: "/agents/programmer.toml", + }, + ]; + const userAgents: AvailableAgent[] = [ + { + slug: "default", + name: "Default", + description: "Default agent", + path: "/agents/default.toml", + }, + ]; + + function userAgentOnlyTool( + spawn = vi.fn(async () => "ua-1"), + getResult = vi.fn(async () => ({ status: "done" as const, result: "nope" })), + ) { + return { + spawn, + getResult, + tool: createSummonTool( + "/tmp/work", + { spawn, getResult }, + subagents, + userAgents, + ["/agents"], + true, // userAgentEnabled + false, // subagentEnabled + ), + }; + } + + it("describes spawning user agents and omits subagent/parallel-work prose", () => { + const { tool } = userAgentOnlyTool(); + expect(tool.description).toContain("Spawn an independent top-level user agent"); + expect(tool.description).toContain("fire-and-forget"); + expect(tool.description).not.toContain("Pattern for parallel work"); + expect(tool.description).not.toContain("Set background=true"); + }); + + it("lists only the user-agent catalog group, not subagents", () => { + const { tool } = userAgentOnlyTool(); + expect(tool.description).toContain("User agents (spawned as independent top-level tabs):"); + expect(tool.description).toContain("default"); + // Subagents must not be advertised in user-agent-only mode. + expect(tool.description).not.toContain("Subagents (spawned as child tabs):"); + expect(tool.description).not.toContain("- programmer: Programmer"); + }); + + it("only lists user-agent slugs in the 'agent' parameter description", () => { + const { tool } = userAgentOnlyTool(); + const agentParam = (tool.parameters as unknown as { shape: { agent: { description: string } } }) + .shape.agent; + expect(agentParam.description).toContain("default"); + expect(agentParam.description).not.toContain("programmer"); + }); + + it("omits the top_level parameter (it is implied)", () => { + const { tool } = userAgentOnlyTool(); + const shape = (tool.parameters as unknown as { shape: Record }).shape; + expect("top_level" in shape).toBe(false); + }); + + it("omits the background parameter (user agents are fire-and-forget)", () => { + const { tool } = userAgentOnlyTool(); + const shape = (tool.parameters as unknown as { shape: Record }).shape; + expect("background" in shape).toBe(false); + }); + + it("forces topLevel=true on spawn even when top_level is not passed", async () => { + const spawn = vi.fn(async () => "ua-99"); + const getResult = vi.fn(async () => ({ status: "done" as const, result: "nope" })); + const { tool } = userAgentOnlyTool(spawn, getResult); + const out = await tool.execute({ task: "do stuff", agent: "default" }); + expect(out).toContain("User agent spawned successfully"); + expect(out).toContain("ua-99"); + expect(out).toContain("fire-and-forget"); + // Never blocks on a result for fire-and-forget user agents. + expect(getResult).not.toHaveBeenCalled(); + const callArg = spawn.mock.calls[0]?.[0]; + expect(callArg).toMatchObject({ topLevel: true, agentSlug: "default" }); + }); +}); + +describe("createSummonTool — subagentEnabled defaults preserve legacy behavior", () => { + it("defaults subagentEnabled=true so omitting it keeps subagent spawning", async () => { + const spawn = vi.fn(async () => "tab-1"); + const getResult = vi.fn(async () => ({ status: "done" as const, result: "child" })); + // No userAgentEnabled/subagentEnabled args → legacy subagent-only mode. + const tool = createSummonTool("/tmp/work", { spawn, getResult }, [], []); + const out = await tool.execute({ task: "x", agent: "programmer" }); + // Foreground subagent summon blocks and returns the child result. + expect(out).toBe("agent_id: tab-1\n\nchild"); + expect(getResult).toHaveBeenCalled(); + const callArg = spawn.mock.calls[0]?.[0]; + expect(callArg).not.toHaveProperty("topLevel"); + }); +}); -- cgit v1.2.3 From e4379da8d1e8c7a8a89c63bdaaef99a74bf56cf2 Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Tue, 2 Jun 2026 15:55:12 +0900 Subject: fix(tabs): say a reply will WAKE you with a new message (clearer than 'arrives on its own') Matches actual behavior: a peer's reply wakes this tab with a new message in a later turn. Updated the send_to_tab description (both canReadTab branches), the delivery-result text (both branches), and the system-prompt one-liner; updated the test assertion accordingly. --- packages/api/src/agent-manager.ts | 2 +- packages/core/src/tools/send-to-tab.ts | 10 +++++----- packages/core/tests/tools/send-to-tab.test.ts | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) (limited to 'packages/api/src') diff --git a/packages/api/src/agent-manager.ts b/packages/api/src/agent-manager.ts index 3d233fc..684f8ec 100644 --- a/packages/api/src/agent-manager.ts +++ b/packages/api/src/agent-manager.ts @@ -84,7 +84,7 @@ const TOOL_DESCRIPTIONS: Record = { youtube_transcribe: "Fetch the transcript/subtitles for a YouTube video. Set background=true to start in the background and get a job_id for later retrieval.", send_to_tab: - "Send a message to another tab (agent) by its short ID, as shown in the tab bar. Fire-and-forget: it queues/wakes the target and returns immediately without waiting for a reply. Do NOT sleep, poll, or run commands to wait — a reply arrives on its own in a later turn; if you are only waiting, end your turn.", + "Send a message to another tab (agent) by its short ID, as shown in the tab bar. Fire-and-forget: it queues/wakes the target and returns immediately without waiting for a reply. Do NOT sleep, poll, or run commands to wait — if the target replies it will wake you with a new message in a later turn; if you are only waiting, end your turn.", read_tab: "Read another tab (agent)'s most recent completed response by its short ID. Returns a non-blocking snapshot; if the target is still running you get its previous completed turn. Use after send_to_tab to collect a reply.", }; diff --git a/packages/core/src/tools/send-to-tab.ts b/packages/core/src/tools/send-to-tab.ts index 50023a7..eae6bfa 100644 --- a/packages/core/src/tools/send-to-tab.ts +++ b/packages/core/src/tools/send-to-tab.ts @@ -63,11 +63,11 @@ function renderOpenHandles(handles: Array<{ handle: string; title: string }>): s export function createSendToTabTool(callbacks: SendToTabCallbacks): ToolDefinition { // The `read_tab` follow-up hint is only truthful when this tab actually // holds the `read_tab` tool (the permissions are split). When it doesn't, - // the only honest guidance is that a reply arrives on its own — never tell + // the only honest guidance is that a reply will wake it as a new message — never tell // the agent to call a tool it wasn't granted. const waitLine = callbacks.canReadTab - ? "money. If the target replies it arrives on its own as a new message in a later turn; you" - : "money. If the target replies it arrives on its own as a new message in a later turn."; + ? "money. If the target replies it will WAKE you with a new message in a later turn; you" + : "money. If the target replies it will WAKE you with a new message in a later turn."; const readTabLine = callbacks.canReadTab ? ["can also call 'read_tab' with the same ID in a FUTURE turn to check. If you have other"] : []; @@ -176,13 +176,13 @@ export function createSendToTabTool(callbacks: SendToTabCallbacks): ToolDefiniti const tail = callbacks.canReadTab ? [ "Do NOT sleep, poll, or run commands to wait for a reply. If the target replies it", - `arrives on its own as a new message later; you can also call read_tab with "${target.handle}"`, + `will WAKE you with a new message later; you can also call read_tab with "${target.handle}"`, "in a FUTURE turn to check. Keep working if you have other tasks; if you are ONLY", "waiting for this reply, end your turn now.", ] : [ "Do NOT sleep, poll, or run commands to wait for a reply. If the target replies it", - "arrives on its own as a new message later. Keep working if you have other tasks; if", + "will WAKE you with a new message later. Keep working if you have other tasks; if", "you are ONLY waiting for this reply, end your turn now.", ]; return [ diff --git a/packages/core/tests/tools/send-to-tab.test.ts b/packages/core/tests/tools/send-to-tab.test.ts index 48ff460..21d8032 100644 --- a/packages/core/tests/tools/send-to-tab.test.ts +++ b/packages/core/tests/tools/send-to-tab.test.ts @@ -38,8 +38,8 @@ describe("createSendToTabTool — schema & description", () => { it("never mentions read_tab in the description when canReadTab is false", () => { const tool = createSendToTabTool(makeCallbacks({ canReadTab: false })); expect(tool.description).not.toContain("read_tab"); - // Still tells the agent a reply arrives on its own + to end its turn. - expect(tool.description.toLowerCase()).toContain("arrives on its own"); + // Still tells the agent a reply will wake it + to end its turn. + expect(tool.description.toLowerCase()).toContain("wake you with a new message"); expect(tool.description.toLowerCase()).toContain("end your turn"); }); }); -- cgit v1.2.3 From 062d01bd2f5c3ab6de7747dc5028e66b81dac6f5 Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Tue, 2 Jun 2026 17:52:14 +0900 Subject: feat(lsp): add config-driven LSP support (Roblox Luau via luau-lsp) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add Language Server Protocol integration modeled on opencode's, wired for this codebase's plain-TypeScript tool/agent architecture. Core (@dispatch/core): - lsp/client.ts: LSP/JSON-RPC client over stdio (vscode-jsonrpc) with the initialize handshake, didOpen/didChange sync, push + pull diagnostics (textDocument/diagnostic, workspace/diagnostic), and a generic request() passthrough for hover/definition/references/documentSymbol. - lsp/server.ts: resolves dispatch.toml [lsp] entries into spawn specs. Config-driven only — no builtin registry, no auto-download. - lsp/manager.ts: process-wide LspManager owning client lifecycles, keyed by root+serverID, lazy spawn + reuse + graceful shutdown. - lsp/language.ts: extension->languageId map incl. .luau -> "luau". - lsp/diagnostic.ts: error-only block formatting (1-based). - tools/lsp.ts: on-demand 'lsp' tool (1-based coords -> 0-based wire). - write-file.ts: optional onAfterWrite hook for diagnostics-on-write. - config schema: validate [lsp] block; DispatchConfig.lsp + LspServerConfig. API (@dispatch/api): - AgentManager owns one LspManager; per-working-directory server cache cleared on config reload; diagnostics appended to write_file results; 'lsp' tool gated by new perm_lsp setting; shutdownAll on destroy(). Config: - dispatch.toml: documented, commented [lsp.luau-lsp] Roblox example. Tests: fake-lsp-server fixture + client/manager/server/diagnostic/schema/ tool/write-hook suites, plus an opt-in real-binary luau-lsp smoke test (auto-skipped when luau-lsp is absent). 652 pass; biome + 3 typechecks green. --- biome.json | 1 + bun.lock | 6 + dispatch.toml | 42 ++ packages/api/src/agent-manager.ts | 150 ++++- packages/api/tests/agent-manager.test.ts | 30 + packages/api/tests/routes.test.ts | 30 + packages/core/package.json | 2 + packages/core/src/config/schema.ts | 107 +++- packages/core/src/index.ts | 15 +- packages/core/src/lsp/client.ts | 658 +++++++++++++++++++++ packages/core/src/lsp/diagnostic.ts | 41 ++ packages/core/src/lsp/index.ts | 18 + packages/core/src/lsp/language.ts | 72 +++ packages/core/src/lsp/manager.ts | 220 +++++++ packages/core/src/lsp/server.ts | 68 +++ packages/core/src/tools/lsp.ts | 135 +++++ packages/core/src/tools/write-file.ts | 30 +- packages/core/src/types/index.ts | 49 ++ packages/core/tests/config/lsp-schema.test.ts | 110 ++++ packages/core/tests/fixture/lsp/fake-lsp-server.js | 195 ++++++ packages/core/tests/lsp/client.test.ts | 146 +++++ packages/core/tests/lsp/diagnostic.test.ts | 67 +++ packages/core/tests/lsp/luau-lsp.smoke.test.ts | 63 ++ packages/core/tests/lsp/manager.test.ts | 120 ++++ packages/core/tests/lsp/server.test.ts | 41 ++ packages/core/tests/tools/lsp-tool.test.ts | 110 ++++ packages/core/tests/tools/write-file.test.ts | 46 ++ 27 files changed, 2565 insertions(+), 7 deletions(-) create mode 100644 packages/core/src/lsp/client.ts create mode 100644 packages/core/src/lsp/diagnostic.ts create mode 100644 packages/core/src/lsp/index.ts create mode 100644 packages/core/src/lsp/language.ts create mode 100644 packages/core/src/lsp/manager.ts create mode 100644 packages/core/src/lsp/server.ts create mode 100644 packages/core/src/tools/lsp.ts create mode 100644 packages/core/tests/config/lsp-schema.test.ts create mode 100644 packages/core/tests/fixture/lsp/fake-lsp-server.js create mode 100644 packages/core/tests/lsp/client.test.ts create mode 100644 packages/core/tests/lsp/diagnostic.test.ts create mode 100644 packages/core/tests/lsp/luau-lsp.smoke.test.ts create mode 100644 packages/core/tests/lsp/manager.test.ts create mode 100644 packages/core/tests/lsp/server.test.ts create mode 100644 packages/core/tests/tools/lsp-tool.test.ts (limited to 'packages/api/src') diff --git a/biome.json b/biome.json index 408d2a0..af4f574 100644 --- a/biome.json +++ b/biome.json @@ -53,6 +53,7 @@ "!**/dist", "!**/build", "!references", + "!roblox-opencode-config-sample", "!packaging", "!**/release" ] diff --git a/bun.lock b/bun.lock index b6c5e6a..b92950e 100644 --- a/bun.lock +++ b/bun.lock @@ -31,6 +31,8 @@ "chokidar": "^5.0.0", "smol-toml": "^1.6.1", "tree-sitter-bash": "^0.25.1", + "vscode-jsonrpc": "8.2.1", + "vscode-languageserver-types": "3.17.5", "web-tree-sitter": "^0.26.8", "zod": "^3.23.0", "zod-to-json-schema": "^3.25.2", @@ -907,6 +909,10 @@ "vitest": ["vitest@3.2.4", "", { "dependencies": { "@types/chai": "^5.2.2", "@vitest/expect": "3.2.4", "@vitest/mocker": "3.2.4", "@vitest/pretty-format": "^3.2.4", "@vitest/runner": "3.2.4", "@vitest/snapshot": "3.2.4", "@vitest/spy": "3.2.4", "@vitest/utils": "3.2.4", "chai": "^5.2.0", "debug": "^4.4.1", "expect-type": "^1.2.1", "magic-string": "^0.30.17", "pathe": "^2.0.3", "picomatch": "^4.0.2", "std-env": "^3.9.0", "tinybench": "^2.9.0", "tinyexec": "^0.3.2", "tinyglobby": "^0.2.14", "tinypool": "^1.1.1", "tinyrainbow": "^2.0.0", "vite": "^5.0.0 || ^6.0.0 || ^7.0.0-0", "vite-node": "3.2.4", "why-is-node-running": "^2.3.0" }, "peerDependencies": { "@edge-runtime/vm": "*", "@types/debug": "^4.1.12", "@types/node": "^18.0.0 || ^20.0.0 || >=22.0.0", "@vitest/browser": "3.2.4", "@vitest/ui": "3.2.4", "happy-dom": "*", "jsdom": "*" }, "optionalPeers": ["@edge-runtime/vm", "@types/debug", "@types/node", "@vitest/browser", "@vitest/ui", "happy-dom", "jsdom"], "bin": { "vitest": "vitest.mjs" } }, "sha512-LUCP5ev3GURDysTWiP47wRRUpLKMOfPh+yKTx3kVIEiu5KOMeqzpnYNsKyOoVrULivR8tLcks4+lga33Whn90A=="], + "vscode-jsonrpc": ["vscode-jsonrpc@8.2.1", "", {}, "sha512-kdjOSJ2lLIn7r1rtrMbbNCHjyMPfRnowdKjBQ+mGq6NAW5QY2bEZC/khaC5OR8svbbjvLEaIXkOq45e2X9BIbQ=="], + + "vscode-languageserver-types": ["vscode-languageserver-types@3.17.5", "", {}, "sha512-Ld1VelNuX9pdF39h2Hgaeb5hEZM2Z3jUrrMgWQAu82jMtZp7p3vJT3BzToKtZI7NgQssZje5o0zryOrhQvzQAg=="], + "web-tree-sitter": ["web-tree-sitter@0.26.8", "", {}, "sha512-4sUwi7ZyOrIk5KLgYLkc2A/F0LFMQnBhfb+2Cdl7ik4ePJ6JD+fk4ofI2sA5eGawBKBaK4Vntt7Ww5KcEsay4A=="], "which": ["which@5.0.0", "", { "dependencies": { "isexe": "^3.1.1" }, "bin": { "node-which": "bin/which.js" } }, "sha512-JEdGzHwwkrbWoGOlIHqQ5gtprKGOenpDHpxE9zVR1bWbOtYRyPPHMe9FaP6x61CmNaTThSkb0DAJte5jD+DmzQ=="], diff --git a/dispatch.toml b/dispatch.toml index 43e164a..9f09ef7 100644 --- a/dispatch.toml +++ b/dispatch.toml @@ -49,3 +49,45 @@ read = "allow" [permissions.external_directory] "~/*" = "ask" "/tmp/*" = "allow" + +# ─── Language Servers (LSP) ────────────────────────────────────── +# Optional. Declare LSP servers to give agents diagnostics (and, with the +# `lsp` tool, hover/definition/references) for the files they edit. This block +# is PROJECT-SCOPED: it is read from the `dispatch.toml` in a tab's effective +# working directory (re-consulted when you change the CWD). Config-driven only — +# there is no builtin server registry and no auto-download, so the executable +# in `command[0]` must already be on PATH. +# +# After `write_file` edits a file whose extension matches a server below, +# dispatch opens it through the server and appends any error diagnostics to the +# tool result ("LSP errors detected in this file, please fix: ..."). Grant the +# `perm_lsp` permission to also expose the on-demand `lsp` tool. +# +# The example below is the Roblox Luau setup using luau-lsp +# (https://github.com/JohnnyMorganz/luau-lsp). Uncomment and adapt for your +# project. luau-lsp's `sourcemap.autogenerate` makes luau-lsp run +# `rojo sourcemap --watch` itself, so `rojo` must be on PATH (or set +# `[lsp.luau-lsp.env]` PATH / luau-lsp's sourcemap.rojoPath accordingly). +# +# [lsp.luau-lsp] +# command = ["luau-lsp", "lsp", "--definitions=globalTypes.d.luau", "--docs=api-docs.json"] +# extensions = [".luau"] +# +# [lsp.luau-lsp.initialization.luau-lsp.platform] +# type = "roblox" +# +# [lsp.luau-lsp.initialization.luau-lsp.sourcemap] +# enabled = true +# autogenerate = true +# rojoProjectFile = "default.project.json" +# +# [lsp.luau-lsp.initialization.luau-lsp.types] +# roblox = true +# definitionFiles = ["globalTypes.d.luau"] +# documentationFiles = ["api-docs.json"] +# +# [lsp.luau-lsp.initialization.luau-lsp.diagnostics] +# strictDatamodelTypes = false +# +# [lsp.luau-lsp.initialization.luau-lsp.completion.imports] +# useConst = true diff --git a/packages/api/src/agent-manager.ts b/packages/api/src/agent-manager.ts index 2795a6c..7af2b67 100644 --- a/packages/api/src/agent-manager.ts +++ b/packages/api/src/agent-manager.ts @@ -14,6 +14,7 @@ import { configToRuleset, createConfigWatcher, createListFilesTool, + createLspTool, createReadFileSliceTool, createReadFileTool, createReadTabTool, @@ -37,6 +38,7 @@ import { getSetting, getTab, getUsageStatsForTab, + LspManager, listOpenTabs, loadAgent, loadAgents, @@ -45,9 +47,12 @@ import { ModelRegistry, type QueuedMessage, type ReasoningEffort, + type ResolvedLspServer, refreshAccountCredentials, refreshAccountCredentialsAsync, + reportDiagnostics, resolveApiKey, + resolveServersFromConfig, resolveTabPrefix, type SkillDefinition, type SystemChunkKind, @@ -87,6 +92,7 @@ const TOOL_DESCRIPTIONS: Record = { "Send a message to another tab (agent) by its short ID, as shown in the tab bar. Fire-and-forget: it queues/wakes the target and returns immediately without waiting for a reply. Do NOT sleep, poll, or run commands to wait — if the target replies it will wake you with a new message in a later turn; if you are only waiting, end your turn.", read_tab: "Read another tab (agent)'s most recent completed response by its short ID. Returns a non-blocking snapshot; if the target is still running you get its previous completed turn. Use after send_to_tab to collect a reply.", + lsp: "Query the configured Language Server (e.g. luau-lsp for Roblox Luau) about a file: diagnostics, hover, definition, references, or documentSymbol. Line/character are 1-based.", }; /** @@ -99,6 +105,14 @@ const TOOL_DESCRIPTIONS: Record = { */ const MAX_AGENT_AUTO_WAKES = 6; +/** + * Cap on how many OTHER files' LSP error blocks are appended to a write_file + * result, after the written file's own errors. Bounds context spend when a + * single edit surfaces project-wide diagnostics. Mirrors opencode's + * MAX_PROJECT_DIAGNOSTICS_FILES. + */ +const MAX_LSP_OTHER_FILE_DIAGNOSTICS = 5; + const DEFAULT_SYSTEM_PROMPT = "You are Dispatch, an agent designed to help with any task that the user asks for. Be helpful and concise."; @@ -251,6 +265,21 @@ export class AgentManager { private claudeAccounts: ClaudeAccount[] = []; + /** + * Process-wide owner of LSP client lifecycles. Servers are declared in the + * `dispatch.toml` of a tab's effective working directory; clients are + * spawned lazily per (root + server) and reused across tabs/turns. Shut + * down in `destroy()`. + */ + private lspManager: LspManager = new LspManager(); + /** + * Cache of resolved LSP servers per working directory, so we parse each + * directory's `dispatch.toml` `[lsp]` block once. Cleared wholesale on any + * config hot-reload (the watcher fires for the root config; directory-level + * configs are re-read on demand after a clear). + */ + private lspServersByDir: Map = new Map(); + constructor(permissionManager?: PermissionManager) { this.permissionManager = permissionManager; @@ -292,6 +321,10 @@ export class AgentManager { } // Update model registry with new config this._initModelRegistry(newConfig); + // LSP server config may have changed — drop the per-directory cache + // so the next tool build re-reads each working directory's + // `dispatch.toml` `[lsp]` block. + this.lspServersByDir.clear(); // Re-discover Claude accounts: a config reload may accompany freshly // imported credentials, and (critically) lets a process that failed // account discovery at boot recover without a full restart. @@ -334,6 +367,77 @@ export class AgentManager { } } + /** + * Resolve (and cache) the LSP servers configured for a working directory. + * + * LSP config is project-scoped: it lives in the `dispatch.toml` of the + * tab's effective working directory, NOT the global config. We read that + * directory's config once and cache the resolved servers; the cache is + * cleared on config hot-reload. Returns `[]` when the directory has no + * `[lsp]` block (the common case). + */ + private getLspServersForDir(dir: string): ResolvedLspServer[] { + const cached = this.lspServersByDir.get(dir); + if (cached) return cached; + let servers: ResolvedLspServer[] = []; + try { + const dirConfig = loadConfig(dir); + servers = resolveServersFromConfig(dirConfig.lsp); + } catch (err) { + console.warn( + `dispatch: failed to load LSP config for ${dir}: ${err instanceof Error ? err.message : String(err)}`, + ); + servers = []; + } + this.lspServersByDir.set(dir, servers); + return servers; + } + + /** + * Build the `onAfterWrite` hook for `createWriteFileTool` when the tab's + * working directory has LSP servers configured. The hook touches the + * just-written file through the LSP and returns a formatted diagnostics + * block (the written file's errors first, then a small cap of other-file + * errors) — opencode's diagnostics-on-write pattern. Returns `undefined` + * when no server matches, so writes stay zero-overhead for non-LSP files. + */ + private buildAfterWriteHook( + workingDirectory: string, + servers: ResolvedLspServer[], + ): ((absolutePath: string) => Promise) | undefined { + if (servers.length === 0) return undefined; + const manager = this.lspManager; + return async (absolutePath: string): Promise => { + if (!manager.hasServerForFile(absolutePath, servers)) return ""; + await manager.touchFile({ + file: absolutePath, + root: workingDirectory, + servers, + mode: "document", + }); + const diagnostics = manager.getDiagnostics({ + root: workingDirectory, + servers, + file: absolutePath, + }); + let output = ""; + let otherFileCount = 0; + for (const [file, issues] of Object.entries(diagnostics)) { + const current = file === absolutePath; + if (!current && otherFileCount >= MAX_LSP_OTHER_FILE_DIAGNOSTICS) continue; + const block = reportDiagnostics(file, issues); + if (!block) continue; + if (current) { + output += `${output ? "\n\n" : ""}LSP errors detected in this file, please fix:\n${block}`; + } else { + otherFileCount++; + output += `${output ? "\n\n" : ""}LSP errors detected in other files:\n${block}`; + } + } + return output; + }; + } + private _initModelRegistry(config: DispatchConfig): void { if (config.keys) { if (this.modelRegistry) { @@ -408,8 +512,9 @@ export class AgentManager { const permReadTab = getSetting("perm_read_tab") === "allow"; const permWebSearch = getSetting("perm_web_search") === "allow"; const permYoutubeTranscribe = getSetting("perm_youtube_transcribe") === "allow"; + const permLsp = getSetting("perm_lsp") === "allow"; const sysPrompt = getSetting("system_prompt") ?? ""; - const permKey = `${permRead}:${permEdit}:${permBash}:${permSummon}:${permUserAgent}:${permSendToTab}:${permReadTab}:${permWebSearch}:${permYoutubeTranscribe}:${sysPrompt}`; + const permKey = `${permRead}:${permEdit}:${permBash}:${permSummon}:${permUserAgent}:${permSendToTab}:${permReadTab}:${permWebSearch}:${permYoutubeTranscribe}:${permLsp}:${sysPrompt}`; // If the override differs or permissions changed, invalidate the cached agent if ( @@ -451,6 +556,12 @@ export class AgentManager { // Ignore — tool execution will surface the error naturally } + // Resolve LSP servers for this working directory once (cached). + // Drives both diagnostics-on-write (the write_file hook) and the + // optional `lsp` tool. Empty for directories with no `[lsp]` block. + const lspServers = this.getLspServersForDir(workingDirectory); + const afterWriteHook = this.buildAfterWriteHook(workingDirectory, lspServers); + // Build tools list — child agents use their toolsOverride whitelist, // parent agents use permission settings from DB const toolEntries: Array<{ name: string; tool: ReturnType }> = []; @@ -475,7 +586,10 @@ export class AgentManager { toolEntries.push({ name: "list_files", tool: createListFilesTool(workingDirectory) }); } if (allowed.has("write_file")) { - toolEntries.push({ name: "write_file", tool: createWriteFileTool(workingDirectory) }); + toolEntries.push({ + name: "write_file", + tool: createWriteFileTool(workingDirectory, afterWriteHook), + }); } if (allowed.has("run_shell")) { toolEntries.push({ @@ -486,6 +600,16 @@ export class AgentManager { if (allowed.has("web_search")) { toolEntries.push({ name: "web_search", tool: createWebSearchTool() }); } + if (allowed.has("lsp") && lspServers.length > 0) { + toolEntries.push({ + name: "lsp", + tool: createLspTool(() => ({ + manager: this.lspManager, + workingDirectory, + servers: lspServers, + })), + }); + } if (allowed.has("youtube_transcribe")) { toolEntries.push({ name: "youtube_transcribe", @@ -561,7 +685,10 @@ export class AgentManager { toolEntries.push({ name: "list_files", tool: createListFilesTool(workingDirectory) }); } if (permEdit) { - toolEntries.push({ name: "write_file", tool: createWriteFileTool(workingDirectory) }); + toolEntries.push({ + name: "write_file", + tool: createWriteFileTool(workingDirectory, afterWriteHook), + }); } if (permBash) { toolEntries.push({ @@ -572,6 +699,19 @@ export class AgentManager { if (permWebSearch) { toolEntries.push({ name: "web_search", tool: createWebSearchTool() }); } + // The `lsp` tool exposes diagnostics + navigation on demand. It is + // gated by `perm_lsp` AND requires at least one server configured + // in the working directory's `dispatch.toml`. + if (permLsp && lspServers.length > 0) { + toolEntries.push({ + name: "lsp", + tool: createLspTool(() => ({ + manager: this.lspManager, + workingDirectory, + servers: lspServers, + })), + }); + } if (permYoutubeTranscribe) { toolEntries.push({ name: "youtube_transcribe", @@ -1858,5 +1998,9 @@ export class AgentManager { destroy(): void { this.configWatcher?.close(); this.skillsWatcher?.close(); + // Shut down all long-lived LSP server processes. Fire-and-forget: the + // promise is detached so `destroy()` stays synchronous (matching its + // existing contract), but every client gets `shutdown()` called. + void this.lspManager.shutdownAll(); } } diff --git a/packages/api/tests/agent-manager.test.ts b/packages/api/tests/agent-manager.test.ts index 3353aff..6efe15e 100644 --- a/packages/api/tests/agent-manager.test.ts +++ b/packages/api/tests/agent-manager.test.ts @@ -228,6 +228,36 @@ vi.mock("@dispatch/core", () => ({ execute: async () => ["file1.ts"], }; }, + createLspTool(_getContext: unknown): ToolDefinition { + return { + name: "lsp", + description: "query the language server", + parameters: { _type: "z.ZodObject", shape: {} } as unknown as ToolDefinition["parameters"], + execute: async () => "mock lsp", + }; + }, + LspManager: class MockLspManager { + hasServerForFile() { + return false; + } + async getClients() { + return []; + } + async touchFile() {} + getDiagnostics() { + return {}; + } + async request() { + return []; + } + async shutdownAll() {} + }, + resolveServersFromConfig(_lsp: unknown) { + return []; + }, + reportDiagnostics(_file: string, _issues: unknown) { + return ""; + }, createRunShellTool(_wd: string): ToolDefinition { return { name: "run_shell", diff --git a/packages/api/tests/routes.test.ts b/packages/api/tests/routes.test.ts index a8db5ce..6ec8ca6 100644 --- a/packages/api/tests/routes.test.ts +++ b/packages/api/tests/routes.test.ts @@ -82,6 +82,36 @@ vi.mock("@dispatch/core", () => ({ execute: async () => ["file1.ts"], }; }, + createLspTool(_getContext: unknown): ToolDefinition { + return { + name: "lsp", + description: "query the language server", + parameters: { _type: "z.ZodObject", shape: {} } as unknown as ToolDefinition["parameters"], + execute: async () => "mock lsp", + }; + }, + LspManager: class MockLspManager { + hasServerForFile() { + return false; + } + async getClients() { + return []; + } + async touchFile() {} + getDiagnostics() { + return {}; + } + async request() { + return []; + } + async shutdownAll() {} + }, + resolveServersFromConfig(_lsp: unknown) { + return []; + }, + reportDiagnostics(_file: string, _issues: unknown) { + return ""; + }, createRunShellTool(_wd: string): ToolDefinition { return { name: "run_shell", diff --git a/packages/core/package.json b/packages/core/package.json index 3ca568b..55dff0f 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -17,6 +17,8 @@ "chokidar": "^5.0.0", "smol-toml": "^1.6.1", "tree-sitter-bash": "^0.25.1", + "vscode-jsonrpc": "8.2.1", + "vscode-languageserver-types": "3.17.5", "web-tree-sitter": "^0.26.8", "zod": "^3.23.0", "zod-to-json-schema": "^3.25.2" diff --git a/packages/core/src/config/schema.ts b/packages/core/src/config/schema.ts index a459a4d..304ee10 100644 --- a/packages/core/src/config/schema.ts +++ b/packages/core/src/config/schema.ts @@ -1,4 +1,9 @@ -import type { ConfigError, DispatchConfig, KeyDefinition } from "../types/index.js"; +import type { + ConfigError, + DispatchConfig, + KeyDefinition, + LspServerConfig, +} from "../types/index.js"; function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); @@ -100,6 +105,99 @@ function validateKey(raw: unknown, path: string, errors: ConfigError[]): KeyDefi }; } +function isStringArray(value: unknown): value is string[] { + return Array.isArray(value) && value.every((v) => typeof v === "string"); +} + +function validateLspServer( + raw: unknown, + path: string, + errors: ConfigError[], +): LspServerConfig | null { + if (!isRecord(raw)) { + errors.push({ path, message: "must be an object" }); + return null; + } + + const disabled = raw.disabled === true; + + // `command` is required and must be a non-empty string array unless the + // entry is explicitly disabled (a disabled entry is skipped wholesale). + if (!disabled) { + if (!isStringArray(raw.command) || raw.command.length === 0) { + errors.push({ + path: `${path}.command`, + message: "must be a non-empty array of strings", + }); + return null; + } + // `extensions` is required for custom servers — without it the client + // cannot know which files should activate the server. + if (!isStringArray(raw.extensions) || raw.extensions.length === 0) { + errors.push({ + path: `${path}.extensions`, + message: 'must be a non-empty array of strings (e.g. [".luau"])', + }); + return null; + } + } else { + // Disabled entries still must not carry a malformed command/extensions + // if present, but we do not require them. + if (raw.command !== undefined && !isStringArray(raw.command)) { + errors.push({ path: `${path}.command`, message: "must be an array of strings" }); + return null; + } + if (raw.extensions !== undefined && !isStringArray(raw.extensions)) { + errors.push({ path: `${path}.extensions`, message: "must be an array of strings" }); + return null; + } + } + + if (raw.env !== undefined && !isStringRecord(raw.env)) { + errors.push({ + path: `${path}.env`, + message: "must be a flat string-keyed object", + }); + return null; + } + + if (raw.initialization !== undefined && !isRecord(raw.initialization)) { + errors.push({ + path: `${path}.initialization`, + message: "must be an object", + }); + return null; + } + + const server: LspServerConfig = { + command: (raw.command as string[] | undefined) ?? [], + extensions: (raw.extensions as string[] | undefined) ?? [], + ...(isStringRecord(raw.env) ? { env: raw.env } : {}), + ...(isRecord(raw.initialization) + ? { initialization: raw.initialization as Record } + : {}), + ...(disabled ? { disabled: true } : {}), + }; + return server; +} + +function validateLsp( + raw: unknown, + path: string, + errors: ConfigError[], +): Record | undefined { + if (!isRecord(raw)) { + errors.push({ path, message: "must be an object" }); + return undefined; + } + const result: Record = {}; + for (const [id, value] of Object.entries(raw)) { + const server = validateLspServer(value, `${path}.${id}`, errors); + if (server) result[id] = server; + } + return Object.keys(result).length > 0 ? result : undefined; +} + export function validateConfig(raw: unknown): { config: DispatchConfig; errors: ConfigError[] } { const errors: ConfigError[] = []; @@ -125,9 +223,16 @@ export function validateConfig(raw: unknown): { config: DispatchConfig; errors: } } + // lsp (optional) + let lsp: Record | undefined; + if (raw.lsp !== undefined) { + lsp = validateLsp(raw.lsp, "lsp", errors); + } + const config: DispatchConfig = { permissions, ...(keys !== undefined && { keys }), + ...(lsp !== undefined && { lsp }), }; return { config, errors }; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 8334102..8608b6a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -68,6 +68,18 @@ export { logStreamEvent, } from "./llm/debug-logger.js"; export { createProvider } from "./llm/provider.js"; +// LSP (Language Server Protocol) +export { + createLspClient, + type Diagnostic as LspDiagnostic, + type LspClient, + LspManager, + type LspServerHandle, + pretty as prettyDiagnostic, + type ResolvedLspServer, + report as reportDiagnostics, + resolveServersFromConfig, +} from "./lsp/index.js"; // Models export { getModelsCatalog, @@ -88,6 +100,7 @@ export { export { prefix as bashArityPrefix } from "./tools/bash-arity.js"; // Tools export { createListFilesTool } from "./tools/list-files.js"; +export { createLspTool, type LspToolContext } from "./tools/lsp.js"; export { createReadFileTool } from "./tools/read-file.js"; export { createReadFileSliceTool } from "./tools/read-file-slice.js"; export { createReadTabTool, type ReadTabCallbacks } from "./tools/read-tab.js"; @@ -111,7 +124,7 @@ export { export { createTaskListTool, TaskList, TODO_DESCRIPTION } from "./tools/task-list.js"; export { clearSpillForTab } from "./tools/truncate.js"; export { createWebSearchTool } from "./tools/web-search.js"; -export { createWriteFileTool } from "./tools/write-file.js"; +export { type AfterWriteHook, createWriteFileTool } from "./tools/write-file.js"; export { BackgroundTranscriptStore, createYoutubeTranscribeTool, diff --git a/packages/core/src/lsp/client.ts b/packages/core/src/lsp/client.ts new file mode 100644 index 0000000..da0c916 --- /dev/null +++ b/packages/core/src/lsp/client.ts @@ -0,0 +1,658 @@ +import type { ChildProcessWithoutNullStreams } from "node:child_process"; +import { readFile } from "node:fs/promises"; +import { extname, isAbsolute, resolve } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { + createMessageConnection, + type MessageConnection, + StreamMessageReader, + StreamMessageWriter, +} from "vscode-jsonrpc/node"; +import type { Diagnostic } from "vscode-languageserver-types"; +import { languageIdForExtension } from "./language.js"; + +export type { Diagnostic } from "vscode-languageserver-types"; + +// ─── Timing constants (mirrors opencode) ───────────────────────── +const DIAGNOSTICS_DEBOUNCE_MS = 150; +const DIAGNOSTICS_DOCUMENT_WAIT_TIMEOUT_MS = 5_000; +const DIAGNOSTICS_FULL_WAIT_TIMEOUT_MS = 10_000; +const DIAGNOSTICS_REQUEST_TIMEOUT_MS = 3_000; +const INITIALIZE_TIMEOUT_MS = 45_000; + +// ─── LSP spec constants ────────────────────────────────────────── +const FILE_CHANGE_CREATED = 1; +const FILE_CHANGE_CHANGED = 2; +const TEXT_DOCUMENT_SYNC_INCREMENTAL = 2; + +/** + * A live spawned language-server process plus the `initializationOptions` to + * hand it. Produced by the server-spawning layer (`server.ts`) and consumed by + * `createLspClient`. + */ +export interface LspServerHandle { + process: ChildProcessWithoutNullStreams; + initialization?: Record; +} + +interface ServerCapabilities { + textDocumentSync?: number | { change?: number }; + diagnosticProvider?: unknown; + [key: string]: unknown; +} + +interface DiagnosticRequestResult { + handled: boolean; + matched: boolean; + byFile: Map; +} + +interface CapabilityRegistration { + id: string; + method: string; + registerOptions?: { + identifier?: string; + workspaceDiagnostics?: boolean; + }; +} + +type DocumentDiagnosticReport = { + items?: Diagnostic[]; + relatedDocuments?: Record; +}; + +type WorkspaceDiagnosticReport = { + items?: { uri?: string; items?: Diagnostic[] }[]; +}; + +/** Public shape of a connected LSP client. */ +export interface LspClient { + readonly serverID: string; + readonly root: string; + readonly connection: MessageConnection; + /** + * Open (or re-sync) a file with the server. Returns the document version + * sent — pass it to `waitForDiagnostics` to wait for diagnostics matching + * this exact sync. + */ + notifyOpen(path: string): Promise; + /** Snapshot of all known diagnostics keyed by absolute file path. */ + readonly diagnostics: Map; + /** Wait until diagnostics for `path` settle (push and/or pull). */ + waitForDiagnostics(request: { + path: string; + version: number; + mode?: "document" | "full"; + after?: number; + }): Promise; + /** Generic LSP request passthrough (hover, definition, references, …). */ + request(method: string, params: unknown): Promise; + /** Shut the connection and child process down. */ + shutdown(): Promise; +} + +function withTimeout(promise: Promise, ms: number): Promise { + return new Promise((resolvePromise, reject) => { + const timer = setTimeout(() => reject(new Error(`LSP request timed out after ${ms}ms`)), ms); + promise.then( + (value) => { + clearTimeout(timer); + resolvePromise(value); + }, + (err) => { + clearTimeout(timer); + reject(err); + }, + ); + }); +} + +function getFilePath(uri: string): string | undefined { + if (!uri.startsWith("file://")) return undefined; + return fileURLToPath(uri); +} + +function getSyncKind(capabilities?: ServerCapabilities): number | undefined { + if (!capabilities) return undefined; + const sync = capabilities.textDocumentSync; + if (typeof sync === "number") return sync; + return sync?.change; +} + +function endPosition(text: string) { + const lines = text.split(/\r\n|\r|\n/); + return { line: lines.length - 1, character: lines.at(-1)?.length ?? 0 }; +} + +function dedupeDiagnostics(items: Diagnostic[]): Diagnostic[] { + const seen = new Set(); + return items.filter((item) => { + const key = JSON.stringify({ + code: item.code, + severity: item.severity, + message: item.message, + source: item.source, + range: item.range, + }); + if (seen.has(key)) return false; + seen.add(key); + return true; + }); +} + +function configurationValue(settings: unknown, section?: string): unknown { + if (!section) return settings ?? null; + const result = section.split(".").reduce((acc, key) => { + if (!acc || typeof acc !== "object" || !(key in acc)) return undefined; + return (acc as Record)[key]; + }, settings); + return result ?? null; +} + +/** + * Create and initialize an LSP client over a spawned server's stdio. + * + * Performs the full `initialize`/`initialized` handshake (with a 45s timeout), + * wires push (`textDocument/publishDiagnostics`) and pull + * (`textDocument/diagnostic`, `workspace/diagnostic`) diagnostics, answers the + * `workspace/configuration`, `workspaceFolders`, and capability-registration + * requests servers commonly make, and returns a small client surface used by + * the manager and tools. Plain-TypeScript port of opencode's `lsp/client.ts`. + */ +export async function createLspClient(input: { + serverID: string; + server: LspServerHandle; + root: string; + directory: string; +}): Promise { + const { serverID, server, root, directory } = input; + + const connection = createMessageConnection( + new StreamMessageReader(server.process.stdout), + new StreamMessageWriter(server.process.stdin), + ); + + // Server stderr is routine for many tools (luau-lsp logs sourcemap status + // there). Keep it quiet unless debugging. + server.process.stderr?.on("data", () => { + /* swallowed — see opencode: stderr is mostly informational */ + }); + + // ─── Connection state ─── + const pushDiagnostics = new Map(); + const pullDiagnostics = new Map(); + const published = new Map(); + const diagnosticRegistrations = new Map(); + const registrationListeners = new Set<() => void>(); + const diagnosticListeners = new Set<(input: { path: string; serverID: string }) => void>(); + const files: Record = {}; + + const mergedDiagnostics = (filePath: string) => + dedupeDiagnostics([ + ...(pushDiagnostics.get(filePath) ?? []), + ...(pullDiagnostics.get(filePath) ?? []), + ]); + const updatePushDiagnostics = (filePath: string, next: Diagnostic[]) => { + pushDiagnostics.set(filePath, next); + for (const listener of diagnosticListeners) listener({ path: filePath, serverID }); + }; + const updatePullDiagnostics = (filePath: string, next: Diagnostic[]) => { + pullDiagnostics.set(filePath, next); + }; + const emitRegistrationChange = () => { + for (const listener of [...registrationListeners]) listener(); + }; + + // ─── Notification / request handlers ─── + connection.onNotification( + "textDocument/publishDiagnostics", + (params: { uri: string; diagnostics: Diagnostic[]; version?: number }) => { + const filePath = getFilePath(params.uri); + if (!filePath) return; + published.set(filePath, { + at: Date.now(), + version: typeof params.version === "number" ? params.version : undefined, + }); + updatePushDiagnostics(filePath, params.diagnostics); + }, + ); + connection.onRequest("window/workDoneProgress/create", () => null); + connection.onRequest("workspace/configuration", (params: { items?: { section?: string }[] }) => { + const items = params.items ?? []; + return items.map((item) => configurationValue(server.initialization, item.section)); + }); + connection.onRequest( + "client/registerCapability", + (params: { registrations?: CapabilityRegistration[] }) => { + const registrations = params.registrations ?? []; + let changed = false; + for (const registration of registrations) { + if (registration.method !== "textDocument/diagnostic") continue; + diagnosticRegistrations.set(registration.id, registration); + changed = true; + } + if (changed) emitRegistrationChange(); + return null; + }, + ); + connection.onRequest( + "client/unregisterCapability", + (params: { unregisterations?: { id: string; method: string }[] }) => { + const registrations = params.unregisterations ?? []; + let changed = false; + for (const registration of registrations) { + if (registration.method !== "textDocument/diagnostic") continue; + diagnosticRegistrations.delete(registration.id); + changed = true; + } + if (changed) emitRegistrationChange(); + return null; + }, + ); + connection.onRequest("workspace/workspaceFolders", () => [ + { name: "workspace", uri: pathToFileURL(root).href }, + ]); + connection.onRequest("workspace/diagnostic/refresh", () => null); + connection.listen(); + + // ─── Initialize handshake ─── + const initialized = await withTimeout( + connection.sendRequest<{ capabilities?: ServerCapabilities }>("initialize", { + rootUri: pathToFileURL(root).href, + processId: server.process.pid ?? null, + workspaceFolders: [{ name: "workspace", uri: pathToFileURL(root).href }], + initializationOptions: { ...server.initialization }, + capabilities: { + window: { workDoneProgress: true }, + workspace: { + configuration: true, + didChangeWatchedFiles: { dynamicRegistration: true }, + diagnostics: { refreshSupport: false }, + }, + textDocument: { + synchronization: { didOpen: true, didChange: true }, + diagnostic: { dynamicRegistration: true, relatedDocumentSupport: true }, + publishDiagnostics: { versionSupport: false }, + }, + }, + }), + INITIALIZE_TIMEOUT_MS, + ); + + const syncKind = getSyncKind(initialized.capabilities); + const hasStaticPullDiagnostics = Boolean(initialized.capabilities?.diagnosticProvider); + + await connection.sendNotification("initialized", {}); + if (server.initialization) { + await connection.sendNotification("workspace/didChangeConfiguration", { + settings: server.initialization, + }); + } + + // ─── Pull-diagnostics helpers ─── + const mergeResults = (filePath: string, results: DiagnosticRequestResult[]) => { + const handled = results.some((r) => r.handled); + const matched = results.some((r) => r.matched); + if (!handled) return { handled: false, matched: false }; + + const merged = new Map(); + for (const result of results) { + for (const [target, items] of result.byFile.entries()) { + merged.set(target, (merged.get(target) ?? []).concat(items)); + } + } + if (matched && !merged.has(filePath)) merged.set(filePath, []); + for (const [target, items] of merged.entries()) { + updatePullDiagnostics(target, dedupeDiagnostics(items)); + } + return { handled, matched }; + }; + + async function requestDiagnosticReport( + filePath: string, + identifier?: string, + ): Promise { + const report = await withTimeout( + connection.sendRequest("textDocument/diagnostic", { + ...(identifier ? { identifier } : {}), + textDocument: { uri: pathToFileURL(filePath).href }, + }), + DIAGNOSTICS_REQUEST_TIMEOUT_MS, + ).catch(() => null); + const empty: DiagnosticRequestResult = { + handled: false, + matched: false, + byFile: new Map(), + }; + if (!report) return empty; + + const byFile = new Map(); + const push = (target: string, items: Diagnostic[]) => { + byFile.set(target, (byFile.get(target) ?? []).concat(items)); + }; + let handled = false; + let matched = false; + if (Array.isArray(report.items)) { + push(filePath, report.items); + handled = true; + matched = true; + } + for (const [uri, related] of Object.entries(report.relatedDocuments ?? {})) { + const relatedPath = getFilePath(uri); + if (!relatedPath || !Array.isArray(related.items)) continue; + push(relatedPath, related.items); + handled = true; + matched = matched || relatedPath === filePath; + } + return { handled, matched, byFile }; + } + + async function requestWorkspaceDiagnosticReport( + filePath: string, + identifier?: string, + ): Promise { + const report = await withTimeout( + connection.sendRequest("workspace/diagnostic", { + ...(identifier ? { identifier } : {}), + previousResultIds: [], + }), + DIAGNOSTICS_REQUEST_TIMEOUT_MS, + ).catch(() => null); + if (!report) return { handled: false, matched: false, byFile: new Map() }; + + const byFile = new Map(); + let matched = false; + for (const item of report.items ?? []) { + const relatedPath = item.uri ? getFilePath(item.uri) : undefined; + if (!relatedPath || !Array.isArray(item.items)) continue; + byFile.set(relatedPath, (byFile.get(relatedPath) ?? []).concat(item.items)); + matched = matched || relatedPath === filePath; + } + return { handled: true, matched, byFile }; + } + + function documentPullState() { + const documentRegistrations = [...diagnosticRegistrations.values()].filter( + (r) => r.registerOptions?.workspaceDiagnostics !== true, + ); + return { + documentIdentifiers: [ + ...new Set(documentRegistrations.flatMap((r) => r.registerOptions?.identifier ?? [])), + ], + supported: hasStaticPullDiagnostics || documentRegistrations.length > 0, + }; + } + + function workspacePullState() { + const workspaceRegistrations = [...diagnosticRegistrations.values()].filter( + (r) => r.registerOptions?.workspaceDiagnostics === true, + ); + return { + workspaceIdentifiers: [ + ...new Set(workspaceRegistrations.flatMap((r) => r.registerOptions?.identifier ?? [])), + ], + supported: workspaceRegistrations.length > 0, + }; + } + + const hasCurrentFileDiagnostics = (filePath: string, results: DiagnosticRequestResult[]) => + results.some((r) => (r.byFile.get(filePath)?.length ?? 0) > 0); + + async function requestDiagnostics( + filePath: string, + requests: Promise[], + done: (results: DiagnosticRequestResult[]) => boolean, + ) { + if (!requests.length) return { handled: false, matched: false }; + const results: DiagnosticRequestResult[] = []; + return new Promise<{ handled: boolean; matched: boolean }>((resolvePromise) => { + let pending = requests.length; + let resolved = false; + const finish = (merged: { handled: boolean; matched: boolean }, force = false) => { + if (resolved) return; + if (!force && !done(results)) return; + resolved = true; + resolvePromise(merged); + }; + for (const request of requests) { + request.then((result) => { + results.push(result); + pending -= 1; + const merged = mergeResults(filePath, results); + finish(merged); + if (pending === 0) finish(merged, true); + }); + } + }); + } + + async function requestDocumentDiagnostics(filePath: string) { + const state = documentPullState(); + if (!state.supported) return { handled: false, matched: false }; + return requestDiagnostics( + filePath, + [ + requestDiagnosticReport(filePath), + ...state.documentIdentifiers.map((id) => requestDiagnosticReport(filePath, id)), + ], + (results) => hasCurrentFileDiagnostics(filePath, results), + ); + } + + async function requestFullDiagnostics(filePath: string) { + const documentState = documentPullState(); + const workspaceState = workspacePullState(); + if (!documentState.supported && !workspaceState.supported) { + return { handled: false, matched: false }; + } + return mergeResults( + filePath, + await Promise.all([ + ...(documentState.supported ? [requestDiagnosticReport(filePath)] : []), + ...documentState.documentIdentifiers.map((id) => requestDiagnosticReport(filePath, id)), + ...(workspaceState.supported ? [requestWorkspaceDiagnosticReport(filePath)] : []), + ...workspaceState.workspaceIdentifiers.map((id) => + requestWorkspaceDiagnosticReport(filePath, id), + ), + ]), + ); + } + + function waitForRegistrationChange(timeout: number) { + if (timeout <= 0) return Promise.resolve(false); + return new Promise((resolvePromise) => { + let finished = false; + let timer: ReturnType | undefined; + const finish = (result: boolean) => { + if (finished) return; + finished = true; + if (timer) clearTimeout(timer); + registrationListeners.delete(listener); + resolvePromise(result); + }; + const listener = () => finish(true); + registrationListeners.add(listener); + timer = setTimeout(() => finish(false), timeout); + }); + } + + function waitForFreshPush(request: { + path: string; + version: number; + after: number; + timeout: number; + }) { + if (request.timeout <= 0) return Promise.resolve(false); + return new Promise((resolvePromise) => { + let finished = false; + let debounceTimer: ReturnType | undefined; + let timeoutTimer: ReturnType | undefined; + let unsub: (() => void) | undefined; + const finish = (result: boolean) => { + if (finished) return; + finished = true; + if (debounceTimer) clearTimeout(debounceTimer); + if (timeoutTimer) clearTimeout(timeoutTimer); + unsub?.(); + resolvePromise(result); + }; + const schedule = () => { + const hit = published.get(request.path); + if (!hit) return; + if (typeof hit.version === "number" && hit.version !== request.version) return; + if (hit.at < request.after && hit.version !== request.version) return; + if (debounceTimer) clearTimeout(debounceTimer); + debounceTimer = setTimeout( + () => finish(true), + Math.max(0, DIAGNOSTICS_DEBOUNCE_MS - (Date.now() - hit.at)), + ); + }; + timeoutTimer = setTimeout(() => finish(false), request.timeout); + const listener = (event: { path: string; serverID: string }) => { + if (event.path !== request.path || event.serverID !== serverID) return; + schedule(); + }; + diagnosticListeners.add(listener); + unsub = () => diagnosticListeners.delete(listener); + schedule(); + }); + } + + async function waitForDocumentDiagnostics(request: { + path: string; + version: number; + after?: number; + }) { + const startedAt = request.after ?? Date.now(); + const pushWait = waitForFreshPush({ + path: request.path, + version: request.version, + after: startedAt, + timeout: DIAGNOSTICS_DOCUMENT_WAIT_TIMEOUT_MS, + }); + while (Date.now() - startedAt < DIAGNOSTICS_DOCUMENT_WAIT_TIMEOUT_MS) { + const result = await requestDocumentDiagnostics(request.path); + if (result.matched) return; + const remaining = DIAGNOSTICS_DOCUMENT_WAIT_TIMEOUT_MS - (Date.now() - startedAt); + if (remaining <= 0) return; + const next = await Promise.race([ + pushWait.then((ready) => (ready ? "push" : "timeout")), + waitForRegistrationChange(remaining).then((c) => (c ? "registration" : "timeout")), + ]); + if (next !== "registration") return; + } + } + + async function waitForFullDiagnostics(request: { + path: string; + version: number; + after?: number; + }) { + const startedAt = request.after ?? Date.now(); + const pushWait = waitForFreshPush({ + path: request.path, + version: request.version, + after: startedAt, + timeout: DIAGNOSTICS_FULL_WAIT_TIMEOUT_MS, + }); + while (Date.now() - startedAt < DIAGNOSTICS_FULL_WAIT_TIMEOUT_MS) { + const result = await requestFullDiagnostics(request.path); + if (result.handled || result.matched) return; + const remaining = DIAGNOSTICS_FULL_WAIT_TIMEOUT_MS - (Date.now() - startedAt); + if (remaining <= 0) return; + const next = await Promise.race([ + pushWait.then((ready) => (ready ? "push" : "timeout")), + waitForRegistrationChange(remaining).then((c) => (c ? "registration" : "timeout")), + ]); + if (next !== "registration") return; + } + } + + const normalize = (p: string) => (isAbsolute(p) ? p : resolve(directory, p)); + + // ─── Public surface ─── + const client: LspClient = { + serverID, + root, + connection, + async notifyOpen(path: string) { + const filePath = normalize(path); + const text = await readFile(filePath, "utf8"); + const languageId = languageIdForExtension(extname(filePath)); + const uri = pathToFileURL(filePath).href; + const document = files[filePath]; + + if (document !== undefined) { + await connection.sendNotification("workspace/didChangeWatchedFiles", { + changes: [{ uri, type: FILE_CHANGE_CHANGED }], + }); + const next = document.version + 1; + files[filePath] = { version: next, text }; + await connection.sendNotification("textDocument/didChange", { + textDocument: { uri, version: next }, + contentChanges: + syncKind === TEXT_DOCUMENT_SYNC_INCREMENTAL + ? [ + { + range: { + start: { line: 0, character: 0 }, + end: endPosition(document.text), + }, + text, + }, + ] + : [{ text }], + }); + return next; + } + + await connection.sendNotification("workspace/didChangeWatchedFiles", { + changes: [{ uri, type: FILE_CHANGE_CREATED }], + }); + pushDiagnostics.delete(filePath); + pullDiagnostics.delete(filePath); + await connection.sendNotification("textDocument/didOpen", { + textDocument: { uri, languageId, version: 0, text }, + }); + files[filePath] = { version: 0, text }; + return 0; + }, + get diagnostics() { + const result = new Map(); + for (const key of new Set([...pushDiagnostics.keys(), ...pullDiagnostics.keys()])) { + result.set(key, mergedDiagnostics(key)); + } + return result; + }, + async waitForDiagnostics(request) { + const normalizedPath = normalize(request.path); + if (request.mode === "document") { + await waitForDocumentDiagnostics({ + path: normalizedPath, + version: request.version, + after: request.after, + }); + return; + } + await waitForFullDiagnostics({ + path: normalizedPath, + version: request.version, + after: request.after, + }); + }, + async request(method: string, params: unknown): Promise { + return connection.sendRequest(method, params).catch(() => null); + }, + async shutdown() { + try { + connection.end(); + connection.dispose(); + } catch { + /* connection may already be closed */ + } + server.process.kill(); + }, + }; + + return client; +} diff --git a/packages/core/src/lsp/diagnostic.ts b/packages/core/src/lsp/diagnostic.ts new file mode 100644 index 0000000..1ad4d0f --- /dev/null +++ b/packages/core/src/lsp/diagnostic.ts @@ -0,0 +1,41 @@ +import type { Diagnostic } from "vscode-languageserver-types"; + +/** + * Diagnostic formatting helpers. Ported from opencode's `lsp/diagnostic.ts`. + * + * LSP positions are 0-based on the wire; we render them 1-based (editor-style) + * so they line up with what `read_file` shows and what editors report. + */ + +/** Max diagnostics rendered per file before truncating with a "… and N more". */ +const MAX_PER_FILE = 20; + +const SEVERITY_LABEL: Record = { + 1: "ERROR", + 2: "WARN", + 3: "INFO", + 4: "HINT", +}; + +/** Render a single diagnostic as `SEVERITY [line:col] message` (1-based). */ +export function pretty(diagnostic: Diagnostic): string { + const severity = SEVERITY_LABEL[diagnostic.severity ?? 1] ?? "ERROR"; + const line = diagnostic.range.start.line + 1; + const col = diagnostic.range.start.character + 1; + return `${severity} [${line}:${col}] ${diagnostic.message}`; +} + +/** + * Build a `` block for a file's ERROR-severity + * diagnostics, or `""` when there are none. Errors only — warnings/info/hints + * are intentionally omitted so the model is nudged toward the things that + * actually break the build (matching opencode's behavior). + */ +export function report(file: string, issues: Diagnostic[]): string { + const errors = issues.filter((item) => item.severity === 1); + if (errors.length === 0) return ""; + const limited = errors.slice(0, MAX_PER_FILE); + const more = errors.length - MAX_PER_FILE; + const suffix = more > 0 ? `\n... and ${more} more` : ""; + return `\n${limited.map(pretty).join("\n")}${suffix}\n`; +} diff --git a/packages/core/src/lsp/index.ts b/packages/core/src/lsp/index.ts new file mode 100644 index 0000000..fd43c2f --- /dev/null +++ b/packages/core/src/lsp/index.ts @@ -0,0 +1,18 @@ +// LSP (Language Server Protocol) integration. +// +// Config-driven only: servers are declared in `dispatch.toml`'s `[lsp.]` +// block (see `LspServerConfig` in `../types`). There is no builtin server +// registry and no auto-download. The primary model-facing surface is +// diagnostics-on-write (the host passes a write hook that calls `touchFile` + +// `report`); an on-demand `lsp` tool exposes hover/definition/references too. + +export { + createLspClient, + type Diagnostic, + type LspClient, + type LspServerHandle, +} from "./client.js"; +export { pretty, report } from "./diagnostic.js"; +export { LANGUAGE_EXTENSIONS, languageIdForExtension } from "./language.js"; +export { LspManager } from "./manager.js"; +export { type ResolvedLspServer, resolveServersFromConfig } from "./server.js"; diff --git a/packages/core/src/lsp/language.ts b/packages/core/src/lsp/language.ts new file mode 100644 index 0000000..3e9fe68 --- /dev/null +++ b/packages/core/src/lsp/language.ts @@ -0,0 +1,72 @@ +/** + * File-extension → LSP `languageId` map. + * + * The LSP `textDocument/didOpen` notification carries a `languageId` string + * that tells the server how to parse the document. This table is a trimmed + * port of opencode's `lsp/language.ts`, with one critical addition for this + * project: `.luau` → `"luau"`. Roblox Luau sources use the `.luau` extension, + * which standard Lua tooling does not recognise — luau-lsp expects the + * `"luau"` languageId. + * + * Extensions are looked up with their leading dot (e.g. `".luau"`). Unknown + * extensions fall back to `"plaintext"` at the call site. + */ +export const LANGUAGE_EXTENSIONS: Record = { + // Luau (Roblox) — the reason this module exists. Keep first for visibility. + ".luau": "luau", + ".lua": "lua", + // A pragmatic subset of common languages, mirroring opencode's table so a + // user can point an arbitrary LSP server at this codebase and have the + // right languageId reported. + ".c": "c", + ".cpp": "cpp", + ".cc": "cpp", + ".cxx": "cpp", + ".h": "c", + ".hpp": "cpp", + ".cs": "csharp", + ".css": "css", + ".dart": "dart", + ".go": "go", + ".html": "html", + ".htm": "html", + ".java": "java", + ".js": "javascript", + ".jsx": "javascriptreact", + ".json": "json", + ".jsonc": "jsonc", + ".kt": "kotlin", + ".kts": "kotlin", + ".md": "markdown", + ".markdown": "markdown", + ".php": "php", + ".py": "python", + ".rb": "ruby", + ".rs": "rust", + ".scss": "scss", + ".sass": "sass", + ".sh": "shellscript", + ".bash": "shellscript", + ".zsh": "shellscript", + ".sql": "sql", + ".svelte": "svelte", + ".swift": "swift", + ".toml": "toml", + ".ts": "typescript", + ".tsx": "typescriptreact", + ".mts": "typescript", + ".cts": "typescript", + ".vue": "vue", + ".xml": "xml", + ".yaml": "yaml", + ".yml": "yaml", + ".zig": "zig", +}; + +/** + * Resolve the LSP `languageId` for a file path's extension, falling back to + * `"plaintext"` when the extension is unknown. + */ +export function languageIdForExtension(extension: string): string { + return LANGUAGE_EXTENSIONS[extension] ?? "plaintext"; +} diff --git a/packages/core/src/lsp/manager.ts b/packages/core/src/lsp/manager.ts new file mode 100644 index 0000000..db8b68e --- /dev/null +++ b/packages/core/src/lsp/manager.ts @@ -0,0 +1,220 @@ +import { extname } from "node:path"; +import { createLspClient, type Diagnostic, type LspClient } from "./client.js"; +import type { ResolvedLspServer } from "./server.js"; + +/** + * Process-wide owner of LSP client lifecycles. + * + * Clients are keyed by `root + serverID` and spawned lazily on the first file + * that matches a server's extensions, then reused. Concurrent spawns for the + * same key are de-duplicated via an in-flight map, and servers that fail to + * start are remembered in `broken` so we don't spawn-spam. Modeled on + * opencode's `lsp/lsp.ts` `getClients` flow, minus the Effect machinery. + * + * The manager is config-agnostic: callers resolve `ResolvedLspServer[]` from a + * tab's working-directory config (`resolveServersFromConfig`) and pass them in + * alongside the `root`. This keeps per-working-directory config out of the + * manager while letting it own all the long-lived processes for the process. + */ +export class LspManager { + private clients = new Map(); + private spawning = new Map>(); + private broken = new Set(); + + private key(root: string, serverID: string): string { + return `${root}\u0000${serverID}`; + } + + private serversForFile(file: string, servers: ResolvedLspServer[]): ResolvedLspServer[] { + const extension = extname(file) || file; + return servers.filter( + (server) => server.extensions.length === 0 || server.extensions.includes(extension), + ); + } + + /** + * True if any provided server is configured to attach to this file's + * extension (regardless of whether it has spawned yet). Used to decide + * whether an LSP operation is even applicable to a file. + */ + hasServerForFile(file: string, servers: ResolvedLspServer[]): boolean { + return this.serversForFile(file, servers).length > 0; + } + + /** + * Get (spawning if needed) all clients that should attach to `file` at + * `root`. Spawn failures are swallowed (logged via `broken`) and simply + * yield fewer clients — callers degrade gracefully to "no diagnostics". + */ + async getClients(input: { + file: string; + root: string; + servers: ResolvedLspServer[]; + }): Promise { + const { file, root, servers } = input; + const matching = this.serversForFile(file, servers); + const result: LspClient[] = []; + + for (const server of matching) { + const key = this.key(root, server.id); + if (this.broken.has(key)) continue; + + const existing = this.clients.get(key); + if (existing) { + result.push(existing); + continue; + } + + const inflight = this.spawning.get(key); + if (inflight) { + const client = await inflight; + if (client) result.push(client); + continue; + } + + const task = this.spawn(server, root, key); + this.spawning.set(key, task); + task.finally(() => { + if (this.spawning.get(key) === task) this.spawning.delete(key); + }); + const client = await task; + if (client) result.push(client); + } + + return result; + } + + private async spawn( + server: ResolvedLspServer, + root: string, + key: string, + ): Promise { + let handle: ReturnType; + try { + handle = server.spawn(root); + } catch (err) { + this.broken.add(key); + console.warn( + `dispatch: failed to spawn LSP server "${server.id}": ${err instanceof Error ? err.message : String(err)}`, + ); + return undefined; + } + + // A spawn that fails asynchronously (e.g. ENOENT — binary not on PATH) + // emits `error` on the child process; mark broken so we don't retry it. + handle.process.on("error", (err) => { + this.broken.add(key); + console.warn(`dispatch: LSP server "${server.id}" process error: ${err.message}`); + }); + + try { + const client = await createLspClient({ + serverID: server.id, + server: handle, + root, + directory: root, + }); + // A racing caller may have created the same client; prefer the + // existing one and discard ours. + const existing = this.clients.get(key); + if (existing) { + await client.shutdown(); + return existing; + } + this.clients.set(key, client); + return client; + } catch (err) { + this.broken.add(key); + try { + handle.process.kill(); + } catch { + /* already dead */ + } + console.warn( + `dispatch: failed to initialize LSP client "${server.id}": ${err instanceof Error ? err.message : String(err)}`, + ); + return undefined; + } + } + + /** + * Open/sync a file with its clients and (optionally) wait for diagnostics + * to settle. `mode: "document"` waits for the file's own diagnostics; + * `"full"` also waits on workspace diagnostics; omitted just syncs. + */ + async touchFile(input: { + file: string; + root: string; + servers: ResolvedLspServer[]; + mode?: "document" | "full"; + }): Promise { + const clients = await this.getClients(input); + await Promise.all( + clients.map(async (client) => { + const after = Date.now(); + const version = await client.notifyOpen(input.file); + if (!input.mode) return; + await client.waitForDiagnostics({ + path: input.file, + version, + mode: input.mode, + after, + }); + }), + ).catch((err) => { + console.warn( + `dispatch: failed to touch file for LSP: ${err instanceof Error ? err.message : String(err)}`, + ); + }); + } + + /** + * Merged diagnostics for a single file across all of its clients, keyed by + * absolute file path. Includes related-file diagnostics a client surfaced + * (e.g. workspace pulls), so the result map may contain more than `file`. + */ + getDiagnostics(input: { + root: string; + servers: ResolvedLspServer[]; + file: string; + }): Record { + const results: Record = {}; + const matching = this.serversForFile(input.file, input.servers); + for (const server of matching) { + const client = this.clients.get(this.key(input.root, server.id)); + if (!client) continue; + for (const [path, diags] of client.diagnostics.entries()) { + results[path] = (results[path] ?? []).concat(diags); + } + } + return results; + } + + /** + * Run a positional LSP request (hover/definition/references/etc.) against + * every client for the file and flatten the (non-null) results. `line`/ + * `character` are 0-based here — the caller converts from editor 1-based. + */ + async request(input: { + file: string; + root: string; + servers: ResolvedLspServer[]; + method: string; + params: Record; + }): Promise { + const clients = await this.getClients(input); + const results = await Promise.all( + clients.map((client) => client.request(input.method, input.params)), + ); + return results.filter((r) => r !== null && r !== undefined); + } + + /** Shut down every live client and clear all state. */ + async shutdownAll(): Promise { + const clients = [...this.clients.values()]; + this.clients.clear(); + this.spawning.clear(); + this.broken.clear(); + await Promise.all(clients.map((client) => client.shutdown().catch(() => {}))); + } +} diff --git a/packages/core/src/lsp/server.ts b/packages/core/src/lsp/server.ts new file mode 100644 index 0000000..1fb002e --- /dev/null +++ b/packages/core/src/lsp/server.ts @@ -0,0 +1,68 @@ +import { type ChildProcessWithoutNullStreams, spawn } from "node:child_process"; +import type { LspServerConfig } from "../types/index.js"; +import type { LspServerHandle } from "./client.js"; + +/** + * A resolved, ready-to-spawn LSP server derived from a `dispatch.toml` + * `[lsp.]` entry. Config-driven only — dispatch ships no builtin server + * registry and performs no auto-download (unlike opencode). The declared + * executable (`command[0]`) must already be on PATH. + */ +export interface ResolvedLspServer { + id: string; + /** Extensions (with leading dot) this server attaches to, e.g. `".luau"`. */ + extensions: string[]; + /** Launch the server over stdio rooted at `root`. */ + spawn(root: string): LspServerHandle; +} + +/** + * Spawn a child process for an LSP server over stdio. Inherits `process.env` + * (so a PATH-resident `rojo` is visible to luau-lsp's sourcemap autogenerate) + * and merges any `env` from the server config on top. + */ +function spawnServer( + command: string[], + cwd: string, + env: Record | undefined, + initialization: Record | undefined, +): LspServerHandle { + const [cmd, ...args] = command; + if (!cmd) throw new Error("LSP server command is empty"); + const proc = spawn(cmd, args, { + cwd, + env: { ...process.env, ...env }, + stdio: ["pipe", "pipe", "pipe"], + }) as ChildProcessWithoutNullStreams; + return { + process: proc, + ...(initialization ? { initialization } : {}), + }; +} + +/** + * Turn the parsed `dispatch.toml` `lsp` block into a list of spawnable + * servers. Disabled entries are dropped. Entries with no `command`/`extensions` + * are skipped defensively (the config validator already enforces these, but we + * guard here too so a hand-built config object can't crash the manager). + */ +export function resolveServersFromConfig( + lsp: Record | undefined, +): ResolvedLspServer[] { + if (!lsp) return []; + const servers: ResolvedLspServer[] = []; + for (const [id, entry] of Object.entries(lsp)) { + if (entry.disabled) continue; + if (!entry.command || entry.command.length === 0) continue; + if (!entry.extensions || entry.extensions.length === 0) continue; + const command = entry.command; + const env = entry.env; + const initialization = entry.initialization; + servers.push({ + id, + extensions: entry.extensions, + spawn: (root: string) => spawnServer(command, root, env, initialization), + }); + } + return servers; +} diff --git a/packages/core/src/tools/lsp.ts b/packages/core/src/tools/lsp.ts new file mode 100644 index 0000000..3842cd4 --- /dev/null +++ b/packages/core/src/tools/lsp.ts @@ -0,0 +1,135 @@ +import { isAbsolute, resolve } from "node:path"; +import { pathToFileURL } from "node:url"; +import { z } from "zod"; +import { report as reportDiagnostics } from "../lsp/diagnostic.js"; +import type { LspManager } from "../lsp/manager.js"; +import type { ResolvedLspServer } from "../lsp/server.js"; +import type { ToolDefinition } from "../types/index.js"; + +const OPERATIONS = ["diagnostics", "hover", "definition", "references", "documentSymbol"] as const; +type Operation = (typeof OPERATIONS)[number]; + +/** + * Context the LSP tool needs from the host: the live manager, the tab's + * effective working directory (used as the LSP `root`), and the servers + * resolved from that directory's `dispatch.toml`. + */ +export interface LspToolContext { + manager: LspManager; + workingDirectory: string; + servers: ResolvedLspServer[]; +} + +/** + * On-demand LSP query tool. Exposes diagnostics plus the navigation + * capabilities (hover/definition/references/documentSymbol) for a file at a + * position. Gated behind `perm_lsp` by the host. + * + * Coordinates are **1-based** in this tool's API (editor-style, matching what + * `read_file` shows); they are converted to the LSP wire's 0-based positions + * before the request. + */ +export function createLspTool(getContext: () => LspToolContext): ToolDefinition { + return { + name: "lsp", + description: + "Query the configured Language Server (e.g. luau-lsp for Roblox Luau) about a file. " + + "Operations: 'diagnostics' (type/lint errors for a file), 'hover' (type/docs at a position), " + + "'definition' (where a symbol is defined), 'references' (all uses of a symbol), " + + "'documentSymbol' (outline of a file). Line and character are 1-based (as shown in editors). " + + "Returns JSON. Requires an [lsp] server configured in dispatch.toml that matches the file's extension.", + parameters: z.object({ + operation: z.enum(OPERATIONS).describe("The LSP operation to perform"), + path: z.string().describe("Path to the file, relative to the working directory"), + line: z + .number() + .int() + .min(1) + .optional() + .describe( + "Line number, 1-based (as shown in editors). Required for hover/definition/references.", + ), + character: z + .number() + .int() + .min(1) + .optional() + .describe( + "Character/column, 1-based (as shown in editors). Required for hover/definition/references.", + ), + }), + execute: async (args: Record): Promise => { + const { manager, workingDirectory, servers } = getContext(); + const operation = args.operation as Operation; + const pathArg = typeof args.path === "string" ? args.path : ""; + if (!pathArg) return "Error: 'path' is required."; + + const file = isAbsolute(pathArg) ? pathArg : resolve(workingDirectory, pathArg); + + if (servers.length === 0) { + return "Error: no LSP servers are configured. Add an [lsp] entry to dispatch.toml."; + } + if (!manager.hasServerForFile(file, servers)) { + return `Error: no configured LSP server matches "${pathArg}" (check the server's extensions in dispatch.toml).`; + } + + // Sync the file so the server has current content, then act. + await manager.touchFile({ file, root: workingDirectory, servers, mode: "document" }); + + if (operation === "diagnostics") { + const all = manager.getDiagnostics({ root: workingDirectory, servers, file }); + const block = reportDiagnostics(file, all[file] ?? []); + return block || `No errors reported for ${pathArg}.`; + } + + if (operation === "documentSymbol") { + const uri = pathToFileURL(file).href; + const results = await manager.request({ + file, + root: workingDirectory, + servers, + method: "textDocument/documentSymbol", + params: { textDocument: { uri } }, + }); + const flat = results.flat().filter(Boolean); + return flat.length === 0 + ? `No symbols found in ${pathArg}.` + : JSON.stringify(flat, null, 2); + } + + // Positional operations need line + character. + const line = typeof args.line === "number" ? Math.floor(args.line) : undefined; + const character = typeof args.character === "number" ? Math.floor(args.character) : undefined; + if (line === undefined || character === undefined) { + return `Error: '${operation}' requires both 'line' and 'character' (1-based).`; + } + + const uri = pathToFileURL(file).href; + // Convert editor 1-based → LSP wire 0-based. + const position = { line: line - 1, character: character - 1 }; + const method = + operation === "hover" + ? "textDocument/hover" + : operation === "definition" + ? "textDocument/definition" + : "textDocument/references"; + const params: Record = { + textDocument: { uri }, + position, + ...(operation === "references" ? { context: { includeDeclaration: true } } : {}), + }; + + const results = await manager.request({ + file, + root: workingDirectory, + servers, + method, + params, + }); + const flat = results.flat().filter(Boolean); + return flat.length === 0 + ? `No results found for ${operation} at ${pathArg}:${line}:${character}.` + : JSON.stringify(flat, null, 2); + }, + }; +} diff --git a/packages/core/src/tools/write-file.ts b/packages/core/src/tools/write-file.ts index aa69c86..8a73352 100644 --- a/packages/core/src/tools/write-file.ts +++ b/packages/core/src/tools/write-file.ts @@ -4,7 +4,21 @@ import { z } from "zod"; import type { ToolDefinition } from "../types/index.js"; import { canonicalize } from "./path-utils.js"; -export function createWriteFileTool(workingDirectory: string): ToolDefinition { +/** + * Optional hook invoked AFTER a successful write, with the canonicalized + * absolute path of the file just written. Its returned string (when non-empty) + * is appended to the tool result. This is how LSP diagnostics are surfaced + * back to the model on write without coupling `@dispatch/core`'s tools to the + * API layer or the LSP manager — the host wires an implementation that touches + * the file through the LSP and formats any diagnostics. Errors thrown here are + * swallowed so a flaky LSP never fails the write itself. + */ +export type AfterWriteHook = (absolutePath: string) => Promise; + +export function createWriteFileTool( + workingDirectory: string, + onAfterWrite?: AfterWriteHook, +): ToolDefinition { return { name: "write_file", description: "Write content to a file relative to the working directory.", @@ -31,10 +45,22 @@ export function createWriteFileTool(workingDirectory: string): ToolDefinition { try { await mkdir(dirname(absolutePath), { recursive: true }); await writeFile(absolutePath, content, "utf8"); - return `Successfully wrote to "${filePath}".`; } catch (err) { return `Error writing file: ${err instanceof Error ? err.message : String(err)}`; } + + let result = `Successfully wrote to "${filePath}".`; + // Post-write hook (e.g. LSP diagnostics). Best-effort: never let a + // hook failure turn a successful write into an error. + if (onAfterWrite) { + try { + const extra = await onAfterWrite(absolutePath); + if (extra) result += `\n\n${extra}`; + } catch { + /* ignore — diagnostics are advisory */ + } + } + return result; }, }; } diff --git a/packages/core/src/types/index.ts b/packages/core/src/types/index.ts index a22b2b7..607b27d 100644 --- a/packages/core/src/types/index.ts +++ b/packages/core/src/types/index.ts @@ -435,6 +435,55 @@ export interface AgentConfig { export interface DispatchConfig { keys?: KeyDefinition[]; permissions: Record>; + /** + * Language Server Protocol servers, keyed by an arbitrary server id (e.g. + * `"luau-lsp"`). Project-scoped: read from the `dispatch.toml` in a tab's + * effective working directory and re-consulted when that directory (or the + * config) changes. Config-driven only — there is no builtin server registry + * and no auto-download; the declared `command[0]` must be on PATH. + */ + lsp?: Record; +} + +/** + * A single LSP server entry as expressed in `dispatch.toml`'s `[lsp.]` + * block. Mirrors opencode's custom-server schema so the Roblox Luau config + * (and any other server) is portable between the two. + * + * Example (`dispatch.toml`): + * ```toml + * [lsp.luau-lsp] + * command = ["luau-lsp", "lsp", "--definitions=globalTypes.d.luau", "--docs=api-docs.json"] + * extensions = [".luau"] + * + * [lsp.luau-lsp.initialization.luau-lsp.platform] + * type = "roblox" + * ``` + */ +export interface LspServerConfig { + /** + * Argv to launch the server over stdio. `command[0]` is the executable + * (resolved via PATH); the rest are arguments. Required for every non- + * disabled entry. + */ + command: string[]; + /** + * File extensions (with leading dot, e.g. `".luau"`) this server attaches + * to. Required for custom servers — without it the client never knows which + * files should activate the server. + */ + extensions: string[]; + /** Extra environment variables merged onto `process.env` for the child. */ + env?: Record; + /** + * `initializationOptions` forwarded verbatim in the LSP `initialize` + * request (and echoed back for `workspace/configuration` / + * `didChangeConfiguration`). For luau-lsp this carries the + * `{ "luau-lsp": { platform, sourcemap, types, diagnostics, ... } }` block. + */ + initialization?: Record; + /** When true, the entry is parsed but skipped (no server launched). */ + disabled?: boolean; } export interface KeyDefinition { diff --git a/packages/core/tests/config/lsp-schema.test.ts b/packages/core/tests/config/lsp-schema.test.ts new file mode 100644 index 0000000..2b71cc2 --- /dev/null +++ b/packages/core/tests/config/lsp-schema.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, it } from "vitest"; +import { validateConfig } from "../../src/config/schema.js"; + +describe("config schema — [lsp] block", () => { + it("parses a valid custom server entry", () => { + const { config, errors } = validateConfig({ + permissions: {}, + lsp: { + "luau-lsp": { + command: ["luau-lsp", "lsp"], + extensions: [".luau"], + initialization: { "luau-lsp": { platform: { type: "roblox" } } }, + }, + }, + }); + expect(errors).toHaveLength(0); + expect(config.lsp).toBeDefined(); + const entry = config.lsp?.["luau-lsp"]; + expect(entry?.command).toEqual(["luau-lsp", "lsp"]); + expect(entry?.extensions).toEqual([".luau"]); + expect(entry?.initialization).toEqual({ + "luau-lsp": { platform: { type: "roblox" } }, + }); + }); + + it("preserves env and nested initialization verbatim", () => { + const { config } = validateConfig({ + permissions: {}, + lsp: { + "luau-lsp": { + command: ["luau-lsp", "lsp"], + extensions: [".luau"], + env: { PATH: "/custom/bin" }, + initialization: { + "luau-lsp": { + sourcemap: { enabled: true, autogenerate: true }, + diagnostics: { strictDatamodelTypes: false }, + }, + }, + }, + }, + }); + const entry = config.lsp?.["luau-lsp"]; + expect(entry?.env).toEqual({ PATH: "/custom/bin" }); + expect(entry?.initialization).toEqual({ + "luau-lsp": { + sourcemap: { enabled: true, autogenerate: true }, + diagnostics: { strictDatamodelTypes: false }, + }, + }); + }); + + it("rejects a custom server missing command", () => { + const { config, errors } = validateConfig({ + permissions: {}, + lsp: { broken: { extensions: [".luau"] } }, + }); + expect(errors.some((e) => e.path === "lsp.broken.command")).toBe(true); + expect(config.lsp).toBeUndefined(); + }); + + it("rejects a custom server missing extensions", () => { + const { errors } = validateConfig({ + permissions: {}, + lsp: { broken: { command: ["x"] } }, + }); + expect(errors.some((e) => e.path === "lsp.broken.extensions")).toBe(true); + }); + + it("rejects an empty command array", () => { + const { errors } = validateConfig({ + permissions: {}, + lsp: { broken: { command: [], extensions: [".luau"] } }, + }); + expect(errors.some((e) => e.path === "lsp.broken.command")).toBe(true); + }); + + it("keeps a disabled entry without requiring command/extensions", () => { + const { config, errors } = validateConfig({ + permissions: {}, + lsp: { "luau-lsp": { disabled: true } }, + }); + expect(errors).toHaveLength(0); + expect(config.lsp?.["luau-lsp"]?.disabled).toBe(true); + }); + + it("skips a malformed entry but keeps valid siblings", () => { + const { config, errors } = validateConfig({ + permissions: {}, + lsp: { + good: { command: ["a"], extensions: [".luau"] }, + bad: { extensions: [".luau"] }, + }, + }); + expect(config.lsp?.good).toBeDefined(); + expect(config.lsp?.bad).toBeUndefined(); + expect(errors.length).toBeGreaterThan(0); + }); + + it("omits lsp entirely when not present", () => { + const { config, errors } = validateConfig({ permissions: {} }); + expect(errors).toHaveLength(0); + expect(config.lsp).toBeUndefined(); + }); + + it("flags a non-object lsp value", () => { + const { errors } = validateConfig({ permissions: {}, lsp: "nope" }); + expect(errors.some((e) => e.path === "lsp")).toBe(true); + }); +}); diff --git a/packages/core/tests/fixture/lsp/fake-lsp-server.js b/packages/core/tests/fixture/lsp/fake-lsp-server.js new file mode 100644 index 0000000..d771ebd --- /dev/null +++ b/packages/core/tests/fixture/lsp/fake-lsp-server.js @@ -0,0 +1,195 @@ +// Minimal JSON-RPC 2.0 LSP-like fake server over stdio, for testing the LSP +// client without a real language server binary. Ported from opencode's +// test/fixture/lsp/fake-lsp-server.js (trimmed to what dispatch's client and +// manager exercise: initialize, didOpen/didChange, push + pull diagnostics). +// +// Test hooks (custom JSON-RPC methods the test driver can call): +// test/get-initialize-params → returns the params sent to `initialize` +// test/get-last-change → returns the last `didChange` params +// test/publish-diagnostics → forwards a `publishDiagnostics` push +// test/configure-pull-diagnostics → sets up pull-diagnostic responses +// test/get-diagnostic-request-count→ how many pull requests were received + +let nextId = 1; +let readBuffer = Buffer.alloc(0); +let lastChange = null; +let initializeParams = null; +let diagnosticRequestCount = 0; +let registeredCapability = false; +let pullConfig = { + registerOn: undefined, + registrations: [], + documentDiagnostics: [], + workspaceDiagnostics: [], + hasDiagnosticProvider: false, +}; + +function encode(message) { + const json = JSON.stringify(message); + const header = `Content-Length: ${Buffer.byteLength(json, "utf8")}\r\n\r\n`; + return Buffer.concat([Buffer.from(header, "utf8"), Buffer.from(json, "utf8")]); +} + +function decodeFrames(buffer) { + const results = []; + while (true) { + const idx = buffer.indexOf("\r\n\r\n"); + if (idx === -1) break; + const header = buffer.slice(0, idx).toString("utf8"); + const match = /Content-Length:\s*(\d+)/i.exec(header); + const length = match ? parseInt(match[1], 10) : 0; + const bodyStart = idx + 4; + const bodyEnd = bodyStart + length; + if (buffer.length < bodyEnd) break; + results.push(buffer.slice(bodyStart, bodyEnd).toString("utf8")); + buffer = buffer.slice(bodyEnd); + } + return { messages: results, rest: buffer }; +} + +function send(message) { + process.stdout.write(encode(message)); +} +function sendRequest(method, params) { + const id = nextId++; + send({ jsonrpc: "2.0", id, method, params }); + return id; +} +function sendResponse(id, result) { + send({ jsonrpc: "2.0", id, result }); +} +function sendNotification(method, params) { + send({ jsonrpc: "2.0", method, params }); +} + +function maybeRegister(method) { + if (pullConfig.registerOn !== method || registeredCapability) return; + registeredCapability = true; + sendRequest("client/registerCapability", { + registrations: pullConfig.registrations.map((registration, index) => ({ + id: registration.id ?? `pull-${index}`, + method: registration.method ?? "textDocument/diagnostic", + registerOptions: registration.registerOptions ?? registration, + })), + }); +} + +function handle(raw) { + let data; + try { + data = JSON.parse(raw); + } catch { + return; + } + + if (data.method === "initialize") { + initializeParams = data.params; + sendResponse(data.id, { + capabilities: { + textDocumentSync: { change: 2, openClose: true }, + ...(pullConfig.hasDiagnosticProvider + ? { + diagnosticProvider: { + identifier: "fake", + interFileDependencies: false, + workspaceDiagnostics: false, + }, + } + : {}), + }, + }); + return; + } + + if (data.method === "test/get-initialize-params") { + sendResponse(data.id, initializeParams); + return; + } + + if (data.method === "initialized" || data.method === "workspace/didChangeConfiguration") { + return; + } + + if (data.method === "textDocument/didOpen") { + maybeRegister("didOpen"); + return; + } + + if (data.method === "textDocument/didChange") { + lastChange = data.params; + maybeRegister("didChange"); + return; + } + + if (data.method === "workspace/didChangeWatchedFiles") { + return; + } + + if (data.method === "test/configure-pull-diagnostics") { + pullConfig = { + registerOn: data.params?.registerOn, + registrations: data.params?.registrations ?? [], + documentDiagnostics: data.params?.documentDiagnostics ?? [], + workspaceDiagnostics: data.params?.workspaceDiagnostics ?? [], + hasDiagnosticProvider: data.params?.hasDiagnosticProvider ?? false, + }; + registeredCapability = false; + sendResponse(data.id, null); + return; + } + + if (data.method === "test/publish-diagnostics") { + sendNotification("textDocument/publishDiagnostics", data.params); + sendResponse(data.id, null); + return; + } + + if (data.method === "test/get-last-change") { + sendResponse(data.id, lastChange); + return; + } + + if (data.method === "test/get-diagnostic-request-count") { + sendResponse(data.id, diagnosticRequestCount); + return; + } + + if (data.method === "textDocument/diagnostic") { + diagnosticRequestCount += 1; + sendResponse(data.id, { kind: "full", items: pullConfig.documentDiagnostics }); + return; + } + + if (data.method === "workspace/diagnostic") { + diagnosticRequestCount += 1; + sendResponse(data.id, { items: pullConfig.workspaceDiagnostics }); + return; + } + + if (data.method === "textDocument/hover") { + sendResponse(data.id, { contents: { kind: "plaintext", value: "fake hover" } }); + return; + } + + if (data.method === "textDocument/definition") { + sendResponse(data.id, [ + { + uri: data.params?.textDocument?.uri, + range: { start: { line: 0, character: 0 }, end: { line: 0, character: 1 } }, + }, + ]); + return; + } + + // Default: respond null to any other request so the client never hangs. + if (typeof data.id !== "undefined") { + sendResponse(data.id, null); + } +} + +process.stdin.on("data", (chunk) => { + readBuffer = Buffer.concat([readBuffer, chunk]); + const { messages, rest } = decodeFrames(readBuffer); + readBuffer = rest; + for (const message of messages) handle(message); +}); diff --git a/packages/core/tests/lsp/client.test.ts b/packages/core/tests/lsp/client.test.ts new file mode 100644 index 0000000..8daf8ab --- /dev/null +++ b/packages/core/tests/lsp/client.test.ts @@ -0,0 +1,146 @@ +import { spawn } from "node:child_process"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import type { Diagnostic } from "vscode-languageserver-types"; +import { createLspClient, type LspServerHandle } from "../../src/lsp/client.js"; + +const FIXTURE = join(dirname(fileURLToPath(import.meta.url)), "../fixture/lsp/fake-lsp-server.js"); + +function spawnFakeServer(): LspServerHandle { + const proc = spawn(process.execPath, [FIXTURE], { stdio: "pipe" }); + return { process: proc as LspServerHandle["process"] }; +} + +const ERROR_DIAG: Diagnostic = { + range: { start: { line: 0, character: 0 }, end: { line: 0, character: 5 } }, + severity: 1, + message: "fake type error", + source: "Fake", +}; + +describe("lsp/client (fake server)", () => { + let workDir: string; + + beforeEach(async () => { + workDir = await mkdtemp(join(tmpdir(), "dispatch-lsp-")); + }); + afterEach(async () => { + await rm(workDir, { recursive: true, force: true }); + }); + + it("completes the initialize handshake and forwards initializationOptions", async () => { + const handle = spawnFakeServer(); + handle.initialization = { "luau-lsp": { platform: { type: "roblox" } } }; + const client = await createLspClient({ + serverID: "fake", + server: handle, + root: workDir, + directory: workDir, + }); + + const params = await client.connection.sendRequest<{ initializationOptions?: unknown }>( + "test/get-initialize-params", + {}, + ); + expect(params.initializationOptions).toEqual({ + "luau-lsp": { platform: { type: "roblox" } }, + }); + await client.shutdown(); + }); + + it("opens a file and receives push diagnostics", async () => { + const handle = spawnFakeServer(); + const client = await createLspClient({ + serverID: "fake", + server: handle, + root: workDir, + directory: workDir, + }); + + const file = join(workDir, "a.luau"); + await writeFile(file, "local x = 1\n"); + const version = await client.notifyOpen(file); + expect(version).toBe(0); + + // Drive a push from the fake server, then assert it lands in the map. + await client.connection.sendRequest("test/publish-diagnostics", { + uri: pathToFileURL(file).href, + diagnostics: [ERROR_DIAG], + }); + await new Promise((r) => setTimeout(r, 50)); + + expect(client.diagnostics.get(file)?.[0]?.message).toBe("fake type error"); + await client.shutdown(); + }); + + it("bumps the document version on re-open (didChange)", async () => { + const handle = spawnFakeServer(); + const client = await createLspClient({ + serverID: "fake", + server: handle, + root: workDir, + directory: workDir, + }); + const file = join(workDir, "a.luau"); + await writeFile(file, "local x = 1\n"); + expect(await client.notifyOpen(file)).toBe(0); + await writeFile(file, "local x = 2\n"); + expect(await client.notifyOpen(file)).toBe(1); + + const lastChange = await client.connection.sendRequest<{ textDocument?: { version?: number } }>( + "test/get-last-change", + {}, + ); + expect(lastChange?.textDocument?.version).toBe(1); + await client.shutdown(); + }); + + it("waits for pull diagnostics when the server advertises a diagnostic provider", async () => { + const handle = spawnFakeServer(); + const client = await createLspClient({ + serverID: "fake", + server: handle, + root: workDir, + directory: workDir, + }); + // Tell the fake server (before initialize? no — it persists) to answer + // pull requests. We configure AFTER connect; the static provider flag is + // read at initialize, so this test exercises the dynamic registration + // path instead. + await client.connection.sendRequest("test/configure-pull-diagnostics", { + registerOn: "didOpen", + registrations: [{ id: "d1", registerOptions: { identifier: "fake" } }], + documentDiagnostics: [ERROR_DIAG], + }); + + const file = join(workDir, "a.luau"); + await writeFile(file, "bad\n"); + const version = await client.notifyOpen(file); + await client.waitForDiagnostics({ path: file, version, mode: "document" }); + + expect(client.diagnostics.get(file)?.some((d) => d.message === "fake type error")).toBe(true); + await client.shutdown(); + }); + + it("request() passes through to the server (hover)", async () => { + const handle = spawnFakeServer(); + const client = await createLspClient({ + serverID: "fake", + server: handle, + root: workDir, + directory: workDir, + }); + const file = join(workDir, "a.luau"); + await writeFile(file, "local x = 1\n"); + await client.notifyOpen(file); + const hover = await client.request<{ contents?: { value?: string } }>("textDocument/hover", { + textDocument: { uri: pathToFileURL(file).href }, + position: { line: 0, character: 6 }, + }); + expect(hover?.contents?.value).toBe("fake hover"); + await client.shutdown(); + }); +}); diff --git a/packages/core/tests/lsp/diagnostic.test.ts b/packages/core/tests/lsp/diagnostic.test.ts new file mode 100644 index 0000000..93ffde9 --- /dev/null +++ b/packages/core/tests/lsp/diagnostic.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from "vitest"; +import type { Diagnostic } from "vscode-languageserver-types"; +import { pretty, report } from "../../src/lsp/diagnostic.js"; + +function diag(partial: Partial & { message: string }): Diagnostic { + return { + range: { start: { line: 0, character: 0 }, end: { line: 0, character: 1 } }, + severity: 1, + ...partial, + }; +} + +describe("lsp/diagnostic", () => { + describe("pretty", () => { + it("renders 1-based line/col with severity label", () => { + const out = pretty( + diag({ + message: "Expected number", + range: { start: { line: 4, character: 2 }, end: { line: 4, character: 8 } }, + }), + ); + expect(out).toBe("ERROR [5:3] Expected number"); + }); + + it("maps severities to labels", () => { + expect(pretty(diag({ message: "w", severity: 2 }))).toMatch(/^WARN /); + expect(pretty(diag({ message: "i", severity: 3 }))).toMatch(/^INFO /); + expect(pretty(diag({ message: "h", severity: 4 }))).toMatch(/^HINT /); + }); + + it("defaults missing severity to ERROR", () => { + expect(pretty(diag({ message: "x", severity: undefined }))).toMatch(/^ERROR /); + }); + }); + + describe("report", () => { + it("returns empty string when there are no errors", () => { + expect(report("a.luau", [])).toBe(""); + // Warnings only → still empty (errors-only). + expect(report("a.luau", [diag({ message: "w", severity: 2 })])).toBe(""); + }); + + it("wraps errors in a block", () => { + const out = report("src/a.luau", [diag({ message: "boom" })]); + expect(out).toContain(''); + expect(out).toContain("ERROR [1:1] boom"); + expect(out).toContain(""); + }); + + it("filters out non-error severities", () => { + const out = report("a.luau", [ + diag({ message: "err" }), + diag({ message: "warn", severity: 2 }), + ]); + expect(out).toContain("err"); + expect(out).not.toContain("warn"); + }); + + it("caps at 20 and notes the remainder", () => { + const issues = Array.from({ length: 25 }, (_, i) => diag({ message: `e${i}` })); + const out = report("a.luau", issues); + expect(out).toContain("... and 5 more"); + expect(out).toContain("e0"); + expect(out).not.toContain("e24"); + }); + }); +}); diff --git a/packages/core/tests/lsp/luau-lsp.smoke.test.ts b/packages/core/tests/lsp/luau-lsp.smoke.test.ts new file mode 100644 index 0000000..381435b --- /dev/null +++ b/packages/core/tests/lsp/luau-lsp.smoke.test.ts @@ -0,0 +1,63 @@ +import { execSync } from "node:child_process"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { LspManager } from "../../src/lsp/manager.js"; +import { resolveServersFromConfig } from "../../src/lsp/server.js"; + +/** + * Opt-in smoke test against the REAL luau-lsp binary. Skipped automatically + * (never fails CI) when `luau-lsp` is not on PATH — mirrors opencode's + * platform-guarded launch test. When the binary IS present, it proves the + * end-to-end path: spawn → initialize handshake → didOpen → real diagnostics. + */ +function hasLuauLsp(): boolean { + try { + execSync("luau-lsp --version", { stdio: "ignore" }); + return true; + } catch { + return false; + } +} + +const RUN = hasLuauLsp(); + +describe.skipIf(!RUN)("luau-lsp real-binary smoke", () => { + let root: string; + let manager: LspManager; + + beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), "dispatch-luau-smoke-")); + manager = new LspManager(); + }); + afterEach(async () => { + await manager.shutdownAll(); + await rm(root, { recursive: true, force: true }); + }); + + it("reports a real type error for a bad .luau file", async () => { + const servers = resolveServersFromConfig({ + "luau-lsp": { + command: ["luau-lsp", "lsp"], + extensions: [".luau"], + initialization: { + "luau-lsp": { + platform: { type: "roblox" }, + diagnostics: { strictDatamodelTypes: false }, + }, + }, + }, + }); + + const file = join(root, "bad.luau"); + await writeFile(file, 'local x: number = "not a number"\nprint(x)\n'); + + await manager.touchFile({ file, root, servers, mode: "document" }); + const diagnostics = manager.getDiagnostics({ root, servers, file }); + const messages = (diagnostics[file] ?? []).map((d) => d.message).join("\n"); + + expect(messages.length).toBeGreaterThan(0); + expect(messages.toLowerCase()).toContain("number"); + }, 60_000); +}); diff --git a/packages/core/tests/lsp/manager.test.ts b/packages/core/tests/lsp/manager.test.ts new file mode 100644 index 0000000..e720413 --- /dev/null +++ b/packages/core/tests/lsp/manager.test.ts @@ -0,0 +1,120 @@ +import { spawn } from "node:child_process"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath, pathToFileURL } from "node:url"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import type { Diagnostic } from "vscode-languageserver-types"; +import { LspManager } from "../../src/lsp/manager.js"; +import type { ResolvedLspServer } from "../../src/lsp/server.js"; + +const FIXTURE = join(dirname(fileURLToPath(import.meta.url)), "../fixture/lsp/fake-lsp-server.js"); + +function makeServer(id: string, extensions: string[]) { + const counter = { count: 0 }; + const server: ResolvedLspServer = { + id, + extensions, + spawn() { + counter.count += 1; + const proc = spawn(process.execPath, [FIXTURE], { stdio: "pipe" }); + return { process: proc as never }; + }, + }; + return { server, counter }; +} + +describe("lsp/manager (fake server)", () => { + let root: string; + let manager: LspManager; + + beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), "dispatch-lspmgr-")); + manager = new LspManager(); + }); + afterEach(async () => { + await manager.shutdownAll(); + await rm(root, { recursive: true, force: true }); + }); + + it("hasServerForFile matches by extension", () => { + const { server } = makeServer("fake", [".luau"]); + expect(manager.hasServerForFile(join(root, "a.luau"), [server])).toBe(true); + expect(manager.hasServerForFile(join(root, "a.ts"), [server])).toBe(false); + }); + + it("spawns lazily and reuses the client across calls", async () => { + const { server, counter } = makeServer("fake", [".luau"]); + const file = join(root, "a.luau"); + await writeFile(file, "local x = 1\n"); + + const c1 = await manager.getClients({ file, root, servers: [server] }); + const c2 = await manager.getClients({ file, root, servers: [server] }); + expect(c1).toHaveLength(1); + expect(c2).toHaveLength(1); + expect(c1[0]).toBe(c2[0]); + expect(counter.count).toBe(1); + }); + + it("does not spawn for a non-matching extension", async () => { + const { server, counter } = makeServer("fake", [".luau"]); + const file = join(root, "a.ts"); + await writeFile(file, "const x = 1\n"); + const clients = await manager.getClients({ file, root, servers: [server] }); + expect(clients).toHaveLength(0); + expect(counter.count).toBe(0); + }); + + it("touchFile + getDiagnostics surfaces a pushed diagnostic", async () => { + const { server } = makeServer("fake", [".luau"]); + const file = join(root, "a.luau"); + await writeFile(file, "bad code\n"); + + await manager.touchFile({ file, root, servers: [server] }); + const [client] = await manager.getClients({ file, root, servers: [server] }); + // Drive a push through the fake server. + const diag: Diagnostic = { + range: { start: { line: 0, character: 0 }, end: { line: 0, character: 3 } }, + severity: 1, + message: "manager error", + }; + await client.connection.sendRequest("test/publish-diagnostics", { + uri: pathToFileURL(file).href, + diagnostics: [diag], + }); + await new Promise((r) => setTimeout(r, 50)); + + const result = manager.getDiagnostics({ root, servers: [server], file }); + expect(result[file]?.[0]?.message).toBe("manager error"); + }); + + it("request() forwards to clients and flattens results", async () => { + const { server } = makeServer("fake", [".luau"]); + const file = join(root, "a.luau"); + await writeFile(file, "local x = 1\n"); + await manager.touchFile({ file, root, servers: [server] }); + + const results = await manager.request({ + file, + root, + servers: [server], + method: "textDocument/definition", + params: { + textDocument: { uri: pathToFileURL(file).href }, + position: { line: 0, character: 6 }, + }, + }); + expect(results.length).toBeGreaterThan(0); + }); + + it("shutdownAll clears state so the next call respawns", async () => { + const { server, counter } = makeServer("fake", [".luau"]); + const file = join(root, "a.luau"); + await writeFile(file, "local x = 1\n"); + await manager.getClients({ file, root, servers: [server] }); + expect(counter.count).toBe(1); + await manager.shutdownAll(); + await manager.getClients({ file, root, servers: [server] }); + expect(counter.count).toBe(2); + }); +}); diff --git a/packages/core/tests/lsp/server.test.ts b/packages/core/tests/lsp/server.test.ts new file mode 100644 index 0000000..bdaf83d --- /dev/null +++ b/packages/core/tests/lsp/server.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from "vitest"; +import { resolveServersFromConfig } from "../../src/lsp/server.js"; + +describe("lsp/server resolveServersFromConfig", () => { + it("returns [] for undefined config", () => { + expect(resolveServersFromConfig(undefined)).toEqual([]); + }); + + it("resolves a server entry with id + extensions", () => { + const servers = resolveServersFromConfig({ + "luau-lsp": { command: ["luau-lsp", "lsp"], extensions: [".luau"] }, + }); + expect(servers).toHaveLength(1); + expect(servers[0]?.id).toBe("luau-lsp"); + expect(servers[0]?.extensions).toEqual([".luau"]); + expect(typeof servers[0]?.spawn).toBe("function"); + }); + + it("skips disabled entries", () => { + const servers = resolveServersFromConfig({ + "luau-lsp": { command: ["luau-lsp", "lsp"], extensions: [".luau"], disabled: true }, + }); + expect(servers).toEqual([]); + }); + + it("skips entries with empty command or extensions", () => { + const servers = resolveServersFromConfig({ + noCommand: { command: [], extensions: [".luau"] }, + noExt: { command: ["x"], extensions: [] }, + }); + expect(servers).toEqual([]); + }); + + it("resolves multiple servers", () => { + const servers = resolveServersFromConfig({ + a: { command: ["a"], extensions: [".luau"] }, + b: { command: ["b"], extensions: [".lua"] }, + }); + expect(servers.map((s) => s.id).sort()).toEqual(["a", "b"]); + }); +}); diff --git a/packages/core/tests/tools/lsp-tool.test.ts b/packages/core/tests/tools/lsp-tool.test.ts new file mode 100644 index 0000000..7f26522 --- /dev/null +++ b/packages/core/tests/tools/lsp-tool.test.ts @@ -0,0 +1,110 @@ +import { describe, expect, it, vi } from "vitest"; +import type { LspManager } from "../../src/lsp/manager.js"; +import type { ResolvedLspServer } from "../../src/lsp/server.js"; +import { createLspTool, type LspToolContext } from "../../src/tools/lsp.js"; + +const SERVER: ResolvedLspServer = { + id: "luau-lsp", + extensions: [".luau"], + spawn: () => ({ process: {} as never }), +}; + +function makeManager(overrides: Partial = {}): LspManager { + return { + hasServerForFile: vi.fn(() => true), + touchFile: vi.fn(async () => {}), + getDiagnostics: vi.fn(() => ({})), + request: vi.fn(async () => []), + getClients: vi.fn(async () => []), + shutdownAll: vi.fn(async () => {}), + ...overrides, + } as unknown as LspManager; +} + +function ctx(manager: LspManager, servers = [SERVER]): () => LspToolContext { + return () => ({ manager, workingDirectory: "/work", servers }); +} + +describe("createLspTool", () => { + it("exposes the expected schema/name", () => { + const tool = createLspTool(ctx(makeManager())); + expect(tool.name).toBe("lsp"); + expect(tool.description).toMatch(/luau-lsp/i); + }); + + it("errors when no servers are configured", async () => { + const tool = createLspTool(ctx(makeManager(), [])); + const out = await tool.execute({ operation: "diagnostics", path: "a.luau" }); + expect(out).toMatch(/no LSP servers are configured/i); + }); + + it("errors when no server matches the file", async () => { + const manager = makeManager({ hasServerForFile: vi.fn(() => false) as never }); + const tool = createLspTool(ctx(manager)); + const out = await tool.execute({ operation: "diagnostics", path: "a.ts" }); + expect(out).toMatch(/no configured LSP server matches/i); + }); + + it("diagnostics: touches the file then reports errors", async () => { + const touchFile = vi.fn(async () => {}); + const getDiagnostics = vi.fn(() => ({ + "/work/a.luau": [ + { + range: { start: { line: 2, character: 1 }, end: { line: 2, character: 9 } }, + severity: 1, + message: "bad type", + }, + ], + })); + const manager = makeManager({ + touchFile: touchFile as never, + getDiagnostics: getDiagnostics as never, + }); + const tool = createLspTool(ctx(manager)); + const out = await tool.execute({ operation: "diagnostics", path: "a.luau" }); + expect(touchFile).toHaveBeenCalledOnce(); + expect(out).toContain("ERROR [3:2] bad type"); + }); + + it("diagnostics: reports clean when no errors", async () => { + const tool = createLspTool(ctx(makeManager())); + const out = await tool.execute({ operation: "diagnostics", path: "a.luau" }); + expect(out).toMatch(/No errors reported/i); + }); + + it("hover: requires line and character", async () => { + const tool = createLspTool(ctx(makeManager())); + const out = await tool.execute({ operation: "hover", path: "a.luau" }); + expect(out).toMatch(/requires both 'line' and 'character'/i); + }); + + it("hover: converts 1-based coords to 0-based on the wire", async () => { + const request = vi.fn(async () => [{ contents: "hi" }]); + const manager = makeManager({ request: request as never }); + const tool = createLspTool(ctx(manager)); + await tool.execute({ operation: "hover", path: "a.luau", line: 5, character: 3 }); + expect(request).toHaveBeenCalledOnce(); + const arg = request.mock.calls[0]?.[0] as { method: string; params: { position: unknown } }; + expect(arg.method).toBe("textDocument/hover"); + expect(arg.params.position).toEqual({ line: 4, character: 2 }); + }); + + it("references: includes declaration context", async () => { + const request = vi.fn(async () => []); + const manager = makeManager({ request: request as never }); + const tool = createLspTool(ctx(manager)); + await tool.execute({ operation: "references", path: "a.luau", line: 1, character: 1 }); + const arg = request.mock.calls[0]?.[0] as { params: { context?: unknown } }; + expect(arg.params.context).toEqual({ includeDeclaration: true }); + }); + + it("documentSymbol: does not require a position", async () => { + const request = vi.fn(async () => [{ name: "foo" }]); + const manager = makeManager({ request: request as never }); + const tool = createLspTool(ctx(manager)); + const out = await tool.execute({ operation: "documentSymbol", path: "a.luau" }); + const arg = request.mock.calls[0]?.[0] as { method: string }; + expect(arg.method).toBe("textDocument/documentSymbol"); + expect(out).toContain("foo"); + }); +}); diff --git a/packages/core/tests/tools/write-file.test.ts b/packages/core/tests/tools/write-file.test.ts index f071e12..0dedbfc 100644 --- a/packages/core/tests/tools/write-file.test.ts +++ b/packages/core/tests/tools/write-file.test.ts @@ -103,4 +103,50 @@ describe("write_file tool", () => { expect(entries).toEqual([]); }); }); + + describe("onAfterWrite hook", () => { + it("appends the hook's returned string to a successful write", async () => { + const tool = createWriteFileTool(workDir, async (abs) => `DIAGNOSTICS for ${abs}`); + const result = await tool.execute({ path: "a.luau", content: "local x = 1" }); + expect(result).toMatch(/successfully wrote/i); + expect(result).toContain("DIAGNOSTICS for"); + expect(result).toContain(join(workDir, "a.luau")); + }); + + it("does not append when the hook returns empty string", async () => { + const tool = createWriteFileTool(workDir, async () => ""); + const result = await tool.execute({ path: "a.luau", content: "local x = 1" }); + expect(result.trim()).toMatch(/^Successfully wrote to "a\.luau"\.$/); + }); + + it("does not run the hook when the write is blocked (traversal)", async () => { + let called = false; + const tool = createWriteFileTool(workDir, async () => { + called = true; + return "should not appear"; + }); + const result = await tool.execute({ path: "../evil.txt", content: "bad" }); + expect(result).toMatch(/outside the working directory/i); + expect(called).toBe(false); + }); + + it("swallows hook errors so a throwing hook never fails the write", async () => { + const tool = createWriteFileTool(workDir, async () => { + throw new Error("lsp blew up"); + }); + const result = await tool.execute({ path: "a.luau", content: "local x = 1" }); + expect(result).toMatch(/successfully wrote/i); + expect(result).not.toContain("lsp blew up"); + }); + + it("passes the canonical absolute path to the hook", async () => { + let seen = ""; + const tool = createWriteFileTool(workDir, async (abs) => { + seen = abs; + return ""; + }); + await tool.execute({ path: "nested/b.luau", content: "x" }); + expect(seen).toBe(join(workDir, "nested/b.luau")); + }); + }); }); -- cgit v1.2.3 From 94ce3401cd65ff3f58c134c73cb3f816b7142093 Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Tue, 2 Jun 2026 19:02:43 +0900 Subject: feat(api): fall back to next port (3000→3010) when the port is in use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The API server bound a fixed port (3000) and died with EADDRINUSE when it was taken — painful when running multiple dispatch instances (e.g. testing several feature branches at once). Replace the static default export with an explicit Bun.serve retry loop that increments the port by one on EADDRINUSE, from START_PORT (PORT env or 3000) up to MAX_PORT (3010), logging the chosen port and a hint to repoint the frontend's API URL on a bump. Guarded by import.meta.main so importing the module (for `app`) never binds a port. Frontend unchanged — set its API URL manually when a bump occurs. --- packages/api/src/index.ts | 61 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 55 insertions(+), 6 deletions(-) (limited to 'packages/api/src') diff --git a/packages/api/src/index.ts b/packages/api/src/index.ts index a0ad025..478abe0 100644 --- a/packages/api/src/index.ts +++ b/packages/api/src/index.ts @@ -70,9 +70,58 @@ app.get( export { app }; -export default { - port: Number(process.env.PORT) || 3000, - idleTimeout: 60, - fetch: app.fetch, - websocket, -}; +// Starting port (overridable via PORT) and the inclusive ceiling we will bump +// up to when a port is already in use. If 3000 is taken we try 3001, 3002, … +// up to MAX_PORT, so multiple dispatch instances (e.g. testing several +// features at once) can coexist without manually juggling ports. The frontend +// defaults to :3000 — point it at the chosen port via the in-app API-URL +// field / VITE_API_URL when a bump happens. +const START_PORT = Number(process.env.PORT) || 3000; +const MAX_PORT = 3010; + +/** + * Bind the server to `START_PORT`, incrementing by one on EADDRINUSE until a + * free port is found or MAX_PORT is exceeded. Bun's `Bun.serve` throws + * synchronously when the port is taken, so we can catch and retry. Returns the + * live server (whose `.port` reflects the port actually bound). + */ +function serveWithPortFallback() { + let lastError: unknown; + for (let port = START_PORT; port <= MAX_PORT; port++) { + try { + const server = Bun.serve({ + port, + idleTimeout: 60, + fetch: app.fetch, + websocket, + }); + if (port !== START_PORT) { + console.warn( + `dispatch: port ${START_PORT} in use — bound to ${port} instead. ` + + `Set the frontend's API URL to http://localhost:${port}.`, + ); + } + console.log(`dispatch: API listening on http://localhost:${server.port}`); + return server; + } catch (err) { + const code = (err as NodeJS.ErrnoException)?.code; + if (code === "EADDRINUSE") { + lastError = err; + continue; + } + throw err; + } + } + console.error( + `dispatch: no free port in range ${START_PORT}-${MAX_PORT}. ` + + `Free one up or set PORT to an open port.`, + ); + throw lastError ?? new Error(`No free port in range ${START_PORT}-${MAX_PORT}`); +} + +// Only start the server when run as the entry point — importing this module +// (e.g. for `app`) must not bind a port. This preserves the prior +// default-export behavior where Bun served only the entry file. +if (import.meta.main) { + serveWithPortFallback(); +} -- cgit v1.2.3 From 3307fec107fb8d1e7cb063bfdbfadf4b1d4f8c71 Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Tue, 2 Jun 2026 20:05:48 +0900 Subject: fix(wake): resolve probe model dynamically from /v1/models by 'haiku' match The wake probe was hardcoded to claude-3-5-haiku-20241022, which the endpoint no longer serves (HTTP 404), exhausting the retry loop. Now the probe fetches the live model list via fetchAnthropicModels (falling back to ANTHROPIC_MODELS_FALLBACK if empty) and selects the current Haiku via a new pure selectHaikuModel() helper (first case-insensitive 'haiku' substring match; newest-first ordering). No-match surfaces a clear per-account error instead of crashing. --- packages/api/src/routes/models.ts | 29 ++++++++++++++++------ packages/core/src/credentials/claude.ts | 17 +++++++++++++ packages/core/src/credentials/index.ts | 1 + packages/core/tests/credentials/wake-probe.test.ts | 26 ++++++++++++++++++- 4 files changed, 64 insertions(+), 9 deletions(-) (limited to 'packages/api/src') diff --git a/packages/api/src/routes/models.ts b/packages/api/src/routes/models.ts index 8f64bbb..eeb6029 100644 --- a/packages/api/src/routes/models.ts +++ b/packages/api/src/routes/models.ts @@ -20,6 +20,7 @@ import { refreshAccountCredentialsAsync, resolveApiKey, resolveContextLimit, + selectHaikuModel, setApiKey, validateAccountCredentials, } from "@dispatch/core"; @@ -568,13 +569,6 @@ modelsRoutes.post("/remove-key", async (c) => { // ─── Shared wake function ───────────────────────────────────── -/** - * Model used for the wake probe. A small/cheap model is enough — the only - * purpose is to register activity against the subscription so its rate-limit - * window keeps resetting on schedule. - */ -const WAKE_PROBE_MODEL = "claude-3-5-haiku-20241022"; - /** Max chars of upstream error body to keep in the surfaced message. */ const MAX_ERROR_BODY_CHARS = 200; @@ -631,6 +625,25 @@ async function wakeAllClaudeAccounts(): Promise< continue; } + // Resolve the probe model dynamically. A fixed model id (the old + // `claude-3-5-haiku-20241022`) eventually stops being served and + // the probe 404s, so pull the live list from `/v1/models` and pick + // the current Haiku. Fall back to the well-known list if the live + // fetch comes back empty (network blip, transient upstream error). + let availableModels = await fetchAnthropicModels(creds.accessToken); + if (availableModels.length === 0) { + availableModels = ANTHROPIC_MODELS_FALLBACK; + } + const probeModel = selectHaikuModel(availableModels); + if (!probeModel) { + results.push({ + label: acct.label, + ok: false, + error: "no 'haiku' model available from /v1/models", + }); + continue; + } + // Mirror a genuine Claude Code CLI request. These are OAuth // (Pro/Max) subscription accounts: Anthropic validates the // `system[]` array and rejects (401/403) any request whose system @@ -648,7 +661,7 @@ async function wakeAllClaudeAccounts(): Promise< "X-Claude-Code-Session-Id": randomUUID(), "x-client-request-id": randomUUID(), }, - body: JSON.stringify(buildWakeProbeBody(WAKE_PROBE_MODEL)), + body: JSON.stringify(buildWakeProbeBody(probeModel)), }); if (res.ok) { diff --git a/packages/core/src/credentials/claude.ts b/packages/core/src/credentials/claude.ts index 432e403..7818222 100644 --- a/packages/core/src/credentials/claude.ts +++ b/packages/core/src/credentials/claude.ts @@ -483,6 +483,23 @@ export const ANTHROPIC_MODELS_FALLBACK = [ "claude-3-opus-20240229", ]; +/** + * Pick the model to use for a Claude "wake" probe from a list of model ids. + * + * The probe only needs a small/cheap model to register activity against the + * subscription, so we target Haiku. Model ids change over time (the old + * hardcoded `claude-3-5-haiku-20241022` started returning HTTP 404), so the + * caller fetches the live list from `/v1/models` and we resolve by substring. + * + * Selection: the FIRST id whose name contains "haiku" (case-insensitive). + * Anthropic's `/v1/models` returns models newest-first, so first-match + * naturally prefers the newest Haiku. Returns `null` when nothing matches so + * the caller can surface a clear error instead of probing an invalid model. + */ +export function selectHaikuModel(models: string[]): string | null { + return models.find((id) => id.toLowerCase().includes("haiku")) ?? null; +} + // ─── Credential Validation ──────────────────────────────────── export interface ClaudeProfile { diff --git a/packages/core/src/credentials/index.ts b/packages/core/src/credentials/index.ts index 46fa5b6..5221dc6 100644 --- a/packages/core/src/credentials/index.ts +++ b/packages/core/src/credentials/index.ts @@ -24,6 +24,7 @@ export { refreshAccountCredentials, refreshAccountCredentialsAsync, SYSTEM_IDENTITY, + selectHaikuModel, validateAccountCredentials, } from "./claude.js"; export { diff --git a/packages/core/tests/credentials/wake-probe.test.ts b/packages/core/tests/credentials/wake-probe.test.ts index 253efec..a97a00c 100644 --- a/packages/core/tests/credentials/wake-probe.test.ts +++ b/packages/core/tests/credentials/wake-probe.test.ts @@ -9,7 +9,7 @@ vi.mock("../../src/db/index.js", () => ({ }), })); -const { buildWakeProbeBody } = await import("../../src/credentials/claude.js"); +const { buildWakeProbeBody, selectHaikuModel } = await import("../../src/credentials/claude.js"); const IDENTITY = "You are Claude Code, Anthropic's official CLI for Claude."; @@ -47,3 +47,27 @@ describe("buildWakeProbeBody", () => { expect(a).toEqual(b); }); }); +describe("selectHaikuModel", () => { + it("returns the id whose name contains 'haiku'", () => { + const models = ["claude-sonnet-4-20250514", "claude-haiku-4-5-20251001"]; + expect(selectHaikuModel(models)).toBe("claude-haiku-4-5-20251001"); + }); + + it("matches case-insensitively", () => { + expect(selectHaikuModel(["Claude-HAIKU-Latest"])).toBe("Claude-HAIKU-Latest"); + }); + + it("returns the FIRST match when several models contain 'haiku'", () => { + // `/v1/models` returns newest-first, so first-match prefers the newest. + const models = ["claude-haiku-4-5-20251001", "claude-3-5-haiku-20241022"]; + expect(selectHaikuModel(models)).toBe("claude-haiku-4-5-20251001"); + }); + + it("returns null when no model contains 'haiku'", () => { + expect(selectHaikuModel(["claude-sonnet-4-20250514", "claude-opus-4-20250514"])).toBeNull(); + }); + + it("returns null for an empty list", () => { + expect(selectHaikuModel([])).toBeNull(); + }); +}); -- cgit v1.2.3