Skip to content
Merged
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
47 changes: 45 additions & 2 deletions src/subagent/mailbox-mail-drive.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ describe("driveMailboxMail", () => {
}),
).toBe(false);
expect(
await driveMailboxMail({
driveMailboxMail({
parentProcessing: false,
mailbox: mapMailbox(new Map()),
lanes: [],
Expand Down Expand Up @@ -315,7 +315,7 @@ describe("driveMailboxMail", () => {
["live", { status: "running" }],
]);
expect(
await driveMailboxMail({
driveMailboxMail({
parentProcessing: false,
mailbox: mapMailbox(records),
lanes: [],
Expand All @@ -329,6 +329,28 @@ describe("driveMailboxMail", () => {
).toBe(false);
});

test("does not begin if the parent starts processing during collect", async () => {
const records = new Map<string, FleetDryMailboxRecord>([
["w1", { status: "done", report: "ok" }],
]);
let processing = false;
const driven = driveMailboxMail({
parentProcessing: false,
isParentProcessing: () => processing,
mailbox: mapMailbox(records),
lanes: [],
beginSystemContinuation: () => {
throw new Error("must not begin");
},
send: () => {
throw new Error("must not send");
},
});
processing = true;
expect(await driven).toBe(false);
expect(records.get("w1")?.collected).not.toBe(true);
});

test("not-delivered send leaves the wake retryable", async () => {
const records = new Map<string, FleetDryMailboxRecord>([
["w1", { status: "done", report: "ok" }],
Expand Down Expand Up @@ -395,8 +417,10 @@ describe("latchMailboxMailDrive", () => {
}),
);
expect(driver()).toBe(true);
expect(driver.claimed()).toBe(true);
expect(driver()).toBe(false);
expect(driver()).toBe(false);
expect(driver.claimed()).toBe(true);
await sent;
expect(sends).toHaveLength(1);
expect(sends[0]).toContain(MAILBOX_MAIL_WAKE_PREFIX);
Expand All @@ -410,9 +434,28 @@ describe("latchMailboxMailDrive", () => {
});
expect(driver()).toBe(false);
expect(driver()).toBe(false);
expect(driver.claimed()).toBe(false);
expect(calls).toBe(2);
});

test("an empty mailbox does not claim the occupancy slot", () => {
const driver = latchMailboxMailDrive(() =>
driveMailboxMail({
parentProcessing: false,
mailbox: mapMailbox(new Map()),
lanes: [],
beginSystemContinuation: () => {
throw new Error("must not begin");
},
send: () => {
throw new Error("must not send");
},
}),
);
expect(driver()).toBe(false);
expect(driver.claimed()).toBe(false);
});

test("after the in-flight drive settles, a new terminal can send", async () => {
const records = new Map<string, FleetDryMailboxRecord>([
["first", { status: "done", report: "one" }],
Expand Down
49 changes: 46 additions & 3 deletions src/subagent/mailbox-mail-drive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,16 +109,41 @@ export function buildMailboxMailPrompt(
return [mailboxMailWakeLine(), JSON.stringify(reports)].join("\n");
}

function hasDeliverableMailboxMail(
mailbox: FleetDryMailbox | undefined,
): boolean {
if (mailbox === undefined) return false;
const delivering = deliveringByMailbox.get(mailbox);
for (const id of mailbox.ids()) {
if (delivering?.has(id)) continue;
const record = mailbox.peek(id);
if (record === undefined || record.collected === true) continue;
if (isLiveWaitStatus(record.status)) continue;
return true;
}
return false;
}

function parentIsProcessing(args: {
parentProcessing: boolean;
isParentProcessing?: () => boolean;
}): boolean {
return args.isParentProcessing?.() ?? args.parentProcessing;
}

export function driveMailboxMail(args: {
parentProcessing: boolean;
/** Live check after awaited collect; occupancy must not start a turn already claimed. */
isParentProcessing?: () => boolean;
mailbox: FleetDryMailbox | undefined;
lanes: readonly FleetDryLane[];
writeBlob?: FleetDryBlobWriter;
beginSystemContinuation: (prompt: string) => void;
send: (prompt: string) => unknown;
onSendFailure?: () => void;
}): boolean | Promise<boolean> {
if (args.parentProcessing) return false;
if (parentIsProcessing(args)) return false;
if (!hasDeliverableMailboxMail(args.mailbox)) return false;
return driveMailboxMailAfterCollect(args);
}

Expand All @@ -138,6 +163,7 @@ async function driveMailboxMailAfterCollect(
)
).filter((report) => !delivering.has(report.agent_id));
if (reports.length === 0) return false;
if (parentIsProcessing(args)) return false;
const ids = reports.map((report) => report.agent_id);
for (const id of ids) delivering.add(id);
const prompt = buildMailboxMailPrompt(reports);
Expand Down Expand Up @@ -169,12 +195,28 @@ async function driveMailboxMailAfterCollect(
* mail. `driveMailboxMail` does not mark the parent busy until after an
* awaited collect, so overlapping flushes would each call `send()` and fill
* the agent's depth-16 queue. Hold one drive until that promise settles.
*
* `flush()` is true only when this call starts a drive. `claimed()` is true
* while that drive owns the slot — ask-wake admission uses it so a wake
* cannot send on the same stack, or on a later stack before collect lands.
*/
export type MailboxMailFlush = (() => boolean) & {
readonly claimed: () => boolean;
};

export function mailboxMailDriveClaimed(
flush: (() => boolean) | undefined,
): boolean {
if (flush === undefined) return false;
const claimed = (flush as MailboxMailFlush).claimed;
return typeof claimed === "function" && claimed() === true;
}

export function latchMailboxMailDrive(
drive: () => boolean | Promise<boolean>,
): () => boolean {
): MailboxMailFlush {
let inFlight = false;
return () => {
const flush = (): boolean => {
if (inFlight) return false;
const driven = drive();
if (driven === false) return false;
Expand All @@ -184,4 +226,5 @@ export function latchMailboxMailDrive(
});
return true;
};
return Object.assign(flush, { claimed: () => inFlight });
}
Loading
Loading