From 278ede9dfa2bbbd237d5b886718eb5c64832fc13 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Tue, 14 Jul 2026 15:54:44 -0700 Subject: [PATCH] fix(FN-7952): recover provider failures without retry loops Preserve authenticated CLI usage after migration, surface OAuth remediation, and use a single distinct model fallback before parking permanent failures. Keep transient credential errors retryable and confirm each OAuth expiry notification independently. Fusion-Task-Id: FN-7952 --- .../fresh-migration-provider-workflows.md | 4 +- .../app/components/OAuthReloginBanner.tsx | 6 +- .../__tests__/AuthenticationSection.test.tsx | 18 +++++ .../__tests__/OAuthReloginBanner.test.tsx | 8 +- .../sections/AuthenticationSection.tsx | 11 ++- .../src/__tests__/routes-auth.test.ts | 6 +- .../dashboard/src/__tests__/usage.test.ts | 6 +- .../src/routes/register-auth-routes.ts | 9 ++- packages/dashboard/src/usage.ts | 4 +- .../__tests__/agent-session-helpers.test.ts | 18 +++++ .../src/__tests__/executor-prompt.test.ts | 8 +- .../__tests__/fallback-model-observer.test.ts | 3 +- .../transient-error-detector.test.ts | 1 + packages/engine/src/__tests__/triage.test.ts | 59 ++++++++++++++ packages/engine/src/agent-session-helpers.ts | 13 ++++ packages/engine/src/executor.ts | 13 +++- .../engine/src/fallback-model-observer.ts | 15 +++- .../__tests__/oauth-expiry-monitor.test.ts | 78 ++++++++++++++++++- .../src/notification/notification-service.ts | 33 +++++++- .../src/notification/oauth-expiry-monitor.ts | 14 +++- packages/engine/src/pi.ts | 26 ++++++- .../engine/src/transient-error-detector.ts | 1 + packages/engine/src/triage.ts | 27 ++++++- 23 files changed, 346 insertions(+), 35 deletions(-) diff --git a/.changeset/fresh-migration-provider-workflows.md b/.changeset/fresh-migration-provider-workflows.md index a05d94fe19..02af360ba0 100644 --- a/.changeset/fresh-migration-provider-workflows.md +++ b/.changeset/fresh-migration-provider-workflows.md @@ -2,6 +2,6 @@ "@runfusion/fusion": patch --- -summary: Preserve OAuth, CLI authentication, and selected workflow behavior after PostgreSQL migration. +summary: Restore provider usage, workflow routing, and failed-task stability after PostgreSQL migration. category: fix -dev: Refreshes Anthropic OAuth canonically, de-duplicates CLI auth rows, parses zero-use Grok billing, and forwards workflow context. +dev: Refreshes migrated OAuth, surfaces re-auth failures, repairs fallback selection and Grok billing, and parks blocked retries. diff --git a/packages/dashboard/app/components/OAuthReloginBanner.tsx b/packages/dashboard/app/components/OAuthReloginBanner.tsx index 84c56ff556..7354ea63a3 100644 --- a/packages/dashboard/app/components/OAuthReloginBanner.tsx +++ b/packages/dashboard/app/components/OAuthReloginBanner.tsx @@ -7,7 +7,7 @@ import "./OAuthReloginBanner.css"; const DISMISS_STORAGE_KEY = "fusion:oauth-relogin-dismissed"; const ANTHROPIC_SUBSCRIPTION_PROVIDER_ID = "anthropic-subscription"; -const ANTHROPIC_FALLBACK_PROVIDER_IDS = new Set(["anthropic-api-key", "claude-cli"]); +const ANTHROPIC_FALLBACK_PROVIDER_IDS = new Set(["anthropic-api-key"]); type ExpiredBannerProvider = { id: string; name: string }; @@ -22,8 +22,8 @@ function getVisibleExpiredOAuthProvidersForGlobalBanner(providers: AuthProvider[ .filter((provider) => provider.type === "oauth" && provider.expired === true) .filter((provider) => { /* - FNXC:ProviderAuth 2026-07-02-12:00: - Active Anthropic API-key or Claude CLI auth suppresses only the global urgent Subscription OAuth banner. Settings must still show `anthropic-subscription` as expired/not connected, and CLI/API-key availability must never mark subscription OAuth healthy. + FNXC:ProviderAuth 2026-07-14-15:46: + An active Anthropic API key can execute direct `anthropic/*` models, but Claude CLI authentication cannot: the execution surfaces intentionally do not reroute. Therefore only a raw API key may suppress the global subscription-expiry banner; a logged-in CLI must not hide the re-auth action while direct-model tasks fail. */ return !(provider.id === ANTHROPIC_SUBSCRIPTION_PROVIDER_ID && hasAuthenticatedAnthropicFallback); }) diff --git a/packages/dashboard/app/components/__tests__/AuthenticationSection.test.tsx b/packages/dashboard/app/components/__tests__/AuthenticationSection.test.tsx index c682369b16..2ea4670f7b 100644 --- a/packages/dashboard/app/components/__tests__/AuthenticationSection.test.tsx +++ b/packages/dashboard/app/components/__tests__/AuthenticationSection.test.tsx @@ -195,6 +195,24 @@ describe("AuthenticationSection", () => { expect(handleSaveApiKey).toHaveBeenCalledWith("anthropic-api-key"); }); + it("renders an OAuth refresh failure durably on the affected provider card", () => { + renderAuthSection([ + { + id: "anthropic-subscription", + name: "Anthropic Subscription", + authenticated: false, + type: "oauth", + expired: true, + loginError: "This OAuth session expired and could not be refreshed. Re-login to restore model access.", + }, + { id: "openai-codex", name: "OpenAI Codex", authenticated: true, type: "oauth" }, + ]); + + const subscriptionCard = screen.getByTestId("auth-provider-icon-anthropic-subscription").closest(".auth-provider-card") as HTMLElement; + expect(within(subscriptionCard).getByRole("alert")).toHaveTextContent("expired and could not be refreshed"); + expect(screen.getAllByRole("alert")).toHaveLength(1); + }); + it("keeps Anthropic OAuth logout separate from a stored API key clear action", () => { const { handleLogout, handleClearApiKey } = renderAuthSection([ { id: "anthropic-subscription", name: "Anthropic Subscription", authenticated: true, type: "oauth" }, diff --git a/packages/dashboard/app/components/__tests__/OAuthReloginBanner.test.tsx b/packages/dashboard/app/components/__tests__/OAuthReloginBanner.test.tsx index a917d53924..da9a8cf24c 100644 --- a/packages/dashboard/app/components/__tests__/OAuthReloginBanner.test.tsx +++ b/packages/dashboard/app/components/__tests__/OAuthReloginBanner.test.tsx @@ -39,7 +39,7 @@ describe("OAuthReloginBanner", () => { window.localStorage.clear(); }); - it("hides expired Anthropic subscription urgency when Claude CLI is authenticated", async () => { + it("shows expired Anthropic subscription urgency when Claude CLI is authenticated", async () => { mockFetchAuthStatus.mockResolvedValueOnce({ providers: [ { id: "anthropic-subscription", name: "Anthropic Subscription", type: "oauth", authenticated: false, expired: true }, @@ -50,10 +50,8 @@ describe("OAuthReloginBanner", () => { render(); - await waitFor(() => expect(mockFetchAuthStatus).toHaveBeenCalledTimes(1)); - expect(screen.queryByRole("status")).toBeNull(); - expect(screen.queryByText(/Re-login required: Anthropic Subscription/i)).toBeNull(); - expect(screen.queryByText(/keep agents running/i)).toBeNull(); + expect(await screen.findByRole("status")).toHaveTextContent("Re-login required: Anthropic Subscription"); + expect(screen.getByRole("status")).toHaveTextContent("keep agents running"); }); it("hides expired Anthropic subscription urgency when API key and Claude CLI are authenticated", async () => { diff --git a/packages/dashboard/app/components/settings/sections/AuthenticationSection.tsx b/packages/dashboard/app/components/settings/sections/AuthenticationSection.tsx index 7a45b306b6..1b1bf6dba2 100644 --- a/packages/dashboard/app/components/settings/sections/AuthenticationSection.tsx +++ b/packages/dashboard/app/components/settings/sections/AuthenticationSection.tsx @@ -128,6 +128,13 @@ export function AuthenticationSection({ auth }: AuthenticationSectionProps) { const showAuthenticatedGroup = authenticatedProviders.length > 0; const showAvailableGroup = unauthenticatedProviders.length > 0; const providerSupportsApiKey = (provider: AuthProvider) => provider.type === "api_key"; + /* + FNXC:ProviderAuth 2026-07-14-15:54: + Provider authentication failures must remain visible on the affected card. Toasts are transient and can fire while Settings is closed, so render the server's loginError beside the provider actions as the durable re-auth remediation. + */ + const renderProviderAuthError = (provider: AuthProvider) => provider.loginError + ? ({provider.loginError}) + : null; const renderApiKeySection = (provider: AuthProvider) => (
setApiKeyInputs((prev) => ({ ...prev, [provider.id]: e.target.value }))} disabled={authActionInProgress === provider.id}/> @@ -223,7 +230,7 @@ export function AuthenticationSection({ auth }: AuthenticationSectionProps) { {provider.authenticated && provider.keyHint && ({t("settings.authentication.key", "Key: ")}{provider.keyHint})}
- {provider.type !== "api_key" && renderAuthenticatedOAuthActions(provider)} + {provider.type !== "api_key" &&
{renderAuthenticatedOAuthActions(provider)}{renderProviderAuthError(provider)}
} {providerSupportsApiKey(provider) && renderApiKeySection(provider)}
))} @@ -243,7 +250,7 @@ export function AuthenticationSection({ auth }: AuthenticationSectionProps) { {provider.keyHint && ({t("settings.authentication.key", "Key: ")}{provider.keyHint})} - {provider.type !== "api_key" && renderAvailableOAuthActions(provider)} + {provider.type !== "api_key" &&
{renderAvailableOAuthActions(provider)}{renderProviderAuthError(provider)}
} {providerSupportsApiKey(provider) && renderApiKeySection(provider)} ))} diff --git a/packages/dashboard/src/__tests__/routes-auth.test.ts b/packages/dashboard/src/__tests__/routes-auth.test.ts index 87b12e5e67..f97be9b1b3 100644 --- a/packages/dashboard/src/__tests__/routes-auth.test.ts +++ b/packages/dashboard/src/__tests__/routes-auth.test.ts @@ -1223,7 +1223,11 @@ describe("GET /auth/status", () => { const anthropic = res.body.providers.find((p: any) => p.id === "anthropic-subscription"); const claudeCli = res.body.providers.find((p: any) => p.id === "claude-cli"); expect(authStorage.getApiKey).toHaveBeenCalledWith("anthropic-subscription"); - expect(anthropic).toMatchObject({ authenticated: false, expired: true }); + expect(anthropic).toMatchObject({ + authenticated: false, + expired: true, + loginError: "This OAuth session expired and could not be refreshed. Re-login to restore model access.", + }); expect(claudeCli).toMatchObject({ type: "cli" }); }); diff --git a/packages/dashboard/src/__tests__/usage.test.ts b/packages/dashboard/src/__tests__/usage.test.ts index c17f1e2d72..8096a5058f 100644 --- a/packages/dashboard/src/__tests__/usage.test.ts +++ b/packages/dashboard/src/__tests__/usage.test.ts @@ -3653,7 +3653,7 @@ describe("usage", () => { expect(mockRequest).toHaveBeenCalledTimes(1); }); - it("treats an omitted protobuf zero percentage as 0% for a valid weekly CLI billing period", async () => { + it("treats an omitted exhausted percentage as 100% used for a valid weekly CLI billing period", async () => { mockReadFile.mockImplementation(async (filePath: string) => { if (String(filePath).includes(".grok/auth.json")) return GROK_CLI_AUTH_JSON; return Promise.reject(new Error("File not found")); @@ -3680,8 +3680,8 @@ describe("usage", () => { expect(grok.windows).toHaveLength(1); expect(grok.windows[0]).toMatchObject({ label: "Weekly (credits)", - percentUsed: 0, - percentLeft: 100, + percentUsed: 100, + percentLeft: 0, }); expect(grok.windows[0].resetText).toContain("resets in"); expect(mockRequest).toHaveBeenCalledTimes(1); diff --git a/packages/dashboard/src/routes/register-auth-routes.ts b/packages/dashboard/src/routes/register-auth-routes.ts index 917b1bbcb6..e9052720a0 100644 --- a/packages/dashboard/src/routes/register-auth-routes.ts +++ b/packages/dashboard/src/routes/register-auth-routes.ts @@ -615,6 +615,13 @@ export const registerAuthRoutes: ApiRouteRegistrar = (ctx) => { const scopeLoginError = missingInferenceScope ? "This Anthropic login is missing the model-access (inference) scope, so model calls will fail. Re-login to grant full access." : undefined; + /* + FNXC:ProviderAuth 2026-07-14-15:54: + Expired OAuth must carry an actionable card message, not only authenticated:false. Refresh failures such as invalid_grant cannot repair themselves; tell the operator to re-login while preserving a more specific background-login or inference-scope error when available. + */ + const expiryLoginError = expired + ? "This OAuth session expired and could not be refreshed. Re-login to restore model access." + : undefined; return { id: statusProvider.id, name: statusProvider.name, @@ -623,7 +630,7 @@ export const registerAuthRoutes: ApiRouteRegistrar = (ctx) => { expired: expired || missingInferenceScope, loginInProgress: loginInProgress.has(statusProvider.id), requiresManualCode: getManualCodeConfig(toOauthLoginProviderId(statusProvider.id), origin) !== undefined || undefined, - loginError: lastLoginError.get(statusProvider.id) ?? scopeLoginError, + loginError: lastLoginError.get(statusProvider.id) ?? scopeLoginError ?? expiryLoginError, }; })); diff --git a/packages/dashboard/src/usage.ts b/packages/dashboard/src/usage.ts index 7343cda15a..499afa2dfa 100644 --- a/packages/dashboard/src/usage.ts +++ b/packages/dashboard/src/usage.ts @@ -1682,13 +1682,13 @@ async function fetchGrokCliBillingUsage(token: string, usage: ProviderUsage): Pr const isWeekly = config.currentPeriod?.type === "USAGE_PERIOD_TYPE_WEEKLY"; /* FNXC:UsageProviders 2026-07-14-14:47: - Grok's billing endpoint uses protobuf-style JSON and omits the numeric `creditUsagePercent` field when its value is zero. The Grok Build CLI renders that valid reduced weekly config as “Weekly limit: 0%”; Fusion must mirror the CLI instead of treating an authenticated 200 response as expired auth. Only infer zero when the response still proves a weekly billing period and reset boundary, so malformed payloads continue to fail closed. + Grok's billing endpoint omits `creditUsagePercent` when the weekly allowance is exhausted. Grok Build renders that valid reduced config as “Weekly limit: 0%” (zero allowance remaining), while Fusion's usage model stores percent consumed. Therefore the omitted exhausted value maps to 100% used—not 0% used. Only infer exhaustion when the response still proves a weekly billing period and reset boundary, so malformed payloads continue to fail closed. */ const rawPercentUsed = config.creditUsagePercent; const pctUsed = typeof rawPercentUsed === "number" && Number.isFinite(rawPercentUsed) ? rawPercentUsed : isWeekly && parsedReset - ? 0 + ? 100 : undefined; if (pctUsed === undefined) return false; diff --git a/packages/engine/src/__tests__/agent-session-helpers.test.ts b/packages/engine/src/__tests__/agent-session-helpers.test.ts index 30709d188e..555dcaaf77 100644 --- a/packages/engine/src/__tests__/agent-session-helpers.test.ts +++ b/packages/engine/src/__tests__/agent-session-helpers.test.ts @@ -927,6 +927,24 @@ describe("resolveImplicitPlanningFallbackModel (FN-7719)", () => { }); }); + it("uses the inherited global default when a project override equals the planning primary", () => { + expect( + resolveImplicitPlanningFallbackModel( + { + defaultProviderOverride: "anthropic", + defaultModelIdOverride: "claude-sonnet-5", + defaultProvider: "openai-codex", + defaultModelId: "gpt-5.5", + }, + "anthropic", + "claude-sonnet-5", + ), + ).toEqual({ + provider: "openai-codex", + modelId: "gpt-5.5", + }); + }); + it("returns undefined/undefined when no project/global default model is configured", () => { expect( resolveImplicitPlanningFallbackModel({}, "9router", "nvidia/moonshotai/kimi-k2.6"), diff --git a/packages/engine/src/__tests__/executor-prompt.test.ts b/packages/engine/src/__tests__/executor-prompt.test.ts index 258bc04d13..f2edb8e254 100644 --- a/packages/engine/src/__tests__/executor-prompt.test.ts +++ b/packages/engine/src/__tests__/executor-prompt.test.ts @@ -1077,7 +1077,7 @@ describe("TaskExecutor pause behavior", () => { expect(resumeLogCalls).toHaveLength(1); }); - it("clears stale failed state before resuming unpaused in-progress task", async () => { + it("does not resurrect a failed in-progress task when an unrelated update is emitted", async () => { const store = createMockStore(); mockedCreateFnAgent.mockImplementation(async () => ({ @@ -1100,6 +1100,7 @@ describe("TaskExecutor pause behavior", () => { dependencies: [], steps: [], currentStep: 0, + comments: [{ id: "oversight-1", text: "[planner-oversight] inject guidance", author: "agent" }], log: [], createdAt: new Date().toISOString(), updatedAt: new Date().toISOString(), @@ -1107,8 +1108,9 @@ describe("TaskExecutor pause behavior", () => { await new Promise((r) => setTimeout(r, 30)); - expect(store.updateTask).toHaveBeenCalledWith("FN-001", { status: null, error: null }); - expect(store.logEntry).toHaveBeenCalledWith("FN-001", "Resuming execution after unpause", undefined, undefined); + expect(store.updateTask).not.toHaveBeenCalledWith("FN-001", { status: null, error: null }); + expect(store.logEntry).not.toHaveBeenCalledWith("FN-001", "Resuming execution after unpause", undefined, undefined); + expect(mockedCreateFnAgent).not.toHaveBeenCalled(); }); it("clears stale failed state before resuming orphaned in-progress task", async () => { diff --git a/packages/engine/src/__tests__/fallback-model-observer.test.ts b/packages/engine/src/__tests__/fallback-model-observer.test.ts index ffda9552e7..dc54aa5dd1 100644 --- a/packages/engine/src/__tests__/fallback-model-observer.test.ts +++ b/packages/engine/src/__tests__/fallback-model-observer.test.ts @@ -29,11 +29,12 @@ describe("createFallbackModelObserver", () => { primaryModel: "openai-codex/gpt-5.3-codex", fallbackModel: "zai/glm-5.1", triggerPoint: "prompt-time", + failureCategory: "authentication", timestamp: "2026-05-03T22:00:00.000Z", }); const expectedMessage = - "[fallback] executor switched from openai-codex/gpt-5.3-codex to zai/glm-5.1 (prompt-time)"; + "[fallback] executor switched from openai-codex/gpt-5.3-codex to zai/glm-5.1 (prompt-time; primary provider authentication failed)"; expect(store.logEntry).toHaveBeenCalledWith("FN-123", expectedMessage); expect(store.appendAgentLog).toHaveBeenCalledWith( diff --git a/packages/engine/src/__tests__/transient-error-detector.test.ts b/packages/engine/src/__tests__/transient-error-detector.test.ts index 21087c813c..e58e90c603 100644 --- a/packages/engine/src/__tests__/transient-error-detector.test.ts +++ b/packages/engine/src/__tests__/transient-error-detector.test.ts @@ -385,6 +385,7 @@ describe("Transient Error Detector", () => { expect(isOperatorActionableAgentError("Authentication failed for provider")).toBe(true); expect(isOperatorActionableAgentError("model gpt-x not found")).toBe(true); expect(isOperatorActionableAgentError("missing OPENAI_API_KEY")).toBe(true); + expect(isOperatorActionableAgentError("No API key for provider: anthropic")).toBe(true); expect(isOperatorActionableAgentError("billing issue: quota exceeded")).toBe(true); expect(isOperatorActionableAgentError("OAuth token does not meet scope requirements")).toBe(true); expect(isOperatorActionableAgentError("insufficient_scope: missing repo grant")).toBe(true); diff --git a/packages/engine/src/__tests__/triage.test.ts b/packages/engine/src/__tests__/triage.test.ts index 4ae16f736b..0164dc4368 100644 --- a/packages/engine/src/__tests__/triage.test.ts +++ b/packages/engine/src/__tests__/triage.test.ts @@ -4573,6 +4573,65 @@ describe("taskCreate tool model inheritance", () => { })); expect(onSpecifyError).toHaveBeenCalled(); }); + + it("parks missing provider credentials instead of making triage immediately claimable again", async () => { + const task = { + id: "FN-7952", + description: "Specify a task with direct Anthropic auth", + column: "triage", + status: "planning", + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + } as unknown as Task; + const store = createMockStore({ + getTask: vi.fn().mockResolvedValue({ ...task, attachments: [] }), + }); + mockCreateFnAgent.mockRejectedValue(new Error("No API key for provider: anthropic")); + + const processor = new TriageProcessor(store, "/test/root", { pollIntervalMs: 100_000 }); + await processor.specifyTask(task); + + expect(store.updateTask).toHaveBeenCalledWith("FN-7952", expect.objectContaining({ + status: "failed", + error: "Specification failed: No API key for provider: anthropic", + recoveryRetryCount: null, + nextRecoveryAt: null, + })); + expect(store.updateTask).not.toHaveBeenCalledWith("FN-7952", expect.objectContaining({ status: null })); + }); + + it("uses bounded transient recovery when a credential refresh fails because the connection reset", async () => { + const task = { + id: "FN-7952-TRANSIENT", + description: "Retry a transient credential refresh failure", + column: "triage", + status: "planning", + dependencies: [], + steps: [], + currentStep: 0, + log: [], + createdAt: new Date().toISOString(), + updatedAt: new Date().toISOString(), + } as unknown as Task; + const store = createMockStore({ + getTask: vi.fn().mockResolvedValue({ ...task, attachments: [] }), + }); + mockCreateFnAgent.mockRejectedValue(new Error("credential refresh failed: connection reset")); + + const processor = new TriageProcessor(store, "/test/root", { pollIntervalMs: 100_000 }); + await processor.specifyTask(task); + + expect(store.updateTask).toHaveBeenCalledWith("FN-7952-TRANSIENT", expect.objectContaining({ + status: null, + recoveryRetryCount: 1, + nextRecoveryAt: expect.any(String), + })); + expect(store.updateTask).not.toHaveBeenCalledWith("FN-7952-TRANSIENT", expect.objectContaining({ status: "failed" })); + }); }); describe("recovery due-time gating (nextRecoveryAt)", () => { diff --git a/packages/engine/src/agent-session-helpers.ts b/packages/engine/src/agent-session-helpers.ts index 91fbb1d237..241fabeb76 100644 --- a/packages/engine/src/agent-session-helpers.ts +++ b/packages/engine/src/agent-session-helpers.ts @@ -497,6 +497,19 @@ export function resolveImplicitPlanningFallbackModel( // (hasDistinctFallback requires the models to differ). Leave both fields // undefined so the existing terminal behavior is preserved cleanly. if (resolvedModel.provider === primaryProvider && resolvedModel.modelId === primaryModelId) { + /* + FNXC:TriageModelFallback 2026-07-14-15:54: + A project default override can also become the resolved planning primary. When that makes the first implicit fallback a self-swap, try the distinct inherited global default pair before declaring that no fallback exists. This preserves the one-swap ceiling while allowing an authenticated global provider to recover a project-override auth failure. + */ + const inheritedGlobalProvider = settings?.defaultProvider; + const inheritedGlobalModelId = settings?.defaultModelId; + if ( + inheritedGlobalProvider + && inheritedGlobalModelId + && (inheritedGlobalProvider !== primaryProvider || inheritedGlobalModelId !== primaryModelId) + ) { + return { provider: inheritedGlobalProvider, modelId: inheritedGlobalModelId }; + } return { provider: undefined, modelId: undefined }; } diff --git a/packages/engine/src/executor.ts b/packages/engine/src/executor.ts index d35a4ac32f..ccdd6f1337 100644 --- a/packages/engine/src/executor.ts +++ b/packages/engine/src/executor.ts @@ -2895,6 +2895,14 @@ export class TaskExecutor { } private async dispatchUnpauseResume(task: Task): Promise { + /* + FNXC:ExecutorResume 2026-07-14-15:31: + A terminal failed in-progress task must not be resurrected by an unrelated `task:updated` event. Planner oversight steering comments emit that event; treating it as an unpause cleared the failure and restarted the same missing-credential execution every 45 seconds. Explicit Retry/Unpause routes clear `status` before emitting their update, while startup orphan recovery has its own bounded path, so keep failed rows parked here for operator action. + */ + if (task.status === "failed") { + return false; + } + if ( this.executing.has(task.id) || this.resumingUnpaused.has(task.id) @@ -3113,8 +3121,9 @@ export class TaskExecutor { } } - // This also covers orphaned states (for example, engine restart while - // paused in-progress). dispatchUnpauseResume owns all duplicate guards. + // Explicit unpause updates and non-failed orphan updates can resume here; + // startup failed-orphan recovery is owned by resumeOrphaned(). + // dispatchUnpauseResume owns the terminal-failure and duplicate guards. if ( !task.paused && task.column === "in-progress" diff --git a/packages/engine/src/fallback-model-observer.ts b/packages/engine/src/fallback-model-observer.ts index 00a4dd0916..37d176a452 100644 --- a/packages/engine/src/fallback-model-observer.ts +++ b/packages/engine/src/fallback-model-observer.ts @@ -24,7 +24,20 @@ function buildFallbackLogMessage( label: string, payload: FallbackModelUsedPayload, ): string { - return `[fallback] ${label} switched from ${payload.primaryModel} to ${payload.fallbackModel} (${payload.triggerPoint})`; + const reason = payload.failureCategory === "authentication" + ? "; primary provider authentication failed" + : payload.failureCategory === "rate-limit" + ? "; primary provider rate limit reached" + : payload.failureCategory === "model-selection" + ? "; primary model was unavailable" + : payload.failureCategory === "provider-error" + ? "; primary provider failed" + : ""; + /* + FNXC:ModelFallback 2026-07-14-15:58: + A successful fallback must still explain the primary failure on the task. Persist a bounded category rather than raw provider text so operators can distinguish authentication from capacity/model failures without leaking credentials or arbitrary response bodies into activity logs. + */ + return `[fallback] ${label} switched from ${payload.primaryModel} to ${payload.fallbackModel} (${payload.triggerPoint}${reason})`; } export function createFallbackModelObserver(options: FallbackModelObserverOptions) { diff --git a/packages/engine/src/notification/__tests__/oauth-expiry-monitor.test.ts b/packages/engine/src/notification/__tests__/oauth-expiry-monitor.test.ts index d5952e9624..c569da599d 100644 --- a/packages/engine/src/notification/__tests__/oauth-expiry-monitor.test.ts +++ b/packages/engine/src/notification/__tests__/oauth-expiry-monitor.test.ts @@ -70,6 +70,79 @@ describe("OAuthExpiryMonitor", () => { monitor.stop(); }); + it("does not start the durable alert cooldown when every notification provider fails", async () => { + vi.useFakeTimers(); + const now = Date.now(); + const authStorage = createAuthStorage({ type: "oauth", expires: now - 1_000 }); + const dispatchConfirmed = vi.fn(async () => false); + const alertState = new OAuthAlertStateStore({ statePath: createStatePath(), clock: () => now }); + const monitor = new OAuthExpiryMonitor({ + authStorage, + notificationService: { dispatchConfirmed } as any, + intervalMs: 100, + clock: () => now, + alertState, + }); + + await monitor.start(); + await vi.runOnlyPendingTimersAsync(); + + expect(dispatchConfirmed).toHaveBeenCalledTimes(2); + expect(alertState.getLastAlertAt("openai-codex")).toBeUndefined(); + monitor.stop(); + }); + + it("dispatches and records each expired provider with a provider-specific dedupe key", async () => { + vi.useFakeTimers(); + const now = Date.now(); + const credentials: Record = { + "openai-codex": { type: "oauth", expires: now - 2_000 }, + "github-copilot": { type: "oauth", expires: now - 1_000 }, + }; + const authStorage: AuthStorageLike = { + reload: vi.fn(), + getOAuthProviders: () => [ + { id: "openai-codex", name: "OpenAI Codex" }, + { id: "github-copilot", name: "GitHub Copilot" }, + ], + get: (providerId: string) => credentials[providerId], + }; + const dispatchConfirmed = vi.fn(async () => true); + const alertState = new OAuthAlertStateStore({ statePath: createStatePath(), clock: () => now }); + const monitor = new OAuthExpiryMonitor({ + authStorage, + notificationService: { dispatchConfirmed } as any, + intervalMs: 100, + clock: () => now, + alertState, + }); + + await monitor.start(); + + expect(dispatchConfirmed).toHaveBeenCalledTimes(2); + expect(dispatchConfirmed).toHaveBeenNthCalledWith( + 1, + "oauth-token-expired", + expect.objectContaining({ + metadata: expect.objectContaining({ + notificationDedupeKey: `oauth-token-expired:openai-codex:${now - 2_000}`, + }), + }), + ); + expect(dispatchConfirmed).toHaveBeenNthCalledWith( + 2, + "oauth-token-expired", + expect.objectContaining({ + metadata: expect.objectContaining({ + notificationDedupeKey: `oauth-token-expired:github-copilot:${now - 1_000}`, + }), + }), + ); + expect(alertState.getLastAlertAt("openai-codex")).toBe(now); + expect(alertState.getLastAlertAt("github-copilot")).toBe(now); + monitor.stop(); + }); + /* FNXC:ProviderAuth 2026-07-11-18:00: Regression coverage for FN-7821: GitHub Copilot's stored OAuth access token is intentionally short-lived and can look expired on an OAuthExpiryMonitor interval tick even though getApiKey() can silently refresh it. The monitor must attempt that refresh and re-check before dispatching so ntfy and OAuthReloginBanner do not disagree. @@ -138,11 +211,12 @@ describe("OAuthExpiryMonitor", () => { expect(dispatch).toHaveBeenCalledWith( "oauth-token-expired", expect.objectContaining({ - metadata: { + metadata: expect.objectContaining({ providerId: "github-copilot", providerName: "GitHub Copilot", expiresAt: new Date(now - 1_000).toISOString(), - }, + notificationDedupeKey: `oauth-token-expired:github-copilot:${now - 1_000}`, + }), }), ); expect(JSON.stringify(dispatch.mock.calls)).not.toContain("opaque-github-copilot-access-token"); diff --git a/packages/engine/src/notification/notification-service.ts b/packages/engine/src/notification/notification-service.ts index 16d6cda38f..17f28d4e72 100644 --- a/packages/engine/src/notification/notification-service.ts +++ b/packages/engine/src/notification/notification-service.ts @@ -549,15 +549,44 @@ export class NotificationService { } async dispatch(eventType: NotificationEvent, payload: NotificationPayload): Promise { + await this.dispatchConfirmed(eventType, payload); + } + + async dispatchConfirmed(eventType: NotificationEvent, payload: NotificationPayload): Promise { if (!this.notificationsEnabled) { await this.refreshNotificationState("manual-dispatch"); if (!this.notificationsEnabled) { - return; + return false; } } const dedupTaskId = payload.taskId ?? "global"; - this.maybeNotify(dedupTaskId, eventType, payload); + const metadataDedupeKey = typeof payload.metadata?.notificationDedupeKey === "string" + ? payload.metadata.notificationDedupeKey.trim() + : ""; + const key = metadataDedupeKey.length > 0 ? metadataDedupeKey : `${dedupTaskId}:${eventType}`; + if (this.notifiedEvents.has(key)) { + return true; + } + + /* + FNXC:OAuthNotifications 2026-07-14-15:46: + OAuth expiry monitoring needs confirmed delivery before it starts the durable 12-hour alert cooldown. Its confirmed dispatch path therefore awaits provider results and reports whether any provider succeeded; existing workflow dispatch keeps its Promise contract and fire-and-forget task events remain on maybeNotify. + */ + this.notifiedEvents.add(key); + try { + const results = await this.dispatcher.dispatch(eventType, payload); + const delivered = results.some((result) => result.success); + if (!delivered) { + this.notifiedEvents.delete(key); + } + return delivered; + } catch (error) { + this.notifiedEvents.delete(key); + const message = error instanceof Error ? error.message : String(error); + schedulerLog.log(`NotificationService.dispatch failed key=${key} error=${message}`); + return false; + } } private async refreshNotificationState(reason: string): Promise { diff --git a/packages/engine/src/notification/oauth-expiry-monitor.ts b/packages/engine/src/notification/oauth-expiry-monitor.ts index 71c945fafc..ac9b5d47e4 100644 --- a/packages/engine/src/notification/oauth-expiry-monitor.ts +++ b/packages/engine/src/notification/oauth-expiry-monitor.ts @@ -175,11 +175,23 @@ export class OAuthExpiryMonitor { providerId: provider.id, providerName: provider.name, expiresAt: new Date(credential.expires).toISOString(), + /* + FNXC:OAuthNotifications 2026-07-14-16:08: + Each provider and credential expiry needs an independent notification identity. A shared global event key makes a successful alert for one expired provider suppress every other provider while falsely starting their durable cooldowns. + */ + notificationDedupeKey: `oauth-token-expired:${provider.id}:${credential.expires}`, }, }; try { - await this.opts.notificationService.dispatch("oauth-token-expired", payload); + const confirmedDispatch = this.opts.notificationService.dispatchConfirmed?.bind(this.opts.notificationService); + const delivered = confirmedDispatch + ? await confirmedDispatch("oauth-token-expired", payload) + : (await this.opts.notificationService.dispatch("oauth-token-expired", payload), true); + if (delivered === false) { + schedulerLog.warn(`OAuth expiry notification had no successful provider provider=${provider.id}`); + continue; + } this.dispatchedExpiryKeys.add(expiryKey); this.alertState.recordAlert(provider.id, credential.expires, now); } catch (error) { diff --git a/packages/engine/src/pi.ts b/packages/engine/src/pi.ts index 34d1d07614..4feb707f9a 100644 --- a/packages/engine/src/pi.ts +++ b/packages/engine/src/pi.ts @@ -969,6 +969,7 @@ export interface FallbackModelUsedPayload { taskId?: string; taskTitle?: string; timestamp?: string; + failureCategory?: "authentication" | "rate-limit" | "model-selection" | "provider-error"; } export class ModelFallbackExhaustedError extends Error { @@ -2502,10 +2503,28 @@ export async function createFnAgent(options: AgentOptions): Promise }); }; - const emitFallbackUsed = async (triggerPoint: "session-creation" | "prompt-time"): Promise => { + const emitFallbackUsed = async ( + triggerPoint: "session-creation" | "prompt-time", + primaryFailure: unknown, + ): Promise => { if (!options.onFallbackModelUsed || !selectedModel || !fallbackModel || !hasDistinctFallback) { return; } + const failureMessage = primaryFailure instanceof Error ? primaryFailure.message : String(primaryFailure); + const normalizedFailure = failureMessage.toLowerCase(); + const failureCategory: FallbackModelUsedPayload["failureCategory"] = + normalizedFailure.includes("auth") + || normalizedFailure.includes("api key") + || normalizedFailure.includes("credential") + || normalizedFailure.includes("oauth") + || normalizedFailure.includes("401") + || normalizedFailure.includes("403") + ? "authentication" + : normalizedFailure.includes("rate limit") || normalizedFailure.includes("429") || normalizedFailure.includes("quota") + ? "rate-limit" + : isRetryableModelSelectionError(failureMessage) + ? "model-selection" + : "provider-error"; await options.onFallbackModelUsed({ primaryModel: `${selectedModel.provider}/${selectedModel.id}`, fallbackModel: `${fallbackModel.provider}/${fallbackModel.id}`, @@ -2513,6 +2532,7 @@ export async function createFnAgent(options: AgentOptions): Promise taskId: options.taskId, taskTitle: options.taskTitle, timestamp: new Date().toISOString(), + failureCategory, }); }; @@ -2537,7 +2557,7 @@ export async function createFnAgent(options: AgentOptions): Promise } catch (fallbackErr: unknown) { throw makeFallbackExhaustedError("session-creation", 2, fallbackErr); } - await emitFallbackUsed("session-creation"); + await emitFallbackUsed("session-creation", err); piLog.log("Fallback session created successfully"); } @@ -2692,7 +2712,7 @@ export async function createFnAgent(options: AgentOptions): Promise usingFallback = true; const fallbackSession = await swapPromptSession(fallbackModel); - await emitFallbackUsed("prompt-time"); + await emitFallbackUsed("prompt-time", err); // Retry with fallback model, also with auto-compaction support try { diff --git a/packages/engine/src/transient-error-detector.ts b/packages/engine/src/transient-error-detector.ts index 80a3c0433b..4d4d5326e2 100644 --- a/packages/engine/src/transient-error-detector.ts +++ b/packages/engine/src/transient-error-detector.ts @@ -309,6 +309,7 @@ const OPERATOR_ACTIONABLE_AGENT_ERROR_PATTERNS: RegExp[] = [ /no such model/i, /credential/i, /missing .*key/i, + /no api key/i, /billing/i, /quota exceeded/i, ]; diff --git a/packages/engine/src/triage.ts b/packages/engine/src/triage.ts index cdf7a9e934..a02898dbc0 100644 --- a/packages/engine/src/triage.ts +++ b/packages/engine/src/triage.ts @@ -128,7 +128,7 @@ import { checkSessionError, type UsageLimitPauser, } from "./usage-limit-detector.js"; -import { isTransientError, isSilentTransientError } from "./transient-error-detector.js"; +import { isOperatorActionableAgentError, isTransientError, isSilentTransientError } from "./transient-error-detector.js"; import { withRateLimitRetry } from "./rate-limit-retry.js"; import { computeRecoveryDecision, formatDelay, MAX_RECOVERY_RETRIES } from "./recovery-policy.js"; import type { StuckTaskDetector } from "./stuck-task-detector.js"; @@ -1523,6 +1523,31 @@ export class TriageProcessor { }); this.options.onSpecifyError?.(task, err); return; + } else if (isOperatorActionableAgentError(errorMessage) && !isTransientError(errorMessage)) { + /* + FNXC:TriageAuth 2026-07-14-15:46: + Provider credentials, OAuth grants, billing, and model-access failures require operator action. Triage must park the task as failed instead of restoring its claimable status, because the scheduler otherwise repeats the same specification attempt every poll while no external state has changed. + + FNXC:TriageAuth 2026-07-14-16:08: + Transient infrastructure signals take precedence when an error also mentions credentials, such as a connection reset during refresh. Those mixed failures keep the bounded retry policy; only genuinely permanent authentication failures park immediately. + */ + const failureMessage = `Specification failed: ${errorMessage}`; + planLog.error(`✗ ${task.id} planning needs operator action: ${errorDetail}`); + await this.store.logEntry(task.id, failureMessage, errorStack).catch((logErr: unknown) => { + const msg = logErr instanceof Error ? logErr.message : String(logErr); + planLog.warn(`${task.id}: failed to persist operator-actionable specification failure: ${msg}`); + }); + await this.store.updateTask(task.id, { + status: "failed", + error: failureMessage, + recoveryRetryCount: null, + nextRecoveryAt: null, + }).catch((updateErr: unknown) => { + const msg = updateErr instanceof Error ? updateErr.message : String(updateErr); + planLog.warn(`${task.id}: failed to park operator-actionable specification failure: ${msg}`); + }); + this.options.onSpecifyError?.(task, err instanceof Error ? err : new Error(errorMessage)); + return; } else if (isTransientError(errorMessage)) { // Transient network/infrastructure error — use bounded recovery policy const decision = computeRecoveryDecision({