fix(advisories): keep the headless remote hint platform-neutral - #10738
Conversation
The headless_remote_hint advisory said "Headless Linux hosts" but its predicate reads no platform term, so a failed preflight on macOS printed Linux-only prose. On macOS the predicate collapses to "TERM_PROGRAM is unset", which is true over SSH, under launchd, under cron and in CI. The remediation the advisory carries, CHAT_UI_URL, is platform-neutral, and the repository treats isHeadlessLikely as a valid macOS signal: the Docker Desktop credential-store advisory uses it on its non-WSL branch. Every other host advisory that names an OS carries a matching platform predicate. Rewording removes the false OS claim and keeps the hint for headless macOS hosts, where it applies unchanged; gating to Linux would have removed a correct hint to fix a wording defect. Signed-off-by: Dongni Yang <dongniy@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 12 included reviews per hour; 4 remain after this review. 📝 WalkthroughWalkthroughThe headless host advisory now uses platform-neutral wording. It recommends SSH port forwarding and conditional ChangesHeadless advisory
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change only makes the headless remote hint platform-neutral while preserving its existing behavior and adds a focused regression test; no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/pluginThe overall line coverage in commit d76ff23 in the TypeScript / code-coverage/cliThe overall line coverage in commit d76ff23 in the Show a line coverage summary of the most impacted files.
Updated |
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
|
PR Review Advisor finished for commit |
prekshivyas
left a comment
There was a problem hiding this comment.
Verified the final diff and exact-SHA checks. The macOS advisory is platform-neutral, its remote-access guidance matches the documented SSH and HTTPS paths, and focused tests, CI, E2E, CodeRabbit, and the Advisor are clear.
Outcome
The
headless_remote_hintadvisory no longer tells macOS users about "Headless Linux hosts". Its prose is platform-neutral, matching its predicate, and its remediation now recommends SSH port forwarding first while reservingCHAT_UI_URLfor an HTTPS external origin.Reason
reviewHeadlessUiSettingsreads no platform term — its predicate is!DISPLAY && !WAYLAND_DISPLAY && !TERM_PROGRAMplus!hasNvidiaGpu. On macOS that collapses to "TERM_PROGRAMis unset", which is true over SSH, underlaunchd, undercron, and in CI, so a failed preflight on a Mac printed Linux-only prose. #10670's reported transcript showed exactly this.#10734 left the remedy open as a product call: gate the check to Linux/WSL, or reword. This PR takes the reword, for three reasons:
CHAT_UI_URLprovides the same override on every OS.isHeadlessLikelyas a valid macOS signal: the Docker Desktop credential-store advisory uses it on its non-WSL branch (src/lib/advisories/checks/host/docker.ts), and Docker Desktop on macOS is the common case.isWsl, while the docker-group advisory returns null unlessplatform === "linux".headless_remote_hintwas the only outlier. Rewording restores the pattern by removing the OS claim rather than adding a gate.Related issues
Closes #10734
Refs #10670
Changes
src/lib/advisories/checks/host/toolchain.ts— removes the Linux-only wording and recommends SSH port forwarding for remote browser access. It directs users to setCHAT_UI_URLonly when the dashboard needs an HTTPS external origin.src/lib/advisories/checks/host/toolchain.test.ts— adds a macOS regression test that verifies the advisory still fires and pins the platform-neutral reason and remote-access command.No new mechanism is added. This is advisory copy plus its protecting test.
Verification
npx vitest run --project cli src/lib/advisories/checks/host/toolchain.test.ts— 3 passednpm run typecheck:cli— passednpm run validate:pr— passedd76ff23— completed successfully; all actionable findings resolvedd76ff23— completed successfullyd76ff23— completed successfullySigned-off-by: Dongni Yang dongniy@nvidia.com