Skip to content

Commit dbfe615

Browse files
Set session stopReason from typed ForcedStopReason only (#657)
stopReasonFromReport was a second source of truth that fabricated reasons from report prose. Thread the structured value from runSubAgent through SessionStore.complete instead.
1 parent 793255b commit dbfe615

8 files changed

Lines changed: 44 additions & 51 deletions

File tree

src/subagent/agent-fleet.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,7 @@ export function createSpawnAgentTool(deps: AgentFleetDeps): AgentTool {
481481
// just because retained:true was requested at spawn.
482482
deps.sessions.complete(session.id, result.report, {
483483
agentRetained: result.agentRetained === true,
484+
...(result.stopReason !== undefined ? { stopReason: result.stopReason } : {}),
484485
});
485486
})
486487
.catch((err) => {

src/subagent/fleet-report.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,13 +152,13 @@ describe("forced-stop reasons", () => {
152152
lane({
153153
id: "api",
154154
status: "done",
155-
stopReason: 'repetition — window "Groaning. " × 1363',
155+
stopReason: "stalled",
156156
}),
157157
lane({ id: "docs" }),
158158
],
159159
T0 + 1000,
160160
);
161-
expect(updates).toEqual(['api stopped — repetition — window "Groaning. " × 1363']);
161+
expect(updates).toEqual(["api stopped — stalled"]);
162162
});
163163

