소스 검색

fix(foreground-fallback): remove 500ms delay between abort and execFallback

The delay gave OpenCode time to restart the retry loop after abort,
so promptAsync arrived when the session was already back in retry mode.
execFallback already handles promptAsync failure internally with its
own abort+retry, so the outer wait is unnecessary and harmful.
Michael Henke 1 개월 전
부모
커밋
2330bd0c21
2개의 변경된 파일107개의 추가작업 그리고 10개의 파일을 삭제
  1. 70 0
      src/hooks/foreground-fallback/index.test.ts
  2. 37 10
      src/hooks/foreground-fallback/index.ts

+ 70 - 0
src/hooks/foreground-fallback/index.test.ts

@@ -560,6 +560,76 @@ describe('ForegroundFallbackManager session.status', () => {
     });
     expect(mocks.promptAsync).toHaveBeenCalledTimes(1);
   });
+
+  test('non-rate-limit status does not clear retries (no infinite loop from abort side effects)', async () => {
+    const { client, mocks } = createMockClient();
+    const mgr = new ForegroundFallbackManager(client, makeChains(), true, 3);
+
+    await mgr.handleEvent({
+      type: 'message.updated',
+      properties: {
+        info: {
+          sessionID: 'sess-nonrl',
+          providerID: 'anthropic',
+          modelID: 'claude-opus-4-5',
+        },
+      },
+    });
+
+    // First rate-limit: triggers fallback, sessionRetries set to 1
+    await mgr.handleEvent({
+      type: 'session.status',
+      properties: {
+        sessionID: 'sess-nonrl',
+        status: {
+          type: 'retry',
+          attempt: 1,
+          message: 'rate limit, retrying...',
+        },
+      },
+    });
+    expect(mocks.promptAsync).toHaveBeenCalledTimes(1);
+
+    // Non-rate-limit status (e.g. abort side effect): must NOT reset retries.
+    // If it did, the next rate-limit would see tried=0 and trigger immediate
+    // fallback again — the infinite loop.
+    await mgr.handleEvent({
+      type: 'session.status',
+      properties: {
+        sessionID: 'sess-nonrl',
+        status: { type: 'retry', attempt: 1, message: 'aborted' },
+      },
+    });
+    expect(mocks.promptAsync).toHaveBeenCalledTimes(1);
+
+    // Second rate-limit: absorbed by budget (tried=1, not 0 from reset)
+    await mgr.handleEvent({
+      type: 'session.status',
+      properties: {
+        sessionID: 'sess-nonrl',
+        status: {
+          type: 'retry',
+          attempt: 2,
+          message: 'rate limit, retrying...',
+        },
+      },
+    });
+    expect(mocks.promptAsync).toHaveBeenCalledTimes(1);
+
+    // Third rate-limit: budget exhausted at maxRetries-1=2, re-triggers
+    await mgr.handleEvent({
+      type: 'session.status',
+      properties: {
+        sessionID: 'sess-nonrl',
+        status: {
+          type: 'retry',
+          attempt: 3,
+          message: 'rate limit, retrying...',
+        },
+      },
+    });
+    expect(mocks.promptAsync).toHaveBeenCalledTimes(2);
+  });
 });
 
 // ---------------------------------------------------------------------------

+ 37 - 10
src/hooks/foreground-fallback/index.ts

@@ -225,12 +225,7 @@ export class ForegroundFallbackManager {
           isRateLimitError(props.error);
         if (isRateLimit) {
           if (this.shouldIntervene(props.sessionID)) {
-            // Let tryFallback handle retry-loop breaking internally —
-            // it sets inProgress first, so the task-session-manager sees
-            // isFallbackInProgress()=true during the abort idle window.
-            // External abort+wait would leave inProgress unset and the
-            // task manager would cancel the pending call on idle.
-            await this.tryFallback(props.sessionID);
+            await this.tryFallbackWithAbort(props.sessionID);
             this.sessionRetries.set(props.sessionID, 1);
           }
         }
@@ -313,6 +308,38 @@ export class ForegroundFallbackManager {
     // Deduplicate: multiple events can fire for a single rate-limit event.
     // Bypass dedup when the model changed since the last trigger - the new
     // model's failure is a separate incident and the cascade should continue.
+    if (this.isDeduped(sessionID)) return;
+
+    this.inProgress.add(sessionID);
+    try {
+      await this.execFallback(sessionID);
+    } finally {
+      this.inProgress.delete(sessionID);
+    }
+  }
+
+  /**
+   * Fallback path for session.status retry events.  Aborts the retry loop
+   * before falling back because promptAsync alone is ignored while the
+   * session is in retry mode.  inProgress is set first so the
+   * task-session-manager sees isFallbackInProgress()=true during the
+   * abort idle window and does not cancel the pending task call.
+   */
+  private async tryFallbackWithAbort(sessionID: string): Promise<void> {
+    if (!sessionID) return;
+    if (this.inProgress.has(sessionID)) return;
+    if (this.isDeduped(sessionID)) return;
+
+    this.inProgress.add(sessionID);
+    try {
+      await abortSessionWithTimeout(this.client, sessionID);
+      await this.execFallback(sessionID);
+    } finally {
+      this.inProgress.delete(sessionID);
+    }
+  }
+
+  private isDeduped(sessionID: string): boolean {
     const now = Date.now();
     const curModel = this.sessionModel.get(sessionID);
     const modelChanged =
@@ -322,13 +349,15 @@ export class ForegroundFallbackManager {
       !modelChanged &&
       now - (this.lastTrigger.get(sessionID) ?? 0) < DEDUP_WINDOW_MS
     )
-      return;
+      return true;
     this.lastTrigger.set(sessionID, now);
     if (curModel !== undefined) {
       this.lastTriggerModel.set(sessionID, curModel);
     }
+    return false;
+  }
 
-    this.inProgress.add(sessionID);
+  private async execFallback(sessionID: string): Promise<void> {
     try {
       let currentModel = this.sessionModel.get(sessionID);
       const agentName = this.sessionAgent.get(sessionID);
@@ -492,8 +521,6 @@ export class ForegroundFallbackManager {
         sessionID,
         error: err instanceof Error ? err.message : String(err),
       });
-    } finally {
-      this.inProgress.delete(sessionID);
     }
   }