Skip to content

Commit 5800902

Browse files
ualtinokalfonso-magic-context
ualtinok
and
alfonso-magic-context
committed
mason: retry Pi subagents without discovered extensions
Co-authored-by: Alfonso [Magic Context] <288211368+alfonso-magic-context@users.noreply.github.com>
1 parent b374a36 commit 5800902

2 files changed

Lines changed: 452 additions & 18 deletions

File tree

‎packages/pi-plugin/src/subagent-runner.test.ts‎

Lines changed: 312 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
import { describe, expect, it, mock } from "bun:test";
1+
import { describe, expect, it, mock, spyOn } from "bun:test";
22
import { EventEmitter } from "node:events";
33
import { existsSync, readFileSync } from "node:fs";
44
import { isAbsolute } from "node:path";
55
import { PassThrough } from "node:stream";
6+
import * as loggerModule from "@magic-context/core/shared/logger";
67
import type { SubagentRunOptions } from "@magic-context/core/shared/subagent-runner";
78

89
import { __test, PiSubagentRunner } from "./subagent-runner";
@@ -13,6 +14,12 @@ const baseOptions: SubagentRunOptions = {
1314
userMessage: "summarize this session",
1415
};
1516
const TEST_SYSTEM_PROMPT_PATH = "/tmp/mc-pi-system-prompt.txt";
17+
const COLLISION_STDERR =
18+
"Agent is already processing. Specify streamingBehavior ('steer' or 'followUp') to queue the message.";
19+
const ISOLATED_RETRY_LOG_MESSAGE =
20+
"pi subagent: a loaded Pi extension started an agent turn before the child's prompt could run; retrying with an isolated extension set (user extensions disabled for this run)";
21+
const ISOLATED_RETRY_MODEL_UNAVAILABLE_LOG_MESSAGE =
22+
"model unavailable in isolated retry: it is provided by a disabled extension; configure it through models.json or add a built-in/provider-configured fallback";
1623

1724
type MockChild = ReturnType<typeof createMockChild>;
1825

@@ -98,16 +105,30 @@ function createMockChild({ stdout = true }: { stdout?: boolean } = {}) {
98105
}
99106

