Merge pull request #593 from 824156793/fix/fallback-three-deadlocks

fix(fallback): resolve three deadlock points in foreground fallback + task-session-manager

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

@@ -698,4 +698,39 @@ describe('ForegroundFallbackManager resolveChain cross-agent isolation', () => {
     expect(call[0].body.model.providerID).toBe('openai');
     expect(call[0].body.model.modelID).toBe('gpt-4o');
   });
+
+  test('falls through to model matching for non-omos agents (e.g. compaction)', async () => {
+    // compaction is an OpenCode built-in agent that is NOT an omos agent,
+    // so it has no chain configured. It should fall through to model
+    // matching and inherit a chain from a configured agent that shares
+    // its model, instead of being silently excluded from fallback.
+    const { client, mocks } = createMockClient();
+    const mgr = new ForegroundFallbackManager(
+      client,
+      { orchestrator: ['openai/gpt-5.4', 'new-api/glm-5.2'] },
+      true,
+    );
+
+    await mgr.handleEvent({
+      type: 'message.updated',
+      properties: {
+        info: {
+          sessionID: 'compaction-sess',
+          agent: 'compaction', // NOT a known omos built-in agent
+          providerID: 'openai',
+          modelID: 'gpt-5.4',
+          error: { message: 'rate limit exceeded' },
+        },
+      },
+    });
+
+    // compaction's model (openai/gpt-5.4) matches orchestrator's chain
+    // → should fall back to the next untried model in that chain
+    expect(mocks.promptAsync).toHaveBeenCalledTimes(1);
+    const call = mocks.promptAsync.mock.calls[0] as [
+      { body: { model: { providerID: string; modelID: string } } },
+    ];
+    expect(call[0].body.model.providerID).toBe('new-api');
+    expect(call[0].body.model.modelID).toBe('glm-5.2');
+  });
 });

src/hooks/foreground-fallback/index.ts

@@ -15,6 +15,7 @@
  */
 
 import type { PluginInput } from '@opencode-ai/plugin';
+import { ALL_AGENT_NAMES } from '../../config/constants';
 import { log } from '../../utils/logger';
 import {
   abortSessionWithTimeout,
@@ -236,17 +237,37 @@ export class ForegroundFallbackManager {
         this.sessionTried.set(sessionID, new Set());
       }
       // biome-ignore lint/style/noNonNullAssertion: We just set this above
-      const tried = this.sessionTried.get(sessionID)!;
+      let tried = this.sessionTried.get(sessionID)!;
       if (currentModel) tried.add(currentModel);
 
-      const nextModel = chain.find((m) => !tried.has(m));
+      let nextModel = chain.find((m) => !tried.has(m));
       if (!nextModel) {
-        log('[foreground-fallback] fallback chain exhausted', {
-          sessionID,
-          agentName,
-          tried: [...tried],
-        });
-        return;
+        if (chain.length > 1) {
+          // Chain exhausted but we have fallbacks: reset tried set and
+          // stick to the deepest fallback model so we stop re-trying the
+          // dead primary model on every subsequent message.
+          const primary = chain[0];
+          const stickyFallback = chain[chain.length - 1];
+          log('[foreground-fallback] resetting tried set for re-fallback', {
+            sessionID,
+            agentName,
+            currentModel,
+            prevTried: [...tried],
+            nextModel: stickyFallback,
+          });
+          tried = new Set();
+          if (primary) tried.add(primary);
+          if (currentModel && currentModel !== primary) tried.add(currentModel);
+          this.sessionTried.set(sessionID, tried);
+          nextModel = stickyFallback;
+        } else {
+          log('[foreground-fallback] fallback chain exhausted', {
+            sessionID,
+            agentName,
+            tried: [...tried],
+          });
+          return;
+        }
       }
       tried.add(nextModel);
 
@@ -352,9 +373,18 @@ export class ForegroundFallbackManager {
     currentModel: string | undefined,
   ): string[] {
     if (agentName) {
-      // Agent is known: use its chain exactly, or no chain at all.
-      // Never fall through to cross-agent chains when the agent is identified.
-      return this.chains[agentName] ?? [];
+      // Agent is known: use its chain exactly if configured.
+      const chain = this.chains[agentName];
+      if (chain) return chain;
+      // Known omos built-in agent (oracle, librarian, …) without a
+      // configured chain: keep isolation — do NOT bleed into other
+      // agents' chains (preserves the cross-agent isolation contract
+      // from PR #199).
+      if ((ALL_AGENT_NAMES as readonly string[]).includes(agentName)) return [];
+      // Unknown agent (e.g. OpenCode built-in "compaction" or "title"
+      // that don't appear in the user preset): fall through to
+      // model-matching so they can inherit a chain from a configured
+      // agent that shares their model.
     }
 
     // Agent unknown: try to infer from the current model.

src/hooks/task-session-manager/index.ts

@@ -12,6 +12,7 @@ import {
 } from '../../utils';
 import { isRecord as isObjectRecord } from '../../utils/guards';
 import { log } from '../../utils/logger';
+import { isRateLimitError } from '../foreground-fallback/index';
 import type { MessagePart, MessageWithParts } from '../types';
 
 interface TaskArgs {
@@ -745,7 +746,17 @@ export function createTaskSessionManagerHook(
         const sessionId =
           input.event.properties?.info?.id ?? input.event.properties?.sessionID;
         if (sessionId && options.shouldManageSession(sessionId)) {
-          terminalJobsInjectedByParent.delete(sessionId);
+          // Only clear injected terminal jobs for fatal errors.
+          // Rate-limit errors are recovered by ForegroundFallbackManager
+          // (abort + reprompt with fallback model); clearing the injected
+          // job state here would make the orchestrator lose track of
+          // completed background tasks and unable to dispatch follow-ups.
+          const props = input.event.properties as
+            | { error?: unknown }
+            | undefined;
+          if (!props?.error || !isRateLimitError(props.error)) {
+            terminalJobsInjectedByParent.delete(sessionId);
+          }
         }
 
         return;