Surface missing credential run failures#1915
Conversation
|
Here's a visual recap of what changed: Open the full interactive recap |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…1-run-status # Conflicts: # packages/core/src/client/agent-chat-adapter.spec.ts
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🟡
Review Details
This PR tightens run finalization across run-manager, run-store, and the shared chat adapter so missing-credential terminal events and earlier streamed failures are preserved as failed runs instead of being washed into successful completions. The server-side reconciliation changes are directionally sound: they correctly treat missing_api_key as errored, prefer earlier real terminal failures over later done/continuation markers, and add strong regression coverage for both direct finalization and stale-run repair paths.
Risk assessment: standard. The blast radius is moderate because this touches shared run-state persistence plus the client follow-loop used by background chat recovery.
Key findings:
- 🟡 Background recovery still misses the modern
error:missing_credentialsterminal reason and therefore skips the credential-specific CTA/event path. - 🟡 Rows that are mistakenly persisted as
status="completed"withterminal_reason="error:*"can still be surfaced as successful chats unless that error code is hardcoded in the adapter map.
Good patterns: the reconciliation precedence logic in run-store.ts is well targeted, and the added tests meaningfully cover the intended regression scenarios.
🧪 Browser testing: Will run after this review (PR touches UI/client chat behavior)
| ? BACKGROUND_TERMINAL_REASON_MESSAGES[terminalReason] | ||
| : undefined; | ||
| const mappedErrorCode = | ||
| terminalReason === "missing_api_key" |
There was a problem hiding this comment.
🟡 Missing-credentials background recovery only recognizes the legacy terminal reason
After stripping the optional error: prefix, this path only special-cases legacy missing_api_key. If /runs/active reports error:missing_credentials after the terminal SSE was missed, the adapter falls back to a generic error and never fires the missing-credentials CTA/event; treat both reason shapes the same here.
Additional Info
Found by 1/3 review passes; verified against AgentChatEvent docs declaring `missing_api_key` legacy and `error:missing_credentials` as the modern terminal error shape.
| return; | ||
| } | ||
|
|
||
| if (status === "completed") { |
There was a problem hiding this comment.
🟡 Completed rows with terminal_reason=error:* can still finalize as successful chats
If /runs/active returns status: "completed" with terminal_reason = "error:<code>", any code not present in BACKGROUND_TERMINAL_REASON_MESSAGES still falls through this branch and is emitted as a successful completion. That preserves the silent-success bug for older failed rows like error:provider_failed; any error:* reason should be treated as a terminal failure here.
Additional Info
Found by 1/3 review passes; manually confirmed from `emitBackgroundTerminalOutcome()` ordering and `terminalReasonForRun()` producing `error:<errorCode>` for terminal error events.
2 participants
Summary
missing_api_keyterminal events as errored in run reconciliation and direct run finalizationterminal_reason=missing_api_keyas actionable missing-credential chat failures@agent-native/coreVerification
node_modules/.bin/oxfmt --check packages/core/src/agent/run-manager.ts packages/core/src/agent/run-manager.spec.ts packages/core/src/agent/run-store.ts packages/core/src/agent/run-store.spec.ts packages/core/src/client/agent-chat-adapter.ts packages/core/src/client/agent-chat-adapter.spec.tscd packages/core && node_modules/.bin/vitest --run src/agent/run-manager.spec.ts src/agent/run-store.spec.ts src/client/agent-chat-adapter.spec.ts src/agent/production-agent.chain-continuation.spec.ts src/agent/run-loop-with-resume.spec.ts src/agent/run-store.durable-background.spec.ts(273 tests)cd packages/core && pnpm typecheckgit diff --check.task-sweep/logs/claude-opus-plan-20260706-134914.log,.task-sweep/logs/claude-opus-plan-output-20260706-134914.jsonl.task-sweep/logs/claude-opus-final-review-20260706-180931.log,.task-sweep/logs/claude-opus-final-review-output-20260706-180931.jsonl.task-sweep/logs/claude-fable-5-refresh-plan-retry-20260709-212943.debug.log,.task-sweep/logs/claude-fable-5-refresh-plan-retry-output-20260709-212943.jsonl.task-sweep/logs/claude-fable-5-refresh-review-20260709-214244.debug.log,.task-sweep/logs/claude-fable-5-refresh-review-output-20260709-214244.jsonl(no blockers; approved)Manual story
As a hosted Analytics user without the required model credential, asking the agent a question now lands in a clear missing-credential failure state instead of a completed/successful run. Mid-stream errors and missing-key terminal events persist as failed run rows with error fields, while ordinary successful runs still complete.
Notes
mainwhile preserving its reconnect, stale-run, and continuation behavior.