fix: release Pi snapshots on failed initialization
This commit is contained in:
@@ -37,6 +37,7 @@ type SpawnFn = (
|
|||||||
|
|
||||||
export class PiProcessManager {
|
export class PiProcessManager {
|
||||||
private runtimes = new Map<string, SessionRuntime>();
|
private runtimes = new Map<string, SessionRuntime>();
|
||||||
|
private agentSnapshotCleanups = new WeakMap<ChildProcessWithoutNullStreams, () => void>();
|
||||||
private spawnFn: (
|
private spawnFn: (
|
||||||
sessionId: string, author: string, provider: string | undefined, principal?: PrincipalContext,
|
sessionId: string, author: string, provider: string | undefined, principal?: PrincipalContext,
|
||||||
runtimeConfigPath?: string,
|
runtimeConfigPath?: string,
|
||||||
@@ -58,6 +59,13 @@ export class PiProcessManager {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private cleanupAgentSnapshot(child: ChildProcessWithoutNullStreams): void {
|
||||||
|
const cleanup = this.agentSnapshotCleanups.get(child);
|
||||||
|
if (!cleanup) return;
|
||||||
|
this.agentSnapshotCleanups.delete(child);
|
||||||
|
cleanup();
|
||||||
|
}
|
||||||
|
|
||||||
private spawnPi(
|
private spawnPi(
|
||||||
spawnFn: SpawnFn, sessionId: string, author: string, provider: string | undefined,
|
spawnFn: SpawnFn, sessionId: string, author: string, provider: string | undefined,
|
||||||
principal?: PrincipalContext, runtimeConfigPath?: string,
|
principal?: PrincipalContext, runtimeConfigPath?: string,
|
||||||
@@ -104,16 +112,19 @@ export class PiProcessManager {
|
|||||||
cwd: this.cfg.harnessDir,
|
cwd: this.cfg.harnessDir,
|
||||||
env,
|
env,
|
||||||
});
|
});
|
||||||
child.once("exit", agent.cleanup);
|
this.agentSnapshotCleanups.set(child, agent.cleanup);
|
||||||
child.once("close", agent.cleanup);
|
child.once("exit", () => this.cleanupAgentSnapshot(child!));
|
||||||
|
child.once("close", () => this.cleanupAgentSnapshot(child!));
|
||||||
// Log stderr for debugging (was silently drained)
|
// Log stderr for debugging (was silently drained)
|
||||||
child.stderr.on("data", (d: Buffer) => console.error(`[pi:${sessionId}] stderr:`, d.toString().trim()));
|
child.stderr.on("data", (d: Buffer) => console.error(`[pi:${sessionId}] stderr:`, d.toString().trim()));
|
||||||
return child;
|
return child;
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (child) {
|
if (child) {
|
||||||
try { child.kill(); } catch { /* preserve the initialization error */ }
|
try { child.kill(); } catch { /* preserve the initialization error */ }
|
||||||
}
|
this.cleanupAgentSnapshot(child);
|
||||||
|
} else {
|
||||||
agent.cleanup();
|
agent.cleanup();
|
||||||
|
}
|
||||||
throw error;
|
throw error;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -208,6 +219,7 @@ export class PiProcessManager {
|
|||||||
if (rt && this.runtimes.get(sessionId) === rt) this.runtimes.delete(sessionId);
|
if (rt && this.runtimes.get(sessionId) === rt) this.runtimes.delete(sessionId);
|
||||||
releaseRuntimeConfig();
|
releaseRuntimeConfig();
|
||||||
try { child.kill(); } catch { /* preserve the initialization error */ }
|
try { child.kill(); } catch { /* preserve the initialization error */ }
|
||||||
|
this.cleanupAgentSnapshot(child);
|
||||||
throw error;
|
throw error;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -276,6 +288,7 @@ export class PiProcessManager {
|
|||||||
this.runtimes.delete(id);
|
this.runtimes.delete(id);
|
||||||
expected.releaseRuntimeConfig?.();
|
expected.releaseRuntimeConfig?.();
|
||||||
expected.child.kill();
|
expected.child.kill();
|
||||||
|
this.cleanupAgentSnapshot(expected.child);
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -306,10 +306,18 @@ test("createFor kills a spawned child when post-spawn initialization throws", ()
|
|||||||
child.stdout = {
|
child.stdout = {
|
||||||
on: () => { throw new Error("READER_INIT_SENTINEL"); },
|
on: () => { throw new Error("READER_INIT_SENTINEL"); },
|
||||||
};
|
};
|
||||||
const mgr = new PiProcessManager(loadConfig({}), { spawnFn: () => child as any });
|
let snapshotDir: string | undefined;
|
||||||
|
const mgr = new PiProcessManager(loadConfig({}), {
|
||||||
|
spawnFn: (_command, _args, options) => {
|
||||||
|
snapshotDir = options.env.PI_CODING_AGENT_DIR;
|
||||||
|
return child as any;
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
expect(() => mgr.createFor("broken-init", {})).toThrow("READER_INIT_SENTINEL");
|
expect(() => mgr.createFor("broken-init", {})).toThrow("READER_INIT_SENTINEL");
|
||||||
|
|
||||||
|
expect(snapshotDir).toBeTruthy();
|
||||||
|
expect(existsSync(snapshotDir!)).toBe(false);
|
||||||
expect(child.kill).toHaveBeenCalledOnce();
|
expect(child.kill).toHaveBeenCalledOnce();
|
||||||
expect(mgr.get("broken-init")).toBeUndefined();
|
expect(mgr.get("broken-init")).toBeUndefined();
|
||||||
expect(mgr.count()).toBe(0);
|
expect(mgr.count()).toBe(0);
|
||||||
|
|||||||
Reference in New Issue
Block a user