mirror of
https://github.com/badlogic/pi-mono.git
synced 2026-11-05 23:37:25 +00:00
fix(coding-agent): keep codemode cancellation prompt and bounded
Refs #10748
This commit is contained in:
parent
b9ab2a6238
commit
3daa20280f
11 changed files with 495 additions and 25 deletions
|
|
@ -6,6 +6,8 @@
|
|||
|
||||
- Pinned internal Pi dependencies to exact release versions to prevent fresh installs from mixing incompatible packages ([#10865](https://github.com/earendil-works/pi/issues/10865))
|
||||
- Fixed resumed sessions dropping tools activated during the session. Resume now restores the session's active tools, with `--tools +name`/`-name` entries applied to them, unless `--tools` with tool names, `--no-tools`, or `--no-builtin-tools` selects tools explicitly ([#10785](https://github.com/earendil-works/pi/issues/10785))
|
||||
- Fixed stopping a codemode script or reaching its deadline waiting for a non-interruptible built-in to return on Bun ([#10748](https://github.com/earendil-works/pi/issues/10748))
|
||||
- Fixed repeated codemode timeouts or aborts accumulating surviving workers; admission now limits live workers to two across sessions, counting cancelled workers until actual exit ([#10748](https://github.com/earendil-works/pi/issues/10748))
|
||||
|
||||
## [1.2.0] - 2026-10-12
|
||||
|
||||
|
|
|
|||
|
|
@ -224,5 +224,7 @@ for (const block of result.output) {
|
|||
## Limits
|
||||
|
||||
- A script's VM has 256 MB of memory. Running out throws `InternalError: out of memory`; filter or aggregate large data instead of accumulating it.
|
||||
- On Bun, stopping a script or reaching its deadline ends the call promptly, but a built-in that does not poll QuickJS's interrupt handler can keep computing in its worker until it returns.
|
||||
- At most two codemode workers may be alive across all sessions in one pi process. A cancelled worker occupies its slot until it exits. At capacity, new scripts fail immediately without starting another worker; retry after an existing worker exits.
|
||||
- A script that waits on a promise that can never settle (no tool call pending) fails immediately, since there are no timers.
|
||||
- Scripts cannot start other `codemode` scripts.
|
||||
|
|
|
|||
|
|
@ -54,6 +54,9 @@ const MAX_CONCURRENT_MODEL_CALLS = 4;
|
|||
* `InternalError: out of memory` inside the script.
|
||||
*/
|
||||
const CODEMODE_MEMORY_LIMIT_BYTES = 256 * 1024 * 1024;
|
||||
// Shared across calls and sessions. Allow one replacement while a cancelled worker is still exiting.
|
||||
const MAX_CONCURRENT_CODEMODE_WORKERS = 2;
|
||||
let activeCodemodeWorkers = 0;
|
||||
const MODEL_TYPES: ReadonlySet<string> = new Set<ModelType>(["chat", "image", "classifier"]);
|
||||
|
||||
function truncateText(text: string, maxChars: number): string {
|
||||
|
|
@ -508,6 +511,17 @@ export async function executeCodemode(
|
|||
memoryLimitBytes: CODEMODE_MEMORY_LIMIT_BYTES,
|
||||
wasm: loadQuickJSWasm(getQuickJSWasmPath()),
|
||||
workerUrl: getCodemodeWorkerSpecifier(),
|
||||
acquireWorker: () => {
|
||||
if (activeCodemodeWorkers >= MAX_CONCURRENT_CODEMODE_WORKERS) {
|
||||
throw new Error(
|
||||
`Codemode worker limit reached (${MAX_CONCURRENT_CODEMODE_WORKERS} workers are still running or shutting down). Retry after they exit.`,
|
||||
);
|
||||
}
|
||||
activeCodemodeWorkers++;
|
||||
return () => {
|
||||
activeCodemodeWorkers--;
|
||||
};
|
||||
},
|
||||
});
|
||||
|
||||
let result: CodemodeResult;
|
||||
|
|
|
|||
118
packages/coding-agent/test/codemode-worker-admission.test.ts
Normal file
118
packages/coding-agent/test/codemode-worker-admission.test.ts
Normal file
|
|
@ -0,0 +1,118 @@
|
|||
import { Worker } from "node:worker_threads";
|
||||
import * as codemode from "@earendil-works/pi-codemode";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { executeCodemode } from "../src/extensions/codemode/execute.ts";
|
||||
|
||||
const terminate = Worker.prototype.terminate;
|
||||
const terminations = new Map<Worker, (code: number) => void>();
|
||||
|
||||
beforeEach(() => {
|
||||
vi.spyOn(Worker.prototype, "terminate").mockImplementation(function (this: Worker) {
|
||||
this.ref();
|
||||
return new Promise<number>((resolve) => {
|
||||
terminations.set(this, resolve);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
vi.restoreAllMocks();
|
||||
await Promise.all(
|
||||
[...terminations].map(async ([worker, resolve]) => {
|
||||
resolve(await terminate.call(worker));
|
||||
}),
|
||||
);
|
||||
terminations.clear();
|
||||
});
|
||||
|
||||
const run = (code: string, signal?: AbortSignal) =>
|
||||
executeCodemode("worker-admission", { code }, signal, undefined, undefined);
|
||||
|
||||
describe("codemode worker admission", () => {
|
||||
// #10748: fresh sandboxes must share capacity and retain it until actual worker exit.
|
||||
it.each(["timeout", "abort", "success"] as const)("bounds surviving workers after %s", async (how) => {
|
||||
const once = vi.spyOn(Worker.prototype, "once");
|
||||
for (let i = 0; i < 2; i++) {
|
||||
const controller = new AbortController();
|
||||
const exitListenersBefore = once.mock.calls.filter(([event]) => event === "exit").length;
|
||||
const pending = run(
|
||||
how === "timeout"
|
||||
? '// @options: {"timeout_ms": 200}\nwhile (true) {}'
|
||||
: how === "abort"
|
||||
? "while (true) {}"
|
||||
: "return 1",
|
||||
controller.signal,
|
||||
);
|
||||
if (how === "abort") {
|
||||
await vi.waitFor(() =>
|
||||
expect(once.mock.calls.filter(([event]) => event === "exit").length).toBeGreaterThan(
|
||||
exitListenersBefore,
|
||||
),
|
||||
);
|
||||
controller.abort();
|
||||
}
|
||||
const result = await pending;
|
||||
if (how === "success") expect(result.isError).toBeUndefined();
|
||||
else {
|
||||
expect(result.isError).toBe(true);
|
||||
expect(result.content).toContainEqual({
|
||||
type: "text",
|
||||
text: expect.stringContaining(how === "timeout" ? "Script timed out:" : "Script aborted:"),
|
||||
});
|
||||
}
|
||||
expect(terminations.size).toBe(i + 1);
|
||||
}
|
||||
|
||||
const blocked = await run("return 3");
|
||||
expect(blocked).toMatchObject({ isError: true, details: { calls: [] } });
|
||||
expect(blocked.content).toContainEqual({
|
||||
type: "text",
|
||||
text: expect.stringContaining("Codemode worker limit reached (2 workers are still running or shutting down)"),
|
||||
});
|
||||
expect(terminations.size).toBe(2);
|
||||
|
||||
const aborted = await run("while (true) {}", AbortSignal.abort());
|
||||
expect(aborted.isError).toBe(true);
|
||||
expect(aborted.content).toContainEqual({ type: "text", text: expect.stringContaining("Script aborted:") });
|
||||
expect(terminations.size).toBe(2);
|
||||
|
||||
// Releasing one actual worker, not its result, permits exactly one new execution.
|
||||
const [worker, resolve] = [...terminations][0];
|
||||
resolve(await terminate.call(worker));
|
||||
terminations.delete(worker);
|
||||
expect((await run("return 42")).isError).toBeUndefined();
|
||||
expect(terminations.size).toBe(2);
|
||||
});
|
||||
|
||||
// #10748: timed-out launches resuming after WASM loading must obey the same limit.
|
||||
it("bounds workers that start after their results already timed out", async () => {
|
||||
const wasm = await codemode.loadQuickJSWasm();
|
||||
const loading = Promise.withResolvers<codemode.CodemodeWasmModule>();
|
||||
vi.spyOn(codemode, "loadQuickJSWasm").mockReturnValue(loading.promise);
|
||||
try {
|
||||
const results = await Promise.all(
|
||||
Array.from({ length: 3 }, () => run('// @options: {"timeout_ms": 20}\nwhile (true) {}')),
|
||||
);
|
||||
expect(results.every((result) => result.isError)).toBe(true);
|
||||
expect(terminations.size).toBe(0);
|
||||
loading.resolve(wasm);
|
||||
const blocked = await run("return 1");
|
||||
expect(blocked.isError).toBe(true);
|
||||
expect(blocked.content).toContainEqual({
|
||||
type: "text",
|
||||
text: expect.stringContaining("Codemode worker limit reached"),
|
||||
});
|
||||
expect(terminations.size).toBe(2);
|
||||
} finally {
|
||||
loading.resolve(wasm);
|
||||
}
|
||||
});
|
||||
|
||||
// #10748: concurrent callers must not race past the shared admission limit.
|
||||
it("admits only two workers from a concurrent burst", async () => {
|
||||
const results = await Promise.all(Array.from({ length: 6 }, () => run("return 1")));
|
||||
expect(results.filter((result) => !result.isError)).toHaveLength(2);
|
||||
expect(results.filter((result) => result.isError)).toHaveLength(4);
|
||||
expect(terminations.size).toBe(2);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue