feat(vscode): configure remote gRPC daemon address - #105
Conversation
Allow the extension to attach to a container or TCP Abbenay daemon via abbenay.daemonAddress (TLS/CA/token), while keeping local IPC auto-start when unset. Reject HTTP :8787 and URL userinfo; coalesce reconnects. Also bump js-yaml to 4.3.1 (GHSA-5p4m-2wfm-xmqj).
There was a problem hiding this comment.
Pull request overview
Adds VS Code extension support for connecting to a remote/container Abbenay daemon over gRPC (host:port) with TLS/CA and consumer token support, while preserving existing local IPC auto-start behavior when unset.
Changes:
- Introduces new
abbenay.daemon*settings, token commands backed by VS Code SecretStorage, and reconnect-on-settings/token-change behavior. - Implements remote TCP+TLS connection path in
DaemonClient, including optional CA loading andx-abbenay-tokenmetadata attachment. - Updates docs/changelog and adds unit tests for connection configuration and dashboard URL derivation.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/vscode/src/test/extension.test.ts | Extends smoke tests to include new commands and default settings. |
| packages/vscode/src/test/daemon/connection-config.test.ts | Adds unit tests for address normalization, config reading, token resolution, and dashboard URL derivation. |
| packages/vscode/src/extension.ts | Adds connection/reconnect orchestration, SecretStorage-backed token commands, and remote-aware dashboard URL handling. |
| packages/vscode/src/daemon/index.ts | Documents new remote/container connection option. |
| packages/vscode/src/daemon/connection-config.ts | Implements normalization + config resolution for local vs remote daemon connection settings. |
| packages/vscode/src/daemon/client.ts | Adds remote gRPC connection with TLS/CA and token metadata; improves connect/cleanup/reset behavior. |
| packages/vscode/README.md | Documents new settings, token commands, and remote/container setup guidance. |
| packages/vscode/package.json | Contributes new settings and token commands to VS Code extension manifest. |
| packages/vscode/CHANGELOG.md | Notes remote gRPC support and token handling in release notes. |
| packages/daemon/package.json | Bumps js-yaml to ^4.3.1. |
| package-lock.json | Lockfile updates reflecting dependency/version changes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Share port validation across IPv4/hostname and bracket IPv6 paths so [::1]:8787 is rejected like 127.0.0.1:8787. Strengthen normalize tests for userinfo and the IPv6 dashboard footgun.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/vscode/src/daemon/client.ts:288
- In DaemonClient.connect(), the catch block resets connectionMode to "local" even when the current settings indicate a remote connection attempt. This makes downstream status/error messages (e.g., daemonStatus) misleading after a failed remote connect; the client should retain the mode derived from settings and only clear transport state.
} catch (err) {
await this.closeChannelOnly();
this.connectionMode = 'local';
this.address = DEFAULT_DAEMON_ADDRESS;
throw err;
packages/vscode/src/daemon/connection-config.ts:43
- normalizeDaemonAddress() only strips http/https schemes; other pasted URL schemes like "grpc://" or "tcp://" will be treated as part of the hostname and later fail in channel creation. Since the function is intended to accept accidental URL forms, strip any RFC3986-style scheme prefix, not just http(s).
// Accept accidental URL forms; gRPC wants host:port.
value = value.replace(/^https?:\/\//i, '');
// Drop path/query if pasted as a URL.
packages/vscode/src/test/daemon/connection-config.test.ts:30
- The normalizeDaemonAddress test suite claims to cover stripping URL schemes, but it doesn't currently include a non-http(s) scheme example. Adding a "grpc://" case will prevent regressions if scheme handling is broadened.
assert.strictEqual(normalizeDaemonAddress(' 127.0.0.1:50051 '), '127.0.0.1:50051');
assert.strictEqual(normalizeDaemonAddress('http://127.0.0.1:50051'), '127.0.0.1:50051');
assert.strictEqual(normalizeDaemonAddress('https://host:50051/path'), 'host:50051');
assert.strictEqual(normalizeDaemonAddress('[::1]:50051'), '[::1]:50051');
assert.strictEqual(normalizeDaemonAddress(''), undefined);
Strip any URL scheme (grpc/tcp/http) when normalizing daemonAddress, keep connectionMode from settings after a failed remote attach, and cover grpc:// / tcp:// in unit tests.
|
Addressed Copilot’s suppressed follow-ups from the post-
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 11 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/vscode/src/daemon/client.ts:911
- initializeDaemon() now calls client.close() in the catch path. DaemonClient.close() resets connectionMode/address to local defaults, which defeats the “keep connection mode from settings after a failed remote attach” behavior (errors/status will read as local after a timeout/failed remote connect).
Consider using a cleanup path that only closes the channel/client without resetting the mode/address (e.g., add a dedicated public cleanup method or an optional parameter to close() to avoid resetting state on failed initialization).
} catch (err) {
// Roll back half-open channel if connect raced past the deadline.
await client.close().catch(() => {});
throw err;
Use disposeTransport() in initializeDaemon's catch path so a failed or timed-out attach does not reset connectionMode/address to local defaults.
|
Follow-up to Copilot’s suppressed note on |
sudhirverma
left a comment
sudhirverma
left a comment
There was a problem hiding this comment.
Verified: lint clean, tsc compile clean, unit-test logic reviewed in depth (couldn't run full vscode-test e2e locally — no Electron in this sandbox).
Solid PR — clean separation in connection-config.ts (well-tested edge cases: :8787 rejection for IPv4/IPv6, userinfo stripping, scheme stripping, token precedence), good security instincts (token never logged, SecretStorage preferred), and the connect/reconnect state machine in client.ts/extension.ts correctly handles failed-attach and rapid setting-change cases (matches the 3 follow-up hardening commits in this PR's history).
Non-blocking nitpicks for a future pass:
normalizeDaemonAddress's explicitunix:prefix check is effectively dead forunix:///path— the URL-scheme-strip regex consumesunix://first; it still ends up rejected via slash-truncation, just not through the path the comment implies.- TLS without
daemonCaPathsilently falls back to the system trust store (warning-only log) — a self-signed container cert will fail with a possibly-unclear gRPC error rather than an upfront actionable message. setDaemonToken/clearDaemonTokentrigger a reconnect even in local mode, where the setting is unused.openDashboard:fallbackUrlderived fromdaemonAddressalways wins over the daemon's ownstartWebServerresponse URL when remote — fine for the common case, could diverge if the daemon binds a different interface.
LGTM to merge.
Summary
host:port, typically:50051) with TLS, CA, and consumer token supportabbenay.daemonAddressis unset, keep existing local IPC detection + SEA/PATH auto-start:8787(IPv4 and IPv6), URL userinfo, and strip accidental schemes (http(s),grpc,tcp)js-yamlto4.3.1(GHSA-5p4m-2wfm-xmqj)Changes
abbenay.daemonAddress,daemonTls,daemonCaPath,daemonSslTargetName,daemonTokenEnvx-abbenay-token)DaemonClientremote TCP+TLS path; channel cleanup on failed connect; sticky-address fix for remote→local:8787when a remote gRPC address is configuredTest plan
cd packages/vscode && npm run lintcd packages/vscode && npm test(41 passing)daemonAddress) still auto-starts / attaches to local daemon-p 50051:50051+--grpc-tls, set address/CA/token, verify status showsremote:… (tls)and chat worksdaemonAddressand confirm reconnect returns to local IPC