Skip to content

Commit 722a228

Browse files
committed
Match forced-stop Summary text exactly, not by substring
Salvage classifiers matched free-text Summary substrings ('no progress', 'cancelled', etc), so a SUCCESSFUL report whose Summary happened to contain those words was recorded as a forced stop, hard-blocking identical re-dispatch. forcedStopReport's summaries are now a single FORCED_STOP_SUMMARIES lookup shared by both the producer and the isXxxSubAgentReport classifiers, which compare against it with exact equality instead of .includes. A typed marker on the report would need a schema change (CL-6786, human-gated, out of scope) so this tightens to exact string matching instead.
1 parent 2541aab commit 722a228

3 files changed

Lines changed: 80 additions & 43 deletions

File tree

src/subagent/brief-dispatch.ts

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
/**
22
* Parent-side re-dispatch caps for task briefs (CL-4343 + CL-5203).
33
*
4-
* Leaf stops already salvage thrash / no-progress / turn-budget / etc. This
4+
* Leaf stops already salvage no-progress / turn-budget / etc. This
55
* module tracks how often the *parent* re-spawns the same brief so:
6-
* - thrash-class salvages hard-block an identical re-dispatch for the rest of
6+
* - hard-block-class salvages refuse an identical re-dispatch for the rest of
77
* the parent chat session (sticky until the fingerprint changes)
88
* - turn-budget salvage flips from "raise maxTurns" to "stop" after enough
99
* same-brief dispatches without a successful complete
@@ -12,9 +12,9 @@
1212
*/
1313

