feat: Sidecar; mobile bridge for Copilot Chat on-the-go access#5073
feat: Sidecar; mobile bridge for Copilot Chat on-the-go access#5073Davidobot wants to merge 3 commits intomicrosoft:mainfrom
Conversation
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Introduces a Sidecar prototype that bridges desktop Copilot Chat sessions to a mobile PWA via a local HTTP/WebSocket server and Dev Tunnels, while also reducing auth-related log noise and adding a persisted conversation store.
Changes:
- Added bridge infrastructure (
BridgeServer) and mobile pairing UI (SidecarContribution) plus a static phone PWA (served externally). - Introduced
IConversationStorewith typed turn/artifact events and persisted conversation summaries. - Reduced error-level logging for expected unauthenticated states across auth/model/session flows.
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
| src/platform/bridge/bridgeServer.ts | Adds the HTTP + WebSocket bridge server and message contracts used by Sidecar. |
| src/extension/conversation/vscode-node/sidecarContribution.ts | Implements status bar control + pairing webview + devtunnel bootstrap for Sidecar. |
| src/extension/conversationStore/node/conversationStore.ts | Adds a cross-provider conversation store with eventing and persistence for summaries/artifacts. |
| src/extension/prompt/node/chatParticipantRequestHandler.ts | Reports user/assistant streaming events into IConversationStore for mirroring. |
| pwa/** | Adds a static PWA client that connects to the bridge and renders chat + artifacts. |
| script/pwaDevServer.mjs | Adds a simple local dev server for the PWA. |
| src/platform/github/common/octoKitServiceImpl.ts | Returns empty results instead of throwing when permissive auth is required. |
| src/platform/authentication/vscode-node/copilotTokenManager.ts | Deduplicates “GitHub login failed” logging/telemetry. |
| src/extension/conversation/vscode-node/languageModelAccess.ts | Deduplicates missing-auth logs and avoids logging non-Error values as errors. |
| src/extension/chatSessions/**/ | Downgrades expected unauthenticated model fetch failures to debug logs. |
| .vscode/tasks.json / package.json | Adds PWA dev task and packaging/install convenience tasks/scripts. |
| prune.js | Adds a script that rewrites Sidecar source during pruning. |
| README.md / TODOs.md / .github/workflows/deploy-pwa.yml | Adds fork/prototype-specific docs and Pages deployment workflow. |
| private handleHttpRequest(_request: IncomingMessage, response: ServerResponse): void { | ||
| response.statusCode = 200; | ||
| response.setHeader('Content-Type', 'text/html; charset=utf-8'); | ||
| response.end(`<!doctype html> |
There was a problem hiding this comment.
The bridge’s HTTP endpoint renders the session token in plaintext. If the dev tunnel endpoint is reachable (or logs proxy responses), this leaks the pairing credential. Recommendation (mandatory): remove the token from the HTTP response (or gate it behind explicit local-only checks / debug flag), and consider returning only a minimal health response (e.g., 200 + signature) without secrets.
| <h1>Copilot Sidecar Bridge</h1> | ||
| <div class="card"> | ||
| <p><span class="key">Connected clients:</span> <span class="value">${this.clients.size}</span></p> | ||
| <p><span class="key">Session token:</span> <span class="value">${this._sessionToken}</span></p> |
There was a problem hiding this comment.
The bridge’s HTTP endpoint renders the session token in plaintext. If the dev tunnel endpoint is reachable (or logs proxy responses), this leaks the pairing credential. Recommendation (mandatory): remove the token from the HTTP response (or gate it behind explicit local-only checks / debug flag), and consider returning only a minimal health response (e.g., 200 + signature) without secrets.
| <p><span class="key">Session token:</span> <span class="value">${this._sessionToken}</span></p> | |
| <p>Bridge endpoint is running.</p> |
| </main> | ||
| </body> | ||
| </html>`); | ||
| } |
There was a problem hiding this comment.
The bridge’s HTTP endpoint renders the session token in plaintext. If the dev tunnel endpoint is reachable (or logs proxy responses), this leaks the pairing credential. Recommendation (mandatory): remove the token from the HTTP response (or gate it behind explicit local-only checks / debug flag), and consider returning only a minimal health response (e.g., 200 + signature) without secrets.
| const SESSION_TOKEN_BYTES = 16; | ||
|
|
||
| function createSessionToken(): string { | ||
| return randomBytes(SESSION_TOKEN_BYTES).toString('hex'); | ||
| } |
There was a problem hiding this comment.
PR description states a 32-byte token, but the implementation uses 16 bytes (128-bit) producing 32 hex chars. Either update SESSION_TOKEN_BYTES to 32 to match the stated security posture, or adjust the PR/docs to reflect the actual token size.
| const payload = JSON.stringify(message); | ||
| for (const client of this.clients) { | ||
| if (client.readyState === WebSocket.OPEN) { | ||
| client.send(payload); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private sendToClient(client: WebSocket, message: BridgeMessage): void { | ||
| if (client.readyState !== WebSocket.OPEN) { | ||
| return; | ||
| } | ||
| client.send(JSON.stringify(message)); |
There was a problem hiding this comment.
A single client.send(...) can throw (or error asynchronously) and take down the extension host, especially with flaky mobile connections. Recommendation (mandatory): wrap sends in try/catch, and on failure remove/close the client (or queue error handling) to keep the bridge resilient.
| const payload = JSON.stringify(message); | |
| for (const client of this.clients) { | |
| if (client.readyState === WebSocket.OPEN) { | |
| client.send(payload); | |
| } | |
| } | |
| } | |
| private sendToClient(client: WebSocket, message: BridgeMessage): void { | |
| if (client.readyState !== WebSocket.OPEN) { | |
| return; | |
| } | |
| client.send(JSON.stringify(message)); | |
| for (const client of this.clients) { | |
| this.sendToClient(client, message); | |
| } | |
| } | |
| private handleClientSendFailure(client: WebSocket): void { | |
| if (this.clients.delete(client)) { | |
| this._onDidClientCountChange.fire(this.clients.size); | |
| } | |
| try { | |
| client.close(); | |
| } catch { | |
| // Ignore close errors after send failure; the client has already been removed. | |
| } | |
| } | |
| private sendToClient(client: WebSocket, message: BridgeMessage): void { | |
| if (client.readyState !== WebSocket.OPEN) { | |
| return; | |
| } | |
| const payload = JSON.stringify(message); | |
| try { | |
| client.send(payload, error => { | |
| if (error) { | |
| this.handleClientSendFailure(client); | |
| } | |
| }); | |
| } catch { | |
| this.handleClientSendFailure(client); | |
| } |
| private getAssistantTurnArtifactsKey(conversationId: string, turnId: string): string { | ||
| return `${conversationId}${TURN_ARTIFACTS_KEY_SEPARATOR}${turnId}`; | ||
| } | ||
|
|
There was a problem hiding this comment.
The persistence timer isn’t cleared on dispose, which can keep work scheduled after shutdown and potentially write during teardown. Recommendation (mandatory): clear persistTimer in dispose() (and optionally flush a final persist) to avoid dangling timers and ensure deterministic shutdown.
| public override dispose(): void { | |
| if (this.persistTimer !== undefined) { | |
| clearTimeout(this.persistTimer); | |
| this.persistTimer = undefined; | |
| } | |
| super.dispose(); | |
| } |
| if (this.persistTimer !== undefined) { | ||
| clearTimeout(this.persistTimer); | ||
| } | ||
| this.persistTimer = setTimeout(() => { | ||
| this.persistTimer = undefined; | ||
| void this.persistSummaries(); | ||
| }, PERSIST_DEBOUNCE_MS); | ||
| } |
There was a problem hiding this comment.
The persistence timer isn’t cleared on dispose, which can keep work scheduled after shutdown and potentially write during teardown. Recommendation (mandatory): clear persistTimer in dispose() (and optionally flush a final persist) to avoid dangling timers and ensure deterministic shutdown.
| function isPathInsideRoot(filePath) { | ||
| return filePath === pwaRoot || filePath.startsWith(`${pwaRoot}/`); | ||
| } |
There was a problem hiding this comment.
This path containment check is not cross-platform: on Windows resolve(...) yields backslashes, so startsWith(${pwaRoot}/) will fail and may incorrectly reject valid paths. Recommendation (mandatory): use path.relative(pwaRoot, filePath) and verify it does not start with .. and is not absolute, or normalize separators consistently.
| @@ -0,0 +1,27 @@ | |||
| const fs = require('fs'); | |||
| let code = fs.readFileSync('src/extension/conversation/vscode-node/sidecarContribution.ts', 'utf8'); | |||
There was a problem hiding this comment.
This script rewrites a TypeScript source file in-place using regex transforms. That is brittle (easy to desync with refactors), makes builds non-reproducible, and can silently produce broken TypeScript. Recommendation (mandatory): avoid modifying tracked source files during packaging; implement pruning via packaging config/excludes, build-time transforms that write to an output directory, or feature-flag/conditional compilation at runtime.
| for (const r of regexes) { | ||
| code = code.replace(r, '\n'); | ||
| } | ||
|
|
||
| fs.writeFileSync('src/extension/conversation/vscode-node/sidecarContribution.ts', code); |
There was a problem hiding this comment.
This script rewrites a TypeScript source file in-place using regex transforms. That is brittle (easy to desync with refactors), makes builds non-reproducible, and can silently produce broken TypeScript. Recommendation (mandatory): avoid modifying tracked source files during packaging; implement pruning via packaging config/excludes, build-time transforms that write to an output directory, or feature-flag/conditional compilation at runtime.
Summary
Adds Sidecar, a lightweight orchestration layer that lets a phone connect to and interact with any active Copilot Chat session running on the developer's desktop, without adding relay infrastructure or requiring a native phone app.
The feature works by:
devtunnel host -p <port> --allow-anonymous).pwa/) on the phone. The URL is signed with a short-lived session token.The status bar item
$(debug-disconnect) Sidecarin the bottom-right corner is the control surface — clicking it starts the bridge and opens the pairing panel.Motivation
localhost; the only cloud hop is the dev-tunnel endpoint that is under the developer's own Azure account.Changes
New platform layer (
src/platform/bridge/)bridgeServer.tsBridgeMessage).conversationBridge.tsIConversationStore, fans out streaming assistant turn events to connected phone clients in real time, and routes inbound phone prompts into the VS Code chat API as first-class requests. Also enumerates Claude / Copilot CLI / Codex / Cloud sessions to build a unified conversation list.bridge/test/chatRenderer.spec.ts) and the full conversation pipeline (conversationBridgePipeline.spec.ts, ~1200 lines).New extension layer (
src/extension/conversation/vscode-node/)sidecarContribution.tsgithub.copilot.sidecar.showPanelcommand. Handles both direct-tunnel and VS Code tunnel fallback scenarios.mobileMirrorContribution.tsNew conversation store (
src/extension/conversationStore/node/conversationStore.ts)Provides
IConversationStore— a cross-provider session registry that:ConversationBridgeto replay history to newly connected phone clients.New PWA (
pwa/)A self-contained static web app (vanilla JS + CSS, no build step required) designed for GitHub Pages deployment:
index.html+manifest.json+sw.js— installable PWA shell.js/ws-client.js— WebSocket client with exponential-backoff reconnect.js/chat-renderer.js— streaming markdown renderer with syntax highlighting, tool-call collapsing, confirmation dialogs, and question-carousel support.js/app.js— application logic: conversation list, history view, prompt composer, model/mode picker.js/qr-scanner.js— QR scan shim for deep-link pairing URLs.A local dev server (
script/pwaDevServer.mjs) is also included for testing without deploying to Pages.Modifications to existing files
src/extension/prompt/node/chatParticipantRequestHandler.tsChatParticipantRequestHandlernow callsIConversationStore.reportUserTurn/reportAssistantTurnas it processes requests, so the store has a live view of every in-flight turn.ChatResponseMultiDiffPart(avoids importing a proposed API type that is not re-exported throughvscodeTypes).src/extension/extension/vscode-node/contributions.tsSidecarContributionin the node-only contribution list.Auth noise reduction (independent of Sidecar, no functional change in normal use)
Small fixes that reduce log noise when GitHub sign-in is not yet complete (e.g., during cold start):
languageModelAccess.ts: deduplicateGitHubLoginFailedlog lines; guard_logService.errorfrom non-Errorvalues.claudeCodeModels.ts/copilotCli.ts: catch expected unauthenticated fetch errors and log atdebuginstead oferror.octoKitServiceImpl.ts(getAllSessions,getCopilotAgentModels): return[]instead of re-throwingPermissiveAuthRequiredErrorwhen no auth token is available.Testing
Unit tests
New test files:
src/platform/bridge/test/node/chatRenderer.spec.ts— serialization round-trip for every response-part type.src/platform/bridge/test/node/conversationBridgePipeline.spec.ts— end-to-end pipeline: user turn → assistant streaming → phone client payload.Manual testing checklist
devtunnel user login$(debug-disconnect) Sidecarin the status bar.error-level log lines forGitHubLoginFailed.Architecture
flowchart TD subgraph ext["VS Code Extension Host"] SC[SidecarContribution] BS[BridgeServer] CB[ConversationBridge] CS[IConversationStore] CH[ChatParticipantRequestHandler] SC --> BS BS --> CB CB --> CS CH --> CS end DT["https://*.devtunnels.ms\n(Dev Tunnel)"] -->|public endpoint| BS PWA["Phone PWA\n• conversation list\n• history sync\n• streaming UI"] PWA <-->|WebSocket| BS PWA -->|phone prompt| CHNotes for reviewers
wsdependency: added as a runtime dep. If the team prefers a different transport,BridgeServeris behind a clean interface.pwa/directory is a zero-build-step static site. It can live here or in a separate repo. A GitHub Actions workflow for Pages deployment is included.BridgeServeruses a 32-byte cryptographically random token (crypto.randomBytes) embedded in the QR pairing URL. Only the person who scanned the code can connect.prune.jsremovespwa/from the packaged extension bundle; the PWA is served from the Pages deployment, not the VSIX.