Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/control-plane/src/session/durable-object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,7 @@ export class SessionDO extends DurableObject<Env> {
messageCreatedAt,
terminalMessageCompletedAt: completedAt,
}),
(error, completedAt) => this.messageQueue.failUnfinishedMessages(error, completedAt),
this.statusService,
(timestamp) => this.lifecycleManager.updateLastActivity(timestamp),
() => this.lifecycleManager.scheduleInactivityCheck(),
Expand Down
52 changes: 52 additions & 0 deletions packages/control-plane/src/session/message-queue.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1346,4 +1346,56 @@ describe("SessionMessageQueue", () => {
expect(h.repository.updateParticipantCoalesce).not.toHaveBeenCalled();
});
});

describe("failUnfinishedMessages", () => {
it("settles a prompt that never left pending and tells its caller why", () => {
const h = buildQueue();
// A boot failure fails the prompt before it is ever dispatched, so it is
// still "pending" — failStuckProcessingMessage only ever sees "processing".
h.repository.listUnfinishedMessages.mockReturnValue([
createMessage({ id: "msg-boot", status: "pending" }),
]);

const failed = h.queue.failUnfinishedMessages("git sync failed for owner/repo", 5000);

expect(failed).toBe(1);
expect(h.repository.recordMessageCompletion).toHaveBeenCalledWith(
expect.objectContaining({
messageId: "msg-boot",
success: false,
error: "git sync failed for owner/repo",
}),
5000,
"pending"
);
expect(h.callbackService.notifyComplete).toHaveBeenCalledWith(
"msg-boot",
false,
"git sync failed for owner/repo"
);
});

it("settles a processing prompt against its own status", () => {
const h = buildQueue();
h.repository.listUnfinishedMessages.mockReturnValue([
createMessage({ id: "msg-running", status: "processing" }),
]);

h.queue.failUnfinishedMessages("sandbox died", 5000);

expect(h.repository.recordMessageCompletion).toHaveBeenCalledWith(
expect.anything(),
5000,
"processing"
);
});

it("stays quiet when there is nothing in flight to settle", () => {
const h = buildQueue();
h.repository.listUnfinishedMessages.mockReturnValue([]);

expect(h.queue.failUnfinishedMessages("sandbox died", 5000)).toBe(0);
expect(h.callbackService.notifyComplete).not.toHaveBeenCalled();
});
});
});
21 changes: 21 additions & 0 deletions packages/control-plane/src/session/message-queue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,27 @@ export class SessionMessageQueue {
await this.sessionStatus.reconcileAfterExecution(false);
}

/**
* Settle every in-flight prompt because the sandbox reported it cannot continue.
*
* A boot failure fails the prompt before it is ever dispatched, so it is still
* `pending` — `failStuckProcessingMessage` only ever sees `processing` — and
* only `execution_complete` otherwise notifies the originating client. Without
* this the caller that asked for the work is told nothing at all.
*/
failUnfinishedMessages(error: string, completedAt: number): number {
let failed = 0;
for (const message of this.messageRepository.listUnfinishedMessages()) {
const expectedStatus = message.status === "processing" ? "processing" : "pending";
if (this.failMessage(message, error, completedAt, expectedStatus)) failed++;
}
if (failed > 0) {
this.messenger.broadcast({ type: "processing_status", isProcessing: false });
this.broadcastPromptQueue();
}
return failed;
}

