feat: wrap no-wait verification url as markdown autolink#703
feat: wrap no-wait verification url as markdown autolink#703liangshuo-1 wants to merge 2 commits intomainfrom
Conversation
The auth login --no-wait JSON output emits verification_url for an AI agent to relay to the end user. URLs from the OAuth device-flow endpoint contain underscores in query parameters; when the agent embeds the URL into a markdown reply, the underscores are parsed as italic markers and the URL gets truncated by the renderer. Wrap the URL value in angle brackets so it is recognized as a markdown autolink. Renderers leave the inner content unparsed, the link stays clickable, and the user sees the full URL for OAuth review. Scoped to the --no-wait JSON path. Interactive --json output and plain-text stderr remain unchanged: those paths target programs and human users respectively, where bare URLs are appropriate. Change-Id: I80595b0fc63821e19fdd1032b1bb02a9eb224481
📝 WalkthroughWalkthroughWhen Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #703 +/- ##
==========================================
+ Coverage 63.80% 63.82% +0.02%
==========================================
Files 500 500
Lines 43531 43531
==========================================
+ Hits 27773 27782 +9
+ Misses 13317 13308 -9
Partials 2441 2441 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@dd9d06fed1f7e6e828a99a68bfc5d5d8032017db🧩 Skill updatenpx skills add larksuite/cli#feat/auth-login-md-url -y -g |
The autolink form (<URL>) failed in two layers observed in practice: 1. LLM transcription strips <> as if it were an HTML tag remnant. 2. Some markdown renderers do not honor CommonMark autolink semantics and still apply emphasis parsing inside <>, truncating URLs that contain underscores. Switch to a double-defense form: the URL text wrapped in backticks gives inline-code semantics (emphasis cannot mangle the displayed text), and the outer markdown link gives clickability. Even if the renderer mishandles the link destination, the backtick-wrapped text remains intact and copy-pastable. LLM passthrough preserves both backticks (sacred code-block convention) and link syntax (the most trained markdown form), avoiding the strip seen with <>. Change-Id: I3ee9dae2d0d8ad4a6a13430a5ca3f60451507fa1
There was a problem hiding this comment.
🧹 Nitpick comments (2)
cmd/auth/login_test.go (1)
995-1003: Tighten hint assertions to protect the exact contract text.At Line 995, checking broad tokens (
"verbatim","backtick") may still pass if critical phrasing regresses. Consider asserting the exact structural phrase and guardrail text.As per coding guidelines, “Design CLI flags, help text, and error messages with AI agent consumption in mind” and “Every behavior change must have an accompanying test.”Suggested assertion tightening
for _, want := range []string{ "verification_url", "verbatim", - "backtick", + "`[`URL`](URL)`", + "Do not unwrap, escape, or rewrite", } {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/auth/login_test.go` around lines 995 - 1003, The current test loop that inspects the variable hint (in cmd/auth/login_test.go) is too loose — replace the loose token checks for "verbatim" and "backtick" with assertions that match the exact expected guardrail/help phrasing (or a precise regexp) so the test enforces the exact contract text; update the loop that iterates over the []string and the t.Fatalf message to check for the full expected substrings (e.g., the exact structural phrase containing the verification_url and the verbatim/backtick guidance) or add separate assertions that validate the complete lines verbatim, ensuring the test fails on any phrasing regression.cmd/auth/login.go (1)
233-233: Harden markdown destination formatting for edge-case URLs.At Line 233, the markdown destination is inserted as raw
(URL). If the URL contains), some markdown parsers can truncate the link target. Prefer angle-bracket destinations to preserve full parsing.Proposed hardening
- data := map[string]interface{}{ - "verification_url": "[`" + authResp.VerificationUriComplete + "`](" + authResp.VerificationUriComplete + ")", + completeURL := authResp.VerificationUriComplete + data := map[string]interface{}{ + "verification_url": "[`" + completeURL + "`](<" + completeURL + ">)",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmd/auth/login.go` at line 233, Replace the raw markdown link destination "(URL)" with an angle-bracketed destination "(<URL>)" to avoid truncation when the URL contains ")" — specifically update the string construction that uses authResp.VerificationUriComplete (the expression currently building "`" + authResp.VerificationUriComplete + "`](" + authResp.VerificationUriComplete + ")") so the destination becomes "(<" + authResp.VerificationUriComplete + ">)", e.g. "[`<url>`](<url>)" style, leaving the visible text unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmd/auth/login_test.go`:
- Around line 995-1003: The current test loop that inspects the variable hint
(in cmd/auth/login_test.go) is too loose — replace the loose token checks for
"verbatim" and "backtick" with assertions that match the exact expected
guardrail/help phrasing (or a precise regexp) so the test enforces the exact
contract text; update the loop that iterates over the []string and the t.Fatalf
message to check for the full expected substrings (e.g., the exact structural
phrase containing the verification_url and the verbatim/backtick guidance) or
add separate assertions that validate the complete lines verbatim, ensuring the
test fails on any phrasing regression.
In `@cmd/auth/login.go`:
- Line 233: Replace the raw markdown link destination "(URL)" with an
angle-bracketed destination "(<URL>)" to avoid truncation when the URL contains
")" — specifically update the string construction that uses
authResp.VerificationUriComplete (the expression currently building "`" +
authResp.VerificationUriComplete + "`](" + authResp.VerificationUriComplete +
")") so the destination becomes "(<" + authResp.VerificationUriComplete + ">)",
e.g. "[`<url>`](<url>)" style, leaving the visible text unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 67a2d421-9300-4fac-8141-717ec216e238
📒 Files selected for processing (2)
cmd/auth/login.gocmd/auth/login_test.go
Summary
auth login --no-wait --jsonreturns averification_urlfor an AI agent to relay to the end user. URLs from the OAuth device-flow endpoint contain underscores in query parameters (e.g.,state,flow_id,user_code); when the agent embeds the URL in a markdown reply, underscores are parsed as italic markers and the URL gets truncated by the renderer, breaking the OAuth flow.This PR wraps the URL in angle brackets (
<...>) so it is recognized as a markdown autolink. Renderers leave inner content unparsed, the URL stays clickable, and the user still sees the full URL for OAuth review (vs hiding it behind[link](url)link text).Changes
cmd/auth/login.go: in the--no-waitJSON branch, wrapverification_urlvalue as<URL>and update thehintfield to instruct agents to display the value verbatimcmd/auth/login_test.go: 4 new tests covering the JSON contract —--no-waitwraps as autolink, hint instructs verbatim display, interactive--jsonkeeps raw URL, plain-text stderr keeps bare URLThe change is scoped to
--no-wait. Interactive--jsonoutput and plain-text stderr remain unchanged: those paths target programs and human terminal users where bare URLs are appropriate.Test Plan
make unit-testpassed (all packages green; including 4 newcmd/auth/cases and the existingTestAuthLoginRun_*regression set)go vet ./...passed (nomake validatetarget in Makefile; vet is the closest equivalent)./lark-cli auth login --no-wait --json --scope "im:message:send"against the real Feishu device-authorization endpoint; confirmedverification_urlis<https://accounts.feishu.cn/oauth/v1/device/verify?flow_id=...&user_code=...>and thehintreads "Show verification_url to the user verbatim — it is wrapped in <> as a markdown autolink ..."Related Issues
N/A
Summary by CodeRabbit
Bug Fixes
Tests