feat: validate credentials after config init

[dc-bytedance]

Summary

After lark-cli config init saves the App ID / App Secret, validate them against the open platform so the user gets immediate, hard-to-miss feedback when the credentials are wrong, instead of discovering it later from a failed business request. The check is best-effort: it stays silent on anything ambiguous (network errors, server 5xx, unknown transport failures, timeout) so a valid configuration is never blocked by upstream noise.

Changes

• Extract internal/credential/tat_fetch.go::FetchTAT from doResolveTAT — a pure-HTTP tenant-access-token mint that takes credentials directly (no config / no keychain read), so a caller holding plaintext credentials (the probe) can validate them without a second keychain round-trip. A non-zero TAT body code is routed through the credential layer's shared classifyTATResponseCode, so a rejection is the canonical CategoryConfig / SubtypeInvalidClient (RFC 6749 invalid_client) typed error — code 10003 (bad/absent app_id) and 10014 (wrong app_secret) both land here. Transport / HTTP-non-200 / JSON-parse failures stay raw (untyped). doResolveTAT now delegates to FetchTAT (one HTTP path, one classifier; behavior unchanged).
• Add cmd/config/init_probe.go::runProbe — best-effort post-init validator under a single 3-second context.WithTimeout. It mints a TAT with the just-saved credentials, then POSTs the application probe endpoint (outcome ignored). When FetchTAT returns a typed error (a deterministic credential rejection), runProbe propagates it so config init exits 3 with the canonical type: config / subtype: invalid_client envelope — byte-identical to what every other token-resolving command (im, calendar, …) shows for the same bad credentials. errs.IsTyped is the discriminator: untyped errors (transport / HTTP / parse / timeout / http-client init) are swallowed (return nil, exit 0).
• Wire runProbe into all four cmd/config/init.go paths (non-interactive, --new, interactive TUI, legacy readline) as if err := runProbe(...); err != nil { return err }. Skipped when the user reused an existing secret. The saved config is intentionally not rolled back on rejection — stdout still records the saved-config JSON, while stderr carries the typed error envelope and exit is 3.
• Tests: internal/credential/tat_fetch_test.go (code 10003/10014 → ConfigError/invalid_client; unknown code → typed APIError; transport/HTTP-non-200/parse → untyped; brand routing; context-cancel) and cmd/config/init_probe_test.go (rejection → propagated ConfigError; ambiguous → nil; probe request shape; brand routing; timeout; stderr-stays-empty invariant).

Test Plan

