Fix sandbox command-session and lazy ACP model recovery

This commit is contained in:
Dotta 2026-09-09 04:44:59 -05:00
parent 17ddb89618
commit cb381c6a5f
8 changed files with 255 additions and 19 deletions

View File

@ -104,6 +104,15 @@ whose cached handles still say running. Failure to stop a reusable sandbox is
reported and retained for retry; it never falls back to deletion or orphan cleanup.
This applies to both runner generations and leaves distinct task/user bindings
isolated.
If Daytona rejects a command because its cached shell session no longer exists,
the provider creates one replacement session and retries within the original
command deadline. This applies only to a confirmed rejection before dispatch;
errors while polling or reading output never replay a potentially executed
command. Concurrent callers share the replacement session.
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 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.

View File

@ -1273,6 +1273,70 @@ describe("Codex ACPX runtime adapter", () => {
}
});
it("verifies a provider lazily started by recovered model selection before admitting it", async () => {
const runtime = fakeRuntime();
const command = fakeCommand();
vi.mocked(command.spawn).mockReturnValue(fakeChild());
let runtimeOptions: AcpRuntimeOptions | undefined;
let confirmOwnership!: () => void;
const ownership = new Promise<void>((resolve) => { confirmOwnership = resolve; });
const observedOwnership = vi.fn(() => ownership);
vi.mocked(runtime.setConfigOption!).mockImplementation(async () => {
await Promise.resolve();
runtimeOptions!.spawnAgent!({ command: "ignored", args: ["--stdio"], options: {} });
});
const port = await openCodexAcpxRuntime(openOptions(command), {
createRegistry: () => registry(), createStore: () => store(),
awaitProviderOwnership: observedOwnership,
awaitProviderExit: providerOwnershipEstablished,
createRuntime: (options) => { runtimeOptions = options; return runtime; },
});
let admitted = false;
const selection = port.setModel!("gpt-5.6-sol").then(() => { admitted = true; });
await vi.waitFor(() => expect(observedOwnership).toHaveBeenCalledOnce());
expect(admitted).toBe(false);
confirmOwnership();
await selection;
expect(admitted).toBe(true);
expect(command.spawn).toHaveBeenCalledOnce();
expect(() => runtimeOptions!.spawnAgent!({ command: "ignored", args: [], options: {} }))
.toThrow("provider spawned after ownership admission was sealed");
await port.close({ reason: "model selection verified" });
});
it("reseals model admission after a rejected config operation", async () => {
const runtime = fakeRuntime();
const failure = new Error("model selection rejected");
vi.mocked(runtime.setConfigOption!).mockRejectedValueOnce(failure).mockResolvedValueOnce(undefined as never);
const port = await openCodexAcpxRuntime(openOptions(fakeCommand()), {
createRegistry: () => registry(), createStore: () => store(), createRuntime: () => runtime,
});
await expect(port.setModel!("gpt-5.6-sol")).rejects.toBe(failure);
await expect(port.setModel!("gpt-5.6-sol")).resolves.toBeUndefined();
await port.close({ reason: "selection error verified" });
await expect(port.setModel!("gpt-5.6-sol")).rejects.toThrow("ownership admission is closed");
expect(runtime.setConfigOption).toHaveBeenCalledTimes(2);
});
it("rejects model admission when the lazily started provider cannot prove ownership", async () => {
const runtime = fakeRuntime();
const command = fakeCommand();
vi.mocked(command.spawn).mockReturnValue(fakeChild());
let runtimeOptions: AcpRuntimeOptions | undefined;
const failure = new Error("provider ownership unavailable");
vi.mocked(runtime.setConfigOption!).mockImplementation(async () => {
runtimeOptions!.spawnAgent!({ command: "ignored", args: [], options: {} });
});
const port = await openCodexAcpxRuntime(openOptions(command), {
createRegistry: () => registry(), createStore: () => store(),
awaitProviderOwnership: async () => { throw failure; },
awaitProviderExit: providerOwnershipEstablished,
createRuntime: (options) => { runtimeOptions = options; return runtime; },
});
await expect(port.setModel!("gpt-5.6-sol")).rejects.toBe(failure);
await port.close({ reason: "unverified model process rejected" });
});
it("maps prompt turns to the admitted ACPX handle", async () => {
const runtime = fakeRuntime();
const turn = {

View File

@ -1037,11 +1037,20 @@ function runtimePort(
...(runtime.setConfigOption
? {
async setModel(model: string) {
await runtime.setConfigOption?.({
handle,
key: "model",
value: model,
});
// Loading a persisted session can leave its provider unstarted.
// ACP config selection may start it before the first prompt, so
// admit and verify that process just as we do for a resumed turn.
const finishOwnershipAdmission =
children.beginLifetimeOwnershipAdmission();
try {
await runtime.setConfigOption?.({
handle,
key: "model",
value: model,
});
} finally {
await finishOwnershipAdmission();
}
},
}
: {}),

View File

@ -1499,6 +1499,77 @@ describe("Daytona sandbox provider plugin", () => {
expect(sessionId).toMatch(/^paperclip-/);
});
const missingSession = () => Object.assign(new MockDaytonaNotFoundError("session not found"), {
code: "PROCESS_NOT_FOUND", statusCode: 404,
});
it("replaces a disappeared session only when dispatch was rejected", async () => {
process.env.DAYTONA_API_KEY = "host-key";
const sandbox = createMockSandbox();
mockGet.mockResolvedValue(sandbox);
await plugin.definition.onEnvironmentExecute?.(sessionExecParams());
sandbox.process.executeSessionCommand.mockRejectedValueOnce(missingSession());
await plugin.definition.onEnvironmentExecute?.(sessionExecParams({ stdin: "input" }));
expect(sandbox.process.createSession).toHaveBeenCalledTimes(2);
const ids = sandbox.process.executeSessionCommand.mock.calls.map(([id]) => id);
expect(ids[0]).toBe(ids[1]);
expect(ids[2]).not.toBe(ids[1]);
expect(sandbox.fs.uploadFile).toHaveBeenCalledTimes(2);
expect(sandbox.fs.deleteFile).toHaveBeenCalledTimes(2);
});
it("shares one replacement when concurrent commands find the same missing session", async () => {
process.env.DAYTONA_API_KEY = "host-key";
const sandbox = createMockSandbox();
mockGet.mockResolvedValue(sandbox);
await plugin.definition.onEnvironmentExecute?.(sessionExecParams());
const staleId = sandbox.process.createSession.mock.calls[0]![0];
sandbox.process.executeSessionCommand.mockImplementation(async (id: string) => {
if (id === staleId) throw missingSession();
return { cmdId: "accepted" };
});
await Promise.all([
plugin.definition.onEnvironmentExecute?.(sessionExecParams()),
plugin.definition.onEnvironmentExecute?.(sessionExecParams()),
]);
expect(sandbox.process.createSession).toHaveBeenCalledTimes(2);
const replacementId = sandbox.process.createSession.mock.calls[1]![0];
expect(sandbox.process.executeSessionCommand.mock.calls.filter(([id]) => id === replacementId)).toHaveLength(2);
});
it("does not replay an accepted command when its session disappears while reading its status", async () => {
process.env.DAYTONA_API_KEY = "host-key";
const sandbox = createMockSandbox();
mockGet.mockResolvedValue(sandbox);
const error = missingSession();
sandbox.process.getSessionCommand.mockRejectedValue(error);
await expect(plugin.definition.onEnvironmentExecute?.(sessionExecParams())).rejects.toBe(error);
expect(sandbox.process.executeSessionCommand).toHaveBeenCalledTimes(1);
expect(sandbox.process.createSession).toHaveBeenCalledTimes(1);
});
it.each([
new Error("session not found"),
new MockDaytonaNotFoundError("sandbox not found"),
])("does not retry an unclassified dispatch failure: %s", async (error) => {
process.env.DAYTONA_API_KEY = "host-key";
const sandbox = createMockSandbox();
mockGet.mockResolvedValue(sandbox);
sandbox.process.executeSessionCommand.mockRejectedValueOnce(error);
await expect(plugin.definition.onEnvironmentExecute?.(sessionExecParams())).rejects.toBe(error);
expect(sandbox.process.executeSessionCommand).toHaveBeenCalledTimes(1);
});
it("bounds missing-session recovery to one replacement", async () => {
process.env.DAYTONA_API_KEY = "host-key";
const sandbox = createMockSandbox();
mockGet.mockResolvedValue(sandbox);
sandbox.process.executeSessionCommand.mockRejectedValue(missingSession());
await expect(plugin.definition.onEnvironmentExecute?.(sessionExecParams())).rejects.toThrow("before command dispatch");
expect(sandbox.process.executeSessionCommand).toHaveBeenCalledTimes(2);
expect(sandbox.process.createSession).toHaveBeenCalledTimes(2);
});
it("opens one session when two first commands overlap", async () => {
process.env.DAYTONA_API_KEY = "host-key";
const sandbox = createMockSandbox();

View File

@ -1413,8 +1413,11 @@ const sandboxHandleSessionStore = (() => {
idByKey.set(sandboxHandleCacheKey(scope), sessionId);
}
function clear(scope: SandboxScope): void {
idByKey.delete(sandboxHandleCacheKey(scope));
function clear(scope: SandboxScope, expectedSessionId?: string): void {
const key = sandboxHandleCacheKey(scope);
if (expectedSessionId === undefined || idByKey.get(key) === expectedSessionId) {
idByKey.delete(key);
}
}
// Single-flight guard for the first-command session create. Two overlapping
@ -1794,6 +1797,12 @@ async function readSessionExitCode(
return null;
}
class MissingSessionBeforeDispatchError extends Error {
constructor(cause: Error) {
super("Daytona session was unavailable before command dispatch", { cause });
}
}
// Dispatch one user command into the persistent session and return its true
// stdout and stderr.
//
@ -1850,7 +1859,17 @@ async function executeInSession(
sessionId,
{ command, runAsync: true },
timeoutSeconds,
);
).catch((error: unknown) => {
// The daemon rejected the session lookup before accepting a command.
// Never classify errors from polling/logs this way: that command may
// already have executed and must not be replayed.
if (error instanceof DaytonaNotFoundError
&& error.code === "PROCESS_NOT_FOUND"
&& error.message.trim().toLowerCase() === "session not found") {
throw new MissingSessionBeforeDispatchError(error);
}
throw error;
});
const commandId = dispatched.cmdId;
// Log-stream path. A session command always tries the stream first: it
@ -2706,7 +2725,23 @@ const plugin = definePlugin({
let result: PluginEnvironmentExecuteResult;
if (!params.bypassSession) {
const sessionId = await getOrCreateSession(sandbox, scope);
result = await executeInSession(sandbox, sessionId, params, config);
const configuredTimeout = resolveTimeoutMs(params.timeoutMs, config);
const dispatchDeadline = Date.now() + (isGitNetworkCommand(params.command, params.args ?? [])
? Math.min(configuredTimeout, GIT_NETWORK_TIMEOUT_MS) : configuredTimeout);
try {
result = await executeInSession(sandbox, sessionId, params, config);
} catch (error) {
if (!(error instanceof MissingSessionBeforeDispatchError)) throw error;
// A stopped/restarted provider can lose its shell independently of
// this worker's cache. Replace only the rejected id; overlapping
// callers must share, rather than invalidate, its replacement.
sandboxHandleSessionStore.clear(scope, sessionId);
if (Date.now() >= dispatchDeadline) throw error;
const replacementId = await getOrCreateSession(sandbox, scope);
const remainingMs = dispatchDeadline - Date.now();
if (remainingMs <= 0) throw error;
result = await executeInSession(sandbox, replacementId, { ...params, timeoutMs: remainingMs }, config);
}
} else {
result = await executeOneShot(sandbox, params, config);
}

View File

@ -1,5 +1,31 @@
import { describe, expect, it } from "vitest";
import { isStagingOrigin, assertDeployedAdapterExclusions, deployedAgentEngine } from "./deployed-stack.js";
import { describe, expect, it, vi } from "vitest";
import { isStagingOrigin, assertDeployedAdapterExclusions, deployedAgentEngine, findDeployedWorkFile } from "./deployed-stack.js";
describe("deployed work-file pagination", () => {
it.each([false, true])("finds a later-page entry while preserving trash=%s", async (trash) => {
const file = { path: "nested/empty.sh", byteSize: 0, executable: true };
const json = vi.fn().mockResolvedValueOnce({ files: [{ path: "unrelated" }], nextCursor: "next-page" })
.mockResolvedValueOnce({ files: [file], nextCursor: null });
expect(await findDeployedWorkFile({ json }, "/api/folder", file.path, trash)).toEqual(file);
expect(json.mock.calls.map(([url]) => url)).toEqual([
`/api/folder?limit=200&trash=${trash}`,
`/api/folder?limit=200&trash=${trash}&cursor=next-page`,
]);
});
it("returns missing only after the final page", async () => {
const json = vi.fn().mockResolvedValueOnce({ files: [], nextCursor: "next-page" })
.mockResolvedValueOnce({ files: [], nextCursor: null });
expect(await findDeployedWorkFile({ json }, "/api/folder", "missing")).toBeUndefined();
expect(json).toHaveBeenCalledTimes(2);
});
it("rejects a repeated cursor instead of looping", async () => {
const json = vi.fn().mockResolvedValue({ files: [], nextCursor: "same-page" });
await expect(findDeployedWorkFile({ json }, "/api/folder", "missing")).rejects.toThrow("repeated its cursor");
expect(json).toHaveBeenCalledTimes(2);
});
});
describe("deployed stack target", () => {
it("requires an explicit HTTPS staging tenant and rejects credential-bearing URLs", () => {

View File

@ -1,5 +1,30 @@
import fs from "node:fs";
import assert from "node:assert/strict";
import type { WorkFolderListing } from "../../packages/shared/src/work-folders.js";
/** Find one entry without assuming a shared folder fits in the first page. */
export async function findDeployedWorkFile(
api: Pick<DeployedStackApi, "json">,
folderPath: string,
filePath: string,
trash = false,
) {
const cursors = new Set<string>();
let cursor: string | null = null;
do {
const query = new URLSearchParams({ limit: "200", trash: String(trash) });
if (cursor) query.set("cursor", cursor);
const listing: WorkFolderListing = await api.json(`${folderPath}?${query}`);
const file = listing.files.find((entry) => entry.path === filePath);
if (file) return file;
cursor = listing.nextCursor;
if (cursor) {
assert(!cursors.has(cursor), "Work-folder listing repeated its cursor");
cursors.add(cursor);
}
} while (cursor);
return undefined;
}
// A separate target contract deliberately has no local-server fallback.
export function isStagingOrigin(value: string) {

View File

@ -1,10 +1,10 @@
import { randomUUID } from "node:crypto";
import { test, expect } from "@playwright/test";
import type { EnvironmentCapabilities } from "../../packages/shared/src/environment-support.js";
import type { SandboxWorkFolderManifest, WorkFolderListing, WorkFolderSyncStatus } from "../../packages/shared/src/work-folders.js";
import type { SandboxWorkFolderManifest, WorkFolderSyncStatus } from "../../packages/shared/src/work-folders.js";
import { QUALIFIED_ACPX_RUNNER_MODELS } from "../../server/src/services/native-runtime/provider-profile.js";
import { pollUntil } from "./api.js";
import { DeployedStackApi, deployedAgentEngine, loadDeployedStack } from "./deployed-stack.js";
import { DeployedStackApi, deployedAgentEngine, findDeployedWorkFile, loadDeployedStack } from "./deployed-stack.js";
import { repoAcceptancePrompt } from "./work-folder-acceptance-prompts.js";
@ -60,15 +60,13 @@ for (const [scope, owner] of [["task", stack.taskId], ["agent", stack.agentId],
});
expect((await write()).ok).toBe(true);
expect((await write()).ok).toBe(true);
const listing = await api.json<WorkFolderListing>(base);
const file = listing.files.find((entry) => entry.path === filename)!;
const file = (await findDeployedWorkFile(api, base, filename))!;
expect(file).toMatchObject({ byteSize: 0, executable: true, deletedAt: null });
const download = await api.request(`${base}/content?path=${encodeURIComponent(filename)}`);
expect(download.status).toBe(200); expect((await download.arrayBuffer()).byteLength).toBe(0);
await api.json(`${base}/operations`, "POST", { action: "delete", path: filename });
expect((await api.request(`${base}/content?path=${encodeURIComponent(filename)}`)).status).toBe(404);
const trash = await api.json<WorkFolderListing>(`${base}?trash=true`);
expect(trash.files.some((entry) => entry.id === file.id)).toBe(true);
expect((await findDeployedWorkFile(api, base, filename, true))?.id).toBe(file.id);
await api.json(`${base}/operations`, "POST", { action: "restore", fileId: file.id });
expect((await api.request(`${base}/content?path=${encodeURIComponent(filename)}`)).status).toBe(200);
});
@ -109,8 +107,7 @@ for (const profile of stack.profiles) {
const read = await api.request(`${scoped}/content?path=${encodeURIComponent(`roundtrip-${nonce}/message.txt`)}`);
expect(read.status, `${profile.id} ${scope} durable bytes`).toBe(200);
expect(await read.text()).toBe(nonce);
const listing = await api.json<WorkFolderListing>(scoped);
expect(listing.files.find(file => file.path === `roundtrip-${nonce}/empty.sh`)).toMatchObject({byteSize:0, executable:true});
expect(await findDeployedWorkFile(api, scoped, `roundtrip-${nonce}/empty.sh`)).toMatchObject({byteSize:0, executable:true});
}
await api.json(`/api/issues/${issue.id}`, "PATCH", { status: "todo", description: repoAcceptancePrompt(nonce, true) });