fix(auth): split bot and user identity diagnostics

[mtsui-cmyk]

Fixes #914

Summary

• add shared identity diagnostics for bot and user auth state
• make auth status --verify report identities.bot and identities.user separately, while preserving legacy top-level fields
• make doctor show bot_identity and user_identity checks and fail only when neither identity is usable

Tests

• go test ./internal/identitydiag ./cmd/auth ./cmd/doctor
• go test ./cmd/... ./internal/...

Summary by CodeRabbit

Release Notes

• New Features

  • Identity status reporting now separately tracks bot and user identities with individual status and verification indicators across auth status and doctor commands.

• Tests

  • Added comprehensive tests for identity diagnostics and verification workflows to ensure accurate bot and user identity reporting.

[coderabbitai]

📝 Walkthrough

Walkthrough

Introduces a new identity diagnostics package that independently evaluates bot and user identity readiness, replacing prior opaque token checks. Integrates diagnostics into auth status and doctor commands to clearly distinguish identity paths and provide targeted remediation guidance.

Changes

Unified Identity Diagnostics

Layer / File(s)	Summary
Identity Diagnostics Framework internal/identitydiag/diagnostics.go, internal/identitydiag/diagnostics_test.go	Adds Diagnose function with Result and Identity structs defining status constants (StatusReady, StatusNotConfigured, StatusMissing, StatusNeedsRefresh, StatusVerifyFailed). diagnoseBot validates config and optionally verifies bot info via HTTP. diagnoseUser reads keychain tokens, derives token status, and optionally verifies via Lark SDK. Supporting helpers resolve bot tokens, fetch bot info with HTTP error handling, format timestamps, and map statuses to messages. Five test cases cover missing user, unconfigured bot, bot/user verification, and token refresh detection.
Auth Status Command Integration cmd/auth/status.go, cmd/auth/status_test.go	Rewrites authStatusRun to call identitydiag.Diagnose instead of directly reading tokens. Removes token-expiration branching and verifyTokenOnServer logic. New helpers (effectiveIdentity, addLegacyUserFields, addEffectiveVerification, addStatusNote) transform diagnostic results into JSON with identities, verified flags, and notes. Tests assert bot/user separation and bot verification with mocked info endpoint.
Doctor Command Integration cmd/doctor/doctor.go, cmd/doctor/doctor_test.go	Calls identitydiag.Diagnose and reports separate bot_identity/user_identity checks instead of opaque token diagnostics. Sets identity_ready to pass when at least one identity is available, fail otherwise. Removes unused mustHTTPClient. Documents "warn" status. Test verifies offline mode marks bot ready, user missing, and identity_ready as pass.

Sequence Diagram(s)

sequenceDiagram
  participant CLI as CLI Command
  participant Diagnose as identitydiag.Diagnose
  participant BotDiag as diagnoseBot
  participant UserDiag as diagnoseUser
  participant Factory as cmdutil.Factory
  participant Keychain as Token Keychain
  participant BotAPI as Bot Info Endpoint
  participant SDK as Lark SDK
  
  CLI->>Diagnose: Diagnose(ctx, factory, cfg, verify)
  Diagnose->>BotDiag: validate config, resolve token
  alt verify enabled
    BotDiag->>BotAPI: GET /open-apis/bot/v3/info
    BotAPI-->>BotDiag: openId, appName
    BotDiag->>BotDiag: set Verified=true
  end
  BotDiag-->>Diagnose: Bot Identity result
  
  Diagnose->>UserDiag: validate config, read keychain
  UserDiag->>Keychain: read stored UAToken
  UserDiag->>UserDiag: derive status from token state
  alt verify enabled
    UserDiag->>Factory: create HTTP client
    UserDiag->>SDK: initialize Lark SDK
    UserDiag->>SDK: VerifyUserToken(accessToken)
    SDK-->>UserDiag: verification result
    UserDiag->>UserDiag: set Verified based on result
  end
  UserDiag-->>Diagnose: User Identity result
  
  Diagnose-->>CLI: Result{Bot, User}

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A clinic for identities, split clean in two,
Bot's path distinct from user's trusty clue,
doctor and auth now see with clearer sight—
Bot ready here, user's login there just right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.71% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix(auth): split bot and user identity diagnostics' clearly and concisely describes the main change—refactoring identity diagnostics to separate bot and user identity handling.
Description check	✅ Passed	The description covers all required template sections: summary explains the motivation (separate bot/user identity diagnostics), changes list the main modifications across three areas (auth status, doctor, shared diagnostics), and tests confirm verification approach.
Linked Issues check	✅ Passed	The changes fully address issue #914's requirements: separate bot and user identity diagnostics are implemented in identitydiag.Diagnose; auth status and doctor commands use this to report bot/user status independently; and both machine-readable and human-friendly outputs are provided.
Out of Scope Changes check	✅ Passed	All changes are scoped to implementing separate bot and user identity diagnostics as required by issue #914; no unrelated modifications such as refactoring, formatting, or feature additions outside the stated objectives are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