• make unit-test passed (cmd/config + internal/credential, under -race)
• make build + go vet ./... + gofmt clean
• golangci-lint --new-from-rev=upstream/main on cmd/config + internal/credential → 0 issues (errs-typed-only / forbidigo / errscontract all pass)
• E2E 7/7 in tests_e2e/config/ passed — verified via LARK_CLI_BIN=$PWD/lark-cli go test ./tests_e2e/config/ against the live endpoint, including TestConfigInit_CredentialRejection_ExitsThree (real endpoint rejects a fake App ID → exit 3 + config/invalid_client envelope). The Docker eval sandbox failed at its own auth status setup gate (environmental, unrelated), so the E2E suite was run directly against the real binary.
• acceptance-reviewer re-run after rebase: 6/6 scenarios + 3 exploratory passed against the live endpoint. Confirmed: (1) fake App ID → exit 3 + type:config / subtype:invalid_client / code:10003, stdout JSON intact; (2) real App ID + wrong secret → code:10014; (3) network-unreachable → silent, exit 0; (4) --brand lark routes to open.larksuite.com (CONNECT-layer proof); (5) the probe rejection envelope is byte-identical to im +chat-list for the same bad credentials — the point of the rebase alignment; (6) §2.6 vocabulary clean in the envelope and both source files.
• manual verification against the live TAT endpoint: real App ID + wrong secret → exit 3, stdout {appId, appSecret:"****", brand}, stderr OK: Configuration saved + {"type":"config","subtype":"invalid_client","code":10014,"message":"app secret invalid","hint":"run \lark-cli config init` to set valid app_id and app_secret"}`.

Consumer note

When config init exits non-zero, key off the exit code, read stdout as the config record and stderr as the diagnostic. On a credential rejection, stdout still carries the saved-config JSON (the config was written) while stderr carries the typed error envelope and exit is 3 — these are not contradictory (saved ≠ authenticates). Do not parse stderr as a single JSON document: it may contain a leading OK: Configuration saved line before the error envelope. The rejection is classified type: config / subtype: invalid_client, consistent with every other command — a consumer that previously matched type == authentication for this case must switch to type == config.

Related Issues

N/A

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Centralizes tenant-access-token exchange in FetchTAT, adds runProbe (3s timeout) invoked after saving app credentials in init flows, returns an AuthenticationError for deterministic credential rejections, and otherwise silently attempts a probe POST. Also refactors provider to use FetchTAT and adds comprehensive tests.

Changes

Post-Config Credential Probing

Layer / File(s)	Summary
TAT fetch core function and error mapping internal/credential/tat_fetch.go	FetchTAT exchanges app credentials for tenant access tokens with comprehensive error typing: request/JSON errors become InternalError, transport failures become retryable NetworkError, HTTP 401/403 become AuthenticationError, HTTP 5xx become retryable NetworkError, and response decode/validation errors map to appropriate error subtypes. Supports brand-based URL routing.
TAT fetch test suite internal/credential/tat_fetch_test.go	Tests cover successful token retrieval, request payload validation, authentication error classification (response codes and HTTP 401/403), network error handling, JSON parse failures, brand-specific routing, and context cancellation. Uses stubRoundTripper and urlRewriteRT test helpers.
Default provider TAT fetch refactor internal/credential/default_provider.go	Refactors existing TAT resolution to use the new FetchTAT function, removing inline HTTP wiring and JSON decode logic while maintaining caching via sync.Once. Removes unused imports.
Probe invocation function and error detection cmd/config/init_probe.go	runProbe performs TAT fetch within a timeout, re-wraps deterministic authentication rejections into a returned AuthenticationError with an actionable message/hint, and silently issues a brand-routed probe POST using the TAT as a bearer token; probe outcomes are ignored and other errors are silent.
Probe test suite with request verification cmd/config/init_probe_test.go	Tests validate TAT error outcomes (API codes and HTTP 401/403 return AuthenticationError; 5xx and transport errors are silent), probe request contracts (HTTP POST, correct URL, Authorization header, from with build.Version), brand routing, client initialization failures, and timeout enforcement. Includes fakeRT, jsonResp, and fakeFactory helpers.
Config init flow integration cmd/config/init.go	Adds runProbe calls after config save in four paths: non-interactive --app-id/--app-secret, --new app creation, interactive TUI (only when new secret), and legacy readline fallback (only when secret input non-empty).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

feature

Suggested reviewers

• liangshuo-1
• Roy-oss1

Poem

🐰 I hop through code where tokens gleam,
I fetch a TAT and softly scheme.
If creds are wrong I raise a bell,
If nets are flaky, I say well—
A quiet probe, then back to dream.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 27.27% 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 clearly summarizes the main change: adding credential validation after config init, which is the primary purpose of this PR.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description is comprehensive and well-structured, covering motivation, implementation details, test validation, and breaking behavioral notes.

_{✏️ 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]

Actionable comments posted: 3

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

Inline comments:
In `@cmd/config/init_probe_test.go`:
- Around line 63-79: The test builds a Factory directly in fakeFactory; replace
that with the repo-standard TestFactory to ensure proper test isolation: in the
shared setup call t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir()) and create
the factory via cmdutil.TestFactory(t, config) instead of manually constructing
&cmdutil.Factory and HttpClient; update fakeFactory (and the similar block at
the other occurrence around lines 255-266) to return the factory and error
buffer from cmdutil.TestFactory so tests use the canonical TestFactory and
isolated config dir.

In `@internal/credential/tat_fetch.go`:
- Around line 23-31: The comment block describing error classification in
internal/credential/tat_fetch.go is out of sync: update the line that currently
states “other non-2xx → *errs.NetworkError” to match the runtime behavior where
non-2xx responses (excluding 401/403 and 5xx) are returned as
*errs.InternalError (subtype "sdk_error"); keep the existing rules for
*errs.AuthenticationError (401/403 or body code via Problem.Code) and
*errs.NetworkError (5xx or transport failures wrapped via Cause) so callers see
the same contract as the code path implemented around the non-2xx handling in
the block that returns *errs.InternalError (see the code near the current return
sites for *errs.InternalError in the 112-119 region).
- Around line 137-147: When parsed.Code == 0 you must also validate
parsed.TenantAccessToken is non-empty; if TenantAccessToken is empty, return an
internal SDK error instead of a successful empty token. Update the success
branch in tat_fetch.go (the block that currently returns
parsed.TenantAccessToken) to check parsed.TenantAccessToken (and/or
parsed.TenantAccessToken == "") and return an errs.InternalError (populate
errs.Problem with Category: errs.CategoryInternal, an appropriate Subtype such
as errs.SubtypeSDK, and a clear Message like "missing tenant_access_token on TAT
API success") so callers fail fast with a clear cause.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d60a3a6b-dc9b-4969-b50f-cab448016cba

📥 Commits

Reviewing files that changed from the base of the PR and between b91f6a2 and 1af43d8.

📒 Files selected for processing (6)

• cmd/config/init.go
• cmd/config/init_probe.go
• cmd/config/init_probe_test.go
• internal/credential/default_provider.go
• internal/credential/tat_fetch.go
• internal/credential/tat_fetch_test.go

[github-actions]

🚀 PR Preview Install Guide

🧰 CLI update

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

🧩 Skill update

npx skills add dc-bytedance/cli#feat/config-probe -y -g

[codecov]

Codecov Report

❌ Patch coverage is 66.66667% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.22%. Comparing base (0aa9e96) to head (aae1c06).

Files with missing lines	Patch %	Lines
cmd/config/init.go	0.00%	10 Missing ⚠️
cmd/config/init_probe.go	76.92%	3 Missing and 3 partials ⚠️
internal/credential/tat_fetch.go	85.71%	2 Missing and 2 partials ⚠️
internal/credential/default_provider.go	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1151      +/-   ##
==========================================
+ Coverage   69.19%   69.22%   +0.02%     
==========================================
  Files         634      636       +2     
  Lines       59482    59523      +41     
==========================================
+ Hits        41161    41205      +44     
+ Misses      15007    14999       -8     
- Partials     3314     3319       +5     

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