diff --git a/apps/desktop/src/main/engine/backends/acp.ts b/apps/desktop/src/main/engine/backends/acp.ts index 563fe3a8..4bac66f1 100644 --- a/apps/desktop/src/main/engine/backends/acp.ts +++ b/apps/desktop/src/main/engine/backends/acp.ts @@ -129,7 +129,15 @@ export class AcpBackend implements Backend { const session = r?.agentCapabilities?.sessionCapabilities ?? {}; resolve({ loadSession: r?.agentCapabilities?.loadSession === true, resume: typeof session.resume === "object" && session.resume !== null, image: r?.agentCapabilities?.promptCapabilities?.image === true }); }) - .catch(reject); + .catch((err: Error) => { + // A live server with a rejected handshake cannot serve later turns. + // Only detach this generation: its late rejection must not stop a replacement. + if (this.child === child) { + this.disconnect(err); + killGroup(child); + } + reject(err); + }); }); return this.ready; } @@ -286,7 +294,7 @@ export class AcpBackend implements Backend { await duringSetup(this.request("session/set_config_option", { sessionId: sid, configId: "model", value: opts.model }, 30_000), signal, this.family.label); } catch (err) { if (signal.aborted) return { status: "interrupted" }; - sink.notice("warn", `${this.family.label} did not accept the model ${opts.model} (${(err as Error).message}); the turn runs on the session's current model.`); + return failed(err, { stage: "set model", sessionId: sid, model: opts.model }); } } if (opts.effort) { diff --git a/apps/desktop/test/acp-backend.test.ts b/apps/desktop/test/acp-backend.test.ts index d4056c8d..fc92688f 100644 --- a/apps/desktop/test/acp-backend.test.ts +++ b/apps/desktop/test/acp-backend.test.ts @@ -8,7 +8,7 @@ import { FakeProcess, fakeSpawn, collectSink } from "./fakeproc.js"; * A scripted `opencode acp` / `mimo acp` server, from the shapes OpenCode 1.19.5 and MiMo Code 0.1.16 sent live * (docs/reviews/2026-10-28-opencode-acp.json): initialize, session/new|resume|load, set_mode, set_config_option, prompt. */ -function fakeServer(proc: FakeProcess, opts: { sessionId?: string; resume?: boolean; load?: boolean; refuseMode?: string; unknownSession?: string; hold?: boolean } = {}) { +function fakeServer(proc: FakeProcess, opts: { sessionId?: string; resume?: boolean; load?: boolean; refuseMode?: string; refuseModel?: boolean; refuseInitialize?: boolean; unknownSession?: string; hold?: boolean } = {}) { const sessionId = opts.sessionId ?? "ses_1"; const seen: { id?: number; method: string; params: Record }[] = []; const prompts: number[] = []; @@ -20,7 +20,8 @@ function fakeServer(proc: FakeProcess, opts: { sessionId?: string; resume?: bool if (msg.method) seen.push({ id: msg.id, method: msg.method, params: msg.params ?? {} }); else responses.push(msg as Record); const reply = (result: unknown) => proc.emitLine({ jsonrpc: "2.0", id: msg.id, result }); - if (msg.method === "initialize") reply({ protocolVersion: 1, agentInfo: { name: "OpenCode", version: "1.19.5" }, agentCapabilities: { loadSession: opts.load !== false, promptCapabilities: { image: true }, sessionCapabilities: opts.resume === false ? {} : { resume: {} } } }); + if (msg.method === "initialize" && opts.refuseInitialize) proc.emitLine({ jsonrpc: "2.0", id: msg.id, error: { code: -32603, message: "initialization failed" } }); + else if (msg.method === "initialize") reply({ protocolVersion: 1, agentInfo: { name: "OpenCode", version: "1.19.5" }, agentCapabilities: { loadSession: opts.load !== false, promptCapabilities: { image: true }, sessionCapabilities: opts.resume === false ? {} : { resume: {} } } }); else if (msg.method === "session/new") reply({ sessionId, configOptions: [] }); else if (msg.method === "session/resume" || msg.method === "session/load") { if (opts.unknownSession && msg.params?.sessionId === opts.unknownSession) proc.emitLine({ jsonrpc: "2.0", id: msg.id, error: { code: -32602, message: "session not found" } }); @@ -32,10 +33,12 @@ function fakeServer(proc: FakeProcess, opts: { sessionId?: string; resume?: bool if (opts.refuseMode && msg.params?.modeId === opts.refuseMode) proc.emitLine({ jsonrpc: "2.0", id: msg.id, error: { code: -32602, message: `unknown mode ${opts.refuseMode}` } }); else reply({}); } else if (msg.method === "session/set_config_option") { - if (msg.params?.configId === "effort") proc.emitLine({ jsonrpc: "2.0", id: msg.id, error: { code: -32602, message: "no such option" } }); + if (opts.refuseModel && msg.params?.configId === "model") proc.emitLine({ jsonrpc: "2.0", id: msg.id, error: { code: -32602, message: "model unavailable" } }); + else if (msg.params?.configId === "effort") proc.emitLine({ jsonrpc: "2.0", id: msg.id, error: { code: -32602, message: "no such option" } }); else reply({ configOptions: [] }); } else if (msg.method === "session/prompt") { prompts.push(msg.id!); + if (opts.refuseModel) reply({ stopReason: "end_turn" }); } else if (msg.method === "session/cancel") { // A notification: the pending prompt answers with the cancelled stop reason. const id = prompts.shift(); @@ -314,3 +317,44 @@ test("ACP tool calls become Modex tools by kind, with commands as shell lines", assert.deepEqual(toolCall({ kind: "read", name: "read", title: "read", rawInput: { filePath: "/r/a" } }), { name: "read", title: "read", args: { filePath: "/r/a" } }); assert.deepEqual(toolCall({ kind: "fetch", name: "webfetch", title: "https://x", rawInput: { url: "https://x" } }), { name: "webfetch", title: "https://x", args: { url: "https://x" } }); }); + + +test("a refused explicit ACP model fails before sending the prompt to the previous provider", async () => { + for (const family of ["opencode", "mimo"] as const) { + const proc = new FakeProcess(); + const server = fakeServer(proc, { refuseModel: true }); + const backend = new AcpBackend(family, family, fakeSpawn(proc).spawn); + const { sink } = collectSink(); + try { + const result = await backend.runTurn("private prompt for selected provider", { ...opts, model: "selected/provider-model" }, sink, new AbortController().signal); + assert.equal(result.status, "failed"); + assert.equal(server.seen.filter((request) => request.method === "session/prompt").length, 0, "a rejected model must not send prompt contents to the session default"); + assert.match(result.error ?? "", /model unavailable/); + } finally { await backend.dispose(); } + } +}); + + +test("a failed ACP initialization releases the server and the next turn starts fresh", async () => { + for (const family of ["opencode", "mimo"] as const) { + const broken = new FakeProcess(); + const healthy = new FakeProcess(); + fakeServer(broken, { refuseInitialize: true }); + const server = fakeServer(healthy); + let starts = 0; + const spawnImpl = (() => starts++ === 0 ? broken : healthy) as unknown as typeof spawn; + const backend = new AcpBackend(family, family, spawnImpl); + const { sink } = collectSink(); + try { + const first = await backend.runTurn("first", opts, sink, new AbortController().signal); + assert.equal(first.status, "failed"); + assert.match(first.error ?? "", /initialization failed/); + assert.ok(broken.killed, "the failed initialization must not leave a server running"); + const retry = backend.runTurn("retry", opts, sink, new AbortController().signal); + await healthy.waitFor((line) => line.includes('"session/prompt"')); + server.endTurn(); + assert.equal((await retry).status, "completed"); + assert.equal(starts, 2); + } finally { await backend.dispose(); } + } +}); diff --git a/docs/opencode.md b/docs/opencode.md index 16bfd621..37f2259f 100644 --- a/docs/opencode.md +++ b/docs/opencode.md @@ -17,20 +17,12 @@ a prompt, and Stop confirmed 17 ms after the cancel. ## How a turn runs -1. **One server per backend.** The first turn starts ` acp --pure` once, detached in its own - process group. `--pure` keeps the CLI's plugins and hooks out of the server. Modex's - configuration travels inline in `OPENCODE_CONFIG_CONTENT` / `MIMOCODE_CONFIG_CONTENT`, which the - CLI merges on top of the person's own `opencode.json`; nothing is written to disk and the person's - providers, models, MCP servers and agents stay. It disables the CLI's self-update and its - transcript sharing. `initialize` offers no `fs` or `terminal` capability, so the CLI reads and - runs through its own tools and never asks Modex to. +1. **One server per backend.** The first turn starts ` acp --pure` once, detached in its own process group. `--pure` keeps the CLI's plugins and hooks out of the server. Modex's configuration travels inline in `OPENCODE_CONFIG_CONTENT` / `MIMOCODE_CONFIG_CONTENT`, which the CLI merges on top of the person's own `opencode.json`; nothing is written to disk and the person's providers, models, MCP servers and agents stay. It disables the CLI's self-update and its transcript sharing. `initialize` offers no `fs` or `terminal` capability, so the CLI reads and runs through its own tools and never asks Modex to. A failed initialization closes that server and clears its cached handshake, so the next turn can start a fresh server. 2. **A session per thread.** `session/new` in the thread's cwd; the session id is the thread's resume handle. A later turn reopens it with `session/resume` (no replay) when the CLI offers it, else `session/load` with the replayed history discarded. A session the server no longer knows starts afresh with a notice. -3. **Mode first.** `session/set_mode` picks the agent below; a refused mode fails the turn, so a - turn never runs looser than asked. Then `session/set_config_option` sets the thread's model - (a refusal is a warning; the session's current model runs) and effort (informational). +3. **Mode first.** `session/set_mode` picks the agent below; a refused mode fails the turn, so a turn never runs looser than asked. Then `session/set_config_option` sets the thread's model (a refusal fails the turn before submitting the prompt) and effort (informational). 4. **The prompt.** `session/prompt` with the text and any image attachments inline (`promptCapabilities.image`). Updates stream as `agent_message_chunk`, `agent_thought_chunk`, `tool_call`, `tool_call_update` and `plan`; the result's `stopReason` and `usage` end the turn. diff --git a/docs/status.md b/docs/status.md index 060894d9..9bd6679c 100644 --- a/docs/status.md +++ b/docs/status.md @@ -61,6 +61,8 @@ and [landing verification](reviews/2026-10-11-landing-clarity.md). ## Review repair delivery +PR #436 repairs two ACP failures after #432 landed: a refused explicit model fails before prompt submission, and failed initialization releases the server so the next turn can start fresh. Scripted-process regressions cover both backend families, and independent review found no blockers in these two repairs. Hosted run 40764795777 at `8db57f5` failed the unchanged 500 ms transcript median gate with a 2019.2 ms median, then reached the macOS job timeout; its desktop suite did not finish and site tests did not run. An exact-candidate local CPU/timeline diagnostic measured a single 234.2 ms cold open but did not reproduce or explain the hosted slowdown and is not five-launch acceptance. The original failure remains unresolved. Integration `78b337d` included main `f03a012`. Fresh build/unit validation passed with 2,244 passes, 26 skips and zero failures; full root E2E passed with 474 desktop cases, seven skips and seven site cases. The five-launch local transcript median was 132.8 ms on Apple M5 Max, 18 cores. These local results do not explain the earlier hosted slowdown; fresh hosted and protected queue validation remain required. The subsequent documentation-only merge of main `33a7820` preserves the same application, test and build tree and reconciles the delivered LM-5 and release rehearsal records. Runtime permission qualification and provider-expansion approval remain separate open gates. + PR #419's LM-5 review found that a rerouted local turn could still be reported completed by the quiet-final timer before interruption was confirmed. The isolated `lm5-reroute-repair` worktree blocks that false success and retains active ownership until native termination. Both event-order regressions failed before the repair. The earlier repair checkpoint passed 53 focused tests and 2,200 unit tests with 25 skips. After the signed delivery rebase, `6ca43ed` passed build, typecheck, full unit validation (2,219 passed, 26 skipped), and root E2E (470 desktop passed, seven skipped; seven site passed). Independent integration review found no blocking findings. PR #424 merged into the #423 delivery branch at `0956376`; its tree exactly matches the reviewed `e0a03ef` candidate. Delivery to `main` landed through PR #423 at `c23e6be`; local-runtime qualification and product admission remain open. The combined branch’s CodeQL alert #28 identified incomplete regex escaping in a test assertion; the repair uses a literal error-message comparison, with five focused tests and the full build/unit command passing (2,219 passed, 26 skipped, zero failures or cancellations). Independent review found no blockers, and hosted CodeQL fixed alert #28. The subsequent macOS run at `d3d61ad` failed the Jev-theme Settings click and teardown (472 passed, four skipped, one failed); its trace is preserved. After integrating main `7a01df2`, including the merged Notes identity repair, final delivery head `618027e` passed protected queue run `40768112164` (473 desktop passes, four skips and seven site passes) and landed through #423. The landed merge tree was verified before its delivery worktree was archived and removed. PR #422's CR-6 review found two blockers: unconfirmed child exit was reported as stopped and released ownership, and provider-bound questions were described as staying on the Mac. The isolated `cr6-review-repair` worktree preserves unknown exit ownership and corrects that disclosure. PR #426 has passed independent repair review, build, unit tests (2,196 passed, 23 skipped), typecheck and full root E2E on diagnostic source `4c5eaee` (470 desktop passed, seven skipped; seven site passed). Earlier failures remain recorded, including unexplained Canvas and IPC failures that passed on later runs. PR #426 subsequently passed its protected queue and merged as `d0482a0`; this repair is on main and unreleased. Native/live acceptance remains separate.