feat(cli): add terminal hyperlinks, live feedback, and verification to guided installer - #157
Open
jack-arturo wants to merge 5 commits into
Open
feat(cli): add terminal hyperlinks, live feedback, and verification to guided installer#157jack-arturo wants to merge 5 commits into
jack-arturo wants to merge 5 commits into
Conversation
…o guided installer - OSC 8 hyperlinks (theme.link) for deploy links, health/recall URLs, git clone, success endpoint (graceful stripAnsi degradation) - Live attempt counters on local "Waiting for AutoMem" spinner via onAttempt callback - Early interactive "Detected on this machine: ..." summary explaining agent pre-selections - Post-apply connectivity + auth verification probe with celebratory line - Guarded action.command printing (only prepare-local + manual-step); linked commands in review - tests/e2e/interactive.mjs strip improved for OSC so harness expectations stay clean - All local gates green: build, 65 vitest, full 5/5 PTY interactive harness Related paths only; unrelated main-checkout dirt (AGENTS, INSTALLATION, other CLI, untracked cloud/grok) left behind per user worktree request.
Place the eslint-disable comments immediately before the two regex literals that contain \x1b / \x07 control characters. The previous placement on the return line was treated as unused and the rule still fired on the character positions inside the regexes. This was the sole cause of the "test" CI failure on the initial PR head. No behavior change.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR enhances the guided installer’s terminal UX by adding OSC 8 hyperlinks, more live feedback during endpoint waits, and a post-install verification probe, while keeping the e2e PTY harness stable by stripping OSC sequences.
Changes:
- Add an OSC 8 hyperlink helper and teach
stripAnsi/visibleLengthto ignore OSC sequences. - Add live attempt counters during local endpoint wait + early “Detected on this machine …” hint + end-of-apply verification message.
- Update e2e transcript stripping and relax a scenario expectation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
tests/e2e/interactive.mjs |
Strip OSC (including hyperlinks) from PTY transcripts and adjust expectations. |
src/cli/ui/theme.ts |
Add Theme.link, implement OSC-aware stripAnsi, and introduce an OSC 8 hyperlink helper. |
src/cli/install.ts |
Use hyperlinks in plan/details, add spinner attempt updates, early detected-clients hint, and final verification probe messaging. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
123
to
128
| bullet: unicode ? '•' : '-', | ||
| arrow: unicode ? '→' : '->', | ||
| line: unicode ? '─' : '-', | ||
| }, | ||
| link: hyperlink, | ||
| }; |
Comment on lines
+161
to
+165
| export function hyperlink(url: string, label?: string): string { | ||
| const text = label && label.length > 0 ? label : url; | ||
| // OSC 8 ; ; <url> ST <text> OSC 8 ; ; ST | ||
| return `\x1b]8;;${url}\x1b\\${text}\x1b]8;;\x1b\\`; | ||
| } |
Comment on lines
14
to
16
| import { playInstallerSplash, shouldUseInstallerAnimation } from './install-ui.js'; | ||
| import { makeTheme } from './ui/theme.js'; | ||
| import { hyperlink, makeTheme } from './ui/theme.js'; | ||
| import { keyValueRows, type TableRow } from './ui/table.js'; |
Comment on lines
+1527
to
+1529
| const t = makeTheme(process.stdout); | ||
| process.stdout.write(`\n${t.style.gold('✦')} ${t.style.dim('Connectivity + auth verified — memory graph is reachable.')}\n`); | ||
| } |
Comment on lines
+1277
to
+1279
| // Friendly early signal of smart defaults (only in interactive TTY so it doesn't | ||
| // pollute dry-run previews or pipes). | ||
| if (interactive && environment.detectedClients.length > 0) { |
…uards - hyperlink(): add safe http(s) URL validation before emitting OSC 8. Non-URLs (e.g. <prompted>) and anything containing controls now return plain text, preventing terminal escape injection. - install.ts: switch the hosted-setup noteBox to use theme.link(...) so it goes through the capability-aware path (no direct hyperlink import). - Remove the now-unused named `hyperlink` import. - "Detected on this machine" guard now also checks !dryRun (the comment claimed it wouldn't pollute dry-run previews on TTY). - Verification success line now omits the hard-coded ✦ glyph when !theme.unicode (consistent with clientGlyph etc.). All other Copilot points were already covered by existing guards or the previous lint fix. Local build + targeted tests still green.
Comment on lines
93
to
129
| export function makeTheme( | ||
| stream: NodeJS.WriteStream = process.stdout, | ||
| options: ThemeOptions = {} | ||
| ): Theme { | ||
| const color = shouldColor(stream, options.color ?? 'auto'); | ||
| const unicode = shouldUseUnicode(stream, options.symbols ?? 'auto'); | ||
| return { | ||
| color, | ||
| unicode, | ||
| width: options.width ?? streamWidth(stream), | ||
| style: { | ||
| bold: wrap(color, ANSI.bold), | ||
| dim: wrap(color, ANSI.dim), | ||
| red: wrap(color, ANSI.red), | ||
| green: wrap(color, ANSI.green), | ||
| yellow: wrap(color, ANSI.yellow), | ||
| blue: wrap(color, ANSI.blue), | ||
| magenta: wrap(color, ANSI.magenta), | ||
| gold: wrap(color, ANSI.gold), | ||
| goldBright: wrap(color, ANSI.goldBright), | ||
| inverseGold: (text) => | ||
| color && text.length > 0 | ||
| ? `${ANSI.goldBg}${ANSI.black}${text}${ANSI.reset}` | ||
| : `[${text.trim()}]`, | ||
| }, | ||
| symbol: { | ||
| check: unicode ? '✓' : '+', | ||
| cross: unicode ? '✗' : 'x', | ||
| warn: unicode ? '▲' : '!', | ||
| info: unicode ? '●' : '*', | ||
| bullet: unicode ? '•' : '-', | ||
| arrow: unicode ? '→' : '->', | ||
| line: unicode ? '─' : '-', | ||
| }, | ||
| link: hyperlink, | ||
| }; | ||
| } |
Comment on lines
+166
to
+179
| export function hyperlink(url: string, label?: string): string { | ||
| const text = label && label.length > 0 ? label : url; | ||
| // Only produce a real hyperlink for safe http(s) URLs. | ||
| try { | ||
| const u = new URL(url); | ||
| if (u.protocol !== 'http:' && u.protocol !== 'https:') { | ||
| return text; | ||
| } | ||
| } catch { | ||
| return text; | ||
| } | ||
| // OSC 8 ; ; <url> ST <text> OSC 8 ; ; ST | ||
| return `\x1b]8;;${url}\x1b\\${text}\x1b]8;;\x1b\\`; | ||
| } |
…l char stripping per Copilot P1
Member
Author
|
/copilot-review |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
stripAnsi/visibleLengthand all non-TTY/CI/NO_COLOR paths continue to see clean plain text.(7/30)).Scope
Only three files:
Verification
npm run buildcleanvitest run src/cli/install.test.ts src/cli/ui/ui.test.ts→ 65 passingnode tests/e2e/interactive.mjs→ all 5 routes (existing-*, cloud, local) passRisk / compatibility
Additive and behind the existing full degradation matrix (TTY, NO_COLOR, AUTOMEM_NO_ANIM, AUTOMEM_ASCII, CI, dumb TERM, win32, non-interactive preview, --dry-run, --yes, pipes). No new deps. Secrets and machine output paths untouched.
Closes the visual-flair gaps vs. 2026 peers (Railway live logs, modern create-*/Vercel-style links + confidence moments) while keeping (and strengthening) the "beautiful but works everywhere" contract.
How to try
(Work done in the user-requested sibling worktree so main remained untouched.)