fix(foreground-fallback): address PR #736 review feedback

- Fix class-level doc: clarify abort only on session.status retry path
- Rename shouldIntervene -> shouldTriggerFallback for clarity
- Rename checkRetryBudget -> consumeRetryBudget (signals mutation)
- Update shouldTriggerFallback JSDoc to reflect all three paths
- Add stale retry event guard (fixes greptile-apps race condition)
- Add ponytail comment for transport/outage error patterns
- Add regression test for stale retry after model switch

src/agents/index.ts

@@ -728,4 +728,3 @@ export function getDisabledAgents(config?: PluginConfig): Set<string> {
   }
   return disabled;
 }
-

src/hooks/foreground-fallback/index.test.ts

@@ -673,7 +673,11 @@ describe('ForegroundFallbackManager session.status', () => {
       type: 'session.status',
       properties: {
         sessionID: 'sess-retry2',
-        status: { type: 'retry', attempt: 1, message: 'rate limit, retrying...' },
+        status: {
+          type: 'retry',
+          attempt: 1,
+          message: 'rate limit, retrying...',
+        },
       },
     });
     expect(mocks.promptAsync).toHaveBeenCalledTimes(1);
@@ -772,6 +776,77 @@ describe('ForegroundFallbackManager session.status', () => {
     });
     expect(mocks.promptAsync).toHaveBeenCalledTimes(1);
   });
+
+  test('ignores stale retry event from original model after fallback switches models', async () => {
+    // greptile-apps race condition: after a fallback succeeds and the manager
+    // switches to model B, a delayed retry event from model A's original retry
+    // loop (already in-flight when the abort happened) should NOT trigger a
+    // second fallback — it carries the old model's error, not model B's.
+    const calls: string[] = [];
+    const { client, mocks } = createMockClient({
+      abortImpl: async () => {
+        calls.push('abort');
+      },
+      promptAsyncImpl: async () => {
+        calls.push('promptAsync');
+        return {};
+      },
+    });
+    const mgr = new ForegroundFallbackManager(client, makeChains(), true, 3);
+
+    // Seed session with model A (anthropic/claude-opus-4-5)
+    await mgr.handleEvent({
+      type: 'message.updated',
+      properties: {
+        info: {
+          sessionID: 'sess-stale',
+          providerID: 'anthropic',
+          modelID: 'claude-opus-4-5',
+        },
+      },
+    });
+
+    // First retry event: model A rate-limited → triggers fallback to model B
+    await mgr.handleEvent({
+      type: 'session.status',
+      properties: {
+        sessionID: 'sess-stale',
+        status: {
+          type: 'retry',
+          attempt: 1,
+          message: 'rate limit, retrying...',
+        },
+      },
+    });
+
+    expect(mocks.abort).toHaveBeenCalledTimes(1);
+    expect(mocks.promptAsync).toHaveBeenCalledTimes(1);
+    const firstCall = mocks.promptAsync.mock.calls[0] as [
+      { body: { model: { providerID: string; modelID: string } } },
+    ];
+    expect(firstCall[0].body.model).toEqual({
+      providerID: 'openai',
+      modelID: 'gpt-4o',
+    });
+
+    // Stale retry event from the ORIGINAL model A arrives after the switch.
+    // The session model is now openai/gpt-4o, so this event should be ignored.
+    await mgr.handleEvent({
+      type: 'session.status',
+      properties: {
+        sessionID: 'sess-stale',
+        status: {
+          type: 'retry',
+          attempt: 2,
+          message: 'rate limit, retrying...',
+        },
+      },
+    });
+
+    // Should NOT trigger another fallback — the event is stale
+    expect(mocks.abort).toHaveBeenCalledTimes(1);
+    expect(mocks.promptAsync).toHaveBeenCalledTimes(1);
+  });
 });
 
 // ---------------------------------------------------------------------------

src/hooks/foreground-fallback/index.ts

