From 5e72aaab88f94e5e30027ae2663b62d77f6a96e0 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Fri, 11 Sep 2026 20:10:59 -0700 Subject: [PATCH] Unmount ask_operator on non-TTY exec --- CHANGELOG.md | 6 ++ docs/ARCHITECTURE.md | 3 +- docs/PRODUCT.md | 4 +- scripts/eval-capability.test.ts | 1 + scripts/eval-capability.ts | 3 + src/agent/tool-search.test.ts | 23 ++++++- src/agent/tool-search.ts | 6 ++ src/agent/tools.ts | 105 +++++++++++++++-------------- src/exec/runner.ts | 9 ++- src/prompts.test.ts | 18 ++++- src/tui/runner/session.ts | 1 + tests/unit/exec/runner.test.ts | 55 +++++++++++++++ tests/unit/tui/agent-tools.test.ts | 11 +++ 13 files changed, 186 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a7c30f46..8d37a3db7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,12 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename ## [Unreleased] +### Changed + +- Headless `corbits exec` unmounts `ask_operator` when stdin/stdout are not + TTYs instead of advertising a cancel stub. TUI Skywalker still mounts it; + TTY exec still prompts on stdin. + ### Removed - Profile files no longer accept a `workflow` field. Workflows start only from diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 668d5c2cc..5537241c8 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -100,7 +100,8 @@ In TUI chat mode there is no completion gate — the session stays open across t - Product non-TUI agent path that **shares** the TUI stack (session mode, ChatDirector, toolset, permission gate, MCP, plugins, hooks, run-sink) without the OpenTUI shell - Bootstrap consumes the shared session assembly (`src/session/assemble-runtime.ts`, layered over `src/session/runtime-assembly.ts`) instead of hand-wiring a second copy. Intentional deltas vs TUI: - No workflow controller (`isWorkflowActive` is always false) - - Non-interactive permission gate by default; optional stdin for `ask_operator` + - Non-interactive permission gate by default + - `ask_operator` is unmounted when stdin/stdout are not TTYs (no cancel stub on the wire); TTY exec still prompts on stdin - Entry: `corbits exec "prompt"` (alias `corbits run`); `loadConfig` sets `command: "exec"` - Streams assistant text deltas to stdout; lifecycle errors to stderr - Shares ChatDirector compaction continuation (`requestContinuation` → content-less deliver after compact) so long runs do not stall post-compact diff --git a/docs/PRODUCT.md b/docs/PRODUCT.md index bf89d2804..3e472dc6d 100644 --- a/docs/PRODUCT.md +++ b/docs/PRODUCT.md @@ -40,7 +40,7 @@ The evidence is in how the product fails today: the personas already produce exc 4. **Safe by default** — Consequential actions (writes, edits, shell) pass a permission gate; secret files and catastrophic commands are denied outright, regardless of intent. 5. **Resume capability** — Runs persist to a git-backed store and resume from the last point after interruption. 6. **Legible loop** — A live event log, working-tree diff panel, plan tracker, and real-time cost meter show what happened, when, and why. -7. **Operator-in-the-loop** — The agent can call `ask_operator` to pause and ask a clarifying question; the operator answers from a modal (TUI) or via stdin when the product agent runs under `corbits exec`. +7. **Operator-in-the-loop** — The agent can call `ask_operator` to pause and ask a clarifying question; the operator answers from a modal (TUI). Headless `corbits exec` unmounts `ask_operator` when stdin/stdout are not TTYs. TTY exec still reads a single line from stdin. 8. **Mid-run steering** — Two modes while the agent is running, keyed to **whose** idle. **Parent-idle** is when the primary Skywalker turn is not inside an in-flight parent tool; **session-idle** is parent-idle **and** no live fleet lanes. **Enter** soft-steers while the parent is busy — delivers at the next **parent** `tool.boundary` without stopping the current run; a long parent `run_shell` or an awaiting `wait_agents` is parent-busy, so Enter is a queued steer, not a new turn. An in-flight TUI-primary `wait_agents` yields as a timeout when that steer is queued so occupancy can deliver it. Idle-with-fleet is shipped: after a non-blocking `spawn_agent` dispatch the parent goes idle while workers keep running, mailbox mail arrives as inbound when a worker finishes or fails, and mid-hold Enter starts a new primary turn instead of queueing a steer. **Alt+Enter** queues a follow-up delivered only on session-idle (`run` goes idle; does not interrupt). Session-idle Alt+Enter is a no-op. **Ctrl+C** stops the run outright. The notice row shows distinct `steer N` / `follow-up M` badges; when steers are pending and a parent tool has been in flight a few seconds, the notice names that command. Shortcuts are listed in `/help` (`Enter` soft-steer · `Alt+Enter` follow-up · `Ctrl+C` stop). 9. **Orchestrator-only (TUI + exec)** — The primary session is always the orchestrator: it can act directly and delegates via `spawn_agent` (then idle; mailbox mail inbound) / `search_agents`. Nested orchestrators still collect with `wait_agents`. Long jobs belong on workers — a parent that runs them itself stays parent-busy and holds Enter steers. Single-agent session mode, the first-run mode picker, and Settings → Session are gone (CL-5814). Legacy `sessionMode` values on disk are ignored. @@ -66,7 +66,7 @@ $ corbits exec "Add JWT auth to the API" $ corbits run "Add JWT auth to the API" ``` -Same directors, tools, permissions, MCP, plugins, and hooks as the TUI — without the OpenTUI shell. Bootstrap shares `src/session/assemble-runtime.ts` with the TUI; see `docs/ARCHITECTURE.md` “Exec Runner” for intentional deltas (no workflow controller; single primary send; non-interactive permission gate). Compaction continuation matches TUI so long runs do not stall after compact. Streams assistant text to stdout for scripts and CI. Non-interactive by default: actions that need operator approval are denied unless `--dangerously-skip-permissions` is set, a persisted `/yolo` default is on, or auto mode covers them. `--dangerously-skip-permissions` still forces this process; secret-guard and authz still apply. `ask_operator` reads a single line from stdin when available. +Same directors, tools, permissions, MCP, plugins, and hooks as the TUI — without the OpenTUI shell. Bootstrap shares `src/session/assemble-runtime.ts` with the TUI; see `docs/ARCHITECTURE.md` “Exec Runner” for intentional deltas (no workflow controller; single primary send; non-interactive permission gate; `ask_operator` unmounted when non-TTY). Compaction continuation matches TUI so long runs do not stall after compact. Streams assistant text to stdout for scripts and CI. Non-interactive by default: actions that need operator approval are denied unless `--dangerously-skip-permissions` is set, a persisted `/yolo` default is on, or auto mode covers them. `--dangerously-skip-permissions` still forces this process; secret-guard and authz still apply. `ask_operator` is not advertised on non-TTY exec; TTY exec still reads a single line from stdin. Local multi-model capability checks use this path (`bun run eval:capability`); see `evals/capability/README.md`. diff --git a/scripts/eval-capability.test.ts b/scripts/eval-capability.test.ts index 429033fb5..6d6ca258b 100644 --- a/scripts/eval-capability.test.ts +++ b/scripts/eval-capability.test.ts @@ -383,6 +383,7 @@ describe("buildEvalDiagnostics", () => { ); expect(diagnostics.advertisedTools).toContain("read_file"); expect(diagnostics.advertisedTools).toContain("run_shell"); + expect(diagnostics.advertisedTools).not.toContain("ask_operator"); expect(diagnostics.reasoningEffort).toBeNull(); }); diff --git a/scripts/eval-capability.ts b/scripts/eval-capability.ts index c8b59c712..b38882226 100755 --- a/scripts/eval-capability.ts +++ b/scripts/eval-capability.ts @@ -683,6 +683,9 @@ export async function buildEvalDiagnostics( overlay.advertisedAllow ?? advertisedToolNamesForSessionMode(sessionMode, { languageServerAvailable: detectLanguageServerAvailable(config.cwd), + // Capability evals run through exec; they are non-TTY, so ask_operator + // is unmounted the same way the runner does when interactive is false. + operatorAvailable: false, }); return { advertisedTools, diff --git a/src/agent/tool-search.test.ts b/src/agent/tool-search.test.ts index 17e37a1cf..2b3f752ad 100644 --- a/src/agent/tool-search.test.ts +++ b/src/agent/tool-search.test.ts @@ -165,10 +165,31 @@ describe("createToolIndex", () => { ).not.toContain("lsp"); }); - test("ask_operator is advertised regardless of availability", () => { + test("ask_operator is advertised when the operator is available", () => { expect( coreToolNamesForSessionMode("orchestrator", NO_AVAILABILITY), ).toContain("ask_operator"); + expect( + advertisedToolNamesForSessionMode("orchestrator", { + languageServerAvailable: true, + operatorAvailable: true, + }), + ).toContain("ask_operator"); + }); + + test("ask_operator is omitted from the advertised prefix when the operator is unavailable", () => { + expect( + coreToolNamesForSessionMode("orchestrator", { + languageServerAvailable: false, + operatorAvailable: false, + }), + ).not.toContain("ask_operator"); + expect( + advertisedToolNamesForSessionMode("orchestrator", { + languageServerAvailable: true, + operatorAvailable: false, + }), + ).not.toContain("ask_operator"); }); test("the advertised set is deterministic — repeat calls with the same inputs are identical", () => { diff --git a/src/agent/tool-search.ts b/src/agent/tool-search.ts index 964d52697..e753afdf6 100644 --- a/src/agent/tool-search.ts +++ b/src/agent/tool-search.ts @@ -71,6 +71,10 @@ export interface ToolAvailability { // Whether a language server was resolvable for this project at startup — // not whether one currently responds. languageServerAvailable: boolean; + // Headless/non-TTY exec has no operator to answer. Omit to keep the TUI + // default (mounted). False drops ask_operator from the advertised prefix + // instead of leaving a cancel stub on the wire. + operatorAvailable?: boolean; } export function coreToolNamesForSessionMode( @@ -82,6 +86,8 @@ export function coreToolNamesForSessionMode( if (!orchestratorEnabled && ORCHESTRATOR_ONLY_TOOL_NAMES.includes(name)) return false; if (name === "lsp") return availability.languageServerAvailable; + if (name === "ask_operator") + return availability.operatorAvailable !== false; return true; }); } diff --git a/src/agent/tools.ts b/src/agent/tools.ts index a22a700d5..db3ed89f0 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -146,7 +146,8 @@ export type OperatorResult = export interface AgentToolsetArgs { cwd: string; permissionGate: PermissionGate; - onOperatorGate: ( + // Interactive operator ask; omit on headless/non-TTY so the tool is unmounted. + onOperatorGate?: ( question: string, options: string[], ) => Promise; @@ -590,55 +591,59 @@ export async function createAgentToolset( return result.content; }, }), - stringTool({ - definition: askOperatorDefinition, - handler: async ( - rawArgs: Record, - _signal: AbortSignal, - ): Promise => { - const parsed = AskOperatorArgs(rawArgs); - if (parsed instanceof type.errors) { - return "Error: ask_operator requires question (string) and options (array of strings)."; - } - const { question, options } = parsed; - if (options.length === 0) { - return "Error: ask_operator requires at least one option."; - } - if (question.length > ASK_OPERATOR_QUESTION_MAX_CHARS) { - return ( - `Error: ask_operator question is ${question.length} characters; ` + - `keep it to ${ASK_OPERATOR_QUESTION_MAX_CHARS} or fewer. ` + - "Put the essay in a transcript reply first, then retry with a brief question." - ); - } - for (let i = 0; i < options.length; i++) { - const option = options[i] ?? ""; - if (option.length > ASK_OPERATOR_OPTION_MAX_CHARS) { - return ( - `Error: ask_operator option ${i + 1} is ${option.length} characters; ` + - `keep each label to ${ASK_OPERATOR_OPTION_MAX_CHARS} or fewer. ` + - "Put the essay in a transcript reply first, then retry with short option labels." - ); - } - } - const result = await onOperatorGate(question, options); - if (result.kind === "cancel") { - return "The operator dismissed the question without answering. Do not ask it again; proceed with your best judgment or continue with other work."; - } - if (result.kind === "custom") { - return result.text; - } - const { index } = result; - if (index < 0 || index >= options.length) { - return `Error: invalid selection ${index}. Valid range: 0-${options.length - 1}.`; - } - const selected = options[index]; - if (selected === undefined) { - return `Error: invalid selection ${index}. Valid range: 0-${options.length - 1}.`; - } - return selected; - }, - }), + ...(onOperatorGate !== undefined + ? [ + stringTool({ + definition: askOperatorDefinition, + handler: async ( + rawArgs: Record, + _signal: AbortSignal, + ): Promise => { + const parsed = AskOperatorArgs(rawArgs); + if (parsed instanceof type.errors) { + return "Error: ask_operator requires question (string) and options (array of strings)."; + } + const { question, options } = parsed; + if (options.length === 0) { + return "Error: ask_operator requires at least one option."; + } + if (question.length > ASK_OPERATOR_QUESTION_MAX_CHARS) { + return ( + `Error: ask_operator question is ${question.length} characters; ` + + `keep it to ${ASK_OPERATOR_QUESTION_MAX_CHARS} or fewer. ` + + "Put the essay in a transcript reply first, then retry with a brief question." + ); + } + for (let i = 0; i < options.length; i++) { + const option = options[i] ?? ""; + if (option.length > ASK_OPERATOR_OPTION_MAX_CHARS) { + return ( + `Error: ask_operator option ${i + 1} is ${option.length} characters; ` + + `keep each label to ${ASK_OPERATOR_OPTION_MAX_CHARS} or fewer. ` + + "Put the essay in a transcript reply first, then retry with short option labels." + ); + } + } + const result = await onOperatorGate(question, options); + if (result.kind === "cancel") { + return "The operator dismissed the question without answering. Do not ask it again; proceed with your best judgment or continue with other work."; + } + if (result.kind === "custom") { + return result.text; + } + const { index } = result; + if (index < 0 || index >= options.length) { + return `Error: invalid selection ${index}. Valid range: 0-${options.length - 1}.`; + } + const selected = options[index]; + if (selected === undefined) { + return `Error: invalid selection ${index}. Valid range: 0-${options.length - 1}.`; + } + return selected; + }, + }), + ] + : []), stringTool({ definition: presentDefinition, handler: async (rawArgs: Record): Promise => { diff --git a/src/exec/runner.ts b/src/exec/runner.ts index fd11ea6c0..3ee04b08b 100644 --- a/src/exec/runner.ts +++ b/src/exec/runner.ts @@ -588,6 +588,7 @@ export async function runExec(config: Config): Promise { const toolWatchdog = toolWatchdogFromSettings(config.settings); const toolAvailability: ToolAvailability = { languageServerAvailable: detectLanguageServerAvailable(config.cwd), + operatorAvailable: interactive, }; let currentAgent: Agent | null = null; @@ -628,8 +629,12 @@ export async function runExec(config: Config): Promise { isWorkflowActive: () => workflowHostHolder.instance?.isActive() === true, completeWorkflowStep: (stepId) => workflowHostHolder.instance?.complete(stepId) ?? "not-current", - onOperatorGate: (question, options) => - promptOperator(question, options, interactive), + ...(interactive + ? { + onOperatorGate: (question: string, options: string[]) => + promptOperator(question, options, true), + } + : {}), sessionMode, toolAvailability, ...(config.mcpServers !== undefined diff --git a/src/prompts.test.ts b/src/prompts.test.ts index 9cba2ca2f..93342e01d 100644 --- a/src/prompts.test.ts +++ b/src/prompts.test.ts @@ -396,9 +396,21 @@ test("sub-agent prompt carries the report-back contract and harness facts", () = }); test("when ask_director is in toolNames, the worker prompt mentions ask_director", () => { - const prompt = buildSubAgentSystemPrompt(undefined, undefined, undefined, { - toolNames: ["read_file", "ask_director"], - }); + const prompt = buildSubAgentSystemPrompt( + undefined, + { + cwd: "/repo/root", + platform: "Darwin 25.4.0", + arch: "arm64", + runtime: "Bun 1.2.0", + date: new Date(2026, 5, 5), + isGitRepo: false, + }, + undefined, + { + toolNames: ["read_file", "ask_director"], + }, + ); expect(prompt).toContain("ask_director"); expect(prompt).toContain("cannot reach the operator"); expect(prompt).not.toContain("ask_operator"); diff --git a/src/tui/runner/session.ts b/src/tui/runner/session.ts index 6346bd071..56803aa52 100644 --- a/src/tui/runner/session.ts +++ b/src/tui/runner/session.ts @@ -311,6 +311,7 @@ export async function assembleTUISession( }); const toolAvailability: ToolAvailability = { languageServerAvailable: detectLanguageServerAvailable(config.cwd), + operatorAvailable: true, }; // The workflow host is built below, after the toolset; the holder lets // submit_output's handler complete the live workflow without a diff --git a/tests/unit/exec/runner.test.ts b/tests/unit/exec/runner.test.ts index 974800b5c..a29f8b738 100644 --- a/tests/unit/exec/runner.test.ts +++ b/tests/unit/exec/runner.test.ts @@ -22,6 +22,7 @@ import { updatePlanDefinition, } from "../../../src/agent/codex-tool-proxies.js"; import { + advertisedToolNamesForSessionMode, createToolIndex, createToolSearchTool, } from "../../../src/agent/tool-search.js"; @@ -672,6 +673,60 @@ describe("resolveExecDirectorOverlay", () => { }); }); +describe("exec advertised tools vs TUI", () => { + const sessionMode = "orchestrator" as const; + + test("non-TTY exec advertised tools exclude ask_operator", () => { + const overlay = resolveExecDirectorOverlay("skywalker"); + const names = + overlay.advertisedAllow ?? + advertisedToolNamesForSessionMode(sessionMode, { + languageServerAvailable: false, + operatorAvailable: false, + }); + expect(names).not.toContain("ask_operator"); + const { computeAdvertised } = createAdvertisedToolset({ + sessionMode, + toolAvailability: { + languageServerAvailable: false, + operatorAvailable: false, + }, + getProvider: () => ({ providerName: "test", model: "test" }), + }); + expect( + computeAdvertised([ + { + name: "ask_operator", + description: "ask", + inputSchema: { type: "object", properties: {} }, + }, + { + name: "read_file", + description: "read", + inputSchema: { type: "object", properties: {} }, + }, + ]).map((d) => d.name), + ).not.toContain("ask_operator"); + }); + + test("TUI advertised tools still include ask_operator", () => { + const names = advertisedToolNamesForSessionMode(sessionMode, { + languageServerAvailable: false, + operatorAvailable: true, + }); + expect(names).toContain("ask_operator"); + const { isAdvertised } = createAdvertisedToolset({ + sessionMode, + toolAvailability: { + languageServerAvailable: false, + operatorAvailable: true, + }, + getProvider: () => ({ providerName: "test", model: "test" }), + }); + expect(isAdvertised("ask_operator")).toBe(true); + }); +}); + describe("exec tool call gate and promoter", () => { const stringTool = ( name: string, diff --git a/tests/unit/tui/agent-tools.test.ts b/tests/unit/tui/agent-tools.test.ts index f9d29147f..64a436c26 100644 --- a/tests/unit/tui/agent-tools.test.ts +++ b/tests/unit/tui/agent-tools.test.ts @@ -252,6 +252,17 @@ test("dynamicRunner contains posix tool names plus ask_operator", async () => { expect(names).not.toContain("apply_patch"); }); +test("dynamicRunner omits ask_operator when onOperatorGate is not provided", async () => { + const toolset = await createAgentToolset({ + cwd: "/fake", + permissionGate: fakePermissionGate, + }); + + const names = toolset.dynamicRunner.currentDefinitions().map((d) => d.name); + expect(names).not.toContain("ask_operator"); + expect(names).toContain("read_file"); +}); + test("onOperatorGate callback is invoked when the operator tool handler is called", async () => { let capturedQuestion = ""; let capturedOptions: string[] = [];