Delete never-edited/never-acted/no-ship salvage classes (CL-6994)

CHANGELOG.md

@@ -26,6 +26,14 @@ parallel copies under `docs/` or `scripts/notes/`. At cut time: rename
   reports directly, not by re-parsing the parent-facing report's prose.
   Removes the `isXxxSubAgentReport` classifier family and per-reason parent
   hint functions in favor of a single structured switch.
+- Removed the `never-edited`, `never-acted`, and `no-ship` sub-agent salvage
+  classes and the sticky hard-block that refused an identical re-dispatch
+  after one fired. `task` re-dispatch is never refused now; turn-budget
+  salvage still throttles repeated same-brief retries. Also removed the
+  now-dead shell-write half of the shell-evidence detector (read detection
+  for `requireEvidence` is unchanged) and the shell-write contribution to
+  `editedPaths` diagnostics.
+
 ### Internal
 
 - Removed the dead Ink-era kill ring copy (`src/tui/kill-ring.ts`); the OpenTUI

src/agent/director.ts

@@ -21,10 +21,6 @@ import { isInternalRecoveryAbortRaw } from "../inference-abort.js";
 import { LOG_NAMESPACE_ROOT } from "../branding.js";
 import { resolveModelFamilyPolicy, type ModelFamilyPolicy } from "./model-family-policy.js";
 import { PRESENT_VIEW_PRIMITIVES_GUIDANCE } from "./tool-schema-normalize.js";
-import { isOperatorOriginated } from "./message-provenance.js";
-import { classifyBriefSalvage, isHardBlockSalvage } from "../subagent/brief-dispatch.js";
-import type { ForcedStopReason } from "../subagent/stop-policy.js";
-import { PRIMARY_SALVAGE_NUDGE } from "./look-tour.js";
 
 const logger = getLogger([LOG_NAMESPACE_ROOT, "agent", "director"]);
 