private failMessage(
message: { id: string; created_at: number },
error: string,
Expand Down
43 changes: 42 additions & 1 deletion packages/control-plane/src/session/sandbox-events.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,11 @@ function createProcessor() {
const diffService = { pinBaselines: vi.fn() };
const triggerSnapshot = vi.fn(async (_reason: string) => {});
const projectTerminalMessage = vi.fn(async () => {});
const statusService = { reconcileAfterExecution: vi.fn(async (_success: boolean) => {}) };
const failUnfinishedMessages = vi.fn((_error: string, _completedAt: number) => 0);
const statusService = {
reconcileAfterExecution: vi.fn(async (_success: boolean) => {}),
transition: vi.fn(async (_status: string) => true),
};
const scheduleInactivityCheck = vi.fn(async () => {});
const processMessageQueue = vi.fn(async () => {});
const broadcastPromptQueue = vi.fn();
Expand Down Expand Up @@ -100,6 +104,7 @@ function createProcessor() {
applySessionTitleUpdate,
triggerSnapshot,
projectTerminalMessage,
failUnfinishedMessages,
statusService as unknown as SessionStatusService,
updateLastActivity,
scheduleInactivityCheck,
Expand All @@ -118,6 +123,7 @@ function createProcessor() {
diffService,
triggerSnapshot,
projectTerminalMessage,
failUnfinishedMessages,
statusService,
scheduleInactivityCheck,
processMessageQueue,
Expand Down Expand Up @@ -939,4 +945,39 @@ describe("SessionSandboxEventProcessor", () => {
expect(h.wsManager.send).not.toHaveBeenCalled();
});
});

it("settles in-flight prompts and drives the session to failed on a fatal error", async () => {
const h = createProcessor();

await h.processor.processSandboxEvent({
type: "error",
error: "git sync failed for owner/repo",
sandboxId: "sandbox-1",
timestamp: 4000,
fatal: true,
} as never);

// Settling is what notifies whoever asked for the work; before this a boot
// failure reached them as silence and the session sat "active" forever.
expect(h.failUnfinishedMessages).toHaveBeenCalledWith(
"git sync failed for owner/repo",
expect.any(Number)
);
expect(h.statusService.transition).toHaveBeenCalledWith("failed");
});

it("leaves in-flight prompts and status alone for a non-fatal error event", async () => {
const h = createProcessor();

await h.processor.processSandboxEvent({
type: "error",
error: "tool blew up",
sandboxId: "sandbox-1",
timestamp: 4000,
messageId: "msg-1",
} as never);

expect(h.failUnfinishedMessages).not.toHaveBeenCalled();
expect(h.statusService.transition).not.toHaveBeenCalled();
});
});
14 changes: 14 additions & 0 deletions packages/control-plane/src/session/sandbox-events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ export class SessionSandboxEventProcessor {
messageCreatedAt: number,
completedAt: number
) => Promise<void>,
/** Settles in-flight prompts and notifies their caller; owned by the message queue. */
private readonly failUnfinishedMessages: (error: string, completedAt: number) => number,
private readonly statusService: SessionStatusService,
private readonly updateLastActivity: (timestamp: number) => void,
private readonly scheduleInactivityCheck: () => Promise<void>,
Expand Down Expand Up @@ -293,6 +295,18 @@ export class SessionSandboxEventProcessor {

this.messenger.broadcast({ type: "sandbox_event", event });

// A fatal error is the sandbox saying it cannot continue. Only
// execution_complete otherwise moves status or notifies the caller, so
// without this a boot failure — a repository that will not clone, an image
// that will not start — left the session sitting "active" with no messages
// forever, and whoever asked for the work hearing nothing at all. Settle
// in-flight prompts so the reason reaches them, and transition after persist
// and broadcast so it is on record first.
if (event.type === "error" && event.fatal) {
this.failUnfinishedMessages(event.error, now);
await this.statusService.transition("failed");
}

if (CRITICAL_EVENT_TYPES.has(event.type)) {
this.sendAck(ackId);
}
Expand Down
11 changes: 10 additions & 1 deletion packages/shared/src/types/sandbox-events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,18 @@ export const sandboxEventSchema = z.discriminatedUnion("type", [
status: gitSyncStatusSchema,
sha: z.string().optional(),
}),
messageSandboxEventBaseSchema.extend({
sandboxEventBaseSchema.extend({
type: z.literal("error"),
error: z.string(),
// Message-scoped when the bridge reports a failure mid-execution, absent
// for a fatal boot error — that happens before any message exists, which
// is why requiring messageId here hard-rejected exactly the case the
// event most needs to carry. processSandboxEvent already falls back to the
// processing message when it is absent.
messageId: z.string().optional(),
// The sandbox cannot continue. Drives the session to "failed" and settles
// in-flight prompts; a plain error stays advisory and leaves status alone.
fatal: z.boolean().optional(),
isSubtask: z.boolean().optional(),
childSessionId: z.string().optional(),
taskCallId: z.string().optional(),
Expand Down
4 changes: 4 additions & 0 deletions packages/web/src/lib/timeline-items.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ export function buildTimelineItems(events: SandboxEvent[]): TimelineItem[] {
for (const event of deduped) {
if (!("isSubtask" in event) || !event.isSubtask || !("taskCallId" in event)) continue;
if (!event.taskCallId) continue;
// An `error` event carries an optional messageId — a fatal boot error
// happens before any message exists — so it cannot be keyed to a parent
// Task call. Leave it at the top level rather than mis-nesting it.
if (!event.messageId) continue;

const key = taskKey(event.messageId, event.taskCallId);
if (!tasks.has(key)) continue;
Expand Down
Loading