Skip to content

Commit 814d06c

Browse files
Merge pull request #1289 from corbitsdev/cl-9002-exec-gate-stdin-tty
fix(exec): prompt on stdin TTY even with stdout piped
2 parents 4ab1b2d + 96e54da commit 814d06c

5 files changed

Lines changed: 193 additions & 6 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: 146 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { describe, expect, test } from "bun:test";
1+
import { describe, expect, spyOn, test } from "bun:test";
2+
import type { ToolCall } from "@intx/types/runtime";
23
import type { AgentTool } from "@intx/agent";
34
import { submitOutputDefinition } from "../agent/director.js";
45
import { DIRECTOR_REGISTRY } from "../agent/directors/registry.js";
@@ -19,6 +20,7 @@ import {
1920
} from "../auth/codex/session.js";
2021
import type { Config } from "../config/index.js";
2122
import { CREDENTIAL_FAILURE_USER_MESSAGE } from "../inference-error-message.js";
23+
import { createPermissionGate } from "../permission/gate.js";
2224
import {
2325
clearActiveRun,
2426
getActiveRun,
@@ -47,6 +49,7 @@ import {
4749
refreshSelectedProviderCredential,
4850
resolveExecDirectorOverlay,
4951
resolveExecDirectorOverlayForPackage,
52+
resolveExecInteractive,
5053
runExec,
5154
} from "./runner.js";
5255

@@ -815,3 +818,145 @@ 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+
// Exec-identical headless gate wiring: interactive:false with the prompt seam
848+
// armed to throw, so a test passes only when decide() denies on the real
849+
// production path (the old direct-seam test never went through decide()).
850+
const headlessGate = (onDeny: (reason: string) => void) =>
851+
createPermissionGate({
852+
approvals: [],
853+
requestApproval: async () => {
854+
throw new Error("headless run must never reach the prompt seam");
855+
},
856+
interactive: false,
857+
onHeadlessDeny: onDeny,
858+
skipPermissions: false,
859+
reactorGated: true,
860+
});
861+
862+
const captureStdio = () => {
863+
const errWrites: string[] = [];
864+
const outWrites: string[] = [];
865+
const stderrSpy = spyOn(process.stderr, "write").mockImplementation(((
866+
chunk: unknown,
867+
) => {
868+
errWrites.push(String(chunk));
869+
return true;
870+
}) as typeof process.stderr.write);
871+
const stdoutSpy = spyOn(process.stdout, "write").mockImplementation(((
872+
chunk: unknown,
873+
) => {
874+
outWrites.push(String(chunk));
875+
return true;
876+
}) as typeof process.stdout.write);
877+
return {
878+
errWrites,
879+
outWrites,
880+
restore: () => {
881+
stderrSpy.mockRestore();
882+
stdoutSpy.mockRestore();
883+
},
884+
};
885+
};
886+
887+
test("headless gate deny names the action and remedy on stderr, stdout clean", async () => {
888+
const stdio = captureStdio();
889+
try {
890+
// Same shape as runner.ts: the denial surfaces on stderr, never stdout.
891+
const gate = headlessGate((reason) => {
892+
process.stderr.write(`Permission denied: ${reason}\n`);
893+
});
894+
const authorized = await gate.authorizeCall({
895+
id: "h1",
896+
name: "web_fetch",
897+
arguments: { url: "https://example.com/docs", format: "markdown" },
898+
} as ToolCall);
899+
expect(authorized.effect).toBe("deny");
900+
const evaluated = await gate.evaluate({
901+
id: "h2",
902+
name: "web_fetch",
903+
arguments: { url: "https://example.com/other", format: "markdown" },
904+
} as ToolCall);
905+
expect(evaluated.allowed).toBe(false);
906+
} finally {
907+
stdio.restore();
908+
}
909+
const denial = stdio.errWrites.join("");
910+
expect(denial).toContain("Permission denied:");
911+
expect(denial).toContain("requires operator approval");
912+
expect(denial).toContain("--dangerously-skip-permissions");
913+
expect(stdio.outWrites).toEqual([]);
914+
});
915+
916+
test("secret-path headless deny names the action without bypass coaching", async () => {
917+
const stdio = captureStdio();
918+
try {
919+
const gate = headlessGate((reason) => {
920+
process.stderr.write(`Permission denied: ${reason}\n`);
921+
});
922+
const verdict = await gate.evaluate({
923+
id: "s1",
924+
name: "run_shell",
925+
arguments: { command: "cat .env" },
926+
} as ToolCall);
927+
expect(verdict.allowed).toBe(false);
928+
} finally {
929+
stdio.restore();
930+
}
931+
const denial = stdio.errWrites.join("");
932+
expect(denial).toContain("sensitive path");
933+
expect(denial).not.toContain("--dangerously-skip-permissions");
934+
expect(stdio.outWrites).toEqual([]);
935+
});
936+
937+
test("piped stdout with a stdin TTY keeps the ask advertised and round-trips", async () => {
938+
expect(resolveExecInteractive({ stdinTTY: true, stdoutTTY: false })).toBe(
939+
true,
940+
);
941+
const seen: string[] = [];
942+
const gate = createPermissionGate({
943+
approvals: [],
944+
requestApproval: async (request) => {
945+
seen.push(request.subject);
946+
return { allow: true };
947+
},
948+
interactive: true,
949+
skipPermissions: false,
950+
reactorGated: true,
951+
});
952+
const call = {
953+
id: "i1",
954+
name: "web_fetch",
955+
arguments: { url: "https://example.com/docs", format: "markdown" },
956+
} as ToolCall;
957+
const authorized = await gate.authorizeCall(call);
958+
expect(authorized.effect).toBe("ask");
959+
expect(await gate.evaluate(call)).toEqual({ allowed: true });
960+
expect(seen).toEqual(["https://example.com/docs"]);
961+
});
962+
});

