fix: retain verified ACPX commands across recovered turns
Keep runtime-owned command snapshots reusable until cleanup and observe optional prompt-start rejection without hiding it from callers. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
90cad7196b
commit
34d274f6d3
|
|
@ -113,6 +113,11 @@ Native ACPX recovery also admits a provider started lazily by model selection.
|
|||
Selection waits for verified process ownership before accepting the configured
|
||||
model; cleanup and out-of-band process launches remain fenced. This matters
|
||||
when reopening a persisted Codex session after stopping its sandbox.
|
||||
The host retains the verified command snapshot and its pinned descriptors until
|
||||
runtime cleanup, so reconnecting for the first turn after model selection can
|
||||
launch again without reopening a mutable executable path. A failed turn-start
|
||||
signal remains observable without crashing a sidecar that consumes only the
|
||||
turn's event stream and result.
|
||||
The new scoped-shell startup setting is not added to an existing unscoped Codex
|
||||
session's protected launch arguments. Its durable provider profile remains
|
||||
unchanged during attachment.
|
||||
|
|
|
|||
|
|
@ -1375,6 +1375,30 @@ describe("Codex ACPX runtime adapter", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("keeps a rejected prompt observable without an unhandled rejection for event-stream consumers", async () => {
|
||||
const runtime = fakeRuntime();
|
||||
const failure = new Error("recovered prompt failed");
|
||||
vi.mocked(runtime.startTurn).mockImplementation(() => ({
|
||||
requestId: "failed-turn",
|
||||
promptStarted: Promise.reject(failure),
|
||||
events: { async *[Symbol.asyncIterator]() { throw failure; } },
|
||||
result: Promise.reject(failure),
|
||||
cancel: vi.fn(), closeStream: vi.fn(),
|
||||
}));
|
||||
const port = await openCodexAcpxRuntime(openOptions(fakeCommand()), {
|
||||
createRegistry: () => registry(), createStore: () => store(),
|
||||
createRuntime: () => runtime,
|
||||
});
|
||||
const turn = port.startTurn({ text: "Resume.", requestId: "failed-turn" });
|
||||
// The sidecar consumes events and result; it never awaits promptStarted.
|
||||
const events = (async () => { for await (const event of turn.events) void event; })();
|
||||
await expect(events).rejects.toBe(failure);
|
||||
await expect(turn.result).rejects.toBe(failure);
|
||||
await new Promise<void>((resolve) => setTimeout(resolve, 10));
|
||||
await expect(turn.promptStarted).rejects.toBe(failure);
|
||||
await port.close({ reason: "failed prompt observed" });
|
||||
});
|
||||
|
||||
it("admits a verified provider that starts with the first recovered turn", async () => {
|
||||
const runtime = fakeRuntime();
|
||||
const child = fakeChild();
|
||||
|
|
|
|||
|
|
@ -1102,9 +1102,15 @@ function turnWithVerifiedLifetimeOwnership(
|
|||
finishOwnershipAdmission(),
|
||||
);
|
||||
void ownershipVerified.catch(() => undefined);
|
||||
const promptStarted = ownershipVerified.then(() => turn.promptStarted);
|
||||
// The sidecar consumes the event stream and result without awaiting this
|
||||
// optional admission signal. Observe its rejection immediately so a failed
|
||||
// recovered prompt cannot terminate the sidecar as an unhandled rejection.
|
||||
// Keep the original rejecting promise available to callers that await it.
|
||||
void promptStarted.catch(() => undefined);
|
||||
return {
|
||||
requestId: turn.requestId,
|
||||
promptStarted: ownershipVerified.then(() => turn.promptStarted),
|
||||
promptStarted,
|
||||
events: eventsAfterLifetimeOwnership(turn.events, ownershipVerified),
|
||||
result: ownershipVerified.then(() => turn.result),
|
||||
cancel: (input) => turn.cancel(input),
|
||||
|
|
|
|||
|
|
@ -727,6 +727,41 @@ describe("ACPX installation integrity", () => {
|
|||
await expectPinnedOutput(lease.spawn(), "verified");
|
||||
});
|
||||
|
||||
it("keeps command leases single-use unless reuse is explicitly requested", async () => {
|
||||
const fixture = await installationFixture();
|
||||
const installation = await verifyQualifiedAcpxInstallation(fixture.profile, fixture.resolve);
|
||||
const lease = await installation.openCommand();
|
||||
await expectPinnedOutput(lease.spawn(), "verified");
|
||||
expect(() => lease.spawn()).toThrow("Verified ACPX command lease is closed");
|
||||
await lease.close();
|
||||
});
|
||||
|
||||
it("reconnects with the same verified bytes after the entry path changes", async () => {
|
||||
const fixture = await installationFixture();
|
||||
const installation = await verifyQualifiedAcpxInstallation(fixture.profile, fixture.resolve);
|
||||
const lease = await installation.openCommand({ reusable: true });
|
||||
try {
|
||||
await expectPinnedOutput(lease.spawn(), "verified");
|
||||
await writeFile(fixture.commandPath, '#!/usr/bin/env node\nprocess.stdout.write("replacement");\n');
|
||||
await expectPinnedOutput(lease.spawn(), "verified");
|
||||
await expectPinnedOutput(lease.spawn(), "verified");
|
||||
} finally {
|
||||
await lease.close();
|
||||
}
|
||||
expect(() => lease.spawn()).toThrow("Verified ACPX command lease is closed");
|
||||
});
|
||||
|
||||
it("closing a reusable lease does not erase bytes already handed to a child", async () => {
|
||||
const fixture = await installationFixture();
|
||||
const installation = await verifyQualifiedAcpxInstallation(fixture.profile, fixture.resolve);
|
||||
const lease = await installation.openCommand({ reusable: true });
|
||||
const output = expectPinnedOutput(lease.spawn(), "verified");
|
||||
await lease.close();
|
||||
await output;
|
||||
await lease.close();
|
||||
expect(() => lease.spawn()).toThrow("Verified ACPX command lease is closed");
|
||||
});
|
||||
|
||||
it("launches the verified bytes after the open inode is modified", async () => {
|
||||
const fixture = await installationFixture();
|
||||
const installation = await verifyQualifiedAcpxInstallation(
|
||||
|
|
|
|||
|
|
@ -408,7 +408,8 @@ export interface VerifiedAcpxInstallation {
|
|||
readonly commandDigest: string;
|
||||
readonly agentServerPackageJsonPath: string;
|
||||
readonly agentRuntimePackageJsonPath: string | null;
|
||||
openCommand(): Promise<VerifiedAcpxCommandLease>;
|
||||
/** Reusable leases retain verified bytes and descriptors until explicitly closed. */
|
||||
openCommand(options?: { reusable?: boolean }): Promise<VerifiedAcpxCommandLease>;
|
||||
}
|
||||
|
||||
export interface VerifiedAcpxCommandLease {
|
||||
|
|
@ -708,7 +709,7 @@ export async function verifyQualifiedAcpxInstallation(
|
|||
commandDigest,
|
||||
agentServerPackageJsonPath: serverPackageJsonPath,
|
||||
agentRuntimePackageJsonPath: runtimePackageJsonPath,
|
||||
async openCommand(): Promise<VerifiedAcpxCommandLease> {
|
||||
async openCommand(options: { reusable?: boolean } = {}): Promise<VerifiedAcpxCommandLease> {
|
||||
const currentDirectory = await openVerifiedCommandDirectory(
|
||||
commandDirectory,
|
||||
"provider",
|
||||
|
|
@ -766,6 +767,7 @@ export async function verifyQualifiedAcpxInstallation(
|
|||
dependencyAncestorFormats,
|
||||
currentRuntimeExecutable,
|
||||
runtimeExecutable?.environmentVariable ?? null,
|
||||
options.reusable === true,
|
||||
);
|
||||
} catch (error) {
|
||||
await Promise.all([
|
||||
|
|
@ -1276,6 +1278,7 @@ function commandLease(
|
|||
providerRuntimeExecutable: FileHandle | null,
|
||||
providerRuntimeEnvironmentVariable:
|
||||
VerifiedAcpxRuntimeExecutable["environmentVariable"] | null,
|
||||
reusable: boolean,
|
||||
): VerifiedAcpxCommandLease {
|
||||
let consumed = false;
|
||||
let directoriesReleased = false;
|
||||
|
|
@ -1306,7 +1309,11 @@ function commandLease(
|
|||
lifetime?: VerifiedAcpxProviderLifetime,
|
||||
): ChildProcess {
|
||||
if (consumed) throw new Error("Verified ACPX command lease is closed");
|
||||
consumed = true;
|
||||
// ACPX may launch once for model selection and reconnect for the turn.
|
||||
// Reuse only the already verified snapshot and pinned descriptors, never
|
||||
// the mutable installation path. The runtime host closes this lease.
|
||||
if (!reusable) consumed = true;
|
||||
const launchBytes = reusable ? Buffer.from(verifiedBytes) : verifiedBytes;
|
||||
let child: ChildProcess;
|
||||
try {
|
||||
const guarded = lifetime !== undefined;
|
||||
|
|
@ -1467,22 +1474,27 @@ function commandLease(
|
|||
providerGuardianOwnership.set(child, ownership);
|
||||
}
|
||||
} catch (error) {
|
||||
consumed = true;
|
||||
launchBytes.fill(0);
|
||||
verifiedBytes.fill(0);
|
||||
releaseDirectoriesBestEffort();
|
||||
throw error;
|
||||
}
|
||||
releaseDirectoriesBestEffort();
|
||||
if (!reusable) releaseDirectoriesBestEffort();
|
||||
const sourceInput = child.stdio[COMMAND_SOURCE_FD] as Writable | null;
|
||||
if (sourceInput === null) {
|
||||
consumed = true;
|
||||
launchBytes.fill(0);
|
||||
verifiedBytes.fill(0);
|
||||
releaseDirectoriesBestEffort();
|
||||
child.kill();
|
||||
throw new Error("Verified ACPX command source pipe was not created");
|
||||
}
|
||||
const release = (): void => {
|
||||
verifiedBytes.fill(0);
|
||||
launchBytes.fill(0);
|
||||
};
|
||||
sourceInput.once("error", release);
|
||||
sourceInput.end(verifiedBytes, release);
|
||||
sourceInput.end(launchBytes, release);
|
||||
return child;
|
||||
},
|
||||
close,
|
||||
|
|
|
|||
|
|
@ -1219,6 +1219,7 @@ describe("ACPX runtime host", () => {
|
|||
),
|
||||
);
|
||||
await commandAdmissionStarted.promise;
|
||||
expect(openCommand).toHaveBeenCalledWith({ reusable: true });
|
||||
|
||||
controller.abort(cancellation);
|
||||
await expect(opening).rejects.toBe(cancellation);
|
||||
|
|
|
|||
|
|
@ -364,7 +364,7 @@ export class AcpxRuntimeHost {
|
|||
}
|
||||
command = await acquireAbortableAdmissionResource({
|
||||
signal: options.signal,
|
||||
acquire: () => installation.openCommand(),
|
||||
acquire: () => installation.openCommand({ reusable: true }),
|
||||
resource: "command",
|
||||
releaseLate: (lateCommand) => lateCommand.close(),
|
||||
reportFailure: (failure) =>
|
||||
|
|
|
|||
Loading…
Reference in New Issue