diff options
| author | Adam Malczewski <[email protected]> | 2026-06-24 13:01:18 +0900 |
|---|---|---|
| committer | Adam Malczewski <[email protected]> | 2026-06-24 13:01:18 +0900 |
| commit | 5630bf177c1f45d8e35ddbe35bf7d5136dc4f244 (patch) | |
| tree | 7603569ef855fe3aa531f7c8bd125dff477eaf79 | |
| parent | 1eb25dcace8c3cb0b3a3871a74d0dd3eaf169eb7 (diff) | |
| download | dispatch-5630bf177c1f45d8e35ddbe35bf7d5136dc4f244.tar.gz dispatch-5630bf177c1f45d8e35ddbe35bf7d5136dc4f244.zip | |
fix(lsp): broken-server recovery + config source attribution
Two issues found by decompiling the running dispatch-server binary
(handoff from a ruby-lsp setup in raylib-jamstack):
Issue 2 (blocker): a failed LSP server was "broken" FOREVER — the
manager's broken set was cleared only in shutdownAll(), so a server
that failed (bad env, missing binary, or a since-fixed config) stayed
state:"error" for the whole process. For an agent running *inside*
dispatch the only recovery (server restart) kills its own session.
Now a broken server self-heals when its resolved config changes since
it was marked broken (discrete event → no retry storm), with a bounded
backoff for transient failures.
Issue 1: .dispatch/lsp.json silently shadowed opencode.json's lsp key
with no warning and no source attribution. Now: shadow warning via
host.logger when both declare lsp; configSource populated on status
(.dispatch/lsp.json / opencode.json / built-in); spawn-failure error
strings name the config source.
Contract: additive configSource?: string on LspServerInfo
(@dispatch/transport-contract 0.20.0→0.21.0). transport-http passes it
through to the wire (was a field-by-field map that dropped it — CR
resolved by the transport-http owner).
tsc -b EXIT 0, biome clean, 1443 vitest pass.
| -rw-r--r-- | packages/lsp/src/config.test.ts | 93 | ||||
| -rw-r--r-- | packages/lsp/src/config.ts | 100 | ||||
| -rw-r--r-- | packages/lsp/src/index.ts | 11 | ||||
| -rw-r--r-- | packages/lsp/src/manager.test.ts | 160 | ||||
| -rw-r--r-- | packages/lsp/src/manager.ts | 127 | ||||
| -rw-r--r-- | packages/lsp/src/types.ts | 7 | ||||
| -rw-r--r-- | packages/transport-contract/package.json | 2 | ||||
| -rw-r--r-- | packages/transport-contract/src/contract.types.test.ts | 14 | ||||
| -rw-r--r-- | packages/transport-contract/src/index.ts | 7 | ||||
| -rw-r--r-- | packages/transport-http/src/app.test.ts | 65 | ||||
| -rw-r--r-- | packages/transport-http/src/app.ts | 1 | ||||
| -rw-r--r-- | tasks.md | 37 |
12 files changed, 574 insertions, 50 deletions
diff --git a/packages/lsp/src/config.test.ts b/packages/lsp/src/config.test.ts index 1462490..6d51774 100644 --- a/packages/lsp/src/config.test.ts +++ b/packages/lsp/src/config.test.ts @@ -3,7 +3,7 @@ import { resolveServers } from "./config.js"; describe("config", () => { it("built-in typescript resolves when tsconfig.json exists", async () => { - const servers = await resolveServers({ + const { servers } = await resolveServers({ cwd: "/project", dispatchLspJson: null, opencodeJson: null, @@ -28,7 +28,7 @@ describe("config", () => { }, }); - const servers = await resolveServers({ + const { servers } = await resolveServers({ cwd: "/project", dispatchLspJson: config, opencodeJson: null, @@ -50,7 +50,7 @@ describe("config", () => { }, }); - const servers = await resolveServers({ + const { servers } = await resolveServers({ cwd: "/project", dispatchLspJson: null, opencodeJson: opencodeConfig, @@ -69,7 +69,7 @@ describe("config", () => { lsp: { fallback: { command: ["fallback-lsp"], extensions: [".f"] } }, }); - const servers = await resolveServers({ + const { servers } = await resolveServers({ cwd: "/project", dispatchLspJson: dispatchConfig, opencodeJson: opencodeConfig, @@ -98,7 +98,7 @@ describe("config", () => { }, }); - const servers = await resolveServers({ + const { servers } = await resolveServers({ cwd: "/project", dispatchLspJson: config, opencodeJson: null, @@ -116,4 +116,87 @@ describe("config", () => { "sourcemap.json", ]); }); + + it("config: resolveServers records configSource", async () => { + // .dispatch/lsp.json entry → ".dispatch/lsp.json" + const dispatch = JSON.stringify({ + servers: { + mylsp: { command: ["my-lsp"], extensions: [".ml"] }, + }, + }); + const fromDispatch = await resolveServers({ + cwd: "/project", + dispatchLspJson: dispatch, + opencodeJson: null, + exists: async () => false, + }); + expect(fromDispatch.servers[0]?.configSource).toBe(".dispatch/lsp.json"); + + // fallback to opencode.json → "opencode.json" + const opencode = JSON.stringify({ + lsp: { + oc: { command: ["oc-lsp"], extensions: [".oc"] }, + }, + }); + const fromOpencode = await resolveServers({ + cwd: "/project", + dispatchLspJson: null, + opencodeJson: opencode, + exists: async () => false, + }); + expect(fromOpencode.servers[0]?.configSource).toBe("opencode.json"); + + // built-in TS → "built-in" + const fromBuiltin = await resolveServers({ + cwd: "/project", + dispatchLspJson: null, + opencodeJson: null, + exists: async () => false, + }); + expect(fromBuiltin.servers[0]?.configSource).toBe("built-in"); + }); + + it("config: shadowed flag is true when .dispatch/lsp.json shadows opencode.json lsp", async () => { + const dispatch = JSON.stringify({ + servers: { primary: { command: ["primary-lsp"], extensions: [".p"] } }, + }); + const opencode = JSON.stringify({ + lsp: { fallback: { command: ["fallback-lsp"], extensions: [".f"] } }, + }); + + const { shadowed, servers } = await resolveServers({ + cwd: "/project", + dispatchLspJson: dispatch, + opencodeJson: opencode, + exists: async () => false, + }); + + // dispatch wins, opencode entry is shadowed + expect(shadowed).toBe(true); + expect(servers.map((s) => s.id)).toEqual(["primary"]); + }); + + it("config: shadowed flag is false when only one config source declares lsp", async () => { + const dispatch = JSON.stringify({ + servers: { primary: { command: ["primary-lsp"], extensions: [".p"] } }, + }); + + // dispatch present, opencode has NO lsp key → not shadowed + const onlyDispatch = await resolveServers({ + cwd: "/project", + dispatchLspJson: dispatch, + opencodeJson: JSON.stringify({ lsp: {} }), + exists: async () => false, + }); + expect(onlyDispatch.shadowed).toBe(false); + + // dispatch absent, opencode has lsp → not shadowed (opencode is used) + const onlyOpencode = await resolveServers({ + cwd: "/project", + dispatchLspJson: null, + opencodeJson: JSON.stringify({ lsp: { fb: { command: ["fb"] } } }), + exists: async () => false, + }); + expect(onlyOpencode.shadowed).toBe(false); + }); }); diff --git a/packages/lsp/src/config.ts b/packages/lsp/src/config.ts index f0c1f3f..afec6bf 100644 --- a/packages/lsp/src/config.ts +++ b/packages/lsp/src/config.ts @@ -19,6 +19,24 @@ export interface ResolvedServer { readonly rootMarkers: readonly string[]; readonly initialization?: Readonly<Record<string, unknown>> | undefined; readonly sidecar?: { readonly command: readonly string[] } | undefined; + /** + * Which config source this server was resolved from: `".dispatch/lsp.json"`, + * `"opencode.json"`, or `"built-in"`. Omitted when not yet resolved. Mirrors + * the wire `LspServerInfo.configSource` so config-shadow debugging surfaces + * to the status caller (a broken `.dispatch/lsp.json` silently shadowing + * `opencode.json`). + */ + readonly configSource?: string | undefined; +} + +/** Which config source a resolved server came from. */ +export type ConfigSource = ".dispatch/lsp.json" | "opencode.json" | "built-in"; + +/** Result of resolving servers: the servers + whether `opencode.json`'s lsp key + * was silently shadowed by a present `.dispatch/lsp.json`. */ +export interface ResolveResult { + readonly servers: readonly ResolvedServer[]; + readonly shadowed: boolean; } export interface ServerConfig { @@ -54,50 +72,68 @@ const BUILT_IN_REGISTRY: Record<string, ResolvedServer> = { command: ["typescript-language-server", "--stdio"], extensions: [".ts", ".tsx", ".mts", ".cts", ".js", ".jsx", ".mjs", ".cjs"], rootMarkers: ["tsconfig.json", "package.json"], + configSource: "built-in", }, }; -export async function resolveServers(deps: ResolveServersDeps): Promise<ResolvedServer[]> { +export async function resolveServers(deps: ResolveServersDeps): Promise<ResolveResult> { const result = new Map<string, ResolvedServer>(); + // Parse opencode.json once — used both as the fallback source and to detect + // whether a present `.dispatch/lsp.json` silently shadows its `lsp` key. + let opencodeConfig: OpencodeJsonConfig | null = null; + if (deps.opencodeJson) { + try { + opencodeConfig = JSON.parse(deps.opencodeJson) as OpencodeJsonConfig; + } catch { + // ignore parse errors + } + } + const opencodeHasLsp = !!opencodeConfig?.lsp && Object.keys(opencodeConfig.lsp).length > 0; + + // 1. cwd/.dispatch/lsp.json (highest precedence) + let dispatchHadServers = false; if (deps.dispatchLspJson) { try { const config = JSON.parse(deps.dispatchLspJson) as LspJsonConfig; if (config.servers) { for (const [key, server] of Object.entries(config.servers)) { - const resolved = resolveServer(key, server); + const resolved = resolveServer(key, server, ".dispatch/lsp.json"); result.set(resolved.id, resolved); } + dispatchHadServers = result.size > 0; } } catch { // ignore parse errors } } - if (result.size === 0 && deps.opencodeJson) { - try { - const config = JSON.parse(deps.opencodeJson) as OpencodeJsonConfig; - if (config.lsp) { - for (const [key, server] of Object.entries(config.lsp)) { - const resolved = resolveServer(key, server); - result.set(resolved.id, resolved); - } - } - } catch { - // ignore parse errors + // 2. fallback cwd/opencode.json lsp (only when dispatch yielded nothing) + if (result.size === 0 && opencodeConfig?.lsp) { + for (const [key, server] of Object.entries(opencodeConfig.lsp)) { + const resolved = resolveServer(key, server, "opencode.json"); + result.set(resolved.id, resolved); } } + // 3. the built-in registry (when nothing else resolved) if (result.size === 0) { - for (const [id, server] of Object.entries(BUILT_IN_REGISTRY)) { - result.set(id, server); + for (const [, server] of Object.entries(BUILT_IN_REGISTRY)) { + result.set(server.id, server); } } - return [...result.values()]; + // `.dispatch/lsp.json` silently shadows `opencode.json`'s lsp key when both + // declare servers — the opencode entry is skipped with no warning otherwise. + const shadowed = dispatchHadServers && opencodeHasLsp; + return { servers: [...result.values()], shadowed }; } -function resolveServer(key: string, config: ServerConfig): ResolvedServer { +function resolveServer( + key: string, + config: ServerConfig, + configSource: ConfigSource, +): ResolvedServer { const id = config.id ?? key; const name = config.name ?? id; const extensions = config.extensions ?? []; @@ -116,6 +152,7 @@ function resolveServer(key: string, config: ServerConfig): ResolvedServer { command: config.command, extensions, rootMarkers, + configSource, }; if (config.env) { (result as { env?: Readonly<Record<string, string>> }).env = config.env; @@ -146,3 +183,32 @@ function detectSidecar( command: ["rojo", "sourcemap", rojoProjectFile, "--watch", "-o", "sourcemap.json"], }; } + +/** + * A stable fingerprint of a resolved server's config — the bytes that decide + * HOW a server spawns (command, env, initialization, root markers, …). It is + * independent of object key insertion order, so two `status()` calls over an + * UNCHANGED config produce identical fingerprints. The manager compares it + * against the fingerprint captured when a server was marked broken: a change + * means the config was edited (a discrete event → cannot storm) and the server + * should be retried; identical means "still the same broken config" and the + * server stays broken until a bounded backoff elapses. + */ +export function configFingerprint(server: ResolvedServer): string { + return stableStringify(server); +} + +/** Deterministic JSON: object keys sorted at every nesting level. */ +function stableStringify(value: unknown): string { + return JSON.stringify(canonicalize(value)); +} + +function canonicalize(value: unknown): unknown { + if (value === null || typeof value !== "object") return value; + if (Array.isArray(value)) return value.map(canonicalize); + const sorted: Record<string, unknown> = {}; + for (const key of Object.keys(value as Record<string, unknown>).sort()) { + sorted[key] = canonicalize((value as Record<string, unknown>)[key]); + } + return sorted; +} diff --git a/packages/lsp/src/index.ts b/packages/lsp/src/index.ts index 19f824b..49be7ae 100644 --- a/packages/lsp/src/index.ts +++ b/packages/lsp/src/index.ts @@ -7,8 +7,15 @@ export type { SpawnProcess, } from "./client.js"; export { LanguageServerClient } from "./client.js"; -export type { LspJsonConfig, OpencodeJsonConfig, ResolvedServer, ServerConfig } from "./config.js"; -export { resolveServers } from "./config.js"; +export type { + ConfigSource, + LspJsonConfig, + OpencodeJsonConfig, + ResolvedServer, + ResolveResult, + ServerConfig, +} from "./config.js"; +export { configFingerprint, resolveServers } from "./config.js"; export type { Diagnostic, DocumentDiagnosticReport, diff --git a/packages/lsp/src/manager.test.ts b/packages/lsp/src/manager.test.ts index 3e8e60e..1649111 100644 --- a/packages/lsp/src/manager.test.ts +++ b/packages/lsp/src/manager.test.ts @@ -201,4 +201,164 @@ describe("manager", () => { expect(a.map((s) => s.id)).toEqual(["a"]); expect(b.map((s) => s.id)).toEqual(["b"]); }, 10000); + + it("manager: broken server recovers after config is fixed (no shutdownAll)", async () => { + const files: Record<string, string> = { + "/project/tsconfig.json": "{}", + "/project/.dispatch/lsp.json": JSON.stringify({ + servers: { + test: { + command: ["bad-lsp"], + extensions: [".ts"], + rootMarkers: ["tsconfig.json"], + }, + }, + }), + }; + + const spawn: SpawnProcess = (command, opts) => { + if (command[0] === "bad-lsp") throw new Error("spawn failed"); + return makeAutoHandshakeSpawn()(command, opts); + }; + + const manager = new LspManager({ + spawn, + fileWatcher: noopFileWatcher(), + fs: fakeFs(files), + now: () => 0, + }); + + // Bad config → spawn fails → broken. + const s1 = await manager.status("/project"); + expect(s1[0]?.state).toBe("error"); + + // Fix the config: switch the command to a working one. NO shutdownAll. + files["/project/.dispatch/lsp.json"] = JSON.stringify({ + servers: { + test: { + command: ["good-lsp"], + extensions: [".ts"], + rootMarkers: ["tsconfig.json"], + }, + }, + }); + + // Next status() re-reads config, detects the change, and re-spawns. + const s2 = await manager.status("/project"); + expect(s2[0]?.state).toBe("connected"); + }, 10000); + + it("manager: no retry storm — repeated status() with no config change does not re-spawn a broken server in a loop", async () => { + let spawnCount = 0; + const failingSpawn: SpawnProcess = () => { + spawnCount++; + throw new Error("spawn failed"); + }; + + const manager = new LspManager({ + spawn: failingSpawn, + fileWatcher: noopFileWatcher(), + fs: fakeFs({ + "/project/.dispatch/lsp.json": JSON.stringify({ + servers: { test: { command: ["test-lsp"], extensions: [".ts"] } }, + }), + }), + // Frozen clock: the bounded backoff never elapses, so a config-unchanged + // broken server is never retried in a loop. + now: () => 0, + }); + + await manager.status("/project"); + await manager.status("/project"); + await manager.status("/project"); + await manager.status("/project"); + + expect(spawnCount).toBe(1); + }); + + it("manager: configSource reaches status()", async () => { + const manager = new LspManager({ + spawn: makeAutoHandshakeSpawn(), + fileWatcher: noopFileWatcher(), + fs: fakeFs({ + "/d/.dispatch/lsp.json": JSON.stringify({ + servers: { d: { command: ["d-lsp"], extensions: [".d"], rootMarkers: [] } }, + }), + "/o/opencode.json": JSON.stringify({ + lsp: { o: { command: ["o-lsp"], extensions: [".o"] } }, + }), + "/b/tsconfig.json": "{}", + }), + now: () => 0, + }); + + const d = await manager.status("/d"); + expect(d[0]?.configSource).toBe(".dispatch/lsp.json"); + + const o = await manager.status("/o"); + expect(o[0]?.configSource).toBe("opencode.json"); + + const b = await manager.status("/b"); + expect(b[0]?.configSource).toBe("built-in"); + }, 10000); + + it("config: shadow warning logged when .dispatch/lsp.json present and opencode.json also has lsp", async () => { + const warns: Array<{ + msg: string; + attrs?: Record<string, string | number | boolean | null>; + }> = []; + + const manager = new LspManager({ + spawn: makeAutoHandshakeSpawn(), + fileWatcher: noopFileWatcher(), + fs: fakeFs({ + "/project/.dispatch/lsp.json": JSON.stringify({ + servers: { d: { command: ["d-lsp"], extensions: [".d"] } }, + }), + "/project/opencode.json": JSON.stringify({ + lsp: { o: { command: ["o-lsp"], extensions: [".o"] } }, + }), + }), + logger: { + info: () => {}, + warn: (msg, attrs) => warns.push({ msg, attrs }), + error: () => {}, + }, + now: () => 0, + }); + + await manager.status("/project"); + + const shadow = warns.find((w) => w.msg.includes("shadowing")); + expect(shadow).toBeDefined(); + expect(shadow?.attrs?.cwd).toBe("/project"); + + // A second status() over the SAME (still-shadowed) cwd does not re-warn. + const before = warns.length; + await manager.status("/project"); + expect(warns.length).toBe(before); + }, 10000); + + it("status: error string includes config source on spawn failure", async () => { + const failingSpawn: SpawnProcess = () => { + throw new Error("spawn failed"); + }; + + const manager = new LspManager({ + spawn: failingSpawn, + fileWatcher: noopFileWatcher(), + fs: fakeFs({ + "/project/.dispatch/lsp.json": JSON.stringify({ + servers: { test: { command: ["test-lsp"], extensions: [".ts"] } }, + }), + }), + now: () => 0, + }); + + const s = await manager.status("/project"); + expect(s[0]?.state).toBe("error"); + expect(s[0]?.configSource).toBe(".dispatch/lsp.json"); + expect(s[0]?.error).toContain("[from .dispatch/lsp.json]"); + expect(s[0]?.error).toContain("spawn failed"); + }); }); diff --git a/packages/lsp/src/manager.ts b/packages/lsp/src/manager.ts index 7080c65..7153956 100644 --- a/packages/lsp/src/manager.ts +++ b/packages/lsp/src/manager.ts @@ -11,7 +11,7 @@ import { LanguageServerClient, type SpawnProcess, } from "./client.js"; -import { type ResolvedServer, resolveServers } from "./config.js"; +import { configFingerprint, type ResolvedServer, resolveServers } from "./config.js"; import { findRoot } from "./root.js"; import type { LspServerState, LspServerStatus } from "./types.js"; @@ -26,6 +26,11 @@ export interface ManagerDeps { readonly fileWatcher: FileWatcher; readonly fs: FsAccess; readonly logger?: Logger | undefined; + /** + * Injected clock (epoch-ms) for bounded-backoff bookkeeping. Defaults to + * `Date.now`; injected in tests so backoff is deterministic. + */ + readonly now?: (() => number) | undefined; } type ClientEntry = { @@ -34,14 +39,35 @@ type ClientEntry = { readonly promise: Promise<void>; }; +/** + * A failed server's recovery bookkeeping. `configFingerprint` is the resolved + * config captured at failure time: a config edit produces a different + * fingerprint (a discrete event → cannot storm) and the next `status()` clears + * the entry and re-spawns. `brokenAt` seeds a bounded backoff so transient + * failures (e.g. a not-yet-installed binary) also self-heal without a storm. + * `error` is the enriched failure reason surfaced while the server stays + * broken. + */ +type BrokenEntry = { + readonly configFingerprint: string; + readonly brokenAt: number; + readonly error: string; +}; + +/** Bounded backoff before a transient (config-unchanged) failure is retried. */ +const BACKOFF_MS = 30_000; + export class LspManager { private clients = new Map<string, ClientEntry>(); - private broken = new Set<string>(); + private broken = new Map<string, BrokenEntry>(); private spawning = new Map<string, Promise<void>>(); - private deps: ManagerDeps; + private shadowWarned = new Set<string>(); + private readonly deps: ManagerDeps; + private readonly now: () => number; constructor(deps: ManagerDeps) { this.deps = deps; + this.now = deps.now ?? Date.now; } async status(cwd: string): Promise<readonly LspServerStatus[]> { @@ -49,30 +75,59 @@ export class LspManager { // project) gets its own .dispatch/lsp.json or opencode.json, not a global one. const dispatchLspJson = await this.readOrNull(join(cwd, ".dispatch", "lsp.json")); const opencodeJson = await this.readOrNull(join(cwd, "opencode.json")); - const servers = await resolveServers({ + const { servers, shadowed } = await resolveServers({ cwd, dispatchLspJson, opencodeJson, exists: this.deps.fs.exists, }); + // A present `.dispatch/lsp.json` silently shadows `opencode.json`'s lsp + // key — warn once per cwd so a broken shadow names itself (an agent + // running inside dispatch can only see `status()`, not the journal). + if (shadowed && !this.shadowWarned.has(cwd)) { + this.shadowWarned.add(cwd); + this.deps.logger?.warn( + `.dispatch/lsp.json is shadowing the opencode.json "lsp" config — its servers take precedence and the opencode.json lsp entry is ignored`, + { cwd }, + ); + } + const results: LspServerStatus[] = []; for (const server of servers) { const root = await findRoot(cwd, cwd, server.rootMarkers, this.deps.fs.exists); const key = `${server.id}:${root}`; - if (this.broken.has(key)) { - const status: LspServerStatus = { - id: server.id, - name: server.name, - root, - extensions: server.extensions, - state: "error", - error: "Previously failed to start", - }; - results.push(status); - continue; + const brokenEntry = this.broken.get(key); + if (brokenEntry) { + // Recovery, storm-safe: a config change is a discrete event, so it + // cannot loop. Transient failures (config unchanged) are retried + // only after a bounded backoff — never in a tight loop. + const configChanged = configFingerprint(server) !== brokenEntry.configFingerprint; + const backoffElapsed = this.now() - brokenEntry.brokenAt >= BACKOFF_MS; + if (configChanged || backoffElapsed) { + this.broken.delete(key); + // Discard the stale client entry (and any leaked process) from + // the failed spawn so status() re-spawns fresh. + const stale = this.clients.get(key); + if (stale) { + this.clients.delete(key); + stale.client.shutdown(); + } + // fall through to (re)spawn + } else { + results.push({ + id: server.id, + name: server.name, + root, + extensions: server.extensions, + state: "error", + error: brokenEntry.error, + configSource: server.configSource, + }); + continue; + } } const existing = this.clients.get(key); @@ -85,9 +140,10 @@ export class LspManager { root, extensions: server.extensions, state: mapState(state), + configSource: server.configSource, }; if (stateError !== undefined) { - (status as { error?: string }).error = stateError; + (status as { error?: string }).error = enrichError(server, stateError); } results.push(status); continue; @@ -105,23 +161,29 @@ export class LspManager { root, extensions: server.extensions, state: mapState(state), + configSource: server.configSource, }; if (stateError !== undefined) { - (status as { error?: string }).error = stateError; + (status as { error?: string }).error = enrichError(server, stateError); } results.push(status); } } catch (err: unknown) { - this.broken.add(key); - const status: LspServerStatus = { + const message = err instanceof Error ? err.message : String(err); + this.broken.set(key, { + configFingerprint: configFingerprint(server), + brokenAt: this.now(), + error: enrichError(server, message), + }); + results.push({ id: server.id, name: server.name, root, extensions: server.extensions, state: "error", - error: err instanceof Error ? err.message : String(err), - }; - results.push(status); + error: enrichError(server, message), + configSource: server.configSource, + }); } } @@ -185,11 +247,17 @@ export class LspManager { await entry.promise; if (client.getState() === "error") { - this.broken.add(key); + const message = client.getStateError() ?? "unknown"; + this.broken.set(key, { + configFingerprint: configFingerprint(server), + brokenAt: this.now(), + error: enrichError(server, message), + }); this.deps.logger?.warn("LSP server failed to start", { serverId: server.id, root, - error: client.getStateError() ?? "unknown", + configSource: server.configSource ?? "unknown", + error: message, }); } else { this.deps.logger?.info("LSP server connected", { @@ -207,9 +275,20 @@ export class LspManager { this.clients.clear(); this.broken.clear(); this.spawning.clear(); + this.shadowWarned.clear(); } } function mapState(state: LspServerState): LspServerState { return state; } + +/** + * Prefix a failure reason with the server id + its config source, so a broken + * config file names itself in the `status()` response (e.g. + * `ruby-lsp [from .dispatch/lsp.json]: spawn failed`). + */ +function enrichError(server: ResolvedServer, message: string): string { + const source = server.configSource ?? "unknown"; + return `${server.id} [from ${source}]: ${message}`; +} diff --git a/packages/lsp/src/types.ts b/packages/lsp/src/types.ts index f89f84d..cc33e03 100644 --- a/packages/lsp/src/types.ts +++ b/packages/lsp/src/types.ts @@ -11,6 +11,13 @@ export interface LspServerStatus { readonly extensions: readonly string[]; readonly state: LspServerState; readonly error?: string | undefined; + /** + * Which config source this server was resolved from: `".dispatch/lsp.json"`, + * `"opencode.json"`, or `"built-in"` (the built-in TypeScript default). + * Mirrors the wire `LspServerInfo.configSource` so a broken config file + * names itself in the status response. + */ + readonly configSource?: string | undefined; } export interface LspService { diff --git a/packages/transport-contract/package.json b/packages/transport-contract/package.json index 5c2a94b..34c7fc4 100644 --- a/packages/transport-contract/package.json +++ b/packages/transport-contract/package.json @@ -1,6 +1,6 @@ { "name": "@dispatch/transport-contract", - "version": "0.20.0", + "version": "0.21.0", "type": "module", "private": true, "main": "dist/index.js", diff --git a/packages/transport-contract/src/contract.types.test.ts b/packages/transport-contract/src/contract.types.test.ts index da7e1e0..e022812 100644 --- a/packages/transport-contract/src/contract.types.test.ts +++ b/packages/transport-contract/src/contract.types.test.ts @@ -59,6 +59,15 @@ const _serverErr: LspServerInfo = { error: "Failed to start: binary not found", }; +const _serverWithSource: LspServerInfo = { + id: "ruby-lsp", + name: "Ruby LSP", + root: "/home/user/raylib", + extensions: [".rb"], + state: "connected", + configSource: ".dispatch/lsp.json", +}; + // ─── LspStatusResponse ─────────────────────────────────────────────────────── const _lspNoCwd: LspStatusResponse = { @@ -108,6 +117,11 @@ describe("transport-contract types compile and are exported", () => { expect(_serverErr.error).toBe("Failed to start: binary not found"); }); + it("LspServerInfo: carries optional configSource", () => { + expect(_serverWithSource.configSource).toBe(".dispatch/lsp.json"); + expect(_serverOk.configSource).toBeUndefined(); + }); + it("LspStatusResponse: empty servers when cwd is null", () => { expect(_lspNoCwd.servers).toEqual([]); }); diff --git a/packages/transport-contract/src/index.ts b/packages/transport-contract/src/index.ts index feeac0c..c251ae4 100644 --- a/packages/transport-contract/src/index.ts +++ b/packages/transport-contract/src/index.ts @@ -432,6 +432,13 @@ export interface LspServerInfo { readonly state: LspServerState; /** Present only when `state === "error"`: a short human-readable reason. */ readonly error?: string; + /** + * Which config source this server was resolved from: `".dispatch/lsp.json"`, + * `"opencode.json"`, or `"built-in"` (the built-in TypeScript default). Omitted + * when not yet resolved. Surfaces config-shadow debugging to the status caller + * (a broken `.dispatch/lsp.json` silently shadowing `opencode.json`). + */ + readonly configSource?: string; } /** Response of `GET /conversations/:id/lsp`. */ diff --git a/packages/transport-http/src/app.test.ts b/packages/transport-http/src/app.test.ts index 153a63a..a84fa44 100644 --- a/packages/transport-http/src/app.test.ts +++ b/packages/transport-http/src/app.test.ts @@ -364,6 +364,7 @@ function createFakeLspService( readonly extensions: readonly string[]; readonly state: "connected" | "starting" | "error" | "not-started"; readonly error?: string; + readonly configSource?: string; }[] = [], ): LspService { return { @@ -381,6 +382,7 @@ function createCapturingLspService( readonly extensions: readonly string[]; readonly state: "connected" | "starting" | "error" | "not-started"; readonly error?: string; + readonly configSource?: string; }[] = [], ): LspService & { readonly statusCalls: readonly string[] } { const calls: string[] = []; @@ -2317,6 +2319,69 @@ describe("GET /conversations/:id/lsp", () => { expect(body.servers).toHaveLength(1); expect(body.servers[0]?.id).toBe("typescript"); }); + + it("GET /conversations/:id/lsp: configSource passes through to the wire", async () => { + const cwdStore = new Map<string, string>([["conv1", "/home/user/project"]]); + const store = createFakeConversationStore(new Map(), new Map(), cwdStore); + // Case 1: configSource is defined → reaches the wire verbatim. + const lspWithSource = createFakeLspService([ + { + id: "typescript", + name: "TypeScript", + root: "/home/user/project", + extensions: [".ts", ".tsx"], + state: "connected" as const, + configSource: ".dispatch/lsp.json", + }, + ]); + const appWithSource = createApp({ + conversationStore: store, + orchestrator: createFakeOrchestrator([]), + credentialStore: createFakeCredentialStore([]), + lspService: lspWithSource, + logger: noopLogger, + }); + const resWithSource = await appWithSource.request("/conversations/conv1/lsp"); + expect(resWithSource.status).toBe(200); + const bodyWithSource = (await resWithSource.json()) as { + conversationId: string; + cwd: string | null; + servers: readonly { + readonly id: string; + readonly configSource?: string; + }[]; + }; + expect(bodyWithSource.servers[0]?.configSource).toBe(".dispatch/lsp.json"); + + // Case 2: configSource is undefined → the field is OMITTED from the + // response (proves exactOptionalPropertyTypes is respected — never + // stamping `undefined` onto the wire object). + const lspWithoutSource = createFakeLspService([ + { + id: "typescript", + name: "TypeScript", + root: "/home/user/project", + extensions: [".ts", ".tsx"], + state: "connected" as const, + }, + ]); + const appWithoutSource = createApp({ + conversationStore: store, + orchestrator: createFakeOrchestrator([]), + credentialStore: createFakeCredentialStore([]), + lspService: lspWithoutSource, + logger: noopLogger, + }); + const resWithoutSource = await appWithoutSource.request("/conversations/conv1/lsp"); + expect(resWithoutSource.status).toBe(200); + const bodyWithoutSource = (await resWithoutSource.json()) as { + conversationId: string; + cwd: string | null; + servers: readonly Record<string, unknown>[]; + }; + expect(bodyWithoutSource.servers).toHaveLength(1); + expect(bodyWithoutSource.servers[0]).not.toHaveProperty("configSource"); + }); }); describe("POST /chat reasoningEffort", () => { diff --git a/packages/transport-http/src/app.ts b/packages/transport-http/src/app.ts index 7fdbb00..2a98ea0 100644 --- a/packages/transport-http/src/app.ts +++ b/packages/transport-http/src/app.ts @@ -703,6 +703,7 @@ export function createApp(opts: CreateServerOptions): Hono { extensions: s.extensions, state: s.state, ...(s.error !== undefined ? { error: s.error } : {}), + ...(s.configSource !== undefined ? { configSource: s.configSource } : {}), }; return info; }); @@ -5,7 +5,42 @@ > Keep this lean and current; do not let it re-accrete a step-by-step changelog. ## Status (current) -`tsc -b` EXIT 0 · biome clean · **1433 vitest** green. +`tsc -b` EXIT 0 · biome clean · **1443 vitest** green. + +## LSP — broken-server recovery + config source attribution (DONE) +Handoff from an agent running in raylib-jamstack (configuring ruby-lsp under the +installed Dispatch harness `/usr/bin/dispatch-server`): two issues found by +decompiling the running binary. (Previous orchestrator session 77574596 did the +investigation + Wave 0 + wrote the prompt; its chat broke mid-summon — resumed.) +- **Issue 2 (blocker):** a failed LSP server was `broken` FOREVER — the manager's + `broken` set (keyed `${serverId}:${root}`) was cleared ONLY in `shutdownAll()`, so a + server that failed (bad env, missing binary, OR a since-fixed bad config) stayed + `state:"error"` for the whole process. For an agent running *inside* dispatch the + only recovery (server restart) kills its own session. +- **Issue 1:** `.dispatch/lsp.json` (read first) silently shadowed `opencode.json`'s + `lsp` key — a broken entry won with no warning, and the caller couldn't tell which + config source a server came from (`status()` was its only visibility). +- **Wave 0 (orchestrator, contracts):** additive `readonly configSource?: string` on + `LspServerInfo` (`@dispatch/transport-contract` `0.20.0→0.21.0`) + a type-test + assertion (8→9). tsc/biome/vitest clean. +- **Wave 1 — `lsp` extension:** (a) broken-server now self-heals when its *resolved + config changes* since it was marked broken (a config edit is a discrete event → no + retry storm; bounded backoff for transient failures); (b) `configSource?` mirrored on + `LspServerStatus` + populated in `status()` (`.dispatch/lsp.json` / `opencode.json` / + `built-in`); (c) shadow warning via `host.logger` when both configs declare lsp; (d) + spawn-failure `error` strings now name the config source. 6 required named tests + + extras. Report: (agent cut off before writing `reports/lsp.md`; work independently + verified — 50 lsp tests, tsc EXIT 0, biome clean). +- **Wave 1 CR (transport-http):** the `GET /conversations/:id/lsp` handler mapped + `LspServerStatus`→`LspServerInfo` field-by-field and DROPPED `configSource` (never + reached the wire). Summoned the transport-http owner for the one-line conditional-spread + pass-through (mirrors `error`, honors `exactOptionalPropertyTypes`) + a named pass-through + test (present + undefined-omitted). Report: `reports/transport-http.md`. +- [x] Verified: `tsc -b` EXIT 0, biome clean, **1443 vitest** pass; all agents in-lane + (only packages/lsp + transport-contract + transport-http touched; pre-existing + uncommitted WIP in kernel/tool-shell left untouched). Zero internal mocks. +- [ ] Live-verify against the dev stack (a failed server recovers after a config edit + without a restart; `configSource` visible on `GET /conversations/:id/lsp`). ## Per-conversation model persistence (DONE) Bug: a chat's selected provider + model was NOT persisted per conversation. |
