fix: sub-agent hang/crash stranding runs mid-phase with no save
Some checks failed
port-to-omp / port (push) Failing after 2s
Some checks failed
port-to-omp / port (push) Failing after 2s
A stalled sub-agent (provider stream never settling after the final tool call, or a throwing session-event listener recursing through the SDK's run-failure path to stack overflow) left the phase stuck in_progress with no state save, no error, and no completion message — observed twice in freno-dev, both times after comments wrote findings.md. - agent-runner: 60-min settle watchdog on every agent phase (PYGIENIUM_AGENT_TIMEOUT_MS, env-tunable); timeout disposes the session and fails the phase loudly with a /pygienium-resume hint instead of hanging the check-runner's await forever. - agent-runner: applySessionEvent — the session.subscribe listener can no longer throw into the SDK event pipeline (guards for partial/malformed events, optional-chained message_update, throwing chat forwarder). - comments: scan no longer regenerates the full report as its final message (findings.md is the source of truth); fix phase reads findings.md with embedded fallback. - check-runner: failing state-save inside the catch path can't double- fault or escape as an unhandled rejection; completion posting is best-effort. - tests: watchdog timeout test, comments task-text regression tests, applySessionEvent malformed-shape unit tests. 146 pass, tsc clean. - README: document PYGIENIUM_AGENT_TIMEOUT_MS.
This commit is contained in:
137
tests/agent-runner.test.ts
Normal file
137
tests/agent-runner.test.ts
Normal file
@@ -0,0 +1,137 @@
|
||||
/**
|
||||
* agent-runner.test.ts — unit tests for the session event accumulator.
|
||||
*
|
||||
* `applySessionEvent` runs synchronously inside the SDK's event pipeline; a
|
||||
* throw there crashes the host (the SDK's run-failure path re-emits events
|
||||
* through the same callback → stack overflow), stranding run-state at the
|
||||
* phase boundary. These tests pin the handler to never throw on the partial /
|
||||
* malformed event shapes streaming sessions actually emit.
|
||||
*/
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import {
|
||||
applySessionEvent,
|
||||
type SessionEventAccumulator,
|
||||
} from "../src/agent-runner.js";
|
||||
import type { AgentSessionEvent } from "@earendil-works/pi-coding-agent";
|
||||
|
||||
function fresh(): SessionEventAccumulator {
|
||||
return { text: "" };
|
||||
}
|
||||
|
||||
/** Build a typed-as-unknown event so malformed shapes compile in tests. */
|
||||
function event(shape: unknown): AgentSessionEvent {
|
||||
return shape as AgentSessionEvent;
|
||||
}
|
||||
|
||||
describe("applySessionEvent", () => {
|
||||
it("accumulates text_delta stream events in order", () => {
|
||||
const acc = fresh();
|
||||
applySessionEvent(
|
||||
acc,
|
||||
event({
|
||||
type: "message_update",
|
||||
assistantMessageEvent: { type: "text_delta", delta: "foo" },
|
||||
}),
|
||||
);
|
||||
applySessionEvent(
|
||||
acc,
|
||||
event({
|
||||
type: "message_update",
|
||||
assistantMessageEvent: { type: "text_delta", delta: "bar" },
|
||||
}),
|
||||
);
|
||||
expect(acc.text).toBe("foobar");
|
||||
});
|
||||
|
||||
it("does not throw on a message_update with no assistantMessageEvent", () => {
|
||||
const acc = fresh();
|
||||
expect(() =>
|
||||
applySessionEvent(acc, event({ type: "message_update" })),
|
||||
).not.toThrow();
|
||||
expect(acc.text).toBe("");
|
||||
});
|
||||
|
||||
it("does not throw on a message_update with an unknown event shape", () => {
|
||||
const acc = fresh();
|
||||
expect(() => applySessionEvent(acc, event({ type: "bogus_event" }))).not.toThrow();
|
||||
expect(() => applySessionEvent(acc, event(null))).not.toThrow();
|
||||
expect(() => applySessionEvent(acc, event(undefined))).not.toThrow();
|
||||
expect(acc.text).toBe("");
|
||||
});
|
||||
|
||||
it("captures full text + stopReason from message_end when nothing streamed", () => {
|
||||
const acc = fresh();
|
||||
applySessionEvent(
|
||||
acc,
|
||||
event({
|
||||
type: "message_end",
|
||||
message: {
|
||||
role: "assistant",
|
||||
stopReason: "stop",
|
||||
content: [{ type: "text", text: "full report" }],
|
||||
},
|
||||
}),
|
||||
);
|
||||
expect(acc.text).toBe("full report");
|
||||
expect(acc.stopReason).toBe("stop");
|
||||
});
|
||||
|
||||
it("does not throw on a message_end with no message", () => {
|
||||
const acc = fresh();
|
||||
expect(() => applySessionEvent(acc, event({ type: "message_end" }))).not.toThrow();
|
||||
expect(acc.text).toBe("");
|
||||
});
|
||||
|
||||
it("records an errorMessage surfaced on the final message", () => {
|
||||
const acc = fresh();
|
||||
applySessionEvent(
|
||||
acc,
|
||||
event({
|
||||
type: "message_end",
|
||||
message: { role: "assistant", errorMessage: "upstream 529" },
|
||||
}),
|
||||
);
|
||||
expect(acc.errorMessage).toBe("upstream 529");
|
||||
});
|
||||
|
||||
it("forwards stream-driving events and swallows a throwing forwarder", () => {
|
||||
const originalError = console.error;
|
||||
console.error = () => {};
|
||||
try {
|
||||
const forwarded: string[] = [];
|
||||
const forward = (ev: AgentSessionEvent) => {
|
||||
forwarded.push(ev.type);
|
||||
if (ev.type === "tool_execution_end") throw new Error("renderer boom");
|
||||
};
|
||||
const acc = fresh();
|
||||
applySessionEvent(
|
||||
acc,
|
||||
event({ type: "tool_execution_start", toolName: "bash" }),
|
||||
forward,
|
||||
);
|
||||
applySessionEvent(
|
||||
acc,
|
||||
event({ type: "tool_execution_end", toolName: "bash" }),
|
||||
forward,
|
||||
);
|
||||
expect(forwarded).toEqual(["tool_execution_start", "tool_execution_end"]);
|
||||
} finally {
|
||||
console.error = originalError;
|
||||
}
|
||||
});
|
||||
|
||||
it("does not forward non-stream-driving events", () => {
|
||||
const forwarded: string[] = [];
|
||||
const acc = fresh();
|
||||
applySessionEvent(
|
||||
acc,
|
||||
event({ type: "message_update", assistantMessageEvent: { type: "text_delta", delta: "x" } }),
|
||||
(ev) => forwarded.push(ev.type),
|
||||
);
|
||||
applySessionEvent(acc, event({ type: "agent_settled" }), (ev) =>
|
||||
forwarded.push(ev.type),
|
||||
);
|
||||
expect(forwarded).toEqual([]);
|
||||
expect(acc.text).toBe("x");
|
||||
});
|
||||
});
|
||||
@@ -20,6 +20,8 @@ import {
|
||||
setAgentRunner,
|
||||
resetAgentRunner,
|
||||
fakeAgentRunner,
|
||||
AGENT_TIMEOUT_ENV,
|
||||
type AgentRunResult,
|
||||
} from "../src/agent-runner.js";
|
||||
import { handleCheckCommand, type PygieniumCtx } from "../src/commands.js";
|
||||
import { loadRunState, runStatePath } from "../src/run-state.js";
|
||||
@@ -128,4 +130,29 @@ describe("check-runner integration", () => {
|
||||
expect(state?.checks.gated.status).toBe("skipped");
|
||||
expect(state?.checks.gated.error).toBe("no source files matched");
|
||||
});
|
||||
|
||||
it("fails the check loudly when the agent never settles", async () => {
|
||||
// Models the silent hang: the sub-agent session never resolves after
|
||||
// its last tool call (stalled provider stream / hung retry), leaving
|
||||
// the run stuck mid-phase with no state save and no completion. The
|
||||
// watchdog must abort the phase with a visible error instead.
|
||||
setAgentRunner(() => new Promise<AgentRunResult>(() => {}));
|
||||
const prev = process.env[AGENT_TIMEOUT_ENV];
|
||||
process.env[AGENT_TIMEOUT_ENV] = "50";
|
||||
try {
|
||||
await handleCheckCommand(smokeCheck(), "", stubCtx(cwd));
|
||||
} finally {
|
||||
if (prev === undefined) delete process.env[AGENT_TIMEOUT_ENV];
|
||||
else process.env[AGENT_TIMEOUT_ENV] = prev;
|
||||
}
|
||||
|
||||
const state = await loadRunState(cwd);
|
||||
expect(state?.checks.smoke.status).toBe("failed");
|
||||
expect(state?.checks.smoke.error).toContain("did not settle within");
|
||||
expect(state?.checks.smoke.error).toContain("/pygienium-resume");
|
||||
expect(
|
||||
state?.checks.smoke.phases.find((p) => p.id === "analysis")?.status,
|
||||
).toBe("failed");
|
||||
expect(state?.status).toBe("failed");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -30,6 +30,8 @@ import {
|
||||
commentsCheck,
|
||||
findingsPath,
|
||||
changesPath,
|
||||
buildCommentsScanTask,
|
||||
buildCommentsFixTask,
|
||||
} from "../src/checks/comments.js";
|
||||
import type { CheckScope } from "../src/checks/registry.js";
|
||||
|
||||
@@ -256,4 +258,31 @@ describe("comments check (end-to-end)", () => {
|
||||
expect(check!.findings).toBeDefined();
|
||||
expect(check!.changes).toBeDefined();
|
||||
});
|
||||
|
||||
it("scan task keeps the full report in findings.md, not the final message", () => {
|
||||
// Regression: the scan task used to demand the agent regenerate the
|
||||
// whole report as its final message right after writing findings.md —
|
||||
// a second huge output that stalled the phase transition (observed in
|
||||
// freno-dev twice). The final message must stay a one-line summary.
|
||||
const scope: CheckScope = {
|
||||
cwd,
|
||||
target,
|
||||
fix: false,
|
||||
rest: [],
|
||||
};
|
||||
const task = buildCommentsScanTask(cwd, scope);
|
||||
expect(task).toContain("ONE-LINE summary");
|
||||
expect(task).toContain("do NOT regenerate the report text");
|
||||
expect(task).not.toContain(
|
||||
"Return the findings report text as your final message",
|
||||
);
|
||||
expect(task).not.toContain("same content as the file");
|
||||
|
||||
// The fix phase must read the detailed findings from the artifact so a
|
||||
// one-line scan summary can't starve it.
|
||||
const fixTask = buildCommentsFixTask(cwd, scope, "fallback-findings-text");
|
||||
expect(fixTask).toContain("Read the detailed per-file findings");
|
||||
expect(fixTask).toContain(findingsPath(scope));
|
||||
expect(fixTask).toContain("fallback-findings-text");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user