feat(resilience): Provider Failover to Anthropic-compatible backup endpoints - #1334
feat(resilience): Provider Failover to Anthropic-compatible backup endpoints#1334pedramamini wants to merge 1 commit into
Conversation
…dpoints Agent Resilience answers an upstream failure by waiting and resending to the same provider. That is right for a 60-second "Overloaded" blip, but poor for plan quota exhaustion: the agent idles until the reset window, which can be hours, and long-running autonomous work stalls with it. This adds the other half. An agent can carry an ordered list of Anthropic-compatible backup endpoints (local vLLM/Ollama, Z.AI, an enterprise proxy, or a second account). When resilience decides an error is retryable and failover is armed, Maestro swaps the endpoint's env vars into the next spawn after a 3s handover instead of waiting out the clock. Endpoints are plain env-var bundles because the claude CLI already reads its base URL and token from the environment, so this stays infrastructure-agnostic: no new API client, just a different env on the spawn. Each endpoint also carries an optional model override, since backup providers publish their own model ids and carrying the primary's across would trade a quota error for a 404. The swap is applied in the main process at the single spawn IPC choke point rather than threaded through the ~20 renderer call sites that build spawn payloads, so Auto Run, Cue, tab naming and background synopsis inherit it for free. Runtime pin state is in-memory only, mirroring the retry engine's rule that a closed app should not keep routing prompts to a backup provider. Fail-back is a lazy probe: after a configurable dwell time (default 60m) the next turn re-tests the primary. If it is still down, the normal failover path moves the agent off again. Probing on a timer would burn quota on an idle agent. Off by default and configured per agent in the edit dialog. Closes #139
📝 WalkthroughWalkthroughThis PR adds configurable Anthropic-compatible backup endpoints. It persists failover settings, switches process overlays during retries, supports model overrides, tracks endpoint state, and periodically re-probes the primary provider. ChangesProvider failover
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant RetryStore
participant FailoverStore
participant MainProcess
participant ClaudeProcess
RetryStore->>FailoverStore: select next available endpoint
FailoverStore->>MainProcess: set environment and model overlay
MainProcess-->>FailoverStore: overlay update completes
FailoverStore-->>RetryStore: endpoint switch completes
RetryStore->>ClaudeProcess: spawn retry
ClaudeProcess-->>RetryStore: success or failure
RetryStore->>FailoverStore: probe primary after dwell time
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryAdds configurable Anthropic-compatible provider failover across persisted agent configuration, renderer retry orchestration, IPC, and main-process spawning.
Confidence Score: 4/5The provider-routing and credential-handling defects need to be fixed before merging because saved configuration can be ignored and primary credentials can reach backup endpoints. Active overlays survive configuration changes, model-less backups inherit primary model identifiers, and URL-only backups preserve primary credentials in the spawned CLI environment. Files Needing Attention: src/renderer/hooks/session/useSessionLifecycle.ts, src/main/ipc/handlers/process.ts, src/shared/providerFailover.ts
|
| Filename | Overview |
|---|---|
| src/shared/providerFailover.ts | Adds failover models, selection, timing, environment resolution, and validation, but URL-only endpoints retain primary credentials. |
| src/main/ipc/handlers/process.ts | Applies failover overlays at the spawn choke point, but does not clear a preexisting primary model when the backup has no override. |
| src/renderer/stores/failoverStore.ts | Manages runtime endpoint pins and fail-back, including a cleanup operation that is not connected to configuration lifecycle changes. |
| src/renderer/stores/retryStore.ts | Routes eligible retries through a short failover handover while retaining normal waits after endpoint exhaustion. |
| src/renderer/hooks/session/useSessionLifecycle.ts | Persists failover configuration but leaves active renderer and main-process pins intact when configuration is disabled or removed. |
| src/renderer/components/NewInstanceModal/AgentFailoverSection.tsx | Adds endpoint editing, validation feedback, enablement, ordering, and fail-back controls. |
| src/main/process-manager/failover-overlay.ts | Adds an agent-keyed in-memory registry for endpoint environments and optional model overrides. |
Sequence Diagram
sequenceDiagram
participant Retry as Retry Store
participant Failover as Failover Store
participant IPC as Preload and IPC
participant Overlay as Main Overlay
participant Spawn as Process Spawn
Retry->>Failover: Retryable provider error
Failover->>IPC: Set next endpoint env and model
IPC->>Overlay: Pin backup by agent id
Retry->>Spawn: Resend queued turn
Spawn->>Overlay: Read pinned env and model
Spawn->>Spawn: Merge overlay and launch CLI
Reviews (1): Last reviewed commit: "feat(resilience): Provider Failover to A..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| maestroPPath: undefined, | ||
| maestroPMode: undefined, | ||
| // Endpoint env carries provider-specific base URLs and tokens. | ||
| failoverConfig: undefined, |
There was a problem hiding this comment.
When an agent is pinned to a backup and the user changes its provider, disables failover, or removes its endpoints, this save path updates only failoverConfig without clearing the renderer state or main-process overlay. Subsequent prompts therefore continue using the former backup URL, credential, and model despite the saved configuration.
Knowledge Base Used:
| const failoverModel = getFailoverModel(baseSessionId); | ||
| if (failoverModel) config.sessionCustomModel = failoverModel; |
There was a problem hiding this comment.
When the agent has a custom model and the selected backup omits its optional model override, this truthy guard leaves the primary sessionCustomModel in the spawn payload. The backup then receives a primary-provider model identifier and rejects the retried turn as an unknown model.
Knowledge Base Used: Process Manager
| if (!endpoint) return baseEnv; | ||
| const merged: Record<string, string> = { ...(baseEnv ?? {}) }; | ||
| for (const [key, value] of Object.entries(endpoint.env ?? {})) { | ||
| // Skip blank values so a half-filled row in the editor can't clobber a | ||
| // working var with an empty string. |
There was a problem hiding this comment.
When a valid backup specifies only ANTHROPIC_BASE_URL, this merge preserves any primary ANTHROPIC_AUTH_TOKEN or ANTHROPIC_API_KEY inherited from the session, global shell, or process environment. The spawned CLI therefore presents the primary credential to the backup service, exposing it to another provider or causing authentication failure.
How this was verified: The URL-only configuration was traced through overlay merging into the inherited local and SSH spawn environments.
Knowledge Base Used:
There was a problem hiding this comment.
Pull request overview
This PR extends Maestro Agent Resilience with an opt-in provider failover mechanism that can swap an agent onto an ordered list of Anthropic-compatible backup endpoints by applying an in-memory env and model overlay at the main-process spawn choke point.
Changes:
- Add shared failover config/state model and pure selection logic (endpoint rotation, fail-back timing, env resolution helpers).
- Integrate failover into the retry engine and dispatch path, with a renderer store that pushes overlays to main before resending.
- Add main-process overlay registry and apply it during spawn; add edit-UI to configure endpoints and persist config on the session.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/shared/types.ts | Adds failoverConfig to session info and re-exports failover types. |
| src/shared/providerFailover.ts | New shared, pure failover model and selection/validation helpers. |
| src/renderer/types/index.ts | Plumbs failover types and session field into renderer type surface. |
| src/renderer/stores/retryStore.ts | Schedules retries with optional failover handover before resend. |
| src/renderer/stores/failoverStore.ts | New renderer runtime store to manage active endpoint pinning and IPC overlay writes. |
| src/renderer/stores/agentStore.ts | Adds lazy fail-back probe before spawning the next turn. |
| src/renderer/hooks/session/useSessionLifecycle.ts | Threads failoverConfig through session updates and clears it on provider switch. |
| src/renderer/global.d.ts | Adds window.maestro.process.setFailoverOverlay API typing. |
| src/renderer/components/NewInstanceModal/types.ts | Extends modal save props to include failoverConfig. |
| src/renderer/components/NewInstanceModal/EditAgentModal.tsx | Loads, edits, and saves failoverConfig via new UI section. |
| src/renderer/components/NewInstanceModal/AgentFailoverSection.tsx | New UI to manage ordered endpoints, env vars, and model override. |
| src/renderer/components/AppModals/AppSessionModals.tsx | Plumbs new save arg through session modals props. |
| src/main/process-manager/failover-overlay.ts | New in-memory overlay registry in main for env and model overrides. |
| src/main/preload/process.ts | Exposes process:setFailoverOverlay via preload API. |
| src/main/ipc/handlers/process.ts | Applies failover env and model overlay during spawn; adds IPC handler to set overlay. |
| src/tests/shared/providerFailover.test.ts | Unit tests for selection logic, env resolution, and validation. |
| src/tests/renderer/stores/failoverStore.test.ts | Renderer store tests, including retry integration ordering guarantees. |
| src/tests/renderer/components/NewInstanceModal/EditAgentModal.test.tsx | Save-chain round-trip test for existing failover config. |
| src/tests/main/process-manager/failover-overlay.test.ts | Main overlay registry unit tests. |
| src/tests/main/ipc/handlers/process.test.ts | Asserts new IPC handler registration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const failoverEnv = getFailoverOverlay(baseSessionId); | ||
| if (failoverEnv) { | ||
| config.sessionCustomEnvVars = { | ||
| ...(config.sessionCustomEnvVars ?? {}), | ||
| ...failoverEnv, | ||
| }; |
| // turn over to it instead of waiting out the primary. This is the whole value of | ||
| // the feature for token-exhaustion, where the strategy wait can be hours. We only | ||
| // DECIDE here (a pure store read); `fireRetry` performs the async switch. | ||
| const failingOver = canFailover(sessionId); |
| // Endpoint env carries provider-specific base URLs and tokens. | ||
| failoverConfig: undefined, |
| /** | ||
| * Drop all failover state (renderer + main). Used when an agent is deleted or the | ||
| * user disarms failover, so a stale pin can't outlive its config. | ||
| */ | ||
| export async function clearFailover(sessionId: string): Promise<void> { | ||
| if (!useFailoverStore.getState().states[sessionId]) return; | ||
| await pushOverlay(sessionId, null); | ||
| useFailoverStore.getState().setState(sessionId, null); | ||
| } |
| /** Drop all overlays. Used when the renderer reloads so stale pins can't leak. */ | ||
| export function clearAllFailoverOverlays(): void { | ||
| if (overlays.size === 0) return; | ||
| logger.info('Cleared all failover overlays', LOG_CONTEXT, { count: overlays.size }); | ||
| overlays.clear(); | ||
| modelOverrides.clear(); | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
src/__tests__/renderer/components/NewInstanceModal/EditAgentModal.test.tsx (1)
310-344: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for editing failover through the section UI.
This test covers the load-and-save round trip for an existing config. No test drives
AgentFailoverSectionitself: adding an endpoint, toggling the enable checkbox, or changing "Return to primary after". The controlled-input defect flagged onsrc/renderer/components/NewInstanceModal/AgentFailoverSection.tsxLines 251-268 would surface in such a test.Do you want me to generate a test that adds an endpoint and edits the dwell time through the editor?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/renderer/components/NewInstanceModal/EditAgentModal.test.tsx` around lines 310 - 344, Add a test that exercises AgentFailoverSection through EditAgentModal by adding an endpoint, toggling the enable control, and changing the “Return to primary after” value before saving. Assert the edited failoverConfig is passed through onSave, covering the controlled-input behavior rather than only the existing-config round trip.src/renderer/hooks/session/useSessionLifecycle.ts (1)
77-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider an options object for
handleSaveEditAgent.The signature now carries 17 positional parameters, and all of them after
nameare optional. The same list is duplicated insrc/renderer/components/NewInstanceModal/types.ts,src/renderer/components/AppModals/AppSessionModals.tsx, and the caller inEditAgentModal.tsx. A single omitted or misordered argument silently disarms failover, and the new test asserts by numeric index (onSave.mock.calls[0][16]), which will not catch an insertion in the middle.A single options object removes the positional coupling. This change touches call sites outside the current scope, so treat it as a follow-up rather than part of this PR.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/hooks/session/useSessionLifecycle.ts` around lines 77 - 101, Refactor handleSaveEditAgent to accept a single options object containing the currently optional parameters after sessionId and name, eliminating positional coupling and duplicated signatures across its declaration and callers. Update the related type declarations and EditAgentModal call site consistently, preserving all existing values and behavior, including failover configuration. Treat this as follow-up work outside the current PR scope.src/__tests__/main/ipc/handlers/process.test.ts (1)
360-360: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a functional test for the failover env merge in
process:spawn.This change verifies handler registration only. Add a test that registers an overlay via
setFailoverOverlay, then callsprocess:spawnfor the same bare session id, and asserts thatProcessManager.spawnreceives the overlay's env merged intocustomEnvVarsand the overlay's model assessionCustomModel. This test file already has the mocking infrastructure forprocess:spawn, so the added cost is small relative to protecting the core spawn-integration contract.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/main/ipc/handlers/process.test.ts` at line 360, Add a functional test in the process IPC handler tests that uses the existing process:spawn mocks to register a failover overlay through setFailoverOverlay, invoke process:spawn with the same bare session ID, and assert ProcessManager.spawn receives the overlay environment merged into customEnvVars and the overlay model as sessionCustomModel.src/__tests__/renderer/stores/failoverStore.test.ts (1)
165-188: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for
maybeReturnToPrimaryfailure during dispatch.Existing tests cover
setFailoverOverlayrejecting insideswitchToNextEndpoint(Lines 143-150) and insidefireRetry(Lines 223-233), but not insidemaybeReturnToPrimarywhen called fromagentStore.processQueuedItem. Add a case wheresetFailoverOverlayrejects, the dwell time has elapsed, andprocessQueuedItemis invoked directly; assert thatprocessQueuedItemstill resolves and still attempts the spawn. This would have caught the unguardedawait maybeReturnToPrimary(sessionId)insrc/renderer/stores/agentStore.ts(Lines 377-383).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/__tests__/renderer/stores/failoverStore.test.ts` around lines 165 - 188, Add a failover dispatch test covering a rejected setFailoverOverlay during maybeReturnToPrimary: configure an elapsed return-to-primary dwell, invoke processQueuedItem directly, mock setFailoverOverlay to reject, and assert processQueuedItem still resolves and attempts the spawn.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/process-manager/failover-overlay.ts`:
- Around line 71-77: Wire the documented teardown callers for the failover
overlay cleanup functions: in src/main/process-manager/failover-overlay.ts lines
71-77, connect clearAllFailoverOverlays to the main-process renderer-reload
reset path; in src/renderer/stores/failoverStore.ts lines 161-169, invoke the
renderer-side cleanup when an agent is deleted or disarmed. Ensure both paths
clear stale routing and pins after configuration changes.
In `@src/renderer/components/NewInstanceModal/AgentFailoverSection.tsx`:
- Around line 251-268: Update the return-to-primary input near the returnMinutes
value and onChange handler to maintain its raw text in local state, allowing
empty and intermediate values such as partial numeric input without immediately
restoring DEFAULT_RETURN_TO_PRIMARY_MINUTES. Commit the parsed value to emit
only when the text represents a valid positive number, and synchronize the local
text with external returnMinutes changes as needed.
In `@src/renderer/stores/agentStore.ts`:
- Around line 377-383: Wrap the await maybeReturnToPrimary(sessionId) call in
its own try/catch block to prevent a rejection from aborting the entire turn.
The catch block should allow the agent to continue sending the message on the
current endpoint instead of propagating the error. Follow the same isolation
pattern used for the equivalent failover call in retryStore.fireRetry (lines
398-407 of src/renderer/stores/retryStore.ts) to ensure that a failed overlay
write degrades gracefully rather than preventing message dispatch.
In `@src/shared/providerFailover.ts`:
- Around line 148-178: Update resolveFailoverEnv to trim each endpoint.env value
before merging it into the result, and skip values that become empty after
trimming. Preserve baseEnv for undefined endpoints and ensure whitespace-only
overrides cannot replace existing working variables.
---
Nitpick comments:
In `@src/__tests__/main/ipc/handlers/process.test.ts`:
- Line 360: Add a functional test in the process IPC handler tests that uses the
existing process:spawn mocks to register a failover overlay through
setFailoverOverlay, invoke process:spawn with the same bare session ID, and
assert ProcessManager.spawn receives the overlay environment merged into
customEnvVars and the overlay model as sessionCustomModel.
In `@src/__tests__/renderer/components/NewInstanceModal/EditAgentModal.test.tsx`:
- Around line 310-344: Add a test that exercises AgentFailoverSection through
EditAgentModal by adding an endpoint, toggling the enable control, and changing
the “Return to primary after” value before saving. Assert the edited
failoverConfig is passed through onSave, covering the controlled-input behavior
rather than only the existing-config round trip.
In `@src/__tests__/renderer/stores/failoverStore.test.ts`:
- Around line 165-188: Add a failover dispatch test covering a rejected
setFailoverOverlay during maybeReturnToPrimary: configure an elapsed
return-to-primary dwell, invoke processQueuedItem directly, mock
setFailoverOverlay to reject, and assert processQueuedItem still resolves and
attempts the spawn.
In `@src/renderer/hooks/session/useSessionLifecycle.ts`:
- Around line 77-101: Refactor handleSaveEditAgent to accept a single options
object containing the currently optional parameters after sessionId and name,
eliminating positional coupling and duplicated signatures across its declaration
and callers. Update the related type declarations and EditAgentModal call site
consistently, preserving all existing values and behavior, including failover
configuration. Treat this as follow-up work outside the current PR scope.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b5f7ba9-1975-45ec-b2b5-0c25cfe59991
📒 Files selected for processing (20)
src/__tests__/main/ipc/handlers/process.test.tssrc/__tests__/main/process-manager/failover-overlay.test.tssrc/__tests__/renderer/components/NewInstanceModal/EditAgentModal.test.tsxsrc/__tests__/renderer/stores/failoverStore.test.tssrc/__tests__/shared/providerFailover.test.tssrc/main/ipc/handlers/process.tssrc/main/preload/process.tssrc/main/process-manager/failover-overlay.tssrc/renderer/components/AppModals/AppSessionModals.tsxsrc/renderer/components/NewInstanceModal/AgentFailoverSection.tsxsrc/renderer/components/NewInstanceModal/EditAgentModal.tsxsrc/renderer/components/NewInstanceModal/types.tssrc/renderer/global.d.tssrc/renderer/hooks/session/useSessionLifecycle.tssrc/renderer/stores/agentStore.tssrc/renderer/stores/failoverStore.tssrc/renderer/stores/retryStore.tssrc/renderer/types/index.tssrc/shared/providerFailover.tssrc/shared/types.ts
| /** Drop all overlays. Used when the renderer reloads so stale pins can't leak. */ | ||
| export function clearAllFailoverOverlays(): void { | ||
| if (overlays.size === 0) return; | ||
| logger.info('Cleared all failover overlays', LOG_CONTEXT, { count: overlays.size }); | ||
| overlays.clear(); | ||
| modelOverrides.clear(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the cleanup hooks documented in failover-overlay.ts and failoverStore.ts are wired up.
rg -n 'clearAllFailoverOverlays' --type=ts
rg -n 'clearFailover\(' --type=ts -g '!**/failoverStore.ts' -g '!**/*.test.ts'Repository: RunMaestro/Maestro
Length of output: 156
🏁 Script executed:
#!/bin/bash
set -u
echo "Tracked failover-related TypeScript files:"
git ls-files -z '*.ts' '*.tsx' | tr '\0' '\n' | rg -n 'failover|overlay|process-manager|stores|index\.ts' || true
echo
echo "Search identifiers with broad file matching:"
rg -n 'clearAllFailoverOverlays|clearFailover|pushOverlay|modelOverrides|overlays\.clear|disarm|delete .*agent|onRenderer.*reload|renderer.*reload|restarted|ready|main\.ready' --glob '!**/node_modules/**' || trueRepository: RunMaestro/Maestro
Length of output: 12092
Wire up the documented failover-overlay teardown paths.
The cleanup functions are documented responsibilities, but the callers for the renderer-reload main-process reset and the agent deletion/disarm renderer-side reset are not shown. Add those call sites so stale failover routing and pins cannot outlive their configuration.
📍 Affects 2 files
src/main/process-manager/failover-overlay.ts#L71-L77(this comment)src/renderer/stores/failoverStore.ts#L161-L169
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/process-manager/failover-overlay.ts` around lines 71 - 77, Wire the
documented teardown callers for the failover overlay cleanup functions: in
src/main/process-manager/failover-overlay.ts lines 71-77, connect
clearAllFailoverOverlays to the main-process renderer-reload reset path; in
src/renderer/stores/failoverStore.ts lines 161-169, invoke the renderer-side
cleanup when an agent is deleted or disarmed. Ensure both paths clear stale
routing and pins after configuration changes.
| <input | ||
| type="number" | ||
| min={1} | ||
| value={returnMinutes} | ||
| onChange={(e) => { | ||
| const parsed = Number(e.target.value); | ||
| emit({ | ||
| returnToPrimaryMinutes: Number.isFinite(parsed) && parsed > 0 ? parsed : undefined, | ||
| }); | ||
| }} | ||
| className="w-20 px-2 py-1 text-xs rounded outline-none" | ||
| style={{ | ||
| backgroundColor: theme.colors.bgMain, | ||
| color: theme.colors.textMain, | ||
| border: `1px solid ${theme.colors.border}`, | ||
| }} | ||
| aria-label="Minutes on a backup before probing the primary again" | ||
| /> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The "Return to primary after" input cannot be edited normally.
The input is controlled by returnMinutes, which falls back to DEFAULT_RETURN_TO_PRIMARY_MINUTES on Line 51. When the user clears the field, Number('') is 0, the guard on Line 258 stores undefined, and the next render immediately shows 60 again. The user cannot clear the field to type a new value. Typing after a clear appends to 60 and produces values such as 605. Partial input such as 4e yields NaN, which also collapses to 60.
Hold the raw text in local state and commit only a valid number.
🐛 Proposed fix
+ // Raw text so the field can be empty mid-edit. `undefined` means "follow the
+ // persisted value / default".
+ const [minutesDraft, setMinutesDraft] = React.useState<string | undefined>(undefined);
+
{endpoints.length > 0 && (
<div className="flex items-center gap-2 mt-2">
<label className="text-xs" style={{ color: theme.colors.textDim }}>
Return to primary after
</label>
<input
type="number"
min={1}
- value={returnMinutes}
- onChange={(e) => {
- const parsed = Number(e.target.value);
- emit({
- returnToPrimaryMinutes: Number.isFinite(parsed) && parsed > 0 ? parsed : undefined,
- });
- }}
+ value={minutesDraft ?? String(returnMinutes)}
+ onChange={(e) => {
+ const raw = e.target.value;
+ setMinutesDraft(raw);
+ const parsed = Number(raw);
+ if (raw.trim() !== '' && Number.isFinite(parsed) && parsed > 0) {
+ emit({ returnToPrimaryMinutes: parsed });
+ }
+ }}
+ onBlur={() => setMinutesDraft(undefined)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <input | |
| type="number" | |
| min={1} | |
| value={returnMinutes} | |
| onChange={(e) => { | |
| const parsed = Number(e.target.value); | |
| emit({ | |
| returnToPrimaryMinutes: Number.isFinite(parsed) && parsed > 0 ? parsed : undefined, | |
| }); | |
| }} | |
| className="w-20 px-2 py-1 text-xs rounded outline-none" | |
| style={{ | |
| backgroundColor: theme.colors.bgMain, | |
| color: theme.colors.textMain, | |
| border: `1px solid ${theme.colors.border}`, | |
| }} | |
| aria-label="Minutes on a backup before probing the primary again" | |
| /> | |
| // Raw text so the field can be empty mid-edit. `undefined` means "follow the | |
| // persisted value / default". | |
| const [minutesDraft, setMinutesDraft] = React.useState<string | undefined>(undefined); | |
| <input | |
| type="number" | |
| min={1} | |
| value={minutesDraft ?? String(returnMinutes)} | |
| onChange={(e) => { | |
| const raw = e.target.value; | |
| setMinutesDraft(raw); | |
| const parsed = Number(raw); | |
| if (raw.trim() !== '' && Number.isFinite(parsed) && parsed > 0) { | |
| emit({ returnToPrimaryMinutes: parsed }); | |
| } | |
| }} | |
| onBlur={() => setMinutesDraft(undefined)} | |
| className="w-20 px-2 py-1 text-xs rounded outline-none" | |
| style={{ | |
| backgroundColor: theme.colors.bgMain, | |
| color: theme.colors.textMain, | |
| border: `1px solid ${theme.colors.border}`, | |
| }} | |
| aria-label="Minutes on a backup before probing the primary again" | |
| /> |
🧰 Tools
🪛 React Doctor (0.9.1)
[error] 256-256: Coercing an input's value with this parse stores 0 for a cleared field and NaN for partial input, which then flows into state or a request body; guard the empty and NaN cases (for example value ? Number(value) : undefined) before using it.
Guard Number(e.target.value) / parseInt(e.target.value) against empty and NaN before storing it. Number('') is 0 and Number('abc') is NaN, both of which silently ship a wrong value.
(no-unguarded-numeric-input-parse)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/components/NewInstanceModal/AgentFailoverSection.tsx` around
lines 251 - 268, Update the return-to-primary input near the returnMinutes value
and onChange handler to maintain its raw text in local state, allowing empty and
intermediate values such as partial numeric input without immediately restoring
DEFAULT_RETURN_TO_PRIMARY_MINUTES. Commit the parsed value to emit only when the
text represents a valid positive number, and synchronize the local text with
external returnMinutes changes as needed.
Source: Linters/SAST tools
| // Provider Failover: lazy fail-back probe. If this agent has sat on a backup | ||
| // endpoint past its dwell time, move it back to the primary now so THIS turn | ||
| // re-tests the real provider. Awaited so the swap lands in main before the | ||
| // spawn below reads it. If the primary is still down, the resulting error | ||
| // sends the agent straight back to a backup. | ||
| await maybeReturnToPrimary(sessionId); | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Isolate the fail-back probe so it cannot abort the whole turn.
await maybeReturnToPrimary(sessionId) runs outside the try block that starts on the next line. If this call rejects (the underlying setFailoverOverlay IPC call can fail), processQueuedItem rejects immediately, before the agent gets a chance to send the message on any endpoint. None of the existing error-recovery logic in the catch block below (resetting tab state, appending an error log entry) runs either, since the rejection happens before entering that try.
The equivalent failover-switch call in retryStore.fireRetry (Lines 398-407 of src/renderer/stores/retryStore.ts) is wrapped in its own try/catch specifically to avoid this: a failed overlay write must degrade to "stay on the current endpoint," not abort the dispatch. Apply the same pattern here.
🐛 Proposed fix
- // Provider Failover: lazy fail-back probe. If this agent has sat on a backup
- // endpoint past its dwell time, move it back to the primary now so THIS turn
- // re-tests the real provider. Awaited so the swap lands in main before the
- // spawn below reads it. If the primary is still down, the resulting error
- // sends the agent straight back to a backup.
- await maybeReturnToPrimary(sessionId);
+ // Provider Failover: lazy fail-back probe. If this agent has sat on a backup
+ // endpoint past its dwell time, move it back to the primary now so THIS turn
+ // re-tests the real provider. Awaited so the swap lands in main before the
+ // spawn below reads it. If the primary is still down, the resulting error
+ // sends the agent straight back to a backup.
+ //
+ // Contained in its own try: a failed overlay write must degrade to "stay on
+ // the current endpoint", never abort this turn before a spawn is attempted.
+ try {
+ await maybeReturnToPrimary(sessionId);
+ } catch (error) {
+ logger.error('[processQueuedItem] Failed to probe primary provider:', undefined, error);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Provider Failover: lazy fail-back probe. If this agent has sat on a backup | |
| // endpoint past its dwell time, move it back to the primary now so THIS turn | |
| // re-tests the real provider. Awaited so the swap lands in main before the | |
| // spawn below reads it. If the primary is still down, the resulting error | |
| // sends the agent straight back to a backup. | |
| await maybeReturnToPrimary(sessionId); | |
| // Provider Failover: lazy fail-back probe. If this agent has sat on a backup | |
| // endpoint past its dwell time, move it back to the primary now so THIS turn | |
| // re-tests the real provider. Awaited so the swap lands in main before the | |
| // spawn below reads it. If the primary is still down, the resulting error | |
| // sends the agent straight back to a backup. | |
| // | |
| // Contained in its own try: a failed overlay write must degrade to "stay on | |
| // the current endpoint", never abort this turn before a spawn is attempted. | |
| try { | |
| await maybeReturnToPrimary(sessionId); | |
| } catch (error) { | |
| logger.error('[processQueuedItem] Failed to probe primary provider:', undefined, error); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/stores/agentStore.ts` around lines 377 - 383, Wrap the await
maybeReturnToPrimary(sessionId) call in its own try/catch block to prevent a
rejection from aborting the entire turn. The catch block should allow the agent
to continue sending the message on the current endpoint instead of propagating
the error. Follow the same isolation pattern used for the equivalent failover
call in retryStore.fireRetry (lines 398-407 of
src/renderer/stores/retryStore.ts) to ensure that a failed overlay write
degrades gracefully rather than preventing message dispatch.
| export function resolveFailoverEnv( | ||
| baseEnv: Record<string, string> | undefined, | ||
| endpoint: FailoverEndpoint | undefined | ||
| ): Record<string, string> | undefined { | ||
| if (!endpoint) return baseEnv; | ||
| const merged: Record<string, string> = { ...(baseEnv ?? {}) }; | ||
| for (const [key, value] of Object.entries(endpoint.env ?? {})) { | ||
| // Skip blank values so a half-filled row in the editor can't clobber a | ||
| // working var with an empty string. | ||
| if (value !== '') merged[key] = value; | ||
| } | ||
| return merged; | ||
| } | ||
|
|
||
| /** | ||
| * Validate an endpoint for the agent editor. Returns a human-readable problem, or | ||
| * null when the endpoint is usable. | ||
| */ | ||
| export function validateEndpoint(endpoint: FailoverEndpoint): string | null { | ||
| if (!endpoint.label.trim()) return 'Name is required.'; | ||
| const keys = Object.keys(endpoint.env ?? {}).filter((k) => k.trim() !== ''); | ||
| if (keys.length === 0) return 'Add at least one environment variable.'; | ||
| if (!keys.some((k) => k === 'ANTHROPIC_BASE_URL')) { | ||
| return 'Set ANTHROPIC_BASE_URL so the agent points at this endpoint.'; | ||
| } | ||
| const baseUrl = endpoint.env.ANTHROPIC_BASE_URL?.trim() ?? ''; | ||
| if (!/^https?:\/\//i.test(baseUrl)) { | ||
| return 'ANTHROPIC_BASE_URL must start with http:// or https://.'; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Trim endpoint env values before they reach the spawn.
validateEndpoint trims the base URL only for the regex check on Line 173. resolveFailoverEnv copies the raw value on Line 157 and skips only the exact empty string. A user who pastes " https://api.z.ai/api/anthropic " passes validation, but the spawn receives the untrimmed value, and the CLI then builds a malformed URL. The same applies to a whitespace-only token, which overwrites a working primary token with blanks.
Trim values in resolveFailoverEnv and treat a whitespace-only value as blank.
🐛 Proposed fix
const merged: Record<string, string> = { ...(baseEnv ?? {}) };
for (const [key, value] of Object.entries(endpoint.env ?? {})) {
// Skip blank values so a half-filled row in the editor can't clobber a
// working var with an empty string.
- if (value !== '') merged[key] = value;
+ const trimmed = value.trim();
+ if (trimmed !== '') merged[key] = trimmed;
}
return merged;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function resolveFailoverEnv( | |
| baseEnv: Record<string, string> | undefined, | |
| endpoint: FailoverEndpoint | undefined | |
| ): Record<string, string> | undefined { | |
| if (!endpoint) return baseEnv; | |
| const merged: Record<string, string> = { ...(baseEnv ?? {}) }; | |
| for (const [key, value] of Object.entries(endpoint.env ?? {})) { | |
| // Skip blank values so a half-filled row in the editor can't clobber a | |
| // working var with an empty string. | |
| if (value !== '') merged[key] = value; | |
| } | |
| return merged; | |
| } | |
| /** | |
| * Validate an endpoint for the agent editor. Returns a human-readable problem, or | |
| * null when the endpoint is usable. | |
| */ | |
| export function validateEndpoint(endpoint: FailoverEndpoint): string | null { | |
| if (!endpoint.label.trim()) return 'Name is required.'; | |
| const keys = Object.keys(endpoint.env ?? {}).filter((k) => k.trim() !== ''); | |
| if (keys.length === 0) return 'Add at least one environment variable.'; | |
| if (!keys.some((k) => k === 'ANTHROPIC_BASE_URL')) { | |
| return 'Set ANTHROPIC_BASE_URL so the agent points at this endpoint.'; | |
| } | |
| const baseUrl = endpoint.env.ANTHROPIC_BASE_URL?.trim() ?? ''; | |
| if (!/^https?:\/\//i.test(baseUrl)) { | |
| return 'ANTHROPIC_BASE_URL must start with http:// or https://.'; | |
| } | |
| return null; | |
| } | |
| export function resolveFailoverEnv( | |
| baseEnv: Record<string, string> | undefined, | |
| endpoint: FailoverEndpoint | undefined | |
| ): Record<string, string> | undefined { | |
| if (!endpoint) return baseEnv; | |
| const merged: Record<string, string> = { ...(baseEnv ?? {}) }; | |
| for (const [key, value] of Object.entries(endpoint.env ?? {})) { | |
| // Skip blank values so a half-filled row in the editor can't clobber a | |
| // working var with an empty string. | |
| const trimmed = value.trim(); | |
| if (trimmed !== '') merged[key] = trimmed; | |
| } | |
| return merged; | |
| } | |
| /** | |
| * Validate an endpoint for the agent editor. Returns a human-readable problem, or | |
| * null when the endpoint is usable. | |
| */ | |
| export function validateEndpoint(endpoint: FailoverEndpoint): string | null { | |
| if (!endpoint.label.trim()) return 'Name is required.'; | |
| const keys = Object.keys(endpoint.env ?? {}).filter((k) => k.trim() !== ''); | |
| if (keys.length === 0) return 'Add at least one environment variable.'; | |
| if (!keys.some((k) => k === 'ANTHROPIC_BASE_URL')) { | |
| return 'Set ANTHROPIC_BASE_URL so the agent points at this endpoint.'; | |
| } | |
| const baseUrl = endpoint.env.ANTHROPIC_BASE_URL?.trim() ?? ''; | |
| if (!/^https?:\/\//i.test(baseUrl)) { | |
| return 'ANTHROPIC_BASE_URL must start with http:// or https://.'; | |
| } | |
| return null; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/shared/providerFailover.ts` around lines 148 - 178, Update
resolveFailoverEnv to trim each endpoint.env value before merging it into the
result, and skip values that become empty after trimming. Preserve baseEnv for
undefined endpoints and ensure whitespace-only overrides cannot replace existing
working variables.
Closes #139
What this does
Agent Resilience today answers an upstream failure by waiting and resending to the same provider. That is right for a 60-second "Overloaded" blip, but poor for plan quota exhaustion: the agent idles until the reset window (potentially hours) and any long-running autonomous work stalls with it.
This adds the other half. An agent can carry an ordered list of Anthropic-compatible backup endpoints (local vLLM/Ollama, Z.AI, an enterprise proxy, or simply a second account). When resilience decides an error is retryable and failover is armed, Maestro swaps the endpoint's env vars into the next spawn after a 3s handover instead of waiting out the clock.
As #139 proposed, endpoints are plain env-var bundles, because the
claudeCLI already reads its base URL and token from the environment. No new API client, no per-provider integration - just a differentenvon the spawn.Design decisions worth a look
1. Each endpoint carries an optional model override. The original proposal only swaps
ANTHROPIC_BASE_URL+ANTHROPIC_AUTH_TOKEN. In practice backup providers publish their own model ids (Z.AI wantsglm-4.6), so carrying the primary's model across would trade a quota error for an unknown-model error.2. The swap is applied in the main process at the spawn IPC choke point, not threaded through the ~20 renderer call sites that build spawn payloads. That means Auto Run, Cue, tab naming and background synopsis inherit failover for free, and the next spawn surface someone adds cannot forget it.
3. Runtime pin state is in-memory only. This mirrors the existing retry engine's rule that a closed app should not silently keep routing prompts to a backup provider. Every agent comes back on its primary after a restart. The endpoint list persists on the session as usual.
4. Fail-back is a lazy probe, not a timer. After a configurable dwell time (default 60m), the next turn re-tests the primary; if it is still down the normal failover path moves the agent off again. A background timer would burn quota on an idle agent just to discover the window had not reopened.
5. Off by default, and the checkbox only appears once an endpoint exists. Failing over changes who sees the user's prompts and what the output costs, so it should be an explicit opt-in.
Where it lives
src/shared/providerFailover.tssrc/main/process-manager/failover-overlay.tssrc/main/ipc/handlers/process.tssrc/renderer/stores/failoverStore.tssrc/renderer/stores/retryStore.tssrc/renderer/components/NewInstanceModal/AgentFailoverSection.tsxTesting
41 new tests across the three new modules, plus a round-trip test through the edit dialog so a dropped arg in the save chain cannot silently disarm the feature. Notably covered: the overlay reaching main before the resend spawns, a failed overlay write degrading to a plain retry rather than swallowing the turn, and endpoint exhaustion falling back to the normal quota wait.
Full suite green locally: 31,930 passed, 0 failed.
npm run lint, eslint and prettier all clean.Open questions for review
customEnvVarsvalue, i.e. plaintext in the store. That matches how users configureANTHROPIC_API_KEYtoday, but a dedicated endpoint list is a more obvious target and may warrant keychain storage.docs/or the agent guides, so I left this undocumented to match. Say the word and I will write up both.Summary by CodeRabbit
New Features
Tests