Skip to content

Commit 80ae487

Browse files
Delete hard_block_salvages and the never-edited/never-acted/no-ship policy salvage classes (CL-6994) (#613)
Re-apply on current main: task re-dispatch is never refused now (admit() always succeeds); turn-budget salvage still throttles same-brief retries. Removes the requireEdit plumbing, the shell-write half of shell-evidence.ts (read detection for requireEvidence stays), the shell-write contribution to editedPaths, and the now-dead hard-block salvage nudge in the parent director (look-tour.ts).
1 parent 70d497d commit 80ae487

17 files changed

Lines changed: 111 additions & 622 deletions

CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,14 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
2626
reports directly, not by re-parsing the parent-facing report's prose.
2727
Removes the `isXxxSubAgentReport` classifier family and per-reason parent
2828
hint functions in favor of a single structured switch.
29+
- Removed the `never-edited`, `never-acted`, and `no-ship` sub-agent salvage
30+
classes and the sticky hard-block that refused an identical re-dispatch
31+
after one fired. `task` re-dispatch is never refused now; turn-budget
32+
salvage still throttles repeated same-brief retries. Also removed the
33+
now-dead shell-write half of the shell-evidence detector (read detection
34+
for `requireEvidence` is unchanged) and the shell-write contribution to
35+
`editedPaths` diagnostics.
36+
2937
### Internal
3038

3139
- Removed the dead Ink-era kill ring copy (`src/tui/kill-ring.ts`); the OpenTUI

src/agent/director.ts

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,6 @@ import { isInternalRecoveryAbortRaw } from "../inference-abort.js";
2121
import { LOG_NAMESPACE_ROOT } from "../branding.js";
2222
import { resolveModelFamilyPolicy, type ModelFamilyPolicy } from "./model-family-policy.js";
2323
import { PRESENT_VIEW_PRIMITIVES_GUIDANCE } from "./tool-schema-normalize.js";
24-
import { isOperatorOriginated } from "./message-provenance.js";
25-
import { classifyBriefSalvage, isHardBlockSalvage } from "../subagent/brief-dispatch.js";
26-
import type { ForcedStopReason } from "../subagent/stop-policy.js";
27-
import { PRIMARY_SALVAGE_NUDGE } from "./look-tour.js";
2824

2925
const logger = getLogger([LOG_NAMESPACE_ROOT, "agent", "director"]);
3026

@@ -394,10 +390,6 @@ class ChatDirectorImpl extends DefaultDirector {
394390
private toolOnlyStreak = 0;
395391
private toolOnlyNudgeFired = false;
396392
private pendingToolOnlyNudge = false;
397-
// One-shot nudge after a hard-block worker salvage. Not a look-count quota.
398-
private salvageNudgeFired = false;
399-
private pendingSalvageNudge: string | null = null;
400-
private pendingTaskCallIds = new Set<string>();
401393

402394
constructor(
403395
systemPrompt: string,
@@ -489,26 +481,6 @@ class ChatDirectorImpl extends DefaultDirector {
489481
return rewritten;
490482
}
491483

492-
/**
493-
* One-shot salvage nudge after a worker hard-block. Fingerprint thrash
494-
* (applyToolOnlyLoopProtection) wins when both apply. Attaches to the infer
495-
* after pending tools have executed.
496-
*/
497-
private applySalvageNudge(
498-
actions: ReactorAction[],
499-
capabilities: ReactorCapabilities,
500-
): ReactorAction[] | null {
501-
if (this.pendingSalvageNudge === null) return null;
502-
const inferIndex = actions.findIndex((a) => a.type === "infer");
503-
if (inferIndex === -1) return null;
504-
const text = this.pendingSalvageNudge;
505-
this.pendingSalvageNudge = null;
506-
const rewritten = [...actions];
507-
const existing = actions[inferIndex] as Extract<ReactorAction, { type: "infer" }>;
508-
rewritten[inferIndex] = inferWithNudge(capabilities, text, existing.options);
509-
return rewritten;
510-
}
511-
512484
private withCurrentTools(
513485
result: ReactorAction | ReactorAction[],
514486
): ReactorAction | ReactorAction[] {
@@ -637,15 +609,6 @@ class ChatDirectorImpl extends DefaultDirector {
637609
this.toolOnlyStreak = 0;
638610
this.toolOnlyNudgeFired = false;
639611
this.pendingToolOnlyNudge = false;
640-
// Only a message carrying OPERATOR_ORIGINATED_FLAG resets the salvage
641-
// nudge — not every message.received. Synthetic system sends
642-
// (compaction continuations, retries, future director continuations)
643-
// also fire message.received but are not a genuine operator checkpoint.
644-
if (isOperatorOriginated(event.message.flags)) {
645-
this.salvageNudgeFired = false;
646-
this.pendingSalvageNudge = null;
647-
this.pendingTaskCallIds.clear();
648-
}
649612
}
650613
if (onTurnBoundary(event)) this.inferenceRecoveries = 0;
651614

@@ -700,16 +663,6 @@ class ChatDirectorImpl extends DefaultDirector {
700663
// toolOnlyStreak is narration-sensitive: any turn with text clears it
701664
// (same as a fresh user message), and it only drives the soft
702665
// check-in nudge at toolOnlyTurnNudgeAt, never a stop.
703-
const turnContent = event.turn.content as readonly {
704-
type: string;
705-
name?: string;
706-
id?: string;
707-
}[];
708-
for (const block of turnContent) {
709-
if (block.type === "tool_call" && block.name === "task" && typeof block.id === "string") {
710-
this.pendingTaskCallIds.add(block.id);
711-
}
712-
}
713666
if (hasToolCalls && !hasText) {
714667
this.toolOnlyStreak++;
715668
} else {
@@ -755,19 +708,6 @@ class ChatDirectorImpl extends DefaultDirector {
755708
}
756709
}
757710

758-
if (event.type === "tool.done" && this.pendingTaskCallIds.has(event.result.callId)) {
759-
this.pendingTaskCallIds.delete(event.result.callId);
760-
const detail = event.result.detail as { stopReason?: ForcedStopReason } | undefined;
761-
const salvage = classifyBriefSalvage({
762-
...(detail?.stopReason !== undefined ? { stopReason: detail.stopReason } : {}),
763-
wasCancelled: false,
764-
});
765-
if (salvage !== null && isHardBlockSalvage(salvage) && !this.salvageNudgeFired) {
766-
this.salvageNudgeFired = true;
767-
this.pendingSalvageNudge = PRIMARY_SALVAGE_NUDGE;
768-
}
769-
}
770-
771711
if (event.type === "tool.done" && this.workflowCalls.has(event.result.callId)) {
772712
const call = this.workflowCalls.get(event.result.callId);
773713
this.workflowCalls.delete(event.result.callId);
@@ -838,8 +778,6 @@ class ChatDirectorImpl extends DefaultDirector {
838778
// wiring in src/subagent/index.ts).
839779
const toolOnlyRewrite = this.applyToolOnlyLoopProtection(baseActions, capabilities);
840780
if (toolOnlyRewrite !== null) return toolOnlyRewrite;
841-
const lookRewrite = this.applySalvageNudge(baseActions, capabilities);
842-
if (lookRewrite !== null) return lookRewrite;
843781

844782
const coordinator = this.workflowCoordinator;
845783
if (coordinator?.isActive() && !coordinator.currentStepIsGate()) {

src/agent/look-tour.test.ts

Lines changed: 0 additions & 10 deletions
This file was deleted.

src/agent/look-tour.ts

Lines changed: 0 additions & 7 deletions
This file was deleted.

src/agent/prompts.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,6 @@ 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 bounded inference budget (unset is unbounded). 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 a thrash / no-ship / never-acted / never-edited 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.",
157156
"- Use manage_tasks for your own coordination checklist; spawning workers is `task`, not manage_tasks.",
158157
"- If context is compacted automatically, do not stop tasks early due to token fear; persist progress via manage_tasks and worker reports.",
159158
]),

src/subagent/brief-dispatch.ts

Lines changed: 12 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,16 @@
11
/**
22
* Parent-side re-dispatch caps 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+
* This module tracks how often the *parent* re-spawns the same brief so
5+
* turn-budget salvage flips from "raise maxTurns" to "stop" after enough
6+
* same-brief dispatches without a successful complete.
107
*
118
* Session-scoped: one ledger per createTaskTool instance (parent chat tool).
129
*/
1310

1411
import type { TaskIntent } from "./report.js";
1512
import type { ForcedStopReason } from "./stop-policy.js";
1613

17-
/** Salvage classes that must not be re-dispatched with an identical brief. */
18-
export type HardBlockSalvage = "no-ship" | "never-acted" | "never-edited";
19-
2014
// Every forced-stop reason a leaf can report maps 1:1 onto a salvage kind
2115
// the parent ledger cares about.
2216
export type BriefSalvageKind = ForcedStopReason;
@@ -32,8 +26,6 @@ export interface TaskBriefFingerprintInput {
3226
export interface BriefDispatchRecord {
3327
/** How many times this fingerprint has been accepted for run (including first). */
3428
dispatchCount: number;
35-
/** Last salvage class observed for this fingerprint, if any. */
36-
lastSalvage?: BriefSalvageKind;
3729
}
3830

3931
/**
@@ -43,12 +35,6 @@ export interface BriefDispatchRecord {
4335
*/
4436
export const TURN_BUDGET_STOP_AFTER_DISPATCHES = 3;
4537

46-
const HARD_BLOCK_SALVAGES = new Set<BriefSalvageKind>(["no-ship", "never-acted", "never-edited"]);
47-
48-
export function isHardBlockSalvage(kind: BriefSalvageKind): kind is HardBlockSalvage {
49-
return HARD_BLOCK_SALVAGES.has(kind);
50-
}
51-
5238
/**
5339
* Classify a completed dispatch as a salvage kind the parent ledger cares
5440
* about, from the structured stop reason the run reported directly — never
@@ -93,13 +79,8 @@ function serializeList(items: readonly string[] | undefined): string {
9379

9480
export interface BriefDispatchLedger {
9581
get: (fingerprint: string) => BriefDispatchRecord | undefined;
96-
/**
97-
* Pre-run gate. Returns ok with the 1-based dispatch count that will be used,
98-
* or a reject message for the parent tool result.
99-
*/
100-
admit: (
101-
fingerprint: string,
102-
) => { ok: true; dispatchCount: number } | { ok: false; message: string };
82+
/** Pre-run gate. Always admits, returning the 1-based dispatch count that will be used. */
83+
admit: (fingerprint: string) => { dispatchCount: number };
10384
/** Record the outcome of an admitted run (salvage kind or null on success). */
10485
recordOutcome: (fingerprint: string, salvage: BriefSalvageKind | null) => void;
10586
/**
@@ -119,77 +100,33 @@ export function createBriefDispatchLedger(): BriefDispatchLedger {
119100

120101
admit(fingerprint) {
121102
const existing = byFingerprint.get(fingerprint);
122-
if (existing?.lastSalvage !== undefined && isHardBlockSalvage(existing.lastSalvage)) {
123-
return {
124-
ok: false,
125-
message: hardBlockMessage(existing.lastSalvage, existing.dispatchCount),
126-
};
127-
}
128103
const nextCount = (existing?.dispatchCount ?? 0) + 1;
129-
byFingerprint.set(fingerprint, {
130-
dispatchCount: nextCount,
131-
...(existing?.lastSalvage !== undefined ? { lastSalvage: existing.lastSalvage } : {}),
132-
});
133-
return { ok: true, dispatchCount: nextCount };
104+
byFingerprint.set(fingerprint, { dispatchCount: nextCount });
105+
return { dispatchCount: nextCount };
134106
},
135107

136108
recordOutcome(fingerprint, salvage) {
137-
const existing = byFingerprint.get(fingerprint);
138-
if (existing === undefined) {
139-
// admit() always runs first in production; keep defensive for unit tests.
140-
byFingerprint.set(fingerprint, {
141-
dispatchCount: salvage === null ? 0 : 1,
142-
...(salvage !== null ? { lastSalvage: salvage } : {}),
143-
});
144-
return;
145-
}
146109
if (salvage === null) {
147-
// CL-6710: a successful complete clears the sticky hard-block too.
148-
// Two concurrent identical-brief dispatches can both admit; if one
149-
// salvages and the other succeeds, the success proves the brief is
150-
// re-dispatchable, so it must not leave the sibling's hard-block
151-
// standing for the rest of the session.
110+
// A successful complete resets the same-brief retry budget.
152111
byFingerprint.set(fingerprint, { dispatchCount: 0 });
153112
return;
154113
}
155-
byFingerprint.set(fingerprint, {
156-
dispatchCount: existing.dispatchCount,
157-
lastSalvage: salvage,
158-
});
114+
const existing = byFingerprint.get(fingerprint);
115+
byFingerprint.set(fingerprint, { dispatchCount: existing?.dispatchCount ?? 1 });
159116
},
160117

161118
release(fingerprint) {
162119
const existing = byFingerprint.get(fingerprint);
163120
if (existing === undefined) return;
164121
if (existing.dispatchCount <= 1) {
165-
if (existing.lastSalvage !== undefined) {
166-
byFingerprint.set(fingerprint, {
167-
dispatchCount: 0,
168-
lastSalvage: existing.lastSalvage,
169-
});
170-
} else {
171-
byFingerprint.delete(fingerprint);
172-
}
122+
byFingerprint.delete(fingerprint);
173123
return;
174124
}
175-
byFingerprint.set(fingerprint, {
176-
dispatchCount: existing.dispatchCount - 1,
177-
...(existing.lastSalvage !== undefined ? { lastSalvage: existing.lastSalvage } : {}),
178-
});
125+
byFingerprint.set(fingerprint, { dispatchCount: existing.dispatchCount - 1 });
179126
},
180127
};
181128
}
182129

183-
function hardBlockMessage(salvage: HardBlockSalvage, priorDispatches: number): string {
184-
return (
185-
`Error: refused re-dispatch of an identical task brief after a ${salvage} salvage ` +
186-
`(already dispatched ${priorDispatches} time${priorDispatches === 1 ? "" : "s"}). ` +
187-
`Change the brief (prompt, agent, intent, success_criteria, and/or do_not) before retrying — ` +
188-
`raising maxTurns alone will not unlock this fingerprint. ` +
189-
`To force a re-run of the same work, alter at least one of those fields so the fingerprint changes.`
190-
);
191-
}
192-
193130
/**
194131
* Whether turn-budget parent hint should recommend stopping rather than
195132
* re-dispatching with a higher maxTurns.

0 commit comments

Comments
 (0)