feat(ui): replace raw "/login" error with a Connect-AI empty state#1754
Draft
kasiazjc wants to merge 5 commits into
Draft
feat(ui): replace raw "/login" error with a Connect-AI empty state#1754kasiazjc wants to merge 5 commits into
kasiazjc wants to merge 5 commits into
Conversation
Detects "no credential resolved for this session's provider" task failures structurally (reusing resolveApiKey's resolution order via a new classifyMissingCredentialFailure daemon hook) instead of matching the raw stderr-passthrough error text, and renders a friendly, actionable MissingCredentialPanel in its place. The panel is scoped live per-viewing-user via the existing check-auth service, and its primary CTA deep-links into Settings' per-provider Agentic Tools tab (UserSettingsModal gains an initialTab prop for this).
…lFailure Matches the existing sessionsRepository injection pattern instead of constructing TaskRepository internally, so the hook is unit-testable without a real database.
…rios Covers: no credential anywhere, workspace/env credential present, per-user credential present, unrelated (non-task-failure) messages, unmapped tools (opencode), missing task/session, and resolveApiKey throwing — the hook must fail closed and leave the message untouched.
Live testing against a fresh no-credential container surfaced a second pathway the original detection missed entirely: the claude CLI can return a *successful* SDK result (subtype 'success', task completes normally, nothing throws) whose result text is actually the CLI's own auth-failure message, with zero real assistant turns and zero tokens consumed. base-executor.ts's catch block — and the is_task_failure marker classifyMissingCredentialFailure keyed off of — only fires when the SDK call throws, so this success-shaped failure sailed straight through as a normal assistant message containing the raw "Not logged in - Please run /login" text. Extend classifyMissingCredentialFailure to also trigger on assistant messages with metadata.tokens.input === 0 && output === 0: a real successful Claude turn always consumes tokens (the system prompt alone costs hundreds), so zero-token assistant output is a reliable structural signal that no real model call happened. That signal is not unique to auth failures — local slash-command output (e.g. /cost) is zero-token too — so it's only ever used to decide whether to *run* the existing resolveApiKey check, never to decide the outcome; an authenticated user's /cost output still has a credential resolve, so it's left untouched. When classified, both pathways are normalized onto system/SYSTEM (the zero-token message was synthesized as if it were a real reply, but never actually was) so the UI keeps a single render branch regardless of which SDK pathway produced it.
…Source 'none' Found while live-verifying the Connect-AI empty state fix against a genuinely credential-less container: MissingCredentialPanel's per-viewer check-auth call reported authenticated:true even though the admin user had zero configured credentials anywhere (no API key, no config.yaml default, no env var, no ~/.claude/ file) and the task that had just run demonstrably failed to reach the model. Root cause: the Claude Agent SDK's accountInfo() signals "no source" with the literal string 'none' rather than omitting the field (confirmed via daemon logs: `tokenSource=none`). check-auth.ts's hasAuthSignal check did a plain truthy check on that raw string, so the sentinel itself counted as a signal. Extracted the fix into a pure isRealAuthSource() helper in a new check-auth-helpers.ts module (rather than inline) so it's unit testable without pulling in check-auth.ts's full import graph - importing it directly for a test hits an unrelated pre-existing OpenTelemetry/ESM resolution break in the Claude Agent SDK's dependency chain.
2 tasks
kasiazjc
force-pushed
the
feature-connect-ai-empty-state
branch
from
06d9d0d to
9d2106f
Compare
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When no credential resolves for a session's configured provider (Claude, Codex, Gemini, etc.), users used to see a raw, unexplained string leaked straight from the provider CLI/SDK's stderr: "Not logged in · Please run /login."
/loginisn't an Agor command, and the message gave no indication of what to do next.This replaces that raw passthrough with a friendly, actionable "Connect AI" empty state — without touching how credentials are stored or resolved.
classifyMissingCredentialFailurebefore-create hook on themessagesservice (apps/agor-daemon/src/hooks/classify-missing-credential.ts) re-checksresolveApiKey— the same resolution chain (per-user key → config.yaml → env var → native CLI/OAuth) thatcheck-auth.tsalready uses for the onboarding wizard's "Test Connection" button — whenever a task fails. It only touches messages the executor tags withmetadata.is_task_failure(a new marker added inbase-executor.ts), so rate limits, network errors, and other failures keep their existing handling untouched.MissingCredentialPanelcomponent re-checks auth for whoever is currently viewing the message via the samecheck-authservice, scoped to their own identity. Two users looking at the same session — one connected, one not — each see the panel state that's actually true for them.AGENTIC_TOOL_DISPLAY_NAMES,AGENTIC_TOOL_KEY_CREATION_URLinpackages/core/src/types/agentic-tool.ts), not hardcoded to Claude.UserSettingsModalgains aninitialTabprop so the panel's primary CTA ("Connect Claude", "Connect Codex", …) lands directly on that provider's Agentic Tools tab instead of the generic Settings screen — wired through a newonOpenAgenticToolSettingsaction onAppActionsContext.SystemMessage/Alert/Empty-style patterns already used elsewhere in the app) — no new UI dependencies.Test plan
classify-missing-credential.test.ts(new, 8 tests): no credential anywhere → classified; workspace/env credential present → not classified; per-user credential present → not classified; unrelated (non-task-failure) messages → untouched,resolveApiKeynever called; unmapped tools (opencode) → falls through; missing task/session → falls through;resolveApiKeythrowing → fails closed, message left untouched@agor/core,@agor/daemon,@agor/executor,agor-uiregister-hooks.test.ts(43),base-executor.test.ts(3),UserSettingsModal.test.tsx(3),ConversationView.test.tsx(11),SessionPanel*.test.tsx(15),session.test.ts(17) — all passing