cmd/auth/status.go (1)

46-49: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Return errors via output.Errorf per coding guidelines.

The RunE function in commands must return output.Errorf or output.ErrWithHint instead of bare errors, because AI agents parse stderr as JSON. As per coding guidelines: "RunE functions in commands must return output.Errorf / output.ErrWithHint — never bare fmt.Errorf".

Proposed fix

 	config, err := f.Config()
 	if err != nil {
-		return err
+		return output.Errorf("failed to load config: %w", err)
 	}

🤖 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 `@cmd/auth/status.go` around lines 46 - 49, The RunE handler in
cmd/auth/status.go currently returns a bare error from f.Config() (config, err
:= f.Config(); if err != nil { return err }) which violates the guideline —
change the error return to use output.Errorf (or output.ErrWithHint when
appropriate) so the command returns machine-parsable JSON; locate the RunE
function (status command) and replace returns of raw err from f.Config() with
something like output.Errorf("failed to load config: %v", err) or
output.ErrWithHint(...) to preserve the original error message and context.

🤖 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.

Outside diff comments:
In `@cmd/auth/status.go`:
- Around line 46-49: The RunE handler in cmd/auth/status.go currently returns a
bare error from f.Config() (config, err := f.Config(); if err != nil { return
err }) which violates the guideline — change the error return to use
output.Errorf (or output.ErrWithHint when appropriate) so the command returns
machine-parsable JSON; locate the RunE function (status command) and replace
returns of raw err from f.Config() with something like output.Errorf("failed to
load config: %v", err) or output.ErrWithHint(...) to preserve the original error
message and context.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7ac92161-d54c-4659-bbd0-1f2aad7e3245

📥 Commits

Reviewing files that changed from the base of the PR and between 583349e and ee5310f.

📒 Files selected for processing (6)

• cmd/auth/status.go
• cmd/auth/status_test.go
• cmd/doctor/doctor.go
• cmd/doctor/doctor_test.go
• internal/identitydiag/diagnostics.go
• internal/identitydiag/diagnostics_test.go

[github-actions]

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@ee5310fa80e5165adca471e4ec311e9e0bf9680a

🧩 Skill update

npx skills add mtsui-cmyk/cli#fix/auth-identity-diagnostics -y -g

[codecov]

Codecov Report

❌ Patch coverage is 45.56452% with 135 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.84%. Comparing base (315e0ab) to head (ee5310f).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/identitydiag/diagnostics.go	42.78%	83 Missing and 24 partials ⚠️
cmd/auth/status.go	44.89%	26 Missing and 1 partial ⚠️
cmd/doctor/doctor.go	91.66%	1 Missing ⚠️

❌ Your patch check has failed because the patch coverage (45.56%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #957      +/-   ##
==========================================
+ Coverage   66.78%   66.84%   +0.05%     
==========================================
  Files         564      565       +1     
  Lines       52441    52611     +170     
==========================================
+ Hits        35024    35167     +143     
+ Misses      14516    14513       -3     
- Partials     2901     2931      +30     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.