Skip to content

fix(daemon): map Claude Not logged in output to /login guidance (#1928)#3050

Open
leno23 wants to merge 1 commit into
nexu-io:mainfrom
leno23:fix/1928-claude-not-logged-in-diagnostics
Open

fix(daemon): map Claude Not logged in output to /login guidance (#1928)#3050
leno23 wants to merge 1 commit into
nexu-io:mainfrom
leno23:fix/1928-claude-not-logged-in-diagnostics

Conversation

@leno23
Copy link
Copy Markdown

@leno23 leno23 commented May 26, 2026

Refs #1928

Summary

When Claude Code exits during a connection test with stdout like Not logged in · Please run /login., Open Design now classifies it as an auth failure and returns the existing /login + CLAUDE_CONFIG_DIR guidance instead of surfacing the raw CLI text.

Surface area

  • apps/daemon/src/claude-diagnostics.ts — recognize not logged in / please run /login output
  • apps/daemon/tests/claude-diagnostics.test.ts — regression test for the reported message shape

Validation

  • cd apps/daemon && pnpm test tests/claude-diagnostics.test.ts

Made with Cursor

…-io#1928)

Treat Claude Code stdout like "Not logged in · Please run /login." as an
auth failure in diagnoseClaudeCliFailure so connection tests and chat
runs surface actionable login guidance instead of raw CLI text.
@lefarcen lefarcen requested a review from mrcfps May 26, 2026 17:00
@lefarcen lefarcen added size/XS PR changes <20 lines risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps type/bugfix Bug fix labels May 26, 2026
Copy link
Copy Markdown
Contributor

@lefarcen lefarcen left a comment

Choose a reason for hiding this comment

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

Hi @leno23 — thanks for tying this directly back to the Claude login report in #1928. The summary and validation are clear; one small PR-body cleanup before pool review: could you add the checklist-style ## Surface area section and tick the relevant box for this user-visible daemon diagnostic change? I’d expect Default behavior change here, since the connection-test failure now maps to guidance instead of showing raw CLI output.

Related: #1928 is the reporter thread this PR is addressing, and I added a short pointer there so the issue stays connected while review runs.

Copy link
Copy Markdown
Contributor

@lefarcen lefarcen left a comment

Choose a reason for hiding this comment

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

Hi @leno23 — thanks for tying this directly back to the Claude login report in #1928. The summary and validation are clear; one small PR-body cleanup before pool review: could you add the checklist-style ## Surface area section and tick the relevant box for this user-visible daemon diagnostic change? I’d expect Default behavior change here, since the connection-test failure now maps to guidance instead of showing raw CLI output.

Related: #1928 is the reporter thread this PR is addressing, and I added a short pointer there so the issue stays connected while review runs.

Copy link
Copy Markdown
Contributor

@mrcfps mrcfps left a comment

Choose a reason for hiding this comment

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

@leno23 I reviewed the daemon diagnostic change and the added regression coverage for the Claude Not logged in · Please run /login. output. The auth classification stays narrowly scoped to Claude failures, and pnpm test tests/claude-diagnostics.test.ts passes locally for the updated fixture set. Thanks for tightening up this login guidance path. 🙌

🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.

@lefarcen
Copy link
Copy Markdown
Contributor

All gates are green here — flagged for the maintainers. Thanks @leno23 for the fix and @mrcfps for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk/high High risk: apps/desktop, daemon, auth, migration, workflows, package deps size/XS PR changes <20 lines type/bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants