add support for -w,--writable-root to add more writable roots for sandbox (#263)
This adds support for a new flag, `-w,--writable-root`, that can be specified multiple times to _amend_ the list of folders that should be configured as "writable roots" by the sandbox used in `full-auto` mode. Values that are passed as relative paths will be resolved to absolute paths. Incidentally, this required updating a number of the `agent*.test.ts` files: it feels like some of the setup logic across those tests could be consolidated. In my testing, it seems that this might be slightly out of distribution for the model, as I had to explicitly tell it to run `apply_patch` and that it had the permissions to write those files (initially, it just showed me a diff and told me to apply it myself). Nevertheless, I think this is a good starting point.
This commit is contained in:
@@ -45,6 +45,9 @@ type AgentLoopParams = {
|
||||
onItem: (item: ResponseItem) => void;
|
||||
onLoading: (loading: boolean) => void;
|
||||
|
||||
/** Extra writable roots to use with sandbox execution. */
|
||||
additionalWritableRoots: ReadonlyArray<string>;
|
||||
|
||||
/** Called when the command is not auto-approved to request explicit user review. */
|
||||
getCommandConfirmation: (
|
||||
command: Array<string>,
|
||||
@@ -58,6 +61,7 @@ export class AgentLoop {
|
||||
private instructions?: string;
|
||||
private approvalPolicy: ApprovalPolicy;
|
||||
private config: AppConfig;
|
||||
private additionalWritableRoots: ReadonlyArray<string>;
|
||||
|
||||
// Using `InstanceType<typeof OpenAI>` sidesteps typing issues with the OpenAI package under
|
||||
// the TS 5+ `moduleResolution=bundler` setup. OpenAI client instance. We keep the concrete
|
||||
@@ -213,6 +217,7 @@ export class AgentLoop {
|
||||
onLoading,
|
||||
getCommandConfirmation,
|
||||
onLastResponseId,
|
||||
additionalWritableRoots,
|
||||
}: AgentLoopParams & { config?: AppConfig }) {
|
||||
this.model = model;
|
||||
this.instructions = instructions;
|
||||
@@ -229,6 +234,7 @@ export class AgentLoop {
|
||||
model,
|
||||
instructions: instructions ?? "",
|
||||
} as AppConfig);
|
||||
this.additionalWritableRoots = additionalWritableRoots;
|
||||
this.onItem = onItem;
|
||||
this.onLoading = onLoading;
|
||||
this.getCommandConfirmation = getCommandConfirmation;
|
||||
@@ -358,6 +364,7 @@ export class AgentLoop {
|
||||
args,
|
||||
this.config,
|
||||
this.approvalPolicy,
|
||||
this.additionalWritableRoots,
|
||||
this.getCommandConfirmation,
|
||||
this.execAbortController?.signal,
|
||||
);
|
||||
|
||||
@@ -16,7 +16,12 @@ const DEFAULT_TIMEOUT_MS = 10_000; // 10 seconds
|
||||
* mapped to a non-zero exit code and the error message should be in stderr.
|
||||
*/
|
||||
export function exec(
|
||||
{ cmd, workdir, timeoutInMillis }: ExecInput,
|
||||
{
|
||||
cmd,
|
||||
workdir,
|
||||
timeoutInMillis,
|
||||
additionalWritableRoots,
|
||||
}: ExecInput & { additionalWritableRoots: ReadonlyArray<string> },
|
||||
sandbox: SandboxType,
|
||||
abortSignal?: AbortSignal,
|
||||
): Promise<ExecResult> {
|
||||
@@ -30,7 +35,12 @@ export function exec(
|
||||
timeout: timeoutInMillis || DEFAULT_TIMEOUT_MS,
|
||||
...(workdir ? { cwd: workdir } : {}),
|
||||
};
|
||||
const writableRoots = [process.cwd(), os.tmpdir()];
|
||||
// Merge default writable roots with any user-specified ones.
|
||||
const writableRoots = [
|
||||
process.cwd(),
|
||||
os.tmpdir(),
|
||||
...additionalWritableRoots,
|
||||
];
|
||||
return execForSandbox(cmd, opts, writableRoots, abortSignal);
|
||||
}
|
||||
|
||||
|
||||
@@ -74,6 +74,7 @@ export async function handleExecCommand(
|
||||
args: ExecInput,
|
||||
config: AppConfig,
|
||||
policy: ApprovalPolicy,
|
||||
additionalWritableRoots: ReadonlyArray<string>,
|
||||
getCommandConfirmation: (
|
||||
command: Array<string>,
|
||||
applyPatch: ApplyPatchCommand | undefined,
|
||||
@@ -91,6 +92,7 @@ export async function handleExecCommand(
|
||||
args,
|
||||
/* applyPatch */ undefined,
|
||||
/* runInSandbox */ false,
|
||||
additionalWritableRoots,
|
||||
abortSignal,
|
||||
).then(convertSummaryToResult);
|
||||
}
|
||||
@@ -138,6 +140,7 @@ export async function handleExecCommand(
|
||||
args,
|
||||
applyPatch,
|
||||
runInSandbox,
|
||||
additionalWritableRoots,
|
||||
abortSignal,
|
||||
);
|
||||
// If the operation was aborted in the meantime, propagate the cancellation
|
||||
@@ -170,7 +173,13 @@ export async function handleExecCommand(
|
||||
} else {
|
||||
// The user has approved the command, so we will run it outside of the
|
||||
// sandbox.
|
||||
const summary = await execCommand(args, applyPatch, false, abortSignal);
|
||||
const summary = await execCommand(
|
||||
args,
|
||||
applyPatch,
|
||||
false,
|
||||
additionalWritableRoots,
|
||||
abortSignal,
|
||||
);
|
||||
return convertSummaryToResult(summary);
|
||||
}
|
||||
} else {
|
||||
@@ -202,6 +211,7 @@ async function execCommand(
|
||||
execInput: ExecInput,
|
||||
applyPatchCommand: ApplyPatchCommand | undefined,
|
||||
runInSandbox: boolean,
|
||||
additionalWritableRoots: ReadonlyArray<string>,
|
||||
abortSignal?: AbortSignal,
|
||||
): Promise<ExecCommandSummary> {
|
||||
let { workdir } = execInput;
|
||||
@@ -239,7 +249,11 @@ async function execCommand(
|
||||
const execResult =
|
||||
applyPatchCommand != null
|
||||
? execApplyPatch(applyPatchCommand.patch)
|
||||
: await exec(execInput, await getSandbox(runInSandbox), abortSignal);
|
||||
: await exec(
|
||||
{ ...execInput, additionalWritableRoots },
|
||||
await getSandbox(runInSandbox),
|
||||
abortSignal,
|
||||
);
|
||||
const duration = Date.now() - start;
|
||||
const { stdout, stderr, exitCode } = execResult;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user