fix(vscode): warn on workspace-overridden settings + validate Direct Connect host URL#1263
Conversation
…Connect host URL applyAllSettings now detects keys shadowed by a conflicting value in the workspace's .vscode/settings.json (via config.inspect) and returns them so the save path shows a warning instead of a misleading "saved successfully" that silently reverts. Also validates that on-prem (Direct Connect) requires a non-empty Host URL, mirroring the cloud teamId check. Fixes rocketride-org#1257 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR implements workspace settings override detection to fix issue ChangesWorkspace Override Detection and Save Feedback
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID. Do not edit or delete. |
Summary
Fixes #1257.
When a settings key is overridden in the workspace's
.vscode/settings.json,ConfigManager.applyAllSettings()wrote every key toConfigurationTarget.Global, so the workspace value kept shadowing the just-written user value. The save then reported "Settings saved successfully!", reloaded the webview from the merged config, and the UI snapped back to the old value — while user-level settings silently diverged (e.g.connectionMode: "onprem"+ emptyhostUrl, breaking the extension in other workspaces).This PR takes the warning approach from the issue's two options (see the discussion comment): keep writing to user settings, but detect the conflict and tell the user, rather than writing into a (usually git-tracked)
.vscode/settings.jsonfrom the panel.Changes
applyAllSettings(config.ts) — each key is written through a small helper that usesconfig.inspect()to detect a conflicting workspace/workspace-folder value (one that differs from what we're saving) before the Global write. The method now returns{ shadowedKeys }.saveAllSettings(SettingsProvider.ts) — when any saved key is shadowed, show awarning(which persists, unlike the auto-dismissed success) explaining the value is overridden by this workspace's.vscode/settings.jsonand won't take effect here, instead of a misleading success.teamIdcheck. (The enum value isonprem; there is nodirect.)isShadowedByWorkspace+ unit tests, kept side-effect-free so it's testable without a live VS Code instance (mirrorsconnectionModeAuth).A matching workspace override (same value the user is saving) is intentionally not flagged, so the warning only fires on real conflicts.
Verification
npx tsc --noEmit— clean (other callers, e.g.WelcomeProvider, ignore the new return value and still compile).npx tsx --test src/test/workspaceOverride.test.ts— 8/8 pass.rocketride.development.connectionMode: "local"pinned in workspace settings, switching to Direct Connect + Save now shows the warning instead of a false success that reverts.Notes for reviewers
src/test/*.test.tsfiles don't appear to be wired intobuilder test, and the existingconnectionModeAuth.test.tsis currently failing ondevelop(impl/test drift). New tests here pass viatsx --testbut aren't run by CI — worth a separate follow-up to wire up the harness..vscode/settings.jsondirectly.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests