Skip to content

Commit ce7ac19

Browse files
committed
Delete never-edited/never-acted/no-ship/no-progress salvage classes
Two peer agent loops have no concept of a stop that bars retry, and neither treats a tool-using run with no net file diff as a failure. This project had five: no-ship, no-progress, never-acted, never-edited, and repetition all hard-blocked identical re-dispatch for the rest of the session. A worker sharing a directory can issue real edits that a concurrent lane absorbs, leaving no net diff — the never-edited class read that as "did nothing" and discarded the worker's real work while also blocking retry. Deletes HARD_BLOCK_SALVAGES/isHardBlockSalvage and the fingerprint-refusal logic in brief-dispatch.ts (the ledger now always admits), the requireEdit branch and the consecutive-identical no-progress check in stop-policy.ts (with their bookkeeping: DEFAULT_SUBAGENT_REPEAT_LIMIT, ToolCallStreak, nextToolCallStreak, subAgentNoProgress), and the four now-false "it will be refused" parent hints. shell-evidence.ts's write-detection existed solely to feed requireEdit and is now dead; shrunk to reads-only (still feeds the CritiqueDirector requireEvidence gate). editedPaths bookkeeping in thrash.ts stays for intervention-log diagnostics only. repetition keeps its detector (a sibling change owns deleting that) but loses its hard-block membership here, matching every other salvage class: it ends the run and reports the loop, but does not refuse a later re-dispatch.
1 parent 6c05612 commit ce7ac19

16 files changed

Lines changed: 179 additions & 801 deletions

CHANGELOG.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,18 @@ matching `## [X.Y.Z]` section (plus install instructions). Do not maintain
1111
parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
1212
`## [Unreleased]` to `## [X.Y.Z] - YYYY-MM-DD`, then run the release script.
1313

14+
## [Unreleased]
15+
16+
### Agent
17+
18+
- Removed the `never-edited`, `never-acted`, `no-ship`, and `no-progress` leaf
19+
salvage classes and the sticky hard-block that refused an identical
20+
re-dispatch after any of them fired. A worker sharing a directory can issue
21+
real edits that a concurrent writer absorbs, leaving no net diff — that is
22+
not a failure, and no salvage class now treats it as one. `turn-budget`,
23+
`deadline`, `stalled`, `cancelled`, `incomplete-report`, and `repetition`
24+
are unaffected.
25+
1426
## [0.2.108] - 2026-08-24
1527

1628
### Agent

src/agent/director.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -840,13 +840,13 @@ describe("ChatDirector tool-only loop protection", () => {
840840
expect(actions.some((a) => a.type === "infer")).toBe(true);
841841
});
842842

843-
test("after a hard-block salvage, Skywalker is nudged once and unique reads do not pause", async () => {
843+
test("after a repetition salvage, Skywalker is nudged once and unique reads do not pause", async () => {
844844
const director = createChatDirector("system", [], {
845845
onTasksChange: () => {},
846846
provider: providerlessPolicy,
847847
});
848848
const capabilities = makeCapabilities();
849-
const salvage = forcedStopReport("no-ship", "mapped the tree, never edited");
849+
const salvage = forcedStopReport("repetition", "looped mid-stream");
850850
await director.decide(
851851
{
852852
type: "inference.done",
@@ -1071,7 +1071,7 @@ describe("ChatDirector tool-only loop protection", () => {
10711071
await director.decide(taskTurn(id), mockState, capabilities);
10721072
const result = actionsArray(
10731073
await director.decide(
1074-
taskDoneEvent(id, { content: forcedStopReport("no-progress", "x") }),
1074+
taskDoneEvent(id, { content: forcedStopReport("turn-budget", "x") }),
10751075
mockState,
10761076
capabilities,
10771077
),

src/agent/director.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ import {
3131
} from "../subagent/stop-policy.js";
3232
import { PRESENT_VIEW_PRIMITIVES_GUIDANCE } from "./tool-schema-normalize.js";
3333
import { isOperatorOriginated } from "./message-provenance.js";
34-
import { classifyBriefSalvage, isHardBlockSalvage } from "../subagent/brief-dispatch.js";
34+
import { classifyBriefSalvage } from "../subagent/brief-dispatch.js";
3535
import { PRIMARY_SALVAGE_NUDGE } from "./look-tour.js";
3636

3737
// Fired when turnsSinceUserMessage reaches TURNS_SINCE_USER_MESSAGE_BACKSTOP.
@@ -935,7 +935,7 @@ class ChatDirectorImpl extends DefaultDirector {
935935
this.pendingTaskCallIds.delete(event.result.callId);
936936
const body = typeof event.result.content === "string" ? event.result.content : "";
937937
const salvage = classifyBriefSalvage(body);
938-
if (salvage !== null && isHardBlockSalvage(salvage) && !this.salvageNudgeFired) {
938+
if (salvage === "repetition" && !this.salvageNudgeFired) {
939939
this.salvageNudgeFired = true;
940940
this.pendingSalvageNudge = PRIMARY_SALVAGE_NUDGE;
941941
}

src/agent/prompts.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ export function buildGuidelines(
153153
"- Prefer the typed spawn contract on every worker: `intent`, `success_criteria` (done-when), `do_not` (scope fence), and `report_focus` so workers finish instead of thrashing. Free-form `prompt` alone is weaker.",
154154
"- After workers return, merge their Summary/Findings into a coherent answer for the operator; do not paste raw sub-agent dumps.",
155155
"- Pass `maxTurns` on `task` when a job needs a larger inference budget (default 30, no hard upper cap). On turn-budget salvage, re-dispatch with continuation context and a higher maxTurns only a few times on the same brief — after the re-dispatch cap, change approach instead of bumping turns again.",
156-
"- After thrash / no-progress / repetition / never-acted salvage, do not re-dispatch an identical brief (prompt/agent/intent/success_criteria/do_not) — it is refused. Change the brief to force a re-run; maxTurns alone does not unlock it.",
156+
"- After a repetition salvage, re-dispatching an identical brief (prompt/agent/intent/success_criteria/do_not) unchanged will likely loop again — change the brief before retrying.",
157157
"- Use manage_tasks for your own coordination checklist; spawning workers is `task`, not manage_tasks.",
158158
"- If context is compacted automatically, do not stop tasks early due to token fear; persist progress via manage_tasks and worker reports.",
159159
]),

src/subagent/brief-dispatch.ts

Lines changed: 14 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
11
/**
2-
* Parent-side re-dispatch caps for task briefs (CL-4343 + CL-5203).
2+
* Parent-side re-dispatch tracking for task briefs (CL-4343 + CL-5203).
33
*
4-
* Leaf stops already salvage no-progress / turn-budget / etc. This
5-
* module tracks how often the *parent* re-spawns the same brief so:
6-
* - hard-block-class salvages refuse an identical re-dispatch for the rest of
7-
* the parent chat session (sticky until the fingerprint changes)
8-
* - turn-budget salvage flips from "raise maxTurns" to "stop" after enough
9-
* same-brief dispatches without a successful complete
4+
* Leaf stops already salvage turn-budget / deadline / etc. This module
5+
* tracks how often the *parent* re-spawns the same brief so turn-budget
6+
* salvage flips from "raise maxTurns" to "stop" after enough same-brief
7+
* dispatches without a successful complete. No salvage class refuses
8+
* re-dispatch (CL-6994) — every dispatch is admitted.
109
*
1110
* Session-scoped: one ledger per createTaskTool instance (parent chat tool).
1211
*/
@@ -15,20 +14,12 @@ import type { TaskIntent } from "./report.js";
1514
import {
1615
isDeadlineSubAgentReport,
1716
isForcedStopSubAgentReport,
18-
isNeverActedSubAgentReport,
19-
isNeverEditedSubAgentReport,
20-
isNoProgressSubAgentReport,
21-
isNoShipSubAgentReport,
2217
isRepetitionSubAgentReport,
2318
isTurnBudgetSubAgentReport,
2419
} from "./stop-policy.js";
2520

26-
/** Salvage classes that must not be re-dispatched with an identical brief. */
27-
export type HardBlockSalvage =
28-
"no-ship" | "no-progress" | "repetition" | "never-acted" | "never-edited";
29-
3021
export type BriefSalvageKind =
31-
HardBlockSalvage | "turn-budget" | "deadline" | "stalled" | "cancelled" | "incomplete-report";
22+
"turn-budget" | "deadline" | "stalled" | "cancelled" | "incomplete-report" | "repetition";
3223

3324
export interface TaskBriefFingerprintInput {
3425
prompt: string;
@@ -41,8 +32,6 @@ export interface TaskBriefFingerprintInput {
4132
export interface BriefDispatchRecord {
4233
/** How many times this fingerprint has been accepted for run (including first). */
4334
dispatchCount: number;
44-
/** Last salvage class observed for this fingerprint, if any. */
45-
lastSalvage?: BriefSalvageKind;
4635
}
4736

4837
/**
@@ -52,18 +41,6 @@ export interface BriefDispatchRecord {
5241
*/
5342
export const TURN_BUDGET_STOP_AFTER_DISPATCHES = 3;
5443

55-
const HARD_BLOCK_SALVAGES = new Set<BriefSalvageKind>([
56-
"no-ship",
57-
"no-progress",
58-
"repetition",
59-
"never-acted",
60-
"never-edited",
61-
]);
62-
63-
export function isHardBlockSalvage(kind: BriefSalvageKind): kind is HardBlockSalvage {
64-
return HARD_BLOCK_SALVAGES.has(kind);
65-
}
66-
6744
/** True when the worker returned a stall salvage report. */
6845
export function isStalledSubAgentReport(report: string): boolean {
6946
return isForcedStopSubAgentReport(report, "stalled");
@@ -85,11 +62,7 @@ export function isIncompleteReportSubAgentReport(report: string): boolean {
8562
*/
8663
export function classifyBriefSalvage(report: string): BriefSalvageKind | null {
8764
// Order: more specific salvage phrases first.
88-
if (isNoShipSubAgentReport(report)) return "no-ship";
8965
if (isRepetitionSubAgentReport(report)) return "repetition";
90-
if (isNeverEditedSubAgentReport(report)) return "never-edited";
91-
if (isNeverActedSubAgentReport(report)) return "never-acted";
92-
if (isNoProgressSubAgentReport(report)) return "no-progress";
9366
if (isTurnBudgetSubAgentReport(report)) return "turn-budget";
9467
if (isDeadlineSubAgentReport(report)) return "deadline";
9568
if (isStalledSubAgentReport(report)) return "stalled";
@@ -127,13 +100,8 @@ function serializeList(items: readonly string[] | undefined): string {
127100

128101
export interface BriefDispatchLedger {
129102
get: (fingerprint: string) => BriefDispatchRecord | undefined;
130-
/**
131-
* Pre-run gate. Returns ok with the 1-based dispatch count that will be used,
132-
* or a reject message for the parent tool result.
133-
*/
134-
admit: (
135-
fingerprint: string,
136-
) => { ok: true; dispatchCount: number } | { ok: false; message: string };
103+
/** Pre-run gate. Always admits; returns the 1-based dispatch count that will be used. */
104+
admit: (fingerprint: string) => { ok: true; dispatchCount: number };
137105
/** Record the outcome of an admitted run (salvage kind or null on success). */
138106
recordOutcome: (fingerprint: string, salvage: BriefSalvageKind | null) => void;
139107
/**
@@ -153,77 +121,31 @@ export function createBriefDispatchLedger(): BriefDispatchLedger {
153121

154122
admit(fingerprint) {
155123
const existing = byFingerprint.get(fingerprint);
156-
if (existing?.lastSalvage !== undefined && isHardBlockSalvage(existing.lastSalvage)) {
157-
return {
158-
ok: false,
159-
message: hardBlockMessage(existing.lastSalvage, existing.dispatchCount),
160-
};
161-
}
162124
const nextCount = (existing?.dispatchCount ?? 0) + 1;
163-
byFingerprint.set(fingerprint, {
164-
dispatchCount: nextCount,
165-
...(existing?.lastSalvage !== undefined ? { lastSalvage: existing.lastSalvage } : {}),
166-
});
125+
byFingerprint.set(fingerprint, { dispatchCount: nextCount });
167126
return { ok: true, dispatchCount: nextCount };
168127
},
169128

170129
recordOutcome(fingerprint, salvage) {
171-
const existing = byFingerprint.get(fingerprint);
172-
if (existing === undefined) {
173-
// admit() always runs first in production; keep defensive for unit tests.
174-
byFingerprint.set(fingerprint, {
175-
dispatchCount: salvage === null ? 0 : 1,
176-
...(salvage !== null ? { lastSalvage: salvage } : {}),
177-
});
178-
return;
179-
}
130+
// A successful complete resets the same-brief retry budget. Any other
131+
// salvage leaves dispatchCount as admit() already recorded it.
180132
if (salvage === null) {
181-
// CL-6710: a successful complete clears the sticky hard-block too.
182-
// Two concurrent identical-brief dispatches can both admit; if one
183-
// salvages and the other succeeds, the success proves the brief is
184-
// re-dispatchable, so it must not leave the sibling's hard-block
185-
// standing for the rest of the session.
186133
byFingerprint.set(fingerprint, { dispatchCount: 0 });
187-
return;
188134
}
189-
byFingerprint.set(fingerprint, {
190-
dispatchCount: existing.dispatchCount,
191-
lastSalvage: salvage,
192-
});
193135
},
194136

195137
release(fingerprint) {
196138
const existing = byFingerprint.get(fingerprint);
197139
if (existing === undefined) return;
198140
if (existing.dispatchCount <= 1) {
199-
if (existing.lastSalvage !== undefined) {
200-
byFingerprint.set(fingerprint, {
201-
dispatchCount: 0,
202-
lastSalvage: existing.lastSalvage,
203-
});
204-
} else {
205-
byFingerprint.delete(fingerprint);
206-
}
141+
byFingerprint.delete(fingerprint);
207142
return;
208143
}
209-
byFingerprint.set(fingerprint, {
210-
dispatchCount: existing.dispatchCount - 1,
211-
...(existing.lastSalvage !== undefined ? { lastSalvage: existing.lastSalvage } : {}),
212-
});
144+
byFingerprint.set(fingerprint, { dispatchCount: existing.dispatchCount - 1 });
213145
},
214146
};
215147
}
216148

217-
function hardBlockMessage(salvage: HardBlockSalvage, priorDispatches: number): string {
218-
return (
219-
`Error: refused re-dispatch of an identical task brief after a ${salvage} salvage ` +
220-
`(already dispatched ${priorDispatches} time${priorDispatches === 1 ? "" : "s"}). ` +
221-
`Change the brief (prompt, agent, intent, success_criteria, and/or do_not) before retrying — ` +
222-
`raising maxTurns alone will not unlock this fingerprint. ` +
223-
`To force a re-run of the same work, alter at least one of those fields so the fingerprint changes.`
224-
);
225-
}
226-
227149
/**
228150
* Whether turn-budget parent hint should recommend stopping rather than
229151
* re-dispatching with a higher maxTurns.

0 commit comments

Comments
 (0)