Add hyperlinks to CLI output#2598
Conversation
📝 WalkthroughWalkthroughReplaces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/packages/cli/src/old.js (1)
93-96:⚠️ Potential issue | 🟡 Minor
padCellwill mis-measure the hyperlinked id cell.
link(...)returns a string containing ANSI OSC-8 escape sequences (\u001B]8;;<url>\u0007<text>\u001B]8;;\u0007) when hyperlinks are supported.padCelluses rawvalue.length, so on supported terminals:
value.lengthfar exceedswidth(37) →substring(0, 37)slices mid-escape, producing broken/unterminated hyperlinks and visible garbage.- Padding calculation is wrong → columns no longer align.
This likely existed with
terminal-linktoo, but the swap is a good opportunity to fix it. Suggest computing the cell from the visible text and appending the escape-wrapped result separately, or stripping ANSI when measuring (e.g. via the existingstrip-ansidep at ^7.1.2) and padding based on that visible length.♻️ Sketch: measure visible width, wrap after padding
- { - header: 'id', - width: 37, - getValue: (triple) => - link(createUrl(triple, job).toString(), triple.entity_id), - }, + { + header: 'id', + width: 37, + // Pad using the visible entity_id, then linkify the padded cell so + // column widths stay correct when OSC-8 escapes are emitted. + getValue: (triple) => triple.entity_id, + render: (triple, cell) => link(createUrl(triple, job).toString(), cell), + },(The table loop would call
col.render?.(triple, padded) ?? paddedafterpadCell.)Also applies to: 118-122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/cli/src/old.js` around lines 93 - 96, padCell currently measures raw value.length which breaks when value contains OSC-8 hyperlink escapes (from link(...)); change padCell to measure the visible text using the existing strip-ansi utility (stripAnsi) or by stripping OSC-8 sequences, then truncate/pad based on that visible length and finally re-wrap or return the original ANSI-wrapped string with the padded visible content; update the other similar place that pads cells (the other padCell usage around the table rendering / col.render flow) so all column alignment uses stripAnsi for measurement before truncation/padding.
🧹 Nitpick comments (2)
client/packages/cli/src/index.ts (1)
118-118: Minor: help-text fallback loses the full URL path.When hyperlinks aren't supported, these render as just
developer.apple.com/dashboard.clerk.com/console.firebase.google.com— readable, but the more specific paths (e.g./account/resources/...) disappear. If that detail matters for discoverability, consider passing the full URL as the label or adjusting the helper's fallback. Non-blocking.Also applies to: 128-128, 130-130
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/cli/src/index.ts` at line 118, The help text currently uses the link helper (link()) for flags like the --services-id description which loses the full URL path in non-hyperlink fallbacks; update the help string(s) in client/packages/cli/src/index.ts where link(...) is used (e.g., the --services-id, related lines around the link usages) to pass the full URL as the label or change the helper invocation so its fallback includes the full path (for example use link(fullUrl, fullUrl) or adjust the helper call to include the path portion) so that when hyperlinks are unsupported the full discoverable URL (including /account/resources/...) remains visible.client/packages/cli/src/logging.ts (1)
36-41: Consider fallback behavior whentextdoesn't match the URL.When hyperlinks aren't supported,
link(url, text)returns justtext, dropping the URL. This is fine whentextrepresents the destination (e.g.,link('https://example.com', 'example.com')), but problematic for labels like"Dashboard:"athandleEnv.ts:110, where users see the label with no URL context.Options:
- Add a comment documenting this behavior for future callers
- Update the fallback to
text && text !== url ? ${text} (${url}) : urlto preserve the URL (matchingterminal-link's default fallback)♻️ Possible fallback change
export const link = (url: string, text?: string): string => { if (supportsHyperlinks.stdout) { return ansiEscapes.link(text ?? url, url); } - return text ?? url; + return text && text !== url ? `${text} (${url})` : url; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/cli/src/logging.ts` around lines 36 - 41, The fallback for the exported link function drops the URL when hyperlinks aren't supported (so labels like "Dashboard:" lose URL context); update link(url, text?) to preserve the URL in the fallback by returning text + " (" + url + ")" when text exists and text !== url, otherwise return url, and add a brief inline comment near the link function explaining this behavior so callers understand the fallback; reference the link function to locate and update the logic and comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/packages/cli/src/lib/handleEnv.ts`:
- Line 110: The current console.log uses link(appDashUrl(appId, dashOrigin),
'Dashboard:') which on terminals without hyperlink support yields only
'Dashboard:' and drops the URL; update the output to always show the URL as well
by rendering the label and the URL explicitly (e.g. show the label and then the
plain appDashUrl(...) in parentheses) or change the link(...) helper to include
the URL in its fallback when text !== url; locate the call to link in
handleEnv.ts and modify it to include the explicit appDashUrl(appId, dashOrigin)
alongside the label so both hyperlink-capable and plain terminals display the
URL.
---
Outside diff comments:
In `@client/packages/cli/src/old.js`:
- Around line 93-96: padCell currently measures raw value.length which breaks
when value contains OSC-8 hyperlink escapes (from link(...)); change padCell to
measure the visible text using the existing strip-ansi utility (stripAnsi) or by
stripping OSC-8 sequences, then truncate/pad based on that visible length and
finally re-wrap or return the original ANSI-wrapped string with the padded
visible content; update the other similar place that pads cells (the other
padCell usage around the table rendering / col.render flow) so all column
alignment uses stripAnsi for measurement before truncation/padding.
---
Nitpick comments:
In `@client/packages/cli/src/index.ts`:
- Line 118: The help text currently uses the link helper (link()) for flags like
the --services-id description which loses the full URL path in non-hyperlink
fallbacks; update the help string(s) in client/packages/cli/src/index.ts where
link(...) is used (e.g., the --services-id, related lines around the link
usages) to pass the full URL as the label or change the helper invocation so its
fallback includes the full path (for example use link(fullUrl, fullUrl) or
adjust the helper call to include the path portion) so that when hyperlinks are
unsupported the full discoverable URL (including /account/resources/...) remains
visible.
In `@client/packages/cli/src/logging.ts`:
- Around line 36-41: The fallback for the exported link function drops the URL
when hyperlinks aren't supported (so labels like "Dashboard:" lose URL context);
update link(url, text?) to preserve the URL in the fallback by returning text +
" (" + url + ")" when text exists and text !== url, otherwise return url, and
add a brief inline comment near the link function explaining this behavior so
callers understand the fallback; reference the link function to locate and
update the logic and comment.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: f87e876f-4163-46e1-8082-51b6be8d34ad
⛔ Files ignored due to path filters (1)
client/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
client/packages/cli/package.jsonclient/packages/cli/src/commands/auth/client/add.tsclient/packages/cli/src/index.tsclient/packages/cli/src/lib/handleEnv.tsclient/packages/cli/src/lib/oauth.tsclient/packages/cli/src/logging.tsclient/packages/cli/src/old.js
|
View Vercel preview at instant-www-js-drewh-cli-links-jsv.vercel.app. |
drew-harris
force-pushed
the
drewh/cli-links
branch
from
4ab1456 to
1f18a69
Compare
drew-harris
force-pushed
the
drewh/cli-links
branch
from
1f18a69 to
42f0896
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
client/packages/cli/src/lib/handleEnv.ts (1)
110-112:⚠️ Potential issue | 🟡 MinorDashboard output regressed: URL is printed twice.
Line 111 currently renders
<url>: <url>, which duplicates the URL and drops theDashboard:label. Prefer a single labeled line with the linkified URL.🔧 Suggested fix
- console.log( - `${link(appDashUrl(appId, dashOrigin))}: ${appDashUrl(appId, dashOrigin)}\n`, - ); + const dashUrl = appDashUrl(appId, dashOrigin); + console.log(`Dashboard: ${link(dashUrl)}\n`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/cli/src/lib/handleEnv.ts` around lines 110 - 112, The current console.log in handleEnv.ts prints the URL twice (`${link(appDashUrl(appId, dashOrigin))}: ${appDashUrl(appId, dashOrigin)}`) and omits the "Dashboard:" label; update the log to print a single labeled line such as "Dashboard: <linkified URL>" by calling link(appDashUrl(appId, dashOrigin)) once and prefixing it with "Dashboard:" (referencing the link and appDashUrl usages and the variables appId and dashOrigin to locate the code).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@client/packages/cli/src/lib/handleEnv.ts`:
- Around line 110-112: The current console.log in handleEnv.ts prints the URL
twice (`${link(appDashUrl(appId, dashOrigin))}: ${appDashUrl(appId,
dashOrigin)}`) and omits the "Dashboard:" label; update the log to print a
single labeled line such as "Dashboard: <linkified URL>" by calling
link(appDashUrl(appId, dashOrigin)) once and prefixing it with "Dashboard:"
(referencing the link and appDashUrl usages and the variables appId and
dashOrigin to locate the code).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7976b519-9be2-401a-ad67-f98360c61d5c
⛔ Files ignored due to path filters (1)
client/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (7)
client/packages/cli/package.jsonclient/packages/cli/src/commands/auth/client/add.tsclient/packages/cli/src/index.tsclient/packages/cli/src/lib/handleEnv.tsclient/packages/cli/src/lib/oauth.tsclient/packages/cli/src/logging.tsclient/packages/cli/src/old.js
✅ Files skipped from review due to trivial changes (2)
- client/packages/cli/package.json
- client/packages/cli/src/old.js
🚧 Files skipped from review as they are similar to previous changes (1)
- client/packages/cli/src/commands/auth/client/add.ts
drew-harris
force-pushed
the
drewh/cli-links
branch
from
a205053 to
9836a1d
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
client/packages/cli/src/commands/auth/client/add.ts (1)
34-34: LGTM — uniform link rendering across providers.All call sites either pass a single URL (so the fallback returns the URL unchanged) or use
developer.apple.comas label forhttps://developer.apple.com(acceptable since text differs only by scheme). No information is lost on terminals without hyperlink support.Minor consistency nit (optional): the prompt suffix punctuation differs between providers — Google/LinkedIn/Apple Services ID/Clerk use
Client ID:, while GitHub usesClient ID(no colon) at line 244, and a few Apple prompts also omit the colon (lines 522, 537). Worth a quick pass for consistency, not blocking.Also applies to: 133-133, 147-147, 244-244, 258-258, 349-349, 363-363, 479-479, 522-522, 537-537, 625-625, 656-656, 740-740
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@client/packages/cli/src/commands/auth/client/add.ts` at line 34, Consistency nit: normalize prompt suffix punctuation for auth client prompts by ensuring all provider prompts use "Client ID:" (with colon) instead of the inconsistent "Client ID " (no colon). Search for the literal strings 'Client ID ' and 'Client ID:' in add.ts and update occurrences used for GitHub and the Apple prompts referenced (and any other providers lacking the colon) so they match Google/LinkedIn/Clerk; maintain existing hyperlink rendering via the imported link symbol if present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@client/packages/cli/src/lib/handleEnv.ts`:
- Around line 110-112: The console.log currently prints the URL twice and drops
the "Dashboard:" label; compute the dashboard URL once (call appDashUrl(appId,
dashOrigin) into a local variable), then log using the link helper with a static
"Dashboard:" prefix (e.g., console.log(`Dashboard: ${link(dashUrl)}`) or
link(dashUrl, 'Dashboard:') if supported) so the label is preserved and
appDashUrl is not invoked twice; update the call site in handleEnv.ts where
console.log and link/appDashUrl are used.
In `@client/packages/cli/src/logging.ts`:
- Around line 36-41: The link helper currently drops the URL when
supportsHyperlinks.stdout is false and a distinct text is provided; update the
link function (export const link) to return a fallback that preserves the URL by
returning "text (url)" when text !== url, while keeping the existing
ansiEscapes.link(text ?? url, url) behavior when hyperlinks are supported;
reference supportsHyperlinks.stdout and ansiEscapes.link to locate and modify
the function and ensure the fallback covers both when text is provided and when
only url is provided.
---
Nitpick comments:
In `@client/packages/cli/src/commands/auth/client/add.ts`:
- Line 34: Consistency nit: normalize prompt suffix punctuation for auth client
prompts by ensuring all provider prompts use "Client ID:" (with colon) instead
of the inconsistent "Client ID " (no colon). Search for the literal strings
'Client ID ' and 'Client ID:' in add.ts and update occurrences used for GitHub
and the Apple prompts referenced (and any other providers lacking the colon) so
they match Google/LinkedIn/Clerk; maintain existing hyperlink rendering via the
imported link symbol if present.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: a0768cb8-a3f7-449d-a2bf-d07c8cf549b7
⛔ Files ignored due to path filters (1)
client/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
client/packages/cli/package.jsonclient/packages/cli/src/commands/auth/client/add.tsclient/packages/cli/src/index.tsclient/packages/cli/src/lib/handleEnv.tsclient/packages/cli/src/lib/oauth.tsclient/packages/cli/src/logging.tsclient/packages/cli/src/old.jsclient/packages/version/src/version.ts
✅ Files skipped from review due to trivial changes (3)
- client/packages/version/src/version.ts
- client/packages/cli/package.json
- client/packages/cli/src/lib/oauth.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- client/packages/cli/src/index.ts
| console.log( | ||
| `${link(appDashUrl(appId, dashOrigin))}: ${appDashUrl(appId, dashOrigin)}\n`, | ||
| ); |
There was a problem hiding this comment.
Output now duplicates the URL and lost the "Dashboard:" label.
This renders as <URL>: <URL> on hyperlink-capable terminals (the linked text is the URL itself, then : , then the same URL again) and URL: URL on plain terminals. The original "Dashboard:" label is gone, and appDashUrl is invoked twice for the same value.
Either keep the label as a static prefix and link the URL, or — if the link helper's fallback is improved per the comment on logging.ts — go back to link(url, 'Dashboard:').
♻️ Minimal fix preserving the original label
- console.log(
- `${link(appDashUrl(appId, dashOrigin))}: ${appDashUrl(appId, dashOrigin)}\n`,
- );
+ const dashUrl = appDashUrl(appId, dashOrigin);
+ console.log(`Dashboard: ${link(dashUrl)}\n`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log( | |
| `${link(appDashUrl(appId, dashOrigin))}: ${appDashUrl(appId, dashOrigin)}\n`, | |
| ); | |
| const dashUrl = appDashUrl(appId, dashOrigin); | |
| console.log(`Dashboard: ${link(dashUrl)}\n`); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/packages/cli/src/lib/handleEnv.ts` around lines 110 - 112, The
console.log currently prints the URL twice and drops the "Dashboard:" label;
compute the dashboard URL once (call appDashUrl(appId, dashOrigin) into a local
variable), then log using the link helper with a static "Dashboard:" prefix
(e.g., console.log(`Dashboard: ${link(dashUrl)}`) or link(dashUrl, 'Dashboard:')
if supported) so the label is preserved and appDashUrl is not invoked twice;
update the call site in handleEnv.ts where console.log and link/appDashUrl are
used.
| export const link = (url: string, text?: string): string => { | ||
| if (supportsHyperlinks.stdout) { | ||
| return ansiEscapes.link(text ?? url, url); | ||
| } | ||
| return text ?? url; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
ansi-escapes link function signature and version history hyperlink support
💡 Result:
The ansi-escapes npm package (latest v7.3.0 as of 2026) provides a link(text, url) function to create clickable terminal hyperlinks using ANSI OSC 8 escape sequences (\u001B]8;;url\u0007text\u001B]8;;\u0007). Supported in terminals like iTerm2, kitty, VTE-based (GNOME Terminal), mintty, Windows Terminal. Use supportsHyperlinks from the separate supports-hyperlinks package to detect support. Function signature: link(text: string, url: string): string Example: import ansiEscapes from 'ansi-escapes'; process.stdout.write(ansiEscapes.link('Google', 'https://google.com')); // Outputs: \u001B]8;;https://google.com\u0007Google\u001B]8;;\u0007 Hyperlink support via OSC 8 was added before v6.x (not explicitly dated in changelog, but present in docs and tested in related packages like ansi-regex since ~2024; confirmed in current v7.x). Package history: v5.0.0 (2021, pure ESM), v6.0.0 (2022, Node 14), v7.0.0 (2024, Node 18), v7.3.0 (2026, synchronized output, tmux/Windows OSC).
Citations:
- 1: https://npmjs.com/package/ansi-escapes
- 2: https://github.com/sindresorhus/ansi-escapes
- 3: https://nodejs.libhunt.com/ansi-escapes-changelog
- 4: https://github.com/sindresorhus/ansi-escapes/releases/tag/v7.0.0
- 5: https://www.npmjs.com/package/ansi-regex?activeTab=versions
- 6: https://github.com/sindresorhus/ansi-escapes/releases
- 7: "+" in links breaks the regex chalk/ansi-regex#57
- 8: https://github.com/chalk/ansi-regex?sponsor=1
Fallback drops the URL when a distinct text is provided.
When supportsHyperlinks.stdout is false and the caller passes a text distinct from url, the helper returns only text and the URL is silently lost. The terminal-link package this replaces defaulted to a text (url) fallback, preserving the URL on non-supporting terminals. This creates a usability gap for any call site that passes a label (e.g., the "Dashboard:" usage in handleEnv.ts, and any future labeled link).
Consider including the URL in the fallback when it differs from the display text:
♻️ Proposed fix
export const link = (url: string, text?: string): string => {
if (supportsHyperlinks.stdout) {
return ansiEscapes.link(text ?? url, url);
}
- return text ?? url;
+ return text === undefined || text === url ? url : `${text} (${url})`;
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@client/packages/cli/src/logging.ts` around lines 36 - 41, The link helper
currently drops the URL when supportsHyperlinks.stdout is false and a distinct
text is provided; update the link function (export const link) to return a
fallback that preserves the URL by returning "text (url)" when text !== url,
while keeping the existing ansiEscapes.link(text ?? url, url) behavior when
hyperlinks are supported; reference supportsHyperlinks.stdout and
ansiEscapes.link to locate and modify the function and ensure the fallback
covers both when text is provided and when only url is provided.
2 participants
Adds hyperlinks via ascii codes wherever applicable, especially in the Oauth client commands.
I noticed that previously we were using the terminal-link package. But that package was not properly detecting the hyperlink support in both ghostty and zed (alacritty based) for me. Re-implementing the library from its 2 dependencies made it work consistently.
How to test:
git switch drewh/cli-links && cd client && pnpm icd sandbox/cli-nodejspnpm exec instant-cli auth client add -hValidate that hyperlink for apple developer site works.