1414
import type { TaskIntent } from "./report.js";
15-
import { parseSubAgentReport } from "./report.js";
1615
import {
1716
isDeadlineSubAgentReport,
17+
isForcedStopSubAgentReport,
1818
isNeverActedSubAgentReport,
1919
isNeverEditedSubAgentReport,
2020
isNoProgressSubAgentReport,
@@ -66,20 +66,17 @@ export function isHardBlockSalvage(kind: BriefSalvageKind): kind is HardBlockSal
6666

6767
/** True when the worker returned a stall salvage report. */
6868
export function isStalledSubAgentReport(report: string): boolean {
69-
const parsed = parseSubAgentReport(report);
70-
return parsed.summary.toLowerCase().includes("long silence");
69+
return isForcedStopSubAgentReport(report, "stalled");
7170
}
7271

7372
/** True when the worker returned a cancel salvage report. */
7473
export function isCancelledSubAgentReport(report: string): boolean {
75-
const parsed = parseSubAgentReport(report);
76-
return parsed.summary.toLowerCase().includes("cancelled");
74+
return isForcedStopSubAgentReport(report, "cancelled");
7775
}
7876

7977
/** True when the worker returned an incomplete-report salvage (narration, no envelope). */
8078
export function isIncompleteReportSubAgentReport(report: string): boolean {
81-
const parsed = parseSubAgentReport(report);
82-
return parsed.summary.toLowerCase().includes("narrated instead of writing a report envelope");
79+
return isForcedStopSubAgentReport(report, "incomplete-report");
8380
}
8481

8582
/**

src/subagent/index.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2235,6 +2235,39 @@ describe("brief re-dispatch ledger (CL-4343 / CL-5203)", () => {
22352235
);
22362236
});
22372237

2238+
test("CL-6704: a successful Summary containing forced-stop phrases is not classified as a salvage", () => {
2239+
const noProgressPhrase = formatSubAgentReport({
2240+
summary: "Investigated the flaky test; root cause is a race, not no progress on our side.",
2241+
findings: "Fixed the race in retry logic.",
2242+
blockers: "None",
2243+
paths: "src/retry.ts",
2244+
});
2245+
expect(classifyBriefSalvage(noProgressPhrase)).toBeNull();
2246+
2247+
const cancelledPhrase = formatSubAgentReport({
2248+
summary: "Implemented the cancelled-order refund flow end to end.",
2249+
findings: "Added refund handler and tests.",
2250+
blockers: "None",
2251+
paths: "src/refunds.ts",
2252+
});
2253+
expect(classifyBriefSalvage(cancelledPhrase)).toBeNull();
2254+
2255+
const longSilencePhrase = formatSubAgentReport({
2256+
summary: "Reduced UI flicker with a long silence period before re-render.",
2257+
findings: "Debounced the re-render.",
2258+
blockers: "None",
2259+
paths: "src/ui.ts",
2260+
});
2261+
expect(classifyBriefSalvage(longSilencePhrase)).toBeNull();
2262+
});
2263+
2264+
test("CL-6704: true forced-stop Summary strings still classify as their salvage kind", () => {
2265+
expect(classifyBriefSalvage(forcedStopReport("no-progress", "x"))).toBe("no-progress");
2266+
expect(classifyBriefSalvage(forcedStopReport("cancelled", "x"))).toBe("cancelled");
2267+
expect(classifyBriefSalvage(forcedStopReport("stalled", "x"))).toBe("stalled");
2268+
expect(classifyBriefSalvage(forcedStopReport("deadline", "x"))).toBe("deadline");
2269+
});
2270+
22382271
test("turn-budget parent hint flips after re-dispatch threshold", () => {
22392272
const report = forcedStopReport("turn-budget", "partial");
22402273
const first = appendSubAgentParentHints(report, { dispatchCount: 1 });

src/subagent/stop-policy.ts

Lines changed: 41 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,28 @@ export type ForcedStopReason =
436436
| "repetition"
437437
| "incomplete-report";
438438

439+
// Exact Summary text for each forced-stop reason. This is the single source
440+
// of truth for both forcedStopReport (the producer) and the isXxxSubAgentReport
441+
// classifiers (the consumers) — CL-6704: classifying on a free-text substring
442+
// like "no progress" or "cancelled" hard-blocks a SUCCESSFUL report whose
443+
// Summary happens to contain that phrase. Matching the exact string a forced
444+
// stop actually produces closes that false-positive path without a report
445+
// schema change (a typed marker would need one; see CL-6786, out of scope).
446+
const FORCED_STOP_SUMMARIES: Record<ForcedStopReason, string> = {
447+
"no-progress": "Stopped: repeated the same tool calls with no progress.",
448+
"no-ship": "Stopped: implement intent searched many files without writing any.",
449+
"never-acted": "Stopped: completed without using any tools.",
450+
"never-edited": "Stopped: implement intent finished without writing any files.",
451+
cancelled: "Stopped: cancelled by operator before finishing.",
452+
deadline: "Stopped: wall-clock deadline reached before finishing.",
453+
stalled:
454+
"Stopped after a long silence with no tool activity. The parent can re-dispatch or check the background work directly.",
455+
repetition:
456+
"Stopped: degenerate repetition in streamed output (same window looping mid-turn).",
457+
"incomplete-report": "Stopped: worker narrated instead of writing a report envelope.",
458+
"turn-budget": "Turn budget reached before finishing.",
459+
};
460+
439461
/**
440462
* Build the parent-facing report when a leaf is force-stopped. There is no
441463
* further inference, so this must already be a full envelope — not an
@@ -449,26 +471,7 @@ export function forcedStopReport(
449471
partialText: string,
450472
detail?: string,
451473
): string {
452-
const summary =
453-
reason === "no-progress"
454-
? "Stopped: repeated the same tool calls with no progress."
455-
: reason === "no-ship"
456-
? "Stopped: implement intent searched many files without writing any."
457-
: reason === "never-acted"
458-
? "Stopped: completed without using any tools."
459-
: reason === "never-edited"
460-
? "Stopped: implement intent finished without writing any files."
461-
: reason === "cancelled"
462-
? "Stopped: cancelled by operator before finishing."
463-
: reason === "deadline"
464-
? "Stopped: wall-clock deadline reached before finishing."
465-
: reason === "stalled"
466-
? "Stopped after a long silence with no tool activity. The parent can re-dispatch or check the background work directly."
467-
: reason === "repetition"
468-
? "Stopped: degenerate repetition in streamed output (same window looping mid-turn)."
469-
: reason === "incomplete-report"
470-
? "Stopped: worker narrated instead of writing a report envelope."
471-
: "Turn budget reached before finishing.";
474+
const summary = FORCED_STOP_SUMMARIES[reason];
472475
const blockers =
473476
reason === "no-progress"
474477
? "Identical tool-call fingerprint repeated consecutively; parent must not re-dispatch the identical brief (it will be refused) — tighten success_criteria/do_not or change approach."
@@ -506,34 +509,40 @@ export function forcedStopReport(
506509
});
507510
}
508511

512+
/**
513+
* True when a report's Summary is exactly the forced-stop text for `reason`
514+
* (CL-6704: exact match, not a free-text substring — a successful report
515+
* whose Summary happens to mention the same words must not classify as a
516+
* forced stop).
517+
*/
518+
export function isForcedStopSubAgentReport(report: string, reason: ForcedStopReason): boolean {
519+
const parsed = parseSubAgentReport(report);
520+
return parsed.summary === FORCED_STOP_SUMMARIES[reason];
521+
}
522+
509523
/** True when the worker returned a turn-budget salvage report for the parent. */
510524
export function isTurnBudgetSubAgentReport(report: string): boolean {
511-
const parsed = parseSubAgentReport(report);
512-
return parsed.summary.includes("Turn budget reached");
525+
return isForcedStopSubAgentReport(report, "turn-budget");
513526
}
514527

515528
/** True when the worker returned a never-acted salvage report for the parent. */
516529
export function isNeverActedSubAgentReport(report: string): boolean {
517-
const parsed = parseSubAgentReport(report);
518-
return parsed.summary.includes("without using any tools");
530+
return isForcedStopSubAgentReport(report, "never-acted");
519531
}
520532

521533
/** True when implement intent finished without any write/edit tools. */
522534
export function isNeverEditedSubAgentReport(report: string): boolean {
523-
const parsed = parseSubAgentReport(report);
524-
return parsed.summary.includes("without writing any files");
535+
return isForcedStopSubAgentReport(report, "never-edited");
525536
}
526537

527538
/** True when the worker returned a deadline salvage report for the parent. */
528539
export function isDeadlineSubAgentReport(report: string): boolean {
529-
const parsed = parseSubAgentReport(report);
530-
return parsed.summary.includes("deadline reached");
540+
return isForcedStopSubAgentReport(report, "deadline");
531541
}
532542

533543
/** True when the worker returned a streamed-repetition salvage report. */
534544
export function isRepetitionSubAgentReport(report: string): boolean {
535-
const parsed = parseSubAgentReport(report);
536-
return parsed.summary.includes("degenerate repetition");
545+
return isForcedStopSubAgentReport(report, "repetition");
537546
}
538547

539548
const TURN_BUDGET_PARENT_HINT =
@@ -604,8 +613,7 @@ export function appendDeadlineParentHint(report: string): string {
604613

605614
/** True when the worker returned a no-ship (search-tour) salvage report. */
606615
export function isNoShipSubAgentReport(report: string): boolean {
607-
const parsed = parseSubAgentReport(report);
608-
return parsed.summary.includes("searched many files without writing");
616+
return isForcedStopSubAgentReport(report, "no-ship");
609617
}
610618

611619
export function appendNoShipParentHint(report: string): string {
@@ -620,8 +628,7 @@ export function appendRepetitionParentHint(report: string): string {
620628

621629
/** True when the worker returned a no-progress salvage report. */
622630
export function isNoProgressSubAgentReport(report: string): boolean {
623-
const parsed = parseSubAgentReport(report);
624-
return parsed.summary.includes("no progress");
631+
return isForcedStopSubAgentReport(report, "no-progress");
625632
}
626633

627634
export function appendNoProgressParentHint(report: string): string {

0 commit comments

Comments
 (0)