Skip to content

Commit bc972f5

Browse files
committed
fix(exec): prompt on stdin TTY even with stdout piped
1 parent 4ab1b2d commit bc972f5

3 files changed

Lines changed: 97 additions & 5 deletions

File tree

‎docs/ARCHITECTURE.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ A worker deny-on-ask registers a harness-owned denied-call envelope (`src/permis
131131
- 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:
132132
- No workflow controller (`isWorkflowActive` is always false)
133133
- Non-interactive permission gate by default
134-
- `ask_operator` is unmounted when stdin/stdout are not TTYs (no cancel stub on the wire); TTY exec still prompts on stdin
134+
- `ask_operator` is unmounted when stdin is not a TTY (no cancel stub on the wire); TTY-stdin exec still prompts on stdin even with stdout piped
135135
- MCP connect is awaited before workflow resume and first inference, abort-capped at 15s so a hung handshake cannot block the run. A rejected batch does not disarm that abort while sibling dials are still in flight. A 1s log fires if connect is still in progress; remaining dials keep running until settle or abort. The TUI still fire-and-forgets connect (no rewrite). `tool_search` does not treat an empty catalog as a definitive miss while servers are still connecting.
136136
- Entry: `corbits exec "prompt"` (alias `corbits run`); `loadConfig` sets `command: "exec"`
137137
- Streams assistant text deltas to stdout; lifecycle errors to stderr

‎src/exec/runner.test.ts‎

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, expect, test } from "bun:test";
1+
import { describe, expect, spyOn, test } from "bun:test";
22
import type { AgentTool } from "@intx/agent";
33
import { submitOutputDefinition } from "../agent/director.js";
44
import { DIRECTOR_REGISTRY } from "../agent/directors/registry.js";
@@ -19,6 +19,7 @@ import {
1919
} from "../auth/codex/session.js";
2020
import type { Config } from "../config/index.js";
2121
import { CREDENTIAL_FAILURE_USER_MESSAGE } from "../inference-error-message.js";
22+
import type { PermissionRequest } from "../permission/types.js";
2223
import {
2324
clearActiveRun,
2425
getActiveRun,
@@ -44,9 +45,11 @@ import {
4445
createExecToolPromoter,
4546
execUserFailureMessage,
4647
isExecOverlayToolAllowed,
48+
promptPermission,
4749
refreshSelectedProviderCredential,
4850
resolveExecDirectorOverlay,
4951
resolveExecDirectorOverlayForPackage,
52+
resolveExecInteractive,
5053
runExec,
5154
} from "./runner.js";
5255

@@ -815,3 +818,63 @@ describe("exec tool call gate and promoter", () => {
815818
).not.toContain("list_dir");
816819
});
817820
});
821+
822+
describe("exec permission prompt gating (CL-9002)", () => {
823+
test("piped stdout with a stdin TTY still prompts", () => {
824+
expect(resolveExecInteractive({ stdinTTY: true, stdoutTTY: false })).toBe(
825+
true,
826+
);
827+
});
828+
829+
test("full TTY prompts", () => {
830+
expect(resolveExecInteractive({ stdinTTY: true, stdoutTTY: true })).toBe(
831+
true,
832+
);
833+
});
834+
835+
test("headless stdin never prompts, whatever stdout is", () => {
836+
expect(resolveExecInteractive({ stdinTTY: false, stdoutTTY: false })).toBe(
837+
false,
838+
);
839+
expect(resolveExecInteractive({ stdinTTY: false, stdoutTTY: true })).toBe(
840+
false,
841+
);
842+
expect(
843+
resolveExecInteractive({ stdinTTY: undefined, stdoutTTY: true }),
844+
).toBe(false);
845+
});
846+
847+
test("headless permission seam fails closed with an explicit stderr denial", async () => {
848+
const request: PermissionRequest = {
849+
tool: "run_shell",
850+
action: "Run shell",
851+
subject: "rm -rf /tmp/work",
852+
scopes: [],
853+
};
854+
const errWrites: string[] = [];
855+
const outWrites: string[] = [];
856+
const stderrSpy = spyOn(process.stderr, "write").mockImplementation(((
857+
chunk: unknown,
858+
) => {
859+
errWrites.push(String(chunk));
860+
return true;
861+
}) as typeof process.stderr.write);
862+
const stdoutSpy = spyOn(process.stdout, "write").mockImplementation(((
863+
chunk: unknown,
864+
) => {
865+
outWrites.push(String(chunk));
866+
return true;
867+
}) as typeof process.stdout.write);
868+
try {
869+
const outcome = await promptPermission(request, false);
870+
expect(outcome).toEqual({ allow: false });
871+
} finally {
872+
stderrSpy.mockRestore();
873+
stdoutSpy.mockRestore();
874+
}
875+
const denial = errWrites.join("");
876+
expect(denial).toContain("Run shell");
877+
expect(denial).toContain("--dangerously-skip-permissions");
878+
expect(outWrites).toEqual([]);
879+
});
880+
});

‎src/exec/runner.ts‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -670,7 +670,10 @@ export async function runExec(config: Config): Promise<ExecResult> {
670670
resolveSessionMode(config.settings, localSettingsForMode) ??
671671
"orchestrator";
672672

673-
const interactive = input.isTTY === true && output.isTTY === true;
673+
const interactive = resolveExecInteractive({
674+
stdinTTY: input.isTTY,
675+
stdoutTTY: output.isTTY,
676+
});
674677

675678
if (config.skipPermissionsFromSettings) {
676679
stderr.write(
@@ -1407,6 +1410,21 @@ export async function runExec(config: Config): Promise<ExecResult> {
14071410
}
14081411
}
14091412

1413+
/**
1414+
* Whether exec may prompt the operator. Permission and operator prompts read
1415+
* stdin and write stderr, so a piped stdout must not disable them: stdin TTY
1416+
* alone means an operator can answer. Fully headless (stdin not a TTY) fails
1417+
* closed — denials name the action and the remedy on stderr instead of
1418+
* prompting. stdoutTTY rides along so the call site stays explicit that a
1419+
* piped stdout is a supported prompting configuration, not an oversight.
1420+
*/
1421+
export function resolveExecInteractive(stdio: {
1422+
stdinTTY: boolean | undefined;
1423+
stdoutTTY: boolean | undefined;
1424+
}): boolean {
1425+
return stdio.stdinTTY === true;
1426+
}
1427+
14101428
async function promptOperator(
14111429
question: string,
14121430
options: string[],
@@ -1432,11 +1450,22 @@ async function promptOperator(
14321450
}
14331451
}
14341452

1435-
async function promptPermission(
1453+
export async function promptPermission(
14361454
request: PermissionRequest,
14371455
interactive: boolean,
14381456
): Promise<ApprovalOutcome> {
1439-
if (!interactive) return { allow: false };
1457+
if (!interactive) {
1458+
// Fail closed and say so on stderr (never stdout: piped stdout may feed
1459+
// JSON consumers — see printResumeHint). A headless ask has no operator
1460+
// to answer, so the denial names the action and the bypass instead of
1461+
// vanishing into a generic block. Same sentence as the gate's
1462+
// non-interactive deny reasons (src/permission/gate.ts) by intent; the
1463+
// gate stays canonical for model-facing text, this seam for the operator.
1464+
stderr.write(
1465+
`Permission denied: ${request.action} requires operator approval, which is unavailable in a non-interactive run. Re-run with --dangerously-skip-permissions to bypass, or narrow the action.\n`,
1466+
);
1467+
return { allow: false };
1468+
}
14401469
// Same freeze-while-deciding contract as the TUI gate: the tool wall-clock
14411470
// budget must not run down while the operator reads the prompt.
14421471
const budget = getToolApprovalBudget();

0 commit comments

Comments
 (0)