From 42a52283f15f68f40b758d5ca07a1ddc310bf2dc Mon Sep 17 00:00:00 2001 From: aivsomkar Date: Tue, 18 Aug 2026 10:30:44 +0530 Subject: [PATCH 01/52] Let a message reach a running Claude turn (steer), and keep one process per session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verified against claude 2.1.221 with --input-format stream-json: the CLI settles a turn with `result` while stdin stays OPEN (EOF is the exit signal, not the turn signal); the next user message on the same stdin is a new turn in the same process; a message that arrives MID-turn is delivered before the model's next call and folded into the same turn's one result. That last behaviour is exactly the "steer" the plan wanted. - claude.ts keeps one live process per thread across turns: reused while idle, unchanged in spawn contract, and the session the harness wants; otherwise closed and respawned with --resume. `result` settles the turn, not the process; the process closes after 10 minutes idle (OMB_CLAUDE_SESSION_IDLE_MS). steer() writes into the open stdin. - contract: capabilities.queueing and an optional adapter.steer() — the one-file driver promise holds; every other driver keeps the 409. - harness: POST /messages while busy on a queueing engine steers instead of 409ing; the message is appended in order and marked `steered`. The composer stays open on such engines ("Enter sends this into the running turn"); a "sent mid-turn" tag on the bubble says the model saw it. - 3.1 remainder: injected local models carry contextWindow from Ollama's /api/ps context_length when the model is running, so a small model's rebuild is sized to what it can hold instead of a name-based guess. - fake claude rewritten line-driven (steer folding, `slow` mode). Items 3.2 (and the 3.1 remainder) of docs/plans/agent-harness-upgrades-v2.md. Answers the plan's open question 3. Co-Authored-By: Claude Opus 5 (1M context) --- server/contracts.ts | 21 ++- server/drivers/claude.test.ts | 77 ++++++++++- server/drivers/claude.ts | 197 ++++++++++++++++++++++------ server/drivers/local-inject.test.ts | 23 ++++ server/drivers/local-inject.ts | 40 +++++- server/harness/registry.ts | 1 + server/index.ts | 18 +++ server/steer-e2e.test.ts | 133 +++++++++++++++++++ server/store.ts | 4 + server/testing/fake-claude-cli.ts | 90 ++++++++++--- src/components/ChatView.tsx | 5 + src/components/Composer.tsx | 10 +- src/state/store.tsx | 4 + 13 files changed, 559 insertions(+), 64 deletions(-) create mode 100644 server/steer-e2e.test.ts diff --git a/server/contracts.ts b/server/contracts.ts index 16b8d1d42..19e609349 100644 --- a/server/contracts.ts +++ b/server/contracts.ts @@ -184,6 +184,12 @@ export interface ProviderAdapter { * the driver cannot set effort, so the app never offers the control — * same rule as computerMcp: never show a knob the driver cannot turn. */ effortLevels?: readonly EffortLevel[]; + /** True when the driver keeps a live session across turns and can take + * a user message MID-TURN (delivered before the model's next call — + * "steer"). The composer stays open during a turn on such an engine; + * others keep the queue-one-and-wait behaviour. Same rule as the other + * flags: never show a control the driver cannot honour. */ + queueing?: boolean; }; sendTurn(input: SendTurnInput): Promise; interruptTurn(threadId: ThreadId, turnId?: TurnId): Promise; @@ -197,6 +203,10 @@ export interface ProviderAdapter { requestId: string, decision: { behavior: "allow" | "deny" | "answer"; message?: string }, ): Promise; + /** Deliver a user message into the RUNNING turn on this thread. Resolves + * false when there is no live turn to steer (the caller then sends it as + * a normal turn). Only drivers with `capabilities.queueing` implement it. */ + steer?(threadId: ThreadId, text: string): Promise; hasSession(threadId: ThreadId): boolean; stopAll(): Promise; onEvent(listener: RuntimeEventListener): () => void; @@ -238,7 +248,16 @@ export interface EngineInstall { // a rejection to an unavailable shadow snapshot. export interface ModelCatalog { default: string; - options: Array<{ id: string; label: string; custom?: boolean; loaded?: boolean }>; + options: Array<{ + id: string; + label: string; + custom?: boolean; + loaded?: boolean; + /** total context window in tokens, when the driver knows it — sizes + * the model-facing rebuild (server/context-rebuild.ts). Unknown falls + * back to a pattern table over the model id, then a conservative default. */ + contextWindow?: number; + }>; } export interface DriverCreateInput { diff --git a/server/drivers/claude.test.ts b/server/drivers/claude.test.ts index 0072d978c..fe9eb36ba 100644 --- a/server/drivers/claude.test.ts +++ b/server/drivers/claude.test.ts @@ -6,7 +6,7 @@ // These used to be POSIX-only: the fake CLI is a shebang script Windows // cannot exec, and the broker is a unix socket. Both now go through // resolveCliSpawn / permissionSocketPath, so they run everywhere. -import { chmodSync, existsSync, mkdtempSync, readFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; import { connect } from "node:net"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; @@ -405,6 +405,81 @@ describe("ClaudeDriver turns (fake CLI)", () => { conn.end(); }); + // ── live session across turns (steer / follow-up / idle close) ──────── + + it("a message sent mid-turn is steered into the running turn, not a new one", async () => { + await create("slow"); + const { turnId } = await instance.adapter.sendTurn({ threadId: "t-steer", text: "first" }); + // the fake leaves a gap after the tool result — steer into it + await recorder.until((e) => e.type === "item.completed" && e.itemType === "tool"); + expect(instance.adapter.capabilities.queueing).toBe(true); + await expect(instance.adapter.steer!("t-steer", "and also this")).resolves.toBe(true); + await recorder.until((e) => e.type === "turn.completed"); + // one turn, one result; the reply carries the folded message + expect(recorder.events.filter((e) => e.type === "turn.completed")).toHaveLength(1); + const reply = recorder.events.find((e) => e.type === "item.completed" && e.itemType === "assistant_text" && (e as { text: string }).text.startsWith("reply to:")) as { text: string }; + expect(reply.text).toContain("steered: and also this"); + expect(recorder.events.every((e) => e.turnId === turnId)).toBe(true); + // nothing running now: steer has nowhere to go + await expect(instance.adapter.steer!("t-steer", "late")).resolves.toBe(false); + }); + + it("the next turn on the same thread reuses the live process instead of spawning", async () => { + await create(); + process.env.FAKE_CLAUDE_DUMP = join(scratch, "dump.json"); + await instance.adapter.sendTurn({ threadId: "t-live", text: "one" }); + await recorder.until((e) => e.type === "turn.completed"); + const inits1 = recorder.events.filter((e) => e.type === "session.started").length; + // second turn: same contract, same session id → same process. The fake + // dumps argv only on its FIRST prompt, so a re-dump would mean a respawn. + const dumpBefore = readFileSync(join(scratch, "dump.json"), "utf8"); + // the harness resumes with the id the CLI announced — pass that one + const announced = (recorder.events.find((e) => e.type === "session.started") as { sessionId: string }).sessionId; + const second = await instance.adapter.sendTurn({ threadId: "t-live", text: "two", resumeCursor: announced }); + await recorder.until((e) => e.type === "turn.completed" && e.turnId === second.turnId); + expect(readFileSync(join(scratch, "dump.json"), "utf8")).toBe(dumpBefore); + // and the CLI announced init again on the same process (the real one does) + expect(recorder.events.filter((e) => e.type === "session.started").length).toBeGreaterThan(inits1); + expect(recorder.events.filter((e) => e.type === "turn.started")).toHaveLength(2); + expect(recorder.events.filter((e) => e.type === "turn.completed")).toHaveLength(2); + }); + + it("a changed spawn contract (another model) closes the live process and resumes in a new one", async () => { + await create(); + process.env.FAKE_CLAUDE_DUMP = join(scratch, "dump.json"); + await instance.adapter.sendTurn({ threadId: "t-switch", text: "one" }); + await recorder.until((e) => e.type === "turn.completed"); + // a fresh process rewrites the dump; the fake ONLY dumps its first prompt + rmSync(join(scratch, "dump.json")); + const announced = (recorder.events.find((e) => e.type === "session.started") as { sessionId: string }).sessionId; + await instance.adapter.sendTurn({ threadId: "t-switch", text: "two", model: "claude-other", resumeCursor: announced }); + await recorder.until((e) => e.type === "turn.completed" && recorder.events.filter((x) => x.type === "turn.completed").length === 2); + const dump = JSON.parse(readFileSync(join(scratch, "dump.json"), "utf8")); + expect(dump.argv).toContain("--resume"); + expect(dump.argv).toContain("claude-other"); + }); + + it("closes an idle session after the idle window", async () => { + process.env.OMB_CLAUDE_SESSION_IDLE_MS = "10000"; // the floor + try { + await create(); + await instance.adapter.sendTurn({ threadId: "t-idle", text: "one" }); + await recorder.until((e) => e.type === "turn.completed"); + // the process is alive between turns… + await expect(instance.adapter.steer!("t-idle", "x")).resolves.toBe(false); // no running turn, but session exists + // …and after the idle window it is gone: the next turn spawns anew + // (observable as a fresh dump) + process.env.FAKE_CLAUDE_DUMP = join(scratch, "dump.json"); + await new Promise((r) => setTimeout(r, 11_000)); + const announced = (recorder.events.find((e) => e.type === "session.started") as { sessionId: string }).sessionId; + await instance.adapter.sendTurn({ threadId: "t-idle", text: "two", resumeCursor: announced }); + await recorder.until((e) => e.type === "turn.completed" && recorder.events.filter((x) => x.type === "turn.completed").length === 2); + expect(JSON.parse(readFileSync(join(scratch, "dump.json"), "utf8")).argv).toContain("--resume"); + } finally { + delete process.env.OMB_CLAUDE_SESSION_IDLE_MS; + } + }, 30_000); + it("passes effort to the CLI, and omits the flag when unset", async () => { await create(); const dump = join(scratch, "effort.json"); diff --git a/server/drivers/claude.ts b/server/drivers/claude.ts index c5aa41e1b..11d1a99fa 100644 --- a/server/drivers/claude.ts +++ b/server/drivers/claude.ts @@ -339,6 +339,59 @@ export const ClaudeDriver: ProviderDriver = { // one active turn per thread; a second send while busy is a caller bug const active = new Map void; turnId: string; broker?: ReturnType }>(); + // One live CLI process per thread, kept across turns. Under + // --input-format stream-json the CLI settles a turn with `result` while + // stdin stays open, takes the next user message on the same stdin as a + // new turn, and folds a message that arrives MID-turn into the running + // one before its next model call (verified against 2.1.221 — that fold + // is what "steer" is). So a session is spawned once, reused while its + // spawn contract (args, MCP config, cwd, model) is unchanged, closed + // after SESSION_IDLE_MS of quiet, and resumed by --resume when needed. + interface Session { + child: ReturnType; + broker?: ReturnType; + mcpConfigPath: string | null; + /** the spawn contract — a different one means a fresh process */ + argsKey: string; + /** the CLI's session id from `init`, what --resume takes later */ + sessionId: string | null; + /** the running turn, or null between turns */ + turn: { turnId: string; settled: boolean; sawStreamDelta: boolean } | null; + idleTimer: ReturnType | null; + closing: boolean; + stderr: string; + } + const sessions = new Map(); + const SESSION_IDLE_MS = Math.max(10_000, Number(process.env.OMB_CLAUDE_SESSION_IDLE_MS) || 10 * 60_000); + + const closeSession = (threadId: string, why: string) => { + const s = sessions.get(threadId); + if (!s || s.closing) return; + s.closing = true; + if (s.idleTimer) clearTimeout(s.idleTimer); + appendNative(threadId, { dir: "out", source: "claude.session", msg: { close: why } }); + // stdin EOF is the CLI's exit signal; give it a moment, then insist + try { + s.child.stdin.end(); + } catch {} + const kill = setTimeout(() => { + if (s.child.exitCode === null) killCliTree(s.child); + }, 5_000); + kill.unref?.(); + }; + const armIdle = (threadId: string) => { + const s = sessions.get(threadId); + if (!s) return; + if (s.idleTimer) clearTimeout(s.idleTimer); + s.idleTimer = setTimeout(() => closeSession(threadId, "idle"), SESSION_IDLE_MS); + s.idleTimer.unref?.(); + }; + const writeUser = (s: Session, threadId: string, text: string) => { + const promptMsg = { type: "user", message: { role: "user", content: text } }; + s.child.stdin.write(JSON.stringify(promptMsg) + "\n"); + appendNative(threadId, { dir: "out", source: "claude.sdk.message", msg: promptMsg }); + }; + const emit = (event: RuntimeEvent) => { for (const l of [...listeners]) l(event); }; @@ -367,8 +420,6 @@ export const ClaudeDriver: ProviderDriver = { "--include-partial-messages", "--permission-mode", config.permissionMode === "auto" ? "acceptEdits" : config.permissionMode, ]; - if (sessionId) args.push("--resume", sessionId); - else args.push("--session-id", newSessionId!); const turnEnvironment: NodeJS.ProcessEnv = { ...process.env, ...input.environment }; const injected = applyClaudeInject({ ...turnEnvironment }, turn.model); if (injected.model) args.push("--model", injected.model); @@ -468,32 +519,74 @@ export const ClaudeDriver: ProviderDriver = { } const env = claudeEnvironment(turn.model, turnEnvironment); + const cwd = turn.cwd ?? homedir(); + // everything that shapes the process, minus session/turn specifics + // (the --mcp-config file is a fresh temp path each time; its CONTENT + // is what matters and mcpServers carries that) + const keyArgs = args.filter((a, i) => a !== "--mcp-config" && args[i - 1] !== "--mcp-config"); + const argsKey = JSON.stringify({ args: keyArgs, mcpServers, cwd, model: injected.model ?? null, base: env.ANTHROPIC_BASE_URL ?? null }); + + // Reuse the live process when it is idle, unchanged, and is the session + // the harness wants resumed. Anything else: close it and spawn fresh + // (with --resume, so the conversation continues in the new process). + const live = sessions.get(threadId); + if (live && !live.turn && !live.closing && live.child.exitCode === null && live.argsKey === argsKey && (!sessionId || sessionId === live.sessionId)) { + if (live.idleTimer) clearTimeout(live.idleTimer); + live.turn = { turnId, settled: false, sawStreamDelta: false }; + active.set(threadId, { stop: () => killCliTree(live.child), turnId, broker: live.broker }); + emit({ ...base(threadId, turnId), type: "turn.started" }); + writeUser(live, threadId, turn.text); + // the MCP config was for the first spawn; nothing to clean here + if (mcpConfigPath) { + try { + rmSync(dirname(mcpConfigPath), { recursive: true, force: true }); + } catch {} + } + broker?.close(); // the session's own broker stays; this one was provisional + return { turnId }; + } + if (live) closeSession(threadId, "spawn contract changed"); + if (sessionId) args.push("--resume", sessionId); + else args.push("--session-id", newSessionId!); const child = spawnCli(config.cli, args, { - cwd: turn.cwd ?? homedir(), + cwd, env, stdio: ["pipe", "pipe", "pipe"], }); + const session: Session = { + child, + broker, + mcpConfigPath, + argsKey, + sessionId: sessionId ?? newSessionId, + turn: { turnId, settled: false, sawStreamDelta: false }, + idleTimer: null, + closing: false, + stderr: "", + }; + sessions.set(threadId, session); - let settled = false; + // settles the TURN, not the process: the CLI stays for the next + // message until it has been quiet for SESSION_IDLE_MS const settle = (ok: boolean, stopReason: string | null, cost: number | null = null) => { - if (settled) return; - settled = true; - broker?.close(); - // the config file holds live credentials — it must not outlive the turn - if (mcpConfigPath) { + const t = session.turn; + if (!t || t.settled) return; + t.settled = true; + // the config file holds live credentials — the CLI read it at start; + // it must not sit on disk for the life of the session + if (session.mcpConfigPath) { try { - rmSync(dirname(mcpConfigPath), { recursive: true, force: true }); + rmSync(dirname(session.mcpConfigPath), { recursive: true, force: true }); } catch {} + session.mcpConfigPath = null; } active.delete(threadId); - emit({ ...base(threadId, turnId), type: "turn.completed", ok, stopReason, cost }); + session.turn = null; + emit({ ...base(threadId, t.turnId), type: "turn.completed", ok, stopReason, cost }); + if (session.child.exitCode === null && !session.closing) armIdle(threadId); }; - - // token streaming: true while --include-partial-messages is delivering - // text deltas for the current assistant message, so the whole-message - // frame that follows doesn't re-emit the same text as one big delta - let sawStreamDelta = false; + const currentTurnId = () => session.turn?.turnId ?? turnId; const handleLine = (line: string) => { let o: any; @@ -506,9 +599,10 @@ export const ClaudeDriver: ProviderDriver = { switch (o.type) { case "system": if (o.subtype === "init") { - emit({ ...base(threadId, turnId), type: "session.started", sessionId: o.session_id, model: o.model }); + if (typeof o.session_id === "string") session.sessionId = o.session_id; + emit({ ...base(threadId, currentTurnId()), type: "session.started", sessionId: o.session_id, model: o.model }); } else if (o.subtype === "thinking_tokens") { - emit({ ...base(threadId, turnId), type: "item.updated", itemType: "reasoning", tokens: o.estimated_tokens }); + emit({ ...base(threadId, currentTurnId()), type: "item.updated", itemType: "reasoning", tokens: o.estimated_tokens }); } break; case "stream_event": { @@ -519,10 +613,10 @@ export const ClaudeDriver: ProviderDriver = { if (ev.type !== "content_block_delta") break; const d = ev.delta ?? {}; if (d.type === "text_delta" && typeof d.text === "string" && d.text) { - sawStreamDelta = true; - emit({ ...base(threadId, turnId), type: "content.delta", streamKind: "assistant_text", delta: d.text }); + if (session.turn) session.turn.sawStreamDelta = true; + emit({ ...base(threadId, currentTurnId()), type: "content.delta", streamKind: "assistant_text", delta: d.text }); } else if (d.type === "thinking_delta" && typeof d.thinking === "string" && d.thinking) { - emit({ ...base(threadId, turnId), type: "content.delta", streamKind: "reasoning_text", delta: d.thinking }); + emit({ ...base(threadId, currentTurnId()), type: "content.delta", streamKind: "reasoning_text", delta: d.thinking }); } break; } @@ -531,20 +625,20 @@ export const ClaudeDriver: ProviderDriver = { const text = firstText(msg.content); if (text.trim()) { // fallback delta for CLIs/paths that never streamed the block - if (!sawStreamDelta) { - emit({ ...base(threadId, turnId), type: "content.delta", streamKind: "assistant_text", delta: text }); + if (!session.turn?.sawStreamDelta) { + emit({ ...base(threadId, currentTurnId()), type: "content.delta", streamKind: "assistant_text", delta: text }); } - sawStreamDelta = false; - emit({ ...base(threadId, turnId), type: "item.completed", itemType: "assistant_text", text }); + if (session.turn) session.turn.sawStreamDelta = false; + emit({ ...base(threadId, currentTurnId()), type: "item.completed", itemType: "assistant_text", text }); } for (const b of Array.isArray(msg.content) ? msg.content : []) { if (b.type === "tool_use") { - emit({ ...base(threadId, turnId), type: "item.started", itemType: "tool", itemId: b.id, title: b.name }); + emit({ ...base(threadId, currentTurnId()), type: "item.started", itemType: "tool", itemId: b.id, title: b.name }); } } if (msg.usage) { emit({ - ...base(threadId, turnId), + ...base(threadId, currentTurnId()), type: "thread.token-usage.updated", input: (msg.usage.input_tokens || 0) + (msg.usage.cache_read_input_tokens || 0), output: msg.usage.output_tokens || 0, @@ -555,7 +649,7 @@ export const ClaudeDriver: ProviderDriver = { case "user": for (const b of Array.isArray(o.message?.content) ? o.message.content : []) { if (b.type === "tool_result") { - emit({ ...base(threadId, turnId), type: "item.completed", itemType: "tool", itemId: b.tool_use_id, ok: !b.is_error }); + emit({ ...base(threadId, currentTurnId()), type: "item.completed", itemType: "tool", itemId: b.tool_use_id, ok: !b.is_error }); } } break; @@ -579,41 +673,59 @@ export const ClaudeDriver: ProviderDriver = { } }); - let stderr = ""; child.stderr.on("data", (c) => { - stderr += c; - if (stderr.length > 8192) stderr = stderr.slice(-8192); + session.stderr += c; + if (session.stderr.length > 8192) session.stderr = session.stderr.slice(-8192); }); child.on("error", (e) => { - emit({ ...base(threadId, turnId), type: "runtime.error", ...describeSpawnFailure(e, config.cli) }); + emit({ ...base(threadId, currentTurnId()), type: "runtime.error", ...describeSpawnFailure(e, config.cli) }); settle(false, "spawn_error"); }); child.on("close", (code) => { - if (!settled) { + // a turn still running when the process died is a failed turn; a + // process that exited between turns (idle close, contract change) + // is just a session ending + if (session.turn && !session.turn.settled) { emit({ - ...base(threadId, turnId), + ...base(threadId, currentTurnId()), type: "runtime.error", - message: `claude exited ${code} before result${stderr ? `: ${stderr.trim().slice(-300)}` : ""}`, + message: `claude exited ${code} before result${session.stderr ? `: ${session.stderr.trim().slice(-300)}` : ""}`, }); settle(false, "exit_before_result"); } + if (session.idleTimer) clearTimeout(session.idleTimer); + session.broker?.close(); + if (session.mcpConfigPath) { + try { + rmSync(dirname(session.mcpConfigPath), { recursive: true, force: true }); + } catch {} + } + if (sessions.get(threadId) === session) sessions.delete(threadId); }); const stop = () => killCliTree(child); active.set(threadId, { stop, turnId, broker }); emit({ ...base(threadId, turnId), type: "turn.started" }); - // prompt over stdin as a stream-json message — never argv (ARG_MAX) - const promptMsg = { type: "user", message: { role: "user", content: turn.text } }; - child.stdin.write(JSON.stringify(promptMsg) + "\n"); - child.stdin.end(); - appendNative(threadId, { dir: "out", source: "claude.sdk.message", msg: promptMsg }); + // prompt over stdin as a stream-json message — never argv (ARG_MAX). + // stdin stays OPEN: that is what keeps the session alive for a + // mid-turn steer or the next turn; closeSession() ends it. + writeUser(session, threadId, turn.text); return { turnId }; }; + /** A user message into the running turn: the CLI delivers it before its + * next model call. False when nothing is running here to steer. */ + const steer = async (threadId: string, text: string): Promise => { + const s = sessions.get(threadId); + if (!s || !s.turn || s.turn.settled || s.closing || s.child.exitCode !== null) return false; + writeUser(s, threadId, text); + return true; + }; + const snapshot = async (): Promise => { const env = claudeEnvironment(undefined, { ...process.env, ...input.environment }); const version = await new Promise((resolve) => { @@ -644,13 +756,15 @@ export const ClaudeDriver: ProviderDriver = { computerMcp: true, composioMcp: true, effortLevels: ["low", "medium", "high", "xhigh", "max"], + queueing: true, }, sendTurn, + steer, interruptTurn: async (threadId) => active.get(threadId)?.stop(), respondToRequest: async (threadId, requestId, decision) => { // fail-closed by construction: no broker, or an ask that already // timed out / settled, is `unavailable` — the caller denies - const broker = active.get(threadId)?.broker; + const broker = sessions.get(threadId)?.broker ?? active.get(threadId)?.broker; if (!broker) return "unavailable"; const behavior = decision.behavior === "answer" ? "answer" : decision.behavior; if (!broker.answer(requestId, behavior, decision.message)) return "unavailable"; @@ -659,6 +773,7 @@ export const ClaudeDriver: ProviderDriver = { hasSession: (threadId) => active.has(threadId), stopAll: async () => { for (const { stop } of active.values()) stop(); + for (const threadId of [...sessions.keys()]) closeSession(threadId, "stopAll"); }, onEvent: (listener) => { listeners.add(listener); diff --git a/server/drivers/local-inject.test.ts b/server/drivers/local-inject.test.ts index 7e7ab60dc..5f42ee888 100644 --- a/server/drivers/local-inject.test.ts +++ b/server/drivers/local-inject.test.ts @@ -20,6 +20,7 @@ import { codexLocalProviderArgs, decodeInjectId, encodeInjectId, + contextWindowsFromPs, loadedIdsFromPayloads, LOCAL_HOSTS, mergeLocalInject, @@ -45,6 +46,28 @@ describe("inject ids", () => { }); }); +describe("contextWindowsFromPs", () => { + it("reads Ollama's per-model context_length from /api/ps, keyed by full and base id", () => { + const windows = contextWindowsFromPs({ + models: [ + { name: "qwen3:8b", model: "qwen3:8b", context_length: 40960 }, + { name: "llama3.2:latest", model: "llama3.2:latest", context_length: 8192 }, + { name: "no-ctx:1b", model: "no-ctx:1b" }, + { name: "bad:1b", model: "bad:1b", context_length: -1 }, + ], + }); + expect(windows.get("qwen3:8b")).toBe(40960); + expect(windows.get("qwen3")).toBe(40960); + expect(windows.get("llama3.2:latest")).toBe(8192); + expect(windows.has("no-ctx:1b")).toBe(false); + expect(windows.has("bad:1b")).toBe(false); + }); + it("tolerates payloads that are not a ps listing", () => { + expect(contextWindowsFromPs(null).size).toBe(0); + expect(contextWindowsFromPs({ data: [] }).size).toBe(0); + }); +}); + describe("loadedIdsFromPayloads", () => { const omlx = LOCAL_HOSTS.find((host) => host.id === "omlx")!; const ollama = LOCAL_HOSTS.find((host) => host.id === "ollama")!; diff --git a/server/drivers/local-inject.ts b/server/drivers/local-inject.ts index 6287afa1c..ba4409e82 100644 --- a/server/drivers/local-inject.ts +++ b/server/drivers/local-inject.ts @@ -38,6 +38,30 @@ export interface InjectedModel { label: string; /** In VRAM / running on the host right now — Custom pins these first. */ loaded?: boolean; + /** the host's own word on the model's context window (Ollama reports it + * for running models in /api/ps) — sizes the model-facing rebuild instead + * of guessing from the name */ + contextWindow?: number; +} + +/** Ollama's /api/ps lists running models with their context_length; a + * small model's real window matters more than a big one's — an 8k model + * guessed at 32k gets a rebuild it cannot hold. */ +export function contextWindowsFromPs(extra: unknown): Map { + const out = new Map(); + const rec = extra && typeof extra === "object" ? (extra as { models?: unknown }) : null; + if (!rec || !Array.isArray(rec.models)) return out; + for (const m of rec.models) { + if (!m || typeof m !== "object") continue; + const row = m as { name?: unknown; model?: unknown; context_length?: unknown }; + const id = typeof row.model === "string" ? row.model : typeof row.name === "string" ? row.name : null; + const ctx = typeof row.context_length === "number" && Number.isFinite(row.context_length) && row.context_length > 0 ? row.context_length : null; + if (id && ctx) { + out.set(id, ctx); + out.set(id.split(":")[0]!, ctx); + } + } + return out; } export function encodeInjectId(host: string, model: string): string { @@ -258,17 +282,20 @@ export async function probeLocalInjects( const extraIds = extra ? idsFromModelsPayload(extra) : []; const loaded = loadedIdsFromPayloads(host, catalog ?? extra, extra); const ids = [...new Set([...catalogIds, ...extraIds, ...loaded])]; - return { host, ids, loaded }; + const windows = contextWindowsFromPs(extra); + return { host, ids, loaded, windows }; }), ); - for (const { host, ids, loaded } of pages) { + for (const { host, ids, loaded, windows } of pages) { for (const model of ids) { + const contextWindow = windows.get(model); found.push({ id: encodeInjectId(host.id, model), host: host.id, model, label: `${model} (${host.label})`, loaded: loaded.has(model), + ...(contextWindow ? { contextWindow } : {}), }); } } @@ -292,10 +319,17 @@ export async function mergeLocalInject( const existing = options.find((option) => option.id === extra.id); if (existing) { if (extra.loaded) existing.loaded = true; + if (extra.contextWindow) existing.contextWindow = extra.contextWindow; continue; } seen.add(extra.id); - options.push({ id: extra.id, label: extra.label, custom: true, ...(extra.loaded ? { loaded: true } : {}) }); + options.push({ + id: extra.id, + label: extra.label, + custom: true, + ...(extra.loaded ? { loaded: true } : {}), + ...(extra.contextWindow ? { contextWindow: extra.contextWindow } : {}), + }); } return { default: catalog.default, options }; } diff --git a/server/harness/registry.ts b/server/harness/registry.ts index 9511b69a2..4f459d2fa 100644 --- a/server/harness/registry.ts +++ b/server/harness/registry.ts @@ -171,6 +171,7 @@ export class ProviderRegistry { agentsMcp: inst.adapter.capabilities.agentsMcp === true, composioMcp: inst.adapter.capabilities.composioMcp === true, effortLevels: inst.adapter.capabilities.effortLevels, + queueing: inst.adapter.capabilities.queueing === true, }, access: driver?.metadata.access ?? "subscription", install: driver?.install, diff --git a/server/index.ts b/server/index.ts index 48bd18c32..b8426e58c 100644 --- a/server/index.ts +++ b/server/index.ts @@ -2648,6 +2648,24 @@ const server = createServer(async (req, res) => { const body = await readBody(req); const text = String(body.text ?? "").trim(); if (!text) return json(res, 400, { error: "text required" }); + // A message during a running turn: engines that keep a live session + // (capabilities.queueing) take it INTO the turn — the model sees it + // before its next call, the way typing into Claude Code's own UI mid- + // task does. It lands in the transcript in order. Engines without + // that keep the 409 the composer already knows how to queue behind. + const busyBot = store.bot(m[1]); + if (busyBot?.busy && !busyBot.hidden) { + const instance = registry.get(busyBot.modelSelection.instanceId); + if (instance?.adapter.capabilities.queueing && instance.adapter.steer) { + const steered = await instance.adapter.steer(busyBot.threadId, text).catch(() => false); + if (steered) { + store.appendMessage(busyBot.threadId, { role: "user", kind: "text", text, steered: true }); + return json(res, 202, { ok: true, steered: true }); + } + // the turn settled between the busy check and the write — fall + // through and send it as the next turn instead + } + } await startTurn(m[1], text); return json(res, 202, { ok: true }); } diff --git a/server/steer-e2e.test.ts b/server/steer-e2e.test.ts new file mode 100644 index 000000000..759f04097 --- /dev/null +++ b/server/steer-e2e.test.ts @@ -0,0 +1,133 @@ +// Mid-turn steering, end to end: boots the real harness with the fake claude +// CLI in `slow` mode (a gap after the tool result the way a real turn has +// between model calls), sends a message WHILE the turn runs, and asserts +// it is taken into the turn — 202 steered, not 409; in the transcript in +// order and marked; folded into the reply — while an engine without a live +// session still gets the 409 the composer queues behind. +// +// POSIX-gated like the other CLI e2es (the fakes are shebang scripts). +import { spawn, type ChildProcess } from "node:child_process"; +import { chmodSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { afterAll, beforeAll, describe, expect, it } from "vitest"; + +const SERVER_DIR = dirname(fileURLToPath(import.meta.url)); +const FAKE_CLAUDE = join(SERVER_DIR, "testing", "fake-claude-cli.ts"); +const FAKE_ACP = join(SERVER_DIR, "testing", "fake-acp-cli.ts"); +const PORT = 18800 + Math.floor(Math.random() * 10_000); +const BASE = `http://127.0.0.1:${PORT}`; +const posixOnly = describe.skipIf(process.platform === "win32"); + +posixOnly("mid-turn steering e2e", () => { + let child: ChildProcess; + let home: string; + let stderr = ""; + + const api = async (method: string, path: string, body?: unknown): Promise<{ status: number; body: any }> => { + const res = await fetch(`${BASE}${path}`, { + method, + headers: body ? { "content-type": "application/json" } : undefined, + body: body ? JSON.stringify(body) : undefined, + }); + return { status: res.status, body: await res.json() }; + }; + const getBot = async (id: string) => (await api("GET", "/api/bots")).body.bots.find((b: any) => b.id === id); + const waitFor = async (predicate: () => Promise, what: string, ms = 30_000) => { + const deadline = Date.now() + ms; + while (!(await predicate())) { + if (Date.now() > deadline) throw new Error(`timed out waiting for ${what}. stderr: ${stderr.slice(-2000)}`); + await new Promise((r) => setTimeout(r, 100)); + } + }; + + beforeAll(async () => { + chmodSync(FAKE_CLAUDE, 0o755); + chmodSync(FAKE_ACP, 0o755); + home = mkdtempSync(join(tmpdir(), "omb-steer-")); + mkdirSync(join(home, ".openmausbot"), { recursive: true }); + writeFileSync( + join(home, ".openmausbot", "config.json"), + JSON.stringify({ + instances: { + claude: { driver: "claudeAgent", environment: { FAKE_CLAUDE_MODE: "slow" }, config: { cli: FAKE_CLAUDE, permissionMode: "bypassPermissions" } }, + // no live session: a message while busy is a 409 the composer queues behind + acp: { driver: "grokAgent", environment: { FAKE_ACP_MODE: "hang" }, config: { cli: FAKE_ACP, fullAuto: true } }, + }, + }), + ); + child = spawn(process.execPath, [join(SERVER_DIR, "index.ts")], { + cwd: join(SERVER_DIR, ".."), + env: { ...(process.env.PATH ? { PATH: process.env.PATH } : {}), HOME: home, USERPROFILE: home, OMB_PORT: String(PORT) }, + stdio: ["ignore", "pipe", "pipe"], + }); + child.stderr!.on("data", (c) => (stderr += c)); + const deadline = Date.now() + 20_000; + for (;;) { + try { + if ((await fetch(`${BASE}/api/health`)).ok) break; + } catch { + /* not up yet */ + } + if (Date.now() > deadline) throw new Error(`server never came up. stderr:\n${stderr}`); + if (child.exitCode !== null) throw new Error(`server exited ${child.exitCode}. stderr:\n${stderr}`); + await new Promise((r) => setTimeout(r, 150)); + } + }, 30_000); + + afterAll(async () => { + child?.kill("SIGTERM"); + await new Promise((resolve) => { + if (!child || child.exitCode !== null) return resolve(); + child.on("close", () => resolve()); + setTimeout(() => (child.kill("SIGKILL"), resolve()), 5_000).unref?.(); + }); + rmSync(home, { recursive: true, force: true }); + }); + + it( + "a message during a Claude turn is steered into it: 202, in the transcript in order and marked, folded into the reply", + async () => { + const created = (await api("POST", "/api/bots")).body.bot; + await api("PATCH", `/api/bots/${created.id}`, { modelSelection: { instanceId: "claude", model: "claude-fake" } }); + const instances = (await api("GET", "/api/instances")).body.instances; + expect(instances.find((i: any) => i.instanceId === "claude").capabilities.queueing).toBe(true); + + expect((await api("POST", `/api/bots/${created.id}/messages`, { text: "first" })).status).toBe(202); + await waitFor(async () => (await getBot(created.id)).busy === true, "the turn to start"); + // the fake pauses after its tool result; this lands inside that gap + await waitFor(async () => (await getBot(created.id)).messages.some((m: any) => m.kind === "activity"), "the tool chip"); + const second = await api("POST", `/api/bots/${created.id}/messages`, { text: "and also this" }); + expect(second.status).toBe(202); + expect(second.body.steered).toBe(true); + + await waitFor(async () => (await getBot(created.id)).busy === false, "the turn to settle"); + const bot = await getBot(created.id); + const texts = bot.messages.filter((m: any) => m.kind === "text").map((m: any) => `${m.role}:${m.text}`); + // order: greeting, first, the fake's opening line, the steered message + // (appended when it was sent — mid-turn), then ONE reply carrying it + expect(texts.slice(1)).toEqual([ + "user:first", + "bot:hello from fake claude", + "user:and also this", + "bot:reply to: first + steered: and also this", + ]); + const steered = bot.messages.find((m: any) => m.text === "and also this"); + expect(steered.steered).toBe(true); + // one turn, not two: exactly one reply + expect(bot.messages.filter((m: any) => m.role === "bot" && m.kind === "text" && m.text.startsWith("reply to:"))).toHaveLength(1); + }, + 40_000, + ); + + it("an engine without a live session still refuses a message while busy (the composer queues it)", async () => { + const created = (await api("POST", "/api/bots")).body.bot; + await api("PATCH", `/api/bots/${created.id}`, { modelSelection: { instanceId: "acp", model: "fake-model" } }); + expect((await api("POST", `/api/bots/${created.id}/messages`, { text: "first" })).status).toBe(202); + await waitFor(async () => (await getBot(created.id)).busy === true, "the hung turn to start"); + expect((await api("POST", `/api/bots/${created.id}/messages`, { text: "second" })).status).toBe(409); + await api("POST", `/api/bots/${created.id}/interrupt`); + await waitFor(async () => (await getBot(created.id)).busy === false, "the turn to settle"); + }, 30_000); +}); diff --git a/server/store.ts b/server/store.ts index dbfd8e09d..87c07c979 100644 --- a/server/store.ts +++ b/server/store.ts @@ -63,6 +63,10 @@ export interface Message { /** `setup` marks an error the user fixes by installing or configuring * something — the UI offers setup instead of a retry that cannot work. */ tool?: { name: string; ok?: boolean; spoken?: string; setup?: boolean }; + /** user messages sent INTO a running turn (capabilities.queueing): the + * model saw it mid-turn, so the transcript marks it — a reader should + * know the reply above it may already account for this line */ + steered?: boolean; /** screen messages: a frame of the bot's computer (base64 image) */ png?: string; mime?: string; diff --git a/server/testing/fake-claude-cli.ts b/server/testing/fake-claude-cli.ts index b6550f0ec..d6c89ae04 100755 --- a/server/testing/fake-claude-cli.ts +++ b/server/testing/fake-claude-cli.ts @@ -52,18 +52,36 @@ if (argv[0] === "auth" && argv[1] === "status") { ); } -let stdin = ""; -process.stdin.on("data", (c) => (stdin += c)); -process.stdin.on("end", () => { - type JsonValue = string | number | boolean | null | JsonValue[] | { [key: string]: JsonValue }; - let prompt: JsonValue = null; - try { - prompt = JSON.parse(stdin.split("\n").find((l) => l.trim()) ?? "null"); - } catch { - /* leave null — the test will see it */ - } +// Line-driven, like the real CLI under --input-format stream-json: each user +// message starts a turn; a message that arrives WHILE a turn is playing is +// folded into it (the real CLI delivers it before the next model call — the +// harness calls that a steer); the process stays alive with stdin open and +// exits only when stdin ends. `slow` leaves a gap between the tool result +// and the reply so a test can steer into it. +type JsonValue = string | number | boolean | null | JsonValue[] | { [key: string]: JsonValue }; +const sessionId = argAfter("--resume") ?? argAfter("--session-id") ?? "fake-session"; +const model = argAfter("--model") ?? "claude-fake"; +let dumped = false; +let initSent = false; +let turnRunning = false; +let steered: string[] = []; +const queue: JsonValue[] = []; +let stdinEnded = false; + +const promptText = (prompt: JsonValue): string => { + const m = prompt && typeof prompt === "object" && !Array.isArray(prompt) ? (prompt as { message?: { content?: unknown } }).message : undefined; + return typeof m?.content === "string" ? m.content : ""; +}; - if (process.env.FAKE_CLAUDE_DUMP) { +const finishIfDone = () => { + if (stdinEnded && !turnRunning && queue.length === 0) process.exit(0); +}; + +const playTurn = (prompt: JsonValue) => { + turnRunning = true; + steered = []; + if (!dumped && process.env.FAKE_CLAUDE_DUMP) { + dumped = true; const configPath = argAfter("--mcp-config"); let mcpConfig: unknown = null; if (configPath) { @@ -76,15 +94,14 @@ process.stdin.on("end", () => { writeFileSync(process.env.FAKE_CLAUDE_DUMP, JSON.stringify({ argv, env: process.env, prompt, mcpConfig }, null, 2)); } - const sessionId = argAfter("--resume") ?? argAfter("--session-id") ?? "fake-session"; - const model = argAfter("--model") ?? "claude-fake"; - if (mode === "exit-early") { process.stderr.write("fake-claude: simulated crash before result\n"); process.exit(3); } + // the real CLI re-announces init on every turn of a live process out({ type: "system", subtype: "init", session_id: sessionId, model }); + initSent = true; if (mode === "hang") { // stay alive until killed — lets tests exercise interrupt + the @@ -121,6 +138,47 @@ process.stdin.on("end", () => { }, }); out({ type: "user", message: { content: [{ type: "tool_result", tool_use_id: "tu-1", is_error: false }] } }); - out({ type: "result", is_error: false, stop_reason: "end_turn", total_cost_usd: 0.01 }); - process.exit(0); + + const finish = () => { + out({ type: "result", is_error: false, stop_reason: "end_turn", total_cost_usd: 0.01, usage: { input_tokens: 10, cache_read_input_tokens: 2, output_tokens: 5 } }); + turnRunning = false; + if (queue.length) playTurn(queue.shift()!); + else finishIfDone(); + }; + if (mode === "slow") { + // a gap a test can steer into; the closing reply carries anything that + // was folded in, the way the real CLI includes a mid-turn message in + // the same turn's next model call + setTimeout(() => { + const tail = steered.length ? ` + steered: ${steered.join(" | ")}` : ""; + out({ type: "assistant", message: { content: [{ type: "text", text: `reply to: ${promptText(prompt)}${tail}` }] } }); + finish(); + }, 800); + } else { + finish(); + } +}; + +let buf = ""; +process.stdin.on("data", (c) => { + buf += c; + let nl; + while ((nl = buf.indexOf("\n")) !== -1) { + const line = buf.slice(0, nl); + buf = buf.slice(nl + 1); + if (!line.trim()) continue; + let prompt: JsonValue = null; + try { + prompt = JSON.parse(line); + } catch { + continue; + } + if (turnRunning) steered.push(promptText(prompt)); + else playTurn(prompt); + } +}); +process.stdin.on("end", () => { + stdinEnded = true; + finishIfDone(); }); +void initSent; diff --git a/src/components/ChatView.tsx b/src/components/ChatView.tsx index 5f83e16b3..103723407 100644 --- a/src/components/ChatView.tsx +++ b/src/components/ChatView.tsx @@ -351,6 +351,11 @@ function Bubble({ > {text} + {message.steered && ( +
+ sent mid-turn +
+ )} {collapsible && ( + + ); +} + +const cnSwitch = (on: boolean) => + `relative h-6 w-11 shrink-0 rounded-full transition-colors ${on ? "bg-accent" : "bg-raised"}`; +const cnKnob = (on: boolean) => + `absolute top-[3px] h-[18px] w-[18px] rounded-full bg-white transition-all ${on ? "left-[21px]" : "left-[3px]"}`; + export function SettingsModal() { const { state, dispatch } = useStore(); const section = state.appSettingsSection; @@ -203,6 +237,7 @@ export function SettingsModal() { + )} diff --git a/src/lib/analytics.test.ts b/src/lib/analytics.test.ts new file mode 100644 index 000000000..05317d63e --- /dev/null +++ b/src/lib/analytics.test.ts @@ -0,0 +1,81 @@ +// The opt-out has one job that matters: an install that turned analytics off +// must not talk to PostHog at all. optAction pins the decision, and the +// storage round-trip pins that the choice survives a restart. +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { analyticsEnabled, initAnalytics, optAction, setAnalyticsEnabled } from "./analytics"; + +// The suite runs on the node environment, which has no localStorage. +const store = new Map(); +vi.stubGlobal("localStorage", { + getItem: (k: string) => store.get(k) ?? null, + setItem: (k: string, v: string) => void store.set(k, v), + removeItem: (k: string) => void store.delete(k), +}); + +beforeEach(() => store.clear()); + +describe("optAction", () => { + it("initialises on the first opt-in of a session that started off", () => { + expect(optAction(true, false)).toBe("init"); + }); + + it("opts a running client back in rather than initialising twice", () => { + expect(optAction(true, true)).toBe("opt-in"); + }); + + it("stops a running client without waiting for a restart", () => { + expect(optAction(false, true)).toBe("opt-out"); + }); + + it("does nothing when there is no client to stop", () => { + // The important half: opting out before init must not reach PostHog to + // tell it so — that request would itself be the leak. + expect(optAction(false, false)).toBe("none"); + }); +}); + +describe("the stored choice", () => { + it("is on for a fresh install", () => { + expect(analyticsEnabled()).toBe(true); + }); + + it("survives a restart once opted out", () => { + setAnalyticsEnabled(false); + expect(analyticsEnabled()).toBe(false); // same read a later launch performs + }); + + it("can be turned back on", () => { + setAnalyticsEnabled(false); + setAnalyticsEnabled(true); + expect(analyticsEnabled()).toBe(true); + }); + + it("treats unusable storage as a fresh install rather than failing", () => { + vi.stubGlobal("localStorage", { + getItem: () => { + throw new Error("denied"); + }, + setItem: () => { + throw new Error("denied"); + }, + }); + expect(analyticsEnabled()).toBe(true); + expect(() => setAnalyticsEnabled(false)).not.toThrow(); + vi.stubGlobal("localStorage", { + getItem: (k: string) => store.get(k) ?? null, + setItem: (k: string, v: string) => void store.set(k, v), + }); + }); +}); + +describe("initAnalytics while opted out", () => { + it("returns before touching the client or the install marker", () => { + // No client is stubbed here on purpose: if init() got past the guard it + // would reach the real posthog-js, and the missing marker proves it did + // not — opting back in later still counts the install. + setAnalyticsEnabled(false); + initAnalytics(); + expect(store.get("omb-installed")).toBeUndefined(); + }); +}); diff --git a/src/lib/analytics.ts b/src/lib/analytics.ts index d8eb2ae23..e55d753bf 100644 --- a/src/lib/analytics.ts +++ b/src/lib/analytics.ts @@ -9,10 +9,57 @@ import posthog from "posthog-js"; const TOKEN = "phc_m2hP39w8y2gLPvHgDvSXAu6xcZ3agjf4ruL56rGcMZEe"; +// Analytics are on by default; Settings → General turns them off. The choice +// lives in localStorage because it has to be readable BEFORE init() runs: an +// opted-out install must never call posthog.init(), so no request — not even +// the library's own — leaves the machine. Once running, opting out routes +// through opt_out_capturing(), which also drops anything already queued. +const OPT_OUT_KEY = "omb-analytics-opt-out"; + let ready = false; +/** False once the user has opted out on this machine. */ +export function analyticsEnabled(): boolean { + try { + return localStorage.getItem(OPT_OUT_KEY) !== "1"; + } catch { + return true; // storage unavailable → behave like a fresh install + } +} + +/** What flipping the switch has to do, given the new setting and whether the + * client is already running. A plain function so the decision can be checked + * without standing up an analytics client to observe. */ +export type OptAction = "init" | "opt-in" | "opt-out" | "none"; +export function optAction(enabled: boolean, running: boolean): OptAction { + if (!enabled) return running ? "opt-out" : "none"; + return running ? "opt-in" : "init"; +} + +/** Flip the setting and act on it immediately, in both directions. */ +export function setAnalyticsEnabled(enabled: boolean) { + try { + localStorage.setItem(OPT_OUT_KEY, enabled ? "0" : "1"); + } catch { + /* a rejected write must not take the toggle down with it */ + } + switch (optAction(enabled, ready)) { + case "opt-out": + posthog.opt_out_capturing(); // also drops whatever is still queued + break; + case "opt-in": + posthog.opt_in_capturing(); + break; + case "init": + initAnalytics(); // first opt-in of a session that started opted out + break; + case "none": + break; + } +} + export function initAnalytics() { - if (ready) return; + if (ready || !analyticsEnabled()) return; posthog.init(TOKEN, { api_host: "https://us.i.posthog.com", autocapture: false, // never capture clicked-element text (conversation leak) @@ -33,12 +80,16 @@ export function initAnalytics() { } export function track(event: string, props?: Record) { - if (!ready) return; + if (!ready || !analyticsEnabled()) return; posthog.capture(event, props); } +// Checked here as well as in track(): this is the one call that would send a +// personal identifier, so it must not depend on opt_out_capturing() alone. +// The address is still stored locally in the profile either way — opting out +// stops it from being reported, not from being used. export function identifyEmail(email: string) { - if (!ready) return; + if (!ready || !analyticsEnabled()) return; posthog.identify(email, { email }); posthog.capture("email_submitted"); } From 8fec1c4ab890b1542706adfebb6e928f7e2c1c61 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=B6khan=20K=C3=B6se?= Date: Tue, 18 Aug 2026 22:39:39 +0200 Subject: [PATCH 03/52] chore: measure the palette against WCAG AA MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pnpm check:contrast` parses src/styles.css and measures 21 pairs. It reads the stylesheet rather than keeping a second copy of the values, so it cannot pass against a palette that is no longer the shipped one. Two things it does that reading the hex values does not: - It composites alpha. --color-ink-secondary is #fcfcfc99, and measuring it as opaque overstates every secondary-text pair in the app by a wide margin (2.86:1 vs the 15:1 the raw value suggests on --color-app). - It measures white on filled surfaces, because that is what the components render — `bg-accent … text-white` — rather than the token against the page. Three pairs sit below AA today. They are listed in KNOWN with their call sites, so this lands without changing a colour in the same commit and fails only on something new: white on accent 3.65:1 every primary button, 12-13px (28 sites) white on danger 3.10:1 the hang-up buttons, 14px accent on card 4.15:1 accent links inside a Card (11.5-12px) The shape matters more than the numbers: --color-accent is fine as text on the page ground (5.33:1 on --color-app) and only falls short on the lighter card, while white falls short ON the accent. Darkening the token fixes the buttons and hurts the links, so the fix is a separate fill colour rather than a nudge. That is a design call and yours to make — this only measures, and deleting a line from KNOWN is how a fix gets locked in. Verified both ways: exit 0 as shipped; drop --color-ink-secondary to #fcfcfc55 and it exits 1 naming all five surfaces. --- package.json | 1 + scripts/check-contrast.mjs | 164 +++++++++++++++++++++++++++++++++++++ 2 files changed, 165 insertions(+) create mode 100644 scripts/check-contrast.mjs diff --git a/package.json b/package.json index 21b05dbcd..8cffcb43d 100644 --- a/package.json +++ b/package.json @@ -23,6 +23,7 @@ "scripts": { "clean": "node scripts/clean.mjs", "lint": "oxlint .", + "check:contrast": "node scripts/check-contrast.mjs", "dev": "vite", "companion": "node --experimental-strip-types companion/src/index.ts", "dev:server": "node --experimental-strip-types server/index.ts", diff --git a/scripts/check-contrast.mjs b/scripts/check-contrast.mjs new file mode 100644 index 000000000..b3afac4b8 --- /dev/null +++ b/scripts/check-contrast.mjs @@ -0,0 +1,164 @@ +#!/usr/bin/env node +// Measures the palette in src/styles.css against WCAG 2.1 AA. +// +// node scripts/check-contrast.mjs (or: pnpm check:contrast) +// +// It parses the stylesheet instead of keeping a second copy of the values, so +// the check can never pass against a palette that is no longer the shipped +// one. Two things it does that a quick eyeball does not: +// +// - composites alpha. --color-ink-secondary is #fcfcfc99, and measuring it +// as opaque #fcfcfc overstates every secondary-text pair in the app. +// - measures white on filled surfaces, which is what the components +// actually render (`bg-accent … text-white`), not the token against the +// page ground. +// +// The two pairs already below AA are listed in KNOWN below, so adopting this +// check does not force a palette change in the same commit. Anything new +// fails the run. +import { readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; + +const root = join(dirname(fileURLToPath(import.meta.url)), ".."); +const css = readFileSync(join(root, "src/styles.css"), "utf8"); + +/** Custom properties from @theme and :root, in cascade order. */ +function parseTokens(source) { + const tokens = {}; + for (const [, body] of source.matchAll(/(?:@theme|:root)[^{]*\{([^}]*)\}/g)) { + for (const [, name, value] of body.matchAll(/(--color-[\w-]+)\s*:\s*([^;]+);/g)) { + tokens[name] = value.trim(); + } + } + return tokens; +} + +/** #rgb, #rrggbb and #rrggbbaa → {r,g,b,a} with channels in 0..1. */ +function parseColor(value) { + const h = value.replace("#", "").trim(); + const full = h.length === 3 || h.length === 4 ? [...h].map((c) => c + c).join("") : h; + if (!/^[0-9a-fA-F]{6}([0-9a-fA-F]{2})?$/.test(full)) return null; + const n = (i) => parseInt(full.slice(i, i + 2), 16) / 255; + return { r: n(0), g: n(2), b: n(4), a: full.length === 8 ? n(6) : 1 }; +} + +/** Lay a possibly-translucent colour over an opaque one. */ +function composite(fg, bg) { + if (fg.a === 1) return fg; + const mix = (f, b) => f * fg.a + b * (1 - fg.a); + return { r: mix(fg.r, bg.r), g: mix(fg.g, bg.g), b: mix(fg.b, bg.b), a: 1 }; +} + +function luminance({ r, g, b }) { + const lin = (c) => (c <= 0.03928 ? c / 12.92 : ((c + 0.055) / 1.055) ** 2.4); + return 0.2126 * lin(r) + 0.7152 * lin(g) + 0.0722 * lin(b); +} + +function contrast(fgValue, bgValue) { + const bg = parseColor(bgValue); + const fg = parseColor(fgValue); + if (!fg || !bg) return null; + if (bg.a !== 1) return null; // a translucent ground has no single answer + const [hi, lo] = [luminance(composite(fg, bg)), luminance(bg)].sort((a, b) => b - a); + return (hi + 0.05) / (lo + 0.05); +} + +const SURFACES = [ + "--color-app", + "--color-panel", + "--color-raised", + "--color-card", + "--color-inset", +]; + +// AA asks 4.5:1 of body text and 3:1 of non-text indicators (1.4.11). +const PAIRS = [ + // body and secondary text on every ground a panel can sit on + ...SURFACES.map((s) => ["--color-ink", s, 4.5, "body text"]), + ...SURFACES.map((s) => ["--color-ink-secondary", s, 4.5, "secondary text (60% alpha)"]), + ["--color-ink", "--color-bubble-user", 4.5, "your own messages"], + // what the filled buttons actually render: white on a solid accent/danger + ["#ffffff", "--color-accent", 4.5, "primary buttons — bg-accent + text-white"], + ["#ffffff", "--color-danger", 4.5, "destructive buttons — bg-danger + text-white"], + // coloured text on a ground + ["--color-accent", "--color-app", 4.5, "accent as text/links"], + ["--color-accent", "--color-card", 4.5, "accent as text on a card"], + ["--color-danger", "--color-card", 4.5, "error text"], + ["--color-success", "--color-card", 4.5, "success text"], + ["--color-warning", "--color-card", 4.5, "warning text"], + // indicators: outline, not glyphs + ["--color-focus", "--color-app", 3, "focus ring"], + ["--color-focus", "--color-panel", 3, "focus ring on a panel"], + ["--color-accent-border", "--color-card", 3, "accent border"], +]; + +// Below AA on the current palette. Listed so this check can be adopted +// without changing a colour in the same commit — and so that fixing one is a +// visible deletion here rather than a silent pass. Where each one shows up: +// +// white on accent 3.65:1 every primary button, 12–13px — Composer send, +// EngineSetup install, ComputerPanel start, the +// Onboarding and Routines actions (28 sites) +// white on danger 3.10:1 CallView.tsx:532 / GroupCallView.tsx:492, the +// hang-up buttons, 14px +// accent on card 4.15:1 accent links inside a Card — ApiKeys.tsx:121 +// (12px), EnginesSettings.tsx:241 (11.5px) +// +// Note the shape of the problem before changing anything: --color-accent +// reads fine as text on the page ground (5.33:1 on --color-app) and only +// falls short on the lighter card, while white falls short ON the accent. +// Darkening the one token fixes the buttons and hurts the links, so the fix +// is a separate fill colour, not a nudge — a design call, which is why this +// check only measures. +const KNOWN = new Set([ + "#ffffff on --color-accent", + "#ffffff on --color-danger", + "--color-accent on --color-card", +]); + +const tokens = parseTokens(css); +const resolve = (name) => (name.startsWith("--") ? tokens[name] : name); + +let failed = false; +const carried = []; +let measured = 0; + +for (const [fg, bg, min, where] of PAIRS) { + const fgValue = resolve(fg); + const bgValue = resolve(bg); + if (!fgValue || !bgValue) { + // An unmeasurable pair is reported, never skipped: silently passing over + // a renamed token is how a check quietly stops checking. + console.log(`✗ undefined token in pair: ${fg} on ${bg}`); + failed = true; + continue; + } + const ratio = contrast(fgValue, bgValue); + if (ratio === null) { + console.log(`✗ cannot measure ${fg} on ${bg} (${fgValue} on ${bgValue})`); + failed = true; + continue; + } + measured++; + if (ratio >= min) continue; + + const line = `${fg} on ${bg}: ${ratio.toFixed(2)}:1 (needs ${min}:1) — ${where}`; + if (KNOWN.has(`${fg} on ${bg}`)) { + carried.push(line); + } else { + console.log(`✗ ${line}`); + failed = true; + } +} + +if (carried.length) { + console.log("Known, carried (listed in KNOWN):"); + for (const line of carried) console.log(` ~ ${line}`); +} +console.log( + failed + ? `\n${measured} pairs measured — new contrast failures above.` + : `\n✓ ${measured} pairs measured, no new failures.`, +); +process.exit(failed ? 1 : 0); From 22761aa5473fa08ec5283cd575930a99a1887d74 Mon Sep 17 00:00:00 2001 From: aivsomkar Date: Wed, 19 Aug 2026 17:00:45 +0530 Subject: [PATCH 04/52] Fold the chat header chips to icon bubbles when the column is narrow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the Computer or Inspector panel open the chat column drops to ~700px and the header row — Stop, + Task, usage, working folder, model, icons — wrapped onto three lines and crushed the bot avatar and name to nothing. The header is now a CSS container (@container/chathead); below 4xl each chip folds to an icon-only shape and the right group stops shrinking so the name truncates instead: - Stop → round bubble with the square - + Task → round bubble with the plus (count-only bubble once there are several tasks) - usage → one short figure: cost when known, else tokens - working folder → rounded square with the folder icon - model → rounded square with the provider mark Full labels still ride the tooltips. Wide headers are unchanged. Co-Authored-By: Claude Fable 5 --- src/components/ChatView.tsx | 30 +++++++++++++++++++++--------- src/components/ModelPicker.tsx | 18 +++++++++++++++--- src/components/TaskPicker.tsx | 21 +++++++++++++++------ src/lib/compact-chip.ts | 14 ++++++++++++++ 4 files changed, 65 insertions(+), 18 deletions(-) create mode 100644 src/lib/compact-chip.ts diff --git a/src/components/ChatView.tsx b/src/components/ChatView.tsx index 7e16ccad8..48cef498e 100644 --- a/src/components/ChatView.tsx +++ b/src/components/ChatView.tsx @@ -47,6 +47,7 @@ import { ReactionBar, ReactionChips } from "./Reactions"; import { SpeakButton } from "./SpeakButton"; import { CallButton, CallOverlay } from "./CallView"; import { cn } from "@/lib/cn"; +import { COMPACT_BUBBLE, COMPACT_SQUARE } from "@/lib/compact-chip"; import { useFocusMessage } from "@/lib/focus-message"; import { webhookMessageView } from "@/lib/webhook-message"; import { BOTTOM_FOLLOW_THRESHOLD, shouldResumeBottomFollow } from "@/lib/bottom-follow"; @@ -803,7 +804,9 @@ export function ChatView({ bot }: { bot: Bot }) { {/* Header */}
}
-
+
{bot.busy && ( )} @@ -1022,13 +1028,16 @@ function UsageChip({ bot }: { bot: Bot }) { ] .filter(Boolean) .join("\n"); + // folded: one figure — cost when the engine reports one, else tokens + const short = usage.costUsd !== null ? formatUsd(usage.costUsd) : formatTokens(usage.input + usage.output); return ( ); } @@ -1045,11 +1054,14 @@ function WorkingFolderChip({ bot }: { bot: Bot }) { return ( ); } diff --git a/src/components/ModelPicker.tsx b/src/components/ModelPicker.tsx index 535d27c0a..f98047d7a 100644 --- a/src/components/ModelPicker.tsx +++ b/src/components/ModelPicker.tsx @@ -10,6 +10,7 @@ import { ProviderMark } from "./ProviderIcons"; import { EngineSetup, needsCli, needsSignIn } from "./EngineSetup"; import { EngineGroupLabel } from "./EngineGroupLabel"; import { cn } from "@/lib/cn"; +import { COMPACT_SQUARE } from "@/lib/compact-chip"; type ModelOption = InstanceInfo["models"]["options"][number]; const COMPACT_MODEL_COUNT = 5; @@ -206,12 +207,23 @@ export function ModelPicker({ bot, className }: { bot: Bot; className?: string } }} aria-expanded={open} aria-haspopup="dialog" - className="flex items-center gap-1.5 rounded-full border border-hairline/40 bg-raised/60 py-1 pl-2 pr-2.5 text-[13px] text-ink hover:bg-raised" + className={cn( + "flex items-center gap-1.5 rounded-full border border-hairline/40 bg-raised/60 py-1 pl-2 pr-2.5 text-[13px] text-ink hover:bg-raised", + // in a narrow chat header fold to a rounded square with just the + // provider mark; the model name rides the tooltip (a bot with no + // resolved engine keeps its label — the mark is what would hide it) + active && COMPACT_SQUARE, + )} title={active ? `${active.displayName} · ${modelLabel(active, selection.model)}` : selection.model} > {active && } - {modelLabel(active, selection.model)} - + + {modelLabel(active, selection.model)} + + {open && ( diff --git a/src/components/TaskPicker.tsx b/src/components/TaskPicker.tsx index f6625211d..62afc6530 100644 --- a/src/components/TaskPicker.tsx +++ b/src/components/TaskPicker.tsx @@ -8,6 +8,7 @@ import { useEffect, useRef, useState } from "react"; import { Check, ChevronDown, Plus, Trash2 } from "lucide-react"; import { useStore, formatTime, type Bot, type Task } from "@/state/store"; import { cn } from "@/lib/cn"; +import { COMPACT_BUBBLE } from "@/lib/compact-chip"; import { formatTokens } from "@/lib/format-tokens"; /** Quiet per-task token tally — input+output combined, because one honest @@ -57,9 +58,13 @@ export function TaskPicker({ bot }: { bot: Bot }) { onClick={() => dispatch({ type: "newTask", botId: bot.id })} disabled={bot.busy} title={bot.busy ? "Let this turn finish first" : "New task — a fresh context on this bot"} - className="flex items-center gap-1 rounded-full border border-hairline/40 px-2.5 py-1 text-[12.5px] text-ink-secondary hover:bg-raised hover:text-ink disabled:opacity-40" + className={cn( + "flex items-center gap-1 rounded-full border border-hairline/40 px-2.5 py-1 text-[12.5px] text-ink-secondary hover:bg-raised hover:text-ink disabled:opacity-40", + COMPACT_BUBBLE, + )} > - Task + + Task ); } @@ -84,11 +89,15 @@ export function TaskPicker({ bot }: { bot: Bot }) { {open && ( diff --git a/src/lib/compact-chip.ts b/src/lib/compact-chip.ts new file mode 100644 index 000000000..b12ae4d1f --- /dev/null +++ b/src/lib/compact-chip.ts @@ -0,0 +1,14 @@ +// Chat-header chips fold to icon-only shapes when the header is narrow — +// the computer/inspector panel is open or the window is small — so the +// row stops wrapping and crushing the bot's name. The header is the +// `@container/chathead`; below 4xl (56rem) these variants kick in. +// +// Kept as plain literal strings so Tailwind's scanner sees every class. + +/** Round bubble, icon only — Stop, + Task. */ +export const COMPACT_BUBBLE = + "@max-4xl/chathead:size-[30px] @max-4xl/chathead:justify-center @max-4xl/chathead:gap-0 @max-4xl/chathead:p-0"; + +/** Rounded square, icon only — working folder, model. */ +export const COMPACT_SQUARE = + "@max-4xl/chathead:size-[30px] @max-4xl/chathead:justify-center @max-4xl/chathead:gap-0 @max-4xl/chathead:rounded-md @max-4xl/chathead:p-0"; From 7558ad7f1a09b13dd8192c57ec02410a3a738599 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=B6khan=20K=C3=B6se?= Date: Wed, 19 Aug 2026 14:15:28 +0200 Subject: [PATCH 05/52] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20com?= =?UTF-8?q?ment=20stripping,=20floors=20for=20known=20pairs,=20count?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - parseTokens strips CSS comments first. A declaration left in a comment reads exactly like a live one, so a removed-but-still-mentioned token was measured at its stale value instead of reported as undefined. - KNOWN is a Map of ratios rather than a Set of names. It was a licence, not a floor: white-on-accent could have slid from 3.65:1 to 2:1 and the run stayed green. A carried pair that gets worse now fails like anything else, with 0.01 of rounding headroom since the floors are quoted to two decimals. - A known pair that climbs past AA is announced so the line gets deleted, rather than sitting in KNOWN shielding a pair that no longer needs it. - The header said two pairs; there are three. Verified all three ways: accent → #0a5db3 fails with "WORSE than the recorded 4.15:1"; accent → #0d5aa8 prints "now measures 6.90:1 — remove it from KNOWN"; commenting out --color-warning fails with "undefined token" instead of quietly measuring the dead value. --- scripts/check-contrast.mjs | 60 +++++++++++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 13 deletions(-) diff --git a/scripts/check-contrast.mjs b/scripts/check-contrast.mjs index b3afac4b8..13e4b0c90 100644 --- a/scripts/check-contrast.mjs +++ b/scripts/check-contrast.mjs @@ -13,9 +13,10 @@ // actually render (`bg-accent … text-white`), not the token against the // page ground. // -// The two pairs already below AA are listed in KNOWN below, so adopting this -// check does not force a palette change in the same commit. Anything new -// fails the run. +// The three pairs already below AA are listed in KNOWN below with the ratio +// they measure today, so adopting this check does not force a palette change +// in the same commit. Anything new fails the run — and so does a known pair +// that gets WORSE than its recorded floor. import { readFileSync } from "node:fs"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; @@ -23,10 +24,15 @@ import { fileURLToPath } from "node:url"; const root = join(dirname(fileURLToPath(import.meta.url)), ".."); const css = readFileSync(join(root, "src/styles.css"), "utf8"); -/** Custom properties from @theme and :root, in cascade order. */ +/** Custom properties from @theme and :root, in cascade order. + * + * Comments are stripped first: a declaration left behind in a comment reads + * exactly like a live one, so a token that was removed but still mentioned + * would be measured at its stale value instead of reported as undefined. */ function parseTokens(source) { const tokens = {}; - for (const [, body] of source.matchAll(/(?:@theme|:root)[^{]*\{([^}]*)\}/g)) { + const live = source.replace(/\/\*[\s\S]*?\*\//g, ""); + for (const [, body] of live.matchAll(/(?:@theme|:root)[^{]*\{([^}]*)\}/g)) { for (const [, name, value] of body.matchAll(/(--color-[\w-]+)\s*:\s*([^;]+);/g)) { tokens[name] = value.trim(); } @@ -111,12 +117,21 @@ const PAIRS = [ // Darkening the one token fixes the buttons and hurts the links, so the fix // is a separate fill colour, not a nudge — a design call, which is why this // check only measures. -const KNOWN = new Set([ - "#ffffff on --color-accent", - "#ffffff on --color-danger", - "--color-accent on --color-card", +// Each entry records the ratio the pair measures TODAY, not just its name. +// A floor, not a licence: a carried pair that gets worse is a regression and +// fails like anything else. Improve one past AA and the run says so, so the +// line gets deleted rather than quietly protecting a pair that no longer +// needs it. +const KNOWN = new Map([ + ["#ffffff on --color-accent", 3.65], + ["#ffffff on --color-danger", 3.1], + ["--color-accent on --color-card", 4.15], ]); +// Rounding headroom: the floors above are quoted to two decimals, so a value +// that is unchanged can measure a hair under its own printed figure. +const DRIFT = 0.01; + const tokens = parseTokens(css); const resolve = (name) => (name.startsWith("--") ? tokens[name] : name); @@ -143,12 +158,31 @@ for (const [fg, bg, min, where] of PAIRS) { measured++; if (ratio >= min) continue; - const line = `${fg} on ${bg}: ${ratio.toFixed(2)}:1 (needs ${min}:1) — ${where}`; - if (KNOWN.has(`${fg} on ${bg}`)) { - carried.push(line); - } else { + const key = `${fg} on ${bg}`; + const line = `${key}: ${ratio.toFixed(2)}:1 (needs ${min}:1) — ${where}`; + const floor = KNOWN.get(key); + if (floor === undefined) { console.log(`✗ ${line}`); failed = true; + } else if (ratio < floor - DRIFT) { + console.log(`✗ ${line} — WORSE than the recorded ${floor.toFixed(2)}:1`); + failed = true; + } else { + carried.push(line); + } +} + +// A known pair that now clears AA never reaches the block above, so say it +// here — otherwise the entry sits in KNOWN forever, shielding a pair that no +// longer needs shielding. +for (const [key, floor] of KNOWN) { + const [fg, bg] = key.split(" on "); + const fgValue = resolve(fg); + const bgValue = resolve(bg); + if (!fgValue || !bgValue) continue; + const ratio = contrast(fgValue, bgValue); + if (ratio !== null && ratio >= 4.5) { + console.log(`✓ ${key} now measures ${ratio.toFixed(2)}:1 — remove it from KNOWN (floor was ${floor.toFixed(2)})`); } } From cdd18333132ad684f967a9ea3190e9b2aa17320d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=B6khan=20K=C3=B6se?= Date: Wed, 19 Aug 2026 14:18:20 +0200 Subject: [PATCH 06/52] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20hol?= =?UTF-8?q?d=20the=20choice=20in=20memory,=20make=20the=20guard=20test=20r?= =?UTF-8?q?eal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both findings were right. An opt-out was only as durable as the write that persisted it. With storage rejecting the write, the setter swallowed the error, the next read found nothing and answered "enabled", and a later initAnalytics() would start the client the user had just switched off — the one failure this feature exists to prevent. The choice now lives in module state, set before the write is attempted; storage is how it survives a restart, not where it lives. The cold-start test also proved nothing: an earlier test had already set the module-scoped `ready` flag, so initAnalytics() returned on that rather than on the opt-out. It now loads the module fresh — resetModules, not module mocking: nothing is replaced, the real module is simply loaded again. Verified by removing the `!analyticsEnabled()` guard from initAnalytics: the test fails. It did not before. 10 tests, full suite green. --- src/lib/analytics.test.ts | 47 +++++++++++++++++++++++++++++++++------ src/lib/analytics.ts | 13 +++++++++-- 2 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/lib/analytics.test.ts b/src/lib/analytics.test.ts index 05317d63e..e518e91d5 100644 --- a/src/lib/analytics.test.ts +++ b/src/lib/analytics.test.ts @@ -3,7 +3,7 @@ // storage round-trip pins that the choice survives a restart. import { beforeEach, describe, expect, it, vi } from "vitest"; -import { analyticsEnabled, initAnalytics, optAction, setAnalyticsEnabled } from "./analytics"; +import { analyticsEnabled, optAction, setAnalyticsEnabled } from "./analytics"; // The suite runs on the node environment, which has no localStorage. const store = new Map(); @@ -51,6 +51,30 @@ describe("the stored choice", () => { expect(analyticsEnabled()).toBe(true); }); + it("holds an opt-out for the session even when the write is rejected", async () => { + // The failure this guards: the setter swallows the write error, the next + // read finds nothing and answers "enabled", and a later initAnalytics() + // starts the client the user just switched off. + vi.resetModules(); + const fresh = await import("./analytics"); + vi.stubGlobal("localStorage", { + getItem: () => null, + setItem: () => { + throw new Error("quota exceeded"); + }, + }); + + fresh.setAnalyticsEnabled(false); + expect(fresh.analyticsEnabled()).toBe(false); + fresh.initAnalytics(); + expect(store.get("omb-installed")).toBeUndefined(); + + vi.stubGlobal("localStorage", { + getItem: (k: string) => store.get(k) ?? null, + setItem: (k: string, v: string) => void store.set(k, v), + }); + }); + it("treats unusable storage as a fresh install rather than failing", () => { vi.stubGlobal("localStorage", { getItem: () => { @@ -70,12 +94,21 @@ describe("the stored choice", () => { }); describe("initAnalytics while opted out", () => { - it("returns before touching the client or the install marker", () => { - // No client is stubbed here on purpose: if init() got past the guard it - // would reach the real posthog-js, and the missing marker proves it did - // not — opting back in later still counts the install. - setAnalyticsEnabled(false); - initAnalytics(); + it("returns before touching the client or the install marker", async () => { + // A fresh module, so the module-scoped `ready` flag starts false: with a + // used module this test passes on `ready` alone and proves nothing about + // the opt-out. resetModules is not module mocking — nothing is replaced, + // the real module is simply loaded again. + vi.resetModules(); + store.set("omb-analytics-opt-out", "1"); // as a previous session left it + const fresh = await import("./analytics"); + + expect(fresh.analyticsEnabled()).toBe(false); + fresh.initAnalytics(); + + // No client is stubbed on purpose: if init() got past the guard it would + // reach the real posthog-js and set this marker. Its absence is the + // proof — and it also means opting back in later still counts the install. expect(store.get("omb-installed")).toBeUndefined(); }); }); diff --git a/src/lib/analytics.ts b/src/lib/analytics.ts index e55d753bf..75c3ef413 100644 --- a/src/lib/analytics.ts +++ b/src/lib/analytics.ts @@ -18,12 +18,20 @@ const OPT_OUT_KEY = "omb-analytics-opt-out"; let ready = false; +// The choice as made in THIS process, which outranks storage. Without it a +// rejected write silently loses an opt-out: the setter would swallow the +// error, the next analyticsEnabled() would read nothing and answer true, and +// a later initAnalytics() would start the client the user just switched off. +// Storage is how the choice survives a restart, not where it lives. +let choice: boolean | undefined; + /** False once the user has opted out on this machine. */ export function analyticsEnabled(): boolean { + if (choice !== undefined) return choice; try { return localStorage.getItem(OPT_OUT_KEY) !== "1"; } catch { - return true; // storage unavailable → behave like a fresh install + return true; // storage unreadable → behave like a fresh install } } @@ -38,10 +46,11 @@ export function optAction(enabled: boolean, running: boolean): OptAction { /** Flip the setting and act on it immediately, in both directions. */ export function setAnalyticsEnabled(enabled: boolean) { + choice = enabled; // before persisting: the decision must not depend on it try { localStorage.setItem(OPT_OUT_KEY, enabled ? "0" : "1"); } catch { - /* a rejected write must not take the toggle down with it */ + /* it will not survive a restart, but it holds for this session */ } switch (optAction(enabled, ready)) { case "opt-out": From 5b3077fb085597f1a07e47578ff3cbde32d0b0f4 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 18 Aug 2026 10:47:01 +0200 Subject: [PATCH 07/52] Let Kimi run a local model without a Kimi login. Kimi ACP session/new checks default_model, not -m. A missing or expired login then becomes "Authentication required" even when the picker is a local host. Overlay Kimi's official KIMI_MODEL_* env on inject turns so the child has an in-memory default, and write protocol plus max_context_size on the on-disk alias so 0.36+ will bind it. --- server/drivers/acp/acp.test.ts | 30 ++++++++++++ server/drivers/acp/core.ts | 7 +++ server/drivers/acp/kimi.ts | 52 +++++++++++++++++++- server/drivers/local-inject.test.ts | 74 ++++++++++++++++++++++++++++- server/testing/fake-acp-cli.ts | 6 +++ 5 files changed, 167 insertions(+), 2 deletions(-) diff --git a/server/drivers/acp/acp.test.ts b/server/drivers/acp/acp.test.ts index ff3dabfb8..c5330177c 100644 --- a/server/drivers/acp/acp.test.ts +++ b/server/drivers/acp/acp.test.ts @@ -562,6 +562,36 @@ describe("ACP turns (fake CLI)", () => { expect(done).toMatchObject({ ok: true }); }); + it("applyTurnEnv sees the picker model after resolveTurnModel", async () => { + const dump = join(scratch, "turn-env.json"); + process.env.FAKE_ACP_DUMP = dump; + const TurnEnvDriver = createAcpDriver({ + ...SELECT_MODEL_SUPPORT, + driverKind: "turnEnvTest", + selectModel: undefined, + applyTurnEnv: (env, { requestedModel }) => { + env.TEST_TURN_MODEL = requestedModel ?? ""; + }, + }); + instance = await TurnEnvDriver.create({ + instanceId: "turn-env-test", + displayName: undefined, + environment: {}, + enabled: true, + config: { cli: FAKE_CLI, fullAuto: false }, + }); + recorder = recordEvents(instance.adapter); + + await instance.adapter.sendTurn({ + threadId: "t-turn-env", + text: "go", + model: "ollama::ornith:35b-bf16", + }); + await recorder.until((e) => e.type === "turn.completed"); + + expect(JSON.parse(readFileSync(dump, "utf8")).env.TEST_TURN_MODEL).toBe("ollama::ornith:35b-bf16"); + }); + it("transformEnv sees the instance config", async () => { const dump = join(scratch, "policy.json"); process.env.FAKE_ACP_DUMP = dump; diff --git a/server/drivers/acp/core.ts b/server/drivers/acp/core.ts index 52f5b0d1a..b175ec880 100644 --- a/server/drivers/acp/core.ts +++ b/server/drivers/acp/core.ts @@ -92,6 +92,12 @@ export interface AcpSupport { /** Mutate the child env in place: strip a key, inject a policy. Receives the * instance config so a support can vary with fullAuto. */ transformEnv?(env: Record, config: AcpConfig): void; + /** Mutate the child env after the turn model is known. Catalog refresh and + * snapshot share `transformEnv` and must not see a per-turn overlay. */ + applyTurnEnv?( + env: Record, + ctx: { model?: string; requestedModel?: string }, + ): void; /** Pick the ACP authenticate methodId from initialize's advertised * authMethods; return null to skip the authenticate step. */ pickAuthMethod(authMethods: Array<{ id?: string }>): string | null; @@ -278,6 +284,7 @@ export function createAcpDriver(support: AcpSupport): ProviderDriver const cwd = turn.cwd ?? config.workspace ?? homedir(); const env = childEnv(); const resolvedModel = support.resolveTurnModel?.(turn.model, env); + support.applyTurnEnv?.(env, { model: resolvedModel, requestedModel: turn.model }); const cliTurn = resolvedModel !== undefined && resolvedModel !== turn.model ? { ...turn, model: resolvedModel } diff --git a/server/drivers/acp/kimi.ts b/server/drivers/acp/kimi.ts index 28dd43450..3f3f505d6 100644 --- a/server/drivers/acp/kimi.ts +++ b/server/drivers/acp/kimi.ts @@ -89,7 +89,16 @@ export function ensureKimiInjectAlias( } if (!hasTomlTable(text, modelHeading)) { blocks.push( - [modelHeading, `provider = ${quoteToml(inject.host)}`, `model = ${quoteToml(inject.model)}`, ""].join("\n"), + [ + modelHeading, + `provider = ${quoteToml(inject.host)}`, + `model = ${quoteToml(inject.model)}`, + // Kimi 0.36+ refuses openai_legacy as a wire protocol; ACP then + // skips default-model binding and falls through to OAuth. + `protocol = "openai"`, + `max_context_size = 262144`, + "", + ].join("\n"), ); } if (blocks.length) { @@ -99,6 +108,41 @@ export function ensureKimiInjectAlias( return alias; } +/** Env keys Kimi 0.36+ reads to synthesize an in-memory default model. + * ACP `session/new` runs auth readiness against `default_model`, not `-m`. + * Without a default, a missing/expired `kimi login` becomes + * "Authentication required" even when the picker is a local host. */ +const KIMI_MODEL_ENV = [ + "KIMI_MODEL_NAME", + "KIMI_MODEL_API_KEY", + "KIMI_MODEL_BASE_URL", + "KIMI_MODEL_PROVIDER_TYPE", + "KIMI_MODEL_DISPLAY_NAME", +] as const; + +/** Overlay a local inject as Kimi's in-memory default. Does not write + * config.toml — Kimi strips these reserved entries on persist. */ +export function applyKimiLocalModelEnv( + env: Record, + modelId: string | undefined, +): void { + const inject = decodeInjectId(modelId); + if (!inject) return; + const host = localHost(inject.host); + if (!host) return; + env.KIMI_MODEL_NAME = inject.model; + env.KIMI_MODEL_API_KEY = hostApiKey(host, env); + env.KIMI_MODEL_BASE_URL = host.baseUrl; + // Env overlay accepts openai | anthropic | kimi — not the toml + // openai_legacy type we write for the on-disk provider row. + env.KIMI_MODEL_PROVIDER_TYPE = "openai"; + env.KIMI_MODEL_DISPLAY_NAME = `${inject.model} (${host.label})`; +} + +function stripKimiModelEnv(env: Record): void { + for (const key of KIMI_MODEL_ENV) delete env[key]; +} + function readKimiModelCatalog(env: Record): ModelCatalog { const dataRoot = kimiDataRoot(env); let text = ""; @@ -180,6 +224,12 @@ const support: AcpSupport = { transformEnv: (env) => { delete env.MOONSHOT_API_KEY; delete env.KIMI_API_KEY; + // A leftover shell overlay would steal every Kimi turn, including + // subscription models. applyTurnEnv puts the inject back per turn. + stripKimiModelEnv(env); + }, + applyTurnEnv: (env, { requestedModel }) => { + applyKimiLocalModelEnv(env, requestedModel); }, // The only advertised authMethod is {id:"login", type:"terminal"} — a diff --git a/server/drivers/local-inject.test.ts b/server/drivers/local-inject.test.ts index 34f456d2a..bfe473f6d 100644 --- a/server/drivers/local-inject.test.ts +++ b/server/drivers/local-inject.test.ts @@ -6,7 +6,7 @@ import { afterEach, describe, expect, it } from "vitest"; import { DroidAgentDriver, ensureDroidInjectModel } from "./acp/droid.ts"; import { ensureGrokInjectSlug, GrokAgentDriver } from "./acp/grok.ts"; -import { ensureKimiInjectAlias, KimiAgentDriver } from "./acp/kimi.ts"; +import { applyKimiLocalModelEnv, ensureKimiInjectAlias, KimiAgentDriver } from "./acp/kimi.ts"; import { ensureOpenCodeInjectModel } from "./acp/opencode-go.ts"; import { AntigravityDriver } from "./antigravity.ts"; @@ -303,6 +303,8 @@ describe("ensureKimiInjectAlias", () => { expect(text.match(/\[providers\.omlx\]/g)?.length).toBe(1); expect(text).toContain(`base_url = "http://127.0.0.1:8080/v1"`); expect(text).toContain(`model = "GLM-5.2-fp8"`); + expect(text).toContain(`protocol = "openai"`); + expect(text).toContain(`max_context_size = 262144`); }); it("treats USERPROFILE as the same home for credentials and config", async () => { @@ -329,6 +331,76 @@ describe("ensureKimiInjectAlias", () => { }); }); +describe("applyKimiLocalModelEnv", () => { + it("overlays an OpenAI-compatible default for a local inject pick", () => { + const env: Record = {}; + applyKimiLocalModelEnv(env, "ollama::ornith:35b-bf16"); + expect(env).toMatchObject({ + KIMI_MODEL_NAME: "ornith:35b-bf16", + KIMI_MODEL_API_KEY: "ollama", + KIMI_MODEL_BASE_URL: "http://127.0.0.1:11434/v1", + KIMI_MODEL_PROVIDER_TYPE: "openai", + }); + }); + + it("leaves subscription slugs and already-resolved aliases alone", () => { + const env: Record = { KIMI_MODEL_NAME: "keep-me" }; + applyKimiLocalModelEnv(env, "kimi-code/k3"); + applyKimiLocalModelEnv(env, "ollama/ornith:35b-bf16"); + applyKimiLocalModelEnv(env, undefined); + expect(env.KIMI_MODEL_NAME).toBe("keep-me"); + expect(env.KIMI_MODEL_API_KEY).toBeUndefined(); + }); + + it("reads the Unsloth token from the turn env", () => { + const env: Record = { UNSLOTH_STUDIO_AUTH_TOKEN: "unsloth-secret" }; + applyKimiLocalModelEnv(env, "unsloth::qwen3-coder"); + expect(env.KIMI_MODEL_API_KEY).toBe("unsloth-secret"); + expect(env.KIMI_MODEL_BASE_URL).toBe("http://127.0.0.1:8888/v1"); + }); + + it("puts the overlay on the Kimi child only for a local inject pick", async () => { + const home = mkdtempSync(join(tmpdir(), "omb-kimi-overlay-")); + scratchDirs.push(home); + mkdirSync(join(home, ".kimi-code"), { recursive: true }); + const dump = join(home, "dump.json"); + const instance = await KimiAgentDriver.create({ + instanceId: "kimi-overlay", + displayName: "Kimi", + environment: { HOME: home, FAKE_ACP_DUMP: dump, KIMI_MODEL_NAME: "from-shell" }, + enabled: true, + config: { cli: FAKE_ACP, fullAuto: false }, + }); + const recorder = recordEvents(instance.adapter); + try { + await instance.adapter.sendTurn({ + threadId: "t-inject", + text: "hi", + model: "ollama::ornith:35b-bf16", + }); + await recorder.until((e) => e.type === "turn.completed"); + const injectDump = JSON.parse(readFileSync(dump, "utf8")) as { env: Record }; + expect(injectDump.env).toMatchObject({ + KIMI_MODEL_NAME: "ornith:35b-bf16", + KIMI_MODEL_API_KEY: "ollama", + KIMI_MODEL_BASE_URL: "http://127.0.0.1:11434/v1", + KIMI_MODEL_PROVIDER_TYPE: "openai", + }); + + await instance.adapter.sendTurn({ + threadId: "t-cloud", + text: "hi", + model: "kimi-code/k3", + }); + await recorder.until((e) => e.type === "turn.completed" && e.threadId === "t-cloud"); + const cloudDump = JSON.parse(readFileSync(dump, "utf8")) as { env: Record }; + expect(cloudDump.env.KIMI_MODEL_NAME).toBeUndefined(); + } finally { + await instance.dispose(); + } + }); +}); + describe("ensureDroidInjectModel", () => { it("upserts a generic-chat-completion BYOK row and reuses it", () => { const home = mkdtempSync(join(tmpdir(), "omb-droid-inject-")); diff --git a/server/testing/fake-acp-cli.ts b/server/testing/fake-acp-cli.ts index d2bd1e958..acd38a05d 100755 --- a/server/testing/fake-acp-cli.ts +++ b/server/testing/fake-acp-cli.ts @@ -74,6 +74,12 @@ const dumpEnv = Object.fromEntries( "UNSLOTH_STUDIO_AUTH_TOKEN", "CURSOR_API_KEY", "CURSOR_AUTH_TOKEN", + "KIMI_MODEL_NAME", + "KIMI_MODEL_API_KEY", + "KIMI_MODEL_BASE_URL", + "KIMI_MODEL_PROVIDER_TYPE", + "KIMI_MODEL_DISPLAY_NAME", + "TEST_TURN_MODEL", ].flatMap((key) => (process.env[key] === undefined ? [] : [[key, process.env[key]]] as const)), ); const dumpState: Record = { argv, env: dumpEnv }; From fc111efbb0083306ea5cd5d535600fd74a114d9d Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 18 Aug 2026 10:59:09 +0200 Subject: [PATCH 08/52] Patch existing Kimi aliases with protocol and context size. Aliases written before this PR were left as-is, so Kimi 0.36+ skipped default-model binding. Fill in protocol and max_context_size when they are missing, and leave any values the user already set. The applyTurnEnv test now checks both the resolved model and the picker id. --- server/drivers/acp/acp.test.ts | 9 +++-- server/drivers/acp/kimi.ts | 37 ++++++++++++++++-- server/drivers/local-inject.test.ts | 58 +++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 6 deletions(-) diff --git a/server/drivers/acp/acp.test.ts b/server/drivers/acp/acp.test.ts index c5330177c..ca3526597 100644 --- a/server/drivers/acp/acp.test.ts +++ b/server/drivers/acp/acp.test.ts @@ -569,8 +569,9 @@ describe("ACP turns (fake CLI)", () => { ...SELECT_MODEL_SUPPORT, driverKind: "turnEnvTest", selectModel: undefined, - applyTurnEnv: (env, { requestedModel }) => { - env.TEST_TURN_MODEL = requestedModel ?? ""; + resolveTurnModel: (model) => (model ? `resolved/${model}` : model), + applyTurnEnv: (env, { model, requestedModel }) => { + env.TEST_TURN_MODEL = `${model ?? ""}|${requestedModel ?? ""}`; }, }); instance = await TurnEnvDriver.create({ @@ -589,7 +590,9 @@ describe("ACP turns (fake CLI)", () => { }); await recorder.until((e) => e.type === "turn.completed"); - expect(JSON.parse(readFileSync(dump, "utf8")).env.TEST_TURN_MODEL).toBe("ollama::ornith:35b-bf16"); + expect(JSON.parse(readFileSync(dump, "utf8")).env.TEST_TURN_MODEL).toBe( + "resolved/ollama::ornith:35b-bf16|ollama::ornith:35b-bf16", + ); }); it("transformEnv sees the instance config", async () => { diff --git a/server/drivers/acp/kimi.ts b/server/drivers/acp/kimi.ts index 3f3f505d6..9282da289 100644 --- a/server/drivers/acp/kimi.ts +++ b/server/drivers/acp/kimi.ts @@ -52,6 +52,30 @@ function hasTomlTable(text: string, heading: string): boolean { return text.split(/\r?\n/).some((line) => line.trim() === heading); } +function tomlTableHasKey(block: string, key: string): boolean { + return block.split(/\r?\n/).some((line) => { + const stripped = line.trim(); + if (!stripped || stripped.startsWith("#")) return false; + const eq = stripped.indexOf("="); + return eq > 0 && stripped.slice(0, eq).trim() === key; + }); +} + +/** Insert missing keys into an existing table. Does not overwrite set values. */ +function patchTomlTable(text: string, heading: string, rows: string[]): string { + const lines = text.split(/\r?\n/); + const start = lines.findIndex((line) => line.trim() === heading); + if (start < 0) return text; + let end = start + 1; + while (end < lines.length && !/^\s*\[/.test(lines[end]!)) end++; + const block = lines.slice(start, end).join("\n"); + const missing = rows.filter((row) => !tomlTableHasKey(block, row.split("=")[0]!.trim())); + if (!missing.length) return text; + let insertAt = end; + while (insertAt > start + 1 && lines[insertAt - 1] === "") insertAt--; + return [...lines.slice(0, insertAt), ...missing, ...lines.slice(insertAt)].join("\n"); +} + /** Write [providers.host] + [models."host/alias"] so `kimi -m` hits the local host. */ export function ensureKimiInjectAlias( modelId: string, @@ -72,6 +96,7 @@ export function ensureKimiInjectAlias( } catch { text = ""; } + const original = text; const providerHeading = `[providers.${inject.host}]`; const modelHeading = `[models.${quoteTomlKey(alias)}]`; @@ -87,14 +112,18 @@ export function ensureKimiInjectAlias( ].join("\n"), ); } - if (!hasTomlTable(text, modelHeading)) { + // Kimi 0.36+ refuses openai_legacy as a wire protocol; ACP then + // skips default-model binding and falls through to OAuth. Patch + // aliases written before those keys existed; do not overwrite a + // user's protocol or context size. + if (hasTomlTable(text, modelHeading)) { + text = patchTomlTable(text, modelHeading, [`protocol = "openai"`, `max_context_size = 262144`]); + } else { blocks.push( [ modelHeading, `provider = ${quoteToml(inject.host)}`, `model = ${quoteToml(inject.model)}`, - // Kimi 0.36+ refuses openai_legacy as a wire protocol; ACP then - // skips default-model binding and falls through to OAuth. `protocol = "openai"`, `max_context_size = 262144`, "", @@ -104,6 +133,8 @@ export function ensureKimiInjectAlias( if (blocks.length) { const prefix = text && !text.endsWith("\n") ? `${text}\n\n` : text ? `${text}\n` : ""; writeFileSync(path, `${prefix}${blocks.join("\n")}`); + } else if (text !== original) { + writeFileSync(path, text); } return alias; } diff --git a/server/drivers/local-inject.test.ts b/server/drivers/local-inject.test.ts index bfe473f6d..84f524ef3 100644 --- a/server/drivers/local-inject.test.ts +++ b/server/drivers/local-inject.test.ts @@ -307,6 +307,64 @@ describe("ensureKimiInjectAlias", () => { expect(text).toContain(`max_context_size = 262144`); }); + it("amends an existing alias with protocol and context size and leaves user keys", () => { + const home = mkdtempSync(join(tmpdir(), "omb-kimi-patch-")); + scratchDirs.push(home); + const root = join(home, ".kimi-code"); + mkdirSync(root, { recursive: true }); + writeFileSync( + join(root, "config.toml"), + [ + "[[hooks]]", + 'event = "Stop"', + "", + "[providers.omlx]", + 'type = "openai_legacy"', + 'base_url = "http://127.0.0.1:8080/v1"', + 'api_key = "omlx"', + "", + '[models."omlx/GLM-5.2-fp8"]', + 'provider = "omlx"', + 'model = "GLM-5.2-fp8"', + 'display_name = "keep me"', + "", + ].join("\n"), + ); + expect(ensureKimiInjectAlias("omlx::GLM-5.2-fp8", { HOME: home })).toBe("omlx/GLM-5.2-fp8"); + expect(ensureKimiInjectAlias("omlx::GLM-5.2-fp8", { HOME: home })).toBe("omlx/GLM-5.2-fp8"); + const text = readFileSync(join(root, "config.toml"), "utf8"); + expect(text).toContain("[[hooks]]"); + expect(text).toContain('display_name = "keep me"'); + expect(text).toContain('provider = "omlx"'); + expect(text).toContain('model = "GLM-5.2-fp8"'); + expect(text.match(/protocol = "openai"/g)?.length).toBe(1); + expect(text.match(/max_context_size = 262144/g)?.length).toBe(1); + }); + + it("does not overwrite a user's protocol or context size", () => { + const home = mkdtempSync(join(tmpdir(), "omb-kimi-keep-")); + scratchDirs.push(home); + const root = join(home, ".kimi-code"); + mkdirSync(root, { recursive: true }); + writeFileSync( + join(root, "config.toml"), + [ + '[models."omlx/GLM-5.2-fp8"]', + 'provider = "omlx"', + 'model = "GLM-5.2-fp8"', + 'protocol = "openai_responses"', + "max_context_size = 8192", + "", + ].join("\n"), + ); + ensureKimiInjectAlias("omlx::GLM-5.2-fp8", { HOME: home }); + const text = readFileSync(join(root, "config.toml"), "utf8"); + expect(text).toContain('protocol = "openai_responses"'); + expect(text).toContain("max_context_size = 8192"); + expect(text).not.toContain('protocol = "openai"'); + expect(text).not.toContain("max_context_size = 262144"); + }); + it("treats USERPROFILE as the same home for credentials and config", async () => { const home = mkdtempSync(join(tmpdir(), "omb-kimi-userprofile-")); scratchDirs.push(home); From e20c1ff96e947b53ab5a14ad83de1b5a26a7bded Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 18 Aug 2026 11:11:09 +0200 Subject: [PATCH 09/52] Parse Kimi config.toml with a string-aware scanner. Line-based heading and key checks missed quoted keys, headings with comments, and bracket lines inside multiline strings. Walk the file outside of strings so existing aliases are patched once, and document the helpers the coverage check was counting. --- server/drivers/acp/kimi.ts | 230 ++++++++++++++++++++++++++-- server/drivers/local-inject.test.ts | 67 ++++++++ 2 files changed, 280 insertions(+), 17 deletions(-) diff --git a/server/drivers/acp/kimi.ts b/server/drivers/acp/kimi.ts index 9282da289..be0971bb8 100644 --- a/server/drivers/acp/kimi.ts +++ b/server/drivers/acp/kimi.ts @@ -39,41 +39,236 @@ function credentialsPath(env: Record) { return join(kimiDataRoot(env), "credentials", "kimi-code.json"); } +/** Quote a TOML string value. */ function quoteToml(value: string): string { return `"${value.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`; } +/** Quote a TOML key when it is not a bare identifier. */ function quoteTomlKey(key: string): string { if (/^[A-Za-z0-9_-]+$/.test(key)) return key; return quoteToml(key); } +/** Strip a `#` comment that is not inside a quoted string. */ +function stripTomlLineComment(line: string): string { + let quote: '"' | "'" | null = null; + for (let i = 0; i < line.length; i++) { + const c = line[i]!; + if (quote) { + if (quote === '"' && c === "\\") { + i += 1; + continue; + } + if (c === quote) quote = null; + continue; + } + if (c === "#") return line.slice(0, i); + if (c === '"' || c === "'") quote = c; + } + return line; +} + +/** Canonical `a.b.c` form of a `[table]` heading, quotes and comments removed. */ +function canonicalizeTomlHeading(heading: string): string | null { + const trimmed = stripTomlLineComment(heading).trim(); + const match = trimmed.match(/^\[([^[\]]+)\]$/); + if (!match) return null; + const parts: string[] = []; + const inner = match[1]!; + let i = 0; + while (i < inner.length) { + if (inner[i] === ".") { + i += 1; + continue; + } + const q = inner[i]; + if (q === '"' || q === "'") { + i += 1; + let value = ""; + while (i < inner.length && inner[i] !== q) { + if (q === '"' && inner[i] === "\\") { + value += inner[i + 1] ?? ""; + i += 2; + continue; + } + value += inner[i]; + i += 1; + } + if (inner[i] === q) i += 1; + parts.push(value); + continue; + } + let value = ""; + while (i < inner.length && inner[i] !== ".") { + value += inner[i]; + i += 1; + } + parts.push(value); + } + return parts.join("."); +} + +/** Unwrap `"key"` / `'key'` so a quoted assignment matches the bare name. */ +function unquoteTomlKey(raw: string): string { + const key = raw.trim(); + if (key.length >= 2 && ((key.startsWith('"') && key.endsWith('"')) || (key.startsWith("'") && key.endsWith("'")))) { + return key.slice(1, -1); + } + return key; +} + +/** Bare key on the left of `key = value`. */ +function tomlRowKey(row: string): string { + const eq = row.indexOf("="); + return unquoteTomlKey(eq < 0 ? row : row.slice(0, eq)); +} + +/** Walk `text` and yield tables, skipping `[` inside strings (including multiline). */ +function tomlTables(text: string): Array<{ name: string; headingStart: number; bodyStart: number; end: number }> { + type Mode = "out" | "basic" | "literal" | "mlbasic" | "mllit"; + const headings: Array<{ name: string; lineStart: number; lineEnd: number }> = []; + let mode: Mode = "out"; + let i = 0; + const atLineStart = (idx: number) => idx === 0 || text[idx - 1] === "\n"; + while (i < text.length) { + if (mode === "mlbasic") { + if (text.startsWith('"""', i)) { + mode = "out"; + i += 3; + continue; + } + i += 1; + continue; + } + if (mode === "mllit") { + if (text.startsWith("'''", i)) { + mode = "out"; + i += 3; + continue; + } + i += 1; + continue; + } + if (mode === "basic") { + if (text[i] === "\\") { + i += 2; + continue; + } + if (text[i] === '"') mode = "out"; + i += 1; + continue; + } + if (mode === "literal") { + if (text[i] === "'") mode = "out"; + i += 1; + continue; + } + if (text.startsWith('"""', i)) { + mode = "mlbasic"; + i += 3; + continue; + } + if (text.startsWith("'''", i)) { + mode = "mllit"; + i += 3; + continue; + } + if (text[i] === '"') { + mode = "basic"; + i += 1; + continue; + } + if (text[i] === "'") { + mode = "literal"; + i += 1; + continue; + } + if (atLineStart(i)) { + let j = i; + while (j < text.length && (text[j] === " " || text[j] === "\t")) j += 1; + if (text[j] === "[") { + const nl = text.indexOf("\n", j); + const lineEnd = nl < 0 ? text.length : nl; + const name = canonicalizeTomlHeading(text.slice(j, lineEnd).replace(/\r$/, "")); + if (name) headings.push({ name, lineStart: i, lineEnd }); + i = lineEnd + (nl < 0 ? 0 : 1); + continue; + } + } + i += 1; + } + return headings.map((heading, index) => ({ + name: heading.name, + headingStart: heading.lineStart, + bodyStart: heading.lineEnd + (text[heading.lineEnd] === "\n" ? 1 : 0), + end: index + 1 < headings.length ? headings[index + 1]!.lineStart : text.length, + })); +} + +/** Keys assigned at line start in a table body, including `"quoted"` keys. */ +function tomlKeys(block: string): Set { + const keys = new Set(); + let lineStart = 0; + let mode: "out" | "mlbasic" | "mllit" = "out"; + const take = (end: number) => { + if (mode !== "out") return; + const line = stripTomlLineComment(block.slice(lineStart, end)); + const eq = line.indexOf("="); + if (eq > 0) keys.add(unquoteTomlKey(line.slice(0, eq))); + }; + for (let i = 0; i < block.length; i++) { + if (mode === "mlbasic") { + if (block.startsWith('"""', i)) { + mode = "out"; + i += 2; + } + } else if (mode === "mllit") { + if (block.startsWith("'''", i)) { + mode = "out"; + i += 2; + } + } else if (block.startsWith('"""', i)) { + mode = "mlbasic"; + i += 2; + } else if (block.startsWith("'''", i)) { + mode = "mllit"; + i += 2; + } else if (block[i] === "\n") { + take(i); + lineStart = i + 1; + } + } + take(block.length); + return keys; +} + +/** Whether `text` already has this table, ignoring quotes and trailing comments. */ function hasTomlTable(text: string, heading: string): boolean { - return text.split(/\r?\n/).some((line) => line.trim() === heading); + const name = canonicalizeTomlHeading(heading); + return name !== null && tomlTables(text).some((table) => table.name === name); } +/** Whether a table body already assigns `key` (`protocol` or `"protocol"`). */ function tomlTableHasKey(block: string, key: string): boolean { - return block.split(/\r?\n/).some((line) => { - const stripped = line.trim(); - if (!stripped || stripped.startsWith("#")) return false; - const eq = stripped.indexOf("="); - return eq > 0 && stripped.slice(0, eq).trim() === key; - }); + return tomlKeys(block).has(key); } /** Insert missing keys into an existing table. Does not overwrite set values. */ function patchTomlTable(text: string, heading: string, rows: string[]): string { - const lines = text.split(/\r?\n/); - const start = lines.findIndex((line) => line.trim() === heading); - if (start < 0) return text; - let end = start + 1; - while (end < lines.length && !/^\s*\[/.test(lines[end]!)) end++; - const block = lines.slice(start, end).join("\n"); - const missing = rows.filter((row) => !tomlTableHasKey(block, row.split("=")[0]!.trim())); + const name = canonicalizeTomlHeading(heading); + if (!name) return text; + const table = tomlTables(text).find((entry) => entry.name === name); + if (!table) return text; + const keys = tomlKeys(text.slice(table.bodyStart, table.end)); + const missing = rows.filter((row) => !keys.has(tomlRowKey(row))); if (!missing.length) return text; - let insertAt = end; - while (insertAt > start + 1 && lines[insertAt - 1] === "") insertAt--; - return [...lines.slice(0, insertAt), ...missing, ...lines.slice(insertAt)].join("\n"); + let insertAt = table.end; + while (insertAt > table.bodyStart && (text[insertAt - 1] === "\n" || text[insertAt - 1] === "\r")) insertAt -= 1; + const before = text.slice(0, insertAt); + const after = text.slice(insertAt); + const pad = before.endsWith("\n") || before.length === 0 ? "" : "\n"; + return `${before}${pad}${missing.join("\n")}${after.startsWith("\n") ? "" : "\n"}${after}`; } /** Write [providers.host] + [models."host/alias"] so `kimi -m` hits the local host. */ @@ -170,6 +365,7 @@ export function applyKimiLocalModelEnv( env.KIMI_MODEL_DISPLAY_NAME = `${inject.model} (${host.label})`; } +/** Drop leftover shell `KIMI_MODEL_*` so they cannot steal a cloud turn. */ function stripKimiModelEnv(env: Record): void { for (const key of KIMI_MODEL_ENV) delete env[key]; } diff --git a/server/drivers/local-inject.test.ts b/server/drivers/local-inject.test.ts index 84f524ef3..4e98477ef 100644 --- a/server/drivers/local-inject.test.ts +++ b/server/drivers/local-inject.test.ts @@ -365,6 +365,73 @@ describe("ensureKimiInjectAlias", () => { expect(text).not.toContain("max_context_size = 262144"); }); + it("treats a quoted protocol key as already set and does not duplicate it", () => { + const home = mkdtempSync(join(tmpdir(), "omb-kimi-quoted-")); + scratchDirs.push(home); + const root = join(home, ".kimi-code"); + mkdirSync(root, { recursive: true }); + writeFileSync( + join(root, "config.toml"), + [ + '[models."omlx/GLM-5.2-fp8"]', + 'provider = "omlx"', + 'model = "GLM-5.2-fp8"', + '"protocol" = "openai"', + "", + ].join("\n"), + ); + ensureKimiInjectAlias("omlx::GLM-5.2-fp8", { HOME: home }); + const text = readFileSync(join(root, "config.toml"), "utf8"); + expect(text.match(/protocol/g)?.length).toBe(1); + expect(text).toContain("max_context_size = 262144"); + }); + + it("finds a heading with a trailing comment and does not append a second table", () => { + const home = mkdtempSync(join(tmpdir(), "omb-kimi-heading-")); + scratchDirs.push(home); + const root = join(home, ".kimi-code"); + mkdirSync(root, { recursive: true }); + writeFileSync( + join(root, "config.toml"), + ['[models."omlx/GLM-5.2-fp8"] # keep', 'provider = "omlx"', 'model = "GLM-5.2-fp8"', ""].join("\n"), + ); + ensureKimiInjectAlias("omlx::GLM-5.2-fp8", { HOME: home }); + const text = readFileSync(join(root, "config.toml"), "utf8"); + expect(text.match(/\[models\./g)?.length).toBe(1); + expect(text).toContain("# keep"); + expect(text).toContain('protocol = "openai"'); + }); + + it("does not treat a bracket line inside a multiline string as a table", () => { + const home = mkdtempSync(join(tmpdir(), "omb-kimi-ml-")); + scratchDirs.push(home); + const root = join(home, ".kimi-code"); + mkdirSync(root, { recursive: true }); + writeFileSync( + join(root, "config.toml"), + [ + '[models."omlx/GLM-5.2-fp8"]', + 'provider = "omlx"', + 'model = "GLM-5.2-fp8"', + 'notes = """', + "[providers.evil]", + 'protocol = "skip"', + '"""', + "", + ].join("\n"), + ); + ensureKimiInjectAlias("omlx::GLM-5.2-fp8", { HOME: home }); + const text = readFileSync(join(root, "config.toml"), "utf8"); + expect(text).toContain('protocol = "skip"'); + expect(text).toContain('protocol = "openai"'); + const notesOpen = text.indexOf('"""', text.indexOf("notes")); + const notesClose = text.indexOf('"""', notesOpen + 3); + const protocolAt = text.indexOf('protocol = "openai"'); + expect(protocolAt).toBeGreaterThan(notesClose); + expect(text).toContain("[providers.omlx]"); + expect(text).toContain("[providers.evil]"); + }); + it("treats USERPROFILE as the same home for credentials and config", async () => { const home = mkdtempSync(join(tmpdir(), "omb-kimi-userprofile-")); scratchDirs.push(home); From ee79549aa516be4f87318b6501f53a3de199384d Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 18 Aug 2026 11:16:16 +0200 Subject: [PATCH 10/52] Let Droid run a local model without a Factory login. Droid ACP session/new requires a Factory login or FACTORY_API_KEY even when the picker is a BYOK custom host. The CLI only checks that the variable is set, then uses the custom row's own key. On a local inject turn, fill a placeholder if the user has no Factory key. Cloud models are unchanged. --- server/drivers/acp/droid.ts | 17 +++++++++ server/drivers/local-inject.test.ts | 58 ++++++++++++++++++++++++++++- server/testing/fake-acp-cli.ts | 1 + 3 files changed, 75 insertions(+), 1 deletion(-) diff --git a/server/drivers/acp/droid.ts b/server/drivers/acp/droid.ts index 79a0e9e81..cc0ad5b7e 100644 --- a/server/drivers/acp/droid.ts +++ b/server/drivers/acp/droid.ts @@ -118,6 +118,20 @@ export function ensureDroidInjectModel( return id; } +/** ACP `session/new` throws "Authentication required" unless a Factory + * login or FACTORY_API_KEY is present — even for a BYOK custom model. + * Droid 0.198 only checks that the env var is set, then uses the + * custom row's own key for the local host. Do not invent a key for + * subscription models, and do not overwrite a real Factory key. */ +export function applyDroidLocalAuthEnv( + env: Record, + modelId: string | undefined, +): void { + if (!decodeInjectId(modelId)) return; + if (env.FACTORY_API_KEY?.trim()) return; + env.FACTORY_API_KEY = "openmausbot-local"; +} + function readSettings(env: Record): FactorySettings { return JSON.parse(readFileSync(join(factoryHome(env), ".factory", "settings.json"), "utf8")) as FactorySettings; } @@ -233,6 +247,9 @@ const support: AcpSupport = { isAuthenticated: (env) => authFilePaths(env).some(existsSync) || Boolean(env.FACTORY_API_KEY), resolveModels, resolveTurnModel: (model, env) => (model ? ensureDroidInjectModel(model, env) : model), + applyTurnEnv: (env, { requestedModel }) => { + applyDroidLocalAuthEnv(env, requestedModel); + }, async configureSession({ request, sessionId, config, turn }) { const modeId = config.fullAuto ? MODE_FULL_AUTO : MODE_DEFAULT; diff --git a/server/drivers/local-inject.test.ts b/server/drivers/local-inject.test.ts index 4e98477ef..e7039b6e1 100644 --- a/server/drivers/local-inject.test.ts +++ b/server/drivers/local-inject.test.ts @@ -4,7 +4,7 @@ import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { afterEach, describe, expect, it } from "vitest"; -import { DroidAgentDriver, ensureDroidInjectModel } from "./acp/droid.ts"; +import { applyDroidLocalAuthEnv, DroidAgentDriver, ensureDroidInjectModel } from "./acp/droid.ts"; import { ensureGrokInjectSlug, GrokAgentDriver } from "./acp/grok.ts"; import { applyKimiLocalModelEnv, ensureKimiInjectAlias, KimiAgentDriver } from "./acp/kimi.ts"; import { ensureOpenCodeInjectModel } from "./acp/opencode-go.ts"; @@ -570,6 +570,62 @@ describe("ensureDroidInjectModel", () => { }); }); +describe("applyDroidLocalAuthEnv", () => { + it("fills a placeholder Factory key only for a local inject pick", () => { + const env: Record = {}; + applyDroidLocalAuthEnv(env, "ollama::ornith:35b-bf16"); + expect(env.FACTORY_API_KEY).toBe("openmausbot-local"); + applyDroidLocalAuthEnv(env, "ollama::ornith:35b-bf16"); + expect(env.FACTORY_API_KEY).toBe("openmausbot-local"); + }); + + it("leaves a real Factory key and cloud slugs alone", () => { + const kept: Record = { FACTORY_API_KEY: "fk-real" }; + applyDroidLocalAuthEnv(kept, "ollama::ornith:35b-bf16"); + expect(kept.FACTORY_API_KEY).toBe("fk-real"); + const cloud: Record = {}; + applyDroidLocalAuthEnv(cloud, "claude-opus-5"); + applyDroidLocalAuthEnv(cloud, undefined); + expect(cloud.FACTORY_API_KEY).toBeUndefined(); + }); + + it("puts the placeholder on the Droid child only for a local inject pick", async () => { + const home = mkdtempSync(join(tmpdir(), "omb-droid-overlay-")); + scratchDirs.push(home); + mkdirSync(join(home, ".factory"), { recursive: true }); + const dump = join(home, "dump.json"); + const instance = await DroidAgentDriver.create({ + instanceId: "droid-overlay", + displayName: "Droid", + environment: { HOME: home, FACTORY_HOME_OVERRIDE: home, FAKE_ACP_DUMP: dump, FACTORY_API_KEY: "" }, + enabled: true, + config: { cli: FAKE_ACP, fullAuto: false }, + }); + const recorder = recordEvents(instance.adapter); + try { + await instance.adapter.sendTurn({ + threadId: "t-inject", + text: "hi", + model: "ollama::ornith:35b-bf16", + }); + await recorder.until((e) => e.type === "turn.completed"); + expect(JSON.parse(readFileSync(dump, "utf8")).env.FACTORY_API_KEY).toBe("openmausbot-local"); + + await instance.adapter.sendTurn({ + threadId: "t-cloud", + text: "hi", + model: "claude-opus-5", + }); + await recorder.until((e) => e.type === "turn.completed" && e.threadId === "t-cloud"); + expect(JSON.parse(readFileSync(dump, "utf8")).env.FACTORY_API_KEY).not.toBe( + "openmausbot-local", + ); + } finally { + await instance.dispose(); + } + }); +}); + describe("ensureOpenCodeInjectModel", () => { it("merges a host provider into opencode.json without dropping existing models", () => { const home = mkdtempSync(join(tmpdir(), "omb-opencode-inject-")); diff --git a/server/testing/fake-acp-cli.ts b/server/testing/fake-acp-cli.ts index acd38a05d..cb0d9ae56 100755 --- a/server/testing/fake-acp-cli.ts +++ b/server/testing/fake-acp-cli.ts @@ -71,6 +71,7 @@ const dumpEnv = Object.fromEntries( "XAI_API_KEY", "BOX_TOKEN", "OMB_TTS_KEY", + "FACTORY_API_KEY", "UNSLOTH_STUDIO_AUTH_TOKEN", "CURSOR_API_KEY", "CURSOR_AUTH_TOKEN", From fadce239a26015906e629bb44281b8f406132f52 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 18 Aug 2026 11:18:11 +0200 Subject: [PATCH 11/52] Drop the unused tomlTableHasKey helper so typecheck passes. --- server/drivers/acp/kimi.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/server/drivers/acp/kimi.ts b/server/drivers/acp/kimi.ts index be0971bb8..a57d817b6 100644 --- a/server/drivers/acp/kimi.ts +++ b/server/drivers/acp/kimi.ts @@ -249,11 +249,6 @@ function hasTomlTable(text: string, heading: string): boolean { return name !== null && tomlTables(text).some((table) => table.name === name); } -/** Whether a table body already assigns `key` (`protocol` or `"protocol"`). */ -function tomlTableHasKey(block: string, key: string): boolean { - return tomlKeys(block).has(key); -} - /** Insert missing keys into an existing table. Does not overwrite set values. */ function patchTomlTable(text: string, heading: string, rows: string[]): string { const name = canonicalizeTomlHeading(heading); From 8497c3105e2a9488c9adeee677735141e70c0f11 Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 18 Aug 2026 12:52:55 +0200 Subject: [PATCH 12/52] Don't crash the UI when older tasks have no costUsd. A usage chip on first paint called toFixed on undefined for bots.json rows written before cost tracking. The packaged window rendered black. --- src/components/ChatView.tsx | 2 +- src/components/SettingsPanel.tsx | 2 +- src/components/UsageSection.tsx | 4 ++-- src/lib/usage.test.ts | 8 ++++++++ src/lib/usage.ts | 2 ++ 5 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/components/ChatView.tsx b/src/components/ChatView.tsx index 50e967b8c..9de2b35e6 100644 --- a/src/components/ChatView.tsx +++ b/src/components/ChatView.tsx @@ -1121,7 +1121,7 @@ function UsageChip({ bot }: { bot: Bot }) { const detail = [ `${usage.turns} turn${usage.turns === 1 ? "" : "s"}`, `${formatTokens(usage.input)} in · ${formatTokens(usage.output)} out`, - usage.costUsd !== null ? `${formatUsd(usage.costUsd)} ${costCaption(billing)}` : null, + typeof usage.costUsd === "number" ? `${formatUsd(usage.costUsd)} ${costCaption(billing)}` : null, ] .filter(Boolean) .join("\n"); diff --git a/src/components/SettingsPanel.tsx b/src/components/SettingsPanel.tsx index 008271887..38091518a 100644 --- a/src/components/SettingsPanel.tsx +++ b/src/components/SettingsPanel.tsx @@ -63,7 +63,7 @@ function BotUsageCard({ bot }: { bot: Bot }) {
Cost
-
{usage.costUsd === null ? "—" : formatUsd(usage.costUsd)}
+
{typeof usage.costUsd === "number" ? formatUsd(usage.costUsd) : "—"}
diff --git a/src/components/UsageSection.tsx b/src/components/UsageSection.tsx index f5015998a..89a327c3e 100644 --- a/src/components/UsageSection.tsx +++ b/src/components/UsageSection.tsx @@ -44,14 +44,14 @@ export function UsageSection() { {formatTokens(usage.input + usage.output)} - {usage.costUsd === null ? — : formatUsd(usage.costUsd)} + {typeof usage.costUsd === "number" ? formatUsd(usage.costUsd) : —}
))}
All bots {total.turns} {formatTokens(total.input + total.output)} - {total.costUsd === null ? "—" : formatUsd(total.costUsd)} + {typeof total.costUsd === "number" ? formatUsd(total.costUsd) : "—"}
{total.costUsd !== null && (
diff --git a/src/lib/usage.test.ts b/src/lib/usage.test.ts index aa9f0c820..c9a5229a1 100644 --- a/src/lib/usage.test.ts +++ b/src/lib/usage.test.ts @@ -16,6 +16,14 @@ describe("usage formatting", () => { expect(formatUsd(0.31)).toBe("$0.31"); }); + it("does not throw on missing usage fields from older bots.json", () => { + expect(formatUsd(undefined as unknown as number)).toBe(""); + expect(formatTokens(undefined as unknown as number)).toBe("0"); + expect( + usageChip({ input: 100, output: 20, turns: 1 } as { input: number; output: number; costUsd: null; turns: number }), + ).toBe("120 tok"); + }); + it("builds the chip: tokens always, cost only when known, nothing when unused", () => { expect(usageChip({ input: 0, output: 0, costUsd: null, turns: 0 })).toBe(""); expect(usageChip({ input: 10_000, output: 2_400, costUsd: null, turns: 3 })).toBe("12.4k tok"); diff --git a/src/lib/usage.ts b/src/lib/usage.ts index 0cdc8d9c2..2430669cd 100644 --- a/src/lib/usage.ts +++ b/src/lib/usage.ts @@ -23,6 +23,7 @@ export function botUsage(bot: Pick): TaskUsage { /** 950 → "950", 12_400 → "12.4k", 2_300_000 → "2.3M" */ export function formatTokens(n: number): string { + if (typeof n !== "number" || !Number.isFinite(n)) return "0"; if (n < 1000) return String(n); if (n < 1_000_000) return `${trim(n / 1000)}k`; return `${trim(n / 1_000_000)}M`; @@ -31,6 +32,7 @@ const trim = (x: number) => (x >= 100 ? Math.round(x).toString() : x.toFixed(1). /** Dollars, with enough precision that a cheap turn isn't "$0.00". */ export function formatUsd(usd: number): string { + if (typeof usd !== "number" || !Number.isFinite(usd)) return ""; if (usd === 0) return "$0"; if (usd < 0.01) return `$${usd.toFixed(3)}`; return `$${usd.toFixed(2)}`; From 974a3482089cb0f7ac9efd7347e8a4efc7caf5dd Mon Sep 17 00:00:00 2001 From: Max Date: Tue, 18 Aug 2026 15:43:15 +0200 Subject: [PATCH 13/52] Read Unsloth Studio's minted API token from the servers map. Current Studio stores keys as servers[url].minted instead of a top-level api_key. Without that, /v1/models returns 401 and Custom never lists Unsloth models. --- server/drivers/local-inject-matrix.test.ts | 15 +++++++++++ server/drivers/local-inject.ts | 30 +++++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/server/drivers/local-inject-matrix.test.ts b/server/drivers/local-inject-matrix.test.ts index 3ec29272e..327cb1082 100644 --- a/server/drivers/local-inject-matrix.test.ts +++ b/server/drivers/local-inject-matrix.test.ts @@ -125,6 +125,21 @@ describe("host credentials", () => { writeFileSync(join(home, ".unsloth", "studio", "auth", "agent_api_key.json"), JSON.stringify({ api_key: "from-file" })); expect(hostApiKey(localHost("unsloth")!, { HOME: home })).toBe("from-file"); }); + + it("reads a minted Unsloth Studio token from the servers map", () => { + const home = scratchHome("omb-unsloth-minted-"); + mkdirSync(join(home, ".unsloth", "studio", "auth"), { recursive: true }); + writeFileSync( + join(home, ".unsloth", "studio", "auth", "agent_api_key.json"), + JSON.stringify({ + servers: { + "http://127.0.0.1:8888": { saved: [], minted: ["sk-unsloth-minted"] }, + }, + }), + ); + expect(hostApiKey(localHost("unsloth")!, { HOME: home })).toBe("sk-unsloth-minted"); + expect(hostApiKey(localHost("unsloth_api")!, { HOME: home })).toBe("sk-unsloth-minted"); + }); }); describe("OpenAI / Anthropic env dialects", () => { diff --git a/server/drivers/local-inject.ts b/server/drivers/local-inject.ts index 6287afa1c..8a99cde67 100644 --- a/server/drivers/local-inject.ts +++ b/server/drivers/local-inject.ts @@ -104,13 +104,41 @@ export function codexLocalProviderArgs( ]; } +function firstUnslothToken(row: unknown): string | null { + if (!row || typeof row !== "object") return null; + const rec = row as { minted?: unknown; saved?: unknown; api_key?: unknown }; + if (typeof rec.api_key === "string" && rec.api_key) return rec.api_key; + for (const bucket of [rec.minted, rec.saved]) { + if (typeof bucket === "string" && bucket) return bucket; + if (Array.isArray(bucket)) { + const token = bucket.find((value) => typeof value === "string" && value); + if (typeof token === "string") return token; + } + } + return null; +} + function readUnslothKey(env: Record): string | null { const home = env.HOME || env.USERPROFILE || homedir(); try { const raw = JSON.parse(readFileSync(join(home, ".unsloth", "studio", "auth", "agent_api_key.json"), "utf8")) as { api_key?: unknown; + servers?: unknown; }; - return typeof raw.api_key === "string" && raw.api_key ? raw.api_key : null; + // Older Studio wrote `{ api_key }`. Current Studio writes + // `{ servers: { "http://127.0.0.1:8888": { minted: ["sk-unsloth-…"] } } }`. + if (typeof raw.api_key === "string" && raw.api_key) return raw.api_key; + if (!raw.servers || typeof raw.servers !== "object") return null; + const servers = raw.servers as Record; + for (const url of ["http://127.0.0.1:8888", "http://localhost:8888"]) { + const token = firstUnslothToken(servers[url]); + if (token) return token; + } + for (const row of Object.values(servers)) { + const token = firstUnslothToken(row); + if (token) return token; + } + return null; } catch { return null; } From 4d194ec506aa115b8dd170c69c3a3a04d5e74eef Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 19 Aug 2026 22:55:19 +0200 Subject: [PATCH 14/52] Honor remaining review notes for Droid, Unsloth, usage, and Kimi TOML. Skip the Droid FACTORY_API_KEY placeholder when a Factory auth file already exists. Prefer localhost minted Unsloth tokens over a stale top-level api_key. Treat NaN/Infinity costs as missing in the chip and settings. Trim whitespace around dotted TOML headings and ignore """ inside comments or single-line strings. --- server/drivers/acp/droid.ts | 4 ++ server/drivers/acp/kimi.ts | 39 +++++++++--- server/drivers/local-inject-matrix.test.ts | 15 +++++ server/drivers/local-inject.test.ts | 70 ++++++++++++++++++++++ server/drivers/local-inject.ts | 25 ++++---- src/components/ChatView.tsx | 4 +- src/components/SettingsPanel.tsx | 6 +- src/components/UsageSection.tsx | 8 +-- src/lib/usage.test.ts | 15 +++++ src/lib/usage.ts | 13 ++-- 10 files changed, 166 insertions(+), 33 deletions(-) diff --git a/server/drivers/acp/droid.ts b/server/drivers/acp/droid.ts index cc0ad5b7e..e3f0e468e 100644 --- a/server/drivers/acp/droid.ts +++ b/server/drivers/acp/droid.ts @@ -129,6 +129,10 @@ export function applyDroidLocalAuthEnv( ): void { if (!decodeInjectId(modelId)) return; if (env.FACTORY_API_KEY?.trim()) return; + // session/new already succeeds on a Factory login file. A placeholder + // FACTORY_API_KEY can take precedence over that login, so leave env + // alone when one of the auth files is present. + if (authFilePaths(env).some(existsSync)) return; env.FACTORY_API_KEY = "openmausbot-local"; } diff --git a/server/drivers/acp/kimi.ts b/server/drivers/acp/kimi.ts index a57d817b6..9c49621c0 100644 --- a/server/drivers/acp/kimi.ts +++ b/server/drivers/acp/kimi.ts @@ -77,11 +77,11 @@ function canonicalizeTomlHeading(heading: string): string | null { const parts: string[] = []; const inner = match[1]!; let i = 0; + const skipSep = () => { + while (i < inner.length && (inner[i] === "." || inner[i] === " " || inner[i] === "\t")) i += 1; + }; + skipSep(); while (i < inner.length) { - if (inner[i] === ".") { - i += 1; - continue; - } const q = inner[i]; if (q === '"' || q === "'") { i += 1; @@ -97,6 +97,7 @@ function canonicalizeTomlHeading(heading: string): string | null { } if (inner[i] === q) i += 1; parts.push(value); + skipSep(); continue; } let value = ""; @@ -104,9 +105,11 @@ function canonicalizeTomlHeading(heading: string): string | null { value += inner[i]; i += 1; } - parts.push(value); + const part = value.trim(); + if (part) parts.push(part); + skipSep(); } - return parts.join("."); + return parts.length ? parts.join(".") : null; } /** Unwrap `"key"` / `'key'` so a quoted assignment matches the bare name. */ @@ -209,10 +212,12 @@ function tomlTables(text: string): Array<{ name: string; headingStart: number; b /** Keys assigned at line start in a table body, including `"quoted"` keys. */ function tomlKeys(block: string): Set { const keys = new Set(); + type Mode = "out" | "basic" | "literal" | "mlbasic" | "mllit"; + let mode: Mode = "out"; let lineStart = 0; - let mode: "out" | "mlbasic" | "mllit" = "out"; + let lineStartMode: Mode = "out"; const take = (end: number) => { - if (mode !== "out") return; + if (lineStartMode !== "out") return; const line = stripTomlLineComment(block.slice(lineStart, end)); const eq = line.indexOf("="); if (eq > 0) keys.add(unquoteTomlKey(line.slice(0, eq))); @@ -228,15 +233,31 @@ function tomlKeys(block: string): Set { mode = "out"; i += 2; } + } else if (mode === "basic") { + if (block[i] === "\\") i += 1; + else if (block[i] === '"') mode = "out"; + } else if (mode === "literal") { + if (block[i] === "'") mode = "out"; + } else if (block[i] === "#") { + const nl = block.indexOf("\n", i); + i = nl < 0 ? block.length : nl; + if (nl < 0) break; } else if (block.startsWith('"""', i)) { mode = "mlbasic"; i += 2; } else if (block.startsWith("'''", i)) { mode = "mllit"; i += 2; - } else if (block[i] === "\n") { + } else if (block[i] === '"') { + mode = "basic"; + } else if (block[i] === "'") { + mode = "literal"; + } + if (i < block.length && block[i] === "\n") { take(i); lineStart = i + 1; + if (mode === "basic" || mode === "literal") mode = "out"; + lineStartMode = mode; } } take(block.length); diff --git a/server/drivers/local-inject-matrix.test.ts b/server/drivers/local-inject-matrix.test.ts index 327cb1082..078d2fb82 100644 --- a/server/drivers/local-inject-matrix.test.ts +++ b/server/drivers/local-inject-matrix.test.ts @@ -140,6 +140,21 @@ describe("host credentials", () => { expect(hostApiKey(localHost("unsloth")!, { HOME: home })).toBe("sk-unsloth-minted"); expect(hostApiKey(localHost("unsloth_api")!, { HOME: home })).toBe("sk-unsloth-minted"); }); + + it("prefers a localhost minted token over a stale top-level api_key", () => { + const home = scratchHome("omb-unsloth-mixed-"); + mkdirSync(join(home, ".unsloth", "studio", "auth"), { recursive: true }); + writeFileSync( + join(home, ".unsloth", "studio", "auth", "agent_api_key.json"), + JSON.stringify({ + api_key: "stale-legacy", + servers: { + "http://127.0.0.1:8888": { saved: [], minted: ["sk-unsloth-fresh"] }, + }, + }), + ); + expect(hostApiKey(localHost("unsloth")!, { HOME: home })).toBe("sk-unsloth-fresh"); + }); }); describe("OpenAI / Anthropic env dialects", () => { diff --git a/server/drivers/local-inject.test.ts b/server/drivers/local-inject.test.ts index e7039b6e1..88fb856f8 100644 --- a/server/drivers/local-inject.test.ts +++ b/server/drivers/local-inject.test.ts @@ -402,6 +402,66 @@ describe("ensureKimiInjectAlias", () => { expect(text).toContain('protocol = "openai"'); }); + it("treats whitespace around dotted heading keys as the same table", () => { + const home = mkdtempSync(join(tmpdir(), "omb-kimi-dots-")); + scratchDirs.push(home); + const root = join(home, ".kimi-code"); + mkdirSync(root, { recursive: true }); + writeFileSync( + join(root, "config.toml"), + ['[models . "omlx/GLM-5.2-fp8"]', 'provider = "omlx"', 'model = "GLM-5.2-fp8"', ""].join("\n"), + ); + ensureKimiInjectAlias("omlx::GLM-5.2-fp8", { HOME: home }); + const text = readFileSync(join(root, "config.toml"), "utf8"); + expect(text.match(/\[models/g)?.length).toBe(1); + expect(text).toContain('protocol = "openai"'); + expect(text).toContain("max_context_size = 262144"); + }); + + it("does not treat a triple-quote inside a single-line string as multiline", () => { + const home = mkdtempSync(join(tmpdir(), "omb-kimi-squote-")); + scratchDirs.push(home); + const root = join(home, ".kimi-code"); + mkdirSync(root, { recursive: true }); + writeFileSync( + join(root, "config.toml"), + [ + '[models."omlx/GLM-5.2-fp8"]', + 'provider = "omlx"', + 'model = "GLM-5.2-fp8"', + `note = '"""'`, + 'protocol = "openai"', + "", + ].join("\n"), + ); + ensureKimiInjectAlias("omlx::GLM-5.2-fp8", { HOME: home }); + const text = readFileSync(join(root, "config.toml"), "utf8"); + expect(text.match(/protocol = "openai"/g)?.length).toBe(1); + expect(text).toContain("max_context_size = 262144"); + }); + + it("does not treat a triple-quote inside a comment as multiline", () => { + const home = mkdtempSync(join(tmpdir(), "omb-kimi-hash-")); + scratchDirs.push(home); + const root = join(home, ".kimi-code"); + mkdirSync(root, { recursive: true }); + writeFileSync( + join(root, "config.toml"), + [ + '[models."omlx/GLM-5.2-fp8"]', + 'provider = "omlx"', + 'model = "GLM-5.2-fp8"', + 'note = "x" # """', + 'protocol = "openai"', + "", + ].join("\n"), + ); + ensureKimiInjectAlias("omlx::GLM-5.2-fp8", { HOME: home }); + const text = readFileSync(join(root, "config.toml"), "utf8"); + expect(text.match(/protocol = "openai"/g)?.length).toBe(1); + expect(text).toContain("max_context_size = 262144"); + }); + it("does not treat a bracket line inside a multiline string as a table", () => { const home = mkdtempSync(join(tmpdir(), "omb-kimi-ml-")); scratchDirs.push(home); @@ -589,6 +649,16 @@ describe("applyDroidLocalAuthEnv", () => { expect(cloud.FACTORY_API_KEY).toBeUndefined(); }); + it("does not invent a Factory key when a Droid auth file already exists", () => { + const home = mkdtempSync(join(tmpdir(), "omb-droid-authfile-")); + scratchDirs.push(home); + mkdirSync(join(home, ".factory"), { recursive: true }); + writeFileSync(join(home, ".factory", "auth.v2.file"), "signed-in"); + const env: Record = { FACTORY_HOME_OVERRIDE: home }; + applyDroidLocalAuthEnv(env, "ollama::ornith:35b-bf16"); + expect(env).toEqual({ FACTORY_HOME_OVERRIDE: home }); + }); + it("puts the placeholder on the Droid child only for a local inject pick", async () => { const home = mkdtempSync(join(tmpdir(), "omb-droid-overlay-")); scratchDirs.push(home); diff --git a/server/drivers/local-inject.ts b/server/drivers/local-inject.ts index 8a99cde67..a979b63aa 100644 --- a/server/drivers/local-inject.ts +++ b/server/drivers/local-inject.ts @@ -107,7 +107,6 @@ export function codexLocalProviderArgs( function firstUnslothToken(row: unknown): string | null { if (!row || typeof row !== "object") return null; const rec = row as { minted?: unknown; saved?: unknown; api_key?: unknown }; - if (typeof rec.api_key === "string" && rec.api_key) return rec.api_key; for (const bucket of [rec.minted, rec.saved]) { if (typeof bucket === "string" && bucket) return bucket; if (Array.isArray(bucket)) { @@ -115,6 +114,7 @@ function firstUnslothToken(row: unknown): string | null { if (typeof token === "string") return token; } } + if (typeof rec.api_key === "string" && rec.api_key) return rec.api_key; return null; } @@ -127,17 +127,20 @@ function readUnslothKey(env: Record): string | null }; // Older Studio wrote `{ api_key }`. Current Studio writes // `{ servers: { "http://127.0.0.1:8888": { minted: ["sk-unsloth-…"] } } }`. - if (typeof raw.api_key === "string" && raw.api_key) return raw.api_key; - if (!raw.servers || typeof raw.servers !== "object") return null; - const servers = raw.servers as Record; - for (const url of ["http://127.0.0.1:8888", "http://localhost:8888"]) { - const token = firstUnslothToken(servers[url]); - if (token) return token; - } - for (const row of Object.values(servers)) { - const token = firstUnslothToken(row); - if (token) return token; + // Prefer the localhost minted token so a stale mixed-format file cannot + // win; keep the top-level key as fallback. + if (raw.servers && typeof raw.servers === "object") { + const servers = raw.servers as Record; + for (const url of ["http://127.0.0.1:8888", "http://localhost:8888"]) { + const token = firstUnslothToken(servers[url]); + if (token) return token; + } + for (const row of Object.values(servers)) { + const token = firstUnslothToken(row); + if (token) return token; + } } + if (typeof raw.api_key === "string" && raw.api_key) return raw.api_key; return null; } catch { return null; diff --git a/src/components/ChatView.tsx b/src/components/ChatView.tsx index 9de2b35e6..97eb847ec 100644 --- a/src/components/ChatView.tsx +++ b/src/components/ChatView.tsx @@ -22,7 +22,7 @@ import { Webhook, X, } from "lucide-react"; -import { costCaption, formatTokens, formatUsd, usageChip } from "@/lib/usage"; +import { costCaption, formatTokens, formatUsd, hasFiniteCost, usageChip } from "@/lib/usage"; import { useStore, useStreaming, @@ -1121,7 +1121,7 @@ function UsageChip({ bot }: { bot: Bot }) { const detail = [ `${usage.turns} turn${usage.turns === 1 ? "" : "s"}`, `${formatTokens(usage.input)} in · ${formatTokens(usage.output)} out`, - typeof usage.costUsd === "number" ? `${formatUsd(usage.costUsd)} ${costCaption(billing)}` : null, + hasFiniteCost(usage.costUsd) ? `${formatUsd(usage.costUsd)} ${costCaption(billing)}` : null, ] .filter(Boolean) .join("\n"); diff --git a/src/components/SettingsPanel.tsx b/src/components/SettingsPanel.tsx index 38091518a..f314e7352 100644 --- a/src/components/SettingsPanel.tsx +++ b/src/components/SettingsPanel.tsx @@ -13,7 +13,7 @@ import { ModelPicker } from "./ModelPicker"; import { useDesktopCapabilities } from "./DesktopCapabilities"; import { cn } from "@/lib/cn"; import { requestNotificationPermission } from "@/lib/notify"; -import { botUsage, costCaption, formatTokens, formatUsd } from "@/lib/usage"; +import { botUsage, costCaption, formatTokens, formatUsd, hasFiniteCost } from "@/lib/usage"; import { shortPath } from "@/lib/short-path"; import { instanceSupportsLocalComputer, localComputerDisabledReason } from "@/lib/local-computer"; @@ -63,11 +63,11 @@ function BotUsageCard({ bot }: { bot: Bot }) {
Cost
-
{typeof usage.costUsd === "number" ? formatUsd(usage.costUsd) : "—"}
+
{hasFiniteCost(usage.costUsd) ? formatUsd(usage.costUsd) : "—"}
- {usage.costUsd === null ? "This engine doesn't report a price; tokens are counted." : `Cost ${costCaption(instance?.snapshot.billing)}.`} + {hasFiniteCost(usage.costUsd) ? `Cost ${costCaption(instance?.snapshot.billing)}.` : "This engine doesn't report a price; tokens are counted."}
); diff --git a/src/components/UsageSection.tsx b/src/components/UsageSection.tsx index 89a327c3e..4540daeb0 100644 --- a/src/components/UsageSection.tsx +++ b/src/components/UsageSection.tsx @@ -5,7 +5,7 @@ import { useStore } from "@/state/store"; import { MausAvatar } from "./Avatar"; import { Card } from "./SettingsPrimitives"; -import { botUsage, costCaption, formatTokens, formatUsd, sumUsage } from "@/lib/usage"; +import { botUsage, costCaption, formatTokens, formatUsd, hasFiniteCost, sumUsage } from "@/lib/usage"; export function UsageSection() { const { state } = useStore(); @@ -44,16 +44,16 @@ export function UsageSection() { {formatTokens(usage.input + usage.output)} - {typeof usage.costUsd === "number" ? formatUsd(usage.costUsd) : —} + {hasFiniteCost(usage.costUsd) ? formatUsd(usage.costUsd) : —} ))}
All bots {total.turns} {formatTokens(total.input + total.output)} - {typeof total.costUsd === "number" ? formatUsd(total.costUsd) : "—"} + {hasFiniteCost(total.costUsd) ? formatUsd(total.costUsd) : "—"}
- {total.costUsd !== null && ( + {hasFiniteCost(total.costUsd) && (
Cost is {billings.size === 1 ? costCaption([...billings][0]) : "as each engine reports it — on a subscription it's an equivalent, not a charge"}.
diff --git a/src/lib/usage.test.ts b/src/lib/usage.test.ts index c9a5229a1..a0abac415 100644 --- a/src/lib/usage.test.ts +++ b/src/lib/usage.test.ts @@ -24,6 +24,21 @@ describe("usage formatting", () => { ).toBe("120 tok"); }); + it("treats NaN and Infinity cost as missing", () => { + expect(formatUsd(Number.NaN)).toBe(""); + expect(formatUsd(Number.POSITIVE_INFINITY)).toBe(""); + expect(formatTokens(Number.NaN)).toBe("0"); + expect(formatTokens(Number.POSITIVE_INFINITY)).toBe("0"); + expect(usageChip({ input: 100, output: 20, costUsd: Number.NaN, turns: 1 })).toBe("120 tok"); + expect(usageChip({ input: 100, output: 20, costUsd: Number.POSITIVE_INFINITY, turns: 1 })).toBe("120 tok"); + expect( + sumUsage([ + { input: 1, output: 1, costUsd: Number.NaN, turns: 1 }, + { input: 2, output: 2, costUsd: 0.01, turns: 1 }, + ]), + ).toEqual({ input: 3, output: 3, costUsd: 0.01, turns: 2 }); + }); + it("builds the chip: tokens always, cost only when known, nothing when unused", () => { expect(usageChip({ input: 0, output: 0, costUsd: null, turns: 0 })).toBe(""); expect(usageChip({ input: 10_000, output: 2_400, costUsd: null, turns: 3 })).toBe("12.4k tok"); diff --git a/src/lib/usage.ts b/src/lib/usage.ts index 2430669cd..179bfe659 100644 --- a/src/lib/usage.ts +++ b/src/lib/usage.ts @@ -4,6 +4,11 @@ import type { Bot, TaskUsage } from "@/state/store"; export const EMPTY_USAGE: TaskUsage = { input: 0, output: 0, costUsd: null, turns: 0 }; +/** True when a stored cost is a real number (not null, NaN, or Infinity). */ +export function hasFiniteCost(value: unknown): value is number { + return typeof value === "number" && Number.isFinite(value); +} + /** Sum a set of usages; cost stays null until any of them has one. */ export function sumUsage(items: Array): TaskUsage { const out: TaskUsage = { ...EMPTY_USAGE }; @@ -12,7 +17,7 @@ export function sumUsage(items: Array): TaskUsage { out.input += u.input; out.output += u.output; out.turns += u.turns; - if (typeof u.costUsd === "number") out.costUsd = (out.costUsd ?? 0) + u.costUsd; + if (hasFiniteCost(u.costUsd)) out.costUsd = (out.costUsd ?? 0) + u.costUsd; } return out; } @@ -23,7 +28,7 @@ export function botUsage(bot: Pick): TaskUsage { /** 950 → "950", 12_400 → "12.4k", 2_300_000 → "2.3M" */ export function formatTokens(n: number): string { - if (typeof n !== "number" || !Number.isFinite(n)) return "0"; + if (!hasFiniteCost(n)) return "0"; if (n < 1000) return String(n); if (n < 1_000_000) return `${trim(n / 1000)}k`; return `${trim(n / 1_000_000)}M`; @@ -32,7 +37,7 @@ const trim = (x: number) => (x >= 100 ? Math.round(x).toString() : x.toFixed(1). /** Dollars, with enough precision that a cheap turn isn't "$0.00". */ export function formatUsd(usd: number): string { - if (typeof usd !== "number" || !Number.isFinite(usd)) return ""; + if (!hasFiniteCost(usd)) return ""; if (usd === 0) return "$0"; if (usd < 0.01) return `$${usd.toFixed(3)}`; return `$${usd.toFixed(2)}`; @@ -43,7 +48,7 @@ export function formatUsd(usd: number): string { export function usageChip(u: TaskUsage): string { if (u.turns === 0 && u.input + u.output === 0) return ""; const parts = [`${formatTokens(u.input + u.output)} tok`]; - if (typeof u.costUsd === "number") parts.push(formatUsd(u.costUsd)); + if (hasFiniteCost(u.costUsd)) parts.push(formatUsd(u.costUsd)); return parts.join(" · "); } From 37aa31bbfe1324a641996def0d1f8ce7a768c5ac Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 20 Aug 2026 20:18:14 +0200 Subject: [PATCH 15/52] Harden the Kimi TOML scanner around comments and array tables. Skip # comments in tomlTables so an apostrophe in a comment cannot open a phantom string and hide the real model heading. Treat [[array]] headings as table boundaries without patching them, so protocol keys land in the model table instead of the following hooks array. --- server/drivers/acp/kimi.ts | 38 +++++++++++++++++------ server/drivers/local-inject.test.ts | 47 +++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 10 deletions(-) diff --git a/server/drivers/acp/kimi.ts b/server/drivers/acp/kimi.ts index 9c49621c0..c55334c9d 100644 --- a/server/drivers/acp/kimi.ts +++ b/server/drivers/acp/kimi.ts @@ -127,10 +127,12 @@ function tomlRowKey(row: string): string { return unquoteTomlKey(eq < 0 ? row : row.slice(0, eq)); } -/** Walk `text` and yield tables, skipping `[` inside strings (including multiline). */ +/** Walk `text` and yield `[table]` spans. `[[array]]` headings bound a table + * but are not themselves patchable. `#` comments in `out` mode are skipped + * so an apostrophe in a comment cannot open a phantom string. */ function tomlTables(text: string): Array<{ name: string; headingStart: number; bodyStart: number; end: number }> { type Mode = "out" | "basic" | "literal" | "mlbasic" | "mllit"; - const headings: Array<{ name: string; lineStart: number; lineEnd: number }> = []; + const headings: Array<{ name: string | null; patchable: boolean; lineStart: number; lineEnd: number }> = []; let mode: Mode = "out"; let i = 0; const atLineStart = (idx: number) => idx === 0 || text[idx - 1] === "\n"; @@ -187,26 +189,42 @@ function tomlTables(text: string): Array<{ name: string; headingStart: number; b i += 1; continue; } + if (text[i] === "#") { + const nl = text.indexOf("\n", i); + i = nl < 0 ? text.length : nl + 1; + continue; + } if (atLineStart(i)) { let j = i; while (j < text.length && (text[j] === " " || text[j] === "\t")) j += 1; if (text[j] === "[") { const nl = text.indexOf("\n", j); const lineEnd = nl < 0 ? text.length : nl; - const name = canonicalizeTomlHeading(text.slice(j, lineEnd).replace(/\r$/, "")); - if (name) headings.push({ name, lineStart: i, lineEnd }); + const raw = text.slice(j, lineEnd).replace(/\r$/, ""); + const stripped = stripTomlLineComment(raw).trim(); + const array = stripped.startsWith("[["); + const name = array + ? canonicalizeTomlHeading(`[${stripped.replace(/^\s*\[\[/, "").replace(/\]\]\s*$/, "")}]`) + : canonicalizeTomlHeading(raw); + headings.push({ name, patchable: !array && name !== null, lineStart: i, lineEnd }); i = lineEnd + (nl < 0 ? 0 : 1); continue; } } i += 1; } - return headings.map((heading, index) => ({ - name: heading.name, - headingStart: heading.lineStart, - bodyStart: heading.lineEnd + (text[heading.lineEnd] === "\n" ? 1 : 0), - end: index + 1 < headings.length ? headings[index + 1]!.lineStart : text.length, - })); + return headings + .map((heading, index) => ({ + heading, + end: index + 1 < headings.length ? headings[index + 1]!.lineStart : text.length, + })) + .filter((entry) => entry.heading.patchable && entry.heading.name) + .map((entry) => ({ + name: entry.heading.name!, + headingStart: entry.heading.lineStart, + bodyStart: entry.heading.lineEnd + (text[entry.heading.lineEnd] === "\n" ? 1 : 0), + end: entry.end, + })); } /** Keys assigned at line start in a table body, including `"quoted"` keys. */ diff --git a/server/drivers/local-inject.test.ts b/server/drivers/local-inject.test.ts index 88fb856f8..c712f7c45 100644 --- a/server/drivers/local-inject.test.ts +++ b/server/drivers/local-inject.test.ts @@ -402,6 +402,53 @@ describe("ensureKimiInjectAlias", () => { expect(text).toContain('protocol = "openai"'); }); + it("does not hide a model table behind an apostrophe in a preceding comment", () => { + const home = mkdtempSync(join(tmpdir(), "omb-kimi-apos-")); + scratchDirs.push(home); + const root = join(home, ".kimi-code"); + mkdirSync(root, { recursive: true }); + writeFileSync( + join(root, "config.toml"), + [ + "# user's setting", + '[models."omlx/GLM-5.2-fp8"]', + 'provider = "omlx"', + 'model = "GLM-5.2-fp8"', + "", + ].join("\n"), + ); + ensureKimiInjectAlias("omlx::GLM-5.2-fp8", { HOME: home }); + const text = readFileSync(join(root, "config.toml"), "utf8"); + expect(text.match(/\[models\./g)?.length).toBe(1); + expect(text).toContain("# user's setting"); + expect(text).toContain('protocol = "openai"'); + expect(text).toContain("max_context_size = 262144"); + }); + + it("stops a model table before a following array-of-tables heading", () => { + const home = mkdtempSync(join(tmpdir(), "omb-kimi-aot-")); + scratchDirs.push(home); + const root = join(home, ".kimi-code"); + mkdirSync(root, { recursive: true }); + writeFileSync( + join(root, "config.toml"), + [ + '[models."omlx/GLM-5.2-fp8"]', + 'provider = "omlx"', + 'model = "GLM-5.2-fp8"', + "", + "[[hooks]]", + 'event = "Stop"', + "", + ].join("\n"), + ); + ensureKimiInjectAlias("omlx::GLM-5.2-fp8", { HOME: home }); + const text = readFileSync(join(root, "config.toml"), "utf8"); + expect(text.indexOf('protocol = "openai"')).toBeLessThan(text.indexOf("[[hooks]]")); + expect(text.indexOf("max_context_size = 262144")).toBeLessThan(text.indexOf("[[hooks]]")); + expect(text).toMatch(/\[\[hooks\]\]\s*event = "Stop"/); + }); + it("treats whitespace around dotted heading keys as the same table", () => { const home = mkdtempSync(join(tmpdir(), "omb-kimi-dots-")); scratchDirs.push(home); From 848fea83ed7b883318eb87116aeaae6f23a508ff Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 20 Aug 2026 20:42:26 +0200 Subject: [PATCH 16/52] Allow Auto mode on this Mac's computer after a warning. Upstream blocked Auto while a bot was on the local computer. On macOS the user can now confirm a warning and let the bot click and type here; destructive and sensitive actions still stop. CUA can fall back to the standalone CuaDriver.app so existing Accessibility grants keep working. Also: Claude turns rewrite leftover Custom slugs onto a live local host so the picker does not demand /login when Unsloth is already serving the model. --- electron/cua.mjs | 67 ++++++++++++++++-- electron/main.mjs | 1 + scripts/prepare-cua.mjs | 5 +- server/auto-approve.test.ts | 11 ++- server/auto-approve.ts | 8 +-- server/drivers/claude-catalog.test.ts | 13 ++++ server/drivers/claude.test.ts | 30 ++++++++ server/drivers/claude.ts | 33 +++++++-- server/drivers/codex.ts | 1 + server/drivers/local-inject.test.ts | 54 +++++++++++++++ server/drivers/local-inject.ts | 24 ++++++- server/index.test.ts | 19 ++---- server/index.ts | 20 ++---- src/components/ComputerPanel.tsx | 25 ++++++- src/components/LocalComputerAutoWarning.tsx | 76 +++++++++++++++++++++ src/components/MacLocalControl.tsx | 56 +++++++++++++++ src/components/SettingsPanel.tsx | 36 +++++++--- src/lib/local-computer.test.ts | 25 +++++++ src/lib/local-computer.ts | 24 +++++-- src/state/store.tsx | 5 +- src/types/ogb.d.ts | 4 +- 21 files changed, 464 insertions(+), 73 deletions(-) create mode 100644 src/components/LocalComputerAutoWarning.tsx create mode 100644 src/components/MacLocalControl.tsx diff --git a/electron/cua.mjs b/electron/cua.mjs index 50c035c9f..76bfcc73c 100644 --- a/electron/cua.mjs +++ b/electron/cua.mjs @@ -83,6 +83,12 @@ export function setCuaStateListener(listener) { stateListener = typeof listener === "function" ? listener : () => {}; } +function persistAndNotify(next) { + const connection = connectionStore.persist(next); + stateListener(connection); + return connection; +} + export function resolveDriverBinary() { if (process.env.CUA_DRIVER_PATH) return process.env.CUA_DRIVER_PATH; if (app.isPackaged) { @@ -124,6 +130,29 @@ async function loadEmbeddedSdk() { return import(pathToFileURL(path.join(process.resourcesPath, "cua-sdk", "cua-sdk.mjs")).href); } +async function attachStandalone() { + const driver = fs.existsSync(INSTALLED_DRIVER) ? INSTALLED_DRIVER : null; + if (!driver) return null; + if (!(await socketAlive(STANDALONE_SOCKET))) { + // Launch CuaDriver.app through LaunchServices so Accessibility / + // Screen Recording stay on com.trycua.driver — the identity this + // machine already granted — instead of the freshly signed OpenMausBot. + spawnSync("open", ["-a", "CuaDriver"], { timeout: 8000 }); + for (let i = 0; i < 25; i++) { + if (await socketAlive(STANDALONE_SOCKET)) break; + await new Promise((resolve) => setTimeout(resolve, 200)); + } + } + if (!(await socketAlive(STANDALONE_SOCKET))) return null; + return { + mode: "standalone", + socketPath: STANDALONE_SOCKET, + mcpCommand: driver, + mcpArgs: ["mcp"], + mcpEnv: { ...CUA_ENV }, + }; +} + async function startEmbedded(binary) { // Import from the staged Resources tree in production. The app intentionally // excludes general node_modules, so a bare package import only works in dev. @@ -154,7 +183,7 @@ export async function startCua() { if (process.platform === "linux") return ensureLinuxRuntime().initialize(); const binary = resolveDriverBinary(); if (!binary) { - return connectionStore.persist({ + return persistAndNotify({ mode: "unavailable", reason: "cua-driver binary not found", }); @@ -168,10 +197,13 @@ export async function startCua() { try { nextConnection = await startEmbedded(binary); } catch (err) { - nextConnection = { - mode: "unavailable", - reason: `embedded host failed: ${err?.message ?? err}`, - }; + nextConnection = await attachStandalone(); + if (!nextConnection) { + nextConnection = { + mode: "unavailable", + reason: `embedded host failed: ${err?.message ?? err}`, + }; + } } } else if (await socketAlive(STANDALONE_SOCKET)) { // Dev machine with CuaDriver.app's daemon already running. @@ -190,7 +222,7 @@ export async function startCua() { }; } - return connectionStore.persist(nextConnection); + return persistAndNotify(nextConnection); } export function cuaPermissionsStatus() { @@ -227,7 +259,7 @@ export async function stopCua() { embeddedHost = null; } if (connectionStore.get()) { - connectionStore.persist({ mode: "unavailable", reason: "desktop-host-stopped" }); + persistAndNotify({ mode: "unavailable", reason: "desktop-host-stopped" }); } } @@ -262,6 +294,27 @@ export function registerCuaIpc() { return ensureLinuxRuntime().getStatus(); }); ipcMain.handle("cua:linux-retry", async () => { + if (process.platform === "darwin") { + try { + await stopCua(); + const connection = await startCua(); + const ready = connection?.mode === "embedded" || connection?.mode === "standalone"; + return { + enabled: ready, + status: ready ? "ready" : "error", + reasonCode: ready ? undefined : "permissions-required", + message: connection?.reason, + }; + } catch (error) { + console.error("[cua] macOS retry failed:", error); + return { + enabled: false, + status: "error", + reasonCode: "permissions-required", + message: error instanceof Error ? error.message : String(error), + }; + } + } if (process.platform !== "linux") { return { enabled: false, status: "unavailable", reasonCode: "unsupported-platform" }; } diff --git a/electron/main.mjs b/electron/main.mjs index 89a50b099..ac4c68aaa 100644 --- a/electron/main.mjs +++ b/electron/main.mjs @@ -543,6 +543,7 @@ ipcMain.handle("perm:open-settings", (_event, pane) => { mic: "Privacy_Microphone", screen: "Privacy_ScreenCapture", speech: "Privacy_SpeechRecognition", + accessibility: "Privacy_Accessibility", }; // own-property lookup only — a renderer-supplied "__proto__"/"constructor" // would otherwise resolve up the prototype chain to a truthy object diff --git a/scripts/prepare-cua.mjs b/scripts/prepare-cua.mjs index 4b00f79db..dcbe8b9fd 100644 --- a/scripts/prepare-cua.mjs +++ b/scripts/prepare-cua.mjs @@ -107,7 +107,10 @@ if (!details.isFile() || (details.mode & 0o111) === 0) { // not on a user's Intel Mac); the SDK's dylib/.node are genuinely per-arch, // pulled from the two darwin native packages that pnpm installs because of // supportedArchitectures in package.json. -const MAC_ARCHES = ["arm64", "x64"]; +const MAC_ARCHES = (process.env.OPENMAUSBOT_CUA_ARCHES ?? "arm64,x64") + .split(",") + .map((arch) => arch.trim()) + .filter(Boolean); const { stdout: archList } = await run("/usr/bin/lipo", ["-archs", binary]); for (const arch of MAC_ARCHES) { diff --git a/server/auto-approve.test.ts b/server/auto-approve.test.ts index a015b60bd..17bad4eb8 100644 --- a/server/auto-approve.test.ts +++ b/server/auto-approve.test.ts @@ -112,9 +112,16 @@ describe("autoDecision", () => { expect(autoDecision({ alwaysAllow: ["Bash"] }, "Bash", "sudo rm -rf /var")).toBeNull(); }); - it("never delegates a local-computer request to auto or remembered grants", () => { + it("auto-approves a local-computer request when Auto mode is on", () => { + expect( + autoDecision({ autoApprove: true }, "mcp__computer__click", "Click the Submit button", { + scope: "local-computer", + }), + ).toBe("auto-approved mcp__computer__click"); + }); + + it("does not let always-allow cover host control without Auto mode", () => { const bot = { - autoApprove: true, alwaysAllow: ["mcp__computer__click", "local-computer:mcp__computer__click"], }; expect( diff --git a/server/auto-approve.ts b/server/auto-approve.ts index a90b20744..81a463ca4 100644 --- a/server/auto-approve.ts +++ b/server/auto-approve.ts @@ -144,11 +144,9 @@ export function autoVerdict( if (sensitive) return { approve: null, source: "sensitive-guard", rule: sensitive }; return { approve: null, source: "no-grant" }; } - if (context?.scope === "local-computer") { - // The user's active desktop is never delegated to bot auto mode or a - // remembered cloud/tool grant in the Linux beta. Same attribution rule - // as the unattended block: a guard that would have carded anyway keeps - // its own name, the block is the story only when it changed the outcome. + if (context?.scope === "local-computer" && !bot.autoApprove) { + // Host control is not covered by a remembered always-allow grant. + // Auto mode can approve these after the user confirms a warning. if (grant) return { approve: null, source: "local-computer-block", rule: grant.rule }; if (destructive) return { approve: null, source: "destructive-guard", rule: destructive }; if (sensitive) return { approve: null, source: "sensitive-guard", rule: sensitive }; diff --git a/server/drivers/claude-catalog.test.ts b/server/drivers/claude-catalog.test.ts index 398e234b0..3d405ee5a 100644 --- a/server/drivers/claude-catalog.test.ts +++ b/server/drivers/claude-catalog.test.ts @@ -39,6 +39,19 @@ describe("readClaudeModelCatalog", () => { ], }); }); + + it("does not list settings.model as a Custom leftover", () => { + const home = mkdtempSync(join(tmpdir(), "omb-claude-leftover-")); + scratchDirs.push(home); + const dir = join(home, ".claude"); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, "settings.json"), + JSON.stringify({ model: "orcarouter/Qwen3.8-27B-Uncensored-GGUF" }), + ); + + expect(readClaudeModelCatalog({ HOME: home })).toEqual(STATIC_CLAUDE_MODELS); + }); }); describe("ClaudeDriver catalog", () => { diff --git a/server/drivers/claude.test.ts b/server/drivers/claude.test.ts index c4428eb6d..1387465c1 100644 --- a/server/drivers/claude.test.ts +++ b/server/drivers/claude.test.ts @@ -250,6 +250,36 @@ describe("ClaudeDriver turns (fake CLI)", () => { expect(seen.env.ANTHROPIC_AUTH_TOKEN).toBe("unsloth-secret"); }); + it("injects a leftover API id when a local host is serving that model", async () => { + const previousFetch = globalThis.fetch; + globalThis.fetch = (async (url: string | URL) => { + if (String(url).includes(":8888")) { + return new Response(JSON.stringify({ data: [{ id: "orcarouter/Qwen3.8-27B-Uncensored-GGUF" }] }), { status: 200 }); + } + return new Response("nope", { status: 500 }); + }) as typeof fetch; + try { + await create(undefined, { UNSLOTH_STUDIO_AUTH_TOKEN: "unsloth-secret" }); + const dump = join(scratch, "dump-leftover.json"); + process.env.FAKE_CLAUDE_DUMP = dump; + + await instance.adapter.sendTurn({ + threadId: "t-leftover-local", + text: "hi", + model: "orcarouter/Qwen3.8-27B-Uncensored-GGUF", + }); + await recorder.until((e) => e.type === "turn.completed"); + + const seen = JSON.parse(readFileSync(dump, "utf8")); + expect(seen.argv[seen.argv.indexOf("--model") + 1]).toBe("orcarouter/Qwen3.8-27B-Uncensored-GGUF"); + expect(seen.env.ANTHROPIC_BASE_URL).toBe("http://127.0.0.1:8888"); + expect(seen.env.ANTHROPIC_AUTH_TOKEN).toBe("unsloth-secret"); + expect(seen.env.ANTHROPIC_API_KEY).toBe("unsloth-secret"); + } finally { + globalThis.fetch = previousFetch; + } + }); + it("mounts the agents comms proxy as an MCP server and pre-allows its tools", async () => { await create(); const dump = join(scratch, "dump.json"); diff --git a/server/drivers/claude.ts b/server/drivers/claude.ts index 133b4a1e9..73b57ad3a 100644 --- a/server/drivers/claude.ts +++ b/server/drivers/claude.ts @@ -30,7 +30,13 @@ import type { } from "../contracts.ts"; import { computerProxyEnv } from "../container-computer.ts"; import { newEventId, newId } from "../contracts.ts"; -import { applyClaudeInject, mergeLocalInject } from "./local-inject.ts"; +import { + applyClaudeInject, + decodeInjectId, + mergeLocalInject, + probeLocalInjects, + resolveInjectId, +} from "./local-inject.ts"; import { appendNative } from "./native.ts"; import { SPAWNED_PROXIES } from "../proxy-paths.ts"; @@ -102,6 +108,19 @@ export const STATIC_CLAUDE_MODELS: ModelCatalog = { const CLAUDE_MODEL_ID = /^[a-z0-9][a-z0-9._:/-]*$/i; +/** Rewrite a leftover API slug (`orcarouter/Qwen…`) to `host::model` when a + * local host is serving it, so the turn injects instead of asking for /login. + * Official cloud ids and already-encoded inject ids skip the probe. */ +async function resolveClaudeTurnModel( + model: string | null | undefined, + env: Record, +): Promise { + if (!model || decodeInjectId(model) || STATIC_CLAUDE_MODELS.options.some((option) => option.id === model)) { + return model; + } + return resolveInjectId(model, await probeLocalInjects(env)) ?? model; +} + function claudeConfigDir(env: Record): string { if (env.CLAUDE_CONFIG_DIR) return env.CLAUDE_CONFIG_DIR; return join(env.HOME || env.USERPROFILE || homedir(), ".claude"); @@ -122,7 +141,11 @@ function extrasFromUnknown(value: unknown): Array<{ id: string; label: string }> }); } -/** Extra ids from ~/.claude/settings.json. Official cloud rows stay untagged. */ +/** Extra ids from ~/.claude/settings.json. Official cloud rows stay untagged. + * `model` is Claude Code's last-used slug, not a catalog — listing it as + * Custom put a non-inject id in the picker and the turn then had no + * ANTHROPIC_API_KEY ("Not logged in · Please run /login"). Live injects + * come from mergeLocalInject. */ export function readClaudeModelCatalog(env: Record = process.env) { let settings: Record = {}; try { @@ -139,7 +162,6 @@ export function readClaudeModelCatalog(env: Record = const nestedEnv = settings.env && typeof settings.env === "object" ? (settings.env as Record) : {}; const envModel = nestedEnv.ANTHROPIC_MODEL ?? env.ANTHROPIC_MODEL; if (typeof envModel === "string") extras.push(...extrasFromUnknown([envModel])); - if (typeof settings.model === "string") extras.push(...extrasFromUnknown([settings.model])); const options = STATIC_CLAUDE_MODELS.options.map((option) => ({ ...option })); const seen = new Set(options.map((option) => option.id)); @@ -402,7 +424,8 @@ export const ClaudeDriver: ProviderDriver = { if (sessionId) args.push("--resume", sessionId); else args.push("--session-id", newSessionId!); const turnEnvironment: NodeJS.ProcessEnv = { ...process.env, ...input.environment }; - const injected = applyClaudeInject({ ...turnEnvironment }, turn.model); + const turnModel = await resolveClaudeTurnModel(turn.model, turnEnvironment); + const injected = applyClaudeInject({ ...turnEnvironment }, turnModel); if (injected.model) args.push("--model", injected.model); if (turn.effort) args.push("--effort", turn.effort); if (turn.system) args.push("--append-system-prompt", turn.system); @@ -505,7 +528,7 @@ export const ClaudeDriver: ProviderDriver = { args.push("--allowedTools", allowed.join(",")); } - const env = claudeEnvironment(turn.model, turnEnvironment); + const env = claudeEnvironment(turnModel, turnEnvironment); const child = spawnCli(config.cli, args, { cwd: turn.cwd ?? homedir(), diff --git a/server/drivers/codex.ts b/server/drivers/codex.ts index a60111544..433a77a52 100644 --- a/server/drivers/codex.ts +++ b/server/drivers/codex.ts @@ -542,6 +542,7 @@ export const CodexDriver: ProviderDriver = { capabilities: { sessionModelSwitch: "unsupported", computerMcp: true, + localComputerMcp: true, composioMcp: true, agentsMcp: true, phoneMcp: true, diff --git a/server/drivers/local-inject.test.ts b/server/drivers/local-inject.test.ts index c712f7c45..5a74cac6e 100644 --- a/server/drivers/local-inject.test.ts +++ b/server/drivers/local-inject.test.ts @@ -24,6 +24,7 @@ import { loadedIdsFromPayloads, LOCAL_HOSTS, mergeLocalInject, + resolveInjectId, } from "./local-inject.ts"; const scratchDirs: string[] = []; @@ -46,6 +47,36 @@ describe("inject ids", () => { }); }); +describe("resolveInjectId", () => { + it("keeps an already-encoded inject id", () => { + expect(resolveInjectId("unsloth::orcarouter/Qwen3.8-27B-Uncensored-GGUF", [])).toBe( + "unsloth::orcarouter/Qwen3.8-27B-Uncensored-GGUF", + ); + }); + + it("maps a leftover API id onto the live host:: row", () => { + expect( + resolveInjectId("orcarouter/Qwen3.8-27B-Uncensored-GGUF", [ + { + id: "unsloth::orcarouter/Qwen3.8-27B-Uncensored-GGUF", + host: "unsloth", + model: "orcarouter/Qwen3.8-27B-Uncensored-GGUF", + label: "orcarouter/Qwen3.8-27B-Uncensored-GGUF (Unsloth)", + }, + ]), + ).toBe("unsloth::orcarouter/Qwen3.8-27B-Uncensored-GGUF"); + }); + + it("prefers a loaded host when several serve the same API id", () => { + expect( + resolveInjectId("GLM-5.2-fp8", [ + { id: "omlx::GLM-5.2-fp8", host: "omlx", model: "GLM-5.2-fp8", label: "GLM-5.2-fp8 (oMLX)" }, + { id: "lmstudio::GLM-5.2-fp8", host: "lmstudio", model: "GLM-5.2-fp8", label: "GLM-5.2-fp8 (LM Studio)", loaded: true }, + ]), + ).toBe("lmstudio::GLM-5.2-fp8"); + }); +}); + describe("loadedIdsFromPayloads", () => { const omlx = LOCAL_HOSTS.find((host) => host.id === "omlx")!; const ollama = LOCAL_HOSTS.find((host) => host.id === "ollama")!; @@ -182,6 +213,29 @@ describe("mergeLocalInject", () => { expect(catalog.options.some((option) => option.id === "omlx::GLM-5.2-fp8" && option.custom)).toBe(true); expect(catalog.options.some((option) => option.id.includes("nomic"))).toBe(false); }); + + it("drops a leftover custom API id that a live inject already covers", async () => { + const catalog = await mergeLocalInject( + { + default: "claude-sonnet-5", + options: [ + { id: "claude-sonnet-5", label: "Claude Sonnet 5" }, + { id: "orcarouter/Qwen3.8-27B-Uncensored-GGUF", label: "orcarouter/Qwen3.8-27B-Uncensored-GGUF", custom: true }, + ], + }, + { VITEST: "true", OPENMAUSBOT_PROBE_LOCAL_INJECT: "1" }, + async (url) => { + if (String(url).includes(":8888")) { + return new Response(JSON.stringify({ data: [{ id: "orcarouter/Qwen3.8-27B-Uncensored-GGUF" }] }), { status: 200 }); + } + return new Response("nope", { status: 500 }); + }, + ); + expect(catalog.options.some((option) => option.id === "orcarouter/Qwen3.8-27B-Uncensored-GGUF")).toBe(false); + expect(catalog.options.some((option) => option.id === "unsloth::orcarouter/Qwen3.8-27B-Uncensored-GGUF" && option.custom)).toBe( + true, + ); + }); }); describe("applyOpenAIInject", () => { diff --git a/server/drivers/local-inject.ts b/server/drivers/local-inject.ts index a979b63aa..2b66074c2 100644 --- a/server/drivers/local-inject.ts +++ b/server/drivers/local-inject.ts @@ -62,6 +62,23 @@ export function injectedApiModel(id: string | null | undefined): string | null { return decodeInjectId(id)?.model ?? null; } +/** + * Map a picker / leftover API id onto a live `host::model` inject id. + * Claude Code's settings.model is the last slug it used (e.g. + * `orcarouter/Qwen3.8-27B-Uncensored-GGUF`) and is not host-encoded, so a + * Custom pick of that leftover would otherwise skip inject and demand /login. + */ +export function resolveInjectId( + modelId: string | null | undefined, + extras: readonly InjectedModel[], +): string | null | undefined { + if (!modelId) return modelId; + if (decodeInjectId(modelId)) return modelId; + const matches = extras.filter((row) => row.id === modelId || row.model === modelId); + const match = matches.find((row) => row.loaded) ?? matches[0]; + return match?.id ?? modelId; +} + /** Anthropic-compatible base (Claude Code wants this without a trailing /v1). */ export function anthropicBaseUrl(host: LocalHost): string { return host.baseUrl.replace(/\/v1\/?$/, ""); @@ -317,7 +334,12 @@ export async function mergeLocalInject( if (vitest === "true" && probe !== "1") return catalog; const extras = await probeLocalInjects(env, fetchImpl); if (!extras.length) return catalog; - const options = catalog.options.map((option) => ({ ...option })); + const liveApiIds = new Set(extras.map((extra) => extra.model)); + // A settings leftover that is just the API id of a live inject is not a + // second model — Custom should only offer the host:: row. + const options = catalog.options + .filter((option) => decodeInjectId(option.id) || !option.custom || !liveApiIds.has(option.id)) + .map((option) => ({ ...option })); const seen = new Set(options.map((option) => option.id)); for (const extra of extras) { const existing = options.find((option) => option.id === extra.id); diff --git a/server/index.test.ts b/server/index.test.ts index 1c1664594..6d584ad4e 100644 --- a/server/index.test.ts +++ b/server/index.test.ts @@ -812,26 +812,17 @@ describe("harness HTTP API", () => { expect(after.modelSelection.effort).toBeUndefined(); }); - it("turns off bot Auto mode when local computer beta is selected", async () => { + it("keeps Auto mode when local computer is selected after a warning", async () => { const created = await api("POST", "/api/bots"); const bot = created.body.bot; expect((await api("PATCH", `/api/bots/${bot.id}`, { autoApprove: true })).body.bot.autoApprove).toBe( true, ); const local = await api("PATCH", `/api/bots/${bot.id}`, { computer: "local" }); - expect(local.body.bot).toMatchObject({ computer: "local", autoApprove: false }); - const rejected = await api("PATCH", `/api/bots/${bot.id}`, { autoApprove: true }); - expect(rejected.status).toBe(400); - expect(rejected.body.error).toContain("local computer beta"); - - const cloud = await api("PATCH", `/api/bots/${bot.id}`, { computer: "cloud" }); - expect(cloud.body.bot.computer).toBe("cloud"); - const simultaneous = await api("PATCH", `/api/bots/${bot.id}`, { - computer: "local", - autoApprove: true, - }); - expect(simultaneous.status).toBe(400); - expect(simultaneous.body.error).toContain("local computer beta"); + expect(local.body.bot).toMatchObject({ computer: "local", autoApprove: true }); + const enabled = await api("PATCH", `/api/bots/${bot.id}`, { autoApprove: true }); + expect(enabled.status).toBe(200); + expect(enabled.body.bot.autoApprove).toBe(true); await api("DELETE", `/api/bots/${bot.id}`); }); diff --git a/server/index.ts b/server/index.ts index a52194e2c..a45474e4e 100644 --- a/server/index.ts +++ b/server/index.ts @@ -793,10 +793,7 @@ bus.subscribe((event: RuntimeEvent) => { allowKey: event.approvalScope ? undefined : approvalKey(tool, summary, event.approvalScope), - held: - event.approvalScope === "local-computer" - ? "Local computer actions always require your approval in this beta." - : "Auto mode couldn't answer this one.", + held: "Auto mode couldn't answer this one.", approvalScope: event.approvalScope, }, }); @@ -838,11 +835,9 @@ bus.subscribe((event: RuntimeEvent) => { : undefined, // in auto mode a card can only mean the guard stopped it — say so held: - permission && event.approvalScope === "local-computer" - ? "Local computer actions always require your approval in this beta." - : permission && asker?.autoApprove - ? "This looked destructive, so auto mode stopped to ask." - : undefined, + permission && asker?.autoApprove + ? "This looked destructive, so auto mode stopped to ask." + : undefined, approvalScope: event.approvalScope, }, }); @@ -3186,7 +3181,6 @@ const server = createServer(async (req, res) => { if (body.cloudBackend !== undefined && !["box", "vps"].includes(String(body.cloudBackend))) { return json(res, 400, { error: "cloudBackend must be box or vps" }); } - const effectiveComputer = body.computer ?? existingBot?.computer; if (body.chiefOfStaff !== undefined && typeof body.chiefOfStaff !== "boolean") { return json(res, 400, { error: "chiefOfStaff must be true or false" }); } @@ -3207,9 +3201,6 @@ const server = createServer(async (req, res) => { // still answer .includes() — with substring matches, not tool names if (body.autoApprove !== undefined) { if (typeof body.autoApprove !== "boolean") return json(res, 400, { error: "autoApprove must be true or false" }); - if (body.autoApprove === true && effectiveComputer === "local") { - return json(res, 400, { error: "Auto mode is unavailable while this bot uses the local computer beta" }); - } patch.autoApprove = body.autoApprove; } if (body.approvePeerComms !== undefined) { @@ -3224,9 +3215,6 @@ const server = createServer(async (req, res) => { } patch.alwaysAllow = [...new Set(body.alwaysAllow as string[])].slice(0, 200); } - if (effectiveComputer === "local" && body.autoApprove === undefined && existingBot?.autoApprove) { - patch.autoApprove = false; - } if (existingBot?.computer === "local" && body.computer !== undefined && body.computer !== "local") { await registry .get(existingBot.modelSelection.instanceId) diff --git a/src/components/ComputerPanel.tsx b/src/components/ComputerPanel.tsx index cc01df839..80efd98e7 100644 --- a/src/components/ComputerPanel.tsx +++ b/src/components/ComputerPanel.tsx @@ -29,11 +29,14 @@ import { RoutineEditor } from "./RoutinesPage"; import { AndroidDevicePanel, useAndroidUsbDevices } from "./AndroidDevicePanel"; import { LocalScreenPreview } from "./LocalScreenPreview"; import { LinuxLocalControl } from "./LinuxLocalControl"; +import { MacLocalControl } from "./MacLocalControl"; +import { LocalComputerAutoWarning } from "./LocalComputerAutoWarning"; import { autoSelectsLocalComputer, instanceSupportsLocalComputer, linuxAutoDescription, localComputerDisabledReason, + localComputerSelectable, } from "@/lib/local-computer"; async function api(path: string, init?: RequestInit): Promise { @@ -91,7 +94,8 @@ export function ComputerPanel({ bot }: { bot: Bot }) { const localAvailable = capabilities.localComputer.available; const isLinux = capabilities.host.platform === "linux"; const providerSupportsLocal = instanceSupportsLocalComputer(state.instances, bot); - const localSelectable = localAvailable && providerSupportsLocal; + const localSelectable = localComputerSelectable({ capabilities, providerSupportsLocal }); + const [localAutoWarning, setLocalAutoWarning] = useState(false); const localDisabledReason = localComputerDisabledReason({ capabilities, providerSupportsLocal }); const [phase, setPhase] = useState("checking"); const [boxState, setBoxState] = useState(null); @@ -165,7 +169,7 @@ export function ComputerPanel({ bot }: { bot: Bot }) { if (!providerSupportsLocal) { setError("This model engine cannot control this computer. Choose Claude or an ACP engine."); } - setPhase(capabilitiesReady && localSelectable ? "local" : "local-unavailable"); + setPhase(capabilitiesReady && localAvailable && providerSupportsLocal ? "local" : "local-unavailable"); return; } if (bot.computer === "vm") { @@ -497,6 +501,7 @@ export function ComputerPanel({ bot }: { bot: Bot }) { } satisfies Record, string>; return ( + <> + setLocalAutoWarning(false)} + onConfirm={() => { + dispatch({ type: "updateBot", botId: bot.id, patch: { computer: "local" } }); + setLocalAutoWarning(false); + }} + /> + ); } diff --git a/src/components/LocalComputerAutoWarning.tsx b/src/components/LocalComputerAutoWarning.tsx new file mode 100644 index 000000000..82883cdae --- /dev/null +++ b/src/components/LocalComputerAutoWarning.tsx @@ -0,0 +1,76 @@ +import { useEffect, useRef } from "react"; +import { AlertTriangle } from "lucide-react"; + +export const LOCAL_COMPUTER_AUTO_WARNING = + "Auto mode will let this bot click, type, and run tools on this computer without asking first. Destructive and sensitive actions still stop. Continue only if you are watching."; + +export function LocalComputerAutoWarning({ + open, + onCancel, + onConfirm, +}: { + open: boolean; + onCancel: () => void; + onConfirm: () => void; +}) { + const confirmRef = useRef(null); + + useEffect(() => { + if (!open) return; + confirmRef.current?.focus(); + const onKey = (event: KeyboardEvent) => { + if (event.key === "Escape") { + event.preventDefault(); + onCancel(); + } + }; + window.addEventListener("keydown", onKey); + return () => window.removeEventListener("keydown", onKey); + }, [open, onCancel]); + + if (!open) return null; + + return ( +
event.target === event.currentTarget && onCancel()} + > +
+
+ +
+

+ Allow Auto mode on this computer? +

+

+ {LOCAL_COMPUTER_AUTO_WARNING} +

+
+
+
+ + +
+
+
+ ); +} diff --git a/src/components/MacLocalControl.tsx b/src/components/MacLocalControl.tsx new file mode 100644 index 000000000..0d1408693 --- /dev/null +++ b/src/components/MacLocalControl.tsx @@ -0,0 +1,56 @@ +import { useState } from "react"; +import { AlertTriangle, Loader2, Shield } from "lucide-react"; +import { useDesktopCapabilities } from "./DesktopCapabilities"; + +export function MacLocalControl() { + const { capabilities } = useDesktopCapabilities(); + const [pending, setPending] = useState(false); + const [error, setError] = useState(null); + + if (capabilities.host.platform !== "darwin") return null; + if (capabilities.localComputer.available) return null; + + const retry = async () => { + setPending(true); + setError(null); + try { + await window.ogb?.permOpenSettings?.("accessibility"); + await window.ogb?.permOpenSettings?.("screen"); + await window.ogb?.localControl?.retry(); + } catch (reason) { + setError(reason instanceof Error ? reason.message : String(reason)); + } finally { + setPending(false); + } + }; + + return ( +
+
+ +
+
Allow control of this computer
+

+ OpenMausBot needs Accessibility and Screen Recording in System Settings before a bot can + use this Mac. After you grant both, click Retry — macOS may still ask you to relaunch the app. +

+ {error && ( +
+ + {error} +
+ )} + +
+
+
+ ); +} diff --git a/src/components/SettingsPanel.tsx b/src/components/SettingsPanel.tsx index f314e7352..574627a7e 100644 --- a/src/components/SettingsPanel.tsx +++ b/src/components/SettingsPanel.tsx @@ -15,7 +15,8 @@ import { cn } from "@/lib/cn"; import { requestNotificationPermission } from "@/lib/notify"; import { botUsage, costCaption, formatTokens, formatUsd, hasFiniteCost } from "@/lib/usage"; import { shortPath } from "@/lib/short-path"; -import { instanceSupportsLocalComputer, localComputerDisabledReason } from "@/lib/local-computer"; +import { instanceSupportsLocalComputer, localComputerDisabledReason, localComputerSelectable } from "@/lib/local-computer"; +import { LocalComputerAutoWarning } from "./LocalComputerAutoWarning"; function Field({ label, @@ -322,7 +323,8 @@ export function SettingsPanel({ bot }: { bot: Bot }) { const [voicesLoading, setVoicesLoading] = useState(false); const { capabilities } = useDesktopCapabilities(); const providerSupportsLocal = instanceSupportsLocalComputer(state.instances, bot); - const localSelectable = capabilities.localComputer.available && providerSupportsLocal; + const localSelectable = localComputerSelectable({ capabilities, providerSupportsLocal }); + const [localAutoWarning, setLocalAutoWarning] = useState<"auto" | "local" | null>(null); const localDisabledReason = localComputerDisabledReason({ capabilities, providerSupportsLocal }); const patch = ( p: Partial< @@ -373,6 +375,7 @@ export function SettingsPanel({ bot }: { bot: Bot }) { }, [state.config?.tts?.configured]); return ( + <> + setLocalAutoWarning(null)} + onConfirm={() => { + if (localAutoWarning === "auto") patch({ autoApprove: true }); + if (localAutoWarning === "local") patch({ computer: "local" }); + setLocalAutoWarning(null); + }} + /> + ); } diff --git a/src/lib/local-computer.test.ts b/src/lib/local-computer.test.ts index 101a64502..f5809d871 100644 --- a/src/lib/local-computer.test.ts +++ b/src/lib/local-computer.test.ts @@ -4,6 +4,7 @@ import { autoSelectsLocalComputer, instanceSupportsLocalComputer, linuxAutoDescription, + localComputerSelectable, } from "./local-computer"; describe("local computer UI eligibility", () => { @@ -24,6 +25,30 @@ describe("local computer UI eligibility", () => { bot, ), ).toBe(false); + expect( + instanceSupportsLocalComputer( + [{ ...instances[0], capabilities: { computerMcp: true } }] as InstanceInfo[], + bot, + ), + ).toBe(true); + }); + + it("keeps This computer selectable on macOS before CUA is granted", () => { + const capabilities = { + host: { platform: "darwin" as const }, + localComputer: { available: false }, + } as DesktopCapabilities; + expect(localComputerSelectable({ capabilities, providerSupportsLocal: true })).toBe(true); + expect(localComputerSelectable({ capabilities, providerSupportsLocal: false })).toBe(false); + expect( + localComputerSelectable({ + capabilities: { + host: { platform: "linux" as const }, + localComputer: { available: false }, + } as DesktopCapabilities, + providerSupportsLocal: true, + }), + ).toBe(false); }); it("states that Linux Auto never selects this computer", () => { diff --git a/src/lib/local-computer.ts b/src/lib/local-computer.ts index 480adbb4a..43a5759f6 100644 --- a/src/lib/local-computer.ts +++ b/src/lib/local-computer.ts @@ -4,10 +4,26 @@ export function instanceSupportsLocalComputer( instances: InstanceInfo[], bot: Pick, ): boolean { - return ( - instances.find((instance) => instance.instanceId === bot.modelSelection.instanceId)?.capabilities - ?.localComputerMcp === true - ); + const capabilities = instances.find( + (instance) => instance.instanceId === bot.modelSelection.instanceId, + )?.capabilities; + return capabilities?.localComputerMcp === true || capabilities?.computerMcp === true; +} + +/** Whether the Runs-on “This computer” control should be clickable. + * macOS keeps the destination available even before CUA has a grant, so + * the user can pick it and then approve Accessibility / Screen Recording + * instead of finding a grayed-out button. */ +export function localComputerSelectable({ + capabilities, + providerSupportsLocal, +}: { + capabilities: DesktopCapabilities; + providerSupportsLocal: boolean; +}): boolean { + if (!providerSupportsLocal) return false; + if (capabilities.localComputer.available) return true; + return capabilities.host.platform === "darwin"; } export function localComputerDisabledReason({ diff --git a/src/state/store.tsx b/src/state/store.tsx index 6fce592f1..d36d120b7 100644 --- a/src/state/store.tsx +++ b/src/state/store.tsx @@ -830,10 +830,7 @@ export function reducer(state: AppState, action: Action): AppState { ), } : animated; - return updateBot(next, action.botId, (b) => { - const merged = { ...b, ...action.patch }; - return merged.computer === "local" ? { ...merged, autoApprove: false } : merged; - }); + return updateBot(next, action.botId, (b) => ({ ...b, ...action.patch })); } case "threadActive": { const bot = state.bots.find((b) => b.threadId === action.threadId); diff --git a/src/types/ogb.d.ts b/src/types/ogb.d.ts index 1c7fe1295..e7412e3b6 100644 --- a/src/types/ogb.d.ts +++ b/src/types/ogb.d.ts @@ -76,8 +76,8 @@ declare global { permStatus(): Promise<{ mic: string }>; /** Triggers the macOS microphone prompt; resolves true when granted. */ permRequestMic(): Promise; - /** Opens System Settings on a privacy pane: mic|screen|speech. */ - permOpenSettings(pane: "mic" | "screen" | "speech"): Promise; + /** Opens System Settings on a privacy pane: mic|screen|speech|accessibility. */ + permOpenSettings(pane: "mic" | "screen" | "speech" | "accessibility"): Promise; /** Copies an engine install command and opens a blank terminal. False * when no terminal could be launched; the clipboard still has it. */ openInstallTerminal?(command: string): Promise; From 1a2cd0cbab3320c9c26dfccf3e41faddb0263528 Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 20 Aug 2026 20:46:10 +0200 Subject: [PATCH 17/52] Decode TOML basic-string unicode escapes in Kimi headings. \u0035 in a quoted table key is 5, not the letters u0035, so an existing [models."omlx/GLM-\u0035.2-fp8"] matches the inject alias and is patched instead of duplicating the table. --- server/drivers/acp/kimi.ts | 33 +++++++++++++++++++++++++++-- server/drivers/local-inject.test.ts | 17 +++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/server/drivers/acp/kimi.ts b/server/drivers/acp/kimi.ts index c55334c9d..a3d2a3759 100644 --- a/server/drivers/acp/kimi.ts +++ b/server/drivers/acp/kimi.ts @@ -69,6 +69,34 @@ function stripTomlLineComment(line: string): string { return line; } +/** Decode a TOML basic-string escape at `text[i]` (`i` points at the `\\`). */ +function takeTomlBasicEscape(text: string, i: number): { value: string; next: number } { + const code = text[i + 1]; + if (code === "u") { + const hex = text.slice(i + 2, i + 6); + if (/^[0-9a-fA-F]{4}$/.test(hex)) { + return { value: String.fromCharCode(parseInt(hex, 16)), next: i + 6 }; + } + } + if (code === "U") { + const hex = text.slice(i + 2, i + 10); + if (/^[0-9a-fA-F]{8}$/.test(hex)) { + const point = parseInt(hex, 16); + return { value: point <= 0x10ffff ? String.fromCodePoint(point) : "", next: i + 10 }; + } + } + const named: Record = { + b: "\b", + t: "\t", + n: "\n", + f: "\f", + r: "\r", + '"': '"', + "\\": "\\", + }; + return { value: named[code ?? ""] ?? code ?? "", next: i + 2 }; +} + /** Canonical `a.b.c` form of a `[table]` heading, quotes and comments removed. */ function canonicalizeTomlHeading(heading: string): string | null { const trimmed = stripTomlLineComment(heading).trim(); @@ -88,8 +116,9 @@ function canonicalizeTomlHeading(heading: string): string | null { let value = ""; while (i < inner.length && inner[i] !== q) { if (q === '"' && inner[i] === "\\") { - value += inner[i + 1] ?? ""; - i += 2; + const taken = takeTomlBasicEscape(inner, i); + value += taken.value; + i = taken.next; continue; } value += inner[i]; diff --git a/server/drivers/local-inject.test.ts b/server/drivers/local-inject.test.ts index c712f7c45..5aff8ab89 100644 --- a/server/drivers/local-inject.test.ts +++ b/server/drivers/local-inject.test.ts @@ -449,6 +449,23 @@ describe("ensureKimiInjectAlias", () => { expect(text).toMatch(/\[\[hooks\]\]\s*event = "Stop"/); }); + it("treats a unicode-escaped model key as the same table as the literal alias", () => { + const home = mkdtempSync(join(tmpdir(), "omb-kimi-unicode-")); + scratchDirs.push(home); + const root = join(home, ".kimi-code"); + mkdirSync(root, { recursive: true }); + writeFileSync( + join(root, "config.toml"), + ['[models."omlx/GLM-\\u0035.2-fp8"]', 'provider = "omlx"', 'model = "GLM-5.2-fp8"', ""].join("\n"), + ); + ensureKimiInjectAlias("omlx::GLM-5.2-fp8", { HOME: home }); + const text = readFileSync(join(root, "config.toml"), "utf8"); + expect(text.match(/\[models\./g)?.length).toBe(1); + expect(text).toContain("GLM-\\u0035.2-fp8"); + expect(text).toContain('protocol = "openai"'); + expect(text).toContain("max_context_size = 262144"); + }); + it("treats whitespace around dotted heading keys as the same table", () => { const home = mkdtempSync(join(tmpdir(), "omb-kimi-dots-")); scratchDirs.push(home); From 61a8b48bc2d576fac6d8cdbf5f688821f20187cc Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 20 Aug 2026 20:46:10 +0200 Subject: [PATCH 18/52] Decode TOML basic-string unicode escapes in Kimi headings. \u0035 in a quoted table key is 5, not the letters u0035, so an existing [models."omlx/GLM-\u0035.2-fp8"] matches the inject alias and is patched instead of duplicating the table. --- server/drivers/acp/kimi.ts | 33 +++++++++++++++++++++++++++-- server/drivers/local-inject.test.ts | 17 +++++++++++++++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/server/drivers/acp/kimi.ts b/server/drivers/acp/kimi.ts index c55334c9d..a3d2a3759 100644 --- a/server/drivers/acp/kimi.ts +++ b/server/drivers/acp/kimi.ts @@ -69,6 +69,34 @@ function stripTomlLineComment(line: string): string { return line; } +/** Decode a TOML basic-string escape at `text[i]` (`i` points at the `\\`). */ +function takeTomlBasicEscape(text: string, i: number): { value: string; next: number } { + const code = text[i + 1]; + if (code === "u") { + const hex = text.slice(i + 2, i + 6); + if (/^[0-9a-fA-F]{4}$/.test(hex)) { + return { value: String.fromCharCode(parseInt(hex, 16)), next: i + 6 }; + } + } + if (code === "U") { + const hex = text.slice(i + 2, i + 10); + if (/^[0-9a-fA-F]{8}$/.test(hex)) { + const point = parseInt(hex, 16); + return { value: point <= 0x10ffff ? String.fromCodePoint(point) : "", next: i + 10 }; + } + } + const named: Record = { + b: "\b", + t: "\t", + n: "\n", + f: "\f", + r: "\r", + '"': '"', + "\\": "\\", + }; + return { value: named[code ?? ""] ?? code ?? "", next: i + 2 }; +} + /** Canonical `a.b.c` form of a `[table]` heading, quotes and comments removed. */ function canonicalizeTomlHeading(heading: string): string | null { const trimmed = stripTomlLineComment(heading).trim(); @@ -88,8 +116,9 @@ function canonicalizeTomlHeading(heading: string): string | null { let value = ""; while (i < inner.length && inner[i] !== q) { if (q === '"' && inner[i] === "\\") { - value += inner[i + 1] ?? ""; - i += 2; + const taken = takeTomlBasicEscape(inner, i); + value += taken.value; + i = taken.next; continue; } value += inner[i]; diff --git a/server/drivers/local-inject.test.ts b/server/drivers/local-inject.test.ts index 5a74cac6e..21516fd91 100644 --- a/server/drivers/local-inject.test.ts +++ b/server/drivers/local-inject.test.ts @@ -503,6 +503,23 @@ describe("ensureKimiInjectAlias", () => { expect(text).toMatch(/\[\[hooks\]\]\s*event = "Stop"/); }); + it("treats a unicode-escaped model key as the same table as the literal alias", () => { + const home = mkdtempSync(join(tmpdir(), "omb-kimi-unicode-")); + scratchDirs.push(home); + const root = join(home, ".kimi-code"); + mkdirSync(root, { recursive: true }); + writeFileSync( + join(root, "config.toml"), + ['[models."omlx/GLM-\\u0035.2-fp8"]', 'provider = "omlx"', 'model = "GLM-5.2-fp8"', ""].join("\n"), + ); + ensureKimiInjectAlias("omlx::GLM-5.2-fp8", { HOME: home }); + const text = readFileSync(join(root, "config.toml"), "utf8"); + expect(text.match(/\[models\./g)?.length).toBe(1); + expect(text).toContain("GLM-\\u0035.2-fp8"); + expect(text).toContain('protocol = "openai"'); + expect(text).toContain("max_context_size = 262144"); + }); + it("treats whitespace around dotted heading keys as the same table", () => { const home = mkdtempSync(join(tmpdir(), "omb-kimi-dots-")); scratchDirs.push(home); From 4a1cb62406ee0737474fe59b484fd5c69eb5c63f Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 20 Aug 2026 20:58:42 +0200 Subject: [PATCH 19/52] Address CodeRabbit notes on the Mac local-computer Auto commit. - Destroy a failed embedded CUA host before falling back to standalone - Refuse a one-arch CUA stage unless PARTIAL=1, matching dual-arch packaging - Open System Settings and Retry are separate; retry after the window refocuses - Sort usage by finite cost only (missing/NaN/Infinity last) - Reset TOML string mode at newlines so a stray quote cannot hide later tables Unclassified GUI clicks still auto-approve when Auto is on after the warning; default-denying every click would restore the ban this PR removes. Destructive and sensitive actions still stop. --- electron/cua.mjs | 29 +++++++++---- scripts/prepare-cua.mjs | 11 ++++- server/auto-approve.ts | 3 +- server/drivers/acp/kimi.ts | 10 +++++ src/components/MacLocalControl.tsx | 66 +++++++++++++++++++++++------- src/components/UsageSection.tsx | 8 +++- 6 files changed, 99 insertions(+), 28 deletions(-) diff --git a/electron/cua.mjs b/electron/cua.mjs index 76bfcc73c..7724a2de4 100644 --- a/electron/cua.mjs +++ b/electron/cua.mjs @@ -168,15 +168,26 @@ async function startEmbedded(binary) { ].filter(Boolean).join(" and "); throw new Error(`${missing || "macOS permissions"} required; grant access in System Settings and restart OpenMausBot`); } - embeddedHost = new sdk.EmbeddedCuaDriverHost(binary, HOST_BUNDLE_ID); - const conn = await embeddedHost.start(); - return { - mode: "embedded", - socketPath: conn.socketPath, - mcpCommand: binary, - mcpArgs: ["mcp", "--embedded", "--socket", conn.socketPath], - mcpEnv: { ...CUA_ENV, CUA_DRIVER_EMBEDDED: "1", CUA_DRIVER_HOST_BUNDLE_ID: HOST_BUNDLE_ID }, - }; + const host = new sdk.EmbeddedCuaDriverHost(binary, HOST_BUNDLE_ID); + try { + const conn = await host.start(); + embeddedHost = host; + return { + mode: "embedded", + socketPath: conn.socketPath, + mcpCommand: binary, + mcpArgs: ["mcp", "--embedded", "--socket", conn.socketPath], + mcpEnv: { ...CUA_ENV, CUA_DRIVER_EMBEDDED: "1", CUA_DRIVER_HOST_BUNDLE_ID: HOST_BUNDLE_ID }, + }; + } catch (err) { + try { + await host.stop(); + } catch { + // startup already failed; stop is best-effort before destroy + } + host.uniffiDestroy?.(); + throw err; + } } export async function startCua() { diff --git a/scripts/prepare-cua.mjs b/scripts/prepare-cua.mjs index dcbe8b9fd..958f4cc84 100644 --- a/scripts/prepare-cua.mjs +++ b/scripts/prepare-cua.mjs @@ -107,10 +107,19 @@ if (!details.isFile() || (details.mode & 0o111) === 0) { // not on a user's Intel Mac); the SDK's dylib/.node are genuinely per-arch, // pulled from the two darwin native packages that pnpm installs because of // supportedArchitectures in package.json. -const MAC_ARCHES = (process.env.OPENMAUSBOT_CUA_ARCHES ?? "arm64,x64") +const DEFAULT_MAC_ARCHES = ["arm64", "x64"]; +const MAC_ARCHES = (process.env.OPENMAUSBOT_CUA_ARCHES ?? DEFAULT_MAC_ARCHES.join(",")) .split(",") .map((arch) => arch.trim()) .filter(Boolean); +if (process.env.OPENMAUSBOT_CUA_ARCHES && process.env.OPENMAUSBOT_CUA_ARCHES_PARTIAL !== "1") { + const missing = DEFAULT_MAC_ARCHES.filter((arch) => !MAC_ARCHES.includes(arch)); + if (missing.length) { + throw new Error( + `OPENMAUSBOT_CUA_ARCHES omits ${missing.join(", ")} but electron-builder packages both arm64 and x64. Set OPENMAUSBOT_CUA_ARCHES_PARTIAL=1 for a one-arch local stage.`, + ); + } +} const { stdout: archList } = await run("/usr/bin/lipo", ["-archs", binary]); for (const arch of MAC_ARCHES) { diff --git a/server/auto-approve.ts b/server/auto-approve.ts index 81a463ca4..bf83565fa 100644 --- a/server/auto-approve.ts +++ b/server/auto-approve.ts @@ -146,7 +146,8 @@ export function autoVerdict( } if (context?.scope === "local-computer" && !bot.autoApprove) { // Host control is not covered by a remembered always-allow grant. - // Auto mode can approve these after the user confirms a warning. + // After the Auto-on-this-computer warning, unclassified GUI actions + // (click/type) may auto-approve; destructive/sensitive still card. if (grant) return { approve: null, source: "local-computer-block", rule: grant.rule }; if (destructive) return { approve: null, source: "destructive-guard", rule: destructive }; if (sensitive) return { approve: null, source: "sensitive-guard", rule: sensitive }; diff --git a/server/drivers/acp/kimi.ts b/server/drivers/acp/kimi.ts index a3d2a3759..06e111bd8 100644 --- a/server/drivers/acp/kimi.ts +++ b/server/drivers/acp/kimi.ts @@ -185,6 +185,11 @@ function tomlTables(text: string): Array<{ name: string; headingStart: number; b continue; } if (mode === "basic") { + if (text[i] === "\n") { + mode = "out"; + i += 1; + continue; + } if (text[i] === "\\") { i += 2; continue; @@ -194,6 +199,11 @@ function tomlTables(text: string): Array<{ name: string; headingStart: number; b continue; } if (mode === "literal") { + if (text[i] === "\n") { + mode = "out"; + i += 1; + continue; + } if (text[i] === "'") mode = "out"; i += 1; continue; diff --git a/src/components/MacLocalControl.tsx b/src/components/MacLocalControl.tsx index 0d1408693..6be2870a7 100644 --- a/src/components/MacLocalControl.tsx +++ b/src/components/MacLocalControl.tsx @@ -1,21 +1,17 @@ -import { useState } from "react"; +import { useEffect, useState } from "react"; import { AlertTriangle, Loader2, Shield } from "lucide-react"; import { useDesktopCapabilities } from "./DesktopCapabilities"; export function MacLocalControl() { const { capabilities } = useDesktopCapabilities(); const [pending, setPending] = useState(false); + const [awaitingGrant, setAwaitingGrant] = useState(false); const [error, setError] = useState(null); - if (capabilities.host.platform !== "darwin") return null; - if (capabilities.localComputer.available) return null; - const retry = async () => { setPending(true); setError(null); try { - await window.ogb?.permOpenSettings?.("accessibility"); - await window.ogb?.permOpenSettings?.("screen"); await window.ogb?.localControl?.retry(); } catch (reason) { setError(reason instanceof Error ? reason.message : String(reason)); @@ -24,6 +20,36 @@ export function MacLocalControl() { } }; + const openSettings = async () => { + setError(null); + setAwaitingGrant(true); + try { + await window.ogb?.permOpenSettings?.("accessibility"); + await window.ogb?.permOpenSettings?.("screen"); + } catch (reason) { + setAwaitingGrant(false); + setError(reason instanceof Error ? reason.message : String(reason)); + } + }; + + useEffect(() => { + if (!awaitingGrant) return; + const onFocus = () => { + if (document.visibilityState !== "visible") return; + setAwaitingGrant(false); + void retry(); + }; + window.addEventListener("focus", onFocus); + document.addEventListener("visibilitychange", onFocus); + return () => { + window.removeEventListener("focus", onFocus); + document.removeEventListener("visibilitychange", onFocus); + }; + }, [awaitingGrant]); + + if (capabilities.host.platform !== "darwin") return null; + if (capabilities.localComputer.available) return null; + return (
@@ -40,15 +66,25 @@ export function MacLocalControl() { {error}
)} - +
+ + +
diff --git a/src/components/UsageSection.tsx b/src/components/UsageSection.tsx index 4540daeb0..05a965400 100644 --- a/src/components/UsageSection.tsx +++ b/src/components/UsageSection.tsx @@ -17,8 +17,12 @@ export function UsageSection() { return { bot, usage, billing: instance?.snapshot.billing }; }) .filter((r) => r.usage.turns > 0) - // money first, then volume - .sort((a, b) => (b.usage.costUsd ?? -1) - (a.usage.costUsd ?? -1) || b.usage.input + b.usage.output - (a.usage.input + a.usage.output)); + // money first, then volume. Non-finite/missing costs sort last. + .sort((a, b) => { + const costOf = (value: number | null | undefined) => + hasFiniteCost(value) ? value : Number.NEGATIVE_INFINITY; + return costOf(b.usage.costUsd) - costOf(a.usage.costUsd) || b.usage.input + b.usage.output - (a.usage.input + a.usage.output); + }); const total = sumUsage(rows.map((r) => r.usage)); const billings = new Set(rows.map((r) => r.billing)); From 4f59e7c1d4ff278711a9368f18873bfe5bed8fe2 Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 20 Aug 2026 21:09:20 +0200 Subject: [PATCH 20/52] Tighten CodeRabbit follow-ups: invalid TOML escapes, empty CUA arches, one-shot retry. Reject unknown/malformed/surrogate unicode escapes so they cannot canonicalize to another alias. Empty OPENMAUSBOT_CUA_ARCHES throws even with PARTIAL=1. Settings-return retry runs at most once if focus and visibility both fire. --- scripts/cua-mac-arches.mjs | 24 ++++++++++++++++++++++++ scripts/cua-mac-arches.test.mjs | 24 ++++++++++++++++++++++++ scripts/prepare-cua.mjs | 15 ++------------- server/drivers/acp/kimi.ts | 29 +++++++++++++++++++---------- server/drivers/local-inject.test.ts | 18 ++++++++++++++++++ src/components/MacLocalControl.tsx | 3 +++ 6 files changed, 90 insertions(+), 23 deletions(-) create mode 100644 scripts/cua-mac-arches.mjs create mode 100644 scripts/cua-mac-arches.test.mjs diff --git a/scripts/cua-mac-arches.mjs b/scripts/cua-mac-arches.mjs new file mode 100644 index 000000000..3a52d147e --- /dev/null +++ b/scripts/cua-mac-arches.mjs @@ -0,0 +1,24 @@ +export const DEFAULT_MAC_ARCHES = ["arm64", "x64"]; + +/** Which darwin CUA trees to stage. electron-builder packages both arches + * unless the caller opts into a one-arch local stage. An empty override + * is never a silent no-op. */ +export function resolveCuaMacArches(env = process.env) { + const raw = env.OPENMAUSBOT_CUA_ARCHES; + if (raw === undefined) return [...DEFAULT_MAC_ARCHES]; + const arches = raw.split(",").map((arch) => arch.trim()).filter(Boolean); + if (arches.length === 0) { + throw new Error( + "OPENMAUSBOT_CUA_ARCHES is empty; omit the variable to stage arm64 and x64", + ); + } + if (env.OPENMAUSBOT_CUA_ARCHES_PARTIAL !== "1") { + const missing = DEFAULT_MAC_ARCHES.filter((arch) => !arches.includes(arch)); + if (missing.length) { + throw new Error( + `OPENMAUSBOT_CUA_ARCHES omits ${missing.join(", ")} but electron-builder packages both arm64 and x64. Set OPENMAUSBOT_CUA_ARCHES_PARTIAL=1 for a one-arch local stage.`, + ); + } + } + return arches; +} diff --git a/scripts/cua-mac-arches.test.mjs b/scripts/cua-mac-arches.test.mjs new file mode 100644 index 000000000..f2573ee74 --- /dev/null +++ b/scripts/cua-mac-arches.test.mjs @@ -0,0 +1,24 @@ +import { describe, expect, it } from "vitest"; +import { DEFAULT_MAC_ARCHES, resolveCuaMacArches } from "./cua-mac-arches.mjs"; + +describe("resolveCuaMacArches", () => { + it("defaults to both packaging targets", () => { + expect(resolveCuaMacArches({})).toEqual(DEFAULT_MAC_ARCHES); + }); + + it("rejects an empty override even when PARTIAL=1", () => { + expect(() => resolveCuaMacArches({ OPENMAUSBOT_CUA_ARCHES: "", OPENMAUSBOT_CUA_ARCHES_PARTIAL: "1" })).toThrow( + /OPENMAUSBOT_CUA_ARCHES is empty/, + ); + expect(() => resolveCuaMacArches({ OPENMAUSBOT_CUA_ARCHES: " , ", OPENMAUSBOT_CUA_ARCHES_PARTIAL: "1" })).toThrow( + /OPENMAUSBOT_CUA_ARCHES is empty/, + ); + }); + + it("rejects a one-arch override unless PARTIAL=1", () => { + expect(() => resolveCuaMacArches({ OPENMAUSBOT_CUA_ARCHES: "arm64" })).toThrow(/omits x64/); + expect(resolveCuaMacArches({ OPENMAUSBOT_CUA_ARCHES: "arm64", OPENMAUSBOT_CUA_ARCHES_PARTIAL: "1" })).toEqual([ + "arm64", + ]); + }); +}); diff --git a/scripts/prepare-cua.mjs b/scripts/prepare-cua.mjs index 958f4cc84..0a25a8cab 100644 --- a/scripts/prepare-cua.mjs +++ b/scripts/prepare-cua.mjs @@ -11,6 +11,7 @@ import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { promisify } from "node:util"; import { build } from "esbuild"; +import { resolveCuaMacArches } from "./cua-mac-arches.mjs"; if (process.platform !== "darwin") throw new Error("prepare-cua is macOS-only"); @@ -107,19 +108,7 @@ if (!details.isFile() || (details.mode & 0o111) === 0) { // not on a user's Intel Mac); the SDK's dylib/.node are genuinely per-arch, // pulled from the two darwin native packages that pnpm installs because of // supportedArchitectures in package.json. -const DEFAULT_MAC_ARCHES = ["arm64", "x64"]; -const MAC_ARCHES = (process.env.OPENMAUSBOT_CUA_ARCHES ?? DEFAULT_MAC_ARCHES.join(",")) - .split(",") - .map((arch) => arch.trim()) - .filter(Boolean); -if (process.env.OPENMAUSBOT_CUA_ARCHES && process.env.OPENMAUSBOT_CUA_ARCHES_PARTIAL !== "1") { - const missing = DEFAULT_MAC_ARCHES.filter((arch) => !MAC_ARCHES.includes(arch)); - if (missing.length) { - throw new Error( - `OPENMAUSBOT_CUA_ARCHES omits ${missing.join(", ")} but electron-builder packages both arm64 and x64. Set OPENMAUSBOT_CUA_ARCHES_PARTIAL=1 for a one-arch local stage.`, - ); - } -} +const MAC_ARCHES = resolveCuaMacArches(process.env); const { stdout: archList } = await run("/usr/bin/lipo", ["-archs", binary]); for (const arch of MAC_ARCHES) { diff --git a/server/drivers/acp/kimi.ts b/server/drivers/acp/kimi.ts index 06e111bd8..43d3d1fa6 100644 --- a/server/drivers/acp/kimi.ts +++ b/server/drivers/acp/kimi.ts @@ -69,21 +69,28 @@ function stripTomlLineComment(line: string): string { return line; } -/** Decode a TOML basic-string escape at `text[i]` (`i` points at the `\\`). */ -function takeTomlBasicEscape(text: string, i: number): { value: string; next: number } { +/** Decode a TOML basic-string escape at `text[i]` (`i` points at the `\\`). + * Invalid / unknown / surrogate / out-of-range sequences return ok:false so + * the heading is not canonicalized to a colliding alias. */ +function takeTomlBasicEscape( + text: string, + i: number, +): { ok: true; value: string; next: number } | { ok: false } { const code = text[i + 1]; + if (code === undefined) return { ok: false }; if (code === "u") { const hex = text.slice(i + 2, i + 6); - if (/^[0-9a-fA-F]{4}$/.test(hex)) { - return { value: String.fromCharCode(parseInt(hex, 16)), next: i + 6 }; - } + if (!/^[0-9a-fA-F]{4}$/.test(hex)) return { ok: false }; + const point = parseInt(hex, 16); + if (point >= 0xd800 && point <= 0xdfff) return { ok: false }; + return { ok: true, value: String.fromCharCode(point), next: i + 6 }; } if (code === "U") { const hex = text.slice(i + 2, i + 10); - if (/^[0-9a-fA-F]{8}$/.test(hex)) { - const point = parseInt(hex, 16); - return { value: point <= 0x10ffff ? String.fromCodePoint(point) : "", next: i + 10 }; - } + if (!/^[0-9a-fA-F]{8}$/.test(hex)) return { ok: false }; + const point = parseInt(hex, 16); + if (point > 0x10ffff || (point >= 0xd800 && point <= 0xdfff)) return { ok: false }; + return { ok: true, value: String.fromCodePoint(point), next: i + 10 }; } const named: Record = { b: "\b", @@ -94,7 +101,8 @@ function takeTomlBasicEscape(text: string, i: number): { value: string; next: nu '"': '"', "\\": "\\", }; - return { value: named[code ?? ""] ?? code ?? "", next: i + 2 }; + if (!(code in named)) return { ok: false }; + return { ok: true, value: named[code]!, next: i + 2 }; } /** Canonical `a.b.c` form of a `[table]` heading, quotes and comments removed. */ @@ -117,6 +125,7 @@ function canonicalizeTomlHeading(heading: string): string | null { while (i < inner.length && inner[i] !== q) { if (q === '"' && inner[i] === "\\") { const taken = takeTomlBasicEscape(inner, i); + if (!taken.ok) return null; value += taken.value; i = taken.next; continue; diff --git a/server/drivers/local-inject.test.ts b/server/drivers/local-inject.test.ts index 21516fd91..c13462bde 100644 --- a/server/drivers/local-inject.test.ts +++ b/server/drivers/local-inject.test.ts @@ -520,6 +520,24 @@ describe("ensureKimiInjectAlias", () => { expect(text).toContain("max_context_size = 262144"); }); + it("does not treat a malformed escape as a canonical alias", () => { + const home = mkdtempSync(join(tmpdir(), "omb-kimi-badesc-")); + scratchDirs.push(home); + const root = join(home, ".kimi-code"); + mkdirSync(root, { recursive: true }); + writeFileSync( + join(root, "config.toml"), + ['[models."omlx/GLM-\\q.2-fp8"]', 'provider = "omlx"', 'model = "nope"', ""].join("\n"), + ); + ensureKimiInjectAlias("omlx::GLM-5.2-fp8", { HOME: home }); + const text = readFileSync(join(root, "config.toml"), "utf8"); + expect(text).toContain("GLM-\\q.2-fp8"); + expect(text).toContain('model = "nope"'); + expect(text.match(/\[models\./g)?.length).toBe(2); + expect(text).toContain('model = "GLM-5.2-fp8"'); + expect(text).toContain('protocol = "openai"'); + }); + it("treats whitespace around dotted heading keys as the same table", () => { const home = mkdtempSync(join(tmpdir(), "omb-kimi-dots-")); scratchDirs.push(home); diff --git a/src/components/MacLocalControl.tsx b/src/components/MacLocalControl.tsx index 6be2870a7..eb73ed3b4 100644 --- a/src/components/MacLocalControl.tsx +++ b/src/components/MacLocalControl.tsx @@ -34,8 +34,11 @@ export function MacLocalControl() { useEffect(() => { if (!awaitingGrant) return; + let used = false; const onFocus = () => { + if (used) return; if (document.visibilityState !== "visible") return; + used = true; setAwaitingGrant(false); void retry(); }; From 69b7f671ae48a2659a2df83cc839ca18a90fef30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=B6khan=20K=C3=B6se?= Date: Thu, 20 Aug 2026 21:57:02 +0200 Subject: [PATCH 21/52] =?UTF-8?q?feat:=20import=20a=20team=20as=20a=20proj?= =?UTF-8?q?ect=20=E2=80=94=20one=20room,=20on=20a=20folder?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `POST /api/teams/import?mode=project` adds the team and opens a room for it, optionally on a folder: `&cwd=`, `&room=`. Without it, setting up a project is three steps — import the team, create a room, pick its members — and the third one is tedious once a team has more than a few people. **The manifest still describes only people.** Room name and folder come from the caller, never from the file. That is deliberate and preserves what v2 established when it dropped its `room` block: a manifest fetched from the library must not be able to create structure in someone's workspace. As a parameter, a local caller gets the one-step setup without opening the format to a remote one. It reads the same way as the neighbouring guards — `seedMessages: false`, `composio: false` — a shared team brings people, not reach. Details worth knowing: - The folder goes to `cwd`, not `pinnedCwd`. It is what the room WANTS; the store pins it on the first turn, which is its call to make, not ours. - The path is validated with the same `validateBotCwd` a bot's folder uses, before anything is created — a bad path is a 400 with no bots and no room left behind. - The room is created last, so any failure above leaves nothing pointing at half-built state. - `add` and `replace` are untouched: they still return no `group`, and the existing test asserting that still passes. Verified by disabling the room creation: the new test fails. Full suite green. --- server/index.test.ts | 47 ++++++++++++++++++++++++++++++++++++++++++++ server/index.ts | 37 +++++++++++++++++++++++++++++++--- 2 files changed, 81 insertions(+), 3 deletions(-) diff --git a/server/index.test.ts b/server/index.test.ts index 7274f70cb..450233f0d 100644 --- a/server/index.test.ts +++ b/server/index.test.ts @@ -424,6 +424,53 @@ describe("harness HTTP API", () => { } }); + it("imports a team as a project: one room, on a folder", async () => { + // The manifest still describes only people. Room name and folder come + // from the CALLER, so a manifest fetched from the library cannot create + // structure in someone's workspace — the property v2 established by + // dropping its `room` block. + const seed = await api("POST", "/api/bots", { name: "Planner", title: "Lead", description: "Plans", color: "purple" }); + const exported = await api("POST", "/api/teams/export", { name: "Client XY" }); + expect(exported.body.team).not.toHaveProperty("room"); + + const roomsBefore = (await api("GET", "/api/bots")).body.groups.length; + const folder = mkdtempSync(join(tmpdir(), "omb-project-")); + + const stream = await openSse(`${BASE}/api/events`); + try { + await stream.until((frame) => frame.kind === "hello"); + + // A folder that does not exist must not leave half a project behind. + const bogus = await api("POST", `/api/teams/import?mode=project&cwd=${encodeURIComponent(join(folder, "nope"))}`, exported.body); + expect(bogus.status).toBe(400); + expect((await api("GET", "/api/bots")).body.groups).toHaveLength(roomsBefore); + + const created = await api("POST", `/api/teams/import?mode=project&cwd=${encodeURIComponent(folder)}`, exported.body); + expect(created.status).toBe(201); + expect(created.body.group).toMatchObject({ name: "Client XY", cwd: folder }); + // the room is made of exactly the bots this import created + expect(created.body.group.memberIds.sort()).toEqual(created.body.bots.map((bot: { id: string }) => bot.id).sort()); + // the folder is the room's WISH; the store pins it on the first turn + expect(created.body.group).not.toHaveProperty("pinnedCwd"); + expect((await api("GET", "/api/bots")).body.groups).toHaveLength(roomsBefore + 1); + await stream.until((frame) => frame.kind === "group" && frame.group?.id === created.body.group.id); + + // an explicit name wins over the team name, and the folder is optional + const named = await api("POST", "/api/teams/import?mode=project&room=Client%20XY%20-%20Ads", exported.body); + expect(named.body.group).toMatchObject({ name: "Client XY - Ads" }); + expect(named.body.group.cwd).toBeUndefined(); + + for (const room of [created.body.group, named.body.group]) { + expect((await api("DELETE", `/api/groups/${room.id}`)).status).toBe(200); + } + for (const bot of [seed.body, ...created.body.bots, ...named.body.bots]) { + await api("DELETE", `/api/bots/${bot.id}`); + } + } finally { + stream.close(); + } + }); + it("keeps the rest of a duplicate's fields when the source engine is offline", async () => { // duplicateBot POSTs a blank bot, then PATCHes the source's whole // modelSelection in one body beside its name, title and description. diff --git a/server/index.ts b/server/index.ts index 4b94dfd35..ac714994f 100644 --- a/server/index.ts +++ b/server/index.ts @@ -2641,8 +2641,23 @@ const server = createServer(async (req, res) => { } if (method === "POST" && path === "/api/teams/import") { const importMode = url.searchParams.get("mode") ?? "add"; - if (importMode !== "add" && importMode !== "replace") { - return json(res, 400, { error: "Team import mode must be add or replace" }); + if (importMode !== "add" && importMode !== "replace" && importMode !== "project") { + return json(res, 400, { error: "Team import mode must be add, replace, or project" }); + } + // `project` adds the team AND opens a room for it on a folder. The room + // is described by the CALLER, never by the manifest: a manifest is a + // list of people, and one fetched from the library must not be able to + // create structure in someone's workspace (which is why v2 dropped its + // `room` block). Naming it here keeps that property while letting a + // local caller set up a project in one step. + let projectCwd: string | null = null; + if (importMode === "project") { + const requested = url.searchParams.get("cwd"); + if (requested !== null) { + const validated = validateBotCwd(requested); + if (!validated.ok) return json(res, 400, { error: validated.error }); + projectCwd = validated.cwd; + } } const body = await readBody(req); let manifest; @@ -2689,7 +2704,23 @@ const server = createServer(async (req, res) => { const publicBots = importedBots.map(publicBot); for (const bot of archivedBots) broadcast({ kind: "bot", bot }); for (const bot of publicBots) broadcast({ kind: "bot", bot }); - return json(res, 201, { bots: publicBots, archivedBots, archived }); + + // The room is created last, so a failure anywhere above leaves no + // half-built project behind — the catch below deletes the bots and + // there is no room pointing at them. + let group; + if (importMode === "project" && importedBots.length > 0) { + const roomName = url.searchParams.get("room")?.trim() || manifest.team.name; + group = store.createGroup(roomName, importedBots.map((bot) => bot.id)); + if (projectCwd) { + // `cwd` is the folder the room WANTS; the store pins it on the + // first turn (pinGroupCwd). Setting the pin here would decide it + // before anyone has worked, which is the store's call, not ours. + group = store.patchGroup(group.id, { cwd: projectCwd }) ?? group; + } + broadcast({ kind: "group", group }); + } + return json(res, 201, { bots: publicBots, archivedBots, archived, group }); } catch (error) { for (const bot of importedBots) store.deleteBot(bot.id); throw error; From a143c47dc26b29fe50a10173ceddc48eee574670 Mon Sep 17 00:00:00 2001 From: Ansygroup Date: Fri, 21 Aug 2026 01:48:19 +0300 Subject: [PATCH 22/52] fix(build): extract Android Platform Tools on Windows (git-bash) The download+extract step in scripts/prepare-android-tools.mjs assumed a native Windows shell where tar is bsdtar (zip-capable). On git-bash it is GNU tar, which (a) reads the C: drive prefix in the -C path as a remote host ('Cannot connect to C:') and (b) cannot read .zip at all. That broke pnpm package:win out of the box on Windows. - Prefer unzip, fall back to tar on Windows (covers git-bash AND native cmd/PowerShell). - Normalize absolute Windows paths to MSYS/POSIX form so git-bash's tar -C never treats the drive letter as a host. Verified: pnpm build:android-tools now stages adb.exe via the real download+extract path; pnpm typecheck passes. --- scripts/prepare-android-tools.mjs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/scripts/prepare-android-tools.mjs b/scripts/prepare-android-tools.mjs index 872c19fa0..b3fef8e6f 100644 --- a/scripts/prepare-android-tools.mjs +++ b/scripts/prepare-android-tools.mjs @@ -32,10 +32,22 @@ try { writeFileSync(zip, Buffer.from(await response.arrayBuffer())); const extraction = join(temporary, "extracted"); mkdirSync(extraction); - const command = process.platform === "win32" ? "tar" : "unzip"; - const args = process.platform === "win32" ? ["-xf", zip, "-C", extraction] : ["-q", zip, "-d", extraction]; - const result = spawnSync(command, args, { encoding: "utf8" }); - if (result.status !== 0) throw new Error(`${command} failed: ${(result.stderr || result.stdout).trim()}`); + // git-bash ships GNU tar (cannot read .zip) but has unzip; a native + // Windows shell has tar (bsdtar, zip-capable) but no unzip. Try unzip + // first and fall back to tar so either environment extracts cleanly. + // git-bash's tar also treats a "C:" drive prefix in an absolute -C path + // as a remote host, so normalize Windows paths to MSYS/POSIX form. + const toMsys = (p) => p.replace(/\\/g, "/").replace(/^([A-Za-z]):/, "/$1"); + const extractors = process.platform === "win32" + ? [["unzip", ["-q", zip, "-d", extraction]], ["tar", ["-xf", toMsys(zip), "-C", toMsys(extraction)]]] + : [["unzip", ["-q", zip, "-d", extraction]]]; + let result; + for (const [command, args] of extractors) { + result = spawnSync(command, args, { encoding: "utf8" }); + if (result.status === 0) break; + console.error(`${command} failed: ${(result.stderr || result.stdout).trim()} — trying next`); + } + if (result.status !== 0) throw new Error(`could not extract Android Platform Tools: ${(result.stderr || result.stdout).trim()}`); cpSync(join(extraction, "platform-tools"), staged, { recursive: true }); } From 795ee651a4c3648afc58fe7cbd7625d0b6613230 Mon Sep 17 00:00:00 2001 From: SomSamantray <> Date: Tue, 18 Aug 2026 16:48:19 +0530 Subject: [PATCH 23/52] docs(plan): plan fix for #211 zombie permission cards on turn teardown --- ...-claude-turn-teardown-zombie-cards-plan.md | 135 ++++++++++++++++++ 1 file changed, 135 insertions(+) create mode 100644 docs/plans/2026-08-18-001-fix-claude-turn-teardown-zombie-cards-plan.md diff --git a/docs/plans/2026-08-18-001-fix-claude-turn-teardown-zombie-cards-plan.md b/docs/plans/2026-08-18-001-fix-claude-turn-teardown-zombie-cards-plan.md new file mode 100644 index 000000000..bb38bdbf5 --- /dev/null +++ b/docs/plans/2026-08-18-001-fix-claude-turn-teardown-zombie-cards-plan.md @@ -0,0 +1,135 @@ +--- +artifact_contract: ce-unified-plan/v1 +artifact_readiness: implementation-ready +execution: code +product_contract_source: ce-plan-bootstrap +--- + +# fix: Kill the child process and drop late asks on Claude turn teardown + +**Origin issue:** [milind-soni/OpenMausBot#211](https://github.com/milind-soni/OpenMausBot/issues/211) — "Turn teardown discards the permission broker without ensuring child CLI/MCP exit — late approval requests become dead 'zombie' cards" + +--- + +## Summary + +When a Claude-driven turn ends, `server/drivers/claude.ts`'s `settle()` tears down the permission broker and forgets the turn — but never kills the spawned `claude -p --resume` child. If the child (or an MCP grandchild doing backgrounded work) doesn't exit on its own, it can later emit a new permission ask on its still-open broker connection. Because `net.Server.close()` doesn't touch already-open sockets, that ask is still processed and surfaces as a `request.opened` card — but the turn's `active` entry (and the broker reference the UI needs to answer it) is already gone, so the card can never be resolved. The fix makes `settle()` unconditionally kill the child's process tree and makes the broker's `close()` actually stop honoring asks on any connection, closed or not. + +## Problem Frame + +`sendTurn()` in `server/drivers/claude.ts` spawns the `claude` CLI per turn and wires a `createPermissionBroker()` instance to a per-turn unix socket (named pipe on Windows). The CLI's own spawned MCP `ogb` process (`server/permission-proxy.ts`) forwards `approve`/`ask_user` tool calls over that socket to the broker, which the harness renders as `request.opened` cards. + +Two independent gaps compound into the reported symptom: + +1. **The child is never killed on teardown.** `settle()` (invoked from the `result` stream-json frame, a spawn `error`, or an exit-before-`result` `close`) deletes the turn from `active` and closes the broker, but nothing calls `killCliTree(child)` — that utility is only wired to `interruptTurn` (an explicit user stop). A `-p` one-shot CLI process is expected to exit right after printing `result`, but if it (or a grandchild doing `run_in_background` work) doesn't, it keeps running with no teardown-side enforcement. + +2. **A closed broker still answers still-open connections.** `createPermissionBroker.close()` calls `server.close()`, which per Node's `net` docs "stops the server from accepting new connections" but does **not** touch sockets that already connected. The `conn.on("data", ...)` handler registered in `createNetServer((conn) => {...})` stays fully wired to any live connection — a lingering child's MCP proxy can still send a `{t:"ask",...}` message, which the handler happily adds to the (still-referenced-via-closure) `pending` Map and forwards via `opts.onAsk(ask)`, emitting a brand-new `request.opened` event. But `active.delete(threadId)` already ran inside `settle()`, so `respondToRequest` finds no broker for that thread and returns `"unavailable"` — the exact unanswerable "Auto mode couldn't answer this one" card the issue describes. + +**Prior art in this codebase:** `server/drivers/antigravity.ts` already tracks live children independently of `active` (a `children: Set`) and reaps them via `reapChildren(escalate)` — `killCliTree` first, then an optional SIGKILL after a 2s grace on POSIX when `escalate` is set. That mechanism is only invoked from `stopAll()`/`dispose()` (whole-driver shutdown), not per-turn, so it does not by itself close this gap — but its escalation shape is the right reference for this fix. + +## Requirements + +- **R1**: On every terminal path of a Claude turn (successful `result`, spawn `error`, or exit-before-`result`), the spawned child's entire process tree must be forcibly terminated as part of teardown — not left to exit on its own, and not deferred to `interruptTurn`/`stopAll`/`dispose`. +- **R2**: A permission/question ask that arrives after the broker for that turn has been closed must never become an actionable `request.opened` UI event, and must not leave the caller's MCP tool call hanging forever. It must be resolved with a system-source deny/answer written directly to the still-open connection — the same shape `close()` already uses for in-flight `pending` asks — never surfaced as a card, and never a silent drop (see KTD2: `server/permission-proxy.ts` has no independent per-ask timeout, so a silent drop leaves that `tools/call` promise pending indefinitely). +- **R3**: Neither fix may change behavior for the already-working case: a turn whose child exits promptly and cleanly after `result`, with no late asks, must produce the exact same event sequence and `turn.completed` payload as today. +- **R4**: `killCliTree`'s existing contract (SIGTERM to the process group on POSIX, forceful `taskkill /T /F` on Windows) must not regress — this fix reuses it, not forks it. + +## Scope Boundaries + +**In scope:** `server/drivers/claude.ts` (`settle()` and `createPermissionBroker()`), its test file `server/drivers/claude.test.ts`, and the shared fake CLI `server/testing/fake-claude-cli.ts` (a new mode to reproduce a post-`result` lingering child). + +**Out of scope:** +- The issue's "point 3" (a startup sweep that reaps orphans left over from a crash where teardown itself never ran). Different failure mode — crash recovery, not normal-path leak — requiring process-marker scanning across the whole app at startup. Deferred to follow-up. +- `server/drivers/codex.ts` and `server/drivers/acp/core.ts` share the structurally identical `const stop = () => killCliTree(child);`-only-on-`interruptTurn` pattern and likely the same latent defect, but the reported issue is specifically about `claude -p --resume`. Same shape of fix, separate surface — deferred to follow-up, not folded into this PR. +- Rewiring `antigravity.ts`'s `reapChildren` to also run per-turn (it has its own driver-specific `children` tracking already; touching it is unrelated to this issue). + +### Deferred to Follow-Up Work +- Startup orphan sweep (issue's point 3). +- Porting the same per-turn kill-on-settle + drop-late-asks fix to `codex.ts` and `acp/core.ts`. + +## Key Technical Decisions + +**KTD1: Kill the child directly inside `settle()`, reusing `killCliTree` as-is — no SIGTERM-then-SIGKILL escalation added at this call site.** +Rationale: `killCliTree` already does a full-strength kill per platform (POSIX: `SIGTERM` to the process group, which is the same signal `antigravity.ts`'s non-escalated path sends; Windows: `taskkill /T /F`, already forceful — escalation is a documented no-op there). `server/kill-tree.test.ts` proves this reaps a grandchild reliably *when that grandchild stays in the spawned child's process group* — which is the case for an ordinary MCP server the CLI spawns without `detached: true` itself. `antigravity.ts`'s SIGKILL-after-grace escalation exists for its `dispose()`/`stopAll()` path, which reaps a *set* of potentially-many children at once and can afford a shared grace window; retrofitting a per-turn `setTimeout` here for a single child adds a timer to manage (and to account for in tests) for a benefit `kill-tree.test.ts` doesn't show is needed. If real-world SIGTERM stalls turn out to matter later, `antigravity.ts`'s escalation is there to copy. +**Known limitation, explicitly out of scope:** if a grandchild deliberately re-detaches itself (calls its own `spawn(..., { detached: true })`/`setsid`, escaping into a new process group — the classic daemonizing pattern), `process.kill(-pid, "SIGTERM")` on the original group will not reach it. `kill-tree.test.ts`'s existing grandchild does *not* self-detach, so it does not prove coverage of this case. No known MCP server this harness spawns (`permission-proxy.ts`, `computer-proxy`, `dweb-proxy`, `agents-proxy`) does this today, so this is a documented gap, not a live regression — but it means KTD1 does not fully close the "backgrounded work" scenario from the Problem Frame if that work ever re-detaches. Follow-up if it becomes a live issue: a kill-tree test with a self-detaching grandchild, and a stronger reaping strategy if it fails. +Alternative considered and rejected: add the same grace+SIGKILL escalation now — rejected as unjustified complexity without evidence a plain SIGTERM leaves processes behind in this codepath (unlike `antigravity.ts`, which already had multi-child cleanup to justify it), and because escalation doesn't help the self-detached case above anyway (a SIGKILL to the wrong process group is still a no-op). + +**KTD2: Guard late asks with a `closed` boolean checked inside the connection's `data` handler; on a late ask, always write a system-source deny/answer to the connection — never a silent drop.** +Rationale: the simplest fix that satisfies R2 without changing `close()`'s existing contract for in-flight `pending` asks. The `data` handler already runs inside the closure that owns `pending`, `timeoutMs`, and now a `closed` flag set at the top of `close()`; checking it before creating a new pending entry is a one-line, easily-tested guard. **The response must be an explicit system-source deny/answer, not a silent drop:** `server/permission-proxy.ts`'s `waiting` map (the child-side promise the CLI's `tools/call` is awaiting) is only resolved by an incoming `{t:"answer",...}` message or by the connection's own `error`/`close` firing `dead()` — nothing in `permission-proxy.ts` times out a single ask on its own. A silent drop on the broker side leaves that specific MCP tool call hanging until something else closes the connection, which is exactly the kind of hang R2 exists to prevent. (An earlier draft of this plan left "log and drop" as an equally-valid alternative; document review caught that this contradicts `permission-proxy.ts`'s actual behavior and R2 has been tightened to require the explicit reply.) +Alternatives considered and rejected: (a) track and destroy live connections in `close()` — rejected as more state for no additional correctness, since the goal is "never create an answerable-looking dead card," not "sever the pipe," and a killed child per KTD1 will sever it anyway in the common case; the `closed` flag plus an explicit reply covers the case where the child hasn't been killed yet. (b) reuse `active.has(threadId)` instead of a dedicated `closed` boolean, since `respondToRequest` already treats a missing `active` entry as "no active turn" — rejected because of an ordering hazard: `createPermissionBroker` is constructed *before* `active.set(threadId, ...)` runs in `sendTurn` (the broker needs to exist to build the MCP config passed to the spawn call), so `active.has(threadId)` would incorrectly read `false` during that brief legitimate startup window, denying an ask that arrives before the turn is even fully registered. A dedicated flag defaulting to `false` has no such window. + +**KTD3: New fake-CLI mode `result-then-hang` for testing fix A; no new mode needed for fix B.** +Rationale: fix A's contract is about the OS process, not just emitted events — the existing `hang` mode never reaches `result`, so it can't prove "settle happened AND the process is gone." A new mode that prints `result` then calls the same idle-forever `setInterval` `hang` already uses is a minimal, symmetric addition. Fix B's test doesn't need a new CLI mode: it drives the broker directly over the socket exactly like the existing "brokers a permission ask" test, just with a second `{t:"ask",...}` sent after `turn.completed`/close — no CLI behavior involved. + +## Implementation Units + +### U1. Kill the child's process tree on every settle() path + +**Goal:** Eliminate the leaked/lingering process (R1). + +**Requirements:** R1, R3, R4 + +**Dependencies:** None + +**Files:** +- `server/drivers/claude.ts` (modify `settle()`, ~line 472-489) +- `server/testing/fake-claude-cli.ts` (add `result-then-hang` mode) +- `server/drivers/claude.test.ts` (new test; test file path already exists) + +**Approach:** +- In `settle()`, call `killCliTree(child)` unconditionally, before or alongside the existing `broker?.close()` and temp-dir cleanup. `child` is already in scope (defined earlier in `sendTurn` at the `spawnCli` call); do not introduce a new binding or rely on the later-declared `stop` const. +- `killCliTree` is a no-op when the process already exited (`child.exitCode !== null || child.signalCode !== null`), so this is safe for the common case where the CLI has already exited by the time `result` is parsed and `settle()` runs — R3's no-regression requirement holds by construction. +- Add `result-then-hang` to `server/testing/fake-claude-cli.ts`: emit the same `system`/`assistant`/`user`/`result` sequence the default `happy` path does, then instead of `process.exit(0)`, call the same `setInterval(() => {}, 1_000)` the `hang` mode uses to stay alive. +- **The test needs the fake CLI's real OS pid to verify it's actually dead, and nothing today exposes it.** `ProviderInstance.adapter` has no pid accessor, and `FAKE_CLAUDE_DUMP`'s payload (`{argv, env, prompt, mcpConfig}`) doesn't carry one. Add `pid: process.pid` to the object `fake-claude-cli.ts` writes via `writeFileSync(process.env.FAKE_CLAUDE_DUMP, ...)` — reusing the existing dump mechanism rather than adding a new one — so the test can read it back the same way existing tests already read `argv`/`env` from that file. +- New test in `server/drivers/claude.test.ts`: set `FAKE_CLAUDE_DUMP` to a scratch path, run a turn in `result-then-hang` mode, wait for `turn.completed`, read the pid back from the dump file, then assert the underlying process is actually gone (poll `process.kill(pid, 0)` throwing, or an equivalent liveness check — see `server/kill-tree.test.ts`'s `alive()` helper for the established pattern) within a bounded timeout. This is the test that would fail today (child stays alive) and pass after the fix. + +**Patterns to follow:** `server/kill-tree.test.ts`'s `alive(pid)` helper (POSIX signal-0 probe) for asserting process death without a platform-specific timeout guess. + +**Test scenarios:** +- Happy path: `result-then-hang` mode — `turn.completed` still fires with the correct `ok`/`stopReason`/`usage` payload (unchanged from today), and the underlying process is verifiably dead shortly after. +- Regression/no-op check: default `happy` mode — turn completes exactly as today (same event sequence, same `turn.completed` fields) with `killCliTree` now also called (should be a harmless no-op since the process already exited). +- Edge case: `exit-early` mode (crash before result) — `killCliTree` runs from the `close` handler's `settle(false, "exit_before_result")` path too; confirm no error is thrown when calling `killCliTree` on an already-exited/crashed child. + +**Verification:** Run `pnpm vitest run server/drivers/claude.test.ts` (or the repo's documented equivalent) locally; all existing tests in the file continue to pass, and the new `result-then-hang` test demonstrates the process is killed. + +### U2. Drop permission/question asks that arrive after the broker has closed + +**Goal:** Eliminate the unanswerable "zombie" card (R2). + +**Requirements:** R2, R3 + +**Dependencies:** U1 (not a hard code dependency, but U1 removes the common trigger for this race; U2 is the correctness backstop for the remaining window between an ask being emitted and the kill signal actually landing) + +**Files:** +- `server/drivers/claude.ts` (modify `createPermissionBroker`, ~line 196-278) +- `server/drivers/claude.test.ts` (new test) + +**Approach:** +- Add a `let closed = false;` inside `createPermissionBroker`'s closure. +- At the top of the `conn.on("data", ...)` handler's per-line processing, after parsing `msg` but before creating the `Ask`/`pending` entry: if `closed` is true, write a system-source reply directly to `conn` — `{t:"answer", id: msg.id, behavior: "deny", message: DENY_TIMEOUT_NOTE}` for a permission ask, or `{t:"answer", id: msg.id, behavior:"answer", message: QUESTION_TIMEOUT_NOTE}` for a question — without registering a `pending` entry or calling `opts.onAsk`, then log the drop and return. **This must always reply; per KTD2/R2, silently returning with no reply is not an option** — it leaves `permission-proxy.ts`'s corresponding `tools/call` promise hanging, since that file only resolves an ask on an explicit `{t:"answer"}` or its own connection `error`/`close`. +- Set `closed = true` as the first line of `close()`, before it iterates `pending`. +- Do not change what `close()` does with the existing `pending` Map — that behavior (system-source deny for permissions, system-source answer for questions) is correct and already tested. + +**Patterns to follow:** The existing "brokers a permission ask into request.opened and answers over the socket" test's connection-driving style in `server/drivers/claude.test.ts` (`connect(permissionSocketPath(...))`, write a raw `{t:"ask",...}` line, read the JSON reply off `conn`). + +**Test scenarios:** +- Happy path (regression): the existing "brokers a permission ask into request.opened and answers over the socket" test continues to pass unchanged — an ask sent while the turn is active still becomes `request.opened` and is answerable. +- Primary regression scenario: start a turn in `hang` mode, open a connection to the permission socket, send one ask and let it resolve (or just proceed without resolving), then call `interruptTurn` and **await `turn.completed`** (interrupt only calls `stop()`/`killCliTree`; `settle()` — which sets `closed = true` — only runs later, asynchronously, off the child's `close` event, so the test must wait for it rather than racing ahead), then send a **second** `{t:"ask",...}` on the **same still-open connection** — assert no new `request.opened` event is emitted for it, and that the connection receives the system-source deny/answer reply (never a bare drop with no reply). +- Edge case: an ask that was already `pending` when `close()` runs must still resolve via the existing system-source deny/answer path (this is the pre-existing "resolves a pending ask as a system denial when the turn is interrupted" test — confirm it still passes unchanged). + +**Verification:** Run `pnpm vitest run server/drivers/claude.test.ts`; the new late-ask test demonstrates no `request.opened` fires for an ask sent after close, and all pre-existing broker tests in the file remain green. + +## Verification Contract + +- Both units are verified by extending the real, already-passing local Vitest suite (`server/drivers/claude.test.ts`, plus the new `fake-claude-cli.ts` mode) — this repo has a working local build and test runner (`pnpm`/`vitest`), unlike prior situations requiring standalone reproduction outside the repo. +- Run the full existing `server/drivers/claude.test.ts` file (not just the new tests) to confirm no regression in the broker/turn-lifecycle behavior the file already covers. +- Run `pnpm typecheck` (or the repo's documented type-check command) since this touches TypeScript control flow inside closures. + +## Definition of Done + +- [ ] `settle()` in `server/drivers/claude.ts` unconditionally calls `killCliTree(child)` on every terminal path (R1, R4). +- [ ] `createPermissionBroker`'s connection handler drops/denies any ask received after `close()` ran, and this cannot resurrect a `request.opened` card for a torn-down turn (R2). +- [ ] New `result-then-hang` fake-CLI mode added; new tests for both units pass. +- [ ] All pre-existing tests in `server/drivers/claude.test.ts` and `server/kill-tree.test.ts` still pass (R3). +- [ ] No changes to `codex.ts`, `acp/core.ts`, `antigravity.ts`, or the issue's point-3 startup sweep — explicitly deferred. +- [ ] Changes committed with a message referencing milind-soni/OpenMausBot#211. From cdd900495cd2cb1aa890fa3e2423875f7ce6a33b Mon Sep 17 00:00:00 2001 From: SomSamantray <> Date: Tue, 18 Aug 2026 16:53:23 +0530 Subject: [PATCH 24/52] fix(claude): kill the child process tree on every turn teardown settle() closed the permission broker and forgot the turn, but never killed the spawned claude -p --resume child. A one-shot process is expected to exit right after printing result, but a backgrounded MCP grandchild can keep it alive with a live broker connection. Adds killCliTree(child) to settle(), unconditionally, on every terminal path. A no-op when the process already exited. Adds a result-then-hang fake-CLI mode (prints result but never exits) to reproduce and test the leak, and exposes the fake CLI's pid via the existing FAKE_CLAUDE_DUMP mechanism so the test can confirm the process is actually gone, not just that turn.completed fired. Part of milind-soni/OpenMausBot#211 --- server/drivers/claude.test.ts | 27 +++++++++++++++++++++++++++ server/drivers/claude.ts | 6 ++++++ server/testing/fake-claude-cli.ts | 31 +++++++++++++++++++++++++------ 3 files changed, 58 insertions(+), 6 deletions(-) diff --git a/server/drivers/claude.test.ts b/server/drivers/claude.test.ts index c4428eb6d..266b2e726 100644 --- a/server/drivers/claude.test.ts +++ b/server/drivers/claude.test.ts @@ -428,6 +428,33 @@ describe("ClaudeDriver turns (fake CLI)", () => { expect(done).toMatchObject({ ok: false, stopReason: "exit_before_result" }); }); + it("kills a child that prints result but never exits on its own (#211)", async () => { + await create("result-then-hang"); + const dump = join(scratch, "dump.json"); + process.env.FAKE_CLAUDE_DUMP = dump; + + await instance.adapter.sendTurn({ threadId: "t-lingering", text: "go" }); + const done = await recorder.until((e) => e.type === "turn.completed"); + expect(done).toMatchObject({ ok: true }); + + const { pid } = JSON.parse(readFileSync(dump, "utf8")); + expect(typeof pid).toBe("number"); + + const alive = () => { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } + }; + const deadline = Date.now() + 5_000; + while (alive() && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 25)); + } + expect(alive()).toBe(false); + }, 10_000); + it("an exit before result becomes runtime.error + failed turn", async () => { await create("exit-early"); await instance.adapter.sendTurn({ threadId: "t-crash", text: "go" }); diff --git a/server/drivers/claude.ts b/server/drivers/claude.ts index 133b4a1e9..50b1b8db8 100644 --- a/server/drivers/claude.ts +++ b/server/drivers/claude.ts @@ -522,6 +522,12 @@ export const ClaudeDriver: ProviderDriver = { ) => { if (settled) return; settled = true; + // A one-shot `-p` process is expected to exit right after printing + // `result`, but a backgrounded MCP grandchild can keep it (or itself) + // alive — leaving a live process with a live broker connection that + // can raise a permission ask nobody can ever answer (issue #211). A + // no-op when the process already exited. + killCliTree(child); broker?.close(); // the config file holds live credentials — it must not outlive the turn if (mcpConfigPath) { diff --git a/server/testing/fake-claude-cli.ts b/server/testing/fake-claude-cli.ts index 37fd8adf7..8c0d4a76a 100755 --- a/server/testing/fake-claude-cli.ts +++ b/server/testing/fake-claude-cli.ts @@ -4,11 +4,19 @@ // scripted session. Failure modes are toggled by env var, mirroring how // the real thing misbehaves: // -// FAKE_CLAUDE_MODE happy (default) | exit-early | hang | malformed -// | stream (partial-message text deltas before the -// whole-message frame, plus subagent noise to drop) -// FAKE_CLAUDE_DUMP path to write {argv, env, prompt, mcpConfig} as JSON, -// so the test can assert on argv shape and env hygiene. +// FAKE_CLAUDE_MODE happy (default) | exit-early | hang | result-then-hang +// | malformed | stream (partial-message text deltas +// before the whole-message frame, plus subagent noise +// to drop) +// result-then-hang prints the same result as happy but +// never exits — simulates a backgrounded grandchild that +// outlives its turn (issue #211), for tests that must +// confirm the process is actually gone, not just that +// the driver emitted turn.completed. +// FAKE_CLAUDE_DUMP path to write {pid, argv, env, prompt, mcpConfig} as +// JSON, so the test can assert on argv shape, env +// hygiene, and (for result-then-hang) poll this process +// for liveness after the driver settles the turn. // mcpConfig is read back from the --mcp-config file the // way the real CLI reads it — the driver writes it to a // private temp file and deletes it when the turn settles, @@ -86,7 +94,10 @@ process.stdin.on("end", () => { /* leave null — the test will see it */ } } - writeFileSync(process.env.FAKE_CLAUDE_DUMP, JSON.stringify({ argv, env: process.env, prompt, mcpConfig }, null, 2)); + writeFileSync( + process.env.FAKE_CLAUDE_DUMP, + JSON.stringify({ pid: process.pid, argv, env: process.env, prompt, mcpConfig }, null, 2), + ); } const sessionId = argAfter("--resume") ?? argAfter("--session-id") ?? "fake-session"; @@ -135,5 +146,13 @@ process.stdin.on("end", () => { }); out({ type: "user", message: { content: [{ type: "tool_result", tool_use_id: "tu-1", is_error: false }] } }); out({ type: "result", is_error: false, stop_reason: "end_turn", total_cost_usd: 0.01, usage: { input_tokens: 10, cache_read_input_tokens: 2, output_tokens: 5 } }); + + if (mode === "result-then-hang") { + // printed `result` but never exits — the process the driver's settle() + // must now forcibly reap + setInterval(() => {}, 1_000); + return; + } + process.exit(0); }); From a5bcbcc896704e02794854c753925d1658eea7cf Mon Sep 17 00:00:00 2001 From: SomSamantray <> Date: Tue, 18 Aug 2026 16:56:04 +0530 Subject: [PATCH 25/52] fix(claude): drop permission asks that arrive after broker close MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit net.Server.close() only stops accepting new connections — it does not touch a connection that's already open. A still-alive child's MCP proxy could keep sending asks on such a connection after the turn ended, and the connection's data handler stayed fully wired to it, adding new pending entries and emitting request.opened cards for a turn the driver had already forgotten (active.delete(threadId) already ran). That card could then never be answered. Adds a closed flag set at the top of close(); any ask received while closed is always replied to directly on the connection (mirroring close()'s existing pending-ask handling) instead of registering a new pending entry or notifying onAsk — never a silent drop, since permission-proxy.ts's MCP tool call only resolves on an explicit answer or the connection's own error/close. Fixes milind-soni/OpenMausBot#211 --- server/drivers/claude.test.ts | 37 +++++++++++++++++++++++++++++++++++ server/drivers/claude.ts | 26 ++++++++++++++++++++++++ 2 files changed, 63 insertions(+) diff --git a/server/drivers/claude.test.ts b/server/drivers/claude.test.ts index 266b2e726..f304f0d65 100644 --- a/server/drivers/claude.test.ts +++ b/server/drivers/claude.test.ts @@ -700,6 +700,43 @@ describe("ClaudeDriver turns (fake CLI)", () => { conn.end(); await instance.adapter.interruptTurn("t-perm-dup-4"); await recorder.until((e) => e.type === "turn.completed"); + it("drops a late ask on an already-closed broker instead of a dead card (#211)", async () => { + await create("hang"); + await instance.adapter.sendTurn({ threadId: "t-perm-late", text: "go" }); + await recorder.until((e) => e.type === "session.started"); + + // Same connection stays open across the turn ending — the exact + // condition that let a still-alive child raise an unanswerable card. + const conn = connect(permissionSocketPath("t-perm-late")); + await new Promise((resolve, reject) => { + conn.on("connect", resolve); + conn.on("error", reject); + }); + + await instance.adapter.interruptTurn("t-perm-late"); + await recorder.until((e) => e.type === "turn.completed"); + + const opensBefore = recorder.events.filter((e) => e.type === "request.opened").length; + const reply = new Promise<{ id: string; behavior: string }>((resolve) => { + let buf = ""; + conn.on("data", (c) => { + buf += c; + const nl = buf.indexOf("\n"); + if (nl !== -1) resolve(JSON.parse(buf.slice(0, nl))); + }); + }); + conn.write(JSON.stringify({ t: "ask", id: "ask-late", tool: "Bash", input: { command: "rm -rf /" } }) + "\n"); + + // A dead card is a request.opened with no way to ever answer it — assert + // the late ask never becomes one, and the connection still gets a + // definite reply rather than hanging forever. + expect(await reply).toMatchObject({ id: "ask-late", behavior: "deny" }); + expect(recorder.events.filter((e) => e.type === "request.opened")).toHaveLength(opensBefore); + await expect(instance.adapter.respondToRequest("t-perm-late", "ask-late", { behavior: "allow" })).resolves.toBe( + "unavailable", + ); + + conn.end(); }); it("passes effort to the CLI, and omits the flag when unset", async () => { diff --git a/server/drivers/claude.ts b/server/drivers/claude.ts index 50b1b8db8..d19cedc1d 100644 --- a/server/drivers/claude.ts +++ b/server/drivers/claude.ts @@ -219,6 +219,14 @@ function createPermissionBroker(opts: { string, { ask: Ask; finish: (behavior: AskBehavior, message: string | undefined, source: AskResolutionSource) => void } >(); + // server.close() only stops accepting NEW connections — it does not touch + // a connection that's already open. A still-alive child's MCP proxy can + // keep sending asks on such a connection after the turn has ended, and + // this handler stays fully wired to it. Without this flag those asks would + // become new `pending` entries and `request.opened` cards for a turn the + // driver already forgot (`active.delete(threadId)` already ran), which can + // never be answered — the "zombie card" in issue #211. + let closed = false; try { unlinkSync(opts.socketPath); } catch {} @@ -256,6 +264,23 @@ function createPermissionBroker(opts: { continue; } const kind = msg.kind === "question" ? ("question" as const) : ("permission" as const); + if (closed) { + // Never register a pending entry or notify onAsk for a closed + // broker — but always reply. permission-proxy.ts only resolves an + // ask on an explicit {t:"answer"} or its own connection error/close, + // so a silent drop here would hang that MCP tool call forever. + try { + conn.write( + JSON.stringify({ + t: "answer", + id: askId, + behavior: kind === "question" ? "answer" : "deny", + message: kind === "question" ? "OpenMausBot: the turn is ending — wrap up." : "OpenMausBot: the turn ended", + }) + "\n", + ); + } catch {} + continue; + } const ask: Ask = { id: askId, kind, tool: msg.tool ?? "tool", input: msg.input ?? {}, at: Date.now() }; const finish = (behavior: AskBehavior, message: string | undefined, source: AskResolutionSource) => { if (!pending.delete(askId)) return; @@ -294,6 +319,7 @@ function createPermissionBroker(opts: { return true; }, close() { + closed = true; for (const p of [...pending.values()]) { if (p.ask.kind === "question") p.finish("answer", "OpenMausBot: the turn is ending — wrap up.", "system"); else p.finish("deny", "OpenMausBot: the turn ended", "system"); From 98ccf9aacf64aab4fea512fdc96f169020c762f1 Mon Sep 17 00:00:00 2001 From: SomSamantray <> Date: Tue, 18 Aug 2026 17:04:37 +0530 Subject: [PATCH 26/52] refactor(claude): extract systemEndedReply, shared by close() and the late-ask path The late-ask reply and close()'s pending-drain loop derived the same kind -> {behavior, message} mapping independently in two places. --- server/drivers/claude.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/server/drivers/claude.ts b/server/drivers/claude.ts index d19cedc1d..37786f5c3 100644 --- a/server/drivers/claude.ts +++ b/server/drivers/claude.ts @@ -183,6 +183,15 @@ const DENY_TIMEOUT_NOTE = const QUESTION_TIMEOUT_NOTE = "OpenMausBot: nobody answered in time. Use your best judgment and continue."; const DUPLICATE_ASK_ID_NOTE = "OpenMausBot: duplicate ask id — skipping this request."; +/** The system-source reply for an ask that outlives the turn — used both to + * drain in-flight `pending` asks on close() and to answer one that arrives + * on an already-closed broker (see the `closed` branch below). */ +function systemEndedReply(kind: Ask["kind"]): { behavior: AskBehavior; message: string } { + return kind === "question" + ? { behavior: "answer", message: "OpenMausBot: the turn is ending — wrap up." } + : { behavior: "deny", message: "OpenMausBot: the turn ended" }; +} + /** One human-readable line for an ask — what the card subtitle shows. */ function askSummary(ask: Ask): string { const input = ask.input ?? {}; @@ -270,14 +279,7 @@ function createPermissionBroker(opts: { // ask on an explicit {t:"answer"} or its own connection error/close, // so a silent drop here would hang that MCP tool call forever. try { - conn.write( - JSON.stringify({ - t: "answer", - id: askId, - behavior: kind === "question" ? "answer" : "deny", - message: kind === "question" ? "OpenMausBot: the turn is ending — wrap up." : "OpenMausBot: the turn ended", - }) + "\n", - ); + conn.write(JSON.stringify({ t: "answer", id: askId, ...systemEndedReply(kind) }) + "\n"); } catch {} continue; } @@ -321,8 +323,8 @@ function createPermissionBroker(opts: { close() { closed = true; for (const p of [...pending.values()]) { - if (p.ask.kind === "question") p.finish("answer", "OpenMausBot: the turn is ending — wrap up.", "system"); - else p.finish("deny", "OpenMausBot: the turn ended", "system"); + const { behavior, message } = systemEndedReply(p.ask.kind); + p.finish(behavior, message, "system"); } try { server.close(); From e687cad4200086fd319b4225c9233aaf75acc663 Mon Sep 17 00:00:00 2001 From: SomSamantray <> Date: Tue, 18 Aug 2026 17:11:19 +0530 Subject: [PATCH 27/52] test(claude): cover the question-kind branch of the late-ask reply systemEndedReply(kind) branches on question vs permission, but only the permission/deny arm was exercised by the existing late-ask test. --- server/drivers/claude.test.ts | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/server/drivers/claude.test.ts b/server/drivers/claude.test.ts index f304f0d65..c4df7fe60 100644 --- a/server/drivers/claude.test.ts +++ b/server/drivers/claude.test.ts @@ -739,6 +739,37 @@ describe("ClaudeDriver turns (fake CLI)", () => { conn.end(); }); + it("drops a late question on an already-closed broker with an answer, not a deny (#211)", async () => { + // systemEndedReply(kind) branches on "question" vs "permission" — cover + // the question arm too, since the deny arm above doesn't exercise it. + await create("hang"); + await instance.adapter.sendTurn({ threadId: "t-question-late", text: "go" }); + await recorder.until((e) => e.type === "session.started"); + + const conn = connect(permissionSocketPath("t-question-late")); + await new Promise((resolve, reject) => { + conn.on("connect", resolve); + conn.on("error", reject); + }); + + await instance.adapter.interruptTurn("t-question-late"); + await recorder.until((e) => e.type === "turn.completed"); + + const reply = new Promise<{ id: string; behavior: string }>((resolve) => { + let buf = ""; + conn.on("data", (c) => { + buf += c; + const nl = buf.indexOf("\n"); + if (nl !== -1) resolve(JSON.parse(buf.slice(0, nl))); + }); + }); + conn.write(JSON.stringify({ t: "ask", kind: "question", id: "q-late", tool: "ask_user", input: { question: "still there?" } }) + "\n"); + + expect(await reply).toMatchObject({ id: "q-late", behavior: "answer" }); + + conn.end(); + }); + it("passes effort to the CLI, and omits the flag when unset", async () => { await create(); const dump = join(scratch, "effort.json"); From 137bba657ac5f4a9f8317bdb0a3d3e5b5a2d0102 Mon Sep 17 00:00:00 2001 From: gus <42593099+lightcloud00@users.noreply.github.com> Date: Thu, 20 Aug 2026 19:07:53 -0400 Subject: [PATCH 28/52] fix(claude): integrate teardown with current main Keep closed-broker handling terminal ahead of active-turn duplicate ask checks, preserve exact late replies, and settle in broker-close/process-kill/cleanup order.\n\nFixes behavior from milind-soni/OpenMausBot#211 while retaining the original #229 author commits. --- ...-claude-turn-teardown-zombie-cards-plan.md | 6 ++-- server/drivers/claude.test.ts | 23 ++++++++++++--- server/drivers/claude.ts | 29 +++++++++++-------- 3 files changed, 39 insertions(+), 19 deletions(-) diff --git a/docs/plans/2026-08-18-001-fix-claude-turn-teardown-zombie-cards-plan.md b/docs/plans/2026-08-18-001-fix-claude-turn-teardown-zombie-cards-plan.md index bb38bdbf5..65e2a9e57 100644 --- a/docs/plans/2026-08-18-001-fix-claude-turn-teardown-zombie-cards-plan.md +++ b/docs/plans/2026-08-18-001-fix-claude-turn-teardown-zombie-cards-plan.md @@ -55,7 +55,7 @@ Rationale: `killCliTree` already does a full-strength kill per platform (POSIX: Alternative considered and rejected: add the same grace+SIGKILL escalation now — rejected as unjustified complexity without evidence a plain SIGTERM leaves processes behind in this codepath (unlike `antigravity.ts`, which already had multi-child cleanup to justify it), and because escalation doesn't help the self-detached case above anyway (a SIGKILL to the wrong process group is still a no-op). **KTD2: Guard late asks with a `closed` boolean checked inside the connection's `data` handler; on a late ask, always write a system-source deny/answer to the connection — never a silent drop.** -Rationale: the simplest fix that satisfies R2 without changing `close()`'s existing contract for in-flight `pending` asks. The `data` handler already runs inside the closure that owns `pending`, `timeoutMs`, and now a `closed` flag set at the top of `close()`; checking it before creating a new pending entry is a one-line, easily-tested guard. **The response must be an explicit system-source deny/answer, not a silent drop:** `server/permission-proxy.ts`'s `waiting` map (the child-side promise the CLI's `tools/call` is awaiting) is only resolved by an incoming `{t:"answer",...}` message or by the connection's own `error`/`close` firing `dead()` — nothing in `permission-proxy.ts` times out a single ask on its own. A silent drop on the broker side leaves that specific MCP tool call hanging until something else closes the connection, which is exactly the kind of hang R2 exists to prevent. (An earlier draft of this plan left "log and drop" as an equally-valid alternative; document review caught that this contradicts `permission-proxy.ts`'s actual behavior and R2 has been tightened to require the explicit reply.) +Rationale: the simplest fix that satisfies R2 without changing `close()`'s existing contract for in-flight `pending` asks. The `data` handler already runs inside the closure that owns `pending`, `timeoutMs`, and now a `closed` flag set at the top of `close()`; checking it before creating a new pending entry is a one-line, easily-tested guard. Closure handling takes precedence over the active-turn duplicate-ID guard. **The response must be an explicit system-source deny/answer, not a silent drop:** permissions receive `deny` with `OpenMausBot: the turn ended`; questions receive `answer` with `OpenMausBot: the turn is ending — wrap up.` `server/permission-proxy.ts`'s `waiting` map (the child-side promise the CLI's `tools/call` is awaiting) is only resolved by an incoming `{t:"answer",...}` message or by the connection's own `error`/`close` firing `dead()` — nothing in `permission-proxy.ts` times out a single ask on its own. A silent drop on the broker side leaves that specific MCP tool call hanging until something else closes the connection, which is exactly the kind of hang R2 exists to prevent. Alternatives considered and rejected: (a) track and destroy live connections in `close()` — rejected as more state for no additional correctness, since the goal is "never create an answerable-looking dead card," not "sever the pipe," and a killed child per KTD1 will sever it anyway in the common case; the `closed` flag plus an explicit reply covers the case where the child hasn't been killed yet. (b) reuse `active.has(threadId)` instead of a dedicated `closed` boolean, since `respondToRequest` already treats a missing `active` entry as "no active turn" — rejected because of an ordering hazard: `createPermissionBroker` is constructed *before* `active.set(threadId, ...)` runs in `sendTurn` (the broker needs to exist to build the MCP config passed to the spawn call), so `active.has(threadId)` would incorrectly read `false` during that brief legitimate startup window, denying an ask that arrives before the turn is even fully registered. A dedicated flag defaulting to `false` has no such window. **KTD3: New fake-CLI mode `result-then-hang` for testing fix A; no new mode needed for fix B.** @@ -77,7 +77,7 @@ Rationale: fix A's contract is about the OS process, not just emitted events — - `server/drivers/claude.test.ts` (new test; test file path already exists) **Approach:** -- In `settle()`, call `killCliTree(child)` unconditionally, before or alongside the existing `broker?.close()` and temp-dir cleanup. `child` is already in scope (defined earlier in `sendTurn` at the `spawnCli` call); do not introduce a new binding or rely on the later-declared `stop` const. +- In `settle()`, first call `broker?.close()` so closure is terminal and all current asks resolve, then call `killCliTree(child)` unconditionally before temp-dir cleanup, active-turn deletion, and `turn.completed`. `child` is already in scope (defined earlier in `sendTurn` at the `spawnCli` call); do not introduce a new binding or rely on the later-declared `stop` const. - `killCliTree` is a no-op when the process already exited (`child.exitCode !== null || child.signalCode !== null`), so this is safe for the common case where the CLI has already exited by the time `result` is parsed and `settle()` runs — R3's no-regression requirement holds by construction. - Add `result-then-hang` to `server/testing/fake-claude-cli.ts`: emit the same `system`/`assistant`/`user`/`result` sequence the default `happy` path does, then instead of `process.exit(0)`, call the same `setInterval(() => {}, 1_000)` the `hang` mode uses to stay alive. - **The test needs the fake CLI's real OS pid to verify it's actually dead, and nothing today exposes it.** `ProviderInstance.adapter` has no pid accessor, and `FAKE_CLAUDE_DUMP`'s payload (`{argv, env, prompt, mcpConfig}`) doesn't carry one. Add `pid: process.pid` to the object `fake-claude-cli.ts` writes via `writeFileSync(process.env.FAKE_CLAUDE_DUMP, ...)` — reusing the existing dump mechanism rather than adding a new one — so the test can read it back the same way existing tests already read `argv`/`env` from that file. @@ -106,7 +106,7 @@ Rationale: fix A's contract is about the OS process, not just emitted events — **Approach:** - Add a `let closed = false;` inside `createPermissionBroker`'s closure. -- At the top of the `conn.on("data", ...)` handler's per-line processing, after parsing `msg` but before creating the `Ask`/`pending` entry: if `closed` is true, write a system-source reply directly to `conn` — `{t:"answer", id: msg.id, behavior: "deny", message: DENY_TIMEOUT_NOTE}` for a permission ask, or `{t:"answer", id: msg.id, behavior:"answer", message: QUESTION_TIMEOUT_NOTE}` for a question — without registering a `pending` entry or calling `opts.onAsk`, then log the drop and return. **This must always reply; per KTD2/R2, silently returning with no reply is not an option** — it leaves `permission-proxy.ts`'s corresponding `tools/call` promise hanging, since that file only resolves an ask on an explicit `{t:"answer"}` or its own connection `error`/`close`. +- At the top of the `conn.on("data", ...)` handler's per-line processing, after parsing `msg` but before duplicate-ID handling or creating the `Ask`/`pending` entry: if `closed` is true, write a system-source reply directly to `conn` — `{t:"answer", id: msg.id, behavior: "deny", message: "OpenMausBot: the turn ended"}` for a permission ask, or `{t:"answer", id: msg.id, behavior:"answer", message: "OpenMausBot: the turn is ending — wrap up."}` for a question — without registering a `pending` entry or calling `opts.onAsk`, then return. **This must always reply; per KTD2/R2, silently returning with no reply is not an option** — it leaves `permission-proxy.ts`'s corresponding `tools/call` promise hanging, since that file only resolves an ask on an explicit `{t:"answer"}` or its own connection `error`/`close`. - Set `closed = true` as the first line of `close()`, before it iterates `pending`. - Do not change what `close()` does with the existing `pending` Map — that behavior (system-source deny for permissions, system-source answer for questions) is correct and already tested. diff --git a/server/drivers/claude.test.ts b/server/drivers/claude.test.ts index c4df7fe60..d5212b44c 100644 --- a/server/drivers/claude.test.ts +++ b/server/drivers/claude.test.ts @@ -700,6 +700,8 @@ describe("ClaudeDriver turns (fake CLI)", () => { conn.end(); await instance.adapter.interruptTurn("t-perm-dup-4"); await recorder.until((e) => e.type === "turn.completed"); + }); + it("drops a late ask on an already-closed broker instead of a dead card (#211)", async () => { await create("hang"); await instance.adapter.sendTurn({ threadId: "t-perm-late", text: "go" }); @@ -717,7 +719,7 @@ describe("ClaudeDriver turns (fake CLI)", () => { await recorder.until((e) => e.type === "turn.completed"); const opensBefore = recorder.events.filter((e) => e.type === "request.opened").length; - const reply = new Promise<{ id: string; behavior: string }>((resolve) => { + const reply = new Promise<{ id: string; behavior: string; message?: string }>((resolve) => { let buf = ""; conn.on("data", (c) => { buf += c; @@ -730,7 +732,11 @@ describe("ClaudeDriver turns (fake CLI)", () => { // A dead card is a request.opened with no way to ever answer it — assert // the late ask never becomes one, and the connection still gets a // definite reply rather than hanging forever. - expect(await reply).toMatchObject({ id: "ask-late", behavior: "deny" }); + expect(await reply).toMatchObject({ + id: "ask-late", + behavior: "deny", + message: "OpenMausBot: the turn ended", + }); expect(recorder.events.filter((e) => e.type === "request.opened")).toHaveLength(opensBefore); await expect(instance.adapter.respondToRequest("t-perm-late", "ask-late", { behavior: "allow" })).resolves.toBe( "unavailable", @@ -755,7 +761,8 @@ describe("ClaudeDriver turns (fake CLI)", () => { await instance.adapter.interruptTurn("t-question-late"); await recorder.until((e) => e.type === "turn.completed"); - const reply = new Promise<{ id: string; behavior: string }>((resolve) => { + const opensBefore = recorder.events.filter((e) => e.type === "request.opened").length; + const reply = new Promise<{ id: string; behavior: string; message?: string }>((resolve) => { let buf = ""; conn.on("data", (c) => { buf += c; @@ -765,7 +772,15 @@ describe("ClaudeDriver turns (fake CLI)", () => { }); conn.write(JSON.stringify({ t: "ask", kind: "question", id: "q-late", tool: "ask_user", input: { question: "still there?" } }) + "\n"); - expect(await reply).toMatchObject({ id: "q-late", behavior: "answer" }); + expect(await reply).toMatchObject({ + id: "q-late", + behavior: "answer", + message: "OpenMausBot: the turn is ending — wrap up.", + }); + expect(recorder.events.filter((e) => e.type === "request.opened")).toHaveLength(opensBefore); + await expect( + instance.adapter.respondToRequest("t-question-late", "q-late", { behavior: "answer", message: "yes" }), + ).resolves.toBe("unavailable"); conn.end(); }); diff --git a/server/drivers/claude.ts b/server/drivers/claude.ts index 37786f5c3..be8ace99e 100644 --- a/server/drivers/claude.ts +++ b/server/drivers/claude.ts @@ -256,6 +256,18 @@ function createPermissionBroker(opts: { } if (msg.t !== "ask") continue; const askId = String(msg.id ?? newId()); + const kind = msg.kind === "question" ? ("question" as const) : ("permission" as const); + if (closed) { + // Closure is terminal and takes precedence over every active-turn + // rule, including duplicate-id rejection. Never register a pending + // entry or notify onAsk, but always answer an existing connection: + // permission-proxy.ts only resolves on an explicit answer (or a + // connection error/close), so a silent drop would hang the tool. + try { + conn.write(JSON.stringify({ t: "answer", id: askId, ...systemEndedReply(kind) }) + "\n"); + } catch {} + continue; + } // `pending` is server-scoped, not per-connection: two asks with the // same id — a buggy/adversarial client, never a legitimate retry // (permission-proxy mints a fresh randomUUID per ask) — would @@ -272,17 +284,6 @@ function createPermissionBroker(opts: { } catch {} continue; } - const kind = msg.kind === "question" ? ("question" as const) : ("permission" as const); - if (closed) { - // Never register a pending entry or notify onAsk for a closed - // broker — but always reply. permission-proxy.ts only resolves an - // ask on an explicit {t:"answer"} or its own connection error/close, - // so a silent drop here would hang that MCP tool call forever. - try { - conn.write(JSON.stringify({ t: "answer", id: askId, ...systemEndedReply(kind) }) + "\n"); - } catch {} - continue; - } const ask: Ask = { id: askId, kind, tool: msg.tool ?? "tool", input: msg.input ?? {}, at: Date.now() }; const finish = (behavior: AskBehavior, message: string | undefined, source: AskResolutionSource) => { if (!pending.delete(askId)) return; @@ -550,13 +551,17 @@ export const ClaudeDriver: ProviderDriver = { ) => { if (settled) return; settled = true; + // Closing first marks the broker terminal and resolves every current + // ask before anything else can observe the turn as gone. Existing + // socket connections remain answerable through the broker's closed + // branch while the CLI tree is being reaped. + broker?.close(); // A one-shot `-p` process is expected to exit right after printing // `result`, but a backgrounded MCP grandchild can keep it (or itself) // alive — leaving a live process with a live broker connection that // can raise a permission ask nobody can ever answer (issue #211). A // no-op when the process already exited. killCliTree(child); - broker?.close(); // the config file holds live credentials — it must not outlive the turn if (mcpConfigPath) { try { From 2b3aba89d5ff9321eedbfeaaa482def8551749fc Mon Sep 17 00:00:00 2001 From: dev Date: Thu, 20 Aug 2026 23:12:15 -0300 Subject: [PATCH 29/52] feat: add pi coding agent as a native RPC engine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pi (@earendil-works/pi-coding-agent) exposes a JSON-RPC mode over stdio (`pi --mode rpc --no-session`) rather than ACP, so — like the Claude Code and Codex CLIs — it gets a native driver that speaks its own protocol and emits canonical RuntimeEvents. pi is a BYOK agent: credentials live in ~/.pi/agent/auth.json and are injected by the pi binary, so the driver holds no API key and needs no sign-in. - server/drivers/pi.ts: native ProviderDriver. Per-turn it spawns `pi --mode rpc --no-session`, resumes the prior session via switch_session (the sessionFile from session.started is the resumeCursor), pins the chosen model with set_model (splitting the picker's provider/modelId composite), and translates the RPC event stream (message_update text/thinking deltas, tool_execution_*, turn_end) into canonical events. toolUse turns are not settled — pi auto-continues to synthesize the reply, and settling early drops it. extension_ui_request select/confirm/input become request.opened; respondToRequest answers via extension_ui_response. A missing CLI surfaces as snapshot unavailable. - server/drivers/pi.test.ts + server/testing/fake-pi-cli.ts: contract tests against a scripted fake pi CLI — catalog parsing, the full happy turn, the toolUse auto-continue, permission brokering, interrupt, and snapshot. - builtIn.ts registers PiDriver; config.ts seeds pi into DEFAULT_FLEET and CUSTOM_ONLY so the engine instance appears (and merges into existing configs via PRODUCT_FLEET_ADDITIONS). `pnpm typecheck` and `pnpm test` pass; the live catalog is flagged `custom` so the model picker's Local pane lists pi's BYOK models. --- server/config.ts | 2 + server/drivers/builtIn.ts | 2 + server/drivers/pi.test.ts | 245 ++++++++++++++ server/drivers/pi.ts | 536 +++++++++++++++++++++++++++++++ server/testing/fake-pi-cli.ts | 163 ++++++++++ src/components/ProviderIcons.tsx | 17 + 6 files changed, 965 insertions(+) create mode 100644 server/drivers/pi.test.ts create mode 100644 server/drivers/pi.ts create mode 100755 server/testing/fake-pi-cli.ts diff --git a/server/config.ts b/server/config.ts index d5036ce38..3e739cc56 100644 --- a/server/config.ts +++ b/server/config.ts @@ -330,10 +330,12 @@ export function instanceConfigs(cfg: AppConfig): InstanceConfigMap { computer: { driver: "boxAgent" }, qwen: { driver: "qwenAgent" }, hermes: { driver: "hermesAgent" }, + pi: { driver: "piAgent" }, }; const CUSTOM_ONLY = { qwen: { driver: "qwenAgent" }, hermes: { driver: "hermesAgent" }, + pi: { driver: "piAgent" }, } as const; // New default-fleet engines that existing product configs would otherwise // never see. Custom-only engines stay in CUSTOM_ONLY so a one-off test map diff --git a/server/drivers/builtIn.ts b/server/drivers/builtIn.ts index 2d5d9c747..928bfee60 100644 --- a/server/drivers/builtIn.ts +++ b/server/drivers/builtIn.ts @@ -14,6 +14,7 @@ import { CursorAgentDriver } from "./acp/cursor.ts"; import { OpenCodeGoDriver } from "./acp/opencode-go.ts"; import { QwenAgentDriver } from "./acp/qwen.ts"; import { HermesAgentDriver } from "./acp/hermes.ts"; +import { PiDriver } from "./pi.ts"; export const BUILT_IN_DRIVERS: readonly AnyProviderDriver[] = [ GrokDriver, @@ -25,6 +26,7 @@ export const BUILT_IN_DRIVERS: readonly AnyProviderDriver[] = [ OpenCodeGoDriver, QwenAgentDriver, HermesAgentDriver, + PiDriver, ClaudeDriver, CodexDriver, AntigravityDriver, diff --git a/server/drivers/pi.test.ts b/server/drivers/pi.test.ts new file mode 100644 index 000000000..1f7257e67 --- /dev/null +++ b/server/drivers/pi.test.ts @@ -0,0 +1,245 @@ +// pi driver contract tests, run against the scripted fake `pi` CLI in +// server/testing/fake-pi-cli.ts: parse the live catalog, normalize a full +// RPC turn into canonical events, ride the toolUse→end_turn auto-continue, +// broker a permission ask, and report availability from `pi --version`. +// +// The fake CLI is a shebang script Windows cannot exec directly; spawnCli +// resolves it to `node