From 4f2bcad4baa2dbca7a3fe28f2b4c9cb80df3f215 Mon Sep 17 00:00:00 2001 From: Bharath Mohan <2254476+bharathm03@users.noreply.github.com> Date: Fri, 28 Aug 2026 21:19:19 +0530 Subject: [PATCH 1/2] An agent judges only on an argv someone has actually run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cursor-agent and mistral-vibe declared headless argvs that had never been executed — both CLIs refuse every invocation on this machine without CURSOR_API_KEY / MISTRAL_API_KEY, so their reach was inferred from help output rather than measured. AgentSpec.headless says outright that an entry goes in only after running it, and the cost of breaking that is not a mislabel: any non-sealed reach makes the agent judgeCapable, arming a supervisor over the user's working tree on an argv nobody has run. Both entries are gone; naming still works by borrowing an installed agent. Copilot's readonly entry stays — writes were re-tested through three vectors (plain create, an explicit shell call, and git init's side effect) and the system blocked all three. A failed naming spawn no longer costs the session its name. The single 'attempted' flag answered three questions with one bit and got two wrong: a signed-out CLI or a timeout marked the conversation spent, so it could never be named even after the cause was fixed. TitleAttempts splits them — an in-flight claim released in a finally, a bounded failure count, and a terminal done — and generateTitleFromContext now returns why it failed, so 'nothing installed can serve this' ends the budget while a spawn that merely failed does not. State a CLI offers no switch to skip is redirected per spawn rather than to a fixed path. COPILOT_HOME pointed at one directory per tool, which kept every session it was ever handed: ~51KB apiece plus a 352KB uncheckpointed WAL after two calls, in a %TEMP% Windows does not reclaim. HeadlessCommand.scratchEnv names the vars, and the runner creates the directory and deletes it with the spawn. --- bridge/CLAUDE.md | 4 +- bridge/src/agent-core.ts | 103 ++++++++++++------------- bridge/src/agents/headless.ts | 42 ++++++++++- bridge/src/agents/registry.ts | 67 ++++------------- bridge/src/agents/title-attempts.ts | 102 +++++++++++++++++++++++++ bridge/src/agents/title-generate.ts | 32 ++++++-- bridge/src/agents/types.ts | 26 ++++++- bridge/src/handler/judge.ts | 5 +- bridge/tests/agent-tools.test.ts | 23 +++--- bridge/tests/headless.test.ts | 67 ++++++++++++++++- bridge/tests/title-attempts.test.ts | 112 ++++++++++++++++++++++++++++ bridge/tests/title-generate.test.ts | 48 +++++++++++- 12 files changed, 495 insertions(+), 136 deletions(-) create mode 100644 bridge/src/agents/title-attempts.ts create mode 100644 bridge/tests/title-attempts.test.ts diff --git a/bridge/CLAUDE.md b/bridge/CLAUDE.md index b15b6fe0..51f21aae 100644 --- a/bridge/CLAUDE.md +++ b/bridge/CLAUDE.md @@ -30,7 +30,9 @@ into the prompt. A judge is never borrowed, and `judgeCapable` is exactly "has a non-sealed entry". The field's own docstring carries the rules a new entry must satisfy (no writes, and how it keeps the run out of the user's `--resume`); add one only after running it. A naming spawn runs in a throwaway cwd, NEVER the -session's checkout — see `headlessScratchCwd`. +session's checkout — see `headlessScratchCwd`. History a CLI offers no switch to +skip is redirected per spawn instead (`HeadlessCommand.scratchEnv`, a fresh dir +the runner deletes after), so never list a var that also carries credentials. The `agent:tools` advert (and the loopback `tools:list` reply) carries TWO arrays, and the split is load-bearing. `tools[]` is the PATH probe — what this diff --git a/bridge/src/agent-core.ts b/bridge/src/agent-core.ts index af8da11c..4f8aa8d5 100644 --- a/bridge/src/agent-core.ts +++ b/bridge/src/agent-core.ts @@ -43,6 +43,7 @@ import { antigravityCliHome } from "./agents/antigravity/title"; import { AntigravityTitleWatcher } from "./agents/antigravity/title-watcher"; import { resolveStructuredTitle } from "./agents/title-dispatch"; import { buildTitleContext, generateTitleFromContext } from "./agents/title-generate"; +import { TitleAttempts, type TitleOutcome } from "./agents/title-attempts"; import { agentSpec, BY_HOOK_NAME, handlerObservable } from "./agents/registry"; import { HandlerEngine, type HandlerEvent } from "./handler/engine"; import { createEntitlementReader, type TierClaimSource } from "./entitlement"; @@ -548,39 +549,18 @@ export async function buildAgentCore(opts: BuildAgentCoreOptions): Promise:` key: terminal ids - // contain colons of their own (`:setup`), so in a flat key space - // one terminal's release reaches another terminal's entries by prefix. - const titleGenAttempted = new Map>(); + // Title-generation budget per conversation, per terminal. The /session-title + // post repeats every turn, so without this a session whose agent never names + // itself would pay a model call per turn, forever. Keyed by agent session + // where there is one, not by slot, so a fresh thread in the same slot gets a + // fresh budget — see TitleTarget for why a chat slot keys on itself instead. + const titleAttempts = new TitleAttempts(); /** Conversation id for the attempt gate — see TitleTarget for why a chat slot * keys on itself rather than on an agent session id. */ const titleAttemptKey = (target: { terminalId: string; agentSessionId?: string }) => target.agentSessionId ?? target.terminalId; - function titleAttemptSpent(terminalId: string, conversationId: string): boolean { - return titleGenAttempted.get(terminalId)?.has(conversationId) ?? false; - } - function claimTitleAttempt(terminalId: string, conversationId: string): void { - const spent = titleGenAttempted.get(terminalId) ?? new Set(); - spent.add(conversationId); - titleGenAttempted.set(terminalId, spent); - } - /** - * Released with the namer's buffered title, never separately. The two halves - * answer the same question — has this slot been named — and a `forget` that - * dropped only the rank left a session whose generated name the next - * first-message read overwrote, with generation refused forever after. A - * resume reuses the agent session id, so the key alone cannot tell the runs - * apart. - */ function forgetTitleAttempts(terminalId: string): void { - titleGenAttempted.delete(terminalId); + titleAttempts.forget(terminalId); } /** Which agent each LIVE chat slot runs — see the driverFactory that fills it. */ const chatTools = new Map(); @@ -2934,14 +2914,14 @@ export async function buildAgentCore(opts: BuildAgentCoreOptions): Promise; + /** Env vars to point at a directory created for this spawn and deleted + * after it — see HeadlessCommand.scratchEnv. */ + scratchEnv?: string[]; }, ): Promise { const spawn = opts.spawn ?? Bun.spawn; let timer: ReturnType | undefined; let abandonTimer: ReturnType | undefined; + const scratch = makeScratchHome(opts.scratchEnv); try { const proc = spawn(cmd, { - cwd: opts.cwd, stdout: "pipe", stderr: "ignore", env: headlessEnv(opts.env), + cwd: opts.cwd, stdout: "pipe", stderr: "ignore", + env: headlessEnv({ ...opts.env, ...scratch?.env }), }); let timedOut = false; // Resolves only if the timeout fires AND the tree kill fails to end the @@ -151,9 +156,42 @@ export async function runHeadless( // budget holding the dead process alive. clearTimeout(timer); clearTimeout(abandonTimer); + scratch?.dispose(); } } +/** + * A private directory for one spawn's redirected state, or null when the + * command asked for none. + * + * Deleted on the way out, which is the whole point: these vars exist because + * the CLI has no ephemeral switch, and a fixed path would accumulate a session + * per call in a temp dir the OS does not reclaim on Windows. + */ +function makeScratchHome(vars?: string[]): + { env: Record; dispose: () => void } | null { + if (!vars?.length) return null; + let dir: string; + try { + dir = mkdtempSync(join(tmpdir(), "antgrid-headless-")); + } catch (err) { + // Falling through to the inherited value would run the spawn against the + // user's REAL agent home and write a session into their history, which is + // the one outcome these entries exist to prevent. + log.warn("no scratch home for a headless spawn: %s", err); + return null; + } + return { + env: Object.fromEntries(vars.map((name) => [name, dir])), + dispose: () => { + // A spawn that outlived its kill still holds its store open, and Windows + // refuses to unlink an open file. One leaked directory on the timeout + // path beats throwing from a `finally` that owes the caller a result. + try { rmSync(dir, { recursive: true, force: true }); } catch { /* leaked */ } + }, + }; +} + /** * The environment a headless spawn runs under. * diff --git a/bridge/src/agents/registry.ts b/bridge/src/agents/registry.ts index 525e03d6..ce600920 100644 --- a/bridge/src/agents/registry.ts +++ b/bridge/src/agents/registry.ts @@ -7,7 +7,7 @@ // compiler now demands. import { existsSync } from "node:fs"; -import { homedir, tmpdir } from "node:os"; +import { homedir } from "node:os"; import { join } from "node:path"; import { readCodexVersionJson, codexHomeDir } from "./codex/home"; import { antigravityCliHome, resolveAntigravityTitle } from "./antigravity/title"; @@ -34,20 +34,6 @@ import { terminalForkHandoff } from "./fork-handoff"; import { pickHeadlessFrom, type AgentKey, type AgentSpec } from "./types"; -// A throwaway store for a headless spawn whose CLI offers no ephemeral flag. -// It sits under the OS temp dir rather than the agent's own home so that -// nothing a one-shot call writes can surface in that agent's --resume picker, -// and so the OS reclaims it. Stable per tool because HeadlessCommand.env is -// static: what is disposable is the directory, never a particular session. -// -// A PATH ONLY: this runs while the AGENTS literal is built, so anything done -// here happens on every import of this file — including each short-lived -// `bridge hook` invocation the agent is blocking on, and on machines that run -// neither of these agents. -function headlessScratchDir(tool: string): string { - return join(tmpdir(), "antgrid-headless", tool); -} - export const AGENTS: Record = { "claude-code": { bin: "claude", @@ -243,25 +229,13 @@ export const AGENTS: Record = { fork: terminalForkHandoff("Cursor"), hooks: cursorHooks, augmentsDefaultSpec: true, - headless: { - // --mode ask is Cursor's own read-only Q&A mode; -p alone would keep the - // write and shell tools its help advertises for print mode. - readonly: { - cmd: (prompt, model) => [ - "cursor-agent", "-p", "--mode", "ask", "--output-format", "text", - ...(model ? ["--model", model] : []), prompt, - ], - // No ephemeral flag exists, so the conversation store is redirected - // instead. CURSOR_DATA_DIR is the right half of Cursor's two-directory - // split — it holds conversations, while credentials live under - // CURSOR_CONFIG_DIR (cli-config.json), which is left alone. Redirecting - // the config dir instead would sign the spawn out, the same way - // claude's --bare forces API-key auth and fails closed on a - // subscription. - env: { CURSOR_DATA_DIR: headlessScratchDir("cursor-agent") }, - noHistory: "ephemeral-store", - }, - }, + // No headless entry. `-p --mode ask` reads like the right argv and has + // never been run: cursor-agent exits 1 on every invocation without an + // `agent login` or CURSOR_API_KEY, so nothing about that argv's reach has + // been observed. Absence is the honest answer, and it is not only about a + // wrong label — ANY non-sealed reach makes the agent judge-capable, which + // would arm a supervisor over the user's working tree on an argv nobody + // has run. Naming is unaffected: a "none" call borrows an installed agent. }, "github-copilot": { bin: "copilot", @@ -303,13 +277,14 @@ export const AGENTS: Record = { ...(model ? ["--model", model] : []), ], // Copilot has no ephemeral flag — a -p run writes session-store.db and a - // whole session-state// tree — so the home is redirected instead. + // whole session-state// tree — so the home is redirected to a + // directory that lives only as long as the spawn. // Safe for auth, and that is NOT the generalization it looks like: // credentials do not live under COPILOT_HOME at all (no GH_TOKEN or // GITHUB_TOKEN path either), so a run against an EMPTY scratch home // still authenticates. Measured, because the opposite is true of vibe, // where the same move would take the credentials with it. - env: { COPILOT_HOME: headlessScratchDir("github-copilot") }, + scratchEnv: ["COPILOT_HOME"], noHistory: "ephemeral-store", }, }, @@ -399,22 +374,10 @@ export const AGENTS: Record = { titleSource: "osc", resume: () => [], initialPrompt: () => [], - headless: { - // Vibe selects a model through --agent profiles, never a --model flag, so - // the model argument is dropped rather than passed: `ask` is its built-in - // read-only profile. --trust is required, not a widening — without it a - // programmatic run stops on the interactive trust prompt, and it is scoped - // to this invocation (never written to trusted_folders.toml). - readonly: { - cmd: (prompt) => ["vibe", "-p", prompt, "--agent", "ask", "--trust", "--output", "text"], - // Vibe's home holds the credentials (.env) as well as the session log, - // so VIBE_HOME must NOT be redirected. Its config layer reads VIBE_* - // with `__` for nesting, which reaches session_logging.enabled directly - // — the same switch that makes SessionLogger write nothing at all. - env: { VIBE_SESSION_LOGGING__ENABLED: "false" }, - noHistory: "flag", - }, - }, + // No headless entry, for the reason spelled out on cursor-agent above: + // `vibe -p --agent ask` is unrun — every invocation here exits 1 on a + // missing MISTRAL_API_KEY — and shipping an unverified argv would arm the + // judge on it. fork: terminalForkHandoff("Mistral Vibe"), }, }; diff --git a/bridge/src/agents/title-attempts.ts b/bridge/src/agents/title-attempts.ts new file mode 100644 index 00000000..f6327d24 --- /dev/null +++ b/bridge/src/agents/title-attempts.ts @@ -0,0 +1,102 @@ +/** How a claimed attempt ended — see {@link TitleAttempts.settle}. */ +export type TitleOutcome = + /** A title was generated and applied. Nothing tries again. */ + | "named" + /** The spawn ran and produced no usable title (signed-out CLI, timeout, + * rambling answer). Counts against the budget; a later turn may retry. */ + | "failed" + /** Nothing installed can serve the call at all. Not this attempt's failure, + * so it does not count against the budget — it ends it. */ + | "unavailable" + /** The generated title was thrown away for a reason unrelated to generating + * it: the user renamed the session mid-spawn, or the conversation moved on. + * Releases the claim and records nothing. */ + | "abandoned"; + +interface AttemptState { + /** A spawn is running for this conversation right now. Mutual exclusion only; + * always cleared, however the spawn ends. */ + inFlight: boolean; + /** Spawns that ran and produced no usable title. Bounded, because the cause + * is usually not transient at all and each attempt costs a ~45s budget. */ + failures: number; + /** Named, or given up on. Terminal either way — only `forget` reopens it. */ + done: boolean; +} + +/** + * Whether a session may spend another title-generation spawn. + * + * Three fields rather than the single "already attempted" flag this replaces, + * because that flag answered three questions with one bit and got two of them + * wrong: a spawn that FAILED — a signed-out CLI, a timeout — marked the + * conversation spent, so the session could never be named afterwards even once + * the cause was fixed. Collapsing them back into a bare retry count + * reintroduces the other half: a count cannot also exclude a second spawn while + * the first is still running, and two turns ending at once both start one. + * + * Keyed by conversation within terminal, never by a flat `:` + * string: terminal ids contain colons of their own (`:setup`), so + * in a flat key space one terminal's release reaches another's entries by + * prefix. + */ +export class TitleAttempts { + private readonly byTerminal = new Map>(); + + constructor(private readonly maxFailures = 2) {} + + /** + * Whether a further spawn would be refused. Reads only, so it is safe as the + * early-out on a hot path — every turn of every session posts a title, and + * without it each one pays a transcript read only to be refused after it. + */ + refused(terminalId: string, conversationId: string): boolean { + const state = this.byTerminal.get(terminalId)?.get(conversationId); + if (!state) return false; + return state.done || state.inFlight || state.failures >= this.maxFailures; + } + + /** + * Claim the conversation for one spawn; false when it is refused. + * + * Check and set together and without an await between them, which is what + * makes it exclusion rather than a hint: two turns can end while a first + * spawn is still running, and a caller that re-read {@link refused} and then + * claimed would let both through. + * + * Every true MUST be paired with a {@link settle} in a `finally` — a claim + * that outlives its spawn is the permanent refusal this class exists to + * remove. + */ + begin(terminalId: string, conversationId: string): boolean { + if (this.refused(terminalId, conversationId)) return false; + let perTerminal = this.byTerminal.get(terminalId); + if (!perTerminal) this.byTerminal.set(terminalId, (perTerminal = new Map())); + const state = perTerminal.get(conversationId) + ?? { inFlight: false, failures: 0, done: false }; + state.inFlight = true; + perTerminal.set(conversationId, state); + return true; + } + + /** Release a claim and record how it ended. */ + settle(terminalId: string, conversationId: string, outcome: TitleOutcome): void { + const state = this.byTerminal.get(terminalId)?.get(conversationId); + if (!state) return; + state.inFlight = false; + if (outcome === "named" || outcome === "unavailable") state.done = true; + else if (outcome === "failed") state.failures += 1; + } + + /** + * Released with the namer's buffered title, never separately. The two halves + * answer the same question — has this slot been named — and a `forget` that + * dropped only the rank left a session whose generated name the next + * first-message read overwrote, with generation refused forever after. A + * resume reuses the agent session id, so the key alone cannot tell the runs + * apart. + */ + forget(terminalId: string): void { + this.byTerminal.delete(terminalId); + } +} diff --git a/bridge/src/agents/title-generate.ts b/bridge/src/agents/title-generate.ts index 8925447a..16b0e091 100644 --- a/bridge/src/agents/title-generate.ts +++ b/bridge/src/agents/title-generate.ts @@ -98,6 +98,21 @@ export async function buildTitleContext(opts: { return context || null; } +/** + * A generated title, or why there is none. + * + * The two reasons are NOT interchangeable to the caller, which is the whole + * point of returning one rather than a bare null: "failed" is a spawn that ran + * and did not produce a usable title — a signed-out CLI, a timeout, a rambling + * answer — any of which the next turn may not repeat, so it is worth a bounded + * retry. "unavailable" is the machine's answer, not this attempt's: no + * installed agent declares an argv that can serve the call at all, so every + * retry would re-read a transcript to reach the same refusal. + */ +export type TitleGeneration = + | { ok: true; title: string } + | { ok: false; reason: "unavailable" | "failed" }; + /** * Name a session by asking a headless CLI, rather than waiting to see whether * the agent names it for us. @@ -112,7 +127,7 @@ export async function buildTitleContext(opts: { * least: the conversation is inlined into the prompt, so it asks for `need: * "none"` and takes whichever installed agent can serve it (see resolveHeadless). * - * Never throws — every failure is a null and the caller keeps the name it has. + * Never throws — every failure is a `reason` and the caller keeps the name it has. */ export async function generateTitleFromContext(context: string, opts: { tool: string; @@ -121,24 +136,27 @@ export async function generateTitleFromContext(context: string, opts: { spawn?: typeof Bun.spawn; /** Test seam; production reads PATH via detectInstalledTools(). */ installedTools?: string[]; -}): Promise { +}): Promise { // `need: "none"` — the conversation is inlined into the prompt, so this asks // for the tightest argv the agent has rather than one that can reach the repo. const picked = resolveHeadless(opts.tool, "none", opts.installedTools); - if (!picked) return null; + if (!picked) return { ok: false, reason: "unavailable" }; logBorrow("none", opts.tool, picked.tool); const result = await runHeadless(picked.command.cmd(buildPrompt(context), opts.model), { cwd: headlessScratchCwd(), timeoutMs: opts.timeoutMs ?? DEFAULT_TIMEOUT_MS, spawn: opts.spawn, env: picked.command.env, + scratchEnv: picked.command.scratchEnv, }); // A timeout or a non-zero exit discards the output rather than parsing it. // These CLIs print their refusals to STDOUT and they are short: "Invalid API // key · Please run /login" clears every one of parseTitleFromOutput's checks // and reads as a title. With the `self` rank outranking the first-message - // re-read, and one attempt per session, that error string would be the - // session's name for good. - if (!result || result.code !== 0) return null; - return parseTitleFromOutput(result.stdout); + // re-read, that error string would be the session's name for good. + if (!result || result.code !== 0) return { ok: false, reason: "failed" }; + const title = parseTitleFromOutput(result.stdout); + // An unparseable answer is a failed attempt, not an absent capability: the + // spawn worked and the model rambled, which the next turn may not repeat. + return title ? { ok: true, title } : { ok: false, reason: "failed" }; } diff --git a/bridge/src/agents/types.ts b/bridge/src/agents/types.ts index 0db1075c..41900e6d 100644 --- a/bridge/src/agents/types.ts +++ b/bridge/src/agents/types.ts @@ -243,6 +243,24 @@ export interface HeadlessCommand { /** Merged over the bridge's environment for this spawn ONLY — never for a * terminal session, which is the spec-level `env`. */ env?: Record; + /** + * Env vars to point at a private, empty directory that lives exactly as long + * as the spawn — the runner creates one, sets every name here to it, and + * deletes it afterwards (see runHeadless). + * + * For a CLI with no ephemeral switch, whose state therefore has to be + * redirected rather than turned off. A fixed path under the temp dir is not + * enough: it is the AGENT's store, so it keeps every session it is ever + * pointed at — one measured copilot run left ~51KB of session-state plus a + * 352KB uncheckpointed WAL, in a directory Windows never reclaims. Per-spawn + * makes the ceiling one call rather than the machine's lifetime. + * + * Only for state that is safe to lose: a var carrying CREDENTIALS must never + * be listed, or every spawn starts signed out (COPILOT_HOME is listable + * precisely because copilot's auth is not under it — measured; vibe's home is + * the counter-example). + */ + scratchEnv?: string[]; /** * HOW this argv keeps the run out of the user's own history. Stated rather * than assumed because nothing else can check it: no passing test can tell a @@ -250,10 +268,10 @@ export interface HeadlessCommand { * does shows up in the user's own `--resume` picker forever. * * "flag" — an explicit off switch, argv or env (claude's - * --no-session-persistence, codex's --ephemeral, - * vibe's VIBE_SESSION_LOGGING__ENABLED=false). - * "ephemeral-store" — `env` points the agent's store somewhere disposable - * (opencode's OPENCODE_DB=:memory:). + * --no-session-persistence, codex's --ephemeral). + * "ephemeral-store" — `env` or `scratchEnv` points the agent's store + * somewhere disposable (opencode's OPENCODE_DB=:memory:, + * copilot's per-spawn COPILOT_HOME). * "stateless" — the CLI writes no history in this mode at all. * * An agent whose CLI offers none of the three gets no entry at that reach: diff --git a/bridge/src/handler/judge.ts b/bridge/src/handler/judge.ts index 0a94e9e8..06a019e1 100644 --- a/bridge/src/handler/judge.ts +++ b/bridge/src/handler/judge.ts @@ -58,7 +58,10 @@ async function runWithRetry(opts: { // masquerade as a title (see runHeadless). const run = (p: string, timeoutMs: number) => runHeadless( resolveCmd(judge.command.cmd(p, opts.model), p), - { cwd: opts.cwd, timeoutMs, spawn, env: judge.command.env }, + { + cwd: opts.cwd, timeoutMs, spawn, + env: judge.command.env, scratchEnv: judge.command.scratchEnv, + }, ); const prompt = opts.makePrompt(path); diff --git a/bridge/tests/agent-tools.test.ts b/bridge/tests/agent-tools.test.ts index 5e64e7e5..fc7afe9c 100644 --- a/bridge/tests/agent-tools.test.ts +++ b/bridge/tests/agent-tools.test.ts @@ -55,15 +55,25 @@ test("buildAgentCatalog describes the whole registry in declaration order", () = // distinction the descriptor exists to carry. Judge-capable and unobservable // are independent: it declares a headless argv that reaches the repo, and // still reports no turn boundaries for anything to watch. + expect(byKey["kilo"]).toEqual({ + tool: "kilo", + label: "Kilo", + chatCapable: false, + judgeCapable: true, + handler: { terminal: false, chat: false }, + }); + // Detectable and nameable, but not a judge: cursor-agent declares no headless + // argv VERIFIED at any reach. The one it used to carry was written from its + // help output and never run, which is the whole distance between naming (a + // borrowed "none" call) and arming a supervisor over the working tree. expect(byKey["cursor-agent"]).toEqual({ tool: "cursor-agent", label: "Cursor", chatCapable: false, - judgeCapable: true, + judgeCapable: false, handler: { terminal: false, chat: false }, }); - // The other half of that pair: no headless argv verified at any reach, so it - // cannot judge either. + // The same verdict reached the other way: no headless block at all. expect(byKey["kimi"]).toEqual({ tool: "kimi", label: "Kimi", @@ -72,13 +82,6 @@ test("buildAgentCatalog describes the whole registry in declaration order", () = handler: { terminal: false, chat: false }, }); expect(byKey["opencode"].handler).toEqual({ terminal: true, chat: true }); - expect(byKey["kilo"]).toEqual({ - tool: "kilo", - label: "Kilo", - chatCapable: false, - judgeCapable: true, - handler: { terminal: false, chat: false }, - }); }); test("agent:tools carries the descriptor array through the schema", () => { diff --git a/bridge/tests/headless.test.ts b/bridge/tests/headless.test.ts index d0df7fe0..e95bc3f1 100644 --- a/bridge/tests/headless.test.ts +++ b/bridge/tests/headless.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { existsSync } from "node:fs"; import { pickHeadless, resolveHeadless, runHeadless } from "../src/agents/headless"; import { AGENTS, judgeCapable } from "../src/agents/registry"; @@ -46,8 +47,7 @@ describe("judgeCapable", () => { // working tree. Adding an agent here is the point at which that has to be a // decision. const JUDGE_CAPABLE = new Set([ - "claude-code", "codex", "opencode", "cursor-agent", "github-copilot", - "kilo", "mistral-vibe", + "claude-code", "codex", "opencode", "github-copilot", "kilo", ]); test("is exactly the set of agents we have armed a judge for", () => { @@ -124,3 +124,66 @@ describe("runHeadless", () => { expect(result).toEqual({ stdout: "", code: null, timedOut: true }); }, 10_000); }); + +// The state redirect for a CLI with no ephemeral switch. What matters is the +// LIFETIME: a fixed path would keep a session per call in a temp dir Windows +// never reclaims, so the directory has to be gone by the time the call returns. +describe("scratchEnv", () => { + /** Records the env it was handed and whether the scratch dir was real at + * spawn time — the moment that matters, since the runner deletes it after. */ + function envCapturingSpawn(stdout = "ok") { + const seen: Array> = []; + const existed: boolean[] = []; + const spawn = ((_cmd: string[], o: Record) => { + const env = o.env as Record; + seen.push(env); + existed.push(Boolean(env.COPILOT_HOME) && existsSync(env.COPILOT_HOME)); + return { + stdout: new Response(stdout).body, + exited: Promise.resolve(0), + kill() {}, + }; + }) as unknown as typeof Bun.spawn; + return { spawn, seen, existed }; + } + + const run = (spawn: typeof Bun.spawn, scratchEnv?: string[]) => runHeadless( + ["agent", "-p", "x"], { cwd: process.cwd(), timeoutMs: 5_000, spawn, scratchEnv }, + ); + + test("the spawn sees a real directory, and it is gone once the call returns", async () => { + const { spawn, seen, existed } = envCapturingSpawn(); + await run(spawn, ["COPILOT_HOME"]); + expect(existed[0]).toBe(true); + expect(existsSync(seen[0]!.COPILOT_HOME!)).toBe(false); + }); + + test("every spawn gets its own, so calls cannot accumulate in one store", async () => { + const { spawn, seen } = envCapturingSpawn(); + await run(spawn, ["COPILOT_HOME"]); + await run(spawn, ["COPILOT_HOME"]); + expect(seen[0]!.COPILOT_HOME).not.toBe(seen[1]!.COPILOT_HOME); + }); + + test("several vars share the one directory", async () => { + const { spawn, seen } = envCapturingSpawn(); + await run(spawn, ["COPILOT_HOME", "OTHER_HOME"]); + expect(seen[0]!.OTHER_HOME).toBe(seen[0]!.COPILOT_HOME!); + }); + + // Absence must leave the inherited value alone rather than blank it: an agent + // that reads its home from the environment would otherwise be redirected by a + // command that never asked to be. + test("a command that asks for none has the var untouched", async () => { + const prior = process.env.COPILOT_HOME; + process.env.COPILOT_HOME = "the-user-s-own-home"; + try { + const { spawn, seen } = envCapturingSpawn(); + await run(spawn); + expect(seen[0]!.COPILOT_HOME).toBe("the-user-s-own-home"); + } finally { + if (prior === undefined) delete process.env.COPILOT_HOME; + else process.env.COPILOT_HOME = prior; + } + }); +}); diff --git a/bridge/tests/title-attempts.test.ts b/bridge/tests/title-attempts.test.ts new file mode 100644 index 00000000..a9bbfb94 --- /dev/null +++ b/bridge/tests/title-attempts.test.ts @@ -0,0 +1,112 @@ +import { describe, expect, test } from "bun:test"; + +import { TitleAttempts } from "../src/agents/title-attempts"; + +const T = "term-1"; +const C = "conv-1"; + +describe("mutual exclusion", () => { + // The half a bare retry count cannot express: two turns ending at once must + // not both spawn, and the second is refused while the first is still running + // even though it has failed nothing. + test("a claim excludes a second spawn until it settles", () => { + const a = new TitleAttempts(); + expect(a.begin(T, C)).toBe(true); + expect(a.begin(T, C)).toBe(false); + expect(a.refused(T, C)).toBe(true); + a.settle(T, C, "failed"); + expect(a.begin(T, C)).toBe(true); + }); + + test("a claim on one conversation leaves the others alone", () => { + const a = new TitleAttempts(); + a.begin(T, C); + expect(a.begin(T, "conv-2")).toBe(true); + expect(a.begin("term-2", C)).toBe(true); + }); +}); + +describe("budget", () => { + // The regression this class was written for: a spawn that failed used to mark + // the conversation spent, so a session whose CLI was merely signed out could + // never be named afterwards — not even once the user logged in. + test("a failure is retriable, and the budget is bounded", () => { + const a = new TitleAttempts(2); + a.begin(T, C); + a.settle(T, C, "failed"); + expect(a.refused(T, C)).toBe(false); + + a.begin(T, C); + a.settle(T, C, "failed"); + expect(a.refused(T, C)).toBe(true); + expect(a.begin(T, C)).toBe(false); + }); + + test("an unavailable verdict ends the budget outright", () => { + // Nothing installed can serve the call, so a retry would re-read a + // transcript every turn to reach the same refusal. + const a = new TitleAttempts(2); + a.begin(T, C); + a.settle(T, C, "unavailable"); + expect(a.refused(T, C)).toBe(true); + }); + + test("a name is final", () => { + const a = new TitleAttempts(); + a.begin(T, C); + a.settle(T, C, "named"); + expect(a.refused(T, C)).toBe(true); + }); + + // A title discarded because the user renamed the session mid-spawn says + // nothing about whether generation works, so it must not consume budget — + // only release the claim. + test("an abandoned attempt costs nothing", () => { + const a = new TitleAttempts(2); + for (let i = 0; i < 5; i++) { + expect(a.begin(T, C)).toBe(true); + a.settle(T, C, "abandoned"); + } + expect(a.refused(T, C)).toBe(false); + }); +}); + +describe("forget", () => { + test("clears a terminal's conversations, and only that terminal's", () => { + const a = new TitleAttempts(); + a.begin(T, C); + a.settle(T, C, "named"); + a.begin("term-2", C); + a.settle("term-2", C, "named"); + + a.forget(T); + expect(a.refused(T, C)).toBe(false); + expect(a.refused("term-2", C)).toBe(true); + }); + + // Terminal ids carry colons of their own (`:setup`), which is why + // the state is nested rather than held under a flat `:` key — + // in a flat key space this release would reach the other terminal by prefix. + test("a release does not reach a terminal whose id extends it", () => { + const a = new TitleAttempts(); + a.begin("chk", C); + a.settle("chk", C, "named"); + a.begin("chk:setup", C); + a.settle("chk:setup", C, "named"); + + a.forget("chk"); + expect(a.refused("chk:setup", C)).toBe(true); + }); +}); + +describe("settle without a claim", () => { + test("is a no-op rather than a phantom failure", () => { + // forget() can land between a claim and its settle (terminal exit while a + // spawn is in flight); the late settle must not resurrect the entry. + const a = new TitleAttempts(1); + a.begin(T, C); + a.forget(T); + a.settle(T, C, "failed"); + expect(a.refused(T, C)).toBe(false); + }); +}); diff --git a/bridge/tests/title-generate.test.ts b/bridge/tests/title-generate.test.ts index 43d0bc45..28592972 100644 --- a/bridge/tests/title-generate.test.ts +++ b/bridge/tests/title-generate.test.ts @@ -57,7 +57,9 @@ async function generateSessionTitle(opts: { installedTools?: string[]; }): Promise { const context = await buildTitleContext(opts); - return context ? await generateTitleFromContext(context, opts) : null; + if (!context) return null; + const result = await generateTitleFromContext(context, opts); + return result.ok ? result.title : null; } describe("parseTitleFromOutput", () => { @@ -214,3 +216,47 @@ describe("generateSessionTitle", () => { })).toBeNull(); }); }); + +// agent-core retries a "failed" within a small budget and never retries an +// "unavailable" (see TitleAttemptState), so which one a given failure produces +// decides whether a session can still be named later. Pinned here because the +// two are one `ok: false` to the type system and nothing else would catch a +// swap. +describe("failure reasons", () => { + test("no installed agent can serve the call is 'unavailable'", async () => { + const { spawn, calls } = fakeSpawn("Add retry to uploader"); + const r = await generateTitleFromContext("ctx", { + tool: "kimi", spawn, installedTools: [], + }); + expect(r).toEqual({ ok: false, reason: "unavailable" }); + // Nothing ran: the refusal is the machine's, not this attempt's. + expect(calls.length).toBe(0); + }); + + test("a non-zero exit is 'failed', so the next turn may try again", async () => { + const { spawn } = fakeSpawn("Invalid API key · Please run /login", { exitCode: 1 }); + const r = await generateTitleFromContext("ctx", { + tool: "claude-code", spawn, installedTools: ["claude-code"], + }); + expect(r).toEqual({ ok: false, reason: "failed" }); + }); + + test("a clean exit with unusable output is 'failed', not a title", async () => { + // Long enough to fail parseTitleFromOutput's cap — the spawn worked and the + // model rambled. + const { spawn } = fakeSpawn("Sure! Here is a title that goes on and on and " + + "on well past any reasonable length for naming a session"); + const r = await generateTitleFromContext("ctx", { + tool: "claude-code", spawn, installedTools: ["claude-code"], + }); + expect(r).toEqual({ ok: false, reason: "failed" }); + }); + + test("a usable title comes back as ok", async () => { + const { spawn } = fakeSpawn("Add retry to uploader"); + const r = await generateTitleFromContext("ctx", { + tool: "claude-code", spawn, installedTools: ["claude-code"], + }); + expect(r).toEqual({ ok: true, title: "Add retry to uploader" }); + }); +}); From 99ab8baf997a33b3bd8495ed249faa2a21067c27 Mon Sep 17 00:00:00 2001 From: Bharath Mohan <2254476+bharathm03@users.noreply.github.com> Date: Fri, 28 Aug 2026 22:15:15 +0530 Subject: [PATCH 2/2] vibe's ask profile gates on approval, it does not restrict writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read against mistralai/mistral-vibe v2.24.5 after the entry was removed for being unrun. The removed argv was wrong in a sharper way than that: `ask` is the approval-gated profile ('Requires approval for tool executions'), while `plan` is the read-only one and the only builtin pinning write_file and edit to permission 'never'. Programmatic mode denies every callback it is handed, which is what made ask look read-only — a write fails closed on an approval nothing can answer. That safety is config-level, never argv-level. An agent profile is only another config layer; ask contributes no bypass_tool_permissions key, so a user's own config survives it, and the loop returns EXECUTE before consulting any permission once that is set. Nothing raises an approval, so nothing is denied — and the same switch defeats `--agent plan`. Recorded on the entry so the argv is not reintroduced as 'readonly'. --- bridge/src/agents/registry.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/bridge/src/agents/registry.ts b/bridge/src/agents/registry.ts index ce600920..191865d8 100644 --- a/bridge/src/agents/registry.ts +++ b/bridge/src/agents/registry.ts @@ -374,10 +374,21 @@ export const AGENTS: Record = { titleSource: "osc", resume: () => [], initialPrompt: () => [], - // No headless entry, for the reason spelled out on cursor-agent above: - // `vibe -p --agent ask` is unrun — every invocation here exits 1 on a - // missing MISTRAL_API_KEY — and shipping an unverified argv would arm the - // judge on it. + // No headless entry, and `-p --agent ask` must not come back as one. Read + // against mistralai/mistral-vibe v2.24.5: `ask` is the APPROVAL-gated + // profile ("Requires approval for tool executions"), not a read-only one — + // that is `plan`, the only builtin pinning write_file and edit to + // permission "never". What makes ask LOOK read-only is that programmatic + // mode denies every callback it is handed (cli/programmatic.py), so a write + // fails closed on an approval it cannot answer. + // + // That is config-level, never argv-level, which is the whole distinction + // HeadlessReach draws: an agent profile is just another config layer, `ask` + // contributes no bypass_tool_permissions key, and the loop returns EXECUTE + // before consulting any permission the moment a user's own config sets one + // — no approval is raised, so nothing is denied. Even `--agent plan` falls + // to the same switch, so the best reach available here is "transcript", and + // it stays unrun besides (no MISTRAL_API_KEY on any machine measured). fork: terminalForkHandoff("Mistral Vibe"), }, };