diff --git a/commands/__tests__/agent-fallback.test.ts b/commands/__tests__/agent-fallback.test.ts index 21b9dde2fc..dd560023eb 100644 --- a/commands/__tests__/agent-fallback.test.ts +++ b/commands/__tests__/agent-fallback.test.ts @@ -34,7 +34,7 @@ test("refuses headless start before spawning", async () => { const spy = { called: false }; const res = await runAgentFallback("agent:start", { repo: REPO, cwd: "/tmp/x", prompt: "hi", surface: "headless" }, - { db, spawnHeadless: () => { spy.called = true; return { exited: Promise.resolve(0), stdout: async () => "" }; } }); + { db, spawnHeadless: () => { spy.called = true; return { exited: Promise.resolve(0), stdout: async () => "", sessionId: () => Promise.resolve(undefined) }; } }); expect(res.ok).toBe(false); if (res.ok) throw new Error("unreachable"); expect(res.error).toBe(HEADLESS_NEEDS_DAEMON); @@ -63,6 +63,21 @@ test("refuses bg start before spawning: bg needs the daemon-owned server this fa expect(spy.called).toBe(false); }); +// The fallback is a short-lived CLI process: codex's session-id capture is a +// detached poll with a ten-minute budget, so scheduling it here would hang the +// process (or be killed mid-flight) and capture nothing either way. +test("a codex herdr start schedules no session-id poll: the CLI would exit before it resolved", async () => { + const db = openStateDb(tmp()); + const calls: string[][] = []; + const res = await runAgentFallback("agent:start", + { repo: REPO, cwd: "/tmp/x", prompt: "hi", surface: "herdr", provider: "codex" }, + { db, herdrRunner: okRunner(calls) }); + expect(res.ok).toBe(true); + // Give a mis-scheduled poll a chance to make its first call before asserting. + await new Promise((r) => setTimeout(r, 50)); + expect(calls.some((c) => c[0] === "agent" && c[1] === "get")).toBe(false); +}); + test("list returns records", async () => { const db = openStateDb(tmp()); const calls: string[][] = []; diff --git a/commands/__tests__/agent.test.ts b/commands/__tests__/agent.test.ts index b6d491414f..8ce3b23229 100644 --- a/commands/__tests__/agent.test.ts +++ b/commands/__tests__/agent.test.ts @@ -52,6 +52,35 @@ describe("parseStartArgs", () => { expect(parseStartArgs(["--extra-args", "--bg"]).bg).toBeUndefined(); expect(parseStartArgs(["--extra-args", "--bg"]).extraArgs).toBe("--bg"); }); + + test("parseStartArgs: --provider codex", () => { + const parsed = __test__.parseStartArgs(["--provider", "codex", "--prompt", "go"]); + expect(parsed.provider).toBe("codex"); + }); + + test("parseStartArgs: invalid --provider throws", () => { + expect(() => __test__.parseStartArgs(["--provider", "cursor"])).toThrow(/invalid provider/); + }); + + test("parseStartArgs: --yolo sets the flag", () => { + const parsed = __test__.parseStartArgs(["--yolo", "--prompt", "go"]); + expect(parsed.yolo).toBe(true); + }); + + test("parseStartArgs: no --yolo leaves it undefined", () => { + const parsed = __test__.parseStartArgs(["--prompt", "go"]); + expect(parsed.yolo).toBeUndefined(); + }); + + // false, not undefined: the daemon reads `payload.yolo ?? the setting`, so + // only an explicit false can override a true agent..yolo. + test("parseStartArgs: --no-yolo sets yolo false, distinct from omitting it", () => { + expect(__test__.parseStartArgs(["--no-yolo", "--prompt", "go"]).yolo).toBe(false); + }); + + test("parseStartArgs: --yolo and --no-yolo together throw", () => { + expect(() => __test__.parseStartArgs(["--yolo", "--no-yolo"])).toThrow(/not both/); + }); }); describe("parseResumeArgs", () => { diff --git a/commands/agent-fallback.ts b/commands/agent-fallback.ts index 4fb97eb483..958e30866b 100644 --- a/commands/agent-fallback.ts +++ b/commands/agent-fallback.ts @@ -44,6 +44,11 @@ export async function runAgentFallback( const handlers = createAgentHandlers({ db, emitEvent: () => 0, + // Same reason headless is refused above: this process exits as soon as the + // verb returns, and codex's session-id capture is a detached poll that can + // outlive it by minutes. Either it holds the CLI open for the whole + // timeout or it dies mid-flight; neither is a capture. + skipSessionCapture: true, ...(deps.herdrRunner !== undefined && { herdrRunner: deps.herdrRunner }), ...(deps.spawnHeadless !== undefined && { spawnHeadless: deps.spawnHeadless }), }); diff --git a/commands/agent.ts b/commands/agent.ts index a5a2c5cc4b..be99264dc8 100644 --- a/commands/agent.ts +++ b/commands/agent.ts @@ -2,7 +2,8 @@ * rt agent: hand a prompt to a Claude Code agent and keep the receipt. * * rt agent start [--repo ] [--prompt | --prompt-file ] - * [--surface herdr|headless] [--model M] [--effort E] + * [--surface herdr|headless] [--provider claude|codex] + * [--model M] [--effort E] [--yolo | --no-yolo] * [--account A] [--label L] [--caller C] * [--workspace W] [--tab T] [--extra-args ""] * [--bg] [--json] @@ -28,7 +29,7 @@ import type { RtResponse } from "../packages/rt-client/src/index.ts"; const FLAGS_WITH_VALUES = new Set([ "--repo", "--prompt", "--prompt-file", "--surface", "--model", "--effort", - "--account", "--label", "--caller", "--workspace", "--tab", "--extra-args", + "--account", "--label", "--caller", "--workspace", "--tab", "--extra-args", "--provider", ]); function fail(msg: string): never { @@ -92,7 +93,7 @@ function parseSurface(s: string | undefined): AgentSurface | undefined { interface StartArgs { prompt?: string; surface?: AgentSurface; model?: string; effort?: string; account?: string; label?: string; caller?: string; workspace?: string; - tab?: string; extraArgs?: string; bg?: boolean; + tab?: string; extraArgs?: string; bg?: boolean; provider?: "claude" | "codex"; yolo?: boolean; } function parseStartArgs(args: string[]): StartArgs { @@ -104,6 +105,11 @@ function parseStartArgs(args: string[]): StartArgs { if (resolved !== undefined) out.prompt = resolved; const surface = parseSurface(flagValue(args, "--surface")); if (surface !== undefined) out.surface = surface; + const provider = flagValue(args, "--provider"); + if (provider !== undefined) { + if (provider !== "claude" && provider !== "codex") throw new Error(`invalid provider "${provider}": expected claude or codex`); + out.provider = provider; + } for (const [flag, key] of [ ["--model", "model"], ["--effort", "effort"], ["--account", "account"], ["--label", "label"], ["--caller", "caller"], ["--workspace", "workspace"], @@ -116,6 +122,13 @@ function parseStartArgs(args: string[]): StartArgs { if (surface === "headless") throw new Error("--bg is a herdr-surface option"); out.bg = true; } + // Three states, not two: --yolo forces on, --no-yolo forces off, and + // omitting both leaves yolo undefined so agent..yolo decides. + const yes = hasFlag(args, "--yolo"); + const no = hasFlag(args, "--no-yolo"); + if (yes && no) throw new Error("pass one of --yolo / --no-yolo, not both"); + if (yes) out.yolo = true; + else if (no) out.yolo = false; return out; } @@ -155,10 +168,12 @@ async function repoAndCwd(args: string[]): Promise<{ repo: string; cwd: string } function renderRecord(r: AgentRecord): string { const bits = [ `${r.id} ${repoLabel(r.repo)} ${r.surface}`, + `provider ${r.provider}`, `session ${r.sessionId}`, r.handle && `handle ${r.handle}`, r.model && `model ${r.model}`, r.account && `account ${r.account}`, + r.yolo && "yolo", r.paneId && `pane ${r.paneId}`, r.finishedAt !== undefined && `exit ${r.exitCode}`, r.lastResumedAt !== undefined && "resumed", @@ -177,10 +192,15 @@ async function runStart(args: string[]): Promise { const payload = { repo, cwd, ...parsed }; const data = unwrap(await dispatch("agent:start", payload, () => agentStart(payload)), "start"); if (args.includes("--json")) { + // Deliberately unannotated: a machine consumer needs the raw record, and + // the session id it carries for codex is provisional (see the note below). console.log(JSON.stringify({ ok: true, agent: data })); return; } console.log(renderRecord(data)); + if (data.provider === "codex") { + console.log(`note: codex mints its own session id; the one above is provisional. Capturing the real one needs the rt daemon running (start it with \`rt daemon start\` if it isn't) -- once it is, \`rt agent show ${data.id}\` confirms the real id before resuming.`); + } } async function runResume(args: string[]): Promise { diff --git a/lib/__tests__/agent-argv-codex.test.ts b/lib/__tests__/agent-argv-codex.test.ts new file mode 100644 index 0000000000..1ccbf9f6f5 --- /dev/null +++ b/lib/__tests__/agent-argv-codex.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, test } from "bun:test"; +import { buildCodexArgv, buildCodexPaneCommand } from "../agent-argv/index.ts"; + +const UUID = "6e225e74-4cb7-4aea-8807-6aa9011d4112"; + +describe("buildCodexArgv", () => { + const bins = { codex: "/abs/codex" }; + + test("headless start: exec --json, no session id emitted", () => { + const argv = buildCodexArgv({ session: { kind: "start", sessionId: UUID }, headless: true, prompt: "do it" }, bins); + expect(argv).toEqual(["/abs/codex", "exec", "--json", "do it"]); + expect(argv).not.toContain(UUID); + }); + + test("all knobs, headless start", () => { + const argv = buildCodexArgv({ + model: "gpt-6-astra", effort: "high", yolo: true, extraArgs: "--search", + session: { kind: "start", sessionId: UUID }, headless: true, prompt: "do it", + }, bins); + expect(argv).toEqual([ + "/abs/codex", "exec", "--json", + "-m", "gpt-6-astra", "-c", "model_reasoning_effort=high", + "--dangerously-bypass-approvals-and-sandbox", "--search", "do it", + ]); + }); + + test("headless resume: exec resume --json ", () => { + const argv = buildCodexArgv({ model: "gpt-6-astra", session: { kind: "resume", sessionId: UUID }, headless: true, prompt: "q" }, bins); + expect(argv).toEqual(["/abs/codex", "exec", "resume", "--json", "-m", "gpt-6-astra", UUID, "q"]); + }); + + test("non-headless resume emits no --json", () => { + const argv = buildCodexArgv({ session: { kind: "resume", sessionId: UUID }, headless: false }, bins); + expect(argv).toEqual(["/abs/codex", "exec", "resume", UUID]); + }); + + // Pins the --json gate, not a production shape: a herdr launch goes through + // buildCodexPaneCommand, so nothing in production calls this with + // headless: false on a start. + test("start with headless: false omits --json", () => { + const argv = buildCodexArgv({ session: { kind: "start", sessionId: UUID }, headless: false }, bins); + expect(argv).not.toContain("--json"); + expect(argv).toEqual(["/abs/codex", "exec"]); + }); + + test("headless without a prompt throws", () => { + expect(() => buildCodexArgv({ session: { kind: "start", sessionId: UUID }, headless: true }, bins)).toThrow(/prompt/); + }); +}); + +describe("buildCodexPaneCommand", () => { + test("start: cd + bare codex + flags + quoted prompt", () => { + const cmd = buildCodexPaneCommand("/repo dir", { + model: "gpt-6-astra", + session: { kind: "start", sessionId: UUID }, headless: false, prompt: "hi 'there'", + }); + expect(cmd).toBe(`cd '/repo dir' && codex '-m' 'gpt-6-astra' 'hi '\\''there'\\'''`); + }); + + test("resume: codex resume ", () => { + const cmd = buildCodexPaneCommand("/r", { session: { kind: "resume", sessionId: UUID }, headless: false }); + expect(cmd).toBe(`cd '/r' && codex resume '${UUID}'`); + }); + + test("env assignments precede the codex head", () => { + const cmd = buildCodexPaneCommand("/w/x", { + session: { kind: "start", sessionId: UUID }, + headless: false, + env: { RT_AGENT_ID: "ag-1" }, + }); + expect(cmd).toContain("cd '/w/x' && RT_AGENT_ID='ag-1' codex"); + }); + + test("yolo maps to --dangerously-bypass-approvals-and-sandbox", () => { + const cmd = buildCodexPaneCommand("/r", { yolo: true, session: { kind: "start", sessionId: UUID }, headless: false }); + expect(cmd).toContain("--dangerously-bypass-approvals-and-sandbox"); + }); +}); diff --git a/lib/__tests__/agent-argv.test.ts b/lib/__tests__/agent-argv.test.ts index d8f8e2d382..3adb86a060 100644 --- a/lib/__tests__/agent-argv.test.ts +++ b/lib/__tests__/agent-argv.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from "bun:test"; -import { buildClaudeArgv, buildPaneCommand, isValidSessionUuid, shellSingleQuote } from "../agent-argv.ts"; +import { buildClaudeArgv, buildPaneCommand, isValidSessionUuid, shellSingleQuote } from "../agent-argv/index.ts"; const UUID = "6e225e74-4cb7-4aea-8807-6aa9011d4112"; @@ -102,6 +102,16 @@ describe("buildClaudeArgv", () => { expect(argv).toContain("--settings"); expect(argv).toContain("/hooks/ag-2.json"); }); + + test("yolo maps to --dangerously-skip-permissions", () => { + const argv = buildClaudeArgv({ yolo: true, session: { kind: "start", sessionId: UUID }, headless: false }, bins); + expect(argv).toContain("--dangerously-skip-permissions"); + }); + + test("no yolo emits no bypass flag", () => { + const argv = buildClaudeArgv({ session: { kind: "start", sessionId: UUID }, headless: false }, bins); + expect(argv).not.toContain("--dangerously-skip-permissions"); + }); }); describe("buildPaneCommand", () => { diff --git a/lib/__tests__/agent-herdr.test.ts b/lib/__tests__/agent-herdr.test.ts index 60b0f5f180..6e28751ee5 100644 --- a/lib/__tests__/agent-herdr.test.ts +++ b/lib/__tests__/agent-herdr.test.ts @@ -2,7 +2,7 @@ import { expect, test } from "bun:test"; import { mkdtempSync, writeFileSync, chmodSync } from "fs"; import { tmpdir } from "os"; import { join } from "path"; -import { defaultHerdrRunner, herdrAgentWait, launchInWorkspace, resolveHerdrBin, type HerdrRunner } from "../agent-herdr.ts"; +import { defaultHerdrRunner, herdrAgentSessionId, herdrAgentWait, launchInWorkspace, resolveHerdrBin, type HerdrRunner } from "../agent-herdr.ts"; function scripted(responses: Record) { const calls: string[][] = []; @@ -94,6 +94,32 @@ test("herdrAgentWait builds the current verb (agent wait --until)", async () => expect(calls[0]).toEqual(["agent", "wait", "wA:p1", "--until", "idle", "--until", "done", "--timeout", "45000"]); }); +// Real `herdr agent get` output, observed 2026-09-15, after codex's first +// prompt completed: {"result":{"agent":{"agent_session":{"agent":"codex", +// "kind":"id","source":"herdr:codex","value":""},...},"type":"agent_info"}} +// A freshly launched, not-yet-prompted pane has no `agent_session` key at +// all -- that shape is what the "still empty" call below models. +test("herdrAgentSessionId returns the id once herdr reports it", async () => { + let call = 0; + const runner: HerdrRunner = async (args) => { + call += 1; + if (args[0] === "agent" && args[1] === "get") { + return call < 2 + ? { stdout: JSON.stringify({ result: { agent: {} } }), exitCode: 0 } + : { stdout: JSON.stringify({ result: { agent: { agent_session: { value: "s_herdr_456" } } } }), exitCode: 0 }; + } + throw new Error(`unexpected herdr call: ${args.join(" ")}`); + }; + const sid = await herdrAgentSessionId("pane-1", 5000, runner); + expect(sid).toBe("s_herdr_456"); +}); + +test("herdrAgentSessionId gives up at the timeout", async () => { + const runner: HerdrRunner = async () => ({ stdout: JSON.stringify({ result: { agent: {} } }), exitCode: 0 }); + const sid = await herdrAgentSessionId("pane-1", 600, runner); + expect(sid).toBeUndefined(); +}); + test("resolveHerdrBin prefers HERDR_BIN over everything else", () => { const bin = resolveHerdrBin({ HERDR_BIN: "/custom/herdr", HOME: "/home/x" }, () => "/opt/homebrew/bin/herdr"); expect(bin).toBe("/custom/herdr"); diff --git a/lib/agent-argv.ts b/lib/agent-argv/claude.ts similarity index 72% rename from lib/agent-argv.ts rename to lib/agent-argv/claude.ts index 4e9330fa46..7f62d29324 100644 --- a/lib/agent-argv.ts +++ b/lib/agent-argv/claude.ts @@ -1,5 +1,5 @@ /** - * lib/agent-argv.ts ... pure claude/cswap invocation building for `rt agent`. + * lib/agent-argv/claude.ts ... pure claude/cswap invocation building for `rt agent`. * * Session uuids are validated here because the claude CLI fails soft: * `--session-id ""` is silently ignored (random id minted) and @@ -14,6 +14,7 @@ import { homedir } from "os"; import { join } from "path"; +import type { AgentInvocation } from "./types.ts"; const UUID_RE = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; @@ -25,28 +26,7 @@ export function shellSingleQuote(s: string): string { return `'${s.replaceAll("'", `'\\''`)}'`; } -export interface ClaudeInvocation { - account?: string; - model?: string; - effort?: string; - /** Chat handle reserved for this agent (lib/chat-names.ts pool); interactive only, see claudeArgs. */ - name?: string; - extraArgs?: string; - session: { kind: "start"; sessionId: string } | { kind: "resume"; sessionId: string }; - headless: boolean; - prompt?: string; - /** Extra environment for the pane shell, exported before the claude head. Values are single-quoted verbatim. */ - env?: Record; - /** - * Absolute path to a settings JSON file. A launch never emits two - * `--settings` flags (repeated-flag semantics are unverified against the - * real CLI): when set, this REPLACES the `--name`-triggered inline - * CROSS_SESSION_INBOUND_SETTINGS JSON below -- the caller (lib/agent-hooks.ts's - * mergeGateForkHookSettings, via lib/daemon/handlers/agent.ts) is - * responsible for folding that same object into the file's content first. - */ - settingsPath?: string; -} +export type ClaudeInvocation = AgentInvocation; /** The inline `--settings` JSON a reserved chat handle triggers on its own (no settingsPath). Exported so a settingsPath caller can merge it into the SAME file instead of the flag being emitted twice. */ export const CROSS_SESSION_INBOUND_SETTINGS = { crossSessionInbound: "accept" } as const; @@ -59,7 +39,7 @@ export function resolveCswapBin(): string { return Bun.which("cswap") ?? join(process.env.HOME ?? homedir(), ".local", "bin", "cswap"); } -function claudeArgs(inv: ClaudeInvocation): string[] { +function claudeArgs(inv: AgentInvocation): string[] { if (!isValidSessionUuid(inv.session.sessionId)) { throw new Error(`invalid session uuid "${inv.session.sessionId}" ... refusing to spawn`); } @@ -68,6 +48,7 @@ function claudeArgs(inv: ClaudeInvocation): string[] { } const args: string[] = []; if (inv.headless) args.push("-p", "--output-format", "json"); + if (inv.yolo) args.push("--dangerously-skip-permissions"); if (inv.model) args.push("--model", inv.model); if (inv.effort) args.push("--effort", inv.effort); // Headless (-p) never signs into chat, so a reserved handle is not passed @@ -86,7 +67,7 @@ function claudeArgs(inv: ClaudeInvocation): string[] { return args; } -export function buildClaudeArgv(inv: ClaudeInvocation, bins?: { claude?: string; cswap?: string }): string[] { +export function buildClaudeArgv(inv: AgentInvocation, bins?: { claude?: string; cswap?: string }): string[] { const args = claudeArgs(inv); // claude args live only after "--"; the literal word "claude" is never // among them since cswap runs claude itself. @@ -94,7 +75,7 @@ export function buildClaudeArgv(inv: ClaudeInvocation, bins?: { claude?: string; return [bins?.claude ?? resolveClaudeBin(), ...args]; } -export function buildPaneCommand(cwd: string, inv: ClaudeInvocation): string { +export function buildPaneCommand(cwd: string, inv: AgentInvocation): string { // Every token is single-quoted, including flag names (no allowlist), so a // prompt equal to a flag like "-p" is still treated as data. const quoted = claudeArgs(inv).map(shellSingleQuote); diff --git a/lib/agent-argv/codex.ts b/lib/agent-argv/codex.ts new file mode 100644 index 0000000000..8b4dccc360 --- /dev/null +++ b/lib/agent-argv/codex.ts @@ -0,0 +1,63 @@ +/** + * lib/agent-argv/codex.ts -- codex CLI invocation building for `rt agent`. + * + * Confirmed against the installed codex-cli 0.153.4's own --help output, not + * from memory. Usage lines: `codex exec [OPTIONS] [PROMPT]`, + * `codex exec resume [OPTIONS] [SESSION_ID] [PROMPT]`, `codex [OPTIONS] + * [PROMPT]`, `codex resume [OPTIONS] [SESSION_ID] [PROMPT]` -- flags always + * precede positionals, matching the order this file emits them. + * + * codex mints its own session id and never accepts an externally chosen one + * (see lib/daemon/handlers/agent.ts's session-id capture for how the real id + * gets learned after the fact), so unlike claude.ts's builder this one never + * emits inv.session.sessionId on start -- only resume takes it, positionally. + */ + +import { homedir } from "os"; +import { join } from "path"; +import { shellSingleQuote } from "./claude.ts"; +import type { AgentInvocation } from "./types.ts"; + +export function resolveCodexBin(): string { + return Bun.which("codex") ?? join(process.env.HOME ?? homedir(), ".local", "bin", "codex"); +} + +/** Flags shared by every codex form (exec, exec resume, interactive, interactive resume). */ +function codexFlags(inv: AgentInvocation): string[] { + const args: string[] = []; + if (inv.model) args.push("-m", inv.model); + // codex has no dedicated --effort flag; model_reasoning_effort is a config + // override (-c key=value), confirmed via `codex exec --help`'s -c examples. + if (inv.effort) args.push("-c", `model_reasoning_effort=${inv.effort}`); + if (inv.yolo) args.push("--dangerously-bypass-approvals-and-sandbox"); + if (inv.extraArgs) args.push(...inv.extraArgs.split(/\s+/).filter(Boolean)); + return args; +} + +export function buildCodexArgv(inv: AgentInvocation, bins?: { codex?: string }): string[] { + if (inv.headless && !inv.prompt) { + throw new Error("headless launch requires a prompt (codex exec with no prompt blocks on stdin)"); + } + const bin = bins?.codex ?? resolveCodexBin(); + const flags = codexFlags(inv); + // --json is gated on inv.headless, mirroring claude.ts's claudeArgs gating + // "-p --output-format json" the same way -- this function is also exercised + // with headless: false in tests (matching buildClaudeArgv's own "plain + // start, pane surface" test), so --json must not be unconditional. + const jsonFlag = inv.headless ? ["--json"] : []; + const args = inv.session.kind === "start" + ? [bin, "exec", ...jsonFlag, ...flags] + : [bin, "exec", "resume", ...jsonFlag, ...flags, inv.session.sessionId]; + if (inv.prompt) args.push(inv.prompt); + return args; +} + +export function buildCodexPaneCommand(cwd: string, inv: AgentInvocation): string { + const flags = codexFlags(inv).map(shellSingleQuote); + const head = inv.session.kind === "start" + ? ["codex", ...flags] + : ["codex", "resume", ...flags, shellSingleQuote(inv.session.sessionId)]; + const tail = inv.prompt ? [shellSingleQuote(inv.prompt)] : []; + const env = Object.entries(inv.env ?? {}).map(([k, v]) => `${k}=${shellSingleQuote(v)}`); + return `cd ${shellSingleQuote(cwd)} && ${[...env, ...head, ...tail].join(" ")}`; +} diff --git a/lib/agent-argv/index.ts b/lib/agent-argv/index.ts new file mode 100644 index 0000000000..acf9f8393d --- /dev/null +++ b/lib/agent-argv/index.ts @@ -0,0 +1,24 @@ +/** + * lib/agent-argv/index.ts -- barrel + provider dispatch for `rt agent`. + * lib/daemon/handlers/agent.ts calls buildAgentArgv/buildAgentPaneCommand + * here instead of reaching into claude.ts or codex.ts directly. + */ +export * from "./types.ts"; +export * from "./claude.ts"; +export * from "./codex.ts"; + +import type { AgentInvocation, AgentProvider } from "./types.ts"; +import { buildClaudeArgv, buildPaneCommand as buildClaudePaneCommand } from "./claude.ts"; +import { buildCodexArgv, buildCodexPaneCommand } from "./codex.ts"; + +export function buildAgentArgv( + provider: AgentProvider, + inv: AgentInvocation, + bins?: { claude?: string; cswap?: string; codex?: string }, +): string[] { + return provider === "codex" ? buildCodexArgv(inv, bins) : buildClaudeArgv(inv, bins); +} + +export function buildAgentPaneCommand(provider: AgentProvider, cwd: string, inv: AgentInvocation): string { + return provider === "codex" ? buildCodexPaneCommand(cwd, inv) : buildClaudePaneCommand(cwd, inv); +} diff --git a/lib/agent-argv/types.ts b/lib/agent-argv/types.ts new file mode 100644 index 0000000000..f2102cd365 --- /dev/null +++ b/lib/agent-argv/types.ts @@ -0,0 +1,23 @@ +export type AgentProvider = "claude" | "codex"; + +export interface AgentInvocation { + model?: string; + effort?: string; + extraArgs?: string; + session: { kind: "start"; sessionId: string } | { kind: "resume"; sessionId: string }; + headless: boolean; + prompt?: string; + /** Extra environment for the pane shell, exported before the agent head. Values are single-quoted verbatim. */ + env?: Record; + /** Maps to each provider's real bypass flag (see claude.ts / codex.ts). A + resume re-applies whatever the record stored; what a resume never does is + re-derive it from `agent..yolo`, so a settings change after the + launch does not retroactively arm or disarm an existing agent. */ + yolo?: boolean; + /** claude-only: cswap account email. */ + account?: string; + /** claude-only: reserved chat handle; interactive only, see claude.ts's claudeArgs. */ + name?: string; + /** claude-only: absolute path to a --settings JSON file. */ + settingsPath?: string; +} diff --git a/lib/agent-herdr.ts b/lib/agent-herdr.ts index 9cb964bad9..47ad5ec347 100644 --- a/lib/agent-herdr.ts +++ b/lib/agent-herdr.ts @@ -116,6 +116,42 @@ export async function launchInWorkspace( return { workspaceId: wsId, tabId: root.tab_id, paneId: root.pane_id, focusedExisting: false }; } +/** + * Polls herdr's own agent-session report until it surfaces codex's real + * session id, or the timeout elapses. herdr's per-CLI SessionStart hook + * (installed by `herdr integration install codex`) reports the id to herdr + * over its socket once codex processes its first turn; this reads it back + * via `herdr agent get`. Confirmed against a real herdr pane 2026-09-15: the + * id lands at `result.agent.agent_session.value` (present only after the + * pane's first prompt completes -- a freshly launched, not-yet-prompted pane + * has no `agent_session` key at all), not the guessed `result.agentSessionId` + * / `agentSessionId`. + */ +export async function herdrAgentSessionId( + paneId: string, + timeoutMs: number, + runner: HerdrRunner = defaultHerdrRunner(), +): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + const r = await runner(["agent", "get", paneId]); + if (r.exitCode === 0) { + try { + const parsed = JSON.parse(r.stdout); + const sid = parsed?.result?.agent?.agent_session?.value; + if (typeof sid === "string" && sid) return sid; + } catch { + // keep polling -- a transient non-JSON response is not fatal + } + } + // 2s, not 500ms: the 10-minute budget above would otherwise spawn up to + // 1200 `herdr agent get` subprocesses waiting on a turn that typically + // takes well over a few seconds anyway. + await new Promise((resolve) => setTimeout(resolve, 2000)); + } + return undefined; +} + export async function herdrAgentWait( paneId: string, until: string[], diff --git a/lib/agent-hooks.ts b/lib/agent-hooks.ts index 750a8606c2..cf4de6a94d 100644 --- a/lib/agent-hooks.ts +++ b/lib/agent-hooks.ts @@ -86,7 +86,7 @@ export function gateForkHookSettings(hookPath: string): GateForkHookSettings { * A launch never emits two `--settings` flags (repeated-flag semantics are * unverified against the real CLI, and a silent last-wins would drop * whichever settings lost), so a launch that already carries its own inline - * settings object (today, only lib/agent-argv.ts's + * settings object (today, only lib/agent-argv/claude.ts's * CROSS_SESSION_INBOUND_SETTINGS) must fold the hook into that SAME object * instead of writing a second file. Additive only: every key of `base` * survives untouched, and the hook's PreToolUse entry is appended to diff --git a/lib/command-tree-def.ts b/lib/command-tree-def.ts index 1deb6c5145..18e74b4f93 100644 --- a/lib/command-tree-def.ts +++ b/lib/command-tree-def.ts @@ -251,8 +251,8 @@ const herdSubcommands: Record = { { name: "Job", flag: "--job", type: "text", placeholder: "acme-1483-facts", hint: "Job name; also the worktree and handle stem" }, { name: "Brief", flag: "--brief", type: "text", placeholder: "brief.md", hint: "File whose text becomes the job brief" }, { name: "Dir", flag: "--dir", type: "text", placeholder: "~/Documents/GitHub/x", hint: "Existing directory to run in instead of a fresh worktree" }, - { name: "Model", flag: "--model", type: "text", placeholder: "opus", hint: "Override agent.model for this worker" }, - { name: "Effort", flag: "--effort", type: "text", placeholder: "high", hint: "Override agent.effort for this worker" }, + { name: "Model", flag: "--model", type: "text", placeholder: "opus", hint: "Override agent.claude.model for this worker" }, + { name: "Effort", flag: "--effort", type: "text", placeholder: "high", hint: "Override agent.claude.effort for this worker" }, { name: "Account", flag: "--account", type: "text", placeholder: "me@example.com", hint: "cswap account for this worker" }, { name: "Disposable", flag: "--disposable", type: "boolean", default: false, hint: "Wrap-up may dispose this job's worktree" }, { name: "JSON", flag: "--json", type: "boolean", default: false, hint: "Emit the spawn record as JSON" }, @@ -1292,7 +1292,7 @@ export const TREE: Record = { // Self-dispatching leaf: agent() routes its own verbs (start/resume/show/list). agent: { - description: "Hand a prompt to a Claude Code agent (herdr pane or headless) and keep the receipt", + description: "Hand a prompt to a coding agent, claude or codex (herdr pane or headless), and keep the receipt", module: "./commands/agent.ts", fn: "agent", omitBehavior: "picker", @@ -1303,14 +1303,17 @@ export const TREE: Record = { { name: "Prompt", flag: "--prompt", type: "text", placeholder: "...", hint: "Initial prompt (required for headless)" }, { name: "Prompt file", flag: "--prompt-file", type: "text", placeholder: "path/to/prompt.md", hint: "Read the prompt from a file (mutually exclusive with --prompt)" }, { name: "Surface", flag: "--surface", type: "text", placeholder: "herdr | headless", hint: "Where the agent runs (default herdr)" }, - { name: "Model", flag: "--model", type: "text", placeholder: "sonnet", hint: "Override agent.model" }, - { name: "Effort", flag: "--effort", type: "text", placeholder: "high", hint: "Override agent.effort" }, - { name: "Account", flag: "--account", type: "text", placeholder: "me@example.com", hint: "cswap account (override agent.account)" }, + { name: "Provider", flag: "--provider", type: "text", placeholder: "claude | codex", hint: "Which CLI to launch (override agent.provider, default claude)" }, + { name: "Model", flag: "--model", type: "text", placeholder: "sonnet", hint: "Override agent..model" }, + { name: "Effort", flag: "--effort", type: "text", placeholder: "high", hint: "Override agent..effort" }, + { name: "Yolo", flag: "--yolo", type: "boolean", hint: "Bypass permission prompts for this launch (override agent..yolo; omitted falls back to the setting, not false)" }, + { name: "No yolo", flag: "--no-yolo", type: "boolean", hint: "Keep permission prompts for this launch even when agent..yolo is true (omitted falls back to the setting, not false)" }, + { name: "Account", flag: "--account", type: "text", placeholder: "me@example.com", hint: "cswap account, claude only (override agent.claude.account)" }, { name: "Label", flag: "--label", type: "text", placeholder: "job7", hint: "Caller's display label; used as the herdr tab name" }, { name: "Caller", flag: "--caller", type: "text", placeholder: "board:review", hint: "Identifies what invoked this handoff" }, { name: "Workspace", flag: "--workspace", type: "text", placeholder: "reviews", hint: "herdr workspace label (default: the repo label)" }, { name: "Tab", flag: "--tab", type: "text", placeholder: "!7", hint: "herdr tab label (default: the label or handoff id)" }, - { name: "Extra args", flag: "--extra-args", type: "text", placeholder: "\"--foo bar\"", hint: "Opaque extra claude arguments appended to the launch (override agent.extraArgs)" }, + { name: "Extra args", flag: "--extra-args", type: "text", placeholder: "\"--foo bar\"", hint: "Opaque extra provider arguments appended to the launch (override agent..extraArgs)" }, { name: "Background", flag: "--bg", type: "boolean", default: false, hint: "Launch onto the daemon-owned background herdr server instead of the visible one (herdr surface only; requires the rt daemon)" }, { name: "JSON", flag: "--json", type: "boolean", default: false, hint: "Emit the record as JSON" }, ], diff --git a/lib/daemon/__tests__/agent-handlers.test.ts b/lib/daemon/__tests__/agent-handlers.test.ts index b869f338e7..38bcc3697d 100644 --- a/lib/daemon/__tests__/agent-handlers.test.ts +++ b/lib/daemon/__tests__/agent-handlers.test.ts @@ -1,11 +1,13 @@ -import { expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; import { tmpdir } from "os"; import { join } from "path"; -import { existsSync, mkdtempSync, readFileSync, rmSync } from "fs"; +import { existsSync, mkdtempSync, readFileSync, realpathSync, rmSync } from "fs"; import pino from "pino"; import { AGENT_NAMES } from "../../chat-names.ts"; -import { openStateDb, signIn } from "../../state/index.ts"; -import { createAgentHandlers, type HeadlessChild } from "../handlers/agent.ts"; +import { rtDir } from "../../rt-paths.ts"; +import { setSetting } from "../../settings/write.ts"; +import { getAgent, openStateDb, signIn } from "../../state/index.ts"; +import { createAgentHandlers, extractSessionId, type HeadlessChild } from "../handlers/agent.ts"; import { createBgClaimsStore, type BgClaimsStore } from "../bg-claims-store.ts"; import { bgSocketPath } from "../bg-service.ts"; import { DAEMON_SOCK_PATH } from "../../daemon-config.ts"; @@ -26,6 +28,20 @@ function okRunner(calls: string[][]): HerdrRunner { }; } +/** okRunner plus an `agent get` that already carries a session id, so a codex + launch's capture poll resolves on its first call instead of spinning for + its whole budget past the end of the test. */ +function codexRunner(calls: string[][], sessionId = "s_codex_poll"): HerdrRunner { + const base = okRunner(calls); + return async (args) => { + if (args[0] === "agent" && args[1] === "get") { + calls.push(args); + return { stdout: JSON.stringify({ result: { agent: { agent_session: { value: sessionId } } } }), exitCode: 0 }; + } + return base(args); + }; +} + interface FakeBg { ensure: () => Promise<{ socket: string; started: boolean }>; reprobe: () => Promise<{ ok: boolean; drift: string[] }>; @@ -175,7 +191,7 @@ test("agent:start refuses to launch when the insert did not persist", async () = runner: okRunner(calls), spawn: () => { spawnCalled = true; - return { exited: Promise.resolve(0), stdout: async () => "{}" }; + return { exited: Promise.resolve(0), stdout: async () => "{}", sessionId: () => Promise.resolve(undefined) }; }, insertAgentFn: () => {}, }); @@ -200,7 +216,7 @@ test("agent:start headless refuses a missing prompt", async () => { // applied; refuse instead of spawning without it. test("agent:start headless with env is refused and spawns nothing", async () => { let spawnCalled = false; - const h = fresh({ spawn: () => { spawnCalled = true; return { exited: Promise.resolve(0), stdout: async () => "{}" }; } }); + const h = fresh({ spawn: () => { spawnCalled = true; return { exited: Promise.resolve(0), stdout: async () => "{}", sessionId: () => Promise.resolve(undefined) }; } }); const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go", env: { HERD_ID: "demo-1" } }); expect(res.ok).toBe(false); if (res.ok) throw new Error("unreachable"); @@ -233,6 +249,7 @@ test("agent:start headless finishes the record and emits agent/done", async () = const child: HeadlessChild = { exited: new Promise((r) => (resolveExit = r)), stdout: async () => JSON.stringify({ result: "ok" }), + sessionId: () => Promise.resolve(undefined), }; const h = fresh({ spawn: () => child, emit: (t) => emitted.push(t) }); const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go" }); @@ -255,6 +272,7 @@ test("agent:start headless whose exit resolves immediately still finds its own r const child: HeadlessChild = { exited: Promise.resolve(0), stdout: async () => JSON.stringify({ result: "ok" }), + sessionId: () => Promise.resolve(undefined), }; const h = fresh({ spawn: () => child }); const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go" }); @@ -348,7 +366,7 @@ test("agent:start headless never reserves a handle or passes --name/inline --set const h = fresh({ spawn: (a) => { argv = a; - return { exited: Promise.resolve(0), stdout: async () => "{}" }; + return { exited: Promise.resolve(0), stdout: async () => "{}", sessionId: () => Promise.resolve(undefined) }; }, }); const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go" }); @@ -417,7 +435,7 @@ test("agent:start headless argv carries --settings for the gate-fork hook; spawn spawn: (a: string[], _cwd: string, env: Record) => { argv = a; spawnEnv = env ?? {}; - return { exited: Promise.resolve(0), stdout: async () => "{}" }; + return { exited: Promise.resolve(0), stdout: async () => "{}", sessionId: () => Promise.resolve(undefined) }; }, }); const subject = "mr:test/3"; @@ -457,7 +475,7 @@ test("agent:start headless with no explicit subject skips gate-fork hook injecti const h = fresh({ spawn: (a: string[]) => { argv = a; - return { exited: Promise.resolve(0), stdout: async () => "{}" }; + return { exited: Promise.resolve(0), stdout: async () => "{}", sessionId: () => Promise.resolve(undefined) }; }, }); const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go" }); @@ -608,6 +626,141 @@ test("agent:start with handle uses it as --name and reserves no pool handle", as expect(paneRun?.[3]).toContain("'--name' 'job-a'"); }); +// The session-id poll must ride the SAME socket-scoped runner the launch did. +// Falling back to defaultHerdrRunner() would query the ambient visible herdr +// server about a pane that only exists on the bg/herd one -- a full timeout of +// dead polling, then a record stuck on rt's placeholder uuid forever. +test("codex herdr capture polls through the socket-scoped runner the launch used, not the ambient default", async () => { + const agentGetSockets: string[] = []; + const runnerFactory = (socket: string): HerdrRunner => async (args) => { + if (args[0] === "agent" && args[1] === "get") { + agentGetSockets.push(socket); + return { stdout: JSON.stringify({ result: { agent: { agent_session: { value: "s_scoped_789" } } } }), exitCode: 0 }; + } + if (args[0] === "workspace" && args[1] === "list") return { stdout: JSON.stringify({ result: { workspaces: [] } }), exitCode: 0 }; + if (args[0] === "workspace" && args[1] === "create") { + return { stdout: JSON.stringify({ result: { root_pane: { pane_id: "w1:p1", tab_id: "w1:t1", workspace_id: "w1" } } }), exitCode: 0 }; + } + return { stdout: "{}", exitCode: 0 }; + }; + const h = fresh({ runnerFactory }); + const res = await h["agent:start"]({ + repo: REPO, cwd: "/tmp/x", prompt: "hi", surface: "herdr", + provider: "codex", herdrSocket: "/tmp/hidden.sock", + }); + if (!res.ok) throw new Error(res.error); + const deadline = Date.now() + 2000; + while (agentGetSockets.length === 0 && Date.now() < deadline) await new Promise((r) => setTimeout(r, 10)); + expect(agentGetSockets[0]).toBe("/tmp/hidden.sock"); + let updated = getAgent(res.data.id, h.db); + while (updated?.sessionId !== "s_scoped_789" && Date.now() < deadline) { + await new Promise((r) => setTimeout(r, 10)); + updated = getAgent(res.data.id, h.db); + } + expect(updated?.sessionId).toBe("s_scoped_789"); +}); + +// The injected runner is also what a test-only double relies on: an +// unscoped launch must poll through opts.herdrRunner, never spawn herdr. +test("codex herdr capture polls through the injected herdrRunner on an unscoped launch", async () => { + const calls: string[][] = []; + const runner: HerdrRunner = async (args) => { + calls.push(args); + if (args[0] === "agent" && args[1] === "get") { + return { stdout: JSON.stringify({ result: { agent: { agent_session: { value: "s_injected_111" } } } }), exitCode: 0 }; + } + if (args[0] === "workspace" && args[1] === "list") return { stdout: JSON.stringify({ result: { workspaces: [] } }), exitCode: 0 }; + if (args[0] === "workspace" && args[1] === "create") { + return { stdout: JSON.stringify({ result: { root_pane: { pane_id: "w1:p1", tab_id: "w1:t1", workspace_id: "w1" } } }), exitCode: 0 }; + } + return { stdout: "{}", exitCode: 0 }; + }; + const h = fresh({ runner }); + const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", prompt: "hi", surface: "herdr", provider: "codex" }); + if (!res.ok) throw new Error(res.error); + const deadline = Date.now() + 2000; + let updated = getAgent(res.data.id, h.db); + while (updated?.sessionId !== "s_injected_111" && Date.now() < deadline) { + await new Promise((r) => setTimeout(r, 10)); + updated = getAgent(res.data.id, h.db); + } + expect(updated?.sessionId).toBe("s_injected_111"); + expect(calls.some((c) => c[0] === "agent" && c[1] === "get")).toBe(true); +}); + +// codex has no --name / chat-handle mechanism and never reads --settings +// (both spec Non-goals), so a codex launch must burn neither an LRU pool +// handle nor a gate-fork hook file. +test("agent:start codex herdr reserves no handle and writes no gate-fork settings file", async () => { + const calls: string[][] = []; + // codexRunner, not okRunner: a plain okRunner answers `agent get` without an + // agent_session, so the capture poll would keep running for its full + // ten-minute budget after this test finished. + const h = fresh({ runner: codexRunner(calls) }); + const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", prompt: "hi", surface: "herdr", provider: "codex", subject: "mr:test/9" }); + if (!res.ok) throw new Error(res.error); + expect(res.data.handle).toBeUndefined(); + const cmd = calls.find((c) => c[0] === "pane" && c[1] === "run")?.[3] ?? ""; + expect(cmd).not.toContain("agent-hooks"); + expect(cmd).not.toContain("--settings"); + expect(existsSync(join(rtDir(), "agent-hooks", `${res.data.id}.json`))).toBe(false); +}); + +// The claude path must be untouched by the provider gate above. +test("agent:start claude herdr still reserves a handle and writes the gate-fork settings file", async () => { + const calls: string[][] = []; + const h = fresh({ runner: okRunner(calls) }); + const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", prompt: "hi", surface: "herdr", provider: "claude", subject: "mr:test/10" }); + if (!res.ok) throw new Error(res.error); + expect(res.data.handle).toBeTruthy(); + const cmd = calls.find((c) => c[0] === "pane" && c[1] === "run")?.[3] ?? ""; + expect(cmd).toContain("agent-hooks"); +}); + +test("agent:start headless codex names codex in the missing-prompt error, not claude", async () => { + const h = fresh(); + const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", provider: "codex" }); + expect(res.ok).toBe(false); + if (res.ok) throw new Error("unreachable"); + expect(res.error).toContain("codex exec"); + expect(res.error).not.toContain("claude -p"); +}); + +test("agent:start defaults provider to claude when unset", async () => { + const h = fresh({ spawn: () => ({ exited: Promise.resolve(0), stdout: async () => "{}", sessionId: () => Promise.resolve(undefined) }) }); + const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go" }); + expect(res.ok).toBe(true); + if (res.ok) expect(res.data.provider).toBe("claude"); +}); + +test("agent:start honors an explicit provider", async () => { + const h = fresh({ spawn: () => ({ exited: Promise.resolve(0), stdout: async () => "{}", sessionId: () => Promise.resolve(undefined) }) }); + const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go", provider: "codex" }); + expect(res.ok).toBe(true); + if (res.ok) expect(res.data.provider).toBe("codex"); +}); + +test("agent:start rejects an unknown provider", async () => { + const h = fresh(); + const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go", provider: "cursor" }); + expect(res.ok).toBe(false); + if (!res.ok) expect(res.error).toMatch(/provider/); +}); + +test("agent:start rejects account with codex", async () => { + const h = fresh(); + const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go", provider: "codex", account: "a@b.c" }); + expect(res.ok).toBe(false); + if (!res.ok) expect(res.error).toMatch(/account/); +}); + +test("agent:start threads yolo into the recorded agent", async () => { + const h = fresh({ spawn: () => ({ exited: Promise.resolve(0), stdout: async () => "{}", sessionId: () => Promise.resolve(undefined) }) }); + const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go", yolo: true }); + expect(res.ok).toBe(true); + if (res.ok) expect(res.data.yolo).toBe(true); +}); + test("agent:list filters by repo", async () => { const h = fresh({ runner: okRunner([]) }); await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", prompt: "a", surface: "herdr" }); @@ -766,3 +919,158 @@ test("agent:resume of a visible record never touches bg: no ensure, no claim/rel expect(bgClaims.released).toEqual([]); expect(lifecycle.watched).toEqual([]); }); + +// Nothing else in this suite exercises the settings side of the resolution: +// every other test hands the value in on the payload, so a broken +// `agent..*` read would look green everywhere. HOME is repointed +// per-test (test-setup.ts already makes it throwaway process-wide) so the rows +// written here never leak into the tests around them. +describe("agent:start resolves unset payload fields from settings", () => { + const origHome = process.env.HOME; + let home: string; + + beforeEach(() => { + home = realpathSync(mkdtempSync(join(tmpdir(), "rt-agent-settings-"))); + process.env.HOME = home; + }); + + afterEach(() => { + process.env.HOME = origHome; + rmSync(home, { recursive: true, force: true }); + }); + + test("provider, model, effort, extraArgs and yolo all come from agent..* when the payload omits them", async () => { + setSetting("agent.provider", "codex", "user"); + setSetting("agent.codex.model", "gpt-6-astra", "user"); + setSetting("agent.codex.effort", "high", "user"); + setSetting("agent.codex.extraArgs", "--search", "user"); + setSetting("agent.codex.yolo", true, "user"); + + let argv: string[] = []; + const h = fresh({ + spawn: (a: string[]) => { + argv = a; + return { exited: Promise.resolve(0), stdout: async () => "{}", sessionId: () => Promise.resolve(undefined) }; + }, + }); + const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go" }); + if (!res.ok) throw new Error(res.error); + expect(res.data).toMatchObject({ provider: "codex", model: "gpt-6-astra", effort: "high", extraArgs: "--search", yolo: true }); + expect(argv).toContain("-m"); + expect(argv).toContain("gpt-6-astra"); + expect(argv).toContain("-c"); + expect(argv).toContain("model_reasoning_effort=high"); + expect(argv).toContain("--dangerously-bypass-approvals-and-sandbox"); + expect(argv).toContain("--search"); + }); + + // The per-provider keys must not cross over: a codex default has to be + // invisible to a claude launch and vice versa. + test("a claude launch reads agent.claude.*, never the codex rows", async () => { + setSetting("agent.claude.model", "opus", "user"); + setSetting("agent.claude.account", "me@example.com", "user"); + setSetting("agent.codex.model", "gpt-6-astra", "user"); + + const h = fresh({ spawn: () => ({ exited: Promise.resolve(0), stdout: async () => "{}", sessionId: () => Promise.resolve(undefined) }) }); + const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go" }); + if (!res.ok) throw new Error(res.error); + expect(res.data).toMatchObject({ provider: "claude", model: "opus", account: "me@example.com" }); + }); + + // The setting is a default, not a floor: --no-yolo sends yolo: false on the + // payload, and the daemon must persist that rather than fall through to it. + test("an explicit payload yolo:false overrides a true agent..yolo setting", async () => { + setSetting("agent.claude.yolo", true, "user"); + + let argv: string[] = []; + const h = fresh({ + spawn: (a: string[]) => { + argv = a; + return { exited: Promise.resolve(0), stdout: async () => "{}", sessionId: () => Promise.resolve(undefined) }; + }, + }); + const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go", yolo: false }); + if (!res.ok) throw new Error(res.error); + expect(res.data.yolo).toBe(false); + expect(getAgent(res.data.id, h.db)?.yolo).toBe(false); + expect(argv).not.toContain("--dangerously-skip-permissions"); + }); + + // An explicit payload provider must beat the global default outright -- + // that is what herd:spawn relies on to stay claude-only. + test("an explicit payload provider overrides agent.provider", async () => { + setSetting("agent.provider", "codex", "user"); + const h = fresh({ spawn: () => ({ exited: Promise.resolve(0), stdout: async () => "{}", sessionId: () => Promise.resolve(undefined) }) }); + const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go", provider: "claude" }); + if (!res.ok) throw new Error(res.error); + expect(res.data.provider).toBe("claude"); + }); +}); + +function streamOf(...lines: string[]): ReadableStream { + return new ReadableStream({ + start(controller) { + for (const line of lines) controller.enqueue(new TextEncoder().encode(line + "\n")); + controller.close(); + }, + }); +} + +describe("extractSessionId", () => { + // Real codex exec --json line, observed 2026-09-15: {"type":"thread.started","thread_id":"01a0a82a-ebd6-7732-aaa8-ea6c6b450ed7"} + test("finds thread_id on the thread.started line, ignoring earlier non-matching JSON", async () => { + const sid = await extractSessionId(streamOf('{"type":"turn.started"}', '{"type":"thread.started","thread_id":"01a0a82a-ebd6-7732-aaa8-ea6c6b450ed7"}')); + expect(sid).toBe("01a0a82a-ebd6-7732-aaa8-ea6c6b450ed7"); + }); + + test("ignores non-JSON lines and keeps scanning", async () => { + const sid = await extractSessionId(streamOf("not json at all", '{"type":"thread.started","thread_id":"s_real_456"}')); + expect(sid).toBe("s_real_456"); + }); + + test("resolves undefined when the stream ends without one", async () => { + const sid = await extractSessionId(streamOf('{"type":"turn.completed"}')); + expect(sid).toBeUndefined(); + }); +}); + +test("headless codex launch captures the real session id from the --json stream", async () => { + const fakeStdout = streamOf('{"type":"turn.started"}', '{"type":"thread.started","thread_id":"s_real_123"}'); + const spawnHeadless = (_argv: string[], _cwd: string, _env: Record, opts?: { captureSessionId?: boolean }) => { + const [forText, forId] = fakeStdout.tee(); + return { + exited: Promise.resolve(0), + stdout: () => new Response(forText).text(), + sessionId: opts?.captureSessionId ? () => extractSessionId(forId) : () => Promise.resolve(undefined), + }; + }; + const h = fresh({ spawn: spawnHeadless }); + const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go", provider: "codex" }); + expect(res.ok).toBe(true); + if (!res.ok) return; + // Session-id capture runs on a detached promise chain (`void + // child.sessionId().then(...)`), so poll briefly rather than assuming one + // tick is enough -- a single setImmediate is a plausible source of + // flakiness here. + const deadline = Date.now() + 2000; + let updated = getAgent(res.data.id, h.db); + while (updated?.sessionId !== "s_real_123" && Date.now() < deadline) { + await new Promise((r) => setTimeout(r, 10)); + updated = getAgent(res.data.id, h.db); + } + expect(updated?.sessionId).toBe("s_real_123"); +}); + +// Claude never mints a session id it did not choose, so its headless launch +// must never be asked to capture one from the stream. +test("headless claude launch never requests session-id capture", async () => { + let capturedOpts: { captureSessionId?: boolean } | undefined; + const spawnHeadless = (_argv: string[], _cwd: string, _env: Record, opts?: { captureSessionId?: boolean }) => { + capturedOpts = opts; + return { exited: Promise.resolve(0), stdout: async () => "{}", sessionId: () => Promise.resolve(undefined) }; + }; + const h = fresh({ spawn: spawnHeadless }); + const res = await h["agent:start"]({ repo: REPO, cwd: "/tmp/x", surface: "headless", prompt: "go" }); + expect(res.ok).toBe(true); + expect(capturedOpts?.captureSessionId).toBeFalsy(); +}); diff --git a/lib/daemon/__tests__/herd-handlers.test.ts b/lib/daemon/__tests__/herd-handlers.test.ts index 45b3c3e28a..99d464b4b5 100644 --- a/lib/daemon/__tests__/herd-handlers.test.ts +++ b/lib/daemon/__tests__/herd-handlers.test.ts @@ -1007,6 +1007,20 @@ describe("herd:spawn", () => { expect(hx.store.getJob(s.data.herd, "job-a")!.pane).toBe("w9:p1"); }); + // Herd workers depend on claude-only machinery (the reserved chat handle + // chat:sign-in binds presence to, and the gate-fork --settings hook), so the + // payload must pin the provider rather than inherit the agent.provider + // default -- a global `agent.provider = codex` would otherwise degrade every + // herd silently. + test("herd:spawn pins provider claude on the agent:start payload", async () => { + const hx = harness(); + const s = await hx.h["herd:start"](START); + if (!s.ok) throw new Error(s.error); + const res = await hx.h["herd:spawn"]({ herd: s.data.herd, job: "job-a", brief: "b", dir: "/t" }); + expect(res.ok).toBe(true); + expect(hx.agentCalls[0].provider).toBe("claude"); + }); + test("a hidden herd passes its socket to agent:start", async () => { const hx = harness(); const s = await hx.h["herd:start"]({ ...START, hidden: true }); diff --git a/lib/daemon/handlers/agent.ts b/lib/daemon/handlers/agent.ts index b63b0c7ecb..9441367478 100644 --- a/lib/daemon/handlers/agent.ts +++ b/lib/daemon/handlers/agent.ts @@ -2,9 +2,11 @@ * agent:* ... daemon handlers for `rt agent` (launch + record + resume; no * liveness by design - spec 2026-08-25). * - * Session uuids are minted here and validated in lib/agent-argv.ts before + * Session uuids are minted here and validated in lib/agent-argv/ before * any spawn; resume always runs under the RECORDED account because claude - * transcripts are per-cswap-profile. + * transcripts are per-cswap-profile. codex mints its own id instead, so a + * codex record's sessionId starts as rt's placeholder uuid and is replaced + * by updateAgentSessionId once the real one is captured. * * Every launch (start and resume, herdr and headless) stamps the * gate-protocol env (RT_AGENT_ID, RT_GATE_SUBJECT, RT_DAEMON_SOCK) @@ -23,11 +25,11 @@ import type { Database } from "bun:sqlite"; import type { Logger } from "pino"; import { deleteAgent, finishAgent, getAgent, insertAgent, isValidChatName, listAgents, markAgentResumed, - newAgentId, reserveAgentHandle, updateAgentPane, type AgentRecord, type AgentSurface, + newAgentId, reserveAgentHandle, updateAgentPane, updateAgentSessionId, type AgentRecord, type AgentSurface, } from "../../state/index.ts"; -import { buildClaudeArgv, buildPaneCommand, CROSS_SESSION_INBOUND_SETTINGS, type ClaudeInvocation } from "../../agent-argv.ts"; +import { buildAgentArgv, buildAgentPaneCommand, CROSS_SESSION_INBOUND_SETTINGS, type AgentInvocation, type AgentProvider } from "../../agent-argv/index.ts"; import { mergeGateForkHookSettings, resolveGateForkHookPath } from "../../agent-hooks.ts"; -import { defaultHerdrRunner, launchInWorkspace, type HerdrRunner } from "../../agent-herdr.ts"; +import { defaultHerdrRunner, herdrAgentSessionId, launchInWorkspace, type HerdrRunner } from "../../agent-herdr.ts"; import { repoLabel } from "../../repo-label.ts"; import { getSetting } from "../../settings/resolve.ts"; import { rtDir } from "../../rt-paths.ts"; @@ -43,9 +45,72 @@ import type { CommandResult } from "./types.ts"; export interface HeadlessChild { exited: Promise; stdout: () => Promise; + /** Resolves with the provider-minted session id once seen in the stream, + or undefined if the stream ended without one. Only populated when + captureSessionId was requested at spawn time (claude never needs this + -- it mints nothing, rt already chose the id). */ + sessionId: () => Promise; } -function defaultSpawnHeadless(argv: string[], cwd: string, env: Record = {}): HeadlessChild { +/** Scans a codex `--json` event stream for the first `thread_id` any event + line carries, without buffering the whole stream (that's `stdout()`'s job, + on the other half of the tee below). In practice that is the + `thread.started` event -- confirmed against a real `codex exec --json` run + 2026-09-15, the line is `{"type":"thread.started","thread_id":""}`; + codex calls this a "thread" in the stream but it is the same id `codex + exec resume ` accepts. The match is deliberately on the field + rather than on `type === "thread.started"`: every thread_id in one exec's + stream is that same session's, so keying on the field survives codex + renaming or reordering the event that first carries it. Exported so it can + be unit-tested directly against a fake stream, rather than only + indirectly through a test double that reimplements the parsing. */ +export function extractSessionId(stream: ReadableStream): Promise { + return (async () => { + // bun-types' ReadableStream and TextDecoderStream's + // WritableStream disagree just enough (BufferSource vs + // Uint8Array) that pipeThrough's own generic can't unify them; the cast + // is a type-level workaround for that mismatch, not a runtime one -- + // TextDecoderStream really does accept a Uint8Array chunk. + const reader = (stream.pipeThrough(new TextDecoderStream() as unknown as ReadableWritablePair)).getReader(); + let buffer = ""; + try { + while (true) { + const { done, value } = await reader.read(); + if (done) break; + buffer += value; + let newlineAt: number; + while ((newlineAt = buffer.indexOf("\n")) !== -1) { + const line = buffer.slice(0, newlineAt); + buffer = buffer.slice(newlineAt + 1); + if (!line.trim()) continue; + try { + const event = JSON.parse(line) as { thread_id?: unknown }; + if (typeof event.thread_id === "string" && event.thread_id) return event.thread_id; + } catch { + // Not every line is JSON we care about; keep scanning. + } + } + } + } finally { + // Cancelled, not merely released: this reader drains one branch of a + // tee, and the tee queues every chunk an undrained branch has not taken + // yet -- so returning early on the id without cancelling would hold a + // long codex run's whole output twice. + try { + await reader.cancel(); + } catch { + // Already closed or errored; nothing left to cancel. + } + reader.releaseLock(); + } + return undefined; + })(); +} + +function defaultSpawnHeadless( + argv: string[], cwd: string, env: Record = {}, + opts: { captureSessionId?: boolean } = {}, +): HeadlessChild { const proc = Bun.spawn(argv as [string, ...string[]], { cwd, env: { ...process.env, ...env }, @@ -53,23 +118,37 @@ function defaultSpawnHeadless(argv: string[], cwd: string, env: Record new Response(proc.stdout).text(), + sessionId: () => Promise.resolve(undefined), + }; + } + const [forText, forId] = proc.stdout.tee(); + const sessionIdPromise = extractSessionId(forId); return { exited: proc.exited, - stdout: () => new Response(proc.stdout).text(), + stdout: () => new Response(forText).text(), + sessionId: () => sessionIdPromise, }; } /** A declared+unset key resolves undefined without throwing, so a caught error here is already the unexpected case. */ -function fromSetting(key: string, log: Logger): string | undefined { +function fromSetting(key: string, log: Logger): T | undefined { try { - return getSetting(key).value ?? undefined; + return getSetting(key).value ?? undefined; } catch (err) { log.warn({ err, key }, "agent: settings read failed"); return undefined; } } -/** Deterministic from the id alone, so it can be known and stored before the headless process is even spawned. */ +/** Deterministic from the id alone, so it can be known and stored before the + headless process is even spawned. The body written there is + provider-dependent: claude's `--output-format json` produces one JSON + object, codex's `--json` produces a JSONL event stream. Nothing in-repo + parses it today; whoever adds a consumer must branch on rec.provider. */ function agentResultPath(id: string): string { return join(rtDir(), "agents", `${id}.json`); } @@ -88,8 +167,10 @@ function extraArgsHasSettingsFlag(extraArgs: string | undefined): boolean { /** * Absolute path to a freshly written per-agent settings file carrying the * AskUserQuestion PreToolUse hook (Task 9), or undefined when injection is - * skipped. Three skip cases, all non-fatal to the launch: the launch - * carries no explicit subject (with no + * skipped. Four skip cases, all non-fatal to the launch: the provider is not + * claude (only claude reads `--settings`; codex's builders ignore + * inv.settingsPath outright, so writing the file would leave a dead one in the + * agent-hooks directory per launch), the launch carries no explicit subject (with no * subject there is no gate for the hook to check, so it would only ever * degrade to allow; skip writing it rather than ship a no-op hook file), * extraArgs already sets --settings (merge is not attempted -- the user's @@ -101,13 +182,17 @@ function extraArgsHasSettingsFlag(extraArgs: string | undefined): boolean { * the --name-triggered inline CROSS_SESSION_INBOUND_SETTINGS JSON (a * reserved handle, non-headless -- claudeArgs' own condition), that object * is folded into this SAME file via mergeGateForkHookSettings instead of - * being emitted as a second flag. lib/agent-argv.ts's claudeArgs skips its + * being emitted as a second flag. lib/agent-argv/claude.ts's claudeArgs skips its * inline JSON whenever settingsPath is set, so the fold here is the only * place that JSON survives for such a launch. A subjectless launch skips * this file entirely, so that inline JSON reverts to riding its own * --settings flag exactly as it did before the gate-fork hook existed. */ function resolveHookSettingsPath(rec: AgentRecord, log: Logger): string | undefined { + if (rec.provider !== "claude") { + log.debug({ id: rec.id, provider: rec.provider }, "agent: provider does not read --settings; gate-fork hook injection skipped"); + return undefined; + } if (rec.subject === undefined) { log.debug({ id: rec.id }, "agent: no explicit subject; gate-fork hook injection skipped"); return undefined; @@ -145,6 +230,22 @@ const ENV_KEY_RE = /^[A-Za-z_][A-Za-z0-9_]*$/; const agentOwner = (id: string): string => `agent:${id}`; +/** Names the real flag the caller will be looking for; codex.ts's own + equivalent throw already spells the codex form, and a claude-worded + message on a codex launch sends the reader to the wrong CLI's docs. */ +function headlessStdinBlurb(provider: AgentProvider): string { + return provider === "codex" + ? "codex exec with no prompt blocks on stdin" + : "claude -p with no prompt blocks on stdin"; +} + +/** herdr only learns codex's session id after the pane finishes its first + turn (see herdrAgentSessionId), and a real codex turn routinely runs for + many minutes, so a short budget would give up before the id ever exists. + Matches AGENT_WAIT_TIMEOUT_MS in lib/rebase-escalation.ts, this repo's + existing precedent for waiting on a herdr pane to finish work. */ +const CODEX_HERDR_SESSION_ID_TIMEOUT_MS = 10 * 60_000; + /** herdr invoked against a freshly-spawned bg server can fail before any pane ever runs: the bg env's PATH may not carry herdr's own binary yet (`bin not found at ...`) or herdr's own process exits 127 resolving it. @@ -161,13 +262,21 @@ export function createAgentHandlers(opts: { log?: Logger; herdrRunner?: HerdrRunner; herdrRunnerForSocket?: (socket: string) => HerdrRunner; - spawnHeadless?: (argv: string[], cwd: string, env: Record) => HeadlessChild; + spawnHeadless?: (argv: string[], cwd: string, env: Record, opts?: { captureSessionId?: boolean }) => HeadlessChild; insertAgentFn?: typeof insertAgent; /** The daemon-owned background herdr server `--bg` launches onto (spec "The bg service"). Omitted, `bg: true` is refused. */ bg?: Pick; bgClaims?: Pick; /** herd-lifecycle.ts's watch is idempotent by socket, so an already-watched bg socket is a no-op here. */ lifecycle?: { watch(socket: string): void }; + /** Suppresses codex's post-launch session-id capture on the herdr path. Set + only by the in-process CLI fallback (commands/agent-fallback.ts), which + returns and exits: a detached poll there would either hold the process + open for the whole timeout or be killed mid-flight. The daemon is + long-lived, so it leaves this false and always captures. Headless needs + no equivalent gate: that fallback refuses the headless surface outright, + before any handler is constructed. */ + skipSessionCapture?: boolean; }): // Direct `unknown`-payload members, not `Pick`: a wider // `unknown` param still satisfies TypedHandlers' narrower one at the @@ -180,10 +289,11 @@ export function createAgentHandlers(opts: { const log = opts.log ?? lazyChildLogger("agent"); const spawnHeadless = opts.spawnHeadless ?? defaultSpawnHeadless; const insertAgentFn = opts.insertAgentFn ?? insertAgent; + const skipSessionCapture = opts.skipSessionCapture ?? false; async function launch( rec: AgentRecord, - session: ClaudeInvocation["session"], + session: AgentInvocation["session"], prompt: string | undefined, tabLabel: string, workspaceLabel: string, @@ -196,7 +306,7 @@ export function createAgentHandlers(opts: { }; const settingsPath = resolveHookSettingsPath(rec, log); - const inv: ClaudeInvocation = { + const inv: AgentInvocation = { session, headless: rec.surface === "headless", ...(rec.account !== undefined && { account: rec.account }), @@ -204,6 +314,7 @@ export function createAgentHandlers(opts: { ...(rec.effort !== undefined && { effort: rec.effort }), ...(rec.handle !== undefined && { name: rec.handle }), ...(rec.extraArgs !== undefined && { extraArgs: rec.extraArgs }), + ...(rec.yolo !== undefined && { yolo: rec.yolo }), ...(prompt !== undefined && { prompt }), // Headless has no pane shell line for buildPaneCommand to interpolate // env into (see the payload.env rejection above); its gate env instead @@ -217,7 +328,7 @@ export function createAgentHandlers(opts: { ? (opts.herdrRunnerForSocket ?? ((socket: string) => defaultHerdrRunner({ ...process.env, HERDR_SOCKET_PATH: socket })))(extra.herdrSocket) : (opts.herdrRunner ?? defaultHerdrRunner()); const out = await launchInWorkspace( - { workspaceLabel, tabLabel, paneCommand: buildPaneCommand(rec.cwd, inv) }, + { workspaceLabel, tabLabel, paneCommand: buildAgentPaneCommand(rec.provider as AgentProvider, rec.cwd, inv) }, runner, ); if (out.focusedExisting) { @@ -231,17 +342,57 @@ export function createAgentHandlers(opts: { rec.paneId = out.paneId; rec.tabId = out.tabId; rec.workspaceId = out.workspaceId; + if (rec.provider === "codex" && !skipSessionCapture) { + // `runner`, not the default: a --bg or herd launch put this pane on a + // socket-scoped herdr server, and defaultHerdrRunner() would poll the + // ambient visible one, which has never heard of the pane -- the whole + // budget spent on a server that can only answer "no such pane", and + // the record left stuck on rt's placeholder uuid forever. + // + // rec.sessionId returned to the caller by the handler below is + // therefore PROVISIONAL for codex: it is rt's placeholder, and this + // chain replaces the stored value once the real id lands. A response + // already in flight keeps the old value, so `rt agent resume ` stops matching (getAgent keys on id OR session_id) -- the + // record id stays stable and is the safe handle. + void herdrAgentSessionId(out.paneId, CODEX_HERDR_SESSION_ID_TIMEOUT_MS, runner).then((sid) => { + if (sid) updateAgentSessionId(rec.id, sid, db); + // Two real causes, not one: `herdr integration install codex` is + // missing/misconfigured, OR (equally likely -- herdr's + // agent_session only populates once codex completes a turn, per + // Step 0) this launch carried no prompt and the pane never ran + // one. Naming only the integration cause here would send someone + // chasing a nonexistent setup problem on a plain promptless launch. + else log.warn({ id: rec.id }, "agent: codex herdr launch never reported a session id (either `herdr integration install codex` isn't set up, or the pane never completed a turn -- e.g. this launch had no prompt)"); + }).catch((err) => { + // The poll has no try/catch around the runner call, so a herdr + // binary that cannot be resolved rejects the whole chain; detached, + // that would surface as an unhandled rejection. + log.warn({ err, id: rec.id }, "agent: codex herdr session-id capture failed"); + }); + } return { ok: true, data: rec }; } - const argv = buildClaudeArgv(inv); + const argv = buildAgentArgv(rec.provider as AgentProvider, inv); const resultPath = agentResultPath(rec.id); rec.resultPath = resultPath; mkdirSync(dirname(resultPath), { recursive: true }); // The caller inserts rec before invoking launch() for every headless // path (start and resume alike), so the row already exists here -- // finishAgent below can never race an insert that hasn't happened yet. - const child = spawnHeadless(argv, rec.cwd, gateEnv); + const child = spawnHeadless(argv, rec.cwd, gateEnv, { captureSessionId: rec.provider === "codex" }); + if (rec.provider === "codex") { + // Same provisional-sessionId caveat as the herdr branch above: the + // record this handler returns still carries rt's placeholder uuid, and + // this chain replaces the stored value when the real id arrives. + void child.sessionId().then((sid) => { + if (sid) updateAgentSessionId(rec.id, sid, db); + else log.warn({ id: rec.id }, "agent: codex headless launch never reported a session id"); + }).catch((err) => { + log.warn({ err, id: rec.id }, "agent: codex headless session-id capture failed"); + }); + } void child.exited.then(async (exitCode) => { try { writeFileSync(resultPath, await child.stdout()); @@ -264,6 +415,14 @@ export function createAgentHandlers(opts: { return { ok: false, error: `invalid surface "${payload.surface}"; must be one of herdr, headless` }; } const surface: AgentSurface = payload.surface ?? "herdr"; + const providerRaw = payload.provider ?? fromSetting("agent.provider", log) ?? "claude"; + if (providerRaw !== "claude" && providerRaw !== "codex") { + return { ok: false, error: `invalid provider "${providerRaw}"; must be one of claude, codex` }; + } + const provider: AgentProvider = providerRaw; + if (payload.account !== undefined && provider === "codex") { + return { ok: false, error: "codex does not support --account in this version (see spec's Non-goals)" }; + } if (payload.bg && surface === "headless") { return { ok: false, error: "--bg is a herdr-surface option" }; } @@ -272,7 +431,7 @@ export function createAgentHandlers(opts: { } const prompt = payload.prompt; if (surface === "headless" && !prompt) { - return { ok: false, error: "headless launch requires a prompt (claude -p with no prompt blocks on stdin)" }; + return { ok: false, error: `headless launch requires a prompt (${headlessStdinBlurb(provider)})` }; } if (payload.env !== undefined && !isStringRecord(payload.env)) { return { ok: false, error: "env must be an object of strings" }; @@ -294,7 +453,7 @@ export function createAgentHandlers(opts: { } const rec: AgentRecord = { id: newAgentId(), - repo, cwd, provider: "claude", surface, + repo, cwd, provider, surface, sessionId: crypto.randomUUID(), createdAt: Date.now(), }; @@ -306,23 +465,33 @@ export function createAgentHandlers(opts: { // this field is whether resolveHookSettingsPath sees an explicit // subject to gate hook injection on. if (payload.subject !== undefined) rec.subject = payload.subject; - const model = payload.model ?? fromSetting("agent.model", log); - const effort = payload.effort ?? fromSetting("agent.effort", log); - const account = payload.account ?? fromSetting("agent.account", log); - const extraArgs = payload.extraArgs ?? fromSetting("agent.extraArgs", log); + const model = payload.model ?? fromSetting(`agent.${provider}.model`, log); + const effort = payload.effort ?? fromSetting(`agent.${provider}.effort`, log); + const extraArgs = payload.extraArgs ?? fromSetting(`agent.${provider}.extraArgs`, log); + const yolo = payload.yolo ?? fromSetting(`agent.${provider}.yolo`, log) ?? false; if (model !== undefined) rec.model = model; if (effort !== undefined) rec.effort = effort; - if (account !== undefined) rec.account = account; if (extraArgs !== undefined) rec.extraArgs = extraArgs; + // Stored unconditionally, including false: an explicit `--no-yolo` + // against a true `agent..yolo` setting has to survive into the + // record, or resume would silently re-derive nothing and leave it unset. + rec.yolo = yolo; + if (provider === "claude") { + const account = payload.account ?? fromSetting("agent.claude.account", log); + if (account !== undefined) rec.account = account; + } if (payload.label !== undefined) rec.label = payload.label; if (payload.caller !== undefined) rec.caller = payload.caller; if (surface === "headless") { rec.resultPath = agentResultPath(rec.id); } else if (payload.handle) { rec.handle = payload.handle; - } else { + } else if (provider === "claude") { // Headless never signs into chat (see claudeArgs), so reserving a - // handle for it would only burn an LRU pool slot no one adopts. + // handle for it would only burn an LRU pool slot no one adopts. Nor + // does codex at any surface: its builders have no --name / chat-handle + // mechanism (a spec Non-goal), so a reserved handle would be a pool + // slot spent on a record signed into nothing. rec.handle = reserveAgentHandle(db); } @@ -410,7 +579,7 @@ export function createAgentHandlers(opts: { } const surface: AgentSurface = payload.surface ?? rec.surface; if (surface === "headless" && !payload.prompt) { - return { ok: false, error: "headless resume requires a prompt (claude -p with no prompt blocks on stdin)" }; + return { ok: false, error: `headless resume requires a prompt (${headlessStdinBlurb(rec.provider as AgentProvider)})` }; } // ↺ prefix: resume tabs must never dedup against the still-open launch // tab; repeated resumes share the label and dedup against each other. diff --git a/lib/daemon/handlers/herd.ts b/lib/daemon/handlers/herd.ts index d2db7cad68..a3ecff1a74 100644 --- a/lib/daemon/handlers/herd.ts +++ b/lib/daemon/handlers/herd.ts @@ -510,6 +510,12 @@ export function createHerdHandlers(deps: HerdDeps) { // while agent:start decides whether there is a new one. store.upsertJob({ herd: herdId, name, worktree, branch, tree, handle: name, status: "spawning", disposable, pane: null, agentSession: null, agentId: null }); const started = await deps.agent["agent:start"]({ + // Pinned, never inherited from the agent.provider default: a worker + // depends on claude-only machinery (the reserved chat handle + // chat:sign-in binds presence to, and the gate-fork --settings hook), + // so a global codex default would silently degrade every herd. codex + // workers are a separate change. + provider: "claude", repo: herd.repo, cwd: worktree, prompt: brief, surface: "herdr", ...(str(p?.model) && { model: p!.model }), ...(str(p?.effort) && { effort: p!.effort }), ...(str(p?.account) && { account: p!.account }), label: name, caller: `herd:${herdId}`, workspace: herd.workspace, tab: name, handle: name, diff --git a/lib/rebase-escalation.ts b/lib/rebase-escalation.ts index acc42b21a4..f41e61eebc 100644 --- a/lib/rebase-escalation.ts +++ b/lib/rebase-escalation.ts @@ -14,7 +14,7 @@ import { mkdirSync, writeFileSync } from "fs"; import { join } from "path"; import type { RebaseResult } from "../commands/git/rebase.ts"; -import { buildPaneCommand } from "./agent-argv.ts"; +import { buildPaneCommand } from "./agent-argv/index.ts"; import { defaultHerdrRunner, herdrAgentWait, launchInWorkspace, type HerdrRunner } from "./agent-herdr.ts"; import { getCurrentBranch, hasUncommittedChanges } from "./git-ops.ts"; import { syncLog } from "./sync-log.ts"; diff --git a/lib/state/__tests__/agents-store.test.ts b/lib/state/__tests__/agents-store.test.ts index 35a35fff80..f597e7bb42 100644 --- a/lib/state/__tests__/agents-store.test.ts +++ b/lib/state/__tests__/agents-store.test.ts @@ -4,7 +4,7 @@ import { join } from "path"; import { openStateDb } from "../db.ts"; import { finishAgent, getAgent, insertAgent, listAgents, markAgentResumed, - newAgentId, updateAgentPane, type AgentRecord, + newAgentId, updateAgentPane, updateAgentSessionId, type AgentRecord, } from "../agents-store.ts"; let n = 0; @@ -59,3 +59,28 @@ test("duplicate session uuid is refused", () => { insertAgent(r, db); expect(() => insertAgent(rec({ sessionId: r.sessionId }), db)).toThrow(); }); + +test("updateAgentSessionId overwrites the stored session id", () => { + const db = freshDb(); + const r = rec({ sessionId: "placeholder-uuid" }); + insertAgent(r, db); + updateAgentSessionId(r.id, "s_2026-09-15-real", db); + expect(getAgent(r.id, db)?.sessionId).toBe("s_2026-09-15-real"); +}); + +test("agents.yolo round-trips true and false", () => { + const db = freshDb(); + const a = rec({ yolo: true }); + const b = rec({ yolo: false }); + insertAgent(a, db); + insertAgent(b, db); + expect(getAgent(a.id, db)?.yolo).toBe(true); + expect(getAgent(b.id, db)?.yolo).toBe(false); +}); + +test("agents.yolo left unset reads back undefined", () => { + const db = freshDb(); + const r = rec(); + insertAgent(r, db); + expect(getAgent(r.id, db)?.yolo).toBeUndefined(); +}); diff --git a/lib/state/agents-store.ts b/lib/state/agents-store.ts index f48e4dcd45..eaebff02ec 100644 --- a/lib/state/agents-store.ts +++ b/lib/state/agents-store.ts @@ -24,13 +24,13 @@ export interface AgentRecord { for the hook to check). */ subject?: string; paneId?: string; tabId?: string; workspaceId?: string; - extraArgs?: string; exitCode?: number; resultPath?: string; + extraArgs?: string; exitCode?: number; resultPath?: string; yolo?: boolean; createdAt: number; lastResumedAt?: number; finishedAt?: number; } const COLUMNS = "id, repo, cwd, provider, surface, session_id, model, effort, account, label, caller, handle, subject, " + - "pane_id, tab_id, workspace_id, extra_args, exit_code, result_path, " + + "pane_id, tab_id, workspace_id, extra_args, exit_code, result_path, yolo, " + "created_at, last_resumed_at, finished_at"; const INSERT_SQL = `INSERT INTO agents (${COLUMNS}) VALUES (${COLUMNS.split(",").map(() => "?").join(", ")});`; @@ -39,6 +39,7 @@ const SELECT_ALL_SQL = `SELECT ${COLUMNS} FROM agents ORDER BY created_at DESC;` const SELECT_REPO_SQL = `SELECT ${COLUMNS} FROM agents WHERE repo = ? ORDER BY created_at DESC;`; const UPDATE_PANE_SQL = `UPDATE agents SET pane_id = ?, tab_id = ?, workspace_id = ? WHERE id = ?;`; const UPDATE_RESUMED_SQL = `UPDATE agents SET last_resumed_at = ? WHERE id = ?;`; +const UPDATE_SESSION_SQL = `UPDATE agents SET session_id = ? WHERE id = ?;`; const UPDATE_FINISH_SQL = `UPDATE agents SET exit_code = ?, result_path = ?, finished_at = ? WHERE id = ?;`; const DELETE_SQL = `DELETE FROM agents WHERE id = ?;`; @@ -56,7 +57,7 @@ interface AgentRow { account: string | null; label: string | null; caller: string | null; handle: string | null; subject: string | null; pane_id: string | null; tab_id: string | null; workspace_id: string | null; - extra_args: string | null; exit_code: number | null; result_path: string | null; + extra_args: string | null; exit_code: number | null; result_path: string | null; yolo: number | null; created_at: number; last_resumed_at: number | null; finished_at: number | null; } @@ -79,6 +80,7 @@ function rowToRecord(r: AgentRow): AgentRecord { if (r.extra_args !== null) rec.extraArgs = r.extra_args; if (r.exit_code !== null) rec.exitCode = r.exit_code; if (r.result_path !== null) rec.resultPath = r.result_path; + if (r.yolo !== null) rec.yolo = r.yolo === 1; if (r.last_resumed_at !== null) rec.lastResumedAt = r.last_resumed_at; if (r.finished_at !== null) rec.finishedAt = r.finished_at; return rec; @@ -96,6 +98,7 @@ export function insertAgent(rec: AgentRecord, db: Database = getStateDb()): void rec.label ?? null, rec.caller ?? null, rec.handle ?? null, rec.subject ?? null, rec.paneId ?? null, rec.tabId ?? null, rec.workspaceId ?? null, rec.extraArgs ?? null, rec.exitCode ?? null, rec.resultPath ?? null, + rec.yolo === undefined ? null : (rec.yolo ? 1 : 0), rec.createdAt, rec.lastResumedAt ?? null, rec.finishedAt ?? null, ); runCriticalWrite("insertAgent", run, { id: rec.id }); @@ -124,6 +127,14 @@ export function markAgentResumed(id: string, at: number, db: Database = getState runCriticalWrite("markAgentResumed", () => db.query(UPDATE_RESUMED_SQL).run(at, id), { id }); } +/** Overwrites the placeholder session id rt mints before spawning a codex + launch (codex never accepts one on start -- it mints its own) once the + real one is captured (see lib/daemon/handlers/agent.ts's session-id + capture). No-op for claude, which never needs this. */ +export function updateAgentSessionId(id: string, sessionId: string, db: Database = getStateDb()): void { + runCriticalWrite("updateAgentSessionId", () => db.query(UPDATE_SESSION_SQL).run(sessionId, id), { id }); +} + export function finishAgent( id: string, args: { exitCode: number; resultPath: string; finishedAt: number }, db: Database = getStateDb(), diff --git a/lib/state/db.ts b/lib/state/db.ts index 261c27b7c0..9073736d05 100644 --- a/lib/state/db.ts +++ b/lib/state/db.ts @@ -381,6 +381,16 @@ function addSubjectColumnIfMissing(db: Database): void { db.exec("ALTER TABLE agents ADD COLUMN subject TEXT;"); } +/** agents.yolo: whether this launch bypassed permission prompts + (--dangerously-skip-permissions / --dangerously-bypass-approvals-and-sandbox). + Same conditional-exec rule as `sections`, `archived_at`, `handle`, `quiet` + and `subject` above. */ +function addYoloColumnIfMissing(db: Database): void { + const columns = db.query("PRAGMA table_info(agents);").all() as { name: string }[]; + if (columns.some((c) => c.name === "yolo")) return; + db.exec("ALTER TABLE agents ADD COLUMN yolo INTEGER;"); +} + /** * endpoint_claims.start_time (S068): the claiming pid's start-time, so a * recycled pid across a reboot reads as dead rather than pinning a port @@ -573,6 +583,7 @@ function runMigrations(db: Database, dir: string): void { addHandleColumnIfMissing(db); addQuietColumnIfMissing(db); addSubjectColumnIfMissing(db); + addYoloColumnIfMissing(db); // Legacy-JSON import is single-shot and only correct from a true // v0 (never-migrated) database: branch-cache's UPSERT would silently // overwrite current rows with stale ones, and project-mrs-store's diff --git a/lib/state/index.ts b/lib/state/index.ts index b66fded90e..b8b8899de8 100644 --- a/lib/state/index.ts +++ b/lib/state/index.ts @@ -141,7 +141,7 @@ export { } from "./chat-store.ts"; export { - insertAgent, getAgent, listAgents, updateAgentPane, markAgentResumed, + insertAgent, getAgent, listAgents, updateAgentPane, updateAgentSessionId, markAgentResumed, finishAgent, deleteAgent, newAgentId, pruneAgents, AGENTS_RETENTION_MS, type AgentRecord, type AgentSurface, } from "./agents-store.ts"; diff --git a/packages/rt-client/src/client.ts b/packages/rt-client/src/client.ts index 830c307dea..bfed9beec9 100644 --- a/packages/rt-client/src/client.ts +++ b/packages/rt-client/src/client.ts @@ -356,7 +356,7 @@ export function agentStart( a: Commands["agent:start"]["payload"], o: RtClientOptions = {}, ): Promise> { const payload: Record = { repo: a.repo, cwd: a.cwd }; - for (const k of ["prompt", "surface", "model", "effort", "account", "label", "caller", "workspace", "tab", "extraArgs", "env", "herdrSocket", "handle", "bg", "subject"] as const) { + for (const k of ["prompt", "surface", "provider", "model", "effort", "account", "label", "caller", "workspace", "tab", "extraArgs", "env", "herdrSocket", "handle", "bg", "subject", "yolo"] as const) { if (a[k] !== undefined) payload[k] = a[k]; } return rtCommand("agent:start", payload, { sockPath: o.sockPath, timeoutMs: o.timeoutMs ?? 30_000 }); diff --git a/packages/rt-client/src/commands.ts b/packages/rt-client/src/commands.ts index 6caced4f7b..f3cc0836be 100644 --- a/packages/rt-client/src/commands.ts +++ b/packages/rt-client/src/commands.ts @@ -360,7 +360,7 @@ export interface AgentRecord { PreToolUse hook gets injected at all. */ subject?: string; paneId?: string; tabId?: string; workspaceId?: string; - extraArgs?: string; exitCode?: number; resultPath?: string; + extraArgs?: string; exitCode?: number; resultPath?: string; yolo?: boolean; createdAt: number; lastResumedAt?: number; finishedAt?: number; } @@ -617,7 +617,7 @@ export interface Commands { "chat:dm-open": { payload: { from: string; to: string; sessionId?: string }; data: { room: string; created: boolean } }; // ─── Agent handoff (rt agent) ──────────────────────────────────────────── - "agent:start": { payload: { repo: string; cwd: string; prompt?: string; surface?: AgentSurface; model?: string; effort?: string; account?: string; label?: string; caller?: string; workspace?: string; tab?: string; extraArgs?: string; env?: Record; herdrSocket?: string; handle?: string; bg?: boolean; subject?: string }; data: AgentRecord }; + "agent:start": { payload: { repo: string; cwd: string; prompt?: string; surface?: AgentSurface; provider?: string; model?: string; effort?: string; account?: string; label?: string; caller?: string; workspace?: string; tab?: string; extraArgs?: string; env?: Record; herdrSocket?: string; handle?: string; bg?: boolean; subject?: string; yolo?: boolean }; data: AgentRecord }; "agent:resume": { payload: { id: string; prompt?: string; surface?: AgentSurface; workspace?: string; tab?: string }; data: AgentRecord }; "agent:get": { payload: { id: string }; data: AgentRecord }; "agent:list": { payload: { repo?: string }; data: { agents: AgentRecord[] } }; diff --git a/packages/rt-client/src/settings/__tests__/registry.test.ts b/packages/rt-client/src/settings/__tests__/registry.test.ts index fb32d1afeb..dc6af11b5e 100644 --- a/packages/rt-client/src/settings/__tests__/registry.test.ts +++ b/packages/rt-client/src/settings/__tests__/registry.test.ts @@ -239,7 +239,7 @@ describe("settings/registry", () => { expect(def?.merge).toBe("replace"); }); - test("has exactly the 26 migrated:true keys and the 43 suite keys", () => { + test("has exactly the 26 migrated:true keys and the 49 suite keys", () => { const migratedFalseKeys: string[] = []; const migratedTrueKeys = [ "rt.roles", "rt.intercepts", "rt.worktrees", "rt.worktreeReadyApproval", "rt.repoIdentityOverrides", "rt.repoRoots", @@ -300,16 +300,22 @@ describe("settings/registry", () => { "chat.herdrWorkspace", "chat.push.provider", "chat.push.target", - "agent.model", - "agent.effort", - "agent.account", - "agent.extraArgs", + "agent.provider", + "agent.claude.model", + "agent.claude.effort", + "agent.claude.account", + "agent.claude.extraArgs", + "agent.claude.yolo", + "agent.codex.model", + "agent.codex.effort", + "agent.codex.extraArgs", + "agent.codex.yolo", "rt.trustedBrowserOrigins", "rt.daemonPath", "rt.notify.eventBridges", "rt.gates.escalationTtlMinutes", ]; - expect(suiteKeys).toHaveLength(59); + expect(suiteKeys).toHaveLength(65); expect(allDefs().map((d) => d.key).sort()).toEqual( [...migratedFalseKeys, ...migratedTrueKeys, ...suiteKeys].sort(), diff --git a/packages/rt-client/src/settings/registry-defs.ts b/packages/rt-client/src/settings/registry-defs.ts index e117d46144..bc62fc040c 100644 --- a/packages/rt-client/src/settings/registry-defs.ts +++ b/packages/rt-client/src/settings/registry-defs.ts @@ -666,35 +666,80 @@ export const REGISTRY: readonly SettingDef[] = [ }, // --- agent (rt agent handoff) -------------------------------------------- - // No defaults by design: an unset key means the flag is omitted from the - // claude invocation entirely (spec "Settings"). + // No defaults on any per-provider row, by design: an unset key means the + // flag is omitted from the launch entirely (spec "Settings"). agent.provider + // is the one exception -- it needs a concrete fallback to preserve + // claude-only behavior with zero code changes for callers who never set it. { - key: "agent.model", + key: "agent.provider", type: "string", scopes: ["user", "machine"], + default: "claude", merge: "replace", - description: "Default --model for rt agent launches; unset omits the flag.", + description: "Which provider rt agent start uses when --provider is not given: \"claude\" or \"codex\".", }, { - key: "agent.effort", + key: "agent.claude.model", type: "string", scopes: ["user", "machine"], merge: "replace", - description: "Default --effort for rt agent launches; unset omits the flag.", + description: "Default --model for claude rt agent launches; unset omits the flag.", }, { - key: "agent.account", + key: "agent.claude.effort", type: "string", scopes: ["user", "machine"], merge: "replace", - description: "cswap account email rt agent launches under; unset uses the default claude profile.", + description: "Default --effort for claude rt agent launches; unset omits the flag.", }, { - key: "agent.extraArgs", + key: "agent.claude.account", type: "string", scopes: ["user", "machine"], merge: "replace", - description: "Opaque extra claude arguments appended to every rt agent launch (escape hatch).", + description: "cswap account email claude rt agent launches under; unset uses the default claude profile.", + }, + { + key: "agent.claude.extraArgs", + type: "string", + scopes: ["user", "machine"], + merge: "replace", + description: "Opaque extra claude arguments appended to every claude rt agent launch (escape hatch).", + }, + { + key: "agent.claude.yolo", + type: "boolean", + scopes: ["user", "machine"], + merge: "replace", + description: "Default --yolo (--dangerously-skip-permissions) for claude rt agent launches; unset behaves as false.", + }, + { + key: "agent.codex.model", + type: "string", + scopes: ["user", "machine"], + merge: "replace", + description: "Default -m/--model for codex rt agent launches; unset omits the flag.", + }, + { + key: "agent.codex.effort", + type: "string", + scopes: ["user", "machine"], + merge: "replace", + description: "Default reasoning effort for codex rt agent launches, passed as -c model_reasoning_effort=; unset omits the override.", + }, + { + key: "agent.codex.extraArgs", + type: "string", + scopes: ["user", "machine"], + merge: "replace", + description: "Opaque extra codex arguments appended to every codex rt agent launch (escape hatch).", + }, + { + key: "agent.codex.yolo", + type: "boolean", + scopes: ["user", "machine"], + merge: "replace", + description: "Default --yolo (--dangerously-bypass-approvals-and-sandbox) for codex rt agent launches; unset behaves as false. codex has no per-agent account setting (see spec's Non-goals).", }, // --- gates (escalation) ---------------------------------------------------- diff --git a/website/docs/reference/agent.mdx b/website/docs/reference/agent.mdx index f03674d6ea..d46ecc0924 100644 --- a/website/docs/reference/agent.mdx +++ b/website/docs/reference/agent.mdx @@ -7,7 +7,7 @@ sidebar_label: agent `rt › agent` -Hand a prompt to a Claude Code agent (herdr pane or headless) and keep the receipt +Hand a prompt to a coding agent, claude or codex (herdr pane or headless), and keep the receipt ## Usage @@ -25,14 +25,17 @@ rt agent [flags] | `--prompt` | text | | Initial prompt (required for headless) | | `--prompt-file` | text | | Read the prompt from a file (mutually exclusive with --prompt) | | `--surface` | text | | Where the agent runs (default herdr) | -| `--model` | text | | Override agent.model | -| `--effort` | text | | Override agent.effort | -| `--account` | text | | cswap account (override agent.account) | +| `--provider` | text | | Which CLI to launch (override agent.provider, default claude) | +| `--model` | text | | Override agent.<provider>.model | +| `--effort` | text | | Override agent.<provider>.effort | +| `--yolo` | boolean | | Bypass permission prompts for this launch (override agent.<provider>.yolo; omitted falls back to the setting, not false) | +| `--no-yolo` | boolean | | Keep permission prompts for this launch even when agent.<provider>.yolo is true (omitted falls back to the setting, not false) | +| `--account` | text | | cswap account, claude only (override agent.claude.account) | | `--label` | text | | Caller's display label; used as the herdr tab name | | `--caller` | text | | Identifies what invoked this handoff | | `--workspace` | text | | herdr workspace label (default: the repo label) | | `--tab` | text | | herdr tab label (default: the label or handoff id) | -| `--extra-args` | text | | Opaque extra claude arguments appended to the launch (override agent.extraArgs) | +| `--extra-args` | text | | Opaque extra provider arguments appended to the launch (override agent.<provider>.extraArgs) | | `--bg` | boolean | `false` | Launch onto the daemon-owned background herdr server instead of the visible one (herdr surface only; requires the rt daemon) | | [`--json`](/guides/common-flags) | boolean | `false` | Emit the record as JSON | diff --git a/website/docs/reference/herd/spawn.mdx b/website/docs/reference/herd/spawn.mdx index 89f660528e..324ddafe7e 100644 --- a/website/docs/reference/herd/spawn.mdx +++ b/website/docs/reference/herd/spawn.mdx @@ -23,8 +23,8 @@ rt herd spawn [flags] | `--job` | text | | Job name; also the worktree and handle stem | | `--brief` | text | | File whose text becomes the job brief | | `--dir` | text | | Existing directory to run in instead of a fresh worktree | -| `--model` | text | | Override agent.model for this worker | -| `--effort` | text | | Override agent.effort for this worker | +| `--model` | text | | Override agent.claude.model for this worker | +| `--effort` | text | | Override agent.claude.effort for this worker | | `--account` | text | | cswap account for this worker | | `--disposable` | boolean | `false` | Wrap-up may dispose this job's worktree | | [`--json`](/guides/common-flags) | boolean | `false` | Emit the spawn record as JSON |