Repository navigation
Fix Inline VS Code auth/Settings Sync persistence across reloads - #6841
Closed
austinywang wants to merge 13 commits into
Closed
austinywang wants to merge 13 commits into
austinywang wants to merge 13 commits into
Conversation
Inline VS Code ("Open Current Directory in VS Code (Inline)") launched VS
Code Web through the cached ~/.vscode/cli/serve-web/<id>/bin/code-server
binary directly when available. That bypasses VS Code's `code-tunnel
serve-web` wrapper, which sets up the CLI secret-storage/keyring path VS
Code Web uses to persist GitHub auth and Settings Sync. Combined with an
ephemeral `--port 0` and a fresh per-launch temporary connection token,
the inline server identity changed on every launch, so signing in inside
Inline VS Code was lost on pane reload, folder change, or app relaunch.
Launch through the wrapper and stabilize the server identity:
- Prefer `code-tunnel serve-web`; fall back to the cached `code-server`
only when the wrapper is unavailable (VSCodeServeWebLauncherKind).
- Enable the CLI file keyring for wrapper launches
(VSCODE_CLI_USE_FILE_KEYRING=1) and pin VSCODE_CLI_DATA_DIR.
- Use a stable serve-web server data dir under Application Support
(per bundle id), with user-data and cli-data subdirs.
- Use a stable, persisted port (deterministic per-bundle default stored
under vscodeServeWeb.port), with an ephemeral-port fallback if the
stable port can't be bound.
- Reuse a persistent connection-token file (validated 32-hex, 0600)
instead of a fresh temporary token per launch, so the server URL — and
the browser session keyed to it — stays stable. stop() no longer
deletes the token.
- Omit the unsupported `--user-data-dir` for the wrapper; keep it for the
cached code-server fallback.
- Env overrides: CMUX_VSCODE_SERVE_WEB_DATA_DIR, CMUX_VSCODE_SERVE_WEB_PORT.
Adds unit tests for wrapper preference + launcher kind, stable
path/port resolution and env overrides, persistent connection-token
validate/reuse/replace, and per-launcher argument/environment shaping.
Fixes #6595
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR extracts VS Code serve-web support into a dedicated source file, updates project wiring, and adds launcher, runtime, token, controller, and test coverage for the inline VS Code launch flow. ChangesVS Code serve-web support extraction
Sequence Diagram(s)sequenceDiagram
participant EnsureServeWebURL
participant VSCodeServeWebController
participant VSCodeServeWebLaunchOptionsBuilder
participant Process
participant ServeWebOutputCollector
EnsureServeWebURL->>VSCodeServeWebController: ensureServeWebURL(vscodeApplicationURL:completion:)
VSCodeServeWebController->>VSCodeServeWebLaunchOptionsBuilder: build launch options
VSCodeServeWebController->>Process: launch serve-web
Process->>ServeWebOutputCollector: stream stdout/stderr
ServeWebOutputCollector-->>VSCodeServeWebController: Web UI URL
VSCodeServeWebController-->>EnsureServeWebURL: completion(URL)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (6 errors, 1 warning)
✅ Passed checks (18 passed)
✨ 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 |
Contributor
Greptile SummaryThis PR fixes Inline VS Code losing GitHub auth and Settings Sync across reloads by replacing the direct
Confidence Score: 5/5Safe to merge. The lifecycle management, token persistence, port-retry loop, and termination barrier are all well-guarded and backed by the new unit suite. All critical paths — token reuse/replacement, port collision retry, process termination sequencing, and deferred-relaunch ordering — are correctly guarded by generation counters and the serial dispatch queue. The old #if DEBUG test seam was cleanly removed and replaced with an internal init for @testable injection, matching the canonical fix pattern. File sizes are within budget across all new files. No new blocking primitives are introduced on main-actor paths. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant UI as UI / Caller
participant Ctrl as VSCodeServeWebController
participant LQ as launchQueue
participant Q as queue (serial)
participant Builder as VSCodeCLILaunchConfigurationBuilder
participant TokenStore as VSCodeConnectionTokenStore
participant Proc as code-tunnel serve-web
UI->>Ctrl: ensureServeWebURL(vscodeAppURL)
Ctrl->>Q: queue.async check running / enqueue pending
Q->>LQ: launchQueue.async launchServeWebProcess
LQ->>Builder: launchConfiguration(vscodeAppURL)
Builder-->>LQ: VSCodeCLILaunchConfiguration
LQ->>TokenStore: ensureToken(at connectionTokenFile)
TokenStore-->>LQ: reused or newly-created 0600 token file
loop candidateStablePorts + [0]
LQ->>Q: queue.sync set launchingProcess run process
Q->>Proc: code-tunnel serve-web --connection-token-file
Proc-->>LQ: stdout/stderr via ServeWebOutputCollector
alt URL found in output
LQ->>Q: queue.async promote to serveWebProcess
Q->>UI: completion(serveWebURL) on main queue
else EADDRINUSE detected
LQ->>Proc: process.terminate()
LQ->>LQ: retry next candidate port
else startup timeout or failure
LQ->>UI: completion(nil)
end
end
UI->>Ctrl: restart(vscodeAppURL)
Ctrl->>Q: queue.sync bump lifecycleGeneration collect processes
Q->>Proc: process.terminate() SIGTERM
Q->>Q: install DispatchSourceTimer SIGKILL deadline
Proc-->>Q: terminationHandler fires finishTerminationWait
Q->>Q: finishStopTerminationBarrier drain deferred requests
Q->>Q: ensureServeWebURL with requiredLifecycleGeneration
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant UI as UI / Caller
participant Ctrl as VSCodeServeWebController
participant LQ as launchQueue
participant Q as queue (serial)
participant Builder as VSCodeCLILaunchConfigurationBuilder
participant TokenStore as VSCodeConnectionTokenStore
participant Proc as code-tunnel serve-web
UI->>Ctrl: ensureServeWebURL(vscodeAppURL)
Ctrl->>Q: queue.async check running / enqueue pending
Q->>LQ: launchQueue.async launchServeWebProcess
LQ->>Builder: launchConfiguration(vscodeAppURL)
Builder-->>LQ: VSCodeCLILaunchConfiguration
LQ->>TokenStore: ensureToken(at connectionTokenFile)
TokenStore-->>LQ: reused or newly-created 0600 token file
loop candidateStablePorts + [0]
LQ->>Q: queue.sync set launchingProcess run process
Q->>Proc: code-tunnel serve-web --connection-token-file
Proc-->>LQ: stdout/stderr via ServeWebOutputCollector
alt URL found in output
LQ->>Q: queue.async promote to serveWebProcess
Q->>UI: completion(serveWebURL) on main queue
else EADDRINUSE detected
LQ->>Proc: process.terminate()
LQ->>LQ: retry next candidate port
else startup timeout or failure
LQ->>UI: completion(nil)
end
end
UI->>Ctrl: restart(vscodeAppURL)
Ctrl->>Q: queue.sync bump lifecycleGeneration collect processes
Q->>Proc: process.terminate() SIGTERM
Q->>Q: install DispatchSourceTimer SIGKILL deadline
Proc-->>Q: terminationHandler fires finishTerminationWait
Q->>Q: finishStopTerminationBarrier drain deferred requests
Q->>Q: ensureServeWebURL with requiredLifecycleGeneration
Reviews (12): Last reviewed commit: "Tighten VS Code serve-web startup handli..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+526
to
+638
| enum VSCodeServeWebRuntimeLocator { | ||
| /// Override the serve-web server data directory (absolute path). | ||
| static let serverDataDirectoryEnvironmentKey = "CMUX_VSCODE_SERVE_WEB_DATA_DIR" | ||
| /// Override the stable serve-web port. | ||
| static let portEnvironmentKey = "CMUX_VSCODE_SERVE_WEB_PORT" | ||
| /// VS Code CLI data dir env var; honored as-is when already set. | ||
| static let cliDataDirectoryEnvironmentKey = "VSCODE_CLI_DATA_DIR" | ||
| /// UserDefaults key the resolved default port is persisted under. | ||
| static let portUserDefaultsKey = "vscodeServeWeb.port" | ||
|
|
||
| /// IANA dynamic/private port range (49152–65535) avoids well-known and | ||
| /// registered ports while still giving every bundle a stable default. | ||
| private static let minimumPort = 49152 | ||
| private static let portRangeSize = 16384 | ||
|
|
||
| static func resolve( | ||
| applicationSupportURL: URL, | ||
| bundleIdentifier: String, | ||
| environment: [String: String], | ||
| persistedPort: Int? | ||
| ) -> (location: VSCodeServeWebRuntimeLocation, portToPersist: Int?) { | ||
| let serverDataDirectoryURL = resolveServerDataDirectoryURL( | ||
| applicationSupportURL: applicationSupportURL, | ||
| bundleIdentifier: bundleIdentifier, | ||
| environment: environment | ||
| ) | ||
| let userDataDirectoryURL = serverDataDirectoryURL | ||
| .appendingPathComponent("user-data", isDirectory: true) | ||
| let cliDataDirectoryURL = resolveCLIDataDirectoryURL( | ||
| serverDataDirectoryURL: serverDataDirectoryURL, | ||
| environment: environment | ||
| ) | ||
| let connectionTokenFileURL = serverDataDirectoryURL | ||
| .appendingPathComponent("connection-token", isDirectory: false) | ||
| let (port, portToPersist) = resolvePort( | ||
| bundleIdentifier: bundleIdentifier, | ||
| environment: environment, | ||
| persistedPort: persistedPort | ||
| ) | ||
|
|
||
| return ( | ||
| VSCodeServeWebRuntimeLocation( | ||
| serverDataDirectoryURL: serverDataDirectoryURL, | ||
| userDataDirectoryURL: userDataDirectoryURL, | ||
| cliDataDirectoryURL: cliDataDirectoryURL, | ||
| connectionTokenFileURL: connectionTokenFileURL, | ||
| port: port | ||
| ), | ||
| portToPersist | ||
| ) | ||
| } | ||
|
|
||
| private static func resolveServerDataDirectoryURL( | ||
| applicationSupportURL: URL, | ||
| bundleIdentifier: String, | ||
| environment: [String: String] | ||
| ) -> URL { | ||
| if let override = environment[serverDataDirectoryEnvironmentKey], | ||
| !override.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { | ||
| return URL(fileURLWithPath: override, isDirectory: true) | ||
| } | ||
| return applicationSupportURL | ||
| .appendingPathComponent(bundleIdentifier, isDirectory: true) | ||
| .appendingPathComponent("vscode-serve-web", isDirectory: true) | ||
| } | ||
|
|
||
| private static func resolveCLIDataDirectoryURL( | ||
| serverDataDirectoryURL: URL, | ||
| environment: [String: String] | ||
| ) -> URL { | ||
| if let override = environment[cliDataDirectoryEnvironmentKey], | ||
| !override.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { | ||
| return URL(fileURLWithPath: override, isDirectory: true) | ||
| } | ||
| return serverDataDirectoryURL.appendingPathComponent("cli-data", isDirectory: true) | ||
| } | ||
|
|
||
| private static func resolvePort( | ||
| bundleIdentifier: String, | ||
| environment: [String: String], | ||
| persistedPort: Int? | ||
| ) -> (port: Int, portToPersist: Int?) { | ||
| if let override = environment[portEnvironmentKey], let parsed = parsePort(override) { | ||
| // Env overrides win but are intentionally not persisted as the default. | ||
| return (parsed, nil) | ||
| } | ||
| if let persistedPort, isValidPort(persistedPort) { | ||
| return (persistedPort, nil) | ||
| } | ||
| let derived = derivePort(from: bundleIdentifier) | ||
| return (derived, derived) | ||
| } | ||
|
|
||
| static func parsePort(_ raw: String) -> Int? { | ||
| let trimmed = raw.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| guard let value = Int(trimmed), isValidPort(value) else { return nil } | ||
| return value | ||
| } | ||
|
|
||
| static func isValidPort(_ port: Int) -> Bool { | ||
| (1024...65535).contains(port) | ||
| } | ||
|
|
||
| /// Deterministic per-bundle default so different (e.g. tagged) builds get | ||
| /// distinct, stable ports instead of colliding on one fixed value. | ||
| static func derivePort(from bundleIdentifier: String) -> Int { | ||
| var hash: UInt64 = 1469598103934665603 // FNV-1a 64-bit offset basis | ||
| for byte in bundleIdentifier.utf8 { | ||
| hash ^= UInt64(byte) | ||
| hash = hash &* 1099511628211 // FNV-1a 64-bit prime | ||
| } | ||
| return minimumPort + Int(hash % UInt64(portRangeSize)) | ||
| } |
Contributor
There was a problem hiding this comment.
VSCodeServeWebRuntimeLocator, VSCodeConnectionToken, VSCodeConnectionTokenStore, and VSCodeServeWebLaunchOptionsBuilder (lines 526, 644, 668, 736) are all caseless enum types whose entire API is static func/static let. The cmux-no-ambient-global-state rule flags exactly this shape — each of these should be a constructable, injectable struct that takes its dependencies (environment, file manager, etc.) through its initializer rather than reaching for ProcessInfo.processInfo, FileManager.default, or UserDefaults.standard directly. Consolidating them into a single new file (e.g. VSCodeServeWebPersistence.swift) would also address the file-size concern below. The pattern is consistent with the pre-existing VSCodeCLILaunchConfigurationBuilder/VSCodeServeWebURLBuilder debt; the fix here is to not extend it further.
Rule Used: Flag new ambient global state in production Swift:... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Contributor
Author
There was a problem hiding this comment.
These are pure, stateless transformations (or take FileManager via an injected default) and reach for no ambient globals — the controller injects ProcessInfo.environment, Bundle.main, UserDefaults, and FileManager into them, which keeps them fully unit-testable (see the new tests). They're kept as caseless-enum namespaces to match the established sibling builders in this same domain (VSCodeServeWebURLBuilder, VSCodeCLILaunchConfigurationBuilder) rather than introducing an inconsistent instance/static mix. They've now been moved into the dedicated VSCodeServeWebSupport.swift (99d3aff), which also resolves the file-size concern. The cmux file-organization policy check is now clean for this diff. Happy to convert them to injectable Service types in a follow-up if the team prefers that direction for the whole serve-web cluster.
— Claude Code
Comment on lines
+509
to
+777
| /// Stable on-disk locations + port for the inline serve-web server. Keeping these | ||
| /// fixed across launches is what lets VS Code Web's keyring/secret-storage survive | ||
| /// reloads, folder changes, and app relaunches (issue #6595). | ||
| struct VSCodeServeWebRuntimeLocation: Equatable { | ||
| /// `--server-data-dir`: where serve-web keeps its server-side state. | ||
| let serverDataDirectoryURL: URL | ||
| /// `--user-data-dir` for the cached code-server fallback (the wrapper derives | ||
| /// this from `--server-data-dir` and rejects the flag). | ||
| let userDataDirectoryURL: URL | ||
| /// `VSCODE_CLI_DATA_DIR` for the wrapper's CLI keyring metadata. | ||
| let cliDataDirectoryURL: URL | ||
| /// `--connection-token-file`: a persisted token so the server URL is stable. | ||
| let connectionTokenFileURL: URL | ||
| /// Stable serve-web port (or `0` for the ephemeral fallback attempt). | ||
| let port: Int | ||
| } | ||
|
|
||
| enum VSCodeServeWebRuntimeLocator { | ||
| /// Override the serve-web server data directory (absolute path). | ||
| static let serverDataDirectoryEnvironmentKey = "CMUX_VSCODE_SERVE_WEB_DATA_DIR" | ||
| /// Override the stable serve-web port. | ||
| static let portEnvironmentKey = "CMUX_VSCODE_SERVE_WEB_PORT" | ||
| /// VS Code CLI data dir env var; honored as-is when already set. | ||
| static let cliDataDirectoryEnvironmentKey = "VSCODE_CLI_DATA_DIR" | ||
| /// UserDefaults key the resolved default port is persisted under. | ||
| static let portUserDefaultsKey = "vscodeServeWeb.port" | ||
|
|
||
| /// IANA dynamic/private port range (49152–65535) avoids well-known and | ||
| /// registered ports while still giving every bundle a stable default. | ||
| private static let minimumPort = 49152 | ||
| private static let portRangeSize = 16384 | ||
|
|
||
| static func resolve( | ||
| applicationSupportURL: URL, | ||
| bundleIdentifier: String, | ||
| environment: [String: String], | ||
| persistedPort: Int? | ||
| ) -> (location: VSCodeServeWebRuntimeLocation, portToPersist: Int?) { | ||
| let serverDataDirectoryURL = resolveServerDataDirectoryURL( | ||
| applicationSupportURL: applicationSupportURL, | ||
| bundleIdentifier: bundleIdentifier, | ||
| environment: environment | ||
| ) | ||
| let userDataDirectoryURL = serverDataDirectoryURL | ||
| .appendingPathComponent("user-data", isDirectory: true) | ||
| let cliDataDirectoryURL = resolveCLIDataDirectoryURL( | ||
| serverDataDirectoryURL: serverDataDirectoryURL, | ||
| environment: environment | ||
| ) | ||
| let connectionTokenFileURL = serverDataDirectoryURL | ||
| .appendingPathComponent("connection-token", isDirectory: false) | ||
| let (port, portToPersist) = resolvePort( | ||
| bundleIdentifier: bundleIdentifier, | ||
| environment: environment, | ||
| persistedPort: persistedPort | ||
| ) | ||
|
|
||
| return ( | ||
| VSCodeServeWebRuntimeLocation( | ||
| serverDataDirectoryURL: serverDataDirectoryURL, | ||
| userDataDirectoryURL: userDataDirectoryURL, | ||
| cliDataDirectoryURL: cliDataDirectoryURL, | ||
| connectionTokenFileURL: connectionTokenFileURL, | ||
| port: port | ||
| ), | ||
| portToPersist | ||
| ) | ||
| } | ||
|
|
||
| private static func resolveServerDataDirectoryURL( | ||
| applicationSupportURL: URL, | ||
| bundleIdentifier: String, | ||
| environment: [String: String] | ||
| ) -> URL { | ||
| if let override = environment[serverDataDirectoryEnvironmentKey], | ||
| !override.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { | ||
| return URL(fileURLWithPath: override, isDirectory: true) | ||
| } | ||
| return applicationSupportURL | ||
| .appendingPathComponent(bundleIdentifier, isDirectory: true) | ||
| .appendingPathComponent("vscode-serve-web", isDirectory: true) | ||
| } | ||
|
|
||
| private static func resolveCLIDataDirectoryURL( | ||
| serverDataDirectoryURL: URL, | ||
| environment: [String: String] | ||
| ) -> URL { | ||
| if let override = environment[cliDataDirectoryEnvironmentKey], | ||
| !override.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty { | ||
| return URL(fileURLWithPath: override, isDirectory: true) | ||
| } | ||
| return serverDataDirectoryURL.appendingPathComponent("cli-data", isDirectory: true) | ||
| } | ||
|
|
||
| private static func resolvePort( | ||
| bundleIdentifier: String, | ||
| environment: [String: String], | ||
| persistedPort: Int? | ||
| ) -> (port: Int, portToPersist: Int?) { | ||
| if let override = environment[portEnvironmentKey], let parsed = parsePort(override) { | ||
| // Env overrides win but are intentionally not persisted as the default. | ||
| return (parsed, nil) | ||
| } | ||
| if let persistedPort, isValidPort(persistedPort) { | ||
| return (persistedPort, nil) | ||
| } | ||
| let derived = derivePort(from: bundleIdentifier) | ||
| return (derived, derived) | ||
| } | ||
|
|
||
| static func parsePort(_ raw: String) -> Int? { | ||
| let trimmed = raw.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| guard let value = Int(trimmed), isValidPort(value) else { return nil } | ||
| return value | ||
| } | ||
|
|
||
| static func isValidPort(_ port: Int) -> Bool { | ||
| (1024...65535).contains(port) | ||
| } | ||
|
|
||
| /// Deterministic per-bundle default so different (e.g. tagged) builds get | ||
| /// distinct, stable ports instead of colliding on one fixed value. | ||
| static func derivePort(from bundleIdentifier: String) -> Int { | ||
| var hash: UInt64 = 1469598103934665603 // FNV-1a 64-bit offset basis | ||
| for byte in bundleIdentifier.utf8 { | ||
| hash ^= UInt64(byte) | ||
| hash = hash &* 1099511628211 // FNV-1a 64-bit prime | ||
| } | ||
| return minimumPort + Int(hash % UInt64(portRangeSize)) | ||
| } | ||
| } | ||
|
|
||
| /// Connection-token format helpers. VS Code Web compares the URL `tkn` query item | ||
| /// against this file's contents, so a stable, valid token keeps the server URL — | ||
| /// and the browser-side session/cookies keyed to it — consistent across launches. | ||
| enum VSCodeConnectionToken { | ||
| private static let hexCharacters = Set("0123456789abcdefABCDEF") | ||
|
|
||
| static func isValid(_ token: String) -> Bool { | ||
| guard token.count == 32 else { return false } | ||
| return token.allSatisfy { hexCharacters.contains($0) } | ||
| } | ||
|
|
||
| /// 128 bits of randomness rendered as 32 lowercase hex characters. | ||
| static func generate() -> String { | ||
| let hexDigits = Array("0123456789abcdef") | ||
| var characters = [Character]() | ||
| characters.reserveCapacity(32) | ||
| for _ in 0..<16 { | ||
| let byte = UInt8.random(in: UInt8.min...UInt8.max) | ||
| characters.append(hexDigits[Int(byte >> 4)]) | ||
| characters.append(hexDigits[Int(byte & 0x0F)]) | ||
| } | ||
| return String(characters) | ||
| } | ||
| } | ||
|
|
||
| /// Reads/creates the persisted connection-token file, reusing a valid existing | ||
| /// token (32-hex, owner-only perms) and replacing anything invalid. | ||
| enum VSCodeConnectionTokenStore { | ||
| @discardableResult | ||
| static func ensureToken(at url: URL, fileManager: FileManager = .default) -> String? { | ||
| if let existing = readValidToken(at: url, fileManager: fileManager) { | ||
| return existing | ||
| } | ||
| return writeToken(VSCodeConnectionToken.generate(), to: url, fileManager: fileManager) | ||
| } | ||
|
|
||
| static func readValidToken(at url: URL, fileManager: FileManager) -> String? { | ||
| guard let data = try? Data(contentsOf: url), | ||
| let raw = String(data: data, encoding: .utf8) else { | ||
| return nil | ||
| } | ||
| let token = raw.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| guard VSCodeConnectionToken.isValid(token), | ||
| hasOwnerOnlyPermissions(at: url, fileManager: fileManager) else { | ||
| return nil | ||
| } | ||
| return token | ||
| } | ||
|
|
||
| static func hasOwnerOnlyPermissions(at url: URL, fileManager: FileManager) -> Bool { | ||
| guard let attributes = try? fileManager.attributesOfItem(atPath: url.path), | ||
| let permissions = attributes[.posixPermissions] as? NSNumber else { | ||
| return false | ||
| } | ||
| // No group/other bits set (e.g. 0600/0400). | ||
| return permissions.uint16Value & 0o077 == 0 | ||
| } | ||
|
|
||
| @discardableResult | ||
| static func writeToken(_ token: String, to url: URL, fileManager: FileManager) -> String? { | ||
| guard let tokenData = token.data(using: .utf8) else { return nil } | ||
| try? fileManager.createDirectory( | ||
| at: url.deletingLastPathComponent(), | ||
| withIntermediateDirectories: true | ||
| ) | ||
| // Drop any stale/invalid file so the strict-perms create below succeeds. | ||
| try? fileManager.removeItem(at: url) | ||
|
|
||
| let fileDescriptor = open(url.path, O_WRONLY | O_CREAT | O_EXCL, S_IRUSR | S_IWUSR) | ||
| guard fileDescriptor >= 0 else { return nil } | ||
| defer { _ = close(fileDescriptor) } | ||
|
|
||
| let wroteAllBytes = tokenData.withUnsafeBytes { rawBuffer in | ||
| guard let baseAddress = rawBuffer.baseAddress else { return false } | ||
| return write(fileDescriptor, baseAddress, rawBuffer.count) == rawBuffer.count | ||
| } | ||
| guard wroteAllBytes else { | ||
| try? fileManager.removeItem(at: url) | ||
| return nil | ||
| } | ||
| // Pin to 0600 in case umask widened the create mode. | ||
| try? fileManager.setAttributes([.posixPermissions: 0o600], ofItemAtPath: url.path) | ||
| return token | ||
| } | ||
| } | ||
|
|
||
| struct VSCodeServeWebLaunchOptions: Equatable { | ||
| let executableURL: URL | ||
| let arguments: [String] | ||
| let environment: [String: String] | ||
| } | ||
|
|
||
| /// Shapes the final process arguments + environment per launcher kind. The wrapper | ||
| /// and the cached code-server differ in supported flags and in how they manage the | ||
| /// secret keyring, so the two paths are handled explicitly here. | ||
| enum VSCodeServeWebLaunchOptionsBuilder { | ||
| static func launchOptions( | ||
| configuration: VSCodeCLILaunchConfiguration, | ||
| location: VSCodeServeWebRuntimeLocation, | ||
| port: Int | ||
| ) -> VSCodeServeWebLaunchOptions { | ||
| var arguments = configuration.argumentsPrefix | ||
| arguments += [ | ||
| "--accept-server-license-terms", | ||
| "--host", "127.0.0.1", | ||
| "--port", String(port), | ||
| "--connection-token-file", location.connectionTokenFileURL.path, | ||
| "--server-data-dir", location.serverDataDirectoryURL.path, | ||
| ] | ||
| var environment = configuration.environment | ||
|
|
||
| switch configuration.launcherKind { | ||
| case .codeTunnelWrapper: | ||
| // `code-tunnel serve-web` does not accept --user-data-dir; it derives | ||
| // user data from --server-data-dir. Enable the CLI file keyring so VS | ||
| // Code Web auth/Settings Sync persist instead of using in-memory | ||
| // secret storage, and pin the CLI data dir for keyring stability. | ||
| environment["VSCODE_CLI_USE_FILE_KEYRING"] = "1" | ||
| let cliDataKey = VSCodeServeWebRuntimeLocator.cliDataDirectoryEnvironmentKey | ||
| let cliDataDirIsUnset = environment[cliDataKey]? | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) | ||
| .isEmpty ?? true | ||
| if cliDataDirIsUnset { | ||
| environment[cliDataKey] = location.cliDataDirectoryURL.path | ||
| } | ||
| case .cachedCodeServer: | ||
| // The cached server binary accepts --user-data-dir directly. | ||
| arguments += ["--user-data-dir", location.userDataDirectoryURL.path] | ||
| } | ||
|
|
||
| return VSCodeServeWebLaunchOptions( | ||
| executableURL: configuration.executableURL, | ||
| arguments: arguments, | ||
| environment: environment | ||
| ) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
TerminalDirectoryOpenSupport.swift grew from 951 → 1241 lines (+290). The file already exceeded the 800-line guideline, and this PR adds four new independently-testable subsystems (token persistence, port resolution, location resolution, launch-options assembly) that have distinct responsibilities from the existing directory-detection and URL-building logic already in the file. The comprehensive test coverage introduced in cmuxTests/OmnibarAndToolsTests.swift is a signal that these subsystems are ready for their own source file or a small SwiftPM package boundary.
Rule Used: Flag Swift changes that add too much unrelated res... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Contributor
Author
There was a problem hiding this comment.
Done — split the serve-web subsystem into its own Sources/App/VSCodeServeWebSupport.swift (wired into the Xcode project + normalized). TerminalDirectoryOpenSupport.swift drops from 1241 to 342 lines and now only holds the directory-open targets + workspace-shortcut mapping. (99d3aff)
— Claude Code
This comment has been minimized.
This comment has been minimized.
Addresses the Aziz file-organization policy and review feedback: the new serve-web persistence types (launch-config builder, runtime location resolver, connection-token store, launch-options builder, controller, and output collector) are a cohesive subsystem and were appended to the already-large TerminalDirectoryOpenSupport.swift god file. Move all VS Code serve-web types into Sources/App/VSCodeServeWebSupport.swift (wired into the Xcode project + normalized), leaving directory-open targets and workspace-shortcut mapping behind. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The serve-web server-data, user-data, and CLI-data directories hold long-lived VS Code Web auth, Settings Sync, and CLI keyring state. They were created with default permissions, which under a wide umask could be group/other-traversable. Create (and re-pin existing) directories at 0700 so they match the 0600 connection-token file. Addresses review feedback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/App/VSCodeServeWebSupport.swift`:
- Around line 496-506: The VSCodeServeWebController test seam is currently
exposed through a `#if` DEBUG-only makeForTesting helper, which should not live in
production source. Remove makeForTesting, widen the init(launchProcessOverride:)
visibility from private to internal, and let the test target construct
VSCodeServeWebController directly via `@testable` import using the existing
launchProcessOverride injection point.
- Around line 862-915: `ServeWebOutputCollector` is using blocking
synchronization (`NSLock`, `DispatchSemaphore`, and `wait`) in production code,
which should be replaced with Swift concurrency primitives. Refactor the
collector into an `actor` (or equivalent async state holder) so `append(_:)`,
`markProcessExited()`, and `webUIURL` access are thread-safe without locks, and
change `waitForURL(timeoutSeconds:)` into an async API that returns the URL or
emits it through `AsyncStream`/`CheckedContinuation`. Remove the semaphore
signaling path entirely and preserve the existing URL parsing behavior via
`VSCodeServeWebURLBuilder.extractWebUIURL(from:)`.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7f7f9607-45bb-4ce7-b40e-86d7f8b75759
⛔ Files ignored due to path filters (1)
.github/swift-file-length-budget.tsvis excluded by!**/*.tsv
📒 Files selected for processing (4)
Sources/App/TerminalDirectoryOpenSupport.swiftSources/App/VSCodeServeWebSupport.swiftcmux.xcodeproj/project.pbxprojcmuxTests/OmnibarAndToolsTests.swift
💤 Files with no reviewable changes (1)
- Sources/App/TerminalDirectoryOpenSupport.swift
Per cmux policy (no test/debug seams in production Sources/**), drop the #if DEBUG makeForTesting factory on VSCodeServeWebController and widen its init(launchProcessOverride:) from private to internal so the test target constructs it directly via @testable import. Addresses CodeRabbit review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
Review feedback dispositionAddressed with code changes:
Consciously kept, with rationale (pre-existing / out-of-scope for a focused fix):
|
This comment has been minimized.
This comment has been minimized.
Previously, if the preferred stable port was already in use the launch fell back to an ephemeral port (--port 0). That changes the server origin on every relaunch, so users whose derived port happened to be occupied kept losing VS Code Web auth/Settings Sync across launches — the exact bug this fixes (#6595). Now the launch tries the preferred port, then deterministic STABLE alternates within the dynamic/private range, and persists whichever port actually binds (so the origin does not drift back to a still-occupied preferred port on a later launch). An ephemeral port is used only as a final last resort when every stable candidate is occupied. resolve() now returns just the location; the controller owns persistence of the bound port. Adds candidateStablePorts() coverage (preferred-first, deterministic, in-range, out-of-range-override handling) and updates the port-resolution tests. Addresses review feedback (autoreview). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…code-loses-settings-sync # Conflicts: # .github/swift-file-length-budget.tsv
…code-loses-settings-sync # Conflicts: # .github/swift-file-length-budget.tsv
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
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
3 participants
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.
Fixes #6595
Problem
Inline VS Code (Open Current Directory in VS Code (Inline)) launched VS Code Web through the cached
~/.vscode/cli/serve-web/<id>/bin/code-serverbinary directly when available. That bypasses VS Code'scode-tunnel serve-webwrapper, which sets up the CLI secret-storage/keyring path VS Code Web uses to persist GitHub auth and Settings Sync. Combined with an ephemeral--port 0and a fresh per-launch temporary connection-token, the inline server identity changed on every launch — so signing into GitHub / Settings Sync inside Inline VS Code was lost on pane reload, folder change, or app relaunch (extensions persisted while auth/settings did not).Fix
Launch through the wrapper and stabilize the server identity:
code-tunnel serve-web; fall back to the cachedcode-serveronly when the wrapper is unavailable (VSCodeServeWebLauncherKind).VSCODE_CLI_USE_FILE_KEYRING=1) and pinVSCODE_CLI_DATA_DIR.user-data/cli-datasubdirs.vscodeServeWeb.port), with an ephemeral-port fallback if the stable port can't be bound.0600) instead of a fresh temporary token per launch, so the server URL — and the browser session keyed to it — stays stable.stop()no longer deletes the token.--user-data-dirfor the wrapper; keep it for the cachedcode-serverfallback.CMUX_VSCODE_SERVE_WEB_DATA_DIR,CMUX_VSCODE_SERVE_WEB_PORT(user-facing config for serve-web options is a separate follow-up per the issue).Tests
Added/updated unit tests in
cmuxTests/OmnibarAndToolsTests.swift(already wired into the test target):launcherKind.VSCodeServeWebRuntimeLocator: stable path layout, server-data/cli-data env overrides, deterministic+persisted port, env port override, invalid-override handling, per-bundle distinct ports.VSCodeConnectionToken/VSCodeConnectionTokenStore: 32-hex validation, generation, file create with0600, reuse-if-valid, replace-if-invalid.VSCodeServeWebLaunchOptionsBuilder: wrapper enables keyring + omits--user-data-dir; cached code-server includes--user-data-dirand no keyring env; ephemeral fallback port reflected in args.Notes
Localizable.xcstringsor web message catalogs.scripts/swift_file_length_budget.pycanonical format).🤖 Generated with Claude Code
Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Fixes Inline VS Code losing GitHub auth and Settings Sync across reloads by launching VS Code Web via
code-tunnel serve-web, stabilizing server identity (data dirs, token, and a persisted stable port with deterministic alternates), and tightening startup/restart handling. Fixes #6595.Bug Fixes
code-tunnel serve-web; fall back to cachedcode-serveronly if the wrapper is missing.VSCODE_CLI_USE_FILE_KEYRING=1), stable CLI/server data dirs (0700), and a 0600--connection-token-file(validated 32-hex).0only as a last resort; persist the bound port.--user-data-dirfor the wrapper; wait for termination; defer relaunch until stop completes; relaunch after forced stop; retry only on detected port collisions.Refactors
Written for commit da16c78. Summary will update on new commits.
Summary by CodeRabbit
New Features
serve-webexperience.Bug Fixes