100107
function runnerWith(
101-
child: MockChild,
108+
childOrChildren: MockChild | MockChild[],
102109
{
103110
piBinary = "pi-test",
104111
platform,
105-
}: { piBinary?: string; platform?: NodeJS.Platform } = {},
112+
extraArgs,
113+
}: {
114+
piBinary?: string;
115+
platform?: NodeJS.Platform;
116+
extraArgs?: readonly string[];
117+
} = {},
106118
) {
107-
const spawnImpl = mock(() => child as never);
119+
const remainingChildren = Array.isArray(childOrChildren)
120+
? [...childOrChildren]
121+
: null;
122+
const spawnImpl = mock(() => {
123+
if (remainingChildren === null) return childOrChildren as never;
124+
const nextChild = remainingChildren.shift();
125+
if (!nextChild) throw new Error("unexpected extra spawn");
126+
return nextChild as never;
127+
});
108128
const runner = new PiSubagentRunner({
109129
piBinary,
110130
platform,
131+
extraArgs,
111132
spawnImpl: spawnImpl as never,
112133
});
113134
return { runner, spawnImpl };
@@ -132,6 +153,10 @@ function agentEnd(messages: unknown[]) {
132153
return { type: "agent_end", messages };
133154
}
134155

156+
function nextTick() {
157+
return new Promise((resolve) => setTimeout(resolve, 0));
158+
}
159+
135160
describe("subagent-runner pure helpers", () => {
136161
it("extracts the last assistant text and status from mixed messages", () => {
137162
const result = __test.extractFinalAssistant([
@@ -205,6 +230,28 @@ describe("subagent-runner pure helpers", () => {
205230
expect(args).toContain("--no-prompt-templates");
206231
});
207232

233+
it("isolated retry disables discovered extensions but keeps explicit --extension paths", () => {
234+
const args = buildArgsForTest(
235+
{
236+
...baseOptions,
237+
agent: "sidekick",
238+
model: "anthropic/claude-sonnet",
239+
},
240+
{
241+
disableDiscoveredExtensions: true,
242+
subagentEntryPath: "/tmp/subagent-entry.js",
243+
},
244+
);
245+
246+
expect(args).toEqual(
247+
expect.arrayContaining([
248+
"--no-extensions",
249+
"--extension",
250+
"/tmp/subagent-entry.js",
251+
]),
252+
);
253+
});
254+
208255
it("disables project context files so hidden subagents see only our prompt", () => {
209256
const args = buildArgsForTest({
210257
...baseOptions,
@@ -1161,6 +1208,267 @@ describe("PiSubagentRunner spawn lifecycle", () => {
11611208
}
11621209
});
11631210

1211+
it("retries once with --no-extensions after an extension turn collision", async () => {
1212+
const first = createMockChild();
1213+
const second = createMockChild();
1214+
const { runner, spawnImpl } = runnerWith([first, second]);
1215+
const logSpy = spyOn(loggerModule, "sessionLog").mockImplementation(
1216+
() => {},
1217+
);
1218+
1219+
try {
1220+
const resultPromise = runner.run({
1221+
...baseOptions,
1222+
model: "anthropic/claude-sonnet",
1223+
});
1224+
first.writeStderr(COLLISION_STDERR);
1225+
first.emitClose(1);
1226+
await nextTick();
1227+
second.writeStdoutLine(
1228+
agentEnd([
1229+
{
1230+
role: "assistant",
1231+
content: [{ type: "text", text: "isolated success" }],
1232+
stopReason: "stop",
1233+
},
1234+
]),
1235+
);
1236+
second.emitClose(0);
1237+
1238+
expect(await resultPromise).toEqual({
1239+
ok: true,
1240+
assistantText: "isolated success",
1241+
toolCallCount: 0,
1242+
durationMs: expect.any(Number),
1243+
meta: { stderr: undefined },
1244+
});
1245+
expect(spawnImpl).toHaveBeenCalledTimes(2);
1246+
expect(spawnImpl.mock.calls[0]?.[1]).not.toContain("--no-extensions");
1247+
expect(spawnImpl.mock.calls[1]?.[1]).toContain("--no-extensions");
1248+
expect(
1249+
logSpy.mock.calls.some(
1250+
(call) =>
1251+
call[0] === "pi-subagent" && call[1] === ISOLATED_RETRY_LOG_MESSAGE,
1252+
),
1253+
).toBe(true);
1254+
} finally {
1255+
logSpy.mockRestore();
1256+
}
1257+
});
1258+
1259+
it("does not retry forever when the isolated retry hits the same collision", async () => {
1260+
const first = createMockChild();
1261+
const second = createMockChild();
1262+
const { runner, spawnImpl } = runnerWith([first, second]);
1263+
1264+
const resultPromise = runner.run({
1265+
...baseOptions,
1266+
model: "anthropic/claude-sonnet",
1267+
});
1268+
first.writeStderr(COLLISION_STDERR);
1269+
first.emitClose(1);
1270+
await nextTick();
1271+
second.writeStderr(COLLISION_STDERR);
1272+
second.emitClose(1);
1273+
1274+
const result = await resultPromise;
1275+
expect(result.ok).toBe(false);
1276+
if (!result.ok) {
1277+
expect(result.reason).toBe("non_zero_exit");
1278+
expect(result.meta).toEqual({
1279+
stderr: COLLISION_STDERR,
1280+
exitCode: 1,
1281+
signal: null,
1282+
});
1283+
}
1284+
expect(spawnImpl).toHaveBeenCalledTimes(2);
1285+
expect(spawnImpl.mock.calls[1]?.[1]).toContain("--no-extensions");
1286+
});
1287+
1288+
it("does not insert an isolated retry for unrelated failures", async () => {
1289+
const first = createMockChild();
1290+
const second = createMockChild();
1291+
const { runner, spawnImpl } = runnerWith([first, second]);
1292+
1293+
const resultPromise = runner.run({
1294+
...baseOptions,
1295+
model: "anthropic/primary",
1296+
fallbackModels: ["openai/fallback"],
1297+
});
1298+
first.writeStderr("auth missing");
1299+
first.emitClose(1);
1300+
await nextTick();
1301+
second.writeStdoutLine(
1302+
agentEnd([
1303+
{
1304+
role: "assistant",
1305+
content: [{ type: "text", text: "fallback success" }],
1306+
stopReason: "stop",
1307+
},
1308+
]),
1309+
);
1310+
second.emitClose(0);
1311+
1312+
expect(await resultPromise).toEqual({
1313+
ok: true,
1314+
assistantText: "fallback success",
1315+
toolCallCount: 0,
1316+
durationMs: expect.any(Number),
1317+
meta: { stderr: undefined },
1318+
});
1319+
expect(spawnImpl).toHaveBeenCalledTimes(2);
1320+
expect(spawnImpl.mock.calls[0]?.[1]).not.toContain("--no-extensions");
1321+
expect(spawnImpl.mock.calls[1]?.[1]).not.toContain("--no-extensions");
1322+
expect(spawnImpl.mock.calls[1]?.[1]).toEqual(
1323+
expect.arrayContaining(["--model", "openai-codex/fallback"]),
1324+
);
1325+
});
1326+
1327+
it("does not start a retry loop when the spawn already disables extensions", async () => {
1328+
const first = createMockChild();
1329+
const second = createMockChild();
1330+
const { runner, spawnImpl } = runnerWith([first, second], {
1331+
extraArgs: ["--no-extensions"],
1332+
});
1333+
const logSpy = spyOn(loggerModule, "sessionLog").mockImplementation(
1334+
() => {},
1335+
);
1336+
1337+
try {
1338+
const resultPromise = runner.run({
1339+
...baseOptions,
1340+
model: "anthropic/primary",
1341+
fallbackModels: ["openai/fallback"],
1342+
});
1343+
first.writeStderr(COLLISION_STDERR);
1344+
first.emitClose(1);
1345+
await nextTick();
1346+
second.writeStdoutLine(
1347+
agentEnd([
1348+
{
1349+
role: "assistant",
1350+
content: [{ type: "text", text: "fallback without retry loop" }],
1351+
stopReason: "stop",
1352+
},
1353+
]),
1354+
);
1355+
second.emitClose(0);
1356+
1357+
expect(await resultPromise).toEqual({
1358+
ok: true,
1359+
assistantText: "fallback without retry loop",
1360+
toolCallCount: 0,
1361+
durationMs: expect.any(Number),
1362+
meta: { stderr: undefined },
1363+
});
1364+
expect(spawnImpl).toHaveBeenCalledTimes(2);
1365+
expect(spawnImpl.mock.calls[0]?.[1]).toContain("--no-extensions");
1366+
expect(spawnImpl.mock.calls[1]?.[1]).toContain("--no-extensions");
1367+
expect(
1368+
logSpy.mock.calls.some(
1369+
(call) => call[1] === ISOLATED_RETRY_LOG_MESSAGE,
1370+
),
1371+
).toBe(false);
1372+
} finally {
1373+
logSpy.mockRestore();
1374+
}
1375+
});
1376+
1377+
it("logs model-unavailable guidance when the isolated retry loses an extension-only model", async () => {
1378+
const first = createMockChild();
1379+
const second = createMockChild();
1380+
const { runner } = runnerWith([first, second]);
1381+
const logSpy = spyOn(loggerModule, "sessionLog").mockImplementation(
1382+
() => {},
1383+
);
1384+
1385+
try {
1386+
const resultPromise = runner.run({
1387+
...baseOptions,
1388+
model: "openai/extension-model",
1389+
});
1390+
first.writeStderr(COLLISION_STDERR);
1391+
first.emitClose(1);
1392+
await nextTick();
1393+
second.writeStderr("Unknown model openai-codex/extension-model");
1394+
second.emitClose(1);
1395+
1396+
const result = await resultPromise;
1397+
expect(result.ok).toBe(false);
1398+
if (!result.ok) {
1399+
expect(result.reason).toBe("non_zero_exit");
1400+
expect(result.error).toContain(
1401+
ISOLATED_RETRY_MODEL_UNAVAILABLE_LOG_MESSAGE,
1402+
);
1403+
expect(result.error).toContain("Original failure:");
1404+
}
1405+
expect(
1406+
logSpy.mock.calls.some(
1407+
(call) => call[1] === ISOLATED_RETRY_LOG_MESSAGE,
1408+
),
1409+
).toBe(true);
1410+
expect(
1411+
logSpy.mock.calls.some(
1412+
(call) => call[1] === ISOLATED_RETRY_MODEL_UNAVAILABLE_LOG_MESSAGE,
1413+
),
1414+
).toBe(true);
1415+
} finally {
1416+
logSpy.mockRestore();
1417+
}
1418+
});
1419+
1420+
it("does not keep isolated mode for the next run", async () => {
1421+
const first = createMockChild();
1422+
const second = createMockChild();
1423+
const third = createMockChild();
1424+
const { runner, spawnImpl } = runnerWith([first, second, third]);
1425+
1426+
const degradedRun = runner.run({
1427+
...baseOptions,
1428+
model: "anthropic/claude-sonnet",
1429+
});
1430+
first.writeStderr(COLLISION_STDERR);
1431+
first.emitClose(1);
1432+
await nextTick();
1433+
second.writeStdoutLine(
1434+
agentEnd([
1435+
{
1436+
role: "assistant",
1437+
content: [{ type: "text", text: "isolated success" }],
1438+
stopReason: "stop",
1439+
},
1440+
]),
1441+
);
1442+
second.emitClose(0);
1443+
await degradedRun;
1444+
1445+
const freshRun = runner.run({
1446+
...baseOptions,
1447+
model: "anthropic/claude-sonnet",
1448+
});
1449+
third.writeStdoutLine(
1450+
agentEnd([
1451+
{
1452+
role: "assistant",
1453+
content: [{ type: "text", text: "extensions restored" }],
1454+
stopReason: "stop",
1455+
},
1456+
]),
1457+
);
1458+
third.emitClose(0);
1459+
1460+
expect(await freshRun).toEqual({
1461+
ok: true,
1462+
assistantText: "extensions restored",
1463+
toolCallCount: 0,
1464+
durationMs: expect.any(Number),
1465+
meta: { stderr: undefined },
1466+
});
1467+
expect(spawnImpl.mock.calls[0]?.[1]).not.toContain("--no-extensions");
1468+
expect(spawnImpl.mock.calls[1]?.[1]).toContain("--no-extensions");
1469+
expect(spawnImpl.mock.calls[2]?.[1]).not.toContain("--no-extensions");
1470+
});
1471+
11641472
it("returns parse_failed when stdout is missing", async () => {
11651473
const child = createMockChild({ stdout: false });
11661474
const { runner } = runnerWith(child);

0 commit comments

Comments
 (0)