‎src/exec/runner.ts‎

Lines changed: 30 additions & 4 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(
@@ -685,7 +688,7 @@ export async function runExec(config: Config): Promise<ExecResult> {
685688
model: config.model,
686689
telemetry: liveTelemetry,
687690
requestApproval: (request: PermissionRequest): Promise<ApprovalOutcome> =>
688-
promptPermission(request, interactive),
691+
promptPermission(request),
689692
getActiveProviderModel: () => `${config.providerName}:${config.model}`,
690693
onPersistNotice: (text) => {
691694
stderr.write(`${text}\n`);
@@ -694,6 +697,14 @@ export async function runExec(config: Config): Promise<ExecResult> {
694697
stderr.write(`${text}\n`);
695698
},
696699
interactive,
700+
// Headless operator surface: the gate denies without an operator before
701+
// ever reaching requestApproval, so the explicit denial lives here —
702+
// naming the action and the remedy on stderr (never stdout: piped stdout
703+
// may feed JSON consumers). The reason already omits the bypass remedy
704+
// on sensitive-path denials; emit it verbatim.
705+
onHeadlessDeny: (reason) => {
706+
stderr.write(`Permission denied: ${reason}\n`);
707+
},
697708
skipPermissions: config.dangerouslySkipPermissions,
698709
auto: config.auto,
699710
// Main session: gating rides the reactor's approval-suspend seam. In
@@ -1407,6 +1418,23 @@ export async function runExec(config: Config): Promise<ExecResult> {
14071418
}
14081419
}
14091420

1421+
/**
1422+
* Whether exec may prompt the operator. Permission and operator prompts read
1423+
* stdin and write stderr, so a piped stdout must not disable them: stdin TTY
1424+
* alone means an operator can answer. Fully headless (stdin not a TTY) fails
1425+
* closed — denials name the action and the remedy on stderr instead of
1426+
* prompting (gate onHeadlessDeny wiring above, not the prompt seam: the gate
1427+
* denies headless asks before requestApproval is ever reached). stdoutTTY
1428+
* rides along so the call site stays explicit that a
1429+
* piped stdout is a supported prompting configuration, not an oversight.
1430+
*/
1431+
export function resolveExecInteractive(stdio: {
1432+
stdinTTY: boolean | undefined;
1433+
stdoutTTY: boolean | undefined;
1434+
}): boolean {
1435+
return stdio.stdinTTY === true;
1436+
}
1437+
14101438
async function promptOperator(
14111439
question: string,
14121440
options: string[],
@@ -1434,9 +1462,7 @@ async function promptOperator(
14341462

14351463
async function promptPermission(
14361464
request: PermissionRequest,
1437-
interactive: boolean,
14381465
): Promise<ApprovalOutcome> {
1439-
if (!interactive) return { allow: false };
14401466
// Same freeze-while-deciding contract as the TUI gate: the tool wall-clock
14411467
// budget must not run down while the operator reads the prompt.
14421468
const budget = getToolApprovalBudget();

‎src/permission/gate.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,13 @@ export interface PermissionGateOptions {
395395
// No operator is attached (headless). An unresolved "ask" becomes a denial
396396
// unless skipPermissions is set.
397397
interactive: boolean;
398+
// Headless operator surface: fired with the deny reason when decide() denies
399+
// because no operator is attached (the !interactive exits above). Exec wires
400+
// it to stderr so the denial names the action and the remedy; gates without
401+
// an operator surface omit it and stay silent by construction. The reason
402+
// already carries the secret-path distinction (no bypass remedy on
403+
// sensitive-path denials), so callers emit it verbatim.
404+
onHeadlessDeny?: ((reason: string) => void) | undefined;
398405
// The --dangerously-skip-permissions escape hatch: auto-allow anything the
399406
// authorization layer did not already deny.
400407
skipPermissions: boolean;
@@ -1023,6 +1030,7 @@ export function createPermissionGate(
10231030
? `${request.action} references a sensitive path and requires operator approval, which is unavailable in a non-interactive run.`
10241031
: `${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.`;
10251032
denialMemory.record(stableId, reason);
1033+
options.onHeadlessDeny?.(reason);
10261034
return { kind: "deny", reason };
10271035
}
10281036

@@ -1067,6 +1075,7 @@ export function createPermissionGate(
10671075
recordAutoDecision(request.tool, "non-interactive", "deny");
10681076
const reason = `${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.`;
10691077
denialMemory.record(stableId, reason);
1078+
options.onHeadlessDeny?.(reason);
10701079
return { kind: "deny", reason };
10711080
}
10721081

‎src/session/assemble-runtime.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,12 @@ export interface SessionGateArgs {
265265
*/
266266
onPendingProjectGrants?: ((text: string) => void) | undefined;
267267
interactive: boolean;
268+
/**
269+
* Headless operator surface (see PermissionGateOptions.onHeadlessDeny):
270+
* fired with the deny reason when the gate denies for want of an operator.
271+
* Exec writes it to stderr; the TUI omits it.
272+
*/
273+
onHeadlessDeny?: ((reason: string) => void) | undefined;
268274
skipPermissions: boolean;
269275
auto?: boolean | undefined;
270276
/** Route this gate's decisions through the reactor authz seam (main session). */
@@ -308,6 +314,7 @@ export async function assembleSessionGate(
308314
),
309315
approvalLog: createApprovalLog(sessionDir(args.cwd, args.sessionId)),
310316
interactive: args.interactive,
317+
onHeadlessDeny: args.onHeadlessDeny,
311318
skipPermissions: args.skipPermissions,
312319
auto: args.auto,
313320
reactorGated: args.reactorGated,

0 commit comments

Comments
 (0)