@@ -394,10 +390,6 @@ class ChatDirectorImpl extends DefaultDirector {
   private toolOnlyStreak = 0;
   private toolOnlyNudgeFired = false;
   private pendingToolOnlyNudge = false;
-  // One-shot nudge after a hard-block worker salvage. Not a look-count quota.
-  private salvageNudgeFired = false;
-  private pendingSalvageNudge: string | null = null;
-  private pendingTaskCallIds = new Set<string>();
 
   constructor(
     systemPrompt: string,
@@ -489,26 +481,6 @@ class ChatDirectorImpl extends DefaultDirector {
     return rewritten;
   }
 
-  /**
-   * One-shot salvage nudge after a worker hard-block. Fingerprint thrash
-   * (applyToolOnlyLoopProtection) wins when both apply. Attaches to the infer
-   * after pending tools have executed.
-   */
-  private applySalvageNudge(
-    actions: ReactorAction[],
-    capabilities: ReactorCapabilities,
-  ): ReactorAction[] | null {
-    if (this.pendingSalvageNudge === null) return null;
-    const inferIndex = actions.findIndex((a) => a.type === "infer");
-    if (inferIndex === -1) return null;
-    const text = this.pendingSalvageNudge;
-    this.pendingSalvageNudge = null;
-    const rewritten = [...actions];
-    const existing = actions[inferIndex] as Extract<ReactorAction, { type: "infer" }>;
-    rewritten[inferIndex] = inferWithNudge(capabilities, text, existing.options);
-    return rewritten;
-  }
-
   private withCurrentTools(
     result: ReactorAction | ReactorAction[],
   ): ReactorAction | ReactorAction[] {
@@ -637,15 +609,6 @@ class ChatDirectorImpl extends DefaultDirector {
       this.toolOnlyStreak = 0;
       this.toolOnlyNudgeFired = false;
       this.pendingToolOnlyNudge = false;
-      // Only a message carrying OPERATOR_ORIGINATED_FLAG resets the salvage
-      // nudge — not every message.received. Synthetic system sends
-      // (compaction continuations, retries, future director continuations)
-      // also fire message.received but are not a genuine operator checkpoint.
-      if (isOperatorOriginated(event.message.flags)) {
-        this.salvageNudgeFired = false;
-        this.pendingSalvageNudge = null;
-        this.pendingTaskCallIds.clear();
-      }
     }
     if (onTurnBoundary(event)) this.inferenceRecoveries = 0;
 
@@ -700,16 +663,6 @@ class ChatDirectorImpl extends DefaultDirector {
       // toolOnlyStreak is narration-sensitive: any turn with text clears it
       // (same as a fresh user message), and it only drives the soft
       // check-in nudge at toolOnlyTurnNudgeAt, never a stop.
-      const turnContent = event.turn.content as readonly {
-        type: string;
-        name?: string;
-        id?: string;
-      }[];
-      for (const block of turnContent) {
-        if (block.type === "tool_call" && block.name === "task" && typeof block.id === "string") {
-          this.pendingTaskCallIds.add(block.id);
-        }
-      }
       if (hasToolCalls && !hasText) {
         this.toolOnlyStreak++;
       } else {
@@ -755,19 +708,6 @@ class ChatDirectorImpl extends DefaultDirector {
       }
     }
 
-    if (event.type === "tool.done" && this.pendingTaskCallIds.has(event.result.callId)) {
-      this.pendingTaskCallIds.delete(event.result.callId);
-      const detail = event.result.detail as { stopReason?: ForcedStopReason } | undefined;
-      const salvage = classifyBriefSalvage({
-        ...(detail?.stopReason !== undefined ? { stopReason: detail.stopReason } : {}),
-        wasCancelled: false,
-      });
-      if (salvage !== null && isHardBlockSalvage(salvage) && !this.salvageNudgeFired) {
-        this.salvageNudgeFired = true;
-        this.pendingSalvageNudge = PRIMARY_SALVAGE_NUDGE;
-      }
-    }
-
     if (event.type === "tool.done" && this.workflowCalls.has(event.result.callId)) {
       const call = this.workflowCalls.get(event.result.callId);
       this.workflowCalls.delete(event.result.callId);
@@ -838,8 +778,6 @@ class ChatDirectorImpl extends DefaultDirector {
     // wiring in src/subagent/index.ts).
     const toolOnlyRewrite = this.applyToolOnlyLoopProtection(baseActions, capabilities);
     if (toolOnlyRewrite !== null) return toolOnlyRewrite;
-    const lookRewrite = this.applySalvageNudge(baseActions, capabilities);
-    if (lookRewrite !== null) return lookRewrite;
 
     const coordinator = this.workflowCoordinator;
     if (coordinator?.isActive() && !coordinator.currentStepIsGate()) {

src/agent/prompts.ts

@@ -153,7 +153,6 @@ export function buildGuidelines(
           "- 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.",
           "- After workers return, merge their Summary/Findings into a coherent answer for the operator; do not paste raw sub-agent dumps.",
           "- 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.",
-          "- 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.",
           "- Use manage_tasks for your own coordination checklist; spawning workers is `task`, not manage_tasks.",
           "- If context is compacted automatically, do not stop tasks early due to token fear; persist progress via manage_tasks and worker reports.",
         ]),

src/subagent/brief-dispatch.ts

@@ -1,22 +1,16 @@
 /**
  * Parent-side re-dispatch caps for task briefs (CL-4343 + CL-5203).
  *
- * Leaf stops already salvage no-progress / turn-budget / etc. This
- * module tracks how often the *parent* re-spawns the same brief so:
- * - hard-block-class salvages refuse an identical re-dispatch for the rest of
- *   the parent chat session (sticky until the fingerprint changes)
- * - turn-budget salvage flips from "raise maxTurns" to "stop" after enough
- *   same-brief dispatches without a successful complete
+ * This module tracks how often the *parent* re-spawns the same brief so
+ * turn-budget salvage flips from "raise maxTurns" to "stop" after enough
+ * same-brief dispatches without a successful complete.
  *
  * Session-scoped: one ledger per createTaskTool instance (parent chat tool).
  */
 
 import type { TaskIntent } from "./report.js";
 import type { ForcedStopReason } from "./stop-policy.js";
 
-/** Salvage classes that must not be re-dispatched with an identical brief. */
-export type HardBlockSalvage = "no-ship" | "never-acted" | "never-edited";
-
 // Every forced-stop reason a leaf can report maps 1:1 onto a salvage kind
 // the parent ledger cares about.
 export type BriefSalvageKind = ForcedStopReason;
@@ -32,8 +26,6 @@ export interface TaskBriefFingerprintInput {
 export interface BriefDispatchRecord {
   /** How many times this fingerprint has been accepted for run (including first). */
   dispatchCount: number;
-  /** Last salvage class observed for this fingerprint, if any. */
-  lastSalvage?: BriefSalvageKind;
 }
 
 /**
@@ -43,12 +35,6 @@ export interface BriefDispatchRecord {
  */
 export const TURN_BUDGET_STOP_AFTER_DISPATCHES = 3;
 
-const HARD_BLOCK_SALVAGES = new Set<BriefSalvageKind>(["no-ship", "never-acted", "never-edited"]);
-
-export function isHardBlockSalvage(kind: BriefSalvageKind): kind is HardBlockSalvage {
-  return HARD_BLOCK_SALVAGES.has(kind);
-}
-
 /**
  * Classify a completed dispatch as a salvage kind the parent ledger cares
  * about, from the structured stop reason the run reported directly — never
@@ -93,13 +79,8 @@ function serializeList(items: readonly string[] | undefined): string {
 
 export interface BriefDispatchLedger {
   get: (fingerprint: string) => BriefDispatchRecord | undefined;
-  /**
-   * Pre-run gate. Returns ok with the 1-based dispatch count that will be used,
-   * or a reject message for the parent tool result.
-   */
-  admit: (
-    fingerprint: string,
-  ) => { ok: true; dispatchCount: number } | { ok: false; message: string };
+  /** Pre-run gate. Always admits, returning the 1-based dispatch count that will be used. */
+  admit: (fingerprint: string) => { dispatchCount: number };
   /** Record the outcome of an admitted run (salvage kind or null on success). */
   recordOutcome: (fingerprint: string, salvage: BriefSalvageKind | null) => void;
   /**
@@ -119,77 +100,33 @@ export function createBriefDispatchLedger(): BriefDispatchLedger {
 
     admit(fingerprint) {
       const existing = byFingerprint.get(fingerprint);
-      if (existing?.lastSalvage !== undefined && isHardBlockSalvage(existing.lastSalvage)) {
-        return {
-          ok: false,
-          message: hardBlockMessage(existing.lastSalvage, existing.dispatchCount),
-        };
-      }
       const nextCount = (existing?.dispatchCount ?? 0) + 1;
-      byFingerprint.set(fingerprint, {
-        dispatchCount: nextCount,
-        ...(existing?.lastSalvage !== undefined ? { lastSalvage: existing.lastSalvage } : {}),
-      });
-      return { ok: true, dispatchCount: nextCount };
+      byFingerprint.set(fingerprint, { dispatchCount: nextCount });
+      return { dispatchCount: nextCount };
     },
 
     recordOutcome(fingerprint, salvage) {
-      const existing = byFingerprint.get(fingerprint);
-      if (existing === undefined) {
-        // admit() always runs first in production; keep defensive for unit tests.
-        byFingerprint.set(fingerprint, {
-          dispatchCount: salvage === null ? 0 : 1,
-          ...(salvage !== null ? { lastSalvage: salvage } : {}),
-        });
-        return;
-      }
       if (salvage === null) {
-        // CL-6710: a successful complete clears the sticky hard-block too.
-        // Two concurrent identical-brief dispatches can both admit; if one
-        // salvages and the other succeeds, the success proves the brief is
-        // re-dispatchable, so it must not leave the sibling's hard-block
-        // standing for the rest of the session.
+        // A successful complete resets the same-brief retry budget.
         byFingerprint.set(fingerprint, { dispatchCount: 0 });
         return;
       }
-      byFingerprint.set(fingerprint, {
-        dispatchCount: existing.dispatchCount,
-        lastSalvage: salvage,
-      });
+      const existing = byFingerprint.get(fingerprint);
+      byFingerprint.set(fingerprint, { dispatchCount: existing?.dispatchCount ?? 1 });
     },
 
     release(fingerprint) {
       const existing = byFingerprint.get(fingerprint);
       if (existing === undefined) return;
       if (existing.dispatchCount <= 1) {
-        if (existing.lastSalvage !== undefined) {
-          byFingerprint.set(fingerprint, {
-            dispatchCount: 0,
-            lastSalvage: existing.lastSalvage,
-          });
-        } else {
-          byFingerprint.delete(fingerprint);
-        }
+        byFingerprint.delete(fingerprint);
         return;
       }
-      byFingerprint.set(fingerprint, {
-        dispatchCount: existing.dispatchCount - 1,
-        ...(existing.lastSalvage !== undefined ? { lastSalvage: existing.lastSalvage } : {}),
-      });
+      byFingerprint.set(fingerprint, { dispatchCount: existing.dispatchCount - 1 });
     },
   };
 }
 
-function hardBlockMessage(salvage: HardBlockSalvage, priorDispatches: number): string {
-  return (
-    `Error: refused re-dispatch of an identical task brief after a ${salvage} salvage ` +
-    `(already dispatched ${priorDispatches} time${priorDispatches === 1 ? "" : "s"}). ` +
-    `Change the brief (prompt, agent, intent, success_criteria, and/or do_not) before retrying — ` +
-    `raising maxTurns alone will not unlock this fingerprint. ` +
-    `To force a re-run of the same work, alter at least one of those fields so the fingerprint changes.`
-  );
-}
-
 /**
  * Whether turn-budget parent hint should recommend stopping rather than
  * re-dispatching with a higher maxTurns.