Skip to content

Commit 5b2de2e

Browse files
author
testikun
committed
fix(runtime): close sandbox negotiation retries safely
Generated-by: OpenAI Codex
1 parent 181087c commit 5b2de2e

4 files changed

Lines changed: 107 additions & 44 deletions

File tree

packages/core/src/__tests__/sandbox-boundary.test.ts

Lines changed: 64 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
decodeExecutionBoundary,
2727
executionBoundaryContains,
2828
executionBoundaryDisplayMode,
29-
projectSandboxBoundaryNegotiation,
29+
projectSandboxBoundaryNegotiation as projectSandboxBoundaryNegotiationImpl,
3030
type SandboxBoundaryRequest,
3131
validateSandboxBoundaryExpansion,
3232
} from '../sandbox-boundary.js';
@@ -40,6 +40,11 @@ import {
4040
type PermissionProfileManaged,
4141
} from '../permission-profile.js';
4242

43+
const projectSandboxBoundaryNegotiation = (
44+
events: readonly RuntimeEvent[],
45+
durableRequests: readonly SandboxBoundaryRequest[] = [],
46+
) => projectSandboxBoundaryNegotiationImpl(events, durableRequests);
47+
4348
describe('executionBoundaryDisplayMode', () => {
4449
test('keeps the read-only/writable distinction the boundary carries (#1611)', () => {
4550
assert.strictEqual(
@@ -512,9 +517,9 @@ describe('projectSandboxBoundaryNegotiation', () => {
512517
kind: 'valid',
513518
state: {
514519
denied: true,
515-
invalidRounds: 1,
516-
unresolvedRounds: 1,
517-
finalizationRequested: false,
520+
invalidRounds: 0,
521+
unresolvedRounds: 0,
522+
finalizationRequested: true,
518523
},
519524
});
520525
});
@@ -642,7 +647,15 @@ describe('projectSandboxBoundaryNegotiation', () => {
642647
kind: 'text',
643648
text: 'Tool arguments failed validation',
644649
};
645-
assert.equal(projectSandboxBoundaryNegotiation([call, response]).kind, 'invalid');
650+
assert.deepEqual(projectSandboxBoundaryNegotiation([call, response]), {
651+
kind: 'valid',
652+
state: {
653+
denied: false,
654+
invalidRounds: 0,
655+
unresolvedRounds: 0,
656+
finalizationRequested: false,
657+
},
658+
});
646659

647660
const blankJustification = request('request-2', 'boundary-2', 'tool-2');
648661
(
@@ -700,6 +713,52 @@ describe('projectSandboxBoundaryNegotiation', () => {
700713
assert.equal(projectSandboxBoundaryNegotiation([call, response]).kind, 'invalid');
701714
});
702715

716+
test('counts internal invalid repair calls as boundary attempts', () => {
717+
const events = [
718+
base('repair-call', {
719+
role: 'model',
720+
author: 'agent',
721+
refs: { toolCallId: 'repair-tool' },
722+
content: {
723+
kind: 'function_call',
724+
id: 'repair-tool',
725+
name: 'invalid',
726+
args: {
727+
tool: 'request_sandbox_boundary',
728+
error: 'boundary was denied',
729+
sandboxBoundaryAttempt: true,
730+
},
731+
},
732+
}),
733+
base('repair-response', {
734+
role: 'tool',
735+
author: 'tool',
736+
refs: { toolCallId: 'repair-tool' },
737+
content: {
738+
kind: 'function_response',
739+
id: 'repair-tool',
740+
name: 'invalid',
741+
isError: true,
742+
result: {
743+
kind: 'text',
744+
text: 'Sandbox boundary correction failed.',
745+
sandboxFailure: { reason: 'invalid_boundary_declaration' },
746+
},
747+
},
748+
}),
749+
];
750+
751+
assert.deepEqual(projectSandboxBoundaryNegotiation(events), {
752+
kind: 'valid',
753+
state: {
754+
denied: false,
755+
invalidRounds: 1,
756+
unresolvedRounds: 0,
757+
finalizationRequested: false,
758+
},
759+
});
760+
});
761+
703762
test('requests finalization after the bounded correction budget', () => {
704763
const events = [
705764
...failurePair(

packages/core/src/sandbox-boundary.ts

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ export interface SandboxBoundarySettlement {
199199
*/
200200
export function projectSandboxBoundaryNegotiation(
201201
events: readonly RuntimeEvent[],
202-
durableRequests: readonly SandboxBoundaryRequest[] = [],
202+
durableRequests: readonly SandboxBoundaryRequest[],
203203
): SandboxBoundaryNegotiationProjection {
204204
let denied = false;
205205
let invalidRounds = 0;
@@ -229,8 +229,25 @@ export function projectSandboxBoundaryNegotiation(
229229
kind: 'invalid',
230230
reason,
231231
});
232+
const applyApproval = (requestId: string): SandboxBoundaryNegotiationProjection | undefined => {
233+
if (denied || finalizationRequested) {
234+
return invalid(`sandbox boundary approval ${requestId} reopens a closed negotiation`);
235+
}
236+
denied = false;
237+
invalidRounds = 0;
238+
unresolvedRounds = 0;
239+
invalidSteps.clear();
240+
unresolvedSteps.clear();
241+
finalizationRequested = false;
242+
return undefined;
243+
};
232244
const addFailure = (kind: 'invalid' | 'unresolved', step: string): void => {
233245
hasStatefulEvent = true;
246+
if (denied) {
247+
finalizationRequested = true;
248+
return;
249+
}
250+
if (finalizationRequested) return;
234251
const steps = kind === 'invalid' ? invalidSteps : unresolvedSteps;
235252
if (steps.has(step)) return;
236253
steps.add(step);
@@ -325,17 +342,8 @@ export function projectSandboxBoundaryNegotiation(
325342
if (decision.status === 'denied') {
326343
denied = true;
327344
} else if (decision.status === 'approved') {
328-
if (denied || finalizationRequested) {
329-
return invalid(
330-
`sandbox boundary approval ${decision.requestId} reopens a closed negotiation`,
331-
);
332-
}
333-
denied = false;
334-
invalidRounds = 0;
335-
unresolvedRounds = 0;
336-
invalidSteps.clear();
337-
unresolvedSteps.clear();
338-
finalizationRequested = false;
345+
const approvalError = applyApproval(decision.requestId);
346+
if (approvalError) return approvalError;
339347
} else {
340348
addFailure('unresolved', `request:${decision.requestId}`);
341349
}
@@ -398,11 +406,6 @@ export function projectSandboxBoundaryNegotiation(
398406
}
399407
const step = event.refs?.stepId ?? boundaryCall?.step ?? call.step;
400408
addFailure(failure, step);
401-
} else if (content.isError === true && boundaryCall) {
402-
// Older ledgers did not carry a structured invalid-boundary marker. Do
403-
// not infer a count from their text; the relevant lineage is incomplete
404-
// and must be rejected closed by the continuation caller.
405-
return invalid(`sandbox boundary call ${content.id} lacks a canonical failure marker`);
406409
}
407410
if (boundaryCall) {
408411
if (boundaryResponses.has(content.id)) {
@@ -462,17 +465,8 @@ export function projectSandboxBoundaryNegotiation(
462465
if (request.status === 'denied') {
463466
denied = true;
464467
} else if (request.status === 'approved') {
465-
if (denied || finalizationRequested) {
466-
return invalid(
467-
`sandbox boundary approval ${request.requestId} reopens a closed negotiation`,
468-
);
469-
}
470-
denied = false;
471-
invalidRounds = 0;
472-
unresolvedRounds = 0;
473-
invalidSteps.clear();
474-
unresolvedSteps.clear();
475-
finalizationRequested = false;
468+
const approvalError = applyApproval(request.requestId);
469+
if (approvalError) return approvalError;
476470
} else {
477471
addFailure('unresolved', `request:${request.requestId}`);
478472
}
@@ -510,6 +504,13 @@ export function projectSandboxBoundaryNegotiation(
510504

511505
function isBoundaryAuthorityCall(toolName: string, args: unknown): boolean {
512506
if (toolName === 'request_sandbox_boundary') return true;
507+
if (
508+
toolName === 'invalid' &&
509+
isRecord(args) &&
510+
args.sandboxBoundaryAttempt === true
511+
) {
512+
return true;
513+
}
513514
if (toolName !== 'Bash' || !isRecord(args)) return false;
514515
return args.boundary_intent !== undefined && args.boundary_intent !== 'current';
515516
}

packages/runtime/src/runtime-kernel.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2847,12 +2847,23 @@ async function revalidateContinuationBoundary(
28472847
'Runtime continuation replay changed after planning',
28482848
);
28492849
}
2850+
const trimmedSuffixEventIds = new Set(
2851+
replay.plan.segments.flatMap((segment) => segment.trimmedSuffixEventIds),
2852+
);
2853+
const negotiationEvents = prefixes
2854+
.flatMap((prefix) => prefix.events)
2855+
.filter((event) => !trimmedSuffixEventIds.has(event.id));
28502856
const negotiation = projectSandboxBoundaryNegotiation(
2851-
prefixes.flatMap((prefix) => prefix.events),
2857+
negotiationEvents,
28522858
durableSandboxBoundaryRequests,
28532859
);
2854-
const sandboxBoundaryNegotiationState =
2855-
negotiation.kind === 'valid' ? negotiation.state : createSandboxBoundaryFinalizationState();
2860+
if (negotiation.kind !== 'valid') {
2861+
throw new RuntimeContinuationRevalidationError(
2862+
'source_replay_changed',
2863+
`Runtime continuation sandbox negotiation is invalid: ${negotiation.reason}`,
2864+
);
2865+
}
2866+
const sandboxBoundaryNegotiationState = negotiation.state;
28562867
return { events: [...prefixes.at(-1)!.events], sandboxBoundaryNegotiationState };
28572868
}
28582869

packages/runtime/src/runtime-resume.ts

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -452,14 +452,6 @@ export class RuntimeContinuationPlanner {
452452
'sandbox boundary interaction log is unavailable',
453453
);
454454
}
455-
try {
456-
await this.deps.readSandboxBoundaryRequests(input.sessionId);
457-
} catch {
458-
return parkedPlan(
459-
'continuation_authority_unavailable',
460-
'sandbox boundary interaction log is unavailable',
461-
);
462-
}
463455
let durableClaimState: ContinuationClaimStateV1 | undefined;
464456
try {
465457
durableClaimState = await this.deps.readContinuationClaimStateByBoundary?.(

0 commit comments

Comments
 (0)