@@ -4,7 +4,9 @@
  * When OpenCode fires a session.error, message.updated, or session.status
  * event containing a rate-limit signal, this manager:
  *   1. Looks up the next untried model in the agent's configured chain
- *   2. Aborts the rate-limited prompt via client.session.abort()
+ *   2. Aborts the rate-limited prompt via client.session.abort() on the
+ *      session.status retry path; session.error and message.updated paths
+ *      re-prompt directly without abort.
  *   3. Re-queues the last user message via client.session.promptAsync()
  *      with the new model - promptAsync returns immediately so we never
  *      block the event handler waiting for a full LLM response.
@@ -49,6 +51,7 @@ const RATE_LIMIT_PATTERNS = [
 ];
 
 const OUTAGE_STATUS_CODES = new Set([500, 502, 503, 504]);
+// ponytail: validated against real OpenCode error shapes
 const TRANSPORT_CODES = new Set([
   'ECONNREFUSED',
   'ECONNRESET',
@@ -279,7 +282,7 @@ export class ForegroundFallbackManager {
         }
         // Failover-worthy error on an individual message
         if (info.error && isFailoverError(info.error)) {
-          if (this.shouldIntervene(sessionID)) {
+          if (this.shouldTriggerFallback(sessionID)) {
             await this.tryFallback(sessionID);
           }
         } else {
@@ -299,7 +302,7 @@ export class ForegroundFallbackManager {
           sessionID &&
           props.error &&
           isFailoverError(props.error) &&
-          this.shouldIntervene(sessionID)
+          this.shouldTriggerFallback(sessionID)
         ) {
           await this.tryFallback(sessionID);
         }
@@ -324,7 +327,22 @@ export class ForegroundFallbackManager {
             (props.status.message !== undefined &&
               isFailoverError({ message: props.status.message })));
         if (isFailoverRetry) {
-          if (this.shouldIntervene(sessionID)) {
+          // Guard: stale retry event from a previous model's retry loop.
+          // When the model changed since the last trigger and we're still
+          // within the dedup window, this is a delayed event from the old
+          // model after a fallback already switched models. Skip it.
+          const prevModel = this.lastTriggerModel.get(sessionID);
+          const curModel = this.sessionModel.get(sessionID);
+          const lastTime = this.lastTrigger.get(sessionID) ?? 0;
+          if (
+            prevModel !== undefined &&
+            curModel !== undefined &&
+            prevModel !== curModel &&
+            Date.now() - lastTime < DEDUP_WINDOW_MS
+          ) {
+            break;
+          }
+          if (this.shouldTriggerFallback(sessionID)) {
             await this.tryFallbackWithAbort(sessionID);
           }
           break;
@@ -378,7 +396,7 @@ export class ForegroundFallbackManager {
    *  Used by shouldIntervene when tried > 0 — each retry counts toward the
    *  budget and only triggers fallback after maxRetries - 1 absorptions.
    *  First failover retry (tried === 0) bypasses the counter via shouldIntervene. */
-  private checkRetryBudget(sessionID: string): boolean {
+  private consumeRetryBudget(sessionID: string): boolean {
     const tried = this.sessionRetries.get(sessionID) ?? 0;
     if (tried < this.maxRetries - 1) {
       this.sessionRetries.set(sessionID, tried + 1);
@@ -393,12 +411,12 @@ export class ForegroundFallbackManager {
     return true;
   }
 
-  /** For non-retry paths (session.error, message.updated): intervene immediately
-   *  unless the session is already in a retry window (has prior retries). */
-  private shouldIntervene(sessionID: string): boolean {
+  /** Intervene immediately on first occurrence (tried === 0), otherwise
+   *  delegate to retry budget. Used by all three event paths. */
+  private shouldTriggerFallback(sessionID: string): boolean {
     const tried = this.sessionRetries.get(sessionID) ?? 0;
     if (tried === 0) return true;
-    return this.checkRetryBudget(sessionID);
+    return this.consumeRetryBudget(sessionID);
   }
 
   private isRecoveredStatus(statusType: string | undefined): boolean {