From 2cba5bc0727e97a558fa27e3d093d422096f736a Mon Sep 17 00:00:00 2001
From: Tom Boucher <trekkie@nomorestars.com>
Date: Tue, 17 Mar 2026 16:01:36 -0400
Subject: [PATCH] fix: break reassess-roadmap skip loop by preventing
 re-persistence of evicted keys (#912) (#927)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

After the skip-loop breaker evicts a completion key, the fallback path
at the bottom of dispatchNextUnit re-persists it because the expected
artifact exists on disk. This recreates the exact loop the breaker was
trying to break:

  evict key → dispatch → verifyArtifact(true) → re-persist key → skip → evict → repeat

Fix: Track recently-evicted keys in a Set. The fallback artifact-check
path skips re-persistence for keys that were just evicted by the
skip-loop breaker. Set is cleared on stopAuto.
---
 src/resources/extensions/gsd/auto.ts | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/resources/extensions/gsd/auto.ts b/src/resources/extensions/gsd/auto.ts
index e36780f8b..cdb9ddb9c 100644
--- a/src/resources/extensions/gsd/auto.ts
+++ b/src/resources/extensions/gsd/auto.ts
@@ -564,6 +564,7 @@ export async function stopAuto(ctx?: ExtensionContext, pi?: ExtensionAPI, reason
   clearSliceProgressCache();
   clearActivityLogState();
   resetProactiveHealing();
+  recentlyEvictedKeys.clear();
   pendingCrashRecovery = null;
   pendingVerificationRetry = null;
   verificationRetryCount.clear();
@@ -2033,6 +2034,9 @@ const MAX_SKIP_DEPTH = 20;
  *  allowed via _skipDepth > 0. */
 let _dispatching = false;
 
+/** Keys recently evicted by skip-loop breaker — prevents re-persistence in the fallback path (#912). */
+const recentlyEvictedKeys = new Set<string>();
+
 async function dispatchNextUnit(
   ctx: ExtensionContext,
   pi: ExtensionAPI,
@@ -2589,6 +2593,7 @@ async function dispatchNextUnit(
         }
         unitConsecutiveSkips.delete(idempotencyKey);
         completedKeySet.delete(idempotencyKey);
+        recentlyEvictedKeys.add(idempotencyKey);
         removePersistedKey(basePath, idempotencyKey);
         invalidateAllCaches();
         ctx.ui.notify(
@@ -2637,9 +2642,11 @@ async function dispatchNextUnit(
   // Fallback: if the idempotency key is missing but the expected artifact already
   // exists on disk, the task completed in a prior session without persisting the key.
   // Persist it now and skip re-dispatch. This prevents infinite loops where a task
-  // completes successfully but the completion key was never written (e.g., completed
-  // on the first attempt before hitting the retry-threshold persistence logic).
-  if (verifyExpectedArtifact(unitType, unitId, basePath)) {
+  // completes successfully but the completion key was never written.
+  //
+  // EXCEPTION: if the key was just evicted by the skip-loop breaker above, do NOT
+  // re-persist — that would recreate the exact loop the breaker was trying to break (#912).
+  if (verifyExpectedArtifact(unitType, unitId, basePath) && !recentlyEvictedKeys.has(idempotencyKey)) {
     persistCompletedKey(basePath, idempotencyKey);
     completedKeySet.add(idempotencyKey);
     invalidateAllCaches();