164164
test("a cancelled lane carries its recorded reason", () => {

src/subagent/index.test.ts

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import {
1515
forcedStopReport,
1616
formatSubAgentReport,
1717
parseSubAgentReport,
18-
stopReasonFromReport,
1918
appendSubAgentParentHints,
2019
createBriefDispatchLedger,
2120
fingerprintTaskBrief,
@@ -396,24 +395,21 @@ describe("sub-agent stop helpers", () => {
396395
).not.toContain("wall-clock deadline");
397396
});
398397

399-
test("forcedStopReport carries a machine-readable Stopped line the parent sees verbatim", () => {
400-
const cancelled = forcedStopReport("cancelled", "partial", "Session closed");
401-
expect(stopReasonFromReport(cancelled)).toBe("cancelled — Session closed");
402-
// Without a detail the line is the bare reason token.
403-
expect(stopReasonFromReport(forcedStopReport("cancelled", "partial"))).toBe("cancelled");
404-
expect(stopReasonFromReport(forcedStopReport("deadline", "x", "30s elapsed"))).toBe(
405-
"deadline — 30s elapsed",
398+
test("forcedStopReport renders a Stopped line for display; classification uses the typed reason", () => {
399+
expect(forcedStopReport("cancelled", "partial", "Session closed")).toMatch(
400+
/^Stopped: cancelled Session closed\n/,
406401
);
407-
408-
// A nested forced-stop quoted in Findings must not leak its Stopped line
409-
// as the outer report's reason.
402+
expect(forcedStopReport("cancelled", "partial")).toMatch(/^Stopped: cancelled\n/);
403+
expect(forcedStopReport("deadline", "x", "30s elapsed")).toMatch(
404+
/^Stopped: deadline 30s elapsed\n/,
405+
);
406+
// Nested Stopped: under Findings is display-only; classify via typed reason.
410407
const nested = forcedStopReport(
411408
"deadline",
412409
forcedStopReport("cancelled", "inner", "inner reason"),
413410
);
414-
expect(stopReasonFromReport(nested)).toBe("deadline");
415-
// A clean report has no Stopped line.
416-
expect(stopReasonFromReport("## Summary\nDone.\n\n## Findings\nx")).toBe(null);
411+
expect(nested).toMatch(/^Stopped: deadline\n/);
412+
expect(nested).toContain("Stopped: cancelled — inner reason");
417413
});
418414

419415
test("createSubAgentRunController aborts on an explicit deadline and reports deadlineHit", async () => {

src/subagent/index.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ export {
2929
formatSubAgentReport,
3030
hasReportEnvelope,
3131
parseSubAgentReport,
32-
stopReasonFromReport,
3332
subAgentToolName,
3433
type DispatchBrief,
3534
type SubAgentReport,

src/subagent/report.ts

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -122,31 +122,15 @@ export interface SubAgentReport {
122122
findings: string;
123123
blockers: string;
124124
paths: string;
125-
/**
126-
* Machine-readable termination reason for a forced stop (e.g.
127-
* `stalled — no output for 120s`). Rendered as a dedicated
128-
* `Stopped:` line above the envelope; absent on successful completes.
129-
*/
125+
/** Display-only `Stopped:` preamble for forcedStopReport; never parsed back. */
130126
stopped?: string;
131127
}
132128

133-
const STOPPED_LINE_RE = /^Stopped:\s*(.+)$/m;
134-
135-
/** Machine-readable stop reason from a report's `Stopped:` line, or null. */
136-
export function stopReasonFromReport(report: string): string | null {
137-
return parseSubAgentReport(report).stopped ?? null;
138-
}
139-
140129
export function parseSubAgentReport(reply: string): SubAgentReport {
141130
const text = reply.trim();
142131
const sections: Record<string, string> = {};
143132
const headingRe = /^##\s+(Summary|Findings|Blockers|Paths)\s*$/gim;
144133
const matches = [...text.matchAll(headingRe)];
145-
// Only the preamble (before the first heading) can carry the report's own
146-
// Stopped: line — a nested forced-stop report quoted under Findings must
147-
// not be read as this report's reason.
148-
const preamble = matches.length > 0 ? text.slice(0, matches[0]?.index ?? 0) : "";
149-
const stopped = STOPPED_LINE_RE.exec(preamble)?.[1]?.trim();
150134
if (matches.length === 0) {
151135
return {
152136
summary: text.length > 0 ? text : "Sub-agent finished without a textual result.",
@@ -167,7 +151,6 @@ export function parseSubAgentReport(reply: string): SubAgentReport {
167151
findings: sections.findings ?? "",
168152
blockers: sections.blockers ?? "",
169153
paths: sections.paths ?? "",
170-
...(stopped !== undefined && stopped.length > 0 ? { stopped } : {}),
171154
};
172155
}
173156

src/subagent/session-store.test.ts

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { describe, expect, test } from "bun:test";
22

33
import { createSubAgentSessionStore } from "./session-store.js";
4+
import { forcedStopReport } from "./stop-policy.js";
45

56
import type { ReactorEmittedEvent } from "@intx/inference";
67

@@ -261,16 +262,21 @@ describe("parallel tool calls", () => {
261262
});
262263

263264
describe("terminal stop reasons", () => {
264-
test("complete() records the report's Stopped line as stopReason", () => {
265+
test("complete() records the typed stopReason, not report prose", () => {
266+
const store = createSubAgentSessionStore();
267+
const session = store.start({ description: "d", agentId: "a", brief: "b" });
268+
store.complete(session.id, forcedStopReport("stalled", "partial"), { stopReason: "stalled" });
269+
expect(store.get(session.id)?.stopReason).toBe("stalled");
270+
});
271+
272+
test("literal Stopped: in report prose does not fabricate stopReason", () => {
265273
const store = createSubAgentSessionStore();
266274
const session = store.start({ description: "d", agentId: "a", brief: "b" });
267275
store.complete(
268276
session.id,
269-
'Stopped: repetition — window "Groaning. " × 1363\n\n## Summary\nStopped: degenerate repetition in streamed output (same window looping mid-turn).',
277+
'Stopped: repetition — window "Groaning. " × 1363\n\n## Summary\nStopped: looping.\n\n## Findings\nx',
270278
);
271-
const stored = store.get(session.id);
272-
expect(stored?.status).toBe("done");
273-
expect(stored?.stopReason).toBe('repetition — window "Groaning. " × 1363');
279+
expect(store.get(session.id)?.stopReason).toBeUndefined();
274280
});
275281

276282
test("a clean complete has no stopReason", () => {

src/subagent/session-store.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import type { ReactorEmittedEvent } from "@intx/inference";
88
import { DEFAULT_CLOSE_DEADLINE_MS } from "./dispose.js";
9-
import { stopReasonFromReport } from "./report.js";
9+
import type { ForcedStopReason } from "./stop-policy.js";
1010
import { toolCallPreview } from "./tool-preview.js";
1111

1212
export type SubAgentSessionStatus = "running" | "done" | "failed" | "cancelled";
@@ -79,9 +79,8 @@ export interface SubAgentSession {
7979
report?: string;
8080
error?: string;
8181
/**
82-
* Machine-readable termination reason for a forced stop (stall abort,
83-
* operator cancel) — the report's `Stopped:` line, or `cancelled — <reason>`
84-
* on cancel. Absent on clean completes.
82+
* Typed ForcedStopReason from runSubAgent, or `cancelled — <reason>` on
83+
* cancel(). Absent on clean completes. Never parsed from report prose.
8584
*/
8685
stopReason?: string;
8786
// Session id of the orchestrator that dispatched this worker, when this is
@@ -147,7 +146,11 @@ export interface SubAgentSessionStore {
147146
// deadline/cancel salvage resolves the same promise agent-fleet routes
148147
// here but always disposes its agent first, so omitting/false-ing this
149148
// keeps a disposed session from ever reporting as resumable.
150-
complete(id: string, report: string, opts?: { agentRetained?: boolean }): void;
149+
complete(
150+
id: string,
151+
report: string,
152+
opts?: { agentRetained?: boolean; stopReason?: ForcedStopReason },
153+
): void;
151154
fail(id: string, error: string): void;
152155
// Register the live abort handle for a running session so cancel() can stop
153156
// the child reactor (agent.close), not only flip status.
@@ -767,7 +770,11 @@ export function createSubAgentSessionStore(
767770
});
768771
},
769772

770-
complete(id: string, report: string, opts?: { agentRetained?: boolean }): void {
773+
complete(
774+
id: string,
775+
report: string,
776+
opts?: { agentRetained?: boolean; stopReason?: ForcedStopReason },
777+
): void {
771778
// CL-7001: run.ts always disposes on a salvage return (deadline/cancel)
772779
// even though it resolves through this same success path — only trust
773780
// "still open, resumable" when the caller says the agent genuinely
@@ -795,10 +802,7 @@ export function createSubAgentSessionStore(
795802
session.finishedAt = now();
796803
clearToolCalls(session);
797804
session.report = report;
798-
// A forced-stop salvage arrives via complete(); its Stopped: line is
799-
// the terminal reason (stall abort, etc).
800-
const stopped = stopReasonFromReport(report);
801-
if (stopped !== null) session.stopReason = stopped;
805+
if (opts?.stopReason !== undefined) session.stopReason = opts.stopReason;
802806
pushEntry(session, { kind: "report", content: capText(report, maxEntryChars) });
803807
// A disposed salvage has nothing left for its close handle to do —
804808
// release it now rather than leaving a stale reference around.

src/subagent/task-tool.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -840,7 +840,11 @@ export function createTaskTool(deps: TaskToolDeps): AgentTool {
840840
),
841841
);
842842
}
843-
if (session !== undefined) deps.sessions?.complete(session.id, result.report);
843+
if (session !== undefined)
844+
deps.sessions?.complete(session.id, result.report, {
845+
...(result.stopReason !== undefined ? { stopReason: result.stopReason } : {}),
846+
});
847+
844848
const reported = appendSubAgentParentHints(result.report, result.stopReason, hintOptions);
845849
return await finishWithWorktree(
846850
taskToolResult(

0 commit comments

Comments
 (0)