From 0a57cd5c0503bc222bf057bf3e0cd7dec1a2935a Mon Sep 17 00:00:00 2001 From: William Wang Date: Mon, 3 Aug 2026 10:40:15 +0800 Subject: [PATCH 1/6] fix: auto-reply session/requestRuntimePreferences handshake that blocks session/create --- src/backend/client.ts | 18 ++++++++++--- src/handlers/server-requests.ts | 14 +++++----- tests/backend.test.ts | 46 +++++++++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 9 deletions(-) diff --git a/src/backend/client.ts b/src/backend/client.ts index 2046cc9..cb05317 100644 --- a/src/backend/client.ts +++ b/src/backend/client.ts @@ -36,7 +36,7 @@ interface PendingRequest { /** A server→client request that we must reply to. */ export interface ServerRequest { - id: number; + id: number | string; method: string; params: ZcodeInteractionPermissionParams | ZcodeInteractionUserInputParams | Record; @@ -161,6 +161,18 @@ export class ZcodeBackend { // id + method: our pending response wins the race; else it's a server→client request. if (this.pending.has(id)) { this.resolvePending(id, msg as unknown as ZcodeResponse); + } else if (method === "session/requestRuntimePreferences") { + // Newer app-servers block `session/create` until this handshake is + // answered. Reply with defaults — no editor interaction needed. + // Keep askUserQuestionAutoResolutionEnabled false so AskUserQuestion + // still flows through the bridge's interaction path instead of being + // auto-resolved server-side. Without this reply, create hangs. + log(`backend: auto-replying ${method} (id=${String(id)}) with default preferences`); + this.sendReply(id, { + nativeSearchEnhancementsEnabled: false, + memoryEnabled: false, + askUserQuestionAutoResolutionEnabled: false, + }); } else { this.serverRequests.push({ id, @@ -251,7 +263,7 @@ export class ZcodeBackend { } /** Reply to a zcode server→client request with a result (id + result). */ - sendReply(id: number, result: unknown): void { + sendReply(id: number | string, result: unknown): void { const stdin = this.proc.stdin; if (!stdin || stdin.destroyed) { warn("backend: sendReply dropped (stdin closed)"); @@ -264,7 +276,7 @@ export class ZcodeBackend { } /** Reply to a zcode server→client request with an error. */ - sendError(id: number, code: number, message: string): void { + sendError(id: number | string, code: number, message: string): void { const stdin = this.proc.stdin; if (!stdin || stdin.destroyed) { warn("backend: sendError dropped (stdin closed)"); diff --git a/src/handlers/server-requests.ts b/src/handlers/server-requests.ts index dfb56d5..80a9f08 100644 --- a/src/handlers/server-requests.ts +++ b/src/handlers/server-requests.ts @@ -44,7 +44,7 @@ import { sendSessionUpdate } from "./io.js"; /** A dedup entry tracking reannounced zcode ids + the cached result. */ interface DedupEntry { - zcodeIds: number[]; + zcodeIds: Array; result?: ZcodeInteractionResponse; } @@ -662,7 +662,7 @@ function sendInteractionReply( backend: ZcodeBackend, pending: Map, dedupKey: string | null, - firstZcodeId: number, + firstZcodeId: number | string, result: ZcodeInteractionResponse, ): void { const ids = dedupKey && pending.has(dedupKey) ? pending.get(dedupKey)!.zcodeIds : [firstZcodeId]; @@ -682,21 +682,23 @@ function sendInteractionReply( /** Send a zcode response (result) for a server→client request id. */ function sendZcodeReply( backend: ZcodeBackend, - zcodeId: number, + zcodeId: number | string, result: ZcodeInteractionResponse, ): void { // zcode expects {id, result} — but our backend.notify sends {method, params}. Use a raw write. // The backend's notify is for notifications; replies need the id. We route via a private seam. - (backend as unknown as { sendReply: (id: number, result: unknown) => void }).sendReply( + (backend as unknown as { sendReply: (id: number | string, result: unknown) => void }).sendReply( zcodeId, result, ); } /** Send a zcode error response. */ -function sendZcodeError(backend: ZcodeBackend, zcodeId: number, message: string): void { +function sendZcodeError(backend: ZcodeBackend, zcodeId: number | string, message: string): void { ( - backend as unknown as { sendError: (id: number, code: number, message: string) => void } + backend as unknown as { + sendError: (id: number | string, code: number, message: string) => void; + } ).sendError(zcodeId, -32601, message); } diff --git a/tests/backend.test.ts b/tests/backend.test.ts index 7d3609e..4c61aeb 100644 --- a/tests/backend.test.ts +++ b/tests/backend.test.ts @@ -168,4 +168,50 @@ describe("ZcodeBackend reader routing (unit)", () => { expect((remaining[0]!.params as { sessionId: string }).sessionId).toBe("sess_b"); b.close(); }); + + it("auto-replies session/requestRuntimePreferences instead of queueing it", async () => { + const b = makeRoutingSubject(); + // Spy on what gets written back to the backend. + const writes: string[] = []; + const stdin = b.proc.stdin; + if (!stdin) throw new Error("test backend has no stdin"); + const origWrite: typeof stdin.write = stdin.write.bind(stdin); + stdin.write = ((chunk: unknown, ...args: unknown[]) => { + writes.push(String(chunk)); + return origWrite(chunk as never, ...(args as never[])); + }) as typeof stdin.write; + + b.route({ + id: "server-1", + method: "session/requestRuntimePreferences", + params: { sessionId: "sess_x", scope: "runtime-materialization" }, + }); + + // The handshake must NOT land in the server-request queue (create is + // awaiting its response; nobody drains the queue during session/new). + expect(b.pollServerRequests()).toHaveLength(0); + // And a schema-valid default reply must be written back immediately. + const written = writes.join(""); + expect(written).toContain('"id":"server-1"'); + expect(written).toContain('"nativeSearchEnhancementsEnabled":false'); + expect(written).toContain('"memoryEnabled":false'); + // Must stay false so AskUserQuestion keeps flowing through the bridge's + // interaction path instead of being auto-resolved by the app-server. + expect(written).toContain('"askUserQuestionAutoResolutionEnabled":false'); + b.close(); + }); + + it("still queues other server→client requests untouched", () => { + const b = makeRoutingSubject(); + b.route({ + id: "server-9", + method: "interaction/requestPermission", + params: { requestId: "r9", toolCallId: "t9", sessionId: "sess_x" }, + }); + const reqs = b.pollServerRequests(); + expect(reqs).toHaveLength(1); + expect(reqs[0]!.id).toBe("server-9"); + expect(reqs[0]!.method).toBe("interaction/requestPermission"); + b.close(); + }); }); From 5ab6b6c33d98cd3b51242c82667047b3036f8d2a Mon Sep 17 00:00:00 2001 From: William Wang Date: Mon, 3 Aug 2026 13:26:24 +0800 Subject: [PATCH 2/6] fix: load third-party models --- src/config/options.ts | 23 +++-- src/config/provider-registry.ts | 120 +++++++++++++++++++++++++ src/config/runtime-model.ts | 61 ++++++++++--- src/handlers/session.ts | 42 +++++++++ tests/provider-registry.test.ts | 151 ++++++++++++++++++++++++++++++++ tests/runtime-model.test.ts | 66 ++++++++++---- 6 files changed, 425 insertions(+), 38 deletions(-) create mode 100644 src/config/provider-registry.ts create mode 100644 tests/provider-registry.test.ts diff --git a/src/config/options.ts b/src/config/options.ts index 28b8419..1e39d70 100644 --- a/src/config/options.ts +++ b/src/config/options.ts @@ -48,18 +48,25 @@ export interface ModelRef { } /** - * Collect models from ALL enabled providers in config.json. + * Collect models from config.json for the dropdown. * - * Only providers with `enabled: true` are listed — unenabled providers - * (including builtins without an `enabled` flag) are excluded so the dropdown - * reflects exactly what the user has activated in the ZCode desktop app. + * Builtin providers (id prefix `builtin:`) must be `enabled: true` — they + * reflect the plans the user activated in the ZCode desktop app. Custom + * (third-party) providers are included UNLESS explicitly `enabled: false`: + * the newer CLI leaves the flag unset on active third-party providers, so + * treating "absent" as enabled keeps them in the dropdown while still + * honoring an explicit disable. */ export function loadAllModels(): ModelRef[] { try { const cfg = readConfig() as ConfigShape; const out: ModelRef[] = []; for (const [pid, p] of Object.entries(cfg.provider ?? {})) { - if (!p?.enabled) continue; + if (isBuiltinProvider(pid)) { + if (p?.enabled !== true) continue; + } else if (p?.enabled === false) { + continue; + } const providerName = p.name ?? pid; for (const modelId of Object.keys(p.models ?? {})) { out.push({ providerId: pid, providerName, modelId }); @@ -107,7 +114,7 @@ export function modelContextWindow(providerId: string, modelId: string): number } /** Builtin providerIds are prefixed with `builtin:` (e.g. `builtin:bigmodel`). */ -function isBuiltinProvider(providerId: string): boolean { +export function isBuiltinProvider(providerId: string): boolean { return providerId.startsWith("builtin:"); } @@ -330,7 +337,9 @@ export async function emitConfigOptionUpdate( size, }); } catch (e) { - log(`options: usage_update after model switch failed (${e instanceof Error ? e.message : String(e)})`); + log( + `options: usage_update after model switch failed (${e instanceof Error ? e.message : String(e)})`, + ); } } return options; diff --git a/src/config/provider-registry.ts b/src/config/provider-registry.ts new file mode 100644 index 0000000..1aee0f3 --- /dev/null +++ b/src/config/provider-registry.ts @@ -0,0 +1,120 @@ +/** + * Build a provider-registry payload for `workspace/updateProviderRegistry`. + * + * The V4 backend doesn't auto-load providers from config.json — the host must + * push them via this RPC after `session/create`, otherwise third-party + * providers fail with `provider_not_configured` (misclassified as a network + * error after turn-retry exhausts). ZCode app does this from its + * ModelProviderService; the bridge mirrors it by reading config.json directly. + * + * Provider element schema (from the backend's `j7t` converter in zcode.cjs): + * { providerId, apiKey?, apiKeyRequired?, apiFormat?, baseURL?, headers?, + * kind?, label?, models?, providerOptions?, source? } + * `apiKey` is a discriminated union `{source:"inline", value:""}` — a + * bare string is rejected. `apiFormat` maps from `kind`: + * anthropic → "anthropic-messages", openai-compatible → "openai-chat-completions". + */ + +import { readFileSync } from "node:fs"; + +import { ZCODE_CREDS_PATH, log } from "../utils.js"; + +/** A provider's raw entry in config.json (`provider.`). */ +interface ProviderEntry { + name?: string; + kind?: string; + enabled?: boolean; + source?: string; + options?: { baseURL?: string; apiKey?: string; apiKeyRequired?: boolean }; + models?: Record; +} + +interface ConfigShape { + provider?: Record; +} + +/** Registry payload for `workspace/updateProviderRegistry`. */ +export interface ProviderRegistryPayload { + providers: ReadonlyArray>; + generatedAt: number; + revision: string; +} + +/** Map config.json `kind` → backend `apiFormat`. */ +function apiFormatForKind(kind: string | undefined): string | undefined { + if (!kind) return undefined; + if (kind.includes("anthropic")) return "anthropic-messages"; + if (kind.includes("openai")) return "openai-chat-completions"; + return undefined; +} + +/** Build a single provider element from a config.json entry. */ +function buildProviderElement(providerId: string, p: ProviderEntry): Record { + // models MUST be an array of {modelId} — the backend's strict schema rejects + // the object form ({modelId: {...}}) that config.json uses. Only the id is + // required; context limits live in the backend's own model catalog. + const models = Object.keys(p.models ?? {}).map((modelId) => ({ modelId })); + const el: Record = { + providerId, + kind: p.kind, + apiFormat: apiFormatForKind(p.kind), + baseURL: p.options?.baseURL, + label: p.name ?? providerId, + models, + source: p.source ?? "custom", + }; + if (p.options?.apiKeyRequired !== undefined) { + el.apiKeyRequired = p.options.apiKeyRequired; + } + // apiKey MUST be the inline union shape — a bare string is rejected by the + // backend's strict schema. When present, the backend stores it into its + // session secrets and resolves auth from there (no separate headers callback + // needed for these providers). + if (p.options?.apiKey) { + el.apiKey = { source: "inline", value: p.options.apiKey }; + } + // Omit undefined values so the payload stays clean. + for (const k of Object.keys(el)) { + if (el[k] === undefined) delete el[k]; + } + return el; +} + +/** + * Build the registry payload from ALL providers in config.json. + * + * Unlike `loadAllModels` (dropdown, enabled-only), the registry pushes every + * configured provider so the backend recognises any of them when a session + * switches to it. The backend applies its own enable/availability rules. + */ +export function buildProviderRegistry(): ProviderRegistryPayload { + const cfg = JSON.parse(readFileSync(ZCODE_CREDS_PATH, "utf8")) as ConfigShape; + const providers = Object.entries(cfg.provider ?? {}).map(([pid, p]) => + buildProviderElement(pid, p ?? {}), + ); + const generatedAt = Date.now(); + // revision is a content hash; the backend skips unchanged revisions. A stable + // JSON hash over provider ids+kinds+baseURLs is enough — apiKey changes are + // rare and a generatedAt bump alone won't force re-apply (revision is the gate). + const revision = hashRevision(providers); + log( + `provider-registry: built ${providers.length} provider(s) ` + + `(ids: ${providers.map((p) => p.providerId).join(", ") || "none"})`, + ); + return { providers, generatedAt, revision }; +} + +/** Stable short hash over provider ids + kind + baseURL (revision gate). */ +function hashRevision(providers: ReadonlyArray>): string { + const sig = providers + .map((p) => `${p.providerId}|${p.kind ?? ""}|${p.baseURL ?? ""}`) + .sort() + .join("\n"); + // FNV-1a 32-bit → hex; cheap, dependency-free, stable. + let h = 0x811c9dc5; + for (let i = 0; i < sig.length; i++) { + h ^= sig.charCodeAt(i); + h = Math.imul(h, 0x01000193); + } + return (h >>> 0).toString(16).padStart(8, "0"); +} diff --git a/src/config/runtime-model.ts b/src/config/runtime-model.ts index a0767ad..4c36452 100644 --- a/src/config/runtime-model.ts +++ b/src/config/runtime-model.ts @@ -1,12 +1,11 @@ /** * runtimeModel overlay plumbing. * - * The runtimeModel NEVER carries `provider.apiKey`: the backend's runtimeModel - * schema (`.strict()`) types apiKey as a discriminated union object - * `{source:"inline"|"credential"|"env"|"server-config", ...}` — a bare string - * is rejected with "Invalid params". The backend resolves auth itself from - * config.json / its OAuth store, so the overlay only needs to name the - * provider+model. + * The runtimeModel names the provider+model a session should use. For THIRD- + * PARTY providers it also carries `apiKey` as `{source:"inline", value:""}`; + * the backend resolves model-call auth from the overlay itself, so omitting it + * yields HTTP 401 "Missing API key". Builtin providers keep using their own + * OAuth/config auth and never inline a key. `apiFormat` mirrors `kind`. * * Two uses: * @@ -20,9 +19,20 @@ * 2. Model switch (`applyModelSwitch`): UI/slash model switching goes through * `session/setModel` with both a `model` ref and a `runtimeModel` provider * definition (runtime-only via `persistAsWorkspaceLastUsed:false`). + * + * Note: a provider registry push (`workspace/updateProviderRegistry`) is ALSO + * required for the backend to recognise third-party providers at all — without + * it the turn fails with `provider_not_configured` before auth is even tried. + * See provider-registry.ts. */ -import { findProviderConfig, formatModelValue, loadAllModels, parseModelValue } from "./options.js"; +import { + findProviderConfig, + formatModelValue, + isBuiltinProvider, + loadAllModels, + parseModelValue, +} from "./options.js"; import type { ModelRef } from "./options.js"; import { log, warn } from "../utils.js"; import type { ZcodeAcpServer } from "../server.js"; @@ -30,7 +40,22 @@ import type { ZcodeAcpServer } from "../server.js"; const DEFAULT_KIND = "anthropic"; const DEFAULT_BASE_URL = "https://open.bigmodel.cn/api/anthropic"; -/** Build a runtimeModel overlay for the given provider+model (no apiKey). */ +/** Map config.json `kind` → backend `apiFormat`. */ +function apiFormatForKind(kind: string | undefined): string { + if (kind?.includes("anthropic")) return "anthropic-messages"; + return "openai-chat-completions"; +} + +/** + * Build a runtimeModel overlay for the given provider+model. + * + * For THIRD-PARTY providers the overlay MUST carry `apiKey` as the inline union + * `{source:"inline", value:""}` — the backend resolves model-call auth from + * the runtimeModel itself, so omitting it yields HTTP 401 "Missing API key". + * (This was previously believed unnecessary; live probing proved otherwise.) + * Builtin providers resolve auth from their own OAuth/config store, so no + * apiKey is sent for them. `apiFormat` mirrors `kind` per the backend's catalog. + */ export function buildRuntimeModel(ref: ModelRef, revision = "bridge"): unknown | null { const p = findProviderConfig(ref.providerId); if (!p) { @@ -42,16 +67,24 @@ export function buildRuntimeModel(ref: ModelRef, revision = "bridge"): unknown | Object.keys(p.models ?? {}).length > 0 ? Object.keys(p.models ?? {}).map((m) => ({ modelId: m })) : [{ modelId: ref.modelId }]; + const provider: Record = { + providerId: ref.providerId, + kind: p.kind ?? DEFAULT_KIND, + apiFormat: apiFormatForKind(p.kind), + baseURL, + models, + }; + // Third-party providers must inline their apiKey — the backend won't resolve + // it from anywhere else and the call fails with 401 without it. Builtin + // providers use OAuth/config auth and must NOT send an inline key. + if (!isBuiltinProvider(ref.providerId) && p.options?.apiKey) { + provider.apiKey = { source: "inline", value: p.options.apiKey }; + } return { revision, generatedAt: Date.now(), model: { providerId: ref.providerId, modelId: ref.modelId }, - provider: { - providerId: ref.providerId, - kind: p.kind ?? DEFAULT_KIND, - baseURL, - models, - }, + provider, }; } diff --git a/src/handlers/session.ts b/src/handlers/session.ts index 5350f35..32b8815 100644 --- a/src/handlers/session.ts +++ b/src/handlers/session.ts @@ -22,6 +22,7 @@ import type { } from "../backend/types.js"; import { buildModes, buildConfigOptions } from "../config/options.js"; import { emitInitialUsage } from "../config/model-cache.js"; +import { buildProviderRegistry } from "../config/provider-registry.js"; import { buildResumeRuntimeModel } from "../config/runtime-model.js"; import { buildDiffContent, @@ -44,6 +45,34 @@ function workspaceFor(cwd?: string): { workspacePath: string; workspaceKey: stri return { workspacePath: p, workspaceKey: p }; } +/** + * Push the provider registry to the backend so third-party providers (those in + * config.json) are recognised. The V4 backend doesn't auto-load them from + * config.json — without this RPC a session switching to a third-party model + * fails with `provider_not_configured`. Best-effort: failures are logged, not + * thrown, so a registry push problem never blocks session creation. + */ +async function syncProviderRegistry(server: ZcodeAcpServer, cwd: string): Promise { + try { + const registry = buildProviderRegistry(); + const resp = await server + .ensureBackend() + .request( + server.nextId(), + "workspace/updateProviderRegistry", + { workspace: workspaceFor(cwd), registry }, + 10000, + ); + if (resp.error) { + warn(`provider-registry: sync failed: ${resp.error.message}`); + return; + } + log("provider-registry: synced to backend"); + } catch (e) { + warn(`provider-registry: sync threw (${e instanceof Error ? e.message : String(e)})`); + } +} + /** Convert a millisecond timestamp to ISO 8601 (for session list). */ function toIso(ms: number | undefined): string | undefined { if (typeof ms !== "number") return undefined; @@ -80,6 +109,11 @@ export async function newSession( log(`session/new → ${sid}`); server.ensureBackgroundListener(sid); + // Push the provider registry so third-party providers in config.json are + // recognised by this isolated backend subprocess. Must happen before any + // model switch / turn that targets a non-builtin provider. + await syncProviderRegistry(server, cwd); + // Sync to the App's tasks-index.sqlite so the App UI shows this session. // Best-effort; failures are logged inside upsertSessionTask and swallowed. const { upsertSessionTask } = await import("../tasks-index.js"); @@ -145,6 +179,10 @@ export async function resumeSession( }; const runtimeModel = buildResumeRuntimeModel(); if (runtimeModel !== null) zcParams.runtimeModel = runtimeModel; + // Push the provider registry BEFORE resume: a resumed session may carry a + // third-party model in its history, and the backend needs the provider + // registered to even process the resume turn. + await syncProviderRegistry(server, cwd); await resumeBackendSession(server, zcParams); server.registerSession(targetSid, targetSid); @@ -180,6 +218,10 @@ export async function loadSession( }; const runtimeModel = buildResumeRuntimeModel(); if (runtimeModel !== null) zcParams.runtimeModel = runtimeModel; + // Push the provider registry BEFORE resume: a loaded session may carry a + // third-party model in its history, and the backend needs the provider + // registered to process it. + await syncProviderRegistry(server, cwd); await resumeBackendSession(server, zcParams); server.registerSession(targetSid, targetSid); log(`session/load → ${targetSid}`); diff --git a/tests/provider-registry.test.ts b/tests/provider-registry.test.ts new file mode 100644 index 0000000..6590d98 --- /dev/null +++ b/tests/provider-registry.test.ts @@ -0,0 +1,151 @@ +/** + * Tests for provider-registry payload construction. + * + * The V4 backend requires `workspace/updateProviderRegistry` to recognise + * third-party providers — without it, switching to a custom model fails with + * `provider_not_configured`. These tests lock the payload schema derived from + * the backend's `j7t` converter in zcode.cjs: apiKey is the inline union + * `{source:"inline", value}`, apiFormat maps from kind, models is an array of + * `{modelId}`, and every configured provider is included (registry is NOT + * enabled-filtered like the dropdown). + * + * All identifiers below are fictional test fixtures — no real provider names, + * URLs, model ids, or keys are used. + */ + +import { describe, expect, it, vi } from "vitest"; + +import { ZCODE_CREDS_PATH } from "../src/utils.js"; + +const FAKE_CONFIG = { + provider: { + "builtin:primary": { + name: "Primary", + kind: "anthropic", + enabled: true, + source: "builtin", + options: { baseURL: "https://example.test/primary" }, + models: { "model-a": { limit: { context: 128000 } } }, + }, + "custom-openai-kind": { + name: "Custom One", + kind: "openai-compatible", + source: "custom", + options: { + apiKey: "test-key-one", + baseURL: "https://example.test/one", + apiKeyRequired: true, + }, + models: { "custom-model-1": { limit: { context: 128000 } } }, + }, + "custom-anthropic-kind": { + name: "Custom Two", + kind: "anthropic", + source: "custom", + options: { apiKey: "test-key-two", baseURL: "https://example.test/two" }, + models: { "custom-model-2": { limit: { context: 128000 } } }, + }, + }, +}; + +vi.mock("node:fs", async () => { + const actual = await vi.importActual("node:fs"); + return { + ...actual, + readFileSync: (p: string) => { + if (p === ZCODE_CREDS_PATH) return JSON.stringify(FAKE_CONFIG); + return actual.readFileSync(p); + }, + }; +}); + +const { buildProviderRegistry } = await import("../src/config/provider-registry.js"); + +function providerById(reg: ReturnType, id: string) { + return reg.providers.find((p) => p.providerId === id); +} + +describe("buildProviderRegistry", () => { + it("includes ALL providers (not enabled-filtered like the dropdown)", () => { + const reg = buildProviderRegistry(); + const ids = reg.providers.map((p) => p.providerId); + expect(ids).toContain("builtin:primary"); + expect(ids).toContain("custom-openai-kind"); + expect(ids).toContain("custom-anthropic-kind"); + }); + + it("wraps apiKey as the inline union {source:'inline', value}, never a bare string", () => { + const reg = buildProviderRegistry(); + const p = providerById(reg, "custom-openai-kind"); + expect(p?.apiKey).toEqual({ source: "inline", value: "test-key-one" }); + // A bare string would be rejected by the backend's strict schema. + expect(typeof p?.apiKey).toBe("object"); + }); + + it("omits apiKey when the provider has none (builtin OAuth)", () => { + const reg = buildProviderRegistry(); + const p = providerById(reg, "builtin:primary"); + expect(p?.apiKey).toBeUndefined(); + }); + + it("maps kind openai-compatible → apiFormat openai-chat-completions", () => { + const reg = buildProviderRegistry(); + const p = providerById(reg, "custom-openai-kind"); + expect(p?.apiFormat).toBe("openai-chat-completions"); + expect(p?.kind).toBe("openai-compatible"); + }); + + it("maps kind anthropic → apiFormat anthropic-messages", () => { + const reg = buildProviderRegistry(); + const p = providerById(reg, "custom-anthropic-kind"); + expect(p?.apiFormat).toBe("anthropic-messages"); + }); + + it("carries baseURL, label, models, source, and apiKeyRequired", () => { + const reg = buildProviderRegistry(); + const p = providerById(reg, "custom-openai-kind"); + expect(p?.baseURL).toBe("https://example.test/one"); + expect(p?.label).toBe("Custom One"); + expect(p?.source).toBe("custom"); + expect(p?.apiKeyRequired).toBe(true); + expect(p?.models).toEqual([{ modelId: "custom-model-1" }]); + }); + + it("serialises models as an array of {modelId}, NOT the config.json object form", () => { + const reg = buildProviderRegistry(); + const p = providerById(reg, "custom-openai-kind"); + expect(Array.isArray(p?.models)).toBe(true); + // The object form ({modelId: {...}}) is rejected by the backend's strict schema. + expect(p?.models).not.toEqual({ "custom-model-1": expect.anything() }); + }); + + it("produces a stable revision for the same input", () => { + const a = buildProviderRegistry(); + const b = buildProviderRegistry(); + expect(a.revision).toBe(b.revision); + expect(a.revision).toMatch(/^[0-9a-f]+$/); + }); + + it("produces a different revision when providers change", () => { + const a = buildProviderRegistry(); + const cfg2 = JSON.parse(JSON.stringify(FAKE_CONFIG)); + cfg2.provider["custom-openai-kind"].kind = "anthropic"; + vi.doMock("node:fs", () => ({ + readFileSync: () => JSON.stringify(cfg2), + })); + // vi.doMock takes effect on next dynamic import; reset and reimport. + vi.resetModules(); + return import("../src/config/provider-registry.js").then((m2) => { + const b = ( + m2 as { buildProviderRegistry: typeof buildProviderRegistry } + ).buildProviderRegistry(); + expect(b.revision).not.toBe(a.revision); + }); + }); + + it("emits a generatedAt timestamp", () => { + const reg = buildProviderRegistry(); + expect(typeof reg.generatedAt).toBe("number"); + expect(reg.generatedAt).toBeGreaterThan(0); + }); +}); diff --git a/tests/runtime-model.test.ts b/tests/runtime-model.test.ts index 4ad3a4a..8a4a252 100644 --- a/tests/runtime-model.test.ts +++ b/tests/runtime-model.test.ts @@ -4,9 +4,13 @@ * History: loadProviderModels() hardcoded a single builtin provider id, so * custom providers configured in the ZCode desktop app never appeared in the * dropdown. These tests lock the new behaviour: loadAllModels() aggregates ALL - * enabled providers, buildRuntimeModel() NEVER carries apiKey (the backend's - * runtimeModel schema rejects it), builtin models encode as bare modelIds, and - * third-party models carry their providerId prefix. + * enabled builtin providers PLUS every custom provider (the newer CLI no + * longer sets `enabled` on third-party providers, so filtering on it would + * drop them), buildRuntimeModel() inlines apiKey as {source:"inline",value} + * for third-party providers (the backend resolves model-call auth from the + * overlay itself; omitting it yields HTTP 401) but omits it for builtins. + * Builtin models encode as bare modelIds, and third-party models carry their + * providerId prefix. */ import { describe, expect, it, vi } from "vitest"; @@ -53,6 +57,14 @@ const FAKE_CONFIG = { options: { apiKey: "test-key-beta", baseURL: "https://example.test/api" }, models: { "beta-1": { limit: { context: 200000 } } }, }, + "custom-provider-gamma": { + name: "Gamma", + kind: "openai-compatible", + enabled: false, + source: "custom", + options: { apiKey: "test-key-gamma", baseURL: "http://127.0.0.1:8001/v1" }, + models: { "gamma-1": { limit: { context: 200000 } } }, + }, }, }; @@ -76,16 +88,27 @@ const { loadAllModels, modelContextWindow, parseModelValue, formatModelValue, bu }); describe("loadAllModels", () => { - it("only collects models from enabled providers", () => { + it("collects enabled builtins + active custom providers", () => { const models = loadAllModels(); const ids = models.map((m) => m.modelId); - // Enabled builtin + enabled custom. The disabled Secondary and the - // custom-without-enabled (Beta) must NOT appear. + // Enabled builtin + enabled custom appear. expect(ids).toContain("model-a"); expect(ids).toContain("model-b"); expect(ids).toContain("alpha-1"); - // beta-1 is in a provider WITHOUT an enabled flag → excluded. - expect(ids).not.toContain("beta-1"); + // Disabled builtin (Secondary) stays out — its model-a never duplicates. + expect(ids.filter((id) => id === "model-a")).toHaveLength(1); + // beta-1 (custom WITHOUT an enabled flag) is included: the newer CLI leaves + // `enabled` unset on active third-party providers, so "absent" = enabled. + expect(ids).toContain("beta-1"); + // gamma-1 (custom with an EXPLICIT enabled:false) is excluded. + expect(ids).not.toContain("gamma-1"); + }); + + it("tracks provider identity for every custom provider", () => { + const models = loadAllModels(); + const beta = models.find((m) => m.modelId === "beta-1"); + expect(beta?.providerName).toBe("Beta"); + expect(beta?.providerId).toBe("custom-provider-beta"); }); it("carries the provider name for display", () => { @@ -140,29 +163,38 @@ describe("parseModelValue / formatModelValue", () => { }); describe("buildRuntimeModel", () => { - it("NEVER carries apiKey — the backend schema rejects it and resolves auth itself", () => { - // Even for a custom provider WITH an apiKey in config (custom-provider-alpha), - // the overlay must omit it: the backend's runtimeModel schema types apiKey as - // a discriminated-union object, and a bare string → "Invalid params". + it("inlines apiKey as {source:'inline', value} for third-party providers", () => { + // The backend resolves model-call auth from the runtimeModel itself — a + // third-party overlay WITHOUT apiKey yields HTTP 401 "Missing API key". + // apiKey is the inline union, never a bare string (the strict schema rejects it). const rm = buildRuntimeModel({ providerId: "custom-provider-alpha", providerName: "Alpha", modelId: "alpha-1", - }) as { provider: { apiKey?: string; baseURL?: string; kind?: string } }; - - expect(rm.provider.apiKey).toBeUndefined(); + }) as { + provider: { + apiKey?: { source: string; value: string }; + baseURL?: string; + kind?: string; + apiFormat?: string; + }; + }; + + expect(rm.provider.apiKey).toEqual({ source: "inline", value: "test-key-alpha" }); expect(rm.provider.baseURL).toBe("http://127.0.0.1:8000/v1"); expect(rm.provider.kind).toBe("openai-compatible"); + expect(rm.provider.apiFormat).toBe("openai-chat-completions"); }); - it("omits apiKey for builtin OAuth providers (no apiKey in config)", () => { + it("omits apiKey for builtin OAuth providers (auth resolved from config/OAuth)", () => { const rm = buildRuntimeModel({ providerId: "builtin:primary", providerName: "Primary", modelId: "model-a", - }) as { provider: { apiKey?: string } }; + }) as { provider: { apiKey?: string; apiFormat?: string } }; expect(rm.provider.apiKey).toBeUndefined(); + expect(rm.provider.apiFormat).toBe("anthropic-messages"); }); it("returns null for an unknown provider", () => { From 90a890d9775609f5f6b915bae04023ce168cd16a Mon Sep 17 00:00:00 2001 From: William Wang Date: Mon, 3 Aug 2026 13:46:27 +0800 Subject: [PATCH 3/6] feat: defer session/create until first use of a session --- docs/ARCHITECTURE.md | 20 +++-- docs/PROTOCOL.md | 5 +- src/config/options.ts | 69 ++++++++++-------- src/handlers/extensions.ts | 37 ++++++---- src/handlers/session.ts | 145 ++++++++++++++++++++++++------------- src/server.ts | 13 ++++ tests/session-lazy.test.ts | 137 +++++++++++++++++++++++++++++++++++ 7 files changed, 325 insertions(+), 101 deletions(-) create mode 100644 tests/session-lazy.test.ts diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index f132f38..2a3ce00 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -41,10 +41,12 @@ resume / fork`. The prompt capabilities (image/audio/embeddedContext) and never supplies credentials. Omitting the `type` field defaults to `"agent"`, which the ACP registry's auth-check accepts as "agent self-handles auth". -`initialize` does **not** spawn the backend. The backend is lazily created on -the first `session/new` (via `ensureBackend()`), so the handshake succeeds -even in an environment without `~/.zcode/v2/config.json` (e.g. the registry -CI runs `initialize` with an isolated `HOME`). +`initialize` does **not** spawn the backend, and neither does `session/new`: +the backend is lazily created on the first backend RPC — for a fresh session +that is the first `session/create` at its first use (prompt / config change / +extension method). This keeps the handshake succeeding even in an environment +without `~/.zcode/v2/config.json` (e.g. the registry CI runs `initialize` with +an isolated `HOME`). Client capabilities advertised at `initialize` are recorded on the server (`clientCapabilities`) and drive later behaviour: `supportsElicitationForm()` @@ -56,13 +58,21 @@ terminal UI. ### 1. Session lifecycle ``` -session/new → session/create → register EventListener +session/new → placeholder id (backend session NOT created yet) + | +first use: prompt / set_config_option / extension method + | + session/create → register EventListener | prompt request → session/send → EventTranslator translates → dispatchEvent | | end_turn / cancelled session/update notification ``` +Sessions are materialized lazily (`ensureRealSession`): an editor startup that +never sends a message leaves no empty session in the backend or the App's task +index. + ### 2. Event stream subscription ``` diff --git a/docs/PROTOCOL.md b/docs/PROTOCOL.md index 57a86d5..48ea95c 100644 --- a/docs/PROTOCOL.md +++ b/docs/PROTOCOL.md @@ -78,7 +78,10 @@ Messages are classified by the presence of `id` and `method`: ### `session/create` -Create a new session. +Create a new session. Note: the bridge defers this call until a session's +first use — ACP `session/new` returns a local placeholder id and materializes +the backend session (this RPC) on the first prompt / config change / extension +method, so an editor startup that never sends a message leaves no session. **Request:** ```json diff --git a/src/config/options.ts b/src/config/options.ts index 1e39d70..a5beb5d 100644 --- a/src/config/options.ts +++ b/src/config/options.ts @@ -151,19 +151,23 @@ export function parseModelValue(value: string): { providerId: string; modelId: s return { providerId: value.slice(0, idx), modelId: value.slice(idx + 1) }; } -/** Build the ACP SessionModeState ({currentModeId, availableModes}). */ +/** Build the ACP SessionModeState ({currentModeId, availableModes}). + * zcodeSid null = pending session (session/new not yet materialized) — skip + * the backend read and return defaults. */ export async function buildModes( server: ZcodeAcpServer, - zcodeSid: string, + zcodeSid: string | null, ): Promise { let currentMode = "yolo"; - try { - const read = await sessionRead(server, zcodeSid); - const settings = (read.settings ?? {}) as Record; - const modeSet = (settings.mode as Record) ?? {}; - currentMode = (modeSet.current as string) ?? currentMode; - } catch { - // keep default + if (zcodeSid !== null) { + try { + const read = await sessionRead(server, zcodeSid); + const settings = (read.settings ?? {}) as Record; + const modeSet = (settings.mode as Record) ?? {}; + currentMode = (modeSet.current as string) ?? currentMode; + } catch { + // keep default + } } return { currentModeId: currentMode, @@ -177,36 +181,41 @@ export async function buildModes( }; } -/** Build the ACP configOptions array (3 items: model/mode/thought). */ +/** Build the ACP configOptions array (3 items: model/mode/thought). + * zcodeSid null = pending session — skip the backend read and use defaults; + * mode defaults to "yolo" (the mode session/create hardcodes) so the dropdown + * matches the mode indicator for a fresh session. */ export async function buildConfigOptions( server: ZcodeAcpServer, - zcodeSid: string, + zcodeSid: string | null, ): Promise { let currentProviderId = ""; let currentModelId = "GLM-5.2"; - let currentMode = "build"; + let currentMode = zcodeSid === null ? "yolo" : "build"; let currentThought = "high"; let thoughtOptions: Array<{ value: string; name: string }> | null = null; - try { - const read = await sessionRead(server, zcodeSid); - const settings = (read.settings ?? {}) as Record; - const modeSet = (settings.mode as Record) ?? {}; - currentMode = (modeSet.current as string) ?? currentMode; - const modelSet = (settings.model as Record) ?? {}; - // settings.model.current is { providerId, modelId, variant? } — read BOTH so - // we can disambiguate same-named models across providers. - const cur = (modelSet.current as { providerId?: string; modelId?: string }) ?? {}; - if (cur.providerId) currentProviderId = cur.providerId; - if (cur.modelId) currentModelId = cur.modelId; - const tlSet = (settings.thoughtLevel as Record) ?? {}; - currentThought = (tlSet.current as string) ?? currentThought; - const tlAvail = (tlSet.available as Array>) ?? []; - if (tlAvail.length > 0) { - thoughtOptions = tlAvail.map((a) => ({ value: a.value, name: a.label ?? a.value })); + if (zcodeSid !== null) { + try { + const read = await sessionRead(server, zcodeSid); + const settings = (read.settings ?? {}) as Record; + const modeSet = (settings.mode as Record) ?? {}; + currentMode = (modeSet.current as string) ?? currentMode; + const modelSet = (settings.model as Record) ?? {}; + // settings.model.current is { providerId, modelId, variant? } — read BOTH so + // we can disambiguate same-named models across providers. + const cur = (modelSet.current as { providerId?: string; modelId?: string }) ?? {}; + if (cur.providerId) currentProviderId = cur.providerId; + if (cur.modelId) currentModelId = cur.modelId; + const tlSet = (settings.thoughtLevel as Record) ?? {}; + currentThought = (tlSet.current as string) ?? currentThought; + const tlAvail = (tlSet.available as Array>) ?? []; + if (tlAvail.length > 0) { + thoughtOptions = tlAvail.map((a) => ({ value: a.value, name: a.label ?? a.value })); + } + } catch { + // keep defaults } - } catch { - // keep defaults } // currentValue encodes provider+model so the switch handler can locate the diff --git a/src/handlers/extensions.ts b/src/handlers/extensions.ts index 2e9654f..c400c82 100644 --- a/src/handlers/extensions.ts +++ b/src/handlers/extensions.ts @@ -20,6 +20,7 @@ import { ProjectionDiffer } from "../translators/projection-differ.js"; import { log, warn } from "../utils.js"; import type { ZcodeAcpServer } from "../server.js"; import { sendSessionUpdate } from "./io.js"; +import { ensureRealSession } from "./session.js"; /** Build the zcode `target` object from ACP params (checkpoint or latest). */ function buildCheckpointTarget(params: ExtensionParams): unknown { @@ -28,11 +29,15 @@ function buildCheckpointTarget(params: ExtensionParams): unknown { return { kind: "latestCheckpoint" }; } -/** Resolve zcode sid from ACP params; throw if unknown. */ -function resolveSidOrThrow(server: ZcodeAcpServer, params: { sessionId: string }): string { - const sid = server.resolveSid(params.sessionId); - if (!sid) throw new Error(`session ${params.sessionId} not found`); - return sid; +/** + * Resolve zcode sid from ACP params, materializing a lazy session/new + * placeholder on first use; throw if the session is unknown. + */ +async function resolveSidOrThrow( + server: ZcodeAcpServer, + params: { sessionId: string }, +): Promise { + return ensureRealSession(server, params.sessionId); } interface ExtensionParams { @@ -44,7 +49,7 @@ type Result = Record; /** session/fork → zcode session/fork: branch a new session from a checkpoint. */ export async function fork(server: ZcodeAcpServer, params: ExtensionParams): Promise { - const zcodeSid = resolveSidOrThrow(server, params); + const zcodeSid = await resolveSidOrThrow(server, params); const backend = server.ensureBackend(); const resp = await backend.request( server.nextId(), @@ -66,7 +71,7 @@ export async function fork(server: ZcodeAcpServer, params: ExtensionParams): Pro /** session/rewind → zcode session/rewind: restore workspace files to a checkpoint. */ export async function rewind(server: ZcodeAcpServer, params: ExtensionParams): Promise { - const zcodeSid = resolveSidOrThrow(server, params); + const zcodeSid = await resolveSidOrThrow(server, params); const zcParams: Record = { sessionId: zcodeSid, target: buildCheckpointTarget(params), @@ -85,7 +90,7 @@ export async function rewindCascade( server: ZcodeAcpServer, params: ExtensionParams, ): Promise { - const zcodeSid = resolveSidOrThrow(server, params); + const zcodeSid = await resolveSidOrThrow(server, params); const zcParams: Record = { sessionId: zcodeSid, target: buildCheckpointTarget(params), @@ -102,7 +107,7 @@ export async function rewindCascade( /** session/goal → zcode session/goal: read/set/replace/clear/pause/resume the goal. */ export async function goal(server: ZcodeAcpServer, params: ExtensionParams): Promise { - const zcodeSid = resolveSidOrThrow(server, params); + const zcodeSid = await resolveSidOrThrow(server, params); const action = (params.action as string) ?? "show"; const zcParams: Record = { sessionId: zcodeSid, action }; if ((action === "set" || action === "replace") && params.objective !== undefined) { @@ -135,7 +140,7 @@ export async function compact( cx: acp.AgentContext, ): Promise { const acpSid = params.sessionId; - const zcodeSid = resolveSidOrThrow(server, params); + const zcodeSid = await resolveSidOrThrow(server, params); const resp = await server .ensureBackend() .request(server.nextId(), "session/compact", { sessionId: zcodeSid }, 30000); @@ -170,7 +175,7 @@ export async function compact( /** session/steer → zcode session/steer: append instructions to a running turn. */ export async function steer(server: ZcodeAcpServer, params: ExtensionParams): Promise { - const zcodeSid = resolveSidOrThrow(server, params); + const zcodeSid = await resolveSidOrThrow(server, params); const content = String(params.content ?? ""); if (!content.trim()) throw new Error("steer requires content"); const resp = await server @@ -187,7 +192,7 @@ export async function cancelBackgroundTask( server: ZcodeAcpServer, params: ExtensionParams, ): Promise { - const zcodeSid = resolveSidOrThrow(server, params); + const zcodeSid = await resolveSidOrThrow(server, params); const taskId = String(params.taskId ?? ""); if (!taskId) throw new Error("cancelBackgroundTask requires taskId"); const resp = await server @@ -214,7 +219,7 @@ export async function setThoughtLevel( server: ZcodeAcpServer, params: ExtensionParams, ): Promise { - const zcodeSid = resolveSidOrThrow(server, params); + const zcodeSid = await resolveSidOrThrow(server, params); // 3.3.0 marks thoughtLevel optional (omitting it resets to the model's // default). Forward it only when present so a reset call isn't rejected. const zcParams: Record = { sessionId: zcodeSid }; @@ -232,7 +237,7 @@ export async function updateRuntimeModelConfig( server: ZcodeAcpServer, params: ExtensionParams, ): Promise { - const zcodeSid = resolveSidOrThrow(server, params); + const zcodeSid = await resolveSidOrThrow(server, params); const runtimeModel = params.runtimeModel; if (!runtimeModel) throw new Error("updateRuntimeModelConfig requires runtimeModel"); const zcParams: Record = { sessionId: zcodeSid, runtimeModel }; @@ -248,7 +253,7 @@ export async function updateRuntimeModelConfig( /** session/setModel → applyModelSwitch (runtime overlay, not persistence). */ export async function setModel(server: ZcodeAcpServer, params: ExtensionParams): Promise { - const zcodeSid = resolveSidOrThrow(server, params); + const zcodeSid = await resolveSidOrThrow(server, params); const modelId = params.modelId as string; if (!modelId) throw new Error("setModel requires modelId"); const ok = await applyModelSwitch(server, zcodeSid, modelId); @@ -264,7 +269,7 @@ export async function setMode( cx: acp.AgentContext, ): Promise { const acpSid = params.sessionId; - const zcodeSid = resolveSidOrThrow(server, params); + const zcodeSid = await resolveSidOrThrow(server, params); const mode = params.mode; if (!mode) throw new Error("setMode requires mode"); const resp = await server diff --git a/src/handlers/session.ts b/src/handlers/session.ts index 32b8815..02c0d72 100644 --- a/src/handlers/session.ts +++ b/src/handlers/session.ts @@ -1,10 +1,14 @@ /** * Session lifecycle handlers: initialize, new, list, resume, load, prompt, cancel. * - * These map ACP session methods to ZCode app-server calls. `session/prompt` runs - * the event-driven turn loop (subscribe-before-send ordering, no-progress - * timeout, stall reconciliation). ZCode events are translated via - * EventTranslator and dispatched as ACP `session/update` notifications. + * These map ACP session methods to ZCode app-server calls. `session/new` is + * lazy: it returns a placeholder id and defers zcode `session/create` to the + * session's first use (`ensureRealSession`), so an editor startup that never + * prompts leaves no empty session in the backend or the App's task index. + * `session/prompt` runs the event-driven turn loop (subscribe-before-send + * ordering, no-progress timeout, stall reconciliation). ZCode events are + * translated via EventTranslator and dispatched as ACP `session/update` + * notifications. */ import process from "node:process"; @@ -79,60 +83,101 @@ function toIso(ms: number | undefined): string | undefined { return new Date(ms).toISOString(); } -/** `session/new` → zcode `session/create` (mode hardcoded yolo). */ +/** + * `session/new` → local placeholder id. The real zcode `session/create` is + * deferred to first use (`ensureRealSession`) so an editor startup that never + * sends a message leaves no empty session in the backend or the App's task + * index. The created session uses mode yolo (hardcoded). + */ export async function newSession( server: ZcodeAcpServer, params: acp.NewSessionRequest, ): Promise { - const backend = server.ensureBackend(); const cwd = params.cwd ?? process.cwd(); - log(`session/new: cwd=${cwd}`); - - const resp = await backend.request( - server.nextId(), - "session/create", - { workspace: workspaceFor(cwd), mode: "yolo" }, - 15000, - ); - if (resp.error) { - throw new Error(`zcode create failed: ${resp.error.message ?? ""}`); - } - const result = (resp.result ?? {}) as ZcodeCreateResult; - const session = result.session ?? {}; - const sid = session.sessionId; - if (!sid) throw new Error("zcode create returned no sessionId"); - - server.registerSession(sid, sid); + // Placeholder id — the client addresses this session with it until the + // backend session materializes; never shown in session/list. + const acpSid = randomUUID(); + server.pendingSessions.set(acpSid, { cwd }); // Only freshly-created sessions are eligible for auto-title on first // end_turn; resumed/loaded sessions already have a title and must keep it. - server.titleEligibleSessions.add(sid); - log(`session/new → ${sid}`); - server.ensureBackgroundListener(sid); - - // Push the provider registry so third-party providers in config.json are - // recognised by this isolated backend subprocess. Must happen before any - // model switch / turn that targets a non-builtin provider. - await syncProviderRegistry(server, cwd); - - // Sync to the App's tasks-index.sqlite so the App UI shows this session. - // Best-effort; failures are logged inside upsertSessionTask and swallowed. - const { upsertSessionTask } = await import("../tasks-index.js"); - void upsertSessionTask({ - workspaceKey: cwd, - taskId: sid, - title: session.title ?? "", - traceId: session.traceId, - }); + server.titleEligibleSessions.add(acpSid); + log(`session/new (lazy) → ${acpSid} cwd=${cwd}`); - const modes = await buildModes(server, sid); - server.lastMode.set(sid, modes.currentModeId); + // No backend RPC yet: modes/configOptions are built from defaults (the + // pending session's real values arrive via updates once materialized). + const modes = await buildModes(server, null); + server.lastMode.set(acpSid, modes.currentModeId); return { - sessionId: sid, + sessionId: acpSid, modes, - configOptions: await buildConfigOptions(server, sid), + configOptions: await buildConfigOptions(server, null), }; } +/** + * Materialize a lazy `session/new` placeholder into a real backend session on + * first use (prompt / set_config_option / extension methods). Idempotent: + * returns the existing mapping for already-created sessions, and concurrent + * first-uses share a single `session/create` via the pending entry's `creating` + * promise. Unknown ids throw. + */ +export async function ensureRealSession(server: ZcodeAcpServer, acpSid: string): Promise { + const existing = server.resolveSid(acpSid); + if (existing) return existing; + const pending = server.pendingSessions.get(acpSid); + if (!pending) throw new Error(`session ${acpSid} not found`); + if (pending.creating) return pending.creating; + + // The create body runs synchronously up to its first await, so the `creating` + // promise is stored before any concurrent caller can observe the entry. + const creating = (async () => { + const backend = server.ensureBackend(); + const resp = await backend.request( + server.nextId(), + "session/create", + { workspace: workspaceFor(pending.cwd), mode: "yolo" }, + 15000, + ); + if (resp.error) { + throw new Error(`zcode create failed: ${resp.error.message ?? ""}`); + } + const result = (resp.result ?? {}) as ZcodeCreateResult; + const session = result.session ?? {}; + const sid = session.sessionId; + if (!sid) throw new Error("zcode create returned no sessionId"); + + server.pendingSessions.delete(acpSid); + server.registerSession(acpSid, sid); + log(`session/new ${acpSid} → created ${sid} (lazy, on first use)`); + server.ensureBackgroundListener(sid); + + // Push the provider registry so third-party providers in config.json are + // recognised by this isolated backend subprocess. Must happen before any + // model switch / turn that targets a non-builtin provider. + await syncProviderRegistry(server, pending.cwd); + + // Sync to the App's tasks-index.sqlite so the App UI shows this session. + // Best-effort; failures are logged inside upsertSessionTask and swallowed. + const { upsertSessionTask } = await import("../tasks-index.js"); + void upsertSessionTask({ + workspaceKey: pending.cwd, + taskId: sid, + title: session.title ?? "", + traceId: session.traceId, + }); + + return sid; + })(); + pending.creating = creating; + try { + return await creating; + } finally { + // Reset the in-flight marker (on success the sessionMap short-circuits + // later calls; on failure this lets the next use retry the create). + pending.creating = undefined; + } +} + /** `session/list` → zcode `session/list`. */ export async function listSessions( server: ZcodeAcpServer, @@ -315,8 +360,6 @@ export async function prompt( requestId: number, ): Promise { const backend = server.ensureBackend(); - const zcodeSid = server.resolveSid(params.sessionId); - if (!zcodeSid) throw new Error(`session ${params.sessionId} not found`); // Extract prompt text + image attachments from ACP ContentBlock[]. const text = extractPromptText(params.prompt); @@ -325,6 +368,10 @@ export async function prompt( // may drag in an image with no accompanying text). if (!text && attachments.length === 0) throw new Error("empty prompt"); + // Materialize a lazy session/new placeholder on first use. Placed after the + // empty-prompt check so an invalid request doesn't create a backend session. + const zcodeSid = await ensureRealSession(server, params.sessionId); + // Slash-command interception: dispatches directly to ZCode methods and // returns end_turn without entering the turn loop. Unknown /x falls through. const { handleSlashCommand } = await import("./slash.js"); @@ -528,11 +575,11 @@ export async function setConfigOptionHandler( params: acp.SetSessionConfigOptionRequest, cx: acp.AgentContext, ): Promise { - const zcodeSid = server.resolveSid(params.sessionId); - if (!zcodeSid) throw new Error(`session ${params.sessionId} not found`); if (typeof params.value !== "string") { throw new Error(`unsupported config value type: ${String(params.value)}`); } + // Materialize a lazy session/new placeholder on first use. + const zcodeSid = await ensureRealSession(server, params.sessionId); const { setConfigOption, emitConfigOptionUpdate } = await import("../config/options.js"); const result = await setConfigOption(server, zcodeSid, params.configId, params.value); if (!result) { diff --git a/src/server.ts b/src/server.ts index 12462b8..2c57cc4 100644 --- a/src/server.ts +++ b/src/server.ts @@ -48,6 +48,19 @@ export class ZcodeAcpServer { * index rather than assuming equality. */ readonly acpSidByZcodeSid = new Map(); + /** + * Sessions returned by `session/new` whose backend session has not been + * created yet (acp_sid → { cwd }). session/new defers `session/create` until + * the session is first used, so an editor startup that never prompts leaves + * no empty session in the backend or the App's task index. `ensureRealSession` + * materializes these on first use; entries live only as long as the bridge + * process (a never-used placeholder vanishes with it). + * + * `creating` holds the in-flight materialization promise while a first-use + * create is running, so concurrent first-uses (e.g. a raced double prompt) + * share one `session/create` instead of creating two backend sessions. + */ + readonly pendingSessions = new Map }>(); /** Currently running turns, keyed by the ACP request id. */ readonly pendingTurns = new Map(); /** diff --git a/tests/session-lazy.test.ts b/tests/session-lazy.test.ts new file mode 100644 index 0000000..950f61d --- /dev/null +++ b/tests/session-lazy.test.ts @@ -0,0 +1,137 @@ +/** + * Lazy session creation tests. + * + * session/new returns a placeholder id and defers zcode `session/create` to + * first use (ensureRealSession), so an editor startup that never prompts + * leaves no session in the backend or the App's task index. + */ + +import type * as acp from "@agentclientprotocol/sdk"; +import { describe, expect, it, vi } from "vitest"; + +import type { ZcodeBackend } from "../src/backend/client.js"; +import { ensureRealSession, newSession } from "../src/handlers/session.js"; +import { ZcodeAcpServer } from "../src/server.js"; + +// Record tasks-index upserts so tests can assert the App sync happens at +// materialization (never at session/new). The real module writes the App's +// ~/.zcode/v2/tasks-index.sqlite and must not be touched by tests. +const mockUpsertCalls: Array> = []; +vi.mock("../src/tasks-index.js", () => ({ + upsertSessionTask: async (opts: Record) => { + mockUpsertCalls.push(opts); + return true; + }, + updateSessionTitle: async () => true, +})); + +/** Fake backend: answers session/create, errors on everything else. */ +function fakeBackend(): ZcodeBackend & { + calls: Array<{ method: string; params: unknown }>; +} { + const calls: Array<{ method: string; params: unknown }> = []; + let created = 0; + const backend = { + isDead: false, + request: async (id: number, method: string, params: unknown) => { + calls.push({ method, params }); + if (method === "session/create") { + created += 1; + return { + id, + result: { session: { sessionId: `sess_lazy_${created}`, title: "", traceId: "trace_1" } }, + }; + } + return { id, error: { message: `unhandled ${method}` } }; + }, + registerEventListener: () => {}, + unregisterEventListener: () => {}, + } as unknown as ZcodeBackend; + return { backend, calls }; +} + +function newSessionParams(cwd: string): acp.NewSessionRequest { + return { cwd } as acp.NewSessionRequest; +} + +describe("session/new lazy creation", () => { + it("returns a placeholder id without spawning the backend or creating a zcode session", async () => { + const server = new ZcodeAcpServer(); + const resp = await newSession(server, newSessionParams("/tmp/ws")); + + expect(server.backend).toBeNull(); + expect(resp.sessionId).toBeTruthy(); + expect(server.pendingSessions.get(resp.sessionId)).toEqual({ cwd: "/tmp/ws" }); + expect(server.resolveSid(resp.sessionId)).toBeUndefined(); + expect(mockUpsertCalls).toHaveLength(0); + // Fresh sessions stay auto-title-eligible on first end_turn. + expect(server.titleEligibleSessions.has(resp.sessionId)).toBe(true); + }); + + it("returns default modes/configOptions consistent with the yolo create", async () => { + const server = new ZcodeAcpServer(); + const resp = await newSession(server, newSessionParams("/tmp/ws")); + + expect(resp.modes.currentModeId).toBe("yolo"); + const modeOpt = resp.configOptions.find((o) => o.id === "mode"); + expect(modeOpt?.currentValue).toBe("yolo"); + }); +}); + +describe("ensureRealSession", () => { + it("materializes the backend session once on first use and registers the mapping", async () => { + const server = new ZcodeAcpServer(); + const resp = await newSession(server, newSessionParams("/tmp/ws")); + const { backend, calls } = fakeBackend(); + server.backend = backend; + + const sid = await ensureRealSession(server, resp.sessionId); + expect(sid).toBe("sess_lazy_1"); + expect(server.resolveSid(resp.sessionId)).toBe(sid); + expect(server.pendingSessions.has(resp.sessionId)).toBe(false); + + const creates = calls.filter((c) => c.method === "session/create"); + expect(creates).toHaveLength(1); + expect(creates[0].params).toMatchObject({ + workspace: { workspacePath: "/tmp/ws", workspaceKey: "/tmp/ws" }, + mode: "yolo", + }); + expect(mockUpsertCalls).toHaveLength(1); + expect(mockUpsertCalls[0]).toMatchObject({ workspaceKey: "/tmp/ws", taskId: sid }); + + // Idempotent: a second call reuses the mapping, no new create. + await expect(ensureRealSession(server, resp.sessionId)).resolves.toBe(sid); + expect(calls.filter((c) => c.method === "session/create")).toHaveLength(1); + }); + + it("serializes concurrent first-uses into a single session/create", async () => { + const server = new ZcodeAcpServer(); + const resp = await newSession(server, newSessionParams("/tmp/ws")); + const { backend, calls } = fakeBackend(); + server.backend = backend; + + const [sidA, sidB] = await Promise.all([ + ensureRealSession(server, resp.sessionId), + ensureRealSession(server, resp.sessionId), + ]); + expect(sidA).toBe(sidB); + expect(calls.filter((c) => c.method === "session/create")).toHaveLength(1); + }); + + it("throws for unknown session ids", async () => { + const server = new ZcodeAcpServer(); + await expect(ensureRealSession(server, "sess_unknown")).rejects.toThrow( + "session sess_unknown not found", + ); + }); + + it("returns the mapping for already-registered sessions without creating", async () => { + const server = new ZcodeAcpServer(); + server.registerSession("acp_existing", "sess_existing"); + const { backend, calls } = fakeBackend(); + server.backend = backend; + + await expect(ensureRealSession(server, "acp_existing")).resolves.toBe("sess_existing"); + expect(calls.filter((c) => c.method === "session/create")).toHaveLength(0); + }); +}); From 7a4185a9aa4af0ca7b89cdad95c159a1dea07768 Mon Sep 17 00:00:00 2001 From: William Wang Date: Mon, 3 Aug 2026 15:20:58 +0800 Subject: [PATCH 4/6] fix: propagate mid-turn model/mode/thought switches via state.updated --- src/backend/client.ts | 48 ++++++++++++++++++-------- src/backend/types.ts | 4 +++ src/handlers/dispatch.ts | 52 +++++++++++++++++++++++++++- src/translators/event-translator.ts | 30 ++++++++++++++++ src/translators/types.ts | 16 ++++++++- tests/bugfixes.test.ts | 53 +++++++++++++++++++++++++++++ tests/dispatch.test.ts | 41 ++++++++++++++++++++++ tests/event-translator.test.ts | 35 +++++++++++++++++++ 8 files changed, 262 insertions(+), 17 deletions(-) diff --git a/src/backend/client.ts b/src/backend/client.ts index cb05317..b36971a 100644 --- a/src/backend/client.ts +++ b/src/backend/client.ts @@ -186,23 +186,41 @@ export class ZcodeBackend { // Notification. if (method === "session/event") { const ev = (msg.params ?? {}) as unknown as ZcodeEvent; - const sid = ev.sessionId; - const set = sid ? this.listeners.get(sid) : undefined; - if (set) { - // Iterate a snapshot so a listener that (un)registers during dispatch - // doesn't mutate the set under us. - for (const listener of [...set]) { - try { - listener.handleEvent(ev); - } catch (e) { - warn( - `backend: listener.handleEvent threw: ${e instanceof Error ? e.message : String(e)}`, - ); - } - } + this.dispatchEvent(ev); + } else if (method === "state.updated") { + // Session settings changed (model/mode/thoughtLevel switch, incl. + // mid-turn). The params carry the authoritative full settings patch: + // { patch: {mode, model, thoughtLevel, …}, reason, revision, sessionId } + // Wrap as a ZcodeEvent so it flows through the same listener pipeline. + const params = (msg.params ?? {}) as Record; + const ev: ZcodeEvent = { + sessionId: String(params.sessionId ?? ""), + seq: 0, + type: "state.updated", + payload: params, + }; + this.dispatchEvent(ev); + } + // Other notifications are currently ignored (process/resourceSample, …). + } + } + + /** Deliver a ZcodeEvent to every listener registered for its session. */ + private dispatchEvent(ev: ZcodeEvent): void { + const sid = ev.sessionId; + const set = sid ? this.listeners.get(sid) : undefined; + if (set) { + // Iterate a snapshot so a listener that (un)registers during dispatch + // doesn't mutate the set under us. + for (const listener of [...set]) { + try { + listener.handleEvent(ev); + } catch (e) { + warn( + `backend: listener.handleEvent threw: ${e instanceof Error ? e.message : String(e)}`, + ); } } - // Other notifications are currently ignored (state.updated, etc.). } } diff --git a/src/backend/types.ts b/src/backend/types.ts index dfe69ac..c1e2055 100644 --- a/src/backend/types.ts +++ b/src/backend/types.ts @@ -68,6 +68,10 @@ export type ZcodeEventType = | "turn.completed" | "turn.failed" | "session.updated" + // Backend pushes this notification (method: `state.updated`) whenever session + // settings change (model/mode/thoughtLevel switch, incl. mid-turn). The bridge + // wraps it as a ZcodeEvent so it flows through the same listener pipeline. + | "state.updated" // app-server 0.15.2+: steer lifecycle + terminal turn. Not yet translated by // the bridge; tracked in docs/BACKLOG.md. Listed here so unknown-type guards // stay accurate. diff --git a/src/handlers/dispatch.ts b/src/handlers/dispatch.ts index 6914734..678eba9 100644 --- a/src/handlers/dispatch.ts +++ b/src/handlers/dispatch.ts @@ -12,7 +12,12 @@ import { randomUUID } from "node:crypto"; import type * as acp from "@agentclientprotocol/sdk"; import { currentModelCached } from "../config/model-cache.js"; -import { modelContextWindow, parseModelValue } from "../config/options.js"; +import { + buildConfigOptions, + formatModelValue, + modelContextWindow, + parseModelValue, +} from "../config/options.js"; import { extractExitCode, parseSubagentMetadata, @@ -20,6 +25,7 @@ import { } from "../translators/tool-helpers.js"; import type { InternalEvent } from "../translators/types.js"; import type { ZcodeAcpServer } from "../server.js"; +import { warn } from "../utils.js"; import { sendSessionUpdate } from "./io.js"; /** Dispatch one internal event to the ACP client as a session/update. */ @@ -63,6 +69,50 @@ export async function dispatchEvent( case "FilesChanged": await dispatchFilesChanged(cx, acpSid, ev); break; + case "ConfigChanged": + await dispatchConfigChanged(server, cx, acpSid, ev); + break; + } +} + +/** + * Session settings changed (model/mode/thoughtLevel switch). Push the rebuilt + * configOptions (+ current_mode_update for mode) so the editor UI follows the + * switch immediately — even mid-turn, without waiting for turn completion. + * + * Builds the option STRUCTURES without a backend `session/read` (zcodeSid=null + * path: model list from config.json, mode/thought enums from CONFIG_META), then + * overlays the authoritative values from the event patch. The backend's + * settings projection may lag mid-turn, so the event values win — matching how + * the zcode app itself refreshes its UI from `state.updated`. + * Best-effort: failures are logged and swallowed, never thrown into the loop. + */ +async function dispatchConfigChanged( + server: ZcodeAcpServer, + cx: acp.AgentContext, + acpSid: string, + ev: Extract, +): Promise { + try { + const options = await buildConfigOptions(server, null); + if (ev.model) options[0].currentValue = formatModelValue(ev.model.providerId, ev.model.modelId); + if (ev.mode !== undefined) options[1].currentValue = ev.mode; + if (ev.thought !== undefined) options[2].currentValue = ev.thought; + await sendSessionUpdate(cx, acpSid, { + sessionUpdate: "config_option_update", + configOptions: options, + }); + if (ev.mode !== undefined) { + // Mirror the advertised mode so turn-completion reconciliation + // (emitModeIfChanged) doesn't re-emit the same value. + server.lastMode.set(acpSid, ev.mode); + await sendSessionUpdate(cx, acpSid, { + sessionUpdate: "current_mode_update", + currentModeId: ev.mode, + }); + } + } catch (e) { + warn(`dispatch: ConfigChanged failed (${e instanceof Error ? e.message : String(e)})`); } } diff --git a/src/translators/event-translator.ts b/src/translators/event-translator.ts index d203821..e825458 100644 --- a/src/translators/event-translator.ts +++ b/src/translators/event-translator.ts @@ -114,6 +114,12 @@ export class EventTranslator { if (typeof used === "number") { results.push({ kind: "UsageDelta", used, size }); } + } else if (etype === "state.updated") { + // Session settings changed (model/mode/thoughtLevel switch, incl. + // mid-turn). The backend notification carries the authoritative full + // settings patch — forward the new values so the editor UI follows the + // switch immediately instead of at the next turn's completion. + results.push(...this.translateStateUpdated(payload)); } else if (etype === "turn.failed") { this.turnDone = true; this.turnFailed = true; @@ -125,6 +131,30 @@ export class EventTranslator { return results; } + /** + * `state.updated` → one ConfigChanged event carrying the new settings values. + * Payload shape (wrapped from the backend notification's params): + * { patch: { mode: {current}, model: {current:{providerId,modelId}}, + * thoughtLevel: {current} }, reason, revision, sessionId } + * Fields missing from the patch are omitted — the dispatcher only emits + * updates for what actually changed. + */ + private translateStateUpdated(payload: Record): InternalEvent[] { + const patch = (payload["patch"] as Record | undefined) ?? {}; + const ev: InternalEvent = { kind: "ConfigChanged" }; + const mode = (patch["mode"] as Record | undefined)?.current; + if (typeof mode === "string") ev.mode = mode; + const model = (patch["model"] as Record | undefined)?.current as + | Record + | undefined; + if (model && typeof model["providerId"] === "string" && typeof model["modelId"] === "string") { + ev.model = { providerId: model["providerId"], modelId: model["modelId"] }; + } + const thought = (patch["thoughtLevel"] as Record | undefined)?.current; + if (typeof thought === "string") ev.thought = thought; + return [ev]; + } + private translateStreaming(payload: Record): InternalEvent[] { const results: InternalEvent[] = []; const kind = (payload["kind"] as string) ?? ""; diff --git a/src/translators/types.ts b/src/translators/types.ts index 6715805..58fbcfa 100644 --- a/src/translators/types.ts +++ b/src/translators/types.ts @@ -104,6 +104,19 @@ export interface FilesChangedEvent { files: string[]; } +/** + * Session settings changed (model/mode/thoughtLevel switch). Carries the new + * authoritative values from the backend's `state.updated` patch so the + * dispatcher can push config_option_update / current_mode_update without a + * `session/read` round-trip (and without waiting for turn completion). + */ +export interface ConfigChangedEvent { + kind: "ConfigChanged"; + mode?: string; + model?: { providerId: string; modelId: string }; + thought?: string; +} + export type InternalEvent = | ToolCallNewEvent | ToolCallUpdateEvent @@ -111,4 +124,5 @@ export type InternalEvent = | TextDeltaEvent | ReasoningDeltaEvent | PlanUpdateEvent - | FilesChangedEvent; + | FilesChangedEvent + | ConfigChangedEvent; diff --git a/tests/bugfixes.test.ts b/tests/bugfixes.test.ts index 7b08072..3a9d25f 100644 --- a/tests/bugfixes.test.ts +++ b/tests/bugfixes.test.ts @@ -286,3 +286,56 @@ describe("Bug #4: flattenTodos flattens todoGroups list (not single object)", () expect(flattenTodos([], undefined)).toEqual([]); }); }); + +describe("Bug: state.updated notification routed to session listeners", () => { + // The backend pushes `state.updated` (method: state.updated) when session + // settings change (model/mode/thoughtLevel switch, incl. mid-turn). It is NOT + // a session/event push — the bridge must wrap it as a ZcodeEvent and deliver + // it to the session's listeners so the turn loop can translate it. + + it("delivers state.updated to registered listeners as a ZcodeEvent", async () => { + const payload = JSON.stringify({ + method: "state.updated", + params: { + patch: { + mode: { current: "plan" }, + model: { current: { providerId: "builtin:bigmodel-coding-plan", modelId: "GLM-5.2" } }, + }, + reason: "mode_changed", + sessionId: "sess_x", + }, + }); + const fake = new ZcodeBackend( + [process.execPath, "-e", `setTimeout(() => process.stdout.write('${payload}\\n'), 100)`], + process.env, + ); + const listener = new EventStreamListener(fake, "sess_x"); + fake.registerEventListener("sess_x", listener); + + const r = await listener.pollEvent(3000); + expect(r).toMatchObject({ + sessionId: "sess_x", + type: "state.updated", + payload: { + patch: { mode: { current: "plan" } }, + sessionId: "sess_x", + }, + }); + }); + + it("does not deliver state.updated to listeners of other sessions", async () => { + const payload = JSON.stringify({ + method: "state.updated", + params: { patch: { mode: { current: "plan" } }, sessionId: "sess_x" }, + }); + const fake = new ZcodeBackend( + [process.execPath, "-e", `setTimeout(() => process.stdout.write('${payload}\\n'), 100)`], + process.env, + ); + const listener = new EventStreamListener(fake, "sess_other"); + fake.registerEventListener("sess_other", listener); + + const r = await listener.pollEvent(400); + expect(r).toBeNull(); + }); +}); diff --git a/tests/dispatch.test.ts b/tests/dispatch.test.ts index 6949175..878df0b 100644 --- a/tests/dispatch.test.ts +++ b/tests/dispatch.test.ts @@ -285,4 +285,45 @@ describe("dispatchEvent", () => { entries: [{ content: "do X", status: "pending", priority: "high" }], }); }); + + it("ConfigChanged (mode) emits config_option_update + current_mode_update", async () => { + const { cx, sent } = mockContext(); + const server = makeServer(false); + await dispatchEvent( + server, + cx, + SID, + { kind: "ConfigChanged", mode: "plan", model: { providerId: "anthropic", modelId: "GLM-5.2" } }, + CHUNK, + ); + expect(sent).toHaveLength(2); + expect(sent[0]).toMatchObject({ + sessionUpdate: "config_option_update", + configOptions: [ + { id: "model", currentValue: "anthropic\\GLM-5.2" }, + { id: "mode", currentValue: "plan" }, + { id: "thought", currentValue: "high" }, + ], + }); + expect(sent[1]).toEqual({ + sessionUpdate: "current_mode_update", + currentModeId: "plan", + }); + // Reconciliation mirror: lastMode reflects the advertised value. + expect(server.lastMode.get(SID)).toBe("plan"); + }); + + it("ConfigChanged (thought only) emits config_option_update, no mode update", async () => { + const { cx, sent } = mockContext(); + await dispatchEvent(makeServer(false), cx, SID, { kind: "ConfigChanged", thought: "max" }, CHUNK); + expect(sent).toHaveLength(1); + expect(sent[0]).toMatchObject({ + sessionUpdate: "config_option_update", + configOptions: [ + { id: "model" }, + { id: "mode" }, + { id: "thought", currentValue: "max" }, + ], + }); + }); }); diff --git a/tests/event-translator.test.ts b/tests/event-translator.test.ts index cde6d45..dac20f8 100644 --- a/tests/event-translator.test.ts +++ b/tests/event-translator.test.ts @@ -128,6 +128,41 @@ describe("EventTranslator", () => { expect(out).toEqual([{ kind: "UsageDelta", used: 1234, size: 200000 }]); }); + it("translates state.updated patch → ConfigChanged (mode/model/thought)", () => { + const t = new EventTranslator(); + const out = t.translate( + ev("state.updated", { + patch: { + mode: { current: "plan" }, + model: { current: { providerId: "builtin:bigmodel-coding-plan", modelId: "GLM-5.2" } }, + thoughtLevel: { current: "max" }, + }, + reason: "mode_changed", + }), + ); + expect(out).toEqual([ + { + kind: "ConfigChanged", + mode: "plan", + model: { providerId: "builtin:bigmodel-coding-plan", modelId: "GLM-5.2" }, + thought: "max", + }, + ]); + }); + + it("omits fields missing from the state.updated patch", () => { + const t = new EventTranslator(); + const out = t.translate( + ev("state.updated", { + patch: { model: { current: { providerId: "anthropic", modelId: "GLM-5.2" } } }, + reason: "model_changed", + }), + ); + expect(out).toEqual([ + { kind: "ConfigChanged", model: { providerId: "anthropic", modelId: "GLM-5.2" } }, + ]); + }); + it("captures turn.failed error and does not treat it as resultType", () => { const t = new EventTranslator(); t.translate( From b4292d85d06393138118063e9203ce9ed6c02592 Mon Sep 17 00:00:00 2001 From: William Wang Date: Mon, 3 Aug 2026 15:36:05 +0800 Subject: [PATCH 5/6] fix: resolve lazy session placeholders on session/resume and session/load --- docs/ARCHITECTURE.md | 6 +- docs/PROTOCOL.md | 7 ++ src/handlers/session.ts | 189 +++++++++++++++++++++---------- src/lazy-sessions.ts | 111 +++++++++++++++++++ tests/lazy-sessions.test.ts | 83 ++++++++++++++ tests/session-lazy.test.ts | 215 ++++++++++++++++++++++++++++++++++-- 6 files changed, 543 insertions(+), 68 deletions(-) create mode 100644 src/lazy-sessions.ts create mode 100644 tests/lazy-sessions.test.ts diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2a3ce00..34bd368 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -71,7 +71,11 @@ prompt request → session/send → EventTranslator translates → dispatchEvent Sessions are materialized lazily (`ensureRealSession`): an editor startup that never sends a message leaves no empty session in the backend or the App's task -index. +index. The placeholder → backend-session mapping is persisted to +`~/.zcode/v2/acp-lazy-sessions.json` (`src/lazy-sessions.ts`), so a `session/ +resume` / `session/load` of a placeholder from a previous bridge lifetime still +resolves: with a recorded backend id the real session is resumed, without one a +fresh (empty) session is materialized — never "Session not found". ### 2. Event stream subscription diff --git a/docs/PROTOCOL.md b/docs/PROTOCOL.md index 48ea95c..7ca5aab 100644 --- a/docs/PROTOCOL.md +++ b/docs/PROTOCOL.md @@ -134,6 +134,13 @@ List all sessions. Resume an existing session. +The sessionId may be a lazy `session/new` placeholder (the editor persists it +and resumes it after a bridge restart). The bridge resolves it before the +backend call: an in-memory or persisted (`acp-lazy-sessions.json`) mapping is +followed to the real backend session — resuming it, or materializing a fresh +empty one if the placeholder was never used. Real ids from `session/list` pass +through unchanged. + **Request:** ```json { diff --git a/src/handlers/session.ts b/src/handlers/session.ts index 02c0d72..23e08e4 100644 --- a/src/handlers/session.ts +++ b/src/handlers/session.ts @@ -28,6 +28,11 @@ import { buildModes, buildConfigOptions } from "../config/options.js"; import { emitInitialUsage } from "../config/model-cache.js"; import { buildProviderRegistry } from "../config/provider-registry.js"; import { buildResumeRuntimeModel } from "../config/runtime-model.js"; +import { + lookupLazySession, + recordMaterializedSession, + rememberLazySession, +} from "../lazy-sessions.js"; import { buildDiffContent, EventTranslator, @@ -98,6 +103,10 @@ export async function newSession( // backend session materializes; never shown in session/list. const acpSid = randomUUID(); server.pendingSessions.set(acpSid, { cwd }); + // Durable alias so the placeholder survives a bridge restart and session/ + // resume can still resolve it (best-effort; failures are swallowed inside + // the store). + rememberLazySession(acpSid, cwd); // Only freshly-created sessions are eligible for auto-title on first // end_turn; resumed/loaded sessions already have a title and must keep it. server.titleEligibleSessions.add(acpSid); @@ -124,7 +133,22 @@ export async function newSession( export async function ensureRealSession(server: ZcodeAcpServer, acpSid: string): Promise { const existing = server.resolveSid(acpSid); if (existing) return existing; - const pending = server.pendingSessions.get(acpSid); + let pending = server.pendingSessions.get(acpSid); + if (!pending) { + // Placeholder from a previous bridge lifetime: recover it from the durable + // store. A record that already carries a zcodeSid maps straight through + // (the backend session still exists — re-register the alias); one without + // re-hydrates the pending entry so the create path below runs. + const record = lookupLazySession(acpSid); + if (record?.zcodeSid) { + server.registerSession(acpSid, record.zcodeSid); + return record.zcodeSid; + } + if (record) { + pending = { cwd: record.cwd }; + server.pendingSessions.set(acpSid, pending); + } + } if (!pending) throw new Error(`session ${acpSid} not found`); if (pending.creating) return pending.creating; @@ -148,6 +172,9 @@ export async function ensureRealSession(server: ZcodeAcpServer, acpSid: string): server.pendingSessions.delete(acpSid); server.registerSession(acpSid, sid); + // Keep the durable alias in sync so a later bridge restart can still + // resume this session via the placeholder id. + recordMaterializedSession(acpSid, sid, pending.cwd); log(`session/new ${acpSid} → created ${sid} (lazy, on first use)`); server.ensureBackgroundListener(sid); @@ -203,44 +230,88 @@ export async function listSessions( return { sessions }; } +/** + * Resolve the backend session id for `session/resume` / `session/load`. + * + * A `session/new` placeholder has no backend counterpart until first use, yet + * the editor may resume it anyway (panel reopen, bridge restart) — resolving it + * here prevents an otherwise unavoidable "Session not found". Resolution order: + * 1. in-memory mapping → the session is already live in this subprocess; + * 2. pending placeholder → materialize it (an empty session, matching the + * pre-lazy behavior where a never-used session/new always resumed); + * 3. durable store → a placeholder from a previous bridge lifetime: with a + * recorded zcodeSid the backend session still exists but isn't loaded into + * this subprocess (the resume RPC is needed); without one, materialize + * fresh; + * 4. anything else (a real id from session/list, or a stale id) → pass + * through unchanged; genuinely missing sessions still error downstream. + */ +async function resolveResumeTarget( + server: ZcodeAcpServer, + acpSid: string, +): Promise<{ zcodeSid: string; alreadyLive: boolean }> { + const mapped = server.resolveSid(acpSid); + if (mapped) return { zcodeSid: mapped, alreadyLive: true }; + if (server.pendingSessions.has(acpSid)) { + return { zcodeSid: await ensureRealSession(server, acpSid), alreadyLive: true }; + } + const record = lookupLazySession(acpSid); + if (record) { + // ensureRealSession recovers the record: with a zcodeSid it re-registers + // the alias (no create), without one it materializes a fresh session. + return { + zcodeSid: await ensureRealSession(server, acpSid), + alreadyLive: !record.zcodeSid, + }; + } + return { zcodeSid: acpSid, alreadyLive: false }; +} + /** `session/resume` → zcode `session/resume` (with runtimeModel overlay). */ export async function resumeSession( server: ZcodeAcpServer, params: acp.ResumeSessionRequest, cx: acp.AgentContext, ): Promise { - const targetSid = params.sessionId; + const acpSid = params.sessionId; const cwd = params.cwd ?? process.cwd(); - if (!targetSid) throw new Error("sessionId required"); - - // runtimeModel overlay: a resumed session may carry a stale/revoked model in - // its history → send fails with "历史模型不可用". Overlaying the current - // enabled provider redirects the session onto a working model. The overlay - // deliberately carries NO apiKey (the backend's schema rejects it; it resolves - // auth from its own config/OAuth store). - const zcParams: Record = { - sessionId: targetSid, - workspace: workspaceFor(cwd), - }; - const runtimeModel = buildResumeRuntimeModel(); - if (runtimeModel !== null) zcParams.runtimeModel = runtimeModel; - // Push the provider registry BEFORE resume: a resumed session may carry a - // third-party model in its history, and the backend needs the provider - // registered to even process the resume turn. - await syncProviderRegistry(server, cwd); - await resumeBackendSession(server, zcParams); - - server.registerSession(targetSid, targetSid); - log(`session/resume -> ${targetSid}`); - server.ensureBackgroundListener(targetSid); + if (!acpSid) throw new Error("sessionId required"); + + // Lazy placeholders (session/new) resolve to their real backend session + // here; alreadyLive targets skip the resume RPC because the session is live + // in this backend subprocess. + const { zcodeSid, alreadyLive } = await resolveResumeTarget(server, acpSid); + + if (!alreadyLive) { + // runtimeModel overlay: a resumed session may carry a stale/revoked model in + // its history → send fails with "历史模型不可用". Overlaying the current + // enabled provider redirects the session onto a working model. The overlay + // deliberately carries NO apiKey (the backend's schema rejects it; it resolves + // auth from its own config/OAuth store). + const zcParams: Record = { + sessionId: zcodeSid, + workspace: workspaceFor(cwd), + }; + const runtimeModel = buildResumeRuntimeModel(); + if (runtimeModel !== null) zcParams.runtimeModel = runtimeModel; + // Push the provider registry BEFORE resume: a resumed session may carry a + // third-party model in its history, and the backend needs the provider + // registered to even process the resume turn. + await syncProviderRegistry(server, cwd); + await resumeBackendSession(server, zcParams); + } + + server.registerSession(acpSid, zcodeSid); + log(`session/resume -> ${zcodeSid}`); + server.ensureBackgroundListener(zcodeSid); // Initial usage_update so the editor shows the context bar immediately for a // resumed session (mirrors Python _on_session_resume → _emit_initial_usage). - await emitInitialUsage(server, cx, targetSid, targetSid, getOrCreateDiffer(server, targetSid)); - const modes = await buildModes(server, targetSid); - server.lastMode.set(targetSid, modes.currentModeId); + await emitInitialUsage(server, cx, acpSid, zcodeSid, getOrCreateDiffer(server, zcodeSid)); + const modes = await buildModes(server, zcodeSid); + server.lastMode.set(acpSid, modes.currentModeId); return { modes, - configOptions: await buildConfigOptions(server, targetSid), + configOptions: await buildConfigOptions(server, zcodeSid), }; } @@ -253,26 +324,32 @@ export async function loadSession( params: acp.LoadSessionRequest, cx: acp.AgentContext, ): Promise { - const targetSid = params.sessionId; + const acpSid = params.sessionId; const cwd = params.cwd ?? process.cwd(); - if (!targetSid) throw new Error("sessionId required"); + if (!acpSid) throw new Error("sessionId required"); - const zcParams: Record = { - sessionId: targetSid, - workspace: workspaceFor(cwd), - }; - const runtimeModel = buildResumeRuntimeModel(); - if (runtimeModel !== null) zcParams.runtimeModel = runtimeModel; - // Push the provider registry BEFORE resume: a loaded session may carry a - // third-party model in its history, and the backend needs the provider - // registered to process it. - await syncProviderRegistry(server, cwd); - await resumeBackendSession(server, zcParams); - server.registerSession(targetSid, targetSid); - log(`session/load → ${targetSid}`); - server.ensureBackgroundListener(targetSid); - - const messages = await fetchMessages(server, targetSid); + // Same placeholder resolution as resumeSession; alreadyLive targets skip the + // backend resume RPC (the session is live in this subprocess). + const { zcodeSid, alreadyLive } = await resolveResumeTarget(server, acpSid); + + if (!alreadyLive) { + const zcParams: Record = { + sessionId: zcodeSid, + workspace: workspaceFor(cwd), + }; + const runtimeModel = buildResumeRuntimeModel(); + if (runtimeModel !== null) zcParams.runtimeModel = runtimeModel; + // Push the provider registry BEFORE resume: a loaded session may carry a + // third-party model in its history, and the backend needs the provider + // registered to process it. + await syncProviderRegistry(server, cwd); + await resumeBackendSession(server, zcParams); + } + server.registerSession(acpSid, zcodeSid); + log(`session/load → ${zcodeSid}`); + server.ensureBackgroundListener(zcodeSid); + + const messages = await fetchMessages(server, zcodeSid); let replayed = 0; for (const m of messages) { const info = m.info ?? {}; @@ -285,7 +362,7 @@ export async function loadSession( const text = (p as { text?: string }).text ?? ""; if (!text) continue; const sessionUpdate = role === "user" ? "user_message_chunk" : "agent_message_chunk"; - await sendSessionUpdate(cx, targetSid, { + await sendSessionUpdate(cx, acpSid, { sessionUpdate, content: { type: "text", text }, messageId: mid, @@ -294,7 +371,7 @@ export async function loadSession( const rp = p as { text?: string; content?: string }; const text = rp.text ?? rp.content ?? ""; if (text) { - await sendSessionUpdate(cx, targetSid, { + await sendSessionUpdate(cx, acpSid, { sessionUpdate: "agent_thought_chunk", content: { type: "text", text }, messageId: `thought_${mid}`, @@ -317,7 +394,7 @@ export async function loadSession( status: (tp.status as acp.ToolCallStatus) ?? "completed", ...(histToolName ? { _meta: { claudeCode: { toolName: histToolName } } } : {}), }; - await sendSessionUpdate(cx, targetSid, update); + await sendSessionUpdate(cx, acpSid, update); } // patch / step-start / other: skipped (history replay focuses on text + tool summary) } @@ -329,11 +406,11 @@ export async function loadSession( // its todos immediately (filter to PlanUpdate only — text/tools were already // replayed above and the differ hasn't mark_seen'd this history). try { - const snapshot = await buildSnapshot(server, targetSid); - const loadDiffer = getOrCreateDiffer(server, targetSid); + const snapshot = await buildSnapshot(server, zcodeSid); + const loadDiffer = getOrCreateDiffer(server, zcodeSid); const planEvents = loadDiffer.diff(snapshot).filter((e) => e.kind === "PlanUpdate"); for (const iev of planEvents) { - await dispatchEvent(server, cx, targetSid, iev, `load_${randomUUID().slice(0, 8)}`); + await dispatchEvent(server, cx, acpSid, iev, `load_${randomUUID().slice(0, 8)}`); } } catch (e) { log( @@ -342,13 +419,13 @@ export async function loadSession( } // Initial usage_update so the editor shows the context bar immediately. - await emitInitialUsage(server, cx, targetSid, targetSid, getOrCreateDiffer(server, targetSid)); + await emitInitialUsage(server, cx, acpSid, zcodeSid, getOrCreateDiffer(server, zcodeSid)); - const modes = await buildModes(server, targetSid); - server.lastMode.set(targetSid, modes.currentModeId); + const modes = await buildModes(server, zcodeSid); + server.lastMode.set(acpSid, modes.currentModeId); return { modes, - configOptions: await buildConfigOptions(server, targetSid), + configOptions: await buildConfigOptions(server, zcodeSid), }; } diff --git a/src/lazy-sessions.ts b/src/lazy-sessions.ts new file mode 100644 index 0000000..0b66c57 --- /dev/null +++ b/src/lazy-sessions.ts @@ -0,0 +1,111 @@ +/** + * Durable alias store for lazy `session/new` placeholders. + * + * `session/new` returns a placeholder id with no backend session behind it + * (the real `session/create` is deferred to first use). The editor stores this + * placeholder and may resume it later — including after a bridge restart, when + * the in-memory `pendingSessions`/`sessionMap` are gone. Without a durable + * record, `session/resume` then fails with "Session not found". + * + * This module keeps a tiny JSON file (`~/.zcode/v2/acp-lazy-sessions.json`, + * next to tasks-index.sqlite) mapping acp_sid → { cwd, zcodeSid?, createdAt }: + * - `rememberLazySession` — written at session/new (no zcodeSid yet); + * - `recordMaterializedSession` — updated once the placeholder materializes; + * - `lookupLazySession` — lets resume/load/ensureRealSession recover a + * placeholder from a previous bridge lifetime. + * + * Best-effort side-channel like tasks-index: failures are logged and swallowed + * so a store problem never breaks session/new or first use. Records older than + * 30 days are pruned on load — the real session stays reachable via + * session/list after that, only the placeholder alias expires. + */ + +import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import path from "node:path"; +import process from "node:process"; + +import { log, warn } from "./utils.js"; + +/** Placeholder alias record persisted in the store. */ +export interface LazySessionRecord { + cwd: string; + /** Backend session id once the placeholder materialized (absent = never used). */ + zcodeSid?: string; + createdAt: number; +} + +/** Store file lives next to config.json / tasks-index.sqlite under ~/.zcode/v2/. */ +const STORE_FILENAME = "acp-lazy-sessions.json"; + +/** Placeholder aliases expire after 30 days; the real session remains listable. */ +const TTL_MS = 30 * 24 * 60 * 60 * 1000; + +/** Resolved at call time so tests can stub HOME without re-importing. */ +function storePath(): string { + const home = process.env.HOME || process.env.USERPROFILE || "~"; + return path.join(home, ".zcode", "v2", STORE_FILENAME); +} + +/** Read the store, pruning expired/corrupt records. Returns {} on any failure. */ +function loadRecords(): Record { + try { + const p = storePath(); + if (!existsSync(p)) return {}; + const raw = JSON.parse(readFileSync(p, "utf8")) as Record; + if (typeof raw !== "object" || raw === null) return {}; + const now = Date.now(); + let pruned = false; + const out: Record = {}; + for (const [acpSid, rec] of Object.entries(raw)) { + if (typeof rec?.createdAt !== "number" || now - rec.createdAt > TTL_MS) { + pruned = true; + continue; + } + out[acpSid] = rec; + } + if (pruned) writeRecords(out); + return out; + } catch (e) { + warn( + `lazy-sessions: store read failed ` + + `(${e instanceof Error ? e.message : String(e)}) — placeholder aliases unavailable`, + ); + return {}; + } +} + +/** Overwrite the store file. Failures are logged, never thrown. */ +function writeRecords(records: Record): void { + try { + const p = storePath(); + mkdirSync(path.dirname(p), { recursive: true }); + writeFileSync(p, JSON.stringify(records, null, 2)); + } catch (e) { + log(`lazy-sessions: store write failed (${e instanceof Error ? e.message : String(e)})`); + } +} + +/** Record a new placeholder at session/new (no backend session yet). */ +export function rememberLazySession(acpSid: string, cwd: string): void { + const records = loadRecords(); + records[acpSid] = { cwd, createdAt: Date.now() }; + writeRecords(records); +} + +/** Attach the backend session id once the placeholder materializes. */ +export function recordMaterializedSession(acpSid: string, zcodeSid: string, cwd: string): void { + const records = loadRecords(); + const existing = records[acpSid]; + if (existing?.zcodeSid === zcodeSid) return; + records[acpSid] = { + cwd: existing?.cwd ?? cwd, + zcodeSid, + createdAt: existing?.createdAt ?? Date.now(), + }; + writeRecords(records); +} + +/** Look up a placeholder alias (undefined = unknown to this bridge and store). */ +export function lookupLazySession(acpSid: string): LazySessionRecord | undefined { + return loadRecords()[acpSid]; +} diff --git a/tests/lazy-sessions.test.ts b/tests/lazy-sessions.test.ts new file mode 100644 index 0000000..4fa8e3b --- /dev/null +++ b/tests/lazy-sessions.test.ts @@ -0,0 +1,83 @@ +/** + * lazy-sessions.ts tests — the durable alias store that keeps lazy session/new + * placeholders resolvable across bridge restarts. + * + * The store writes ~/.zcode/v2/acp-lazy-sessions.json (path derived from HOME + * at call time); tests stub HOME and mock node:fs so nothing touches disk. + */ + +import path from "node:path"; + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { + lookupLazySession, + recordMaterializedSession, + rememberLazySession, +} from "../src/lazy-sessions.js"; + +const mockFiles = new Map(); +const mockDirs = new Set(); +const STORE = "/fake-home/.zcode/v2/acp-lazy-sessions.json"; + +vi.mock("node:fs", async () => { + const actual = await vi.importActual("node:fs"); + return { + ...actual, + existsSync: (p: string) => mockFiles.has(p) || mockDirs.has(p), + readFileSync: (p: string) => { + if (mockFiles.has(p)) return mockFiles.get(p)!; + throw new Error(`ENOENT: ${p}`); + }, + writeFileSync: (p: string, data: string) => { + mockDirs.add(path.dirname(p)); + mockFiles.set(p, String(data)); + }, + mkdirSync: (p: string) => { + mockDirs.add(String(p)); + }, + }; +}); + +beforeEach(() => { + mockFiles.clear(); + mockDirs.clear(); + vi.stubEnv("HOME", "/fake-home"); +}); + +afterEach(() => { + vi.unstubAllEnvs(); +}); + +describe("lazy session alias store", () => { + it("records a placeholder at session/new and reads it back", () => { + rememberLazySession("acp_1", "/tmp/ws"); + + expect(lookupLazySession("acp_1")).toEqual({ cwd: "/tmp/ws", createdAt: expect.any(Number) }); + expect(lookupLazySession("acp_missing")).toBeUndefined(); + }); + + it("attaches the backend session id at materialization, keeping cwd and createdAt", () => { + rememberLazySession("acp_1", "/tmp/ws"); + recordMaterializedSession("acp_1", "sess_1", "/tmp/ws"); + + const rec = lookupLazySession("acp_1"); + expect(rec?.zcodeSid).toBe("sess_1"); + expect(rec?.cwd).toBe("/tmp/ws"); + expect(rec?.createdAt).toBeTypeOf("number"); + }); + + it("drops expired records on load and rewrites the file", () => { + const old = Date.now() - 31 * 24 * 60 * 60 * 1000; // older than the 30-day TTL + mockFiles.set(STORE, JSON.stringify({ stale: { cwd: "/tmp/ws", createdAt: old } })); + + expect(lookupLazySession("stale")).toBeUndefined(); + expect(JSON.parse(mockFiles.get(STORE)!)).toEqual({}); + }); + + it("tolerates a corrupt store file", () => { + mockFiles.set(STORE, "{not json"); + + expect(lookupLazySession("acp_1")).toBeUndefined(); + }); +}); diff --git a/tests/session-lazy.test.ts b/tests/session-lazy.test.ts index 950f61d..124d990 100644 --- a/tests/session-lazy.test.ts +++ b/tests/session-lazy.test.ts @@ -3,14 +3,21 @@ * * session/new returns a placeholder id and defers zcode `session/create` to * first use (ensureRealSession), so an editor startup that never prompts - * leaves no session in the backend or the App's task index. + * leaves no session in the backend or the App's task index. Placeholders stay + * resolvable by session/resume and session/load — including after a bridge + * restart, via the durable alias store (mocked below). */ import type * as acp from "@agentclientprotocol/sdk"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import type { ZcodeBackend } from "../src/backend/client.js"; -import { ensureRealSession, newSession } from "../src/handlers/session.js"; +import { + ensureRealSession, + loadSession, + newSession, + resumeSession, +} from "../src/handlers/session.js"; import { ZcodeAcpServer } from "../src/server.js"; // Record tasks-index upserts so tests can assert the App sync happens at @@ -25,7 +32,33 @@ vi.mock("../src/tasks-index.js", () => ({ updateSessionTitle: async () => true, })); -/** Fake backend: answers session/create, errors on everything else. */ +// In-memory durable alias store (src/lazy-sessions.ts persists this to +// ~/.zcode/v2/acp-lazy-sessions.json — never touch real disk in tests). +const mockStore = new Map(); +vi.mock("../src/lazy-sessions.js", () => ({ + rememberLazySession: (acpSid: string, cwd: string) => { + mockStore.set(acpSid, { cwd, createdAt: Date.now() }); + }, + recordMaterializedSession: (acpSid: string, zcodeSid: string, cwd: string) => { + const existing = mockStore.get(acpSid); + mockStore.set(acpSid, { + cwd: existing?.cwd ?? cwd, + zcodeSid, + createdAt: existing?.createdAt ?? Date.now(), + }); + }, + lookupLazySession: (acpSid: string) => mockStore.get(acpSid), +})); + +beforeEach(() => { + mockStore.clear(); +}); + +/** + * Fake backend: answers session/create (counting creates), session/resume, + * session/read (empty projection/settings), session/messages (empty) and the + * provider-registry push; errors on everything else. + */ function fakeBackend(): ZcodeBackend & { calls: Array<{ method: string; params: unknown }>; } { @@ -35,14 +68,25 @@ function fakeBackend(): ZcodeBackend & { isDead: false, request: async (id: number, method: string, params: unknown) => { calls.push({ method, params }); - if (method === "session/create") { - created += 1; - return { - id, - result: { session: { sessionId: `sess_lazy_${created}`, title: "", traceId: "trace_1" } }, - }; + switch (method) { + case "session/create": + created += 1; + return { + id, + result: { + session: { sessionId: `sess_lazy_${created}`, title: "", traceId: "trace_1" }, + }, + }; + case "session/resume": + case "workspace/updateProviderRegistry": + return { id, result: {} }; + case "session/read": + return { id, result: { projection: { contextUsed: 0 }, settings: {} } }; + case "session/messages": + return { id, result: { messages: [] } }; + default: + return { id, error: { message: `unhandled ${method}` } }; } - return { id, error: { message: `unhandled ${method}` } }; }, registerEventListener: () => {}, unregisterEventListener: () => {}, @@ -134,4 +178,153 @@ describe("ensureRealSession", () => { await expect(ensureRealSession(server, "acp_existing")).resolves.toBe("sess_existing"); expect(calls.filter((c) => c.method === "session/create")).toHaveLength(0); }); + + it("recovers a materialized placeholder from a previous bridge lifetime", async () => { + const server = new ZcodeAcpServer(); + mockStore.set("acp_old", { cwd: "/tmp/ws", zcodeSid: "sess_old", createdAt: Date.now() }); + const { backend, calls } = fakeBackend(); + server.backend = backend; + + await expect(ensureRealSession(server, "acp_old")).resolves.toBe("sess_old"); + expect(server.resolveSid("acp_old")).toBe("sess_old"); + expect(calls.filter((c) => c.method === "session/create")).toHaveLength(0); + }); + + it("re-hydrates a never-used placeholder from a previous bridge lifetime", async () => { + const server = new ZcodeAcpServer(); + mockStore.set("acp_old_unused", { cwd: "/tmp/ws", createdAt: Date.now() }); + const { backend, calls } = fakeBackend(); + server.backend = backend; + + await expect(ensureRealSession(server, "acp_old_unused")).resolves.toBe("sess_lazy_1"); + expect(server.resolveSid("acp_old_unused")).toBe("sess_lazy_1"); + expect(calls.filter((c) => c.method === "session/create")).toHaveLength(1); + expect(calls[0].params).toMatchObject({ + workspace: { workspacePath: "/tmp/ws", workspaceKey: "/tmp/ws" }, + }); + }); +}); + +describe("resumeSession with lazy placeholders", () => { + it("materializes a pending placeholder and skips the backend resume RPC", async () => { + const server = new ZcodeAcpServer(); + const resp = await newSession(server, newSessionParams("/tmp/ws")); + const { backend, calls } = fakeBackend(); + server.backend = backend; + const cx = {} as acp.AgentContext; + + const out = await resumeSession( + server, + { sessionId: resp.sessionId } as acp.ResumeSessionRequest, + cx, + ); + + expect(server.resolveSid(resp.sessionId)).toBe("sess_lazy_1"); + expect(calls.some((c) => c.method === "session/create")).toBe(true); + expect(calls.some((c) => c.method === "session/resume")).toBe(false); + expect(out.modes.currentModeId).toBe("yolo"); + }); + + it("resumes an already-materialized placeholder without backend resume", async () => { + const server = new ZcodeAcpServer(); + const resp = await newSession(server, newSessionParams("/tmp/ws")); + const { backend, calls } = fakeBackend(); + server.backend = backend; + await ensureRealSession(server, resp.sessionId); + calls.length = 0; + + await resumeSession( + server, + { sessionId: resp.sessionId } as acp.ResumeSessionRequest, + {} as acp.AgentContext, + ); + + expect(calls.some((c) => c.method === "session/create")).toBe(false); + expect(calls.some((c) => c.method === "session/resume")).toBe(false); + }); + + it("passes a real backend id through to session/resume", async () => { + const server = new ZcodeAcpServer(); + const { backend, calls } = fakeBackend(); + server.backend = backend; + + await resumeSession( + server, + { sessionId: "sess_real" } as acp.ResumeSessionRequest, + {} as acp.AgentContext, + ); + + const resume = calls.find((c) => c.method === "session/resume"); + expect(resume?.params).toMatchObject({ sessionId: "sess_real" }); + expect(server.resolveSid("sess_real")).toBe("sess_real"); + }); + + it("recovers a previous-lifetime placeholder and resumes its real session", async () => { + const server = new ZcodeAcpServer(); + mockStore.set("acp_old", { cwd: "/tmp/ws", zcodeSid: "sess_old", createdAt: Date.now() }); + const { backend, calls } = fakeBackend(); + server.backend = backend; + + await resumeSession( + server, + { sessionId: "acp_old" } as acp.ResumeSessionRequest, + {} as acp.AgentContext, + ); + + const resume = calls.find((c) => c.method === "session/resume"); + expect(resume?.params).toMatchObject({ sessionId: "sess_old" }); + expect(server.resolveSid("acp_old")).toBe("sess_old"); + expect(calls.some((c) => c.method === "session/create")).toBe(false); + }); + + it("materializes a previous-lifetime placeholder that was never used", async () => { + const server = new ZcodeAcpServer(); + mockStore.set("acp_old_unused", { cwd: "/tmp/ws", createdAt: Date.now() }); + const { backend, calls } = fakeBackend(); + server.backend = backend; + + await resumeSession( + server, + { sessionId: "acp_old_unused" } as acp.ResumeSessionRequest, + {} as acp.AgentContext, + ); + + expect(server.resolveSid("acp_old_unused")).toBe("sess_lazy_1"); + expect(calls.some((c) => c.method === "session/create")).toBe(true); + expect(calls.some((c) => c.method === "session/resume")).toBe(false); + }); +}); + +describe("loadSession with lazy placeholders", () => { + it("materializes a pending placeholder without the backend resume RPC", async () => { + const server = new ZcodeAcpServer(); + const resp = await newSession(server, newSessionParams("/tmp/ws")); + const { backend, calls } = fakeBackend(); + server.backend = backend; + + await loadSession( + server, + { sessionId: resp.sessionId } as acp.LoadSessionRequest, + {} as acp.AgentContext, + ); + + expect(server.resolveSid(resp.sessionId)).toBe("sess_lazy_1"); + expect(calls.some((c) => c.method === "session/create")).toBe(true); + expect(calls.some((c) => c.method === "session/resume")).toBe(false); + }); + + it("passes a real backend id through to session/resume", async () => { + const server = new ZcodeAcpServer(); + const { backend, calls } = fakeBackend(); + server.backend = backend; + + await loadSession( + server, + { sessionId: "sess_real" } as acp.LoadSessionRequest, + {} as acp.AgentContext, + ); + + const resume = calls.find((c) => c.method === "session/resume"); + expect(resume?.params).toMatchObject({ sessionId: "sess_real" }); + }); }); From 8a19626f8e885c2f5ee2daeb58dac74022898ab0 Mon Sep 17 00:00:00 2001 From: William Wang Date: Mon, 3 Aug 2026 16:06:41 +0800 Subject: [PATCH 6/6] fix: keep selected model in dropdown when state.updated patch lacks model --- src/handlers/dispatch.ts | 16 +++++++----- tests/dispatch.test.ts | 55 +++++++++++++++++++++++++++++++++++----- 2 files changed, 58 insertions(+), 13 deletions(-) diff --git a/src/handlers/dispatch.ts b/src/handlers/dispatch.ts index 678eba9..9160b03 100644 --- a/src/handlers/dispatch.ts +++ b/src/handlers/dispatch.ts @@ -80,11 +80,12 @@ export async function dispatchEvent( * configOptions (+ current_mode_update for mode) so the editor UI follows the * switch immediately — even mid-turn, without waiting for turn completion. * - * Builds the option STRUCTURES without a backend `session/read` (zcodeSid=null - * path: model list from config.json, mode/thought enums from CONFIG_META), then - * overlays the authoritative values from the event patch. The backend's - * settings projection may lag mid-turn, so the event values win — matching how - * the zcode app itself refreshes its UI from `state.updated`. + * Builds the option structures from the session's authoritative settings + * (`session/read` via buildConfigOptions), then overlays the values from the + * event patch. A state.updated patch only carries the fields that actually + * changed, so building from the null defaults instead would reset every + * untouched field — most visibly the model dropdown jumping back to the + * default model mid-conversation. * Best-effort: failures are logged and swallowed, never thrown into the loop. */ async function dispatchConfigChanged( @@ -94,7 +95,10 @@ async function dispatchConfigChanged( ev: Extract, ): Promise { try { - const options = await buildConfigOptions(server, null); + // Fall back to null (defaults) only if the session mapping isn't live yet — + // events routed through a registered turn loop always have it. + const zcodeSid = server.resolveSid(acpSid) ?? null; + const options = await buildConfigOptions(server, zcodeSid); if (ev.model) options[0].currentValue = formatModelValue(ev.model.providerId, ev.model.modelId); if (ev.mode !== undefined) options[1].currentValue = ev.mode; if (ev.thought !== undefined) options[2].currentValue = ev.thought; diff --git a/tests/dispatch.test.ts b/tests/dispatch.test.ts index 878df0b..b6c398a 100644 --- a/tests/dispatch.test.ts +++ b/tests/dispatch.test.ts @@ -293,7 +293,11 @@ describe("dispatchEvent", () => { server, cx, SID, - { kind: "ConfigChanged", mode: "plan", model: { providerId: "anthropic", modelId: "GLM-5.2" } }, + { + kind: "ConfigChanged", + mode: "plan", + model: { providerId: "anthropic", modelId: "GLM-5.2" }, + }, CHUNK, ); expect(sent).toHaveLength(2); @@ -315,15 +319,52 @@ describe("dispatchEvent", () => { it("ConfigChanged (thought only) emits config_option_update, no mode update", async () => { const { cx, sent } = mockContext(); - await dispatchEvent(makeServer(false), cx, SID, { kind: "ConfigChanged", thought: "max" }, CHUNK); + await dispatchEvent( + makeServer(false), + cx, + SID, + { kind: "ConfigChanged", thought: "max" }, + CHUNK, + ); expect(sent).toHaveLength(1); expect(sent[0]).toMatchObject({ sessionUpdate: "config_option_update", - configOptions: [ - { id: "model" }, - { id: "mode" }, - { id: "thought", currentValue: "max" }, - ], + configOptions: [{ id: "model" }, { id: "mode" }, { id: "thought", currentValue: "max" }], }); }); + + it("ConfigChanged without model keeps the session's current model (no default reset)", async () => { + const { cx, sent } = mockContext(); + const server = makeServer(false); + server.registerSession(SID, "sess_real"); + // Fake backend: session/read reports DeepSeek as the session's current + // model. A mid-turn state.updated that changes only mode/thought must NOT + // reset the model dropdown to the default — regression for the model + // jumping back to the default when sending a message. + server.backend = { + isDead: false, + request: async () => ({ + result: { + settings: { + model: { current: { providerId: "deepseek", modelId: "DeepSeek-V3.5" } }, + mode: { current: "build" }, + thoughtLevel: { current: "high" }, + }, + }, + }), + } as unknown as NonNullable; + + await dispatchEvent( + server, + cx, + SID, + { kind: "ConfigChanged", mode: "plan", thought: "high" }, + CHUNK, + ); + expect(sent).toHaveLength(2); + const options = (sent[0] as { configOptions: acp.SessionConfigOption[] }).configOptions; + expect(options[0]).toMatchObject({ id: "model", currentValue: "deepseek\\DeepSeek-V3.5" }); + expect(options[1]).toMatchObject({ id: "mode", currentValue: "plan" }); + expect(options[2]).toMatchObject({ id: "thought", currentValue: "high" }); + }); });