Răsfoiți Sursa

Merge pull request #822 from Jiajun0413/fix/foreground-fallback-no-chain-noise

fix(foreground-fallback): skip abort/log when agent has no chain
Alvin 2 săptămâni în urmă
părinte
comite
7b2fd3fdc0

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

@@ -1392,6 +1392,108 @@ describe('ForegroundFallbackManager resolveChain cross-agent isolation', () => {
   });
 });
 
+// ---------------------------------------------------------------------------
+// No-chain sessions (councillor / self-managed agents)
+// ---------------------------------------------------------------------------
+
+describe('ForegroundFallbackManager no-chain sessions', () => {
+  test('councillor session.status retry: no abort and no re-prompt', async () => {
+    // Councillor is owned by CouncilManager (own model chain + timeout).
+    // FG must not abort or re-prompt — that races the council lifecycle and
+    // previously produced "[foreground-fallback] no chain configured" noise.
+    const { client, mocks } = createMockClient();
+    const mgr = new ForegroundFallbackManager(client, makeChains(), true, 3);
+
+    await mgr.handleEvent({
+      type: 'message.updated',
+      properties: {
+        info: {
+          sessionID: 'councillor-sess',
+          agent: 'councillor',
+          providerID: 'openai',
+          modelID: 'gpt-5.4',
+        },
+      },
+    });
+
+    await mgr.handleEvent({
+      type: 'session.status',
+      properties: {
+        sessionID: 'councillor-sess',
+        status: {
+          type: 'retry',
+          attempt: 1,
+          message: 'rate limit, retrying...',
+        },
+      },
+    });
+
+    expect(mocks.abort).not.toHaveBeenCalled();
+    expect(mocks.promptAsync).not.toHaveBeenCalled();
+  });
+
+  test('councillor session.error: no abort and no re-prompt', async () => {
+    const { client, mocks } = createMockClient();
+    const mgr = new ForegroundFallbackManager(client, makeChains(), true);
+
+    await mgr.handleEvent({
+      type: 'message.updated',
+      properties: {
+        info: {
+          sessionID: 'councillor-err',
+          agent: 'councillor',
+          providerID: 'openai',
+          modelID: 'gpt-5.4',
+        },
+      },
+    });
+
+    await mgr.handleEvent({
+      type: 'session.error',
+      properties: {
+        sessionID: 'councillor-err',
+        error: { message: 'rate limit exceeded' },
+      },
+    });
+
+    expect(mocks.abort).not.toHaveBeenCalled();
+    expect(mocks.promptAsync).not.toHaveBeenCalled();
+  });
+
+  test('disableChain agent on session.status: no abort (not just no re-prompt)', async () => {
+    const { client, mocks } = createMockClient();
+    const mgr = new ForegroundFallbackManager(client, makeChains(), true, 3);
+    mgr.disableChain('orchestrator');
+
+    await mgr.handleEvent({
+      type: 'message.updated',
+      properties: {
+        info: {
+          sessionID: 'disabled-status',
+          agent: 'orchestrator',
+          providerID: 'anthropic',
+          modelID: 'claude-opus-4-5',
+        },
+      },
+    });
+
+    await mgr.handleEvent({
+      type: 'session.status',
+      properties: {
+        sessionID: 'disabled-status',
+        status: {
+          type: 'retry',
+          attempt: 1,
+          message: 'rate limit, retrying...',
+        },
+      },
+    });
+
+    expect(mocks.abort).not.toHaveBeenCalled();
+    expect(mocks.promptAsync).not.toHaveBeenCalled();
+  });
+});
+
 // ---------------------------------------------------------------------------
 // disableChain API
 // ---------------------------------------------------------------------------

+ 20 - 7
src/hooks/foreground-fallback/index.ts

@@ -470,6 +470,9 @@ export class ForegroundFallbackManager {
   private async tryFallback(sessionID: string): Promise<void> {
     if (!sessionID) return;
     if (this.inProgress.has(sessionID)) return;
+    // No chain → no fallback. Skip before dedup so we don't stamp lastTrigger
+    // for sessions we will never re-prompt (e.g. councillor via CouncilManager).
+    if (!this.hasFallbackChain(sessionID)) return;
 
     // Deduplicate: multiple events can fire for a single rate-limit event.
     // Bypass dedup when the model changed since the last trigger - the new
@@ -490,10 +493,15 @@ export class ForegroundFallbackManager {
    * 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.
+   *
+   * When no chain is available, do nothing (no abort, no log). Aborting
+   * without a replacement model only races owners that manage their own
+   * lifecycle (e.g. CouncilManager for councillor) and produces noise.
    */
   private async tryFallbackWithAbort(sessionID: string): Promise<void> {
     if (!sessionID) return;
     if (this.inProgress.has(sessionID)) return;
+    if (!this.hasFallbackChain(sessionID)) return;
     if (this.isDeduped(sessionID)) return;
 
     this.inProgress.add(sessionID);
@@ -528,13 +536,8 @@ export class ForegroundFallbackManager {
       let currentModel = this.sessionModel.get(sessionID);
       const agentName = this.sessionAgent.get(sessionID);
       const chain = this.resolveChain(agentName, currentModel);
-      if (!chain.length) {
-        log('[foreground-fallback] no chain configured', {
-          sessionID,
-          agentName,
-        });
-        return;
-      }
+      // Callers pre-check via hasFallbackChain; keep as defensive guard only.
+      if (!chain.length) return;
 
       // When the agent is known but no model was captured (common for
       // subagent error events that fire before message.updated), infer
@@ -675,6 +678,16 @@ export class ForegroundFallbackManager {
   // Chain resolution
   // ---------------------------------------------------------------------------
 
+  /** True when resolveChain yields at least one model for this session. */
+  private hasFallbackChain(sessionID: string): boolean {
+    return (
+      this.resolveChain(
+        this.sessionAgent.get(sessionID),
+        this.sessionModel.get(sessionID),
+      ).length > 0
+    );
+  }
+
   /**
    * Determine the fallback chain to use for a session.
    *

+ 2 - 2
src/index.ts

@@ -312,8 +312,8 @@ const OhMyOpenCodeLite: Plugin = async (ctx) => {
     chatHeadersHook = createChatHeadersHook(ctx);
 
     // Initialize foreground fallback manager for runtime model switching.
-    // Enabled by default even without fallback chains — the manager can still
-    // abort rate-limited sessions after maxRetries to prevent infinite freezes.
+    // Agents without a chain (e.g. councillor, owned by CouncilManager) are
+    // left alone — FG only aborts/re-prompts when it has a model to switch to.
     foregroundFallback = new ForegroundFallbackManager(
       ctx.client,
       runtimeChains,