Skip to content

Remove automatic provider failover - #747

Merged
TheGreatAxios merged 4 commits into
mainfrom
remove-provider-fallbacks
Sep 1, 2026
Merged

Remove automatic provider failover#747
TheGreatAxios merged 4 commits into
mainfrom
remove-provider-fallbacks

Conversation

@TheGreatAxios

@TheGreatAxios TheGreatAxios commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Restrict main sessions and subagents to the explicitly selected provider and model
  • Surface safe /model guidance for terminal provider failures while retaining raw diagnostics in non-rendered observability
  • Preserve same-provider retries and keep unrelated failures distinct
  • Coordinate subagent stream settlement and keep fused, split-fleet, TUI, and exec behavior consistent

Verification

  • bun run check passes
  • 5,733 tests pass with 0 failures

@TheGreatAxios TheGreatAxios left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Self-review · clean

This PR removes implicit cross-provider failover and surfaces safe provider-failure guidance across TUI, exec, and subagent paths.

  • src/config/inference-sources.ts:160 keeps main and subagent source bundles on the explicitly selected provider/model.
  • src/tui/stream-event-map.ts:460 replaces the director-resolved raw failure reply once and resets state for later inference.
  • src/subagent/task-tool.ts:857 clears retry state so unrelated failures remain distinct.
  • tests/unit/inference-sources.test.ts:122 and src/subagent/index.test.ts:1203 pin the no-fallback and retry-state contracts.

Verdict: no blocking findings. bun run check passes with 5,729 tests and 0 failures.

@TheGreatAxios TheGreatAxios left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness review · request changes

  • Blocking — src/exec/runner.ts:137: inferenceStarted is treated as proof that every later exception is a provider failure. Since it is set before activeAgent.send() at src/exec/runner.ts:774, a director or context-store exception such as disk full is replaced with the provider-switch message instead of remaining distinct. Please classify an observed provider/OAuth failure rather than all post-start exceptions, and retain a regression test for a non-provider send failure.
  • Blocking — src/tui/runner.ts:2144: every non-abort agentProxy.send() rejection is surfaced as a provider failure. For example, a rebuild failure rethrown at src/tui/runner.ts:1804 occurs before inference but still tells the operator to switch providers. Please preserve unrelated rebuild/director errors and add a test alongside the provider-failure path.
  • Should-fix — src/subagent/agent-fleet.ts:789: split-fleet failures forward the ID-only message created at src/subagent/run.ts:1119. With provider ID codex/work and display name Codex, wait_agents says codex/work Provider failed... while fused task, TUI, and exec say Codex Provider failed.... Please apply the configured display label and pin that case in src/subagent/run-resolved-provider-failure.test.ts:152.

@TheGreatAxios TheGreatAxios left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Architecture review · revise

The selected-source invariant is owned at the right boundary: src/config/inference-sources.ts:159 builds one source for both main and subagent sessions, and same-provider retry remains runtime-owned.

  • High: src/subagent/run.ts:1117 reads stream-derived terminalProviderDiagnostic immediately after agent.send() resolves. The stream consumer can lag the send promise, so DefaultDirector’s raw resolved reply can be accepted as a successful report before inference.error is observed. The retained-worker path has the same race at src/subagent/run.ts:1055.
  • High: src/subagent/agent-fleet.ts:784 sanitizes only branded resolved failures and recognized auth errors. The fused path tracks inference.error at src/subagent/task-tool.ts:857, but the split fleet path does not. A provider failure that emits inference.error and then rejects can therefore expose its diagnostic through wait_agents.
  • Medium: src/tui/runner.ts:2157 attributes send failure to the current mutable provider, which live switching replaces at src/tui/runner.ts:2376. An in-flight request that rejects after /model switches can blame the newly selected provider.
  • Medium: src/exec/runner.ts:137 classifies every exception after inference starts as a provider failure, hiding local director, context, stream, or teardown failures behind misleading provider copy.

Provider-failure settlement should produce one structured outcome at the run boundary, carrying immutable attempt identity and the raw diagnostic. Entry points can then expose the shared safe message without duplicating event-state machines.

@TheGreatAxios TheGreatAxios left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hygiene review · request changes

Should fix:

  • src/subagent/task-tool.ts:1145 and src/subagent/agent-fleet.ts:795 persist raw provider diagnostics through sessions.fail(). src/subagent/session-store.ts:915 converts that value into a transcript report, and src/tui/runner-host.ts:185 renders it verbatim. Keep diagnostics in non-rendered observability and use safe text for transcript entries.
  • src/session/run-sink.test.ts:177 and src/session/run-sink.test.ts:206 retain model/provider-switch fixtures under “retry” terminology, while docs/TELEMETRY.md:172 implies a retry can require unknown-provider attribution. Make these fixtures describe actual same-provider retry behavior or explicitly defensive mismatched attribution.
  • src/tui/runner.ts:1510 and src/exec/runner.ts:617 retain dead multi-candidate buildInitialSourceFallback() machinery although src/config/inference-sources.ts:160 now guarantees exactly one selected source. Remove the unreachable fallback constructors and branches.

Optional cleanup:

  • src/subagent/agent-fleet.ts:789 uses provider ID while fused task uses the configured display label; use one presentation path.
  • src/subagent/task-tool.ts:439 recognizes provider failure by duplicating the exact English suffix; carry structured failure metadata or centralize the predicate.
  • src/subagent/run-resolved-provider-failure.test.ts:41 leaks temporary directories and uses a fixed sleep; add cleanup and deterministic synchronization.

@TheGreatAxios TheGreatAxios left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleanup review · clean

This follow-up resolves the correctness, architecture, and hygiene review findings without restoring cross-provider failover.

  • src/exec/runner.ts:140 classifies observed/typed provider failures precisely while preserving unrelated post-start errors.
  • src/tui/runner.ts:600 uses immutable attempt identity and keeps unrelated send/rebuild failures distinct.
  • src/subagent/run-event-settlement.ts:19 coordinates stream settlement deterministically for initial and retained-worker replies.
  • src/subagent/agent-fleet.ts:808 applies configured display labels, while src/subagent/agent-fleet.ts:938 carries structured provider-failure metadata instead of matching English text.
  • src/inference-error-message.ts:95 centralizes safe provider presentation and avoids duplicated Provider suffixes.

Verdict: no blocking findings remain. Independent verification passes: 217 focused tests, 5,733 full-suite tests, git diff --check, and bun run check.

@TheGreatAxios
TheGreatAxios force-pushed the remove-provider-fallbacks branch from 1ad4b55 to 73098c4 Compare September 1, 2026 05:28
Auth and OAuth-refresh send failures were still suggesting /model.
Keep that copy on login, drop the fused task coverage after spawn/wait
became the only path, and document the no-failover behavior.
@TheGreatAxios
TheGreatAxios force-pushed the remove-provider-fallbacks branch from ec0c9b7 to c8862b8 Compare September 1, 2026 06:21
@TheGreatAxios
TheGreatAxios enabled auto-merge (squash) September 1, 2026 06:22
@TheGreatAxios
TheGreatAxios merged commit 04ab6e0 into main Sep 1, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant