From 93b485ffb6f13b072f7f47560dde7ea81221b15d Mon Sep 17 00:00:00 2001 From: mrleemurray Date: Mon, 20 Apr 2026 11:25:15 +0100 Subject: [PATCH 01/17] Refactor chat UI styles for improved alignment and consistency --- .../contrib/chat/browser/media/chatInput.css | 25 ++++++----- .../contrib/chat/browser/media/chatWidget.css | 42 +++++++++++++++---- .../chat/browser/media/newChatInSession.css | 2 +- .../contrib/chat/browser/newChatInput.ts | 2 +- 4 files changed, 52 insertions(+), 19 deletions(-) diff --git a/src/vs/sessions/contrib/chat/browser/media/chatInput.css b/src/vs/sessions/contrib/chat/browser/media/chatInput.css index 02ebdad018ca6..1a991076a3253 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatInput.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatInput.css @@ -62,7 +62,7 @@ /* Height constraints are driven by MIN_EDITOR_HEIGHT / MAX_EDITOR_HEIGHT in newChatViewPane.ts */ .sessions-chat-editor { - padding: 0 8px 6px 10px; + padding: 6px 6px 0 6px; flex-shrink: 1; } @@ -77,8 +77,8 @@ .sessions-chat-toolbar { display: flex; align-items: center; - padding: 0 6px 6px 6px; - gap: 4px; + padding: 4px 6px 6px 6px; + gap: 6px; color: var(--vscode-icon-foreground); } @@ -108,11 +108,10 @@ display: flex; align-items: center; height: 16px; - padding: 3px 3px 3px 6px; + padding: 3px 1px 3px 7px; background-color: transparent; border: none; border-radius: 4px; - font-size: 13px; cursor: pointer; touch-action: manipulation; color: var(--vscode-icon-foreground); @@ -126,14 +125,20 @@ color: var(--vscode-foreground); } +.sessions-chat-config-toolbar .action-label, +.sessions-chat-config-toolbar .action-label .chat-input-picker-label { + font-size: 12px; +} + .sessions-chat-config-toolbar .action-label .codicon { - font-size: 14px; + font-size: 12px !important; flex-shrink: 0; } .sessions-chat-config-toolbar .action-label .codicon-chevron-down { - font-size: 12px; - margin-left: 6px; + font-size: 10px !important; + margin-left: 4px; + opacity: 0.75; } /* Send button - wraps a Button widget */ @@ -166,7 +171,7 @@ } .sessions-chat-send-button .monaco-button .codicon { - font-size: 16px; + font-size: 14px; } /* Loading spinner in toolbar */ @@ -231,7 +236,7 @@ } .sessions-chat-attach-button .codicon { - font-size: 16px; + font-size: 14px; } /* Attached context container */ diff --git a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css index a2c0ee31efcef..0849f2e10db2b 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css @@ -102,6 +102,26 @@ min-width: 0; } +/* Match VS Code chat-secondary-toolbar sizing for action items in the bottom row + * (e.g. session control pickers and repo config pickers) so they align with the + * mode/model picker font in the input toolbar. */ +.new-chat-widget-container .new-chat-bottom-container .action-label { + height: 16px; + padding: 3px 1px 3px 7px; + font-size: 12px; + color: var(--vscode-icon-foreground); +} + +.new-chat-widget-container .new-chat-bottom-container .action-label .codicon { + font-size: 12px !important; +} + +.new-chat-widget-container .new-chat-bottom-container .action-label .codicon-chevron-down { + font-size: 10px !important; + margin-left: 4px; + opacity: 0.75; +} + .chat-controls-container .monaco-editor-background { background-color: var(--vscode-input-background) !important; } @@ -168,7 +188,7 @@ } .sessions-chat-dropdown-label { - margin-left: 4px; + margin-left: 2px; } .sessions-chat-picker-slot { @@ -182,11 +202,10 @@ display: flex; align-items: center; height: 16px; - padding: 3px 3px 3px 6px; + padding: 3px 1px 3px 7px; background-color: transparent; border: none; color: var(--vscode-icon-foreground); - font-size: 13px; cursor: pointer; touch-action: manipulation; white-space: nowrap; @@ -245,18 +264,27 @@ } .sessions-chat-picker-slot .action-label .codicon { - font-size: 14px; + font-size: 12px; flex-shrink: 0; + /* Override the base .monaco-action-bar .action-item .codicon { width:16px; height:16px } + * so codicons here size naturally to their font-size, matching pickers (like Copilot CLI) + * that are rendered outside a monaco-action-bar. */ + width: auto; + height: auto; } .sessions-chat-picker-slot .action-label .codicon-chevron-down { display: inline-flex; align-items: center; justify-content: center; - font-size: 12px; - margin-left: 6px; + font-size: 10px; + margin-left: 4px; + opacity: 0.75; line-height: 1; - transform: translateY(1px); + /* Match the 16x16 codicon box that the action-bar gives the regular chat + * picker's chevron, so the small chevron icon has equal padding on both sides. */ + width: 16px; + height: 16px; } .sessions-chat-picker-slot .action-label .chat-session-option-label { diff --git a/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css b/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css index 4952777247bd2..155ee8f5e0541 100644 --- a/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css +++ b/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css @@ -95,7 +95,7 @@ } .new-chat-in-session .sessions-chat-dropdown-label { - margin-left: 4px; + margin-left: 2px; } /* Disable fade-in animations — show content immediately */ diff --git a/src/vs/sessions/contrib/chat/browser/newChatInput.ts b/src/vs/sessions/contrib/chat/browser/newChatInput.ts index c4ad8638e37b1..33d9d5fb003d8 100644 --- a/src/vs/sessions/contrib/chat/browser/newChatInput.ts +++ b/src/vs/sessions/contrib/chat/browser/newChatInput.ts @@ -384,7 +384,7 @@ export class NewChatInputWidget extends Disposable implements IHistoryNavigation title: localize('send', "Send"), ariaLabel: localize('send', "Send"), })); - sendButton.icon = Codicon.send; + sendButton.icon = Codicon.arrowUp; this._register(sendButton.onDidClick(() => this._send())); } From 4c9b954dcb114f4b1b28929fafcfdc6ad1280653 Mon Sep 17 00:00:00 2001 From: mrleemurray Date: Mon, 20 Apr 2026 11:28:52 +0100 Subject: [PATCH 02/17] Add spacing between action items in chat controls and repo-config toolbar Co-authored-by: Copilot --- src/vs/sessions/contrib/chat/browser/media/chatWidget.css | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css index 0849f2e10db2b..b42c2b32187a8 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css @@ -93,6 +93,7 @@ .new-chat-widget-container .new-chat-bottom-container .new-chat-controls-container { display: flex; + gap: 4px; } .new-chat-widget-container .new-chat-bottom-container .new-chat-repo-config-container { @@ -102,6 +103,11 @@ min-width: 0; } +/* Spacing between action items inside the repo-config toolbar (e.g. Worktree, branch) */ +.new-chat-widget-container .monaco-action-bar .actions-container { + gap: 4px; +} + /* Match VS Code chat-secondary-toolbar sizing for action items in the bottom row * (e.g. session control pickers and repo config pickers) so they align with the * mode/model picker font in the input toolbar. */ From 184d892d5d153bfe7c0b6f8cc9b90d9ac95c0030 Mon Sep 17 00:00:00 2001 From: mrleemurray Date: Mon, 20 Apr 2026 11:31:53 +0100 Subject: [PATCH 03/17] Adjust spacing and padding in chat widget for improved layout --- src/vs/sessions/contrib/chat/browser/media/chatWidget.css | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css index b42c2b32187a8..bcc36497017f0 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css @@ -39,7 +39,6 @@ display: none; width: 100%; max-width: 800px; - margin: 0 0 8px 0; padding: 0; box-sizing: border-box; } @@ -77,7 +76,8 @@ .new-chat-widget-container .new-chat-bottom-container { width: 100%; max-width: 800px; - margin-top: 8px; + margin-top: 4px; + padding: 0 4px; box-sizing: border-box; display: none; flex-direction: row; From 9c4313db2136705d578c6b09a1bbcd563ef47d79 Mon Sep 17 00:00:00 2001 From: mrleemurray Date: Mon, 20 Apr 2026 12:18:39 +0100 Subject: [PATCH 04/17] Enhance chat widget and config toolbar styles for better label ellipsizing and overflow handling Co-authored-by: Copilot --- .../contrib/chat/browser/media/chatInput.css | 20 ++++++++- .../contrib/chat/browser/media/chatWidget.css | 42 +++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/src/vs/sessions/contrib/chat/browser/media/chatInput.css b/src/vs/sessions/contrib/chat/browser/media/chatInput.css index 1a991076a3253..425c02b5a52ef 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatInput.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatInput.css @@ -97,11 +97,21 @@ .sessions-chat-config-toolbar .monaco-toolbar { height: auto; + min-width: 0; + overflow: hidden; +} + +.sessions-chat-config-toolbar .monaco-action-bar, +.sessions-chat-config-toolbar .monaco-action-bar .actions-container { + min-width: 0; + overflow: hidden; } .sessions-chat-config-toolbar .monaco-action-bar .action-item { display: flex; align-items: center; + min-width: 48px; + overflow: hidden; } .sessions-chat-config-toolbar .action-label { @@ -116,7 +126,7 @@ touch-action: manipulation; color: var(--vscode-icon-foreground); white-space: nowrap; - min-width: 0; + min-width: 38px; overflow: hidden; } @@ -130,6 +140,14 @@ font-size: 12px; } +/* Allow long labels (e.g. the model picker name) to ellipsize when space is tight */ +.sessions-chat-config-toolbar .action-label .chat-input-picker-label { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + min-width: 0; +} + .sessions-chat-config-toolbar .action-label .codicon { font-size: 12px !important; flex-shrink: 0; diff --git a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css index bcc36497017f0..b14d87e2c5602 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css @@ -94,6 +94,8 @@ .new-chat-widget-container .new-chat-bottom-container .new-chat-controls-container { display: flex; gap: 4px; + min-width: 0; + overflow: hidden; } .new-chat-widget-container .new-chat-bottom-container .new-chat-repo-config-container { @@ -101,6 +103,42 @@ align-items: center; gap: 2px; min-width: 0; + overflow: hidden; +} + +/* Allow nested toolbar items to shrink so labels can ellipsize when space is tight. + * Mirrors the regular chat-input-toolbar pattern: each flex layer between the + * bounded container and the ellipsizing label gets `min-width: 0; overflow: hidden`. */ +.new-chat-widget-container .new-chat-bottom-container .new-chat-controls-container > *, +.new-chat-widget-container .new-chat-bottom-container .new-chat-repo-config-container > *, +.new-chat-widget-container .new-chat-bottom-container .monaco-action-bar .action-item, +.new-chat-widget-container .new-chat-bottom-container .sessions-chat-picker-slot .action-label { + min-width: 0; + overflow: hidden; +} + +/* Floor each picker so the icon + chevron (+ padding) stay visible even when + * the label is fully ellipsized. Approx: 7px padding-left + 12px icon + 2px + * label margin + 16px chevron box + 1px padding-right ~= 38px. */ +.new-chat-widget-container .new-chat-bottom-container .sessions-chat-picker-slot .action-label, +.new-chat-widget-container .new-chat-bottom-container .monaco-action-bar .action-item .action-label { + min-width: 38px; +} + +/* Below this width the bottom-row pickers can't fit their labels comfortably, + * so collapse to icon + chevron only. The .new-chat-widget-container declares + * `container-type: size` which makes this a size container query. */ +@container (max-width: 320px) { + /* Bottom-row pickers (Copilot CLI, Default Approvals, Worktree, branch): icon-only */ + .new-chat-widget-container .new-chat-bottom-container .sessions-chat-dropdown-label { + display: none; + } + + /* Chat input config toolbar: hide mode picker label (uses sessions-chat-dropdown-label), + * but keep the model picker label (uses chat-input-picker-label) visible. */ + .new-chat-widget-container .sessions-chat-config-toolbar .sessions-chat-dropdown-label { + display: none; + } } /* Spacing between action items inside the repo-config toolbar (e.g. Worktree, branch) */ @@ -195,6 +233,10 @@ .sessions-chat-dropdown-label { margin-left: 2px; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + min-width: 0; } .sessions-chat-picker-slot { From bbbde53600e91d5f247bbfbe9b68413e1e1512b2 Mon Sep 17 00:00:00 2001 From: mrleemurray Date: Mon, 20 Apr 2026 12:53:15 +0100 Subject: [PATCH 05/17] Refactor chat widget and input styles for improved alignment and spacing Co-authored-by: Copilot --- .../contrib/chat/browser/media/chatInput.css | 28 +++++++++++++--- .../contrib/chat/browser/media/chatWidget.css | 32 +++++++++++-------- .../chat/browser/media/newChatInSession.css | 6 +++- 3 files changed, 47 insertions(+), 19 deletions(-) diff --git a/src/vs/sessions/contrib/chat/browser/media/chatInput.css b/src/vs/sessions/contrib/chat/browser/media/chatInput.css index 425c02b5a52ef..826d05bfd83d4 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatInput.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatInput.css @@ -148,21 +148,38 @@ min-width: 0; } -.sessions-chat-config-toolbar .action-label .codicon { - font-size: 12px !important; +.sessions-chat-config-toolbar .monaco-action-bar .action-item .action-label > .codicon { + font-size: 12px; flex-shrink: 0; + /* Override the base .monaco-action-bar .action-item .codicon { width:16px; height:16px } + * so the leading icon sizes naturally to its font-size, matching the bottom-row pickers. */ + width: auto; + height: auto; } -.sessions-chat-config-toolbar .action-label .codicon-chevron-down { - font-size: 10px !important; +.sessions-chat-config-toolbar .monaco-action-bar .action-item .action-label > .codicon-chevron-down { + display: inline-flex; + align-items: center; + justify-content: center; + font-size: 10px; margin-left: 4px; - opacity: 0.75; + line-height: 1; + /* Restore the 16x16 codicon box for the chevron so it has equal padding on both sides, + * matching the bottom-row picker chevrons. */ + width: 16px; + height: 16px; +} + +/* Spacing between action items inside the chat input config toolbar (mode + model picker) */ +.sessions-chat-config-toolbar .monaco-action-bar .actions-container { + gap: 4px; } /* Send button - wraps a Button widget */ .sessions-chat-send-button { display: flex; align-items: center; + flex-shrink: 0; } .sessions-chat-send-button .monaco-button { @@ -236,6 +253,7 @@ justify-content: center; width: 22px; height: 22px; + flex-shrink: 0; border-radius: 4px; cursor: pointer; color: var(--vscode-icon-foreground); diff --git a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css index b14d87e2c5602..c7ea9a05049f8 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css @@ -14,13 +14,17 @@ .new-chat-widget-container { display: flex; flex-direction: column; - align-items: center; + /* Cross-axis: keep children flush left so the input's left edge — and therefore + * the attach (+) button — stays anchored as the viewport narrows past max-width. */ + align-items: flex-start; justify-content: center; width: 100%; height: 100%; box-sizing: border-box; overflow: hidden; padding: 16px 16px 20px 16px; + /* Establishes a size container so the @container (max-width: 320px) query below + * can collapse picker labels to icon-only when the new-chat area is narrow. */ container-type: size; position: relative; } @@ -111,7 +115,6 @@ * bounded container and the ellipsizing label gets `min-width: 0; overflow: hidden`. */ .new-chat-widget-container .new-chat-bottom-container .new-chat-controls-container > *, .new-chat-widget-container .new-chat-bottom-container .new-chat-repo-config-container > *, -.new-chat-widget-container .new-chat-bottom-container .monaco-action-bar .action-item, .new-chat-widget-container .new-chat-bottom-container .sessions-chat-picker-slot .action-label { min-width: 0; overflow: hidden; @@ -119,16 +122,20 @@ /* Floor each picker so the icon + chevron (+ padding) stay visible even when * the label is fully ellipsized. Approx: 7px padding-left + 12px icon + 2px - * label margin + 16px chevron box + 1px padding-right ~= 38px. */ + * label margin + 16px chevron box + 1px padding-right ~= 38px. The floor must + * be applied to the outermost flex item (.action-item), not just the label, + * because the parent's `min-width: 0` would otherwise let it clip the chevron. */ +.new-chat-widget-container .new-chat-bottom-container .monaco-action-bar .action-item, .new-chat-widget-container .new-chat-bottom-container .sessions-chat-picker-slot .action-label, .new-chat-widget-container .new-chat-bottom-container .monaco-action-bar .action-item .action-label { min-width: 38px; + overflow: hidden; } /* Below this width the bottom-row pickers can't fit their labels comfortably, * so collapse to icon + chevron only. The .new-chat-widget-container declares * `container-type: size` which makes this a size container query. */ -@container (max-width: 320px) { +@container (max-width: 330px) { /* Bottom-row pickers (Copilot CLI, Default Approvals, Worktree, branch): icon-only */ .new-chat-widget-container .new-chat-bottom-container .sessions-chat-dropdown-label { display: none; @@ -141,14 +148,15 @@ } } -/* Spacing between action items inside the repo-config toolbar (e.g. Worktree, branch) */ -.new-chat-widget-container .monaco-action-bar .actions-container { +/* Spacing between action items inside the bottom-row toolbars (e.g. Worktree, branch) */ +.new-chat-widget-container .new-chat-bottom-container .monaco-action-bar .actions-container { gap: 4px; } /* Match VS Code chat-secondary-toolbar sizing for action items in the bottom row * (e.g. session control pickers and repo config pickers) so they align with the - * mode/model picker font in the input toolbar. */ + * mode/model picker font in the input toolbar. The 1px right padding is intentional + * because the chevron's 16x16 codicon box supplies the visual right padding. */ .new-chat-widget-container .new-chat-bottom-container .action-label { height: 16px; padding: 3px 1px 3px 7px; @@ -156,14 +164,13 @@ color: var(--vscode-icon-foreground); } -.new-chat-widget-container .new-chat-bottom-container .action-label .codicon { - font-size: 12px !important; +.new-chat-widget-container .new-chat-bottom-container .action-label > .codicon { + font-size: 12px; } -.new-chat-widget-container .new-chat-bottom-container .action-label .codicon-chevron-down { - font-size: 10px !important; +.new-chat-widget-container .new-chat-bottom-container .action-label > .codicon-chevron-down { + font-size: 10px; margin-left: 4px; - opacity: 0.75; } .chat-controls-container .monaco-editor-background { @@ -327,7 +334,6 @@ justify-content: center; font-size: 10px; margin-left: 4px; - opacity: 0.75; line-height: 1; /* Match the 16x16 codicon box that the action-bar gives the regular chat * picker's chevron, so the small chevron icon has equal padding on both sides. */ diff --git a/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css b/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css index 155ee8f5e0541..afdaf983bffba 100644 --- a/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css +++ b/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css @@ -16,7 +16,11 @@ .new-chat-in-session .new-chat-widget-content { width: 100%; max-width: 950px; - margin: 0 auto !important; + /* Left-align (rather than centering with `margin: 0 auto`) so the input's + * left edge — and therefore the attach (+) button — stays anchored as the + * viewport narrows past max-width. */ + margin: 0 !important; + align-self: flex-start !important; padding: 4px 10px !important; box-sizing: border-box; } From 2163b5fcb45a10e112e2b4909124366232a73518 Mon Sep 17 00:00:00 2001 From: mrleemurray Date: Mon, 20 Apr 2026 12:57:15 +0100 Subject: [PATCH 06/17] Center align chat widget elements for improved layout consistency --- src/vs/sessions/contrib/chat/browser/media/chatWidget.css | 1 + src/vs/sessions/contrib/chat/browser/media/newChatInSession.css | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css index c7ea9a05049f8..d3da9174a5df0 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css @@ -75,6 +75,7 @@ display: flex; flex-direction: column; align-items: stretch; + align-self: center; } .new-chat-widget-container .new-chat-bottom-container { diff --git a/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css b/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css index afdaf983bffba..b090e30fcad89 100644 --- a/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css +++ b/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css @@ -20,7 +20,6 @@ * left edge — and therefore the attach (+) button — stays anchored as the * viewport narrows past max-width. */ margin: 0 !important; - align-self: flex-start !important; padding: 4px 10px !important; box-sizing: border-box; } From dcfb24c4f49595ef5f45710c7ae968f38c46a806 Mon Sep 17 00:00:00 2001 From: Lee Murray Date: Mon, 20 Apr 2026 15:02:19 +0100 Subject: [PATCH 07/17] Update src/vs/sessions/contrib/chat/browser/media/chatWidget.css Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/vs/sessions/contrib/chat/browser/media/chatWidget.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css index d3da9174a5df0..11850c9d9df53 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css @@ -23,7 +23,7 @@ box-sizing: border-box; overflow: hidden; padding: 16px 16px 20px 16px; - /* Establishes a size container so the @container (max-width: 320px) query below + /* Establishes a size container so the @container (max-width: 330px) query below * can collapse picker labels to icon-only when the new-chat area is narrow. */ container-type: size; position: relative; From 664f0e48cd296e71b66681bdfb4b30d69b4bcee9 Mon Sep 17 00:00:00 2001 From: mrleemurray Date: Mon, 20 Apr 2026 17:19:16 +0100 Subject: [PATCH 08/17] Refactor chat input and widget styles for improved alignment and spacing Co-authored-by: Copilot --- .../contrib/chat/browser/media/chatInput.css | 22 +++++++++++-------- .../contrib/chat/browser/media/chatWidget.css | 12 +++++----- .../chat/browser/media/newChatInSession.css | 2 +- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/vs/sessions/contrib/chat/browser/media/chatInput.css b/src/vs/sessions/contrib/chat/browser/media/chatInput.css index 826d05bfd83d4..cb454b2988a77 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatInput.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatInput.css @@ -78,7 +78,7 @@ display: flex; align-items: center; padding: 4px 6px 6px 6px; - gap: 6px; + gap: 4px; color: var(--vscode-icon-foreground); } @@ -137,17 +137,23 @@ .sessions-chat-config-toolbar .action-label, .sessions-chat-config-toolbar .action-label .chat-input-picker-label { - font-size: 12px; + font-size: 11px; } /* Allow long labels (e.g. the model picker name) to ellipsize when space is tight */ .sessions-chat-config-toolbar .action-label .chat-input-picker-label { + margin-left: 6px; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; min-width: 0; } +/* When the picker has no leading icon (e.g. model picker), drop the icon-to-label gap. */ +.sessions-chat-config-toolbar .action-label .chat-input-picker-label:first-child { + margin-left: 0; +} + .sessions-chat-config-toolbar .monaco-action-bar .action-item .action-label > .codicon { font-size: 12px; flex-shrink: 0; @@ -162,12 +168,10 @@ align-items: center; justify-content: center; font-size: 10px; - margin-left: 4px; + margin-left: 2px; line-height: 1; - /* Restore the 16x16 codicon box for the chevron so it has equal padding on both sides, - * matching the bottom-row picker chevrons. */ - width: 16px; - height: 16px; + width: 12px; + height: 12px; } /* Spacing between action items inside the chat input config toolbar (mode + model picker) */ @@ -205,7 +209,7 @@ background-color: var(--vscode-toolbar-hoverBackground) !important; } -.sessions-chat-send-button .monaco-button .codicon { +.monaco-workbench .sessions-chat-send-button .monaco-button .codicon[class*='codicon-'] { font-size: 14px; } @@ -271,7 +275,7 @@ outline-offset: -1px; } -.sessions-chat-attach-button .codicon { +.monaco-workbench .sessions-chat-attach-button .codicon[class*='codicon-'] { font-size: 14px; } diff --git a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css index 11850c9d9df53..7197328fefec5 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css @@ -161,7 +161,7 @@ .new-chat-widget-container .new-chat-bottom-container .action-label { height: 16px; padding: 3px 1px 3px 7px; - font-size: 12px; + font-size: 11px; color: var(--vscode-icon-foreground); } @@ -240,7 +240,7 @@ } .sessions-chat-dropdown-label { - margin-left: 2px; + margin-left: 6px; overflow: hidden; text-overflow: ellipsis; white-space: nowrap; @@ -334,12 +334,10 @@ align-items: center; justify-content: center; font-size: 10px; - margin-left: 4px; + margin-left: 2px; line-height: 1; - /* Match the 16x16 codicon box that the action-bar gives the regular chat - * picker's chevron, so the small chevron icon has equal padding on both sides. */ - width: 16px; - height: 16px; + width: 12px; + height: 12px; } .sessions-chat-picker-slot .action-label .chat-session-option-label { diff --git a/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css b/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css index b090e30fcad89..88b335086c6ec 100644 --- a/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css +++ b/src/vs/sessions/contrib/chat/browser/media/newChatInSession.css @@ -98,7 +98,7 @@ } .new-chat-in-session .sessions-chat-dropdown-label { - margin-left: 2px; + margin-left: 6px; } /* Disable fade-in animations — show content immediately */ From 918ed8d4e74ad2e636d10bf986ad0b79f59e1c1b Mon Sep 17 00:00:00 2001 From: mrleemurray Date: Mon, 20 Apr 2026 17:28:57 +0100 Subject: [PATCH 09/17] Adjust min-width for action items and prevent mode picker from shrinking in chat toolbar for improved layout consistency Co-authored-by: Copilot --- .../sessions/contrib/chat/browser/media/chatInput.css | 10 ++++++++-- .../sessions/contrib/chat/browser/media/chatWidget.css | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/vs/sessions/contrib/chat/browser/media/chatInput.css b/src/vs/sessions/contrib/chat/browser/media/chatInput.css index cb454b2988a77..60489450b87ec 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatInput.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatInput.css @@ -110,10 +110,16 @@ .sessions-chat-config-toolbar .monaco-action-bar .action-item { display: flex; align-items: center; - min-width: 48px; + min-width: 30px; overflow: hidden; } +/* Prevent the mode picker from shrinking so the model picker label + * ellipsizes first rather than the mode picker collapsing to icon-only. */ +.sessions-chat-config-toolbar .monaco-action-bar .action-item:has(.sessions-chat-dropdown-label) { + flex-shrink: 0; +} + .sessions-chat-config-toolbar .action-label { display: flex; align-items: center; @@ -126,7 +132,7 @@ touch-action: manipulation; color: var(--vscode-icon-foreground); white-space: nowrap; - min-width: 38px; + min-width: 30px; overflow: hidden; } diff --git a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css index 7197328fefec5..fd0ee642e7d89 100644 --- a/src/vs/sessions/contrib/chat/browser/media/chatWidget.css +++ b/src/vs/sessions/contrib/chat/browser/media/chatWidget.css @@ -129,7 +129,7 @@ .new-chat-widget-container .new-chat-bottom-container .monaco-action-bar .action-item, .new-chat-widget-container .new-chat-bottom-container .sessions-chat-picker-slot .action-label, .new-chat-widget-container .new-chat-bottom-container .monaco-action-bar .action-item .action-label { - min-width: 38px; + min-width: 30px; overflow: hidden; } From e8a7c6d6dfaf63003de2b5b0a0aaee2f4d2b6904 Mon Sep 17 00:00:00 2001 From: Anthony Kim <62267334+anthonykim1@users.noreply.github.com> Date: Mon, 20 Apr 2026 21:25:55 -0700 Subject: [PATCH 10/17] Stop copying node-pty into Copilot CLI SDK (#310925) * Stop copying node-pty into Copilot CLI SDK * reanem Co-authored-by: Copilot * resovle copilot comment * recognize other sdk prebuilds NOT node-pty Co-authored-by: Copilot --------- Co-authored-by: Copilot --- build/gulpfile.reh.ts | 9 +- build/gulpfile.vscode.ts | 9 +- build/lib/copilot.ts | 86 +++-------- extensions/copilot/eslint.config.mjs | 1 - .../chatSessions/copilotcli/AGENTS.md | 3 +- .../copilotcli/node/copilotCli.ts | 8 +- .../copilotcli/node/nodePtyShim.ts | 144 ------------------ .../test/copilotCLISDKUpgrade.spec.ts | 29 ++-- 8 files changed, 43 insertions(+), 246 deletions(-) delete mode 100644 extensions/copilot/src/extension/chatSessions/copilotcli/node/nodePtyShim.ts diff --git a/build/gulpfile.reh.ts b/build/gulpfile.reh.ts index 2c44fc9427f39..3c0e3a16d02e9 100644 --- a/build/gulpfile.reh.ts +++ b/build/gulpfile.reh.ts @@ -34,7 +34,7 @@ import * as cp from 'child_process'; import log from 'fancy-log'; import buildfile from './buildfile.ts'; import { fetchUrls, fetchGithub } from './lib/fetch.ts'; -import { getCopilotExcludeFilter, copyCopilotNativeDeps, prepareBuiltInCopilotExtensionShims } from './lib/copilot.ts'; +import { getCopilotExcludeFilter, prepareBuiltInCopilotRipgrepShim } from './lib/copilot.ts'; import jsonEditor from 'gulp-json-editor'; @@ -463,14 +463,13 @@ function patchWin32DependenciesTask(destinationFolderName: string) { }; } -function copyCopilotNativeDepsTaskREH(platform: string, arch: string, destinationFolderName: string) { +function prepareCopilotRipgrepShimTaskREH(platform: string, arch: string, destinationFolderName: string) { return async () => { const outputDir = path.join(BUILD_ROOT, destinationFolderName); const nodeModulesDir = path.join(outputDir, 'node_modules'); - copyCopilotNativeDeps(platform, arch, nodeModulesDir); const builtInCopilotExtensionDir = path.join(outputDir, 'extensions', 'copilot'); - prepareBuiltInCopilotExtensionShims(platform, arch, builtInCopilotExtensionDir, nodeModulesDir); + prepareBuiltInCopilotRipgrepShim(platform, arch, builtInCopilotExtensionDir, nodeModulesDir); }; } @@ -523,7 +522,7 @@ function tweakProductForServerWeb(product: typeof import('../product.json')) { gulp.task(`node-${platform}-${arch}`) as task.Task, util.rimraf(path.join(BUILD_ROOT, destinationFolderName)), packageTask(type, platform, arch, sourceFolderName, destinationFolderName), - copyCopilotNativeDepsTaskREH(platform, arch, destinationFolderName) + prepareCopilotRipgrepShimTaskREH(platform, arch, destinationFolderName) ]; if (platform === 'win32') { diff --git a/build/gulpfile.vscode.ts b/build/gulpfile.vscode.ts index 31188a4ce9ebc..75ccf64fe7e60 100644 --- a/build/gulpfile.vscode.ts +++ b/build/gulpfile.vscode.ts @@ -31,7 +31,7 @@ import minimist from 'minimist'; import { compileBuildWithoutManglingTask, compileBuildWithManglingTask } from './gulpfile.compile.ts'; import { compileNonNativeExtensionsBuildTask, compileNativeExtensionsBuildTask, compileAllExtensionsBuildTask, compileExtensionMediaBuildTask, cleanExtensionsBuildTask, compileCopilotExtensionBuildTask } from './gulpfile.extensions.ts'; import { copyCodiconsTask } from './lib/compilation.ts'; -import { getCopilotExcludeFilter, copyCopilotNativeDeps, prepareBuiltInCopilotExtensionShims } from './lib/copilot.ts'; +import { getCopilotExcludeFilter, prepareBuiltInCopilotRipgrepShim } from './lib/copilot.ts'; import type { EmbeddedProductInfo } from './lib/embeddedType.ts'; import { useEsbuildTranspile } from './buildConfig.ts'; import { promisify } from 'util'; @@ -700,7 +700,7 @@ function patchWin32DependenciesTask(destinationFolderName: string) { }; } -function copyCopilotNativeDepsTask(platform: string, arch: string, destinationFolderName: string) { +function prepareCopilotRipgrepShimTask(platform: string, arch: string, destinationFolderName: string) { const outputDir = path.join(path.dirname(root), destinationFolderName); return async () => { @@ -711,10 +711,9 @@ function copyCopilotNativeDepsTask(platform: string, arch: string, destinationFo ? path.join(outputDir, `${product.nameLong}.app`, 'Contents', 'Resources', 'app') : path.join(outputDir, versionedResourcesFolder, 'resources', 'app'); const appNodeModulesDir = path.join(appBase, 'node_modules'); - copyCopilotNativeDeps(platform, arch, appNodeModulesDir); const builtInCopilotExtensionDir = path.join(appBase, 'extensions', 'copilot'); - prepareBuiltInCopilotExtensionShims(platform, arch, builtInCopilotExtensionDir, appNodeModulesDir); + prepareBuiltInCopilotRipgrepShim(platform, arch, builtInCopilotExtensionDir, appNodeModulesDir); }; } @@ -743,7 +742,7 @@ BUILD_TARGETS.forEach(buildTarget => { compileNativeExtensionsBuildTask, util.rimraf(path.join(buildRoot, destinationFolderName)), packageTask(platform, arch, sourceFolderName, destinationFolderName, opts), - copyCopilotNativeDepsTask(platform, arch, destinationFolderName) + prepareCopilotRipgrepShimTask(platform, arch, destinationFolderName) ]; if (platform === 'win32') { diff --git a/build/lib/copilot.ts b/build/lib/copilot.ts index f0e50ce9a8071..fec1c3d0d8593 100644 --- a/build/lib/copilot.ts +++ b/build/lib/copilot.ts @@ -46,8 +46,9 @@ function toNodePlatformArch(platform: string, arch: string): { nodePlatform: str * for architectures other than the build target. * * For platforms the copilot SDK doesn't natively support (e.g. alpine, armhf), - * ALL platform packages are stripped - that's fine because the SDK doesn't ship - * binaries for those platforms anyway, and we replace them with VS Code's own. + * ALL platform packages are stripped - that's fine because the copilot CLI SDK + * resolves `node-pty` from the embedder (VS Code) first via `hostRequire`, + * falling back to its bundled copy only if the embedder can't provide it. */ export function getCopilotExcludeFilter(platform: string, arch: string): string[] { const { nodePlatform, nodeArch } = toNodePlatformArch(platform, arch); @@ -55,74 +56,30 @@ export function getCopilotExcludeFilter(platform: string, arch: string): string[ const nonTargetPlatforms = copilotPlatforms.filter(p => p !== targetPlatformArch); // Strip wrong-architecture @github/copilot-{platform} packages. - // All copilot prebuilds are stripped by .moduleignore; VS Code's own - // node-pty is copied into the prebuilds location by a post-packaging task. + // All copilot prebuilds are stripped by .moduleignore; the copilot CLI SDK + // resolves `node-pty` from VS Code's own node_modules via `hostRequire`. const excludes = nonTargetPlatforms.map(p => `!**/node_modules/@github/copilot-${p}/**`); return ['**', ...excludes]; } /** - * Copies VS Code's own node-pty binaries into the copilot SDK's - * expected locations so the copilot CLI subprocess can find them at runtime. - * The copilot-bundled prebuilds are stripped by .moduleignore; - * this replaces them with the same binaries VS Code already ships, avoiding - * new system dependency requirements. - * - * This works even for platforms the copilot SDK doesn't natively support - * (e.g. alpine, armhf) because the SDK's native module loader simply - * looks for `prebuilds/{process.platform}-{process.arch}/pty.node` - it - * doesn't validate the platform against a supported list. - * - * Failures are logged but do not throw, to avoid breaking the build on - * platforms where something unexpected happens. - * - * @param nodeModulesDir Absolute path to the node_modules directory that - * contains both the source binaries (node-pty) and the copilot SDK - * target directories. - */ -export function copyCopilotNativeDeps(platform: string, arch: string, nodeModulesDir: string): void { - const { nodePlatform, nodeArch } = toNodePlatformArch(platform, arch); - const platformArch = `${nodePlatform}-${nodeArch}`; - - const copilotBase = path.join(nodeModulesDir, '@github', 'copilot'); - if (!fs.existsSync(copilotBase)) { - console.warn(`[copyCopilotNativeDeps] @github/copilot not found at ${copilotBase}, skipping`); - return; - } - - const nodePtySource = path.join(nodeModulesDir, 'node-pty', 'build', 'Release'); - if (!fs.existsSync(nodePtySource)) { - console.warn(`[copyCopilotNativeDeps] node-pty source not found at ${nodePtySource}, skipping`); - return; - } - - try { - // Copy node-pty (pty.node + spawn-helper on Unix, conpty.node + conpty/ on Windows) - // into copilot prebuilds so the SDK finds them via loadNativeModule. - const copilotPrebuildsDir = path.join(copilotBase, 'prebuilds', platformArch); - fs.mkdirSync(copilotPrebuildsDir, { recursive: true }); - fs.cpSync(nodePtySource, copilotPrebuildsDir, { recursive: true }); - console.log(`[copyCopilotNativeDeps] Copied node-pty from ${nodePtySource} to ${copilotPrebuildsDir}`); - } catch (err) { - console.warn(`[copyCopilotNativeDeps] Failed to copy node-pty for ${platformArch}: ${err}`); - } -} - -/** - * Materializes copilot CLI shims directly inside the built-in copilot extension. + * Materializes the copilot CLI ripgrep shim directly inside the built-in copilot extension. * * This is used when copilot is shipped as a built-in extension so startup does - * not need to create shims at runtime. The destination layout matches the + * not need to create the shim at runtime. The destination layout matches the * runtime shim logic in the copilot extension: - * - node-pty: node_modules/@github/copilot/sdk/prebuilds/{platform-arch} * - ripgrep: node_modules/@github/copilot/sdk/ripgrep/bin/{platform-arch} * - marker: node_modules/@github/copilot/shims.txt * + * Note: `node-pty` is no longer shimmed. The copilot CLI SDK resolves + * `node-pty` from the embedder (VS Code) via `hostRequire` and falls back to + * its bundled copy only if that fails. + * * Failures throw to fail the build because built-in packaging must guarantee - * these artifacts are present. + * this artifact is present. */ -export function prepareBuiltInCopilotExtensionShims(platform: string, arch: string, builtInCopilotExtensionDir: string, appNodeModulesDir: string): void { +export function prepareBuiltInCopilotRipgrepShim(platform: string, arch: string, builtInCopilotExtensionDir: string, appNodeModulesDir: string): void { const { nodePlatform, nodeArch } = toNodePlatformArch(platform, arch); const platformArch = `${nodePlatform}-${nodeArch}`; @@ -130,33 +87,24 @@ export function prepareBuiltInCopilotExtensionShims(platform: string, arch: stri const copilotBase = path.join(extensionNodeModules, '@github', 'copilot'); const copilotSdkBase = path.join(copilotBase, 'sdk'); if (!fs.existsSync(copilotSdkBase)) { - throw new Error(`[prepareBuiltInCopilotExtensionShims] Copilot SDK directory not found at ${copilotSdkBase}`); - } - - const nodePtySource = path.join(appNodeModulesDir, 'node-pty', 'build', 'Release'); - if (!fs.existsSync(nodePtySource)) { - throw new Error(`[prepareBuiltInCopilotExtensionShims] node-pty source not found at ${nodePtySource}`); + throw new Error(`[prepareBuiltInCopilotRipgrepShim] Copilot SDK directory not found at ${copilotSdkBase}`); } const ripgrepSource = path.join(appNodeModulesDir, '@vscode', 'ripgrep', 'bin'); if (!fs.existsSync(ripgrepSource)) { - throw new Error(`[prepareBuiltInCopilotExtensionShims] ripgrep source not found at ${ripgrepSource}`); + throw new Error(`[prepareBuiltInCopilotRipgrepShim] ripgrep source not found at ${ripgrepSource}`); } - const nodePtyDest = path.join(copilotSdkBase, 'prebuilds', platformArch); const ripgrepDest = path.join(copilotSdkBase, 'ripgrep', 'bin', platformArch); const shimMarkerPath = path.join(copilotBase, 'shims.txt'); try { - fs.mkdirSync(nodePtyDest, { recursive: true }); - fs.cpSync(nodePtySource, nodePtyDest, { recursive: true }); - fs.mkdirSync(ripgrepDest, { recursive: true }); fs.cpSync(ripgrepSource, ripgrepDest, { recursive: true }); fs.writeFileSync(shimMarkerPath, 'Shims created successfully'); - console.log(`[prepareBuiltInCopilotExtensionShims] Materialized shims for ${platformArch} in ${builtInCopilotExtensionDir}`); + console.log(`[prepareBuiltInCopilotRipgrepShim] Materialized ripgrep shim for ${platformArch} in ${builtInCopilotExtensionDir}`); } catch (err) { - throw new Error(`[prepareBuiltInCopilotExtensionShims] Failed to materialize shims for ${platformArch}: ${err}`); + throw new Error(`[prepareBuiltInCopilotRipgrepShim] Failed to materialize ripgrep shim for ${platformArch}: ${err}`); } } diff --git a/extensions/copilot/eslint.config.mjs b/extensions/copilot/eslint.config.mjs index d5bbb8e4f2708..7edfb1982d931 100644 --- a/extensions/copilot/eslint.config.mjs +++ b/extensions/copilot/eslint.config.mjs @@ -338,7 +338,6 @@ export default tseslint.config( ignores: [ 'src/util/vs/**/*.ts', // vendored code 'src/**/*.spec.ts', // allow in tests - './src/extension/agents/copilotcli/node/nodePtyShim.ts', './src/extension/byok/common/anthropicMessageConverter.ts', './src/extension/byok/common/geminiFunctionDeclarationConverter.ts', './src/extension/byok/common/geminiMessageConverter.ts', diff --git a/extensions/copilot/src/extension/chatSessions/copilotcli/AGENTS.md b/extensions/copilot/src/extension/chatSessions/copilotcli/AGENTS.md index eb343ccf15bdb..ebd7169ae234b 100644 --- a/extensions/copilot/src/extension/chatSessions/copilotcli/AGENTS.md +++ b/extensions/copilot/src/extension/chatSessions/copilotcli/AGENTS.md @@ -78,7 +78,6 @@ copilotcli/ │ ├── copilotCLISkills.ts # Skills location resolution │ ├── copilotCLIImageSupport.ts # Image attachment handling │ ├── mcpHandler.ts # MCP server configuration for SDK sessions -│ ├── nodePtyShim.ts # Copies VS Code's node-pty for SDK use │ ├── userInputHelpers.ts # User question/input handling interface │ ├── exitPlanModeHandler.ts # Plan mode exit flow with user choice │ ├── ripgrepShim.ts # Copies VS Code's ripgrep for SDK use @@ -313,7 +312,7 @@ Orchestrates the start and end of each chat request turn, coordinating worktree ## Critical Pitfalls -- **Shims before SDK import**: `ensureNodePtyShim()` and `ensureRipgrepShim()` in `node/nodePtyShim.ts` / `node/ripgrepShim.ts` MUST be called before any `import('@github/copilot/sdk')`. They copy VS Code's bundled native binaries to the SDK's expected locations. See `node/copilotCli.ts` for the initialization order. +- **Shims before SDK import**: `ensureRipgrepShim()` in `node/ripgrepShim.ts` MUST be called before any `import('@github/copilot/sdk')`. It copies VS Code's bundled ripgrep binary to the SDK's expected location. `node-pty` is no longer shimmed: the copilot CLI SDK resolves it from VS Code's own `node_modules` via `hostRequire`, falling back to its bundled copy only if that fails. See `node/copilotCli.ts` for the initialization order. - **Delayed permission UI**: Tool invocation messages are held in `toolCallWaitingForPermissions` until permission resolves. `flushPendingInvocationMessageForToolCallId()` flushes only the specific approved tool, not all pending tools. This is intentional — don't bypass it. diff --git a/extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotCli.ts b/extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotCli.ts index 72308407e8c8c..5ba70073994f4 100644 --- a/extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotCli.ts +++ b/extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotCli.ts @@ -23,7 +23,6 @@ import { basename } from '../../../../util/vs/base/common/resources'; import { URI } from '../../../../util/vs/base/common/uri'; import { IInstantiationService } from '../../../../util/vs/platform/instantiation/common/instantiation'; import { getCopilotLogger } from './logger'; -import { ensureNodePtyShim } from './nodePtyShim'; import { ensureRipgrepShim } from './ripgrepShim'; import { CancellationToken } from '../../../../util/vs/base/common/cancellation'; @@ -460,7 +459,7 @@ export class CopilotCLISDK implements ICopilotCLISDK { public async getPackage(): Promise { try { - // Ensure the node-pty shim exists before importing the SDK (required for CLI sessions) + // Ensure the ripgrep shim exists before importing the SDK (required for CLI sessions) await this._ensureShimsPromise; return await import('@github/copilot/sdk'); } catch (error) { @@ -497,10 +496,7 @@ export class CopilotCLISDK implements ICopilotCLISDK { if (await checkFileExists(successfulPlaceholder)) { return; } - await Promise.all([ - ensureNodePtyShim(this.extensionContext.extensionPath, this.envService.appRoot, this.logService), - ensureRipgrepShim(this.extensionContext.extensionPath, this.envService.appRoot, this.logService) - ]); + await ensureRipgrepShim(this.extensionContext.extensionPath, this.envService.appRoot, this.logService); await fs.writeFile(successfulPlaceholder, 'Shims created successfully'); } diff --git a/extensions/copilot/src/extension/chatSessions/copilotcli/node/nodePtyShim.ts b/extensions/copilot/src/extension/chatSessions/copilotcli/node/nodePtyShim.ts deleted file mode 100644 index c7db274412ad1..0000000000000 --- a/extensions/copilot/src/extension/chatSessions/copilotcli/node/nodePtyShim.ts +++ /dev/null @@ -1,144 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { promises as fs } from 'fs'; -import * as path from 'path'; -import { ILogService } from '../../../../platform/log/common/logService'; - -let shimCreated: Promise | undefined = undefined; - -const RETRIABLE_COPY_ERROR_CODES = new Set(['EPERM', 'EBUSY']); -const MAX_COPY_ATTEMPTS = 6; -const RETRY_DELAY_BASE_MS = 50; -const RETRY_DELAY_CAP_MS = 500; -const MATERIALIZATION_TIMEOUT_MS = 4000; -const MATERIALIZATION_POLL_INTERVAL_MS = 100; - -/** - * Copies the node-pty files from VS Code's installation into a @github/copilot location - * - * MUST be called before any `import('@github/copilot/sdk')` or `import('@github/copilot')`. - * - * @github/copilot bundles the node-pty code and its no longer possible to shim the package. - * - * @param extensionPath The extension's path (where to create the shim) - * @param vscodeAppRoot VS Code's installation path (where node-pty is located) - */ -export async function ensureNodePtyShim(extensionPath: string, vscodeAppRoot: string, logService: ILogService): Promise { - if (shimCreated) { - return shimCreated; - } - - const creation = _ensureNodePtyShim(extensionPath, vscodeAppRoot, logService); - shimCreated = creation.catch(error => { - shimCreated = undefined; - throw error; - }); - return shimCreated; -} - -async function _ensureNodePtyShim(extensionPath: string, vscodeAppRoot: string, logService: ILogService): Promise { - const vscodeNodePtyPath = path.join(vscodeAppRoot, 'node_modules', 'node-pty', 'build', 'Release'); - - await copyNodePtyFiles(extensionPath, vscodeNodePtyPath, logService); -} - -export async function copyNodePtyFiles(extensionPath: string, sourceNodePtyPath: string, logService: ILogService): Promise { - const nodePtyDir = path.join(extensionPath, 'node_modules', '@github', 'copilot', 'sdk', 'prebuilds', process.platform + '-' + process.arch); - logService.info(`Creating node-pty shim: source=${sourceNodePtyPath}, dest=${nodePtyDir}`); - - try { - await fs.mkdir(nodePtyDir, { recursive: true }); - const entries = await fs.readdir(sourceNodePtyPath); - const uniqueEntries = [...new Set(entries)]; - logService.info(`Found ${uniqueEntries.length} entries to copy${uniqueEntries.length !== entries.length ? ` (${entries.length - uniqueEntries.length} duplicates ignored)` : ''}: ${uniqueEntries.join(', ')}`); - - await copyNodePtyWithRetries(sourceNodePtyPath, nodePtyDir, uniqueEntries, logService); - } catch (error) { - logService.error(`Failed to create node-pty shim (source dir: ${sourceNodePtyPath}, extension dir: ${nodePtyDir})`, error); - throw error; - } -} - -async function copyNodePtyWithRetries(sourceDir: string, destDir: string, entries: string[], logService: ILogService): Promise { - const primaryBinary = entries.find(entry => entry.endsWith('.node')); - for (let attempt = 1; attempt <= MAX_COPY_ATTEMPTS; attempt++) { - try { - await fs.cp(sourceDir, destDir, { - recursive: true, - dereference: true, - force: true, - filter: async (srcPath) => shouldCopyEntry(srcPath, logService) - }); - logService.trace(`Copied node-pty prebuilds to ${destDir} (attempt ${attempt})`); - return; - } catch (error) { - if (await waitForMaterializedShim(destDir, primaryBinary, logService)) { - logService.trace(`Detected node-pty shim materialized at ${destDir} by another extension host`); - return; - } - - if (!RETRIABLE_COPY_ERROR_CODES.has(error?.code) || attempt === MAX_COPY_ATTEMPTS) { - throw error; - } - - const delayMs = Math.min(RETRY_DELAY_BASE_MS * Math.pow(2, attempt - 1), RETRY_DELAY_CAP_MS); - logService.warn(`Retryable error (${error.code}) copying node-pty shim. Retrying in ${delayMs}ms (attempt ${attempt + 1}/${MAX_COPY_ATTEMPTS})`); - await new Promise(resolve => setTimeout(resolve, delayMs)); - } - } -} - -async function shouldCopyEntry(srcPath: string, logService: ILogService): Promise { - try { - const stat = await fs.stat(srcPath); - if (stat.isDirectory()) { - return true; - } - - if (stat.size === 0) { - logService.trace(`Skipping ${path.basename(srcPath)}: zero-byte file (likely symlink or special file)`); - return false; - } - - return true; - } catch (error) { - logService.warn(`Failed to stat ${srcPath}: ${error?.message ?? error}`); - return false; - } -} - -async function waitForMaterializedShim(destDir: string, primaryBinary: string | undefined, logService: ILogService): Promise { - const deadline = Date.now() + MATERIALIZATION_TIMEOUT_MS; - while (Date.now() <= deadline) { - if (await isShimMaterialized(destDir, primaryBinary)) { - logService.trace(`Reusing node-pty shim that materialized at ${destDir}`); - return true; - } - - await new Promise(resolve => setTimeout(resolve, MATERIALIZATION_POLL_INTERVAL_MS)); - } - - return false; -} - -async function isShimMaterialized(destDir: string, primaryBinary: string | undefined): Promise { - if (primaryBinary) { - const binaryStat = await fs.stat(path.join(destDir, primaryBinary)).catch(() => undefined); - if (binaryStat && binaryStat.isFile() && binaryStat.size > 0) { - return true; - } - } - - const entries = await fs.readdir(destDir).catch(() => []); - for (const entry of entries) { - const stat = await fs.stat(path.join(destDir, entry)).catch(() => undefined); - if (stat && stat.isFile() && stat.size > 0) { - return true; - } - } - - return false; -} diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCLISDKUpgrade.spec.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCLISDKUpgrade.spec.ts index a244f8f0f3233..98cf628b6610e 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCLISDKUpgrade.spec.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/test/copilotCLISDKUpgrade.spec.ts @@ -8,7 +8,6 @@ import { isBinaryFile } from 'isbinaryfile'; import * as path from 'path'; import { beforeAll, describe, it } from 'vitest'; import { TestLogService } from '../../../../platform/testing/common/testLogService'; -import { copyNodePtyFiles } from '../../copilotcli/node/nodePtyShim'; import { copyRipgrepShim } from '../../copilotcli/node/ripgrepShim'; describe('CopilotCLI SDK Upgrade', function () { @@ -59,6 +58,16 @@ describe('CopilotCLI SDK Upgrade', function () { // win32 native module (formerly win_error_mode) path.join('prebuilds', 'win32-arm64', 'win32.node'), path.join('prebuilds', 'win32-x64', 'win32.node'), + // Second copy of computer.node / win32.node re-shipped by the @github/copilot/sdk subpackage + // (previously hidden by a broad sdk/prebuilds/** exclusion that masked the node-pty files we used to shim in at test setup). + path.join('sdk', 'prebuilds', 'darwin-arm64', 'computer.node'), + path.join('sdk', 'prebuilds', 'darwin-x64', 'computer.node'), + path.join('sdk', 'prebuilds', 'linux-arm64', 'computer.node'), + path.join('sdk', 'prebuilds', 'linux-x64', 'computer.node'), + path.join('sdk', 'prebuilds', 'win32-arm64', 'computer.node'), + path.join('sdk', 'prebuilds', 'win32-x64', 'computer.node'), + path.join('sdk', 'prebuilds', 'win32-arm64', 'win32.node'), + path.join('sdk', 'prebuilds', 'win32-x64', 'win32.node'), path.join('ripgrep', 'bin', 'darwin-arm64', 'rg'), path.join('ripgrep', 'bin', 'darwin-x64', 'rg'), path.join('ripgrep', 'bin', 'linux-x64', 'rg'), @@ -89,15 +98,13 @@ describe('CopilotCLI SDK Upgrade', function () { 'tree-sitter-scala.wasm', ].map(p => path.join(copilotSDKPath, p))); - // Exclude ripgrep files that we copy over in src/extension/agents/copilotcli/node/ripgrepShim.ts (until we get better API/solution from SDK) + // Exclude ripgrep files that we copy over in src/extension/chatSessions/copilotcli/node/ripgrepShim.ts (until we get better API/solution from SDK) const ripgrepFilesWeCopy = path.join(copilotSDKPath, 'sdk', 'ripgrep', 'bin'); - // Exclude nodepty files that we copy over in src/extension/agents/copilotcli/node/nodePtyShim.ts (until we get better API/solution from SDK) - const nodeptyFilesWeCopy = path.join(copilotSDKPath, 'sdk', 'prebuilds'); const errors: string[] = []; // Look for new binaries for (const binary of existingBinaries) { - if (binary.startsWith(ripgrepFilesWeCopy) || binary.startsWith(nodeptyFilesWeCopy)) { + if (binary.startsWith(ripgrepFilesWeCopy)) { continue; } const binaryName = path.basename(binary); @@ -110,7 +117,7 @@ describe('CopilotCLI SDK Upgrade', function () { } // Look for removed binaries. for (const binary of knownBinaries) { - if (binary.startsWith(ripgrepFilesWeCopy) || binary.startsWith(nodeptyFilesWeCopy)) { + if (binary.startsWith(ripgrepFilesWeCopy)) { continue; } if (!existingBinaries.has(binary)) { @@ -124,19 +131,13 @@ describe('CopilotCLI SDK Upgrade', function () { }); it('should be able to load the @github/copilot module without errors', async function () { - await copyNodePtyFiles( - extensionPath, - path.join(copilotSDKPath, 'prebuilds', process.platform + '-' + process.arch), - new TestLogService() - ); await import('@github/copilot/sdk'); }); }); async function copyBinaries(extensionPath: string) { - const nodePtyPrebuilds = path.join(extensionPath, 'node_modules', '@github', 'copilot', 'prebuilds', process.platform + '-' + process.arch); - const vscodeRipgrepPath = path.join(extensionPath, 'node_modules', '@github', 'copilot', 'ripgrep', 'bin', process.platform + '-' + process.arch); - await copyNodePtyFiles(extensionPath, nodePtyPrebuilds, new TestLogService()); + const copilotSDKPath = path.join(extensionPath, 'node_modules', '@github', 'copilot'); + const vscodeRipgrepPath = path.join(copilotSDKPath, 'ripgrep', 'bin', process.platform + '-' + process.arch); await copyRipgrepShim(extensionPath, vscodeRipgrepPath, new TestLogService()); } async function findAllBinaries(dir: string): Promise { From 1a39bedfe80501cdade609f5216dbd773a20bc2b Mon Sep 17 00:00:00 2001 From: Justin Chen <54879025+justschen@users.noreply.github.com> Date: Mon, 20 Apr 2026 22:07:40 -0700 Subject: [PATCH 11/17] show animation around chat input while working (#311125) * developer joy animation around input box * add back removed stuff * don't show border while stuff is working * better css * add color components and cleanup * progress border --- .../lib/stylelint/vscode-known-variables.json | 5 ++ src/vs/sessions/browser/media/style.css | 9 +++ .../contrib/chat/browser/chat.contribution.ts | 7 +- .../chatProgressContentPart.ts | 3 +- .../chatThinkingContentPart.ts | 3 +- .../chat/browser/widget/chatListRenderer.ts | 5 +- .../contrib/chat/browser/widget/chatWidget.ts | 20 ++++++ .../chat/browser/widget/media/chat.css | 66 +++++++++++++++++++ .../contrib/chat/common/constants.ts | 1 + .../contrib/chat/common/widget/chatColors.ts | 15 +++++ 10 files changed, 129 insertions(+), 5 deletions(-) diff --git a/build/lib/stylelint/vscode-known-variables.json b/build/lib/stylelint/vscode-known-variables.json index f0be1a9ea91d7..f8dcaac522a5f 100644 --- a/build/lib/stylelint/vscode-known-variables.json +++ b/build/lib/stylelint/vscode-known-variables.json @@ -61,6 +61,9 @@ "--vscode-chat-avatarForeground", "--vscode-chat-checkpointSeparator", "--vscode-chat-editedFileForeground", + "--vscode-chat-inputWorkingBorderColor1", + "--vscode-chat-inputWorkingBorderColor2", + "--vscode-chat-inputWorkingBorderColor3", "--vscode-chat-linesAddedForeground", "--vscode-chat-linesRemovedForeground", "--vscode-chat-requestBackground", @@ -1037,6 +1040,8 @@ "--monaco-editor-warning-decoration", "--animation-angle", "--animation-opacity", + "--chat-input-anim-angle", + "--chat-input-working-fill", "--chat-setup-dialog-glow-angle", "--vscode-chat-font-family", "--vscode-chat-font-size-body-l", diff --git a/src/vs/sessions/browser/media/style.css b/src/vs/sessions/browser/media/style.css index 1d4c768c0b18e..53f46f7cf228f 100644 --- a/src/vs/sessions/browser/media/style.css +++ b/src/vs/sessions/browser/media/style.css @@ -387,12 +387,21 @@ border-color: var(--vscode-agentsChatInput-border) !important; background-color: var(--vscode-agentsChatInput-background); color: var(--vscode-agentsChatInput-foreground); + /* Preserve the agents-app input background under the developer-joy ring. */ + --chat-input-working-fill: var(--vscode-agentsChatInput-background); } .agent-sessions-workbench .interactive-session .chat-input-container.focused { border-color: var(--vscode-agentsChatInput-focusBorder, var(--vscode-focusBorder)) !important; } +/* While the developer-joy animated border is active, suppress the static + border so it doesn't visually conflict with the spinning gradient ring. */ +.agent-sessions-workbench .interactive-session .chat-input-container.working, +.agent-sessions-workbench .interactive-session .chat-input-container.working.focused { + border-color: transparent !important; +} + /* Make the Monaco editor inside the chat input transparent so it inherits the chatInput.background */ .agent-sessions-workbench .interactive-session .interactive-input-part .chat-editor-container .interactive-input-editor .monaco-editor, .agent-sessions-workbench .interactive-session .interactive-input-part .chat-editor-container .interactive-input-editor .monaco-editor .monaco-editor-background { diff --git a/src/vs/workbench/contrib/chat/browser/chat.contribution.ts b/src/vs/workbench/contrib/chat/browser/chat.contribution.ts index e76d44b8645d2..2774b967bfa67 100644 --- a/src/vs/workbench/contrib/chat/browser/chat.contribution.ts +++ b/src/vs/workbench/contrib/chat/browser/chat.contribution.ts @@ -638,7 +638,12 @@ configurationRegistry.registerConfiguration({ [ChatConfiguration.ChatPersistentProgressEnabled]: { type: 'boolean', default: product.quality !== 'stable', - description: nls.localize('chat.persistentProgress.enabled', "Show elapsed time and token usage in chat response progress."), + description: nls.localize('chat.persistentProgress.enabled', "Always show progress in chat."), + }, + [ChatConfiguration.ProgressBorder]: { + type: 'boolean', + default: false, + markdownDescription: nls.localize('chat.progressBorder.enabled', "Show an animated gradient border around the chat input while the agent is working or thinking. When enabled, this overrides {0} to be off.", '`#chat.persistentProgress.enabled#`'), }, [ChatConfiguration.NotifyWindowOnResponseReceived]: { type: 'string', diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatProgressContentPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatProgressContentPart.ts index dacfc4d1184a8..ac8bcdf076c83 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatProgressContentPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatProgressContentPart.ts @@ -186,7 +186,8 @@ export class ChatWorkingProgressContentPart extends Disposable implements IChatC ) { super(); this.explicitContent = workingProgress.content; - const persistentProgressEnabled = configurationService.getValue(ChatConfiguration.ChatPersistentProgressEnabled) !== false; + const persistentProgressEnabled = configurationService.getValue(ChatConfiguration.ChatPersistentProgressEnabled) !== false + && configurationService.getValue(ChatConfiguration.ProgressBorder) !== true; if (persistentProgressEnabled) { const pool = buildPhrasePool(defaultThinkingMessages, configurationService); this.label = pool[Math.floor(Math.random() * pool.length)]; diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts index 05a004c927299..a4ac737f62398 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/chatThinkingContentPart.ts @@ -304,7 +304,8 @@ export class ChatThinkingContentPart extends ChatCollapsibleContentPart implemen this.id = content.id; this.content = content; this.allThinkingParts.push(content); - this.showProgressDetails = this.configurationService.getValue(ChatConfiguration.ChatPersistentProgressEnabled) !== false; + this.showProgressDetails = this.configurationService.getValue(ChatConfiguration.ChatPersistentProgressEnabled) !== false + && this.configurationService.getValue(ChatConfiguration.ProgressBorder) !== true; const configuredMode = this.configurationService.getValue('chat.agent.thinkingStyle') ?? ThinkingDisplayMode.Collapsed; this.fixedScrollingMode = configuredMode === ThinkingDisplayMode.FixedScrolling; diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts index b9d1f1d95a35e..4a1c20895e239 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatListRenderer.ts @@ -1061,7 +1061,8 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer(ChatConfiguration.ChatPersistentProgressEnabled) !== false; + const showProgressDetails = this.configService.getValue(ChatConfiguration.ChatPersistentProgressEnabled) !== false + && this.configService.getValue(ChatConfiguration.ProgressBorder) !== true; if (element.isComplete) { return undefined; } @@ -1227,7 +1228,7 @@ export class ChatListItemRenderer extends Disposable implements ITreeRenderer(ChatConfiguration.ChatPersistentProgressEnabled) !== false) { + if (element.isComplete && this.configService.getValue(ChatConfiguration.ChatPersistentProgressEnabled) !== false && this.configService.getValue(ChatConfiguration.ProgressBorder) !== true) { return; } diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts index 20951ea08e619..ea235a6c13645 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatWidget.ts @@ -465,6 +465,9 @@ export class ChatWidget extends Disposable implements IChatWidget { this.updateChatViewVisibility(); } } + if (e.affectsConfiguration(ChatConfiguration.ProgressBorder)) { + this.updateWorkingProgressBorder(); + } })); this._register(bindContextKey(decidedChatEditingResourceContextKey, contextKeyService, (reader) => { @@ -656,6 +659,20 @@ export class ChatWidget extends Disposable implements IChatWidget { return this.inlineInputPartDisposable.value!; } + private updateWorkingProgressBorder(): void { + const inputPart = this.inputPartDisposable.value; + if (!inputPart) { + return; + } + const inputContainer = inputPart.inputContainerElement; + if (!inputContainer) { + return; + } + const enabled = this.configurationService.getValue(ChatConfiguration.ProgressBorder) === true; + const inProgress = !!this.viewModel?.model.requestInProgress.get(); + inputContainer.classList.toggle('working', enabled && inProgress); + } + get inputEditor(): ICodeEditor { return this.input.inputEditor; } @@ -1980,6 +1997,7 @@ export class ChatWidget extends Disposable implements IChatWidget { this.finishedEditing(); } this.viewModel = undefined; + this.updateWorkingProgressBorder(); this.onDidChangeItems(); this._hasPendingRequestsContextKey.set(false); return; @@ -2035,6 +2053,7 @@ export class ChatWidget extends Disposable implements IChatWidget { this.requestInProgress.set(this.viewModel.model.requestInProgress.get()); this.hasActiveRequest.set(this.viewModel.model.hasActiveRequest.get()); + this.updateWorkingProgressBorder(); // Update the editor's placeholder text when it changes in the view model if (events?.some(e => e?.kind === 'changePlaceholder')) { @@ -2053,6 +2072,7 @@ export class ChatWidget extends Disposable implements IChatWidget { } // Disposes the viewmodel and listeners this.viewModel = undefined; + this.updateWorkingProgressBorder(); this.onDidChangeItems(); })); this._sessionIsEmptyContextKey.set(model.getRequests().length === 0); diff --git a/src/vs/workbench/contrib/chat/browser/widget/media/chat.css b/src/vs/workbench/contrib/chat/browser/widget/media/chat.css index 4b777d2711c02..05f8a04272fa6 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/media/chat.css +++ b/src/vs/workbench/contrib/chat/browser/widget/media/chat.css @@ -871,6 +871,72 @@ have to be updated for changes to the rules above, or to support more deeply nes overflow: hidden; } +/* Animated gradient border shown around the chat input while the agent is + working or thinking. Toggled by the `chat.progressBorder.enabled` + setting and the chat widget's request-in-progress state. The ring is + rendered using layered `background-image` + `background-clip` + (`padding-box`/`border-box`), so it traces the input's outer corner + radius and isn't clipped by `overflow: hidden` on the parent. */ +@property --chat-input-anim-angle { + syntax: ''; + inherits: false; + initial-value: 135deg; +} + +@keyframes chat-input-working-border-spin { + from { + --chat-input-anim-angle: 135deg; + } + + to { + --chat-input-anim-angle: 495deg; + } +} + +@keyframes chat-input-working-border-glow { + + 0%, + 100% { + box-shadow: + 0 0 4px 0 color-mix(in srgb, var(--vscode-chat-inputWorkingBorderColor1) 18%, transparent), + 0 0 10px 0 color-mix(in srgb, var(--vscode-chat-inputWorkingBorderColor3) 8%, transparent); + } + + 50% { + box-shadow: + 0 0 6px 0 color-mix(in srgb, var(--vscode-chat-inputWorkingBorderColor2) 22%, transparent), + 0 0 14px 1px color-mix(in srgb, var(--vscode-chat-inputWorkingBorderColor1) 12%, transparent); + } +} + +.monaco-workbench .interactive-session .chat-input-container.working { + border-color: transparent; + /* The padding-box layer fills the input interior. It defaults to the + standard input background, but each host can override + `--chat-input-working-fill` to keep its own background color (e.g. the + Sessions workbench sets it to `--vscode-agentsChatInput-background`). + The conic-gradient layer is clipped to the border box so it paints + exactly where the (transparent) border lives. */ + background: + linear-gradient(var(--chat-input-working-fill, var(--vscode-input-background)), + var(--chat-input-working-fill, var(--vscode-input-background))) padding-box, + conic-gradient(from var(--chat-input-anim-angle), + var(--vscode-chat-inputWorkingBorderColor1), + var(--vscode-chat-inputWorkingBorderColor2), + var(--vscode-chat-inputWorkingBorderColor3), + var(--vscode-chat-inputWorkingBorderColor2), + var(--vscode-chat-inputWorkingBorderColor1)) border-box; + animation: + chat-input-working-border-spin 1.2s linear infinite, + chat-input-working-border-glow 3s ease-in-out infinite; +} + +@media (prefers-reduced-motion: reduce) { + .monaco-workbench .interactive-session .chat-input-container.working { + animation: none; + } +} + /* Context usage widget container - positioned in the secondary toolbar below input */ .interactive-session .chat-input-toolbars .chat-context-usage-container, .interactive-session .chat-secondary-toolbar .chat-context-usage-container { diff --git a/src/vs/workbench/contrib/chat/common/constants.ts b/src/vs/workbench/contrib/chat/common/constants.ts index 49a79cf797d9e..f26809501a5f4 100644 --- a/src/vs/workbench/contrib/chat/common/constants.ts +++ b/src/vs/workbench/contrib/chat/common/constants.ts @@ -48,6 +48,7 @@ export enum ChatConfiguration { ChatViewProgressBadgeEnabled = 'chat.viewProgressBadge.enabled', ChatContextUsageEnabled = 'chat.contextUsage.enabled', ChatPersistentProgressEnabled = 'chat.persistentProgress.enabled', + ProgressBorder = 'chat.progressBorder.enabled', SubagentToolCustomAgents = 'chat.customAgentInSubagent.enabled', GeneralPurposeAgentEnabled = 'chat.generalPurposeAgent.enabled', SubagentsAllowInvocationsFromSubagents = 'chat.subagents.allowInvocationsFromSubagents', diff --git a/src/vs/workbench/contrib/chat/common/widget/chatColors.ts b/src/vs/workbench/contrib/chat/common/widget/chatColors.ts index bb256ebba1988..7b56d9af30699 100644 --- a/src/vs/workbench/contrib/chat/common/widget/chatColors.ts +++ b/src/vs/workbench/contrib/chat/common/widget/chatColors.ts @@ -87,3 +87,18 @@ export const chatThinkingShimmer = registerColor( 'chat.thinkingShimmer', { dark: '#ffffff', light: '#000000', hcDark: '#ffffff', hcLight: '#000000' }, localize('chat.thinkingShimmer', 'Shimmer highlight for thinking/working labels.'), true); + +export const chatInputWorkingBorderColor1 = registerColor( + 'chat.inputWorkingBorderColor1', + { dark: '#b44aff', light: '#b44aff', hcDark: '#b44aff', hcLight: '#b44aff' }, + localize('chat.inputWorkingBorderColor1', 'First color stop of the animated chat input border shown while a request is in flight.'), true); + +export const chatInputWorkingBorderColor2 = registerColor( + 'chat.inputWorkingBorderColor2', + { dark: '#4af0c0', light: '#4af0c0', hcDark: '#4af0c0', hcLight: '#4af0c0' }, + localize('chat.inputWorkingBorderColor2', 'Second color stop of the animated chat input border shown while a request is in flight.'), true); + +export const chatInputWorkingBorderColor3 = registerColor( + 'chat.inputWorkingBorderColor3', + { dark: '#51a2ff', light: '#51a2ff', hcDark: '#51a2ff', hcLight: '#51a2ff' }, + localize('chat.inputWorkingBorderColor3', 'Third color stop of the animated chat input border shown while a request is in flight.'), true); From 2f926976902f7d293ec3135b8c51cf95e3dbc6ac Mon Sep 17 00:00:00 2001 From: Maruthan G <113752568+maruthang@users.noreply.github.com> Date: Tue, 21 Apr 2026 10:38:08 +0530 Subject: [PATCH 12/17] fix(chat): enable scrollbar for height-capped code blocks in tool confirmations (#283242) (#310975) * fix(chat): enable scrollbar for height-capped code blocks in tool confirmations (#283242) * fix(chat): use chat scrollbar width for capped code blocks (Written by Copilot) --------- Co-authored-by: Rob Lourens --- .../widget/chatContentParts/codeBlockPart.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/codeBlockPart.ts b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/codeBlockPart.ts index 0022affa04472..e7f303cfd9dd3 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/codeBlockPart.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/codeBlockPart.ts @@ -119,6 +119,7 @@ export interface ICodeBlockRenderOptions { } const defaultCodeblockPadding = 10; +const defaultChatScrollbarSize = 7; export class CodeBlockPart extends Disposable { /** @@ -380,6 +381,15 @@ export class CodeBlockPart extends Disposable { } private getEditorOptionsFromConfig(): IEditorOptions { + const renderOptions = this.currentCodeBlockData?.renderOptions; + // When the code block is height-capped via `maxHeightInLines`, content can + // exceed the visible area. In that case the default hidden vertical + // scrollbar leaves users unable to reach the clipped content (see #283242). + // Enable a chat-sized visible scrollbar. Callers can still override + // via `renderOptions.editorOptions.scrollbar`. + const scrollbar: IEditorOptions['scrollbar'] | undefined = renderOptions?.maxHeightInLines + ? { vertical: 'auto', verticalScrollbarSize: defaultChatScrollbarSize, ...renderOptions?.editorOptions?.scrollbar } + : undefined; return { wordWrap: this.editorOptions.configuration.resultEditor.wordWrap, fontLigatures: this.editorOptions.configuration.resultEditor.fontLigatures, @@ -390,7 +400,8 @@ export class CodeBlockPart extends Disposable { fontSize: this.editorOptions.configuration.resultEditor.fontSize, fontWeight: this.editorOptions.configuration.resultEditor.fontWeight, lineHeight: this.editorOptions.configuration.resultEditor.lineHeight, - ...this.currentCodeBlockData?.renderOptions?.editorOptions, + ...renderOptions?.editorOptions, + ...(scrollbar ? { scrollbar } : {}), }; } From 938f78a972c8aec490d2687bea319c01a5725223 Mon Sep 17 00:00:00 2001 From: Justin Chen <54879025+justschen@users.noreply.github.com> Date: Mon, 20 Apr 2026 22:52:18 -0700 Subject: [PATCH 13/17] add better padding for thinking + persistant progress (#311603) --- .../widget/chatContentParts/media/chatThinkingContent.css | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatThinkingContent.css b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatThinkingContent.css index 47031cccd9734..0fe0b4f3d4b7e 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatThinkingContent.css +++ b/src/vs/workbench/contrib/chat/browser/widget/chatContentParts/media/chatThinkingContent.css @@ -376,7 +376,7 @@ } &.chat-thinking-persistent-streaming { - margin-bottom: 0; + margin-bottom: 3px; } > .monaco-scrollable-element > .shadow { From d844c098294938a0e8cc4bc71a4e099b4f3f46b2 Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Tue, 21 Apr 2026 01:12:06 -0700 Subject: [PATCH 14/17] handle tap and click properly (#311604) * handle tap and click properly Co-authored-by: Copilot * :lipstick: type Co-authored-by: Copilot --------- Co-authored-by: Copilot --- .../contrib/chat/browser/sessionTypePicker.ts | 12 ++++++++---- .../contrib/chat/browser/sessionWorkspacePicker.ts | 12 ++++++++---- .../copilotChatSessions/browser/branchPicker.ts | 12 ++++++++---- .../browser/claudePermissionModePicker.ts | 12 ++++++++---- .../copilotChatSessions/browser/isolationPicker.ts | 12 ++++++++---- .../copilotChatSessions/browser/modePicker.ts | 12 ++++++++---- .../copilotChatSessions/browser/modelPicker.ts | 12 ++++++++---- .../copilotChatSessions/browser/permissionPicker.ts | 12 ++++++++---- .../chat/browser/widget/input/chatModelPicker.ts | 4 ++-- 9 files changed, 66 insertions(+), 34 deletions(-) diff --git a/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts b/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts index 281eaec54bc91..a5b3caabcc53f 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionTypePicker.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as dom from '../../../../base/browser/dom.js'; +import { Gesture, EventType as TouchEventType } from '../../../../base/browser/touch.js'; import { Codicon } from '../../../../base/common/codicons.js'; import { Disposable, DisposableStore } from '../../../../base/common/lifecycle.js'; import { renderIcon } from '../../../../base/browser/ui/iconLabel/iconLabels.js'; @@ -81,10 +82,13 @@ export class SessionTypePicker extends Disposable { this._triggerElement = trigger; this._updateTriggerLabel(); - this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.CLICK, (e) => { - dom.EventHelper.stop(e, true); - this._showPicker(); - })); + this._renderDisposables.add(Gesture.addTarget(trigger)); + for (const eventType of [dom.EventType.CLICK, TouchEventType.Tap]) { + this._renderDisposables.add(dom.addDisposableListener(trigger, eventType, (e) => { + dom.EventHelper.stop(e, true); + this._showPicker(); + })); + } this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.KEY_DOWN, (e) => { if (e.key === 'Enter' || e.key === ' ') { diff --git a/src/vs/sessions/contrib/chat/browser/sessionWorkspacePicker.ts b/src/vs/sessions/contrib/chat/browser/sessionWorkspacePicker.ts index ba888405c0894..5ff4dcc58eb50 100644 --- a/src/vs/sessions/contrib/chat/browser/sessionWorkspacePicker.ts +++ b/src/vs/sessions/contrib/chat/browser/sessionWorkspacePicker.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as dom from '../../../../base/browser/dom.js'; +import * as touch from '../../../../base/browser/touch.js'; import { IAction, SubmenuAction, toAction } from '../../../../base/common/actions.js'; import { Codicon } from '../../../../base/common/codicons.js'; import { Emitter, Event } from '../../../../base/common/event.js'; @@ -160,10 +161,13 @@ export class WorkspacePicker extends Disposable { this._updateTriggerLabel(); - this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.CLICK, (e) => { - dom.EventHelper.stop(e, true); - this.showPicker(); - })); + this._renderDisposables.add(touch.Gesture.addTarget(trigger)); + [dom.EventType.CLICK, touch.EventType.Tap].forEach(eventType => { + this._renderDisposables.add(dom.addDisposableListener(trigger, eventType, (e) => { + dom.EventHelper.stop(e, true); + this.showPicker(); + })); + }); this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.KEY_DOWN, (e) => { if (e.key === 'Enter' || e.key === ' ') { diff --git a/src/vs/sessions/contrib/copilotChatSessions/browser/branchPicker.ts b/src/vs/sessions/contrib/copilotChatSessions/browser/branchPicker.ts index 61bd15c3bf54d..ef948ec88bd8b 100644 --- a/src/vs/sessions/contrib/copilotChatSessions/browser/branchPicker.ts +++ b/src/vs/sessions/contrib/copilotChatSessions/browser/branchPicker.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as dom from '../../../../base/browser/dom.js'; +import { Gesture, EventType as TouchEventType } from '../../../../base/browser/touch.js'; import { Codicon } from '../../../../base/common/codicons.js'; import { Disposable, DisposableStore } from '../../../../base/common/lifecycle.js'; import { autorun } from '../../../../base/common/observable.js'; @@ -75,10 +76,13 @@ export class BranchPicker extends Disposable { this._triggerElement = trigger; this._updateTriggerLabel(); - this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.CLICK, (e) => { - dom.EventHelper.stop(e, true); - this.showPicker(); - })); + this._renderDisposables.add(Gesture.addTarget(trigger)); + for (const eventType of [dom.EventType.CLICK, TouchEventType.Tap]) { + this._renderDisposables.add(dom.addDisposableListener(trigger, eventType, (e) => { + dom.EventHelper.stop(e, true); + this.showPicker(); + })); + } this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.KEY_DOWN, (e) => { if (e.key === 'Enter' || e.key === ' ') { diff --git a/src/vs/sessions/contrib/copilotChatSessions/browser/claudePermissionModePicker.ts b/src/vs/sessions/contrib/copilotChatSessions/browser/claudePermissionModePicker.ts index 9f56fca68a8ab..70e56549575d0 100644 --- a/src/vs/sessions/contrib/copilotChatSessions/browser/claudePermissionModePicker.ts +++ b/src/vs/sessions/contrib/copilotChatSessions/browser/claudePermissionModePicker.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as dom from '../../../../base/browser/dom.js'; +import { Gesture, EventType as TouchEventType } from '../../../../base/browser/touch.js'; import { renderIcon } from '../../../../base/browser/ui/iconLabel/iconLabels.js'; import { Codicon } from '../../../../base/common/codicons.js'; import { Disposable, DisposableStore } from '../../../../base/common/lifecycle.js'; @@ -72,10 +73,13 @@ export class ClaudePermissionModePicker extends Disposable { this._updateTriggerLabel(trigger); - this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.CLICK, (e) => { - dom.EventHelper.stop(e, true); - this._showPicker(); - })); + this._renderDisposables.add(Gesture.addTarget(trigger)); + for (const eventType of [dom.EventType.CLICK, TouchEventType.Tap]) { + this._renderDisposables.add(dom.addDisposableListener(trigger, eventType, (e) => { + dom.EventHelper.stop(e, true); + this._showPicker(); + })); + } this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.KEY_DOWN, (e) => { if (e.key === 'Enter' || e.key === ' ') { diff --git a/src/vs/sessions/contrib/copilotChatSessions/browser/isolationPicker.ts b/src/vs/sessions/contrib/copilotChatSessions/browser/isolationPicker.ts index 11b60551d1a2f..f913ebd00fe0e 100644 --- a/src/vs/sessions/contrib/copilotChatSessions/browser/isolationPicker.ts +++ b/src/vs/sessions/contrib/copilotChatSessions/browser/isolationPicker.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as dom from '../../../../base/browser/dom.js'; +import { Gesture, EventType as TouchEventType } from '../../../../base/browser/touch.js'; import { Codicon } from '../../../../base/common/codicons.js'; import { Disposable, DisposableStore } from '../../../../base/common/lifecycle.js'; import { autorun } from '../../../../base/common/observable.js'; @@ -103,10 +104,13 @@ export class IsolationPicker extends Disposable { this._triggerElement = trigger; this._updateTriggerLabel(); - this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.CLICK, (e) => { - dom.EventHelper.stop(e, true); - this._showPicker(); - })); + this._renderDisposables.add(Gesture.addTarget(trigger)); + for (const eventType of [dom.EventType.CLICK, TouchEventType.Tap]) { + this._renderDisposables.add(dom.addDisposableListener(trigger, eventType, (e) => { + dom.EventHelper.stop(e, true); + this._showPicker(); + })); + } this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.KEY_DOWN, (e) => { if (e.key === 'Enter' || e.key === ' ') { diff --git a/src/vs/sessions/contrib/copilotChatSessions/browser/modePicker.ts b/src/vs/sessions/contrib/copilotChatSessions/browser/modePicker.ts index 438ab2251c02b..4fd157dc52adf 100644 --- a/src/vs/sessions/contrib/copilotChatSessions/browser/modePicker.ts +++ b/src/vs/sessions/contrib/copilotChatSessions/browser/modePicker.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as dom from '../../../../base/browser/dom.js'; +import { Gesture, EventType as TouchEventType } from '../../../../base/browser/touch.js'; import { Codicon } from '../../../../base/common/codicons.js'; import { Emitter, Event } from '../../../../base/common/event.js'; import { Disposable, DisposableStore } from '../../../../base/common/lifecycle.js'; @@ -94,10 +95,13 @@ export class ModePicker extends Disposable { this._updateTriggerLabel(); - this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.CLICK, (e) => { - dom.EventHelper.stop(e, true); - this._showPicker(); - })); + this._renderDisposables.add(Gesture.addTarget(trigger)); + for (const eventType of [dom.EventType.CLICK, TouchEventType.Tap]) { + this._renderDisposables.add(dom.addDisposableListener(trigger, eventType, (e) => { + dom.EventHelper.stop(e, true); + this._showPicker(); + })); + } this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.KEY_DOWN, (e) => { if (e.key === 'Enter' || e.key === ' ') { diff --git a/src/vs/sessions/contrib/copilotChatSessions/browser/modelPicker.ts b/src/vs/sessions/contrib/copilotChatSessions/browser/modelPicker.ts index e62b24fe7eca2..277078c2582af 100644 --- a/src/vs/sessions/contrib/copilotChatSessions/browser/modelPicker.ts +++ b/src/vs/sessions/contrib/copilotChatSessions/browser/modelPicker.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as dom from '../../../../base/browser/dom.js'; +import { Gesture, EventType as TouchEventType } from '../../../../base/browser/touch.js'; import { Codicon } from '../../../../base/common/codicons.js'; import { Emitter, Event } from '../../../../base/common/event.js'; import { Disposable, DisposableStore } from '../../../../base/common/lifecycle.js'; @@ -108,10 +109,13 @@ export class CloudModelPicker extends Disposable { this._updateTriggerLabel(); - this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.CLICK, (e) => { - dom.EventHelper.stop(e, true); - this._showPicker(); - })); + this._renderDisposables.add(Gesture.addTarget(trigger)); + for (const eventType of [dom.EventType.CLICK, TouchEventType.Tap]) { + this._renderDisposables.add(dom.addDisposableListener(trigger, eventType, (e) => { + dom.EventHelper.stop(e, true); + this._showPicker(); + })); + } this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.KEY_DOWN, (e) => { if (e.key === 'Enter' || e.key === ' ') { diff --git a/src/vs/sessions/contrib/copilotChatSessions/browser/permissionPicker.ts b/src/vs/sessions/contrib/copilotChatSessions/browser/permissionPicker.ts index ad080429eb714..ea75265e07a07 100644 --- a/src/vs/sessions/contrib/copilotChatSessions/browser/permissionPicker.ts +++ b/src/vs/sessions/contrib/copilotChatSessions/browser/permissionPicker.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as dom from '../../../../base/browser/dom.js'; +import { Gesture, EventType as TouchEventType } from '../../../../base/browser/touch.js'; import { Codicon } from '../../../../base/common/codicons.js'; import { Disposable, DisposableStore } from '../../../../base/common/lifecycle.js'; import { autorun, IObservable } from '../../../../base/common/observable.js'; @@ -101,10 +102,13 @@ export class PermissionPicker extends Disposable { this._updateTriggerLabel(trigger); - this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.CLICK, (e) => { - dom.EventHelper.stop(e, true); - this.showPicker(); - })); + this._renderDisposables.add(Gesture.addTarget(trigger)); + for (const eventType of [dom.EventType.CLICK, TouchEventType.Tap]) { + this._renderDisposables.add(dom.addDisposableListener(trigger, eventType, (e) => { + dom.EventHelper.stop(e, true); + this.showPicker(); + })); + } this._renderDisposables.add(dom.addDisposableListener(trigger, dom.EventType.KEY_DOWN, (e) => { if (e.key === 'Enter' || e.key === ' ') { diff --git a/src/vs/workbench/contrib/chat/browser/widget/input/chatModelPicker.ts b/src/vs/workbench/contrib/chat/browser/widget/input/chatModelPicker.ts index 5774694645740..8ca3c47be607e 100644 --- a/src/vs/workbench/contrib/chat/browser/widget/input/chatModelPicker.ts +++ b/src/vs/workbench/contrib/chat/browser/widget/input/chatModelPicker.ts @@ -616,8 +616,8 @@ export class ModelPickerWidget extends Disposable { this._renderLabel(); - // Open picker on click - this._register(dom.addDisposableListener(this._domNode, dom.EventType.MOUSE_DOWN, (e) => { + // Open picker on click (uses pointerdown on iOS where mousedown is unreliable) + this._register(dom.addDisposableGenericMouseDownListener(this._domNode, e => { if (e.button !== 0) { return; // only left click } From 018a47f3df8dd2121e9bf96c5e24cbbabd8ed9da Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 21 Apr 2026 19:11:44 +1000 Subject: [PATCH 15/17] feat(copilotcli): add WorktreeSessionIndex for managing chat session entries with JSONL persistence (#311609) * feat(copilotcli): add WorktreeSessionIndex for managing chat session entries with JSONL persistence * Udpates Co-authored-by: Copilot * Updates * Update extensions/copilot/src/extension/chatSessions/vscode-node/chatSessionMetadataStoreImpl.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fixes Co-authored-by: Copilot --------- Co-authored-by: Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../common/chatSessionMetadataStore.ts | 21 + .../test/mockChatSessionMetadataStore.ts | 4 + .../copilotcli/node/cliHelpers.ts | 18 + .../chatSessionMetadataStoreImpl.ts | 578 ++++++--- .../chatSessionWorktreeServiceImpl.ts | 10 +- .../vscode-node/copilotCLIChatSessions.ts | 1 + .../copilotCLIChatSessionsContribution.ts | 5 + .../test/chatSessionMetadataStoreImpl.spec.ts | 1033 ++++++++--------- .../test/worktreeSessionIndex.spec.ts | 190 +++ .../vscode-node/worktreeSessionIndex.ts | 221 ++++ 10 files changed, 1353 insertions(+), 728 deletions(-) create mode 100644 extensions/copilot/src/extension/chatSessions/vscode-node/test/worktreeSessionIndex.spec.ts create mode 100644 extensions/copilot/src/extension/chatSessions/vscode-node/worktreeSessionIndex.ts diff --git a/extensions/copilot/src/extension/chatSessions/common/chatSessionMetadataStore.ts b/extensions/copilot/src/extension/chatSessions/common/chatSessionMetadataStore.ts index 97e2c19b43969..e38c94ebdffa0 100644 --- a/extensions/copilot/src/extension/chatSessions/common/chatSessionMetadataStore.ts +++ b/extensions/copilot/src/extension/chatSessions/common/chatSessionMetadataStore.ts @@ -91,6 +91,21 @@ export interface ChatSessionMetadataFile { * session or if the session is a child session created from the Agents app. */ parentSessionId?: string; + /** Milliseconds since epoch when this metadata was first written. */ + created?: number; + /** Milliseconds since epoch of the last write. Used for top-N trim sort and cross-process merge. */ + modified?: number; +} + +/** + * One line in `~/.copilot/vscode.session.worktree.jsonl`. Maps a session id + * to the path of its worktree so folder → session lookups work even when the + * session has been evicted from the bulk metadata cache. + */ +export interface WorktreeSessionEntry { + readonly id: string; + readonly path: string; + readonly created: number; } export const IChatSessionMetadataStore = createServiceIdentifier('IChatSessionMetadataStore'); @@ -126,4 +141,10 @@ export interface IChatSessionMetadataStore { getSessionOrigin(sessionId: string): Promise<'vscode' | 'other'>; setSessionParentId(sessionId: string, parentSessionId: string): Promise; getSessionParentId(sessionId: string): Promise; + /** + * Re-read the shared bulk metadata file from disk and merge into the in-memory cache. + * Wired to the chat-sessions UI refresh action so cross-process writes become visible + * on demand. Concurrent calls collapse: at most one in-flight + one pending. + */ + refresh(): Promise; } diff --git a/extensions/copilot/src/extension/chatSessions/common/test/mockChatSessionMetadataStore.ts b/extensions/copilot/src/extension/chatSessions/common/test/mockChatSessionMetadataStore.ts index b4d76b55ec77b..96aa2ee811ec1 100644 --- a/extensions/copilot/src/extension/chatSessions/common/test/mockChatSessionMetadataStore.ts +++ b/extensions/copilot/src/extension/chatSessions/common/test/mockChatSessionMetadataStore.ts @@ -31,6 +31,10 @@ export class MockChatSessionMetadataStore implements IChatSessionMetadataStore { this._requestDetails.delete(sessionId); } + async refresh(): Promise { + // no-op in mock — there is no on-disk state to reload. + } + async storeWorktreeInfo(sessionId: string, properties: ChatSessionWorktreeProperties): Promise { this._worktreeProperties.set(sessionId, properties); } diff --git a/extensions/copilot/src/extension/chatSessions/copilotcli/node/cliHelpers.ts b/extensions/copilot/src/extension/chatSessions/copilotcli/node/cliHelpers.ts index 3ecbf0330d59a..ea9074ae5d3bc 100644 --- a/extensions/copilot/src/extension/chatSessions/copilotcli/node/cliHelpers.ts +++ b/extensions/copilot/src/extension/chatSessions/copilotcli/node/cliHelpers.ts @@ -36,3 +36,21 @@ export function getCopilotCLISessionEventsFile(sessionId: string) { export function getCopilotCLIWorkspaceFile(sessionId: string) { return join(getCopilotCLISessionDir(sessionId), 'workspace.yaml'); } + +/** + * Path of the shared bulk metadata cache file. This file is shared by all VS Code + * installs (Stable, Insiders, OSS, Exploration) and the Agents application. + */ +export function getCopilotBulkMetadataFile(): string { + return join(getCopilotHome(), 'vscode.session.metadata.cache.json'); +} + +/** + * Path of the shared worktree-sessions JSONL index. Append-only, one + * {@link WorktreeSessionEntry} per line. + * Used as a worktree folder → session-id fallback + * when an entry has been evicted from the bulk cache. + */ +export function getCopilotWorktreeSessionsFile(): string { + return join(getCopilotHome(), 'vscode.session.worktree.jsonl'); +} diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/chatSessionMetadataStoreImpl.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/chatSessionMetadataStoreImpl.ts index 8a9f81751d994..fc5790906f792 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode-node/chatSessionMetadataStoreImpl.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/chatSessionMetadataStoreImpl.ts @@ -10,30 +10,65 @@ import { createDirectoryIfNotExists, IFileSystemService } from '../../../platfor import { ILogService } from '../../../platform/log/common/logService'; import { findLast } from '../../../util/vs/base/common/arraysFind'; import { SequencerByKey, ThrottledDelayer } from '../../../util/vs/base/common/async'; -import { Lazy } from '../../../util/vs/base/common/lazy'; import { Disposable } from '../../../util/vs/base/common/lifecycle'; import { dirname, isEqual } from '../../../util/vs/base/common/resources'; -import { ChatSessionMetadataFile, IChatSessionMetadataStore, RepositoryProperties, RequestDetails, WorkspaceFolderEntry } from '../common/chatSessionMetadataStore'; -import { ChatSessionWorktreeData, ChatSessionWorktreeProperties } from '../common/chatSessionWorktreeService'; +import { ChatSessionMetadataFile, IChatSessionMetadataStore, RepositoryProperties, RequestDetails, WorkspaceFolderEntry, WorktreeSessionEntry } from '../common/chatSessionMetadataStore'; +import { ChatSessionWorktreeProperties } from '../common/chatSessionWorktreeService'; import { isUntitledSessionId } from '../common/utils'; import { IWorkspaceInfo } from '../common/workspaceInfo'; -import { getCopilotCLISessionDir } from '../copilotcli/node/cliHelpers'; +import { getCopilotBulkMetadataFile, getCopilotCLISessionDir, getCopilotCLISessionStateDir, getCopilotWorktreeSessionsFile } from '../copilotcli/node/cliHelpers'; import { ICopilotCLIAgents } from '../copilotcli/node/copilotCli'; +import { WorktreeSessionIndex } from './worktreeSessionIndex'; -const WORKSPACE_FOLDER_MEMENTO_KEY = 'github.copilot.cli.sessionWorkspaceFolders'; -const WORKTREE_MEMENTO_KEY = 'github.copilot.cli.sessionWorktrees'; -const BULK_METADATA_FILENAME = 'copilotcli.session.metadata.json'; +// const WORKSPACE_FOLDER_MEMENTO_KEY = 'github.copilot.cli.sessionWorkspaceFolders'; +// const WORKTREE_MEMENTO_KEY = 'github.copilot.cli.sessionWorktrees'; +const LEGACY_BULK_METADATA_FILENAME = 'copilotcli.session.metadata.json'; +const LEGACY_BULK_MIGRATED_KEY = 'github.copilot.cli.legacyBulkMigrated'; +const JSONL_SCAN_DONE_KEY = 'github.copilot.cli.events.jsonl.scaned'; const REQUEST_MAPPING_FILENAME = 'vscode.requests.metadata.json'; +const SESSION_SCAN_BATCH_SIZE = 20; + +/** + * Maximum number of sessions kept in the shared bulk metadata cache file + * (`~/.copilot/vscode.session.metadata.cache.json`). Older entries (by `modified`) + * are evicted from the file but remain available via the per-session metadata files + * (`~/.copilot/session-state/{id}/vscode.metadata.json`) and the JSONL worktree index. + */ +const MAX_BULK_STORAGE_ENTRIES = 1000; + +/** Single-key sequencer key used to serialize bulk-file flush against {@link refresh}. */ +const BULK_SEQUENCER_KEY = 'bulk'; export class ChatSessionMetadataStore extends Disposable implements IChatSessionMetadataStore { declare _serviceBrand: undefined; + + /** + * In-memory mirror of the bulk metadata file plus on-demand entries hydrated by + * {@link getSessionMetadata}. Always retains everything it has seen; only the on-disk + * file is trimmed to {@link MAX_BULK_STORAGE_ENTRIES}. + */ private _cache: Record = {}; - private readonly _cacheDirectory: Uri; - private readonly _cacheFile: Uri; - private readonly _intialize: Lazy>; + + /** Maps session id → JSONL entry and folder path → session id. Owns JSONL file persistence. */ + private readonly _worktreeSessions: WorktreeSessionIndex; + + /** Path of the shared bulk metadata cache file in `~/.copilot/`. */ + private readonly _cacheFile = Uri.file(getCopilotBulkMetadataFile()); + + /** + * Single-promise gate. Initially set to `initializeStorage()`; {@link refresh} chains + * a {@link reloadBulkFromDisk} call onto it so concurrent refreshes collapse to at + * most one in-flight + one pending. Reads and writes both `await` this so they queue + * behind any in-flight refresh. + */ + private _ready: Promise; + private readonly _updateStorageDebouncer = this._register(new ThrottledDelayer(1_000)); private readonly _requestMappingWriteSequencer = new SequencerByKey(); private readonly _metadataWriteSequencer = new SequencerByKey(); + /** Serializes bulk-file flush against {@link reloadBulkFromDisk}. */ + private readonly _bulkSequencer = new SequencerByKey(); + constructor( @IFileSystemService private readonly fileSystemService: IFileSystemService, @ILogService private readonly logService: ILogService, @@ -42,103 +77,52 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession ) { super(); - this._cacheDirectory = Uri.joinPath(this.extensionContext.globalStorageUri, 'copilotcli'); - this._cacheFile = Uri.joinPath(this._cacheDirectory, BULK_METADATA_FILENAME); - this._intialize = new Lazy>(this.initializeStorage.bind(this)); - this._intialize.value.catch(error => { + this._worktreeSessions = new WorktreeSessionIndex( + this.fileSystemService, + this.logService, + getCopilotWorktreeSessionsFile(), + ); + + this._ready = this.initializeStorage(); + this._ready.catch(error => { this.logService.error('[ChatSessionMetadataStore] Initialization failed: ', error); }); } - private async initializeStorage(): Promise { - try { - this._cache = await this.getGlobalStorageData(); - // In case user closed vscode early or we couldn't save the session information for some reason. - for (const [sessionId, metadata] of Object.entries(this._cache)) { - if (sessionId.startsWith('untitled-')) { - delete this._cache[sessionId]; - continue; - } - if (!metadata.writtenToDisc) { - if ((metadata.workspaceFolder || metadata.worktreeProperties || metadata.additionalWorkspaces?.length)) { - this.updateSessionMetadata(sessionId, metadata, false).catch(ex => { - this.logService.error(ex, `[ChatSessionMetadataStore] Failed to write metadata for session ${sessionId} to session state: `); - }); - } else { - // invalid data, we don't need this in our cache. - delete this._cache[sessionId]; - } - } - } - // Dont' exit from here, keep reaading from global storage. - // Possible we had a bug and we missed writing some metadata to disc. - } catch { - // - } + public refresh(): Promise { + // Chain onto the existing `_ready` — concurrent calls collapse to at most one + // in-flight + one pending. `.catch(() => undefined)` ensures a failed prior + // step does not poison subsequent reads/writes. + this._ready = this._ready.catch(() => undefined).then(() => this.reloadBulkFromDisk()); + return this._ready; + } - let cacheUpdated = false; - // Collect workspace folder entries from global state - const workspaceFolderData = this.extensionContext.globalState.get>>(WORKSPACE_FOLDER_MEMENTO_KEY, {}); - for (const [sessionId, entry] of Object.entries(workspaceFolderData)) { - if (typeof entry === 'string' || !entry.folderPath || !entry.timestamp) { - continue; - } - if (sessionId.startsWith('untitled-')) { - continue; - } - if (sessionId in this._cache && this._cache[sessionId].workspaceFolder) { - continue; - } - cacheUpdated = true; - this._cache[sessionId] = { workspaceFolder: { folderPath: entry.folderPath, timestamp: entry.timestamp } }; - } + private async initializeStorage(): Promise { + // One-time migration from the legacy per-install bulk file in + // globalStorageUri to the shared `~/.copilot/` location. + await this.migrateLegacyBulkFile(); - // Collect worktree entries from global state - const worktreeData = this.extensionContext.globalState.get>(WORKTREE_MEMENTO_KEY, {}); - for (const [sessionId, value] of Object.entries(worktreeData)) { - if (typeof value === 'string') { - continue; - } + this._cache = await this.getGlobalStorageData().catch(() => ({} as Record)); + // In case user closed vscode early or we couldn't save the session information for some reason. + for (const [sessionId, metadata] of Object.entries(this._cache)) { if (sessionId.startsWith('untitled-')) { + delete this._cache[sessionId]; continue; } - if (sessionId in this._cache && this._cache[sessionId].worktreeProperties) { - const parsedData: ChatSessionWorktreeProperties = value.version === 1 ? { ...JSON.parse(value.data), version: 1 } : JSON.parse(value.data); - const changesInFileStorage = this._cache[sessionId].worktreeProperties?.changes; - const changesInGlobalState = parsedData.changes; - // There was a bug that resulted in changes not being written to file storage, but they were written to global state. - // In that case we want to keep the changes from global state, otherwise we might lose data. - if ((changesInGlobalState || []).length === (changesInFileStorage || []).length) { - continue; - } - } - cacheUpdated = true; - { - const parsedData: ChatSessionWorktreeProperties = value.version === 1 ? { ...JSON.parse(value.data), version: 1 } : JSON.parse(value.data); - this._cache[sessionId] = { ...this._cache[sessionId], workspaceFolder: undefined, worktreeProperties: parsedData, writtenToDisc: false }; + if (!(metadata.workspaceFolder || metadata.worktreeProperties || metadata.additionalWorkspaces?.length)) { + // invalid data, we don't need this in our cache. + delete this._cache[sessionId]; } } - for (const [sessionId, metadata] of Object.entries(this._cache)) { - // These promises can run in background and no need to wait for them. - // Even if user exits early we have all the data in the global storage and we'll restore from that next time. - if (!metadata.writtenToDisc) { - if ((metadata.workspaceFolder || metadata.worktreeProperties || metadata.additionalWorkspaces?.length)) { - this.updateSessionMetadata(sessionId, metadata, false).catch(ex => { - this.logService.error(ex, `[ChatSessionMetadataStore] Failed to write metadata for session ${sessionId} to session state: `); - }); - } - } - } + // this.extensionContext.globalState.update(WORKTREE_MEMENTO_KEY, undefined); + // this.extensionContext.globalState.update(WORKSPACE_FOLDER_MEMENTO_KEY, undefined); - if (cacheUpdated) { - // Writing to file is most important. - await this.writeToGlobalStorage(this._cache); - } - // To be enabled after testing. So we dont' blow away the data. - // this.extensionContext.globalState.update(WORKSPACE_FOLDER_MEMENTO_KEY, undefined); - // this.extensionContext.globalState.update(WORKTREE_MEMENTO_KEY, undefined); + // Ensure every cached session with a worktreePath has a JSONL + // entry. Only appends entries that are missing; falls back to a full rewrite when + // the load detected duplicates or malformed lines. + await this.topUpJsonlIndexFromCache(); } public getMetadataFileUri(sessionId: string): vscode.Uri { @@ -150,16 +134,19 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession } async deleteSessionMetadata(sessionId: string): Promise { - await this._intialize.value; + await this._ready; if (sessionId in this._cache) { delete this._cache[sessionId]; - const data = await this.getGlobalStorageData(); + const data = await this.getGlobalStorageData().catch(() => ({} as Record)); delete data[sessionId]; await this.writeToGlobalStorage(data); } try { - await this.fileSystemService.delete(this.getMetadataFileUri(sessionId)); - await this.fileSystemService.delete(this.getRequestMappingFileUri(sessionId)); + await Promise.allSettled([ + this._worktreeSessions.removeAndWriteToDisk(sessionId), + this.fileSystemService.delete(this.getMetadataFileUri(sessionId)), + this.fileSystemService.delete(this.getRequestMappingFileUri(sessionId)) + ]); } catch { // File may not exist, ignore. } @@ -169,11 +156,14 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession if (isUntitledSessionId(sessionId)) { return; } - await this._intialize.value; + await this._ready; + // Optimistically update in-memory cache so callers in the same process observe + // the change immediately. We pass only the partial `fields` to + // `updateSessionMetadata` — that method reads fresh from disk and merges, so it + // cannot stomp fields written by other processes (Step 3b: stale-cache fix). const existing = this._cache[sessionId] ?? {}; - const metadata: ChatSessionMetadataFile = { ...existing, ...fields }; - this._cache[sessionId] = metadata; - await this.updateSessionMetadata(sessionId, metadata); + this._cache[sessionId] = { ...existing, ...fields }; + await this.updateSessionMetadata(sessionId, fields); this.updateGlobalStorage(); } @@ -197,30 +187,44 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession getWorktreeProperties(sessionId: string): Promise; getWorktreeProperties(folder: Uri): Promise; async getWorktreeProperties(sessionId: string | Uri): Promise { - await this._intialize.value; + await this._ready; if (typeof sessionId === 'string') { const metadata = await this.getSessionMetadata(sessionId); return metadata?.worktreeProperties; - } else { - const folder = sessionId; - for (const metadata of Object.values(this._cache)) { - if (!metadata.worktreeProperties?.worktreePath) { - continue; - } - if (isEqual(Uri.file(metadata.worktreeProperties.worktreePath), folder)) { - return metadata.worktreeProperties; - } + } + const folder = sessionId; + // First check the in-memory cache. + for (const metadata of Object.values(this._cache)) { + if (metadata.worktreeProperties?.worktreePath && isEqual(Uri.file(metadata.worktreeProperties.worktreePath), folder)) { + return metadata.worktreeProperties; } } + // Fallback to the JSONL worktree index → hydrate from the per-session file. + const id = await this.findSessionIdForWorktree(folder); + if (id) { + const metadata = await this.getSessionMetadata(id); + return metadata?.worktreeProperties; + } + return undefined; } async getSessionIdForWorktree(folder: vscode.Uri): Promise { - await this._intialize.value; + await this._ready; for (const [sessionId, value] of Object.entries(this._cache)) { if (value.worktreeProperties?.worktreePath && isEqual(vscode.Uri.file(value.worktreeProperties.worktreePath), folder)) { return sessionId; } } - return undefined; + return this.findSessionIdForWorktree(folder); + } + + /** Looks up a session id for a worktree folder via the JSONL index, with a throttled disk reload. */ + private async findSessionIdForWorktree(folder: vscode.Uri): Promise { + const cached = this._worktreeSessions.getSessionIdForFolder(folder); + if (cached) { + return cached; + } + await this._worktreeSessions.reloadIfStale(); + return this._worktreeSessions.getSessionIdForFolder(folder); } async getSessionWorkspaceFolder(sessionId: string): Promise { @@ -284,7 +288,7 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession } async getRequestDetails(sessionId: string): Promise { - await this._intialize.value; + await this._ready; const fileUri = this.getRequestMappingFileUri(sessionId); try { const content = await this.fileSystemService.readFile(fileUri); @@ -295,7 +299,7 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession } async updateRequestDetails(sessionId: string, details: (Partial & { vscodeRequestId: string })[]): Promise { - await this._intialize.value; + await this._ready; if (isUntitledSessionId(sessionId)) { return; } @@ -324,7 +328,7 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession } private async writeRequestDetails(sessionId: string, details: RequestDetails[]): Promise { - await this._intialize.value; + await this._ready; if (isUntitledSessionId(sessionId)) { return; } @@ -337,7 +341,7 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession } async storeForkedSessionMetadata(sourceSessionId: string, targetSessionId: string, customTitle: string): Promise { - await this._intialize.value; + await this._ready; const sourceMetadata = await this.getSessionMetadata(sourceSessionId); const forkedMetadata: ChatSessionMetadataFile = { ...sourceMetadata, @@ -351,7 +355,7 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession } public async setSessionOrigin(sessionId: string): Promise { - await this._intialize.value; + await this._ready; await this.updateMetadataFields(sessionId, { origin: 'vscode' }); } @@ -372,7 +376,7 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession } public async setSessionParentId(sessionId: string, parentSessionId: string): Promise { - await this._intialize.value; + await this._ready; await this.updateMetadataFields(sessionId, { parentSessionId, kind: 'sub-session' }); } @@ -385,29 +389,38 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession if (isUntitledSessionId(sessionId)) { return undefined; } - await this._intialize.value; + await this._ready; if (sessionId in this._cache) { return this._cache[sessionId]; } - const fileUri = this.getMetadataFileUri(sessionId); - try { - const content = await this.fileSystemService.readFile(fileUri); - const metadata: ChatSessionMetadataFile = JSON.parse(new TextDecoder().decode(content)); + const metadata = await this.readSessionMetadataFile(sessionId); + if (metadata) { this._cache[sessionId] = metadata; return metadata; + } + + // So we don't try again. + this._cache[sessionId] = {}; + if (createMetadataFileIfNotFound) { + await this.updateSessionMetadata(sessionId, { origin: 'other' }); + this.updateGlobalStorage(); + } + return undefined; + } + + /** Reads a per-session metadata file directly. Returns `undefined` if it doesn't exist or is invalid. */ + private async readSessionMetadataFile(sessionId: string): Promise { + try { + const fileUri = this.getMetadataFileUri(sessionId); + const content = await this.fileSystemService.readFile(fileUri); + return JSON.parse(new TextDecoder().decode(content)) as ChatSessionMetadataFile; } catch { - // So we don't try again. - this._cache[sessionId] = {}; - if (createMetadataFileIfNotFound) { - await this.updateSessionMetadata(sessionId, { origin: 'other' }); - this.updateGlobalStorage(); - } return undefined; } } - private async updateSessionMetadata(sessionId: string, metadata: ChatSessionMetadataFile, createDirectoryIfNotFound = true): Promise { + private async updateSessionMetadata(sessionId: string, updates: Partial, createDirectoryIfNotFound = true): Promise { if (isUntitledSessionId(sessionId)) { // Don't write metadata for untitled sessions, as they are temporary and can be created in large numbers. return; @@ -420,17 +433,19 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession // Try to read existing file first (will succeed 99% of the time). // This preserves data written by other processes when merging. let existing: ChatSessionMetadataFile = {}; + let diskFileExisted = true; try { const rawContent = await this.fileSystemService.readFile(fileUri); existing = JSON.parse(new TextDecoder().decode(rawContent)); } catch { + diskFileExisted = false; // File doesn't exist yet — check if the directory exists. try { await this.fileSystemService.stat(dirUri); } catch { if (!createDirectoryIfNotFound) { // Lets not delete the session from our storage, but mark it as written to session state so that we won't try to write to session state again and again. - this._cache[sessionId] = { ...metadata, writtenToDisc: true }; + this._cache[sessionId] = { ...updates, writtenToDisc: true }; this.updateGlobalStorage(); return; } @@ -438,10 +453,13 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession } } - // Merge: overwrite fields that are explicitly provided, delete fields set to undefined. - // This preserves data written by other processes. - const merged: ChatSessionMetadataFile = { ...existing }; - for (const [key, value] of Object.entries(metadata)) { + // Merge order: cache (locally-known fields not yet flushed to disk) + // → disk existing (cross-process writes win over stale cache, Step 3b) + // → explicit `metadata` fields from this call (caller wins). + // `undefined` values in `metadata` delete the corresponding key. + const cacheExisting = diskFileExisted ? {} : (this._cache[sessionId] ?? {}); + const merged: ChatSessionMetadataFile = { ...cacheExisting, ...existing }; + for (const [key, value] of Object.entries(updates)) { if (value === undefined) { delete (merged as Record)[key]; } else { @@ -449,8 +467,38 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession } } + // Stamp timestamps. `created` is set only on first write; `modified` is + // bumped on every write. + const now = Date.now(); + merged.modified = now; + if (merged.created === undefined) { + merged.created = now; + } + + const promises: Promise[] = []; + + // Maintain the JSONL worktree index based on the post-merge worktreePath: + // - new entry → append a line and remember it + // - changed path → rewrite the file (rare) + // - cleared path → remove via rewrite + const worktreePath = merged.worktreeProperties?.worktreePath; + const indexed = this._worktreeSessions.getSessionEntry(sessionId); + if (worktreePath) { + if (!indexed) { + promises.push(this._worktreeSessions.appendBatchToDisk([{ id: sessionId, path: worktreePath, created: merged.created }])); + } else if (indexed.path !== worktreePath && !merged.kind) { + this._worktreeSessions.addEntry({ id: sessionId, path: worktreePath, created: indexed.created }); + promises.push(this._worktreeSessions.writeToDisk()); + } + } else if (indexed) { + promises.push(this._worktreeSessions.removeAndWriteToDisk(sessionId)); + } + const content = new TextEncoder().encode(JSON.stringify(merged, null, 2)); - await this.fileSystemService.writeFile(fileUri, content); + promises.push(this.fileSystemService.writeFile(fileUri, content)); + + await Promise.all(promises); + this._cache[sessionId] = { ...merged, writtenToDisc: true }; this.updateGlobalStorage(); this.logService.trace(`[ChatSessionMetadataStore] Wrote metadata for session ${sessionId}`); @@ -468,34 +516,258 @@ export class ChatSessionMetadataStore extends Disposable implements IChatSession private async updateGlobalStorageImpl() { try { - const data = this._cache; - try { - const storageData = await this.getGlobalStorageData(); - for (const [sessionId, metadata] of Object.entries(storageData)) { - if (sessionId in data) { - // Ignore this. - } else { - data[sessionId] = metadata; + // Serialize against `refresh()` and other bulk-file flushes via the shared + // single-key sequencer. Inside the queue we re-read the on-disk file and + // merge it with the in-memory cache using last-modified-wins semantics so + // concurrent writers in another process do not lose data. + await this._bulkSequencer.queue(BULK_SEQUENCER_KEY, async () => { + const data: Record = { ...this._cache }; + try { + const storageData = await this.getGlobalStorageData(); + for (const [sessionId, diskEntry] of Object.entries(storageData)) { + const local = data[sessionId]; + if (!local) { + data[sessionId] = diskEntry; + this._cache[sessionId] = diskEntry; + continue; + } + const localModified = local.modified ?? 0; + const diskModified = diskEntry.modified ?? 0; + if (diskModified > localModified) { + data[sessionId] = diskEntry; + this._cache[sessionId] = diskEntry; + } } + } catch { + // } - } catch { - // - } - await this.writeToGlobalStorage(data); + await this.writeToGlobalStorage(data); + }); } catch (error) { this.logService.error('[ChatSessionMetadataStore] Failed to update global storage: ', error); } } private async writeToGlobalStorage(allMetadata: Record): Promise { + // Make a shallow copy and trim to the top MAX_BULK_STORAGE_ENTRIES by `modified` desc. + // The in-memory `_cache` is unaffected — only the on-disk file is bounded. + // Per-session files in `~/.copilot/session-state/{id}/vscode.metadata.json` remain + // the source of truth for evicted entries. + const entries = Object.entries(allMetadata); + let toWrite: Record; + if (entries.length <= MAX_BULK_STORAGE_ENTRIES) { + toWrite = { ...allMetadata }; + } else { + entries.sort(([, a], [, b]) => (b.modified ?? 0) - (a.modified ?? 0)); + toWrite = Object.fromEntries(entries.slice(0, MAX_BULK_STORAGE_ENTRIES)); + } + + const dirUri = dirname(this._cacheFile); try { - await this.fileSystemService.stat(this._cacheDirectory); + await this.fileSystemService.stat(dirUri); } catch { - await this.fileSystemService.createDirectory(this._cacheDirectory); + await this.fileSystemService.createDirectory(dirUri); } - const content = new TextEncoder().encode(JSON.stringify(allMetadata, null, 2)); + const content = new TextEncoder().encode(JSON.stringify(toWrite, null, 2)); await this.fileSystemService.writeFile(this._cacheFile, content); - this.logService.trace(`[ChatSessionMetadataStore] Wrote bulk metadata file with ${Object.keys(allMetadata).length} session(s)`); + this.logService.trace(`[ChatSessionMetadataStore] Wrote bulk metadata file with ${Object.keys(toWrite).length} session(s)`); + } + + /** + * Re-reads the shared bulk file from disk and merges into `_cache` using + * last-modified-wins. Runs inside the bulk sequencer so it is serialized + * against {@link updateGlobalStorageImpl}. Never drops in-memory entries. + */ + private async reloadBulkFromDisk(): Promise { + return this._bulkSequencer.queue(BULK_SEQUENCER_KEY, async () => { + let onDisk: Record; + try { + onDisk = await this.getGlobalStorageData(); + } catch { + return; + } + for (const [id, diskEntry] of Object.entries(onDisk)) { + const local = this._cache[id]; + if (!local) { + this._cache[id] = diskEntry; + continue; + } + const localModified = local.modified ?? 0; + const diskModified = diskEntry.modified ?? 0; + if (diskModified > localModified) { + this._cache[id] = diskEntry; + } + } + }); + } + + /** + * Merges the per-install legacy bulk file (`globalStorageUri/copilotcli/…`) into + * the shared `~/.copilot/` bulk file using last-modified-wins. This handles: + * - First-run: shared file missing → copy legacy content into the shared file. + * - Late-joiner: Process A already created the shared file → merge so entries + * unique to this install are not lost. + * - No legacy file: nothing to do. + */ + private async migrateLegacyBulkFile(): Promise { + // Skip if this install already migrated. + if (this.extensionContext.globalState.get(LEGACY_BULK_MIGRATED_KEY)) { + return; + } + + const legacyCacheFile = Uri.joinPath(this.extensionContext.globalStorageUri, 'copilotcli', LEGACY_BULK_METADATA_FILENAME); + let legacyData: Record; + try { + const raw = await this.fileSystemService.readFile(legacyCacheFile); + legacyData = JSON.parse(new TextDecoder().decode(raw)); + } catch { + // No legacy file — mark as migrated so we don't retry. + await this.extensionContext.globalState.update(LEGACY_BULK_MIGRATED_KEY, true); + return; + } + + try { + await createDirectoryIfNotExists(this.fileSystemService, dirname(this._cacheFile)); + + // Try to read the shared file (may or may not exist yet). + let sharedData: Record = {}; + try { + const raw = await this.fileSystemService.readFile(this._cacheFile); + sharedData = JSON.parse(new TextDecoder().decode(raw)); + } catch { + // Shared file doesn't exist yet — start empty. + } + + // Merge legacy into shared using last-modified-wins. + let merged = false; + for (const [id, legacyEntry] of Object.entries(legacyData)) { + const sharedEntry = sharedData[id]; + if (!sharedEntry) { + sharedData[id] = legacyEntry; + merged = true; + } else { + const sharedModified = sharedEntry.modified ?? 0; + const legacyModified = legacyEntry.modified ?? 0; + if (legacyModified > sharedModified) { + sharedData[id] = legacyEntry; + merged = true; + } + } + } + + if (merged) { + const content = new TextEncoder().encode(JSON.stringify(sharedData, null, 2)); + await this.fileSystemService.writeFile(this._cacheFile, content); + } + + // Mark as migrated so subsequent startups skip this path. + await this.extensionContext.globalState.update(LEGACY_BULK_MIGRATED_KEY, true); + this.logService.info('[ChatSessionMetadataStore] Migrated legacy bulk metadata file to ~/.copilot/'); + } catch (err) { + this.logService.error('[ChatSessionMetadataStore] Failed to migrate legacy bulk file: ', err); + } + } + + /** + * For every cached session with a `worktreePath`, ensure a JSONL entry exists. + */ + private async topUpJsonlIndexFromCache(): Promise { + // Load the JSONL worktree index from disk first so the scan below can + // tell which entries already exist and avoid re-appending duplicates. + let { rewriteNeeded } = await this._worktreeSessions.loadFromDisk(); + + const toAppend: WorktreeSessionEntry[] = []; + for (const [id, metadata] of Object.entries(this._cache)) { + const path = metadata.worktreeProperties?.worktreePath; + if (!path || metadata.kind) { + continue; + } + const existing = this._worktreeSessions.getSessionEntry(id); + if (existing && existing.path === path) { + continue; + } + const entry: WorktreeSessionEntry = { id, path, created: existing?.created ?? metadata.created ?? Date.now() }; + this._worktreeSessions.addEntry(entry); + if (existing) { + // Path changed — a full rewrite is needed. + rewriteNeeded = true; + } else { + toAppend.push(entry); + } + } + + if (rewriteNeeded) { + await this._worktreeSessions.writeToDisk(); + } else if (toAppend.length > 0) { + await this._worktreeSessions.appendBatchToDisk(toAppend); + } + + // One-time full scan of ~/.copilot/session-state/ to discover worktree + // sessions that were never recorded in the JSONL (e.g. sessions created + // before the JSONL index existed, or evicted from the bulk cache). + await this.scanSessionStateDirForWorktrees(); + } + + /** + * One-time scan of `~/.copilot/session-state/` to discover worktree sessions + * not yet in the JSONL index. Reads per-session metadata files in batches of + * {@link SESSION_SCAN_BATCH_SIZE} to avoid saturating I/O. Gated by a memento + * so it only runs once per install. + */ + private async scanSessionStateDirForWorktrees(): Promise { + if (this.extensionContext.globalState.get(JSONL_SCAN_DONE_KEY)) { + return; + } + + const sessionStateDir = Uri.file(getCopilotCLISessionStateDir()); + let entries: [string, number][]; + try { + entries = await this.fileSystemService.readDirectory(sessionStateDir); + } catch { + // Directory doesn't exist — nothing to scan. + await this.extensionContext.globalState.update(JSONL_SCAN_DONE_KEY, true); + return; + } + + // Collect session IDs we don't already know about. + const unknownIds: string[] = []; + for (const [name] of entries) { + if (name in this._cache || this._worktreeSessions.has(name)) { + continue; + } + unknownIds.push(name); + } + + if (unknownIds.length === 0) { + await this.extensionContext.globalState.update(JSONL_SCAN_DONE_KEY, true); + return; + } + + // Read metadata files in batches. + let discovered = false; + for (let i = 0; i < unknownIds.length; i += SESSION_SCAN_BATCH_SIZE) { + const batch = unknownIds.slice(i, i + SESSION_SCAN_BATCH_SIZE); + const results = await Promise.all(batch.map(async id => { + const metadata = await this.readSessionMetadataFile(id); + return { id, metadata }; + })); + for (const { id, metadata } of results) { + if (!metadata?.worktreeProperties?.worktreePath || metadata.kind) { + continue; + } + const path = metadata.worktreeProperties.worktreePath; + if (!this._worktreeSessions.has(id)) { + this._worktreeSessions.addEntry({ id, path, created: metadata.created ?? Date.now() }); + discovered = true; + } + } + } + + if (discovered) { + await this._worktreeSessions.writeToDisk(); + } + await this.extensionContext.globalState.update(JSONL_SCAN_DONE_KEY, true); + this.logService.info(`[ChatSessionMetadataStore] Session-state scan complete: checked ${unknownIds.length} unknown session(s)`); } } diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/chatSessionWorktreeServiceImpl.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/chatSessionWorktreeServiceImpl.ts index 0a23e98bea82b..09a2aee2f371b 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode-node/chatSessionWorktreeServiceImpl.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/chatSessionWorktreeServiceImpl.ts @@ -22,9 +22,9 @@ import { isEqual } from '../../../util/vs/base/common/resources'; import { generateUuid } from '../../../util/vs/base/common/uuid'; import { IAgentSessionsWorkspace } from '../common/agentSessionsWorkspace'; import { IChatSessionMetadataStore } from '../common/chatSessionMetadataStore'; -import { ChatSessionWorktreeData, ChatSessionWorktreeFile, ChatSessionWorktreeProperties, ChatSessionWorktreePropertiesV2, IChatSessionWorktreeService } from '../common/chatSessionWorktreeService'; +import { ChatSessionWorktreeFile, ChatSessionWorktreeProperties, ChatSessionWorktreePropertiesV2, IChatSessionWorktreeService } from '../common/chatSessionWorktreeService'; -const CHAT_SESSION_WORKTREE_MEMENTO_KEY = 'github.copilot.cli.sessionWorktrees'; +// const CHAT_SESSION_WORKTREE_MEMENTO_KEY = 'github.copilot.cli.sessionWorktrees'; export class ChatSessionWorktreeService extends Disposable implements IChatSessionWorktreeService { declare _serviceBrand: undefined; @@ -42,6 +42,8 @@ export class ChatSessionWorktreeService extends Disposable implements IChatSessi @IChatSessionMetadataStore private readonly metadataStore: IChatSessionMetadataStore, ) { super(); + // This is not used. + // void this.extensionContext.globalState.update(CHAT_SESSION_WORKTREE_MEMENTO_KEY, undefined); } async createWorktree(repositoryPath: vscode.Uri, stream?: vscode.ChatResponseStream, baseBranch?: string, branchName?: string): Promise { @@ -202,11 +204,7 @@ export class ChatSessionWorktreeService extends Disposable implements IChatSessi async setWorktreeProperties(sessionId: string, properties: ChatSessionWorktreeProperties): Promise { this._sessionWorktrees.set(sessionId, properties); - - const sessionWorktreesProperties = this.extensionContext.globalState.get>(CHAT_SESSION_WORKTREE_MEMENTO_KEY, {}); - sessionWorktreesProperties[sessionId] = { data: JSON.stringify(properties), version: properties.version }; await this.metadataStore.storeWorktreeInfo(sessionId, properties); - await this.extensionContext.globalState.update(CHAT_SESSION_WORKTREE_MEMENTO_KEY, sessionWorktreesProperties); } async getWorktreeRepository(sessionId: string): Promise { diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessions.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessions.ts index 33b1d3fc1f81e..27a48a7ab4a12 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessions.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessions.ts @@ -155,6 +155,7 @@ export class CopilotCLIChatSessionContentProvider extends Disposable implements } isRefreshing = true; const stopwatch = new StopWatch(); + void this._metadataStore.refresh().catch(error => this.logService.error(error, 'Failed to refresh session metadata store during session list refresh')); try { const sessions = await this.sessionService.getAllSessions(CancellationToken.None); const items = await Promise.all(sessions.map(async session => this.toChatSessionItem(session))); diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts index 8f15c18d065bb..b9f2a2ae63ce6 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessionsContribution.ts @@ -212,10 +212,15 @@ export class CopilotCLIChatSessionItemProvider extends Disposable implements vsc } public notifySessionsChange(): void { + // Refresh the bulk metadata cache from disk so cross-process writes + // (e.g. another VS Code window editing the same session) become visible + // before consumers re-read items. + this.chatSessionMetadataStore.refresh().catch(() => { /* logged inside */ }); this._onDidChangeChatSessionItems.fire(); } public async refreshSession(refreshOptions: { reason: 'update'; sessionId: string } | { reason: 'update'; sessionIds: string[] } | { reason: 'delete'; sessionId: string }): Promise { + await this.chatSessionMetadataStore.refresh().catch(() => { /* logged inside */ }); this._onDidChangeChatSessionItems.fire(); } diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/test/chatSessionMetadataStoreImpl.spec.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/test/chatSessionMetadataStoreImpl.spec.ts index 40a7bd4b9ed61..4df78020aeec5 100644 --- a/extensions/copilot/src/extension/chatSessions/vscode-node/test/chatSessionMetadataStoreImpl.spec.ts +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/test/chatSessionMetadataStoreImpl.spec.ts @@ -12,23 +12,30 @@ import { ILogService } from '../../../../platform/log/common/logService'; import { mock } from '../../../../util/common/test/simpleMock'; import { Emitter } from '../../../../util/vs/base/common/event'; import { URI } from '../../../../util/vs/base/common/uri'; -import { eventToPromise } from '../../../completions-core/vscode-node/lib/src/prompt/asyncUtils'; -import { ChatSessionWorktreeData, ChatSessionWorktreeProperties } from '../../common/chatSessionWorktreeService'; +import { ChatSessionWorktreeProperties } from '../../common/chatSessionWorktreeService'; import { IWorkspaceInfo } from '../../common/workspaceInfo'; import { getCopilotCLISessionDir } from '../../copilotcli/node/cliHelpers'; import { NullCopilotCLIAgents } from '../../copilotcli/node/test/testHelpers'; import { ChatSessionMetadataStore } from '../chatSessionMetadataStoreImpl'; +// Hoisted holder lets each test point the JSONL helper at its own mock path. +const jsonlPathHolder = vi.hoisted(() => { + const p = '/mock/copilot-home/worktree.jsonl'; + return { get: () => p }; +}); + vi.mock('../../copilotcli/node/cliHelpers', async (importOriginal) => { const actual = await importOriginal(); return { ...actual, getCopilotCLISessionDir: (sessionId: string) => `/mock/session-state/${sessionId}`, + getCopilotCLISessionStateDir: () => '/mock/session-state', + // New shared bulk + JSONL paths — all go through the mocked IFileSystemService. + getCopilotBulkMetadataFile: () => '/mock/copilot-home/vscode.session.metadata.cache.json', + getCopilotWorktreeSessionsFile: () => jsonlPathHolder.get(), }; }); -const WORKSPACE_FOLDER_MEMENTO_KEY = 'github.copilot.cli.sessionWorkspaceFolders'; -const WORKTREE_MEMENTO_KEY = 'github.copilot.cli.sessionWorktrees'; class MockGlobalState implements vscode.Memento { private data = new Map(); @@ -82,8 +89,12 @@ class MockLogService extends mock() { } // Paths used by the store -const GLOBAL_STORAGE_DIR = Uri.joinPath(Uri.file('/mock/global/storage'), 'copilotcli'); -const BULK_METADATA_FILE = Uri.joinPath(GLOBAL_STORAGE_DIR, 'copilotcli.session.metadata.json'); +// New shared bulk file (top-N cache). Lives at `~/.copilot/...` in production but is +// mocked above so the IFileSystemService mock can intercept reads/writes. +const BULK_METADATA_FILE = Uri.file('/mock/copilot-home/vscode.session.metadata.cache.json'); +// Legacy bulk file location in the per-install globalStorageUri — used only by the +// one-time migration in `initializeStorage()`. +const LEGACY_BULK_METADATA_FILE = Uri.joinPath(Uri.file('/mock/global/storage'), 'copilotcli', 'copilotcli.session.metadata.json'); function sessionDirectoryUri(sessionId: string): Uri { return Uri.file(getCopilotCLISessionDir(sessionId)); @@ -121,9 +132,6 @@ function makeWorktreeV2Props(overrides?: Partial) } as ChatSessionWorktreeProperties; } -function makeWorktreeData(props: ChatSessionWorktreeProperties): ChatSessionWorktreeData { - return { data: JSON.stringify(props), version: props.version }; -} class MockFileSystemServiceWithMotification extends MockFileSystemService { onDidCreateFile = new Emitter(); @@ -145,14 +153,14 @@ describe('ChatSessionMetadataStore', () => { let logService: MockLogService; let extensionContext: MockExtensionContext; - beforeEach(() => { + beforeEach(async () => { vi.useFakeTimers(); mockFs = new MockFileSystemServiceWithMotification(); logService = new MockLogService(); extensionContext = new MockExtensionContext(); }); - afterEach(() => { + afterEach(async () => { mockFs.dispose(); vi.useRealTimers(); vi.restoreAllMocks(); @@ -160,7 +168,7 @@ describe('ChatSessionMetadataStore', () => { /** * Creates the store and waits for initialization to complete. - * Constructor eagerly triggers lazy init; we flush microtasks so it settles. + * Constructor eagerly triggers init; we flush microtasks so it settles. */ async function createStore(): Promise { const store = new ChatSessionMetadataStore( @@ -169,8 +177,11 @@ describe('ChatSessionMetadataStore', () => { extensionContext, new NullCopilotCLIAgents(), ); - // Flush microtasks to let initialization settle - await vi.advanceTimersByTimeAsync(0); + // Flush enough microtask rounds so that initializeStorage() — + // which chains several async I/O steps — fully settles. + for (let i = 0; i < 5; i++) { + await vi.advanceTimersByTimeAsync(0); + } return store; } @@ -194,158 +205,6 @@ describe('ChatSessionMetadataStore', () => { store.dispose(); }); - it('should also read global state memento keys to pick up missing entries when bulk file exists', async () => { - // Global state has data not yet in the bulk file — it should be merged in - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-x': { folderPath: Uri.file('/from/global/state').fsPath, timestamp: 999 }, - }); - mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({})); - - const getSpy = vi.spyOn(extensionContext.globalState, 'get'); - const store = await createStore(); - - // globalState.get SHOULD now be called for the memento keys - const mementoCalls = getSpy.mock.calls.filter( - c => c[0] === WORKSPACE_FOLDER_MEMENTO_KEY || c[0] === WORKTREE_MEMENTO_KEY, - ); - expect(mementoCalls.length).toBeGreaterThan(0); - - // session-x should now be accessible since it was merged from global state - const folder = await store.getSessionWorkspaceFolder('session-x'); - expect(folder?.fsPath).toBe(Uri.file('/from/global/state').fsPath); - store.dispose(); - }); - - it('should not overwrite entries already in bulk file from global state', async () => { - // Bulk file already has session-x with one path; global state has a different path - const existingData = { - 'session-x': { workspaceFolder: { folderPath: Uri.file('/from/bulk').fsPath, timestamp: 100 } }, - }; - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-x': { folderPath: Uri.file('/from/global/state').fsPath, timestamp: 999 }, - }); - mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify(existingData)); - - const store = await createStore(); - - // Should keep the bulk file version, not the global state version - const folder = await store.getSessionWorkspaceFolder('session-x'); - expect(folder?.fsPath).toBe(Uri.file('/from/bulk').fsPath); - store.dispose(); - }); - - it('should attempt to write per-session files for entries not yet writtenToDisc', async () => { - const existingData = { - 'session-1': { workspaceFolder: { folderPath: Uri.file('/workspace/a').fsPath, timestamp: 100 } }, - }; - const fileUri = sessionMetadataFileUri('session-1'); - mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify(existingData)); - mockFs.mockDirectory(fileUri, []); - - // Pre-create the session directory so the write succeeds - await mockFs.createDirectory(sessionDirectoryUri('session-1')); - const fileCreated = eventToPromise(mockFs.onDidCreateFile.event); - - const store = await createStore(); - - // wait for file to get created. - await fileCreated; - const rawContent = await mockFs.readFile(fileUri); - const written = JSON.parse(new TextDecoder().decode(rawContent)); - expect(written.workspaceFolder?.folderPath).toBe(Uri.file('/workspace/a').fsPath); - store.dispose(); - }); - - it('should not attempt to write per-session files for entries already marked writtenToDisc', async () => { - const existingData = { - 'session-1': { workspaceFolder: { folderPath: Uri.file('/workspace/a').fsPath, timestamp: 100 }, writtenToDisc: true }, - }; - mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify(existingData)); - - const writeSpy = vi.spyOn(mockFs, 'writeFile'); - const store = await createStore(); - await vi.advanceTimersByTimeAsync(0); - - // No writes to per-session files should have occurred - const perSessionWrites = writeSpy.mock.calls.filter( - c => c[0].toString().includes('vscode.metadata.json'), - ); - expect(perSessionWrites).toHaveLength(0); - store.dispose(); - }); - - it('should skip per-session file write gracefully when directory does not exist during retry', async () => { - const existingData = { - 'session-1': { workspaceFolder: { folderPath: Uri.file('/workspace/a').fsPath, timestamp: 100 } }, - }; - mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify(existingData)); - // Do NOT create the session directory - - const createDirSpy = vi.spyOn(mockFs, 'createDirectory'); - const store = await createStore(); - await vi.advanceTimersByTimeAsync(0); - - // createDirectory should NOT have been called for the per-session dir - // because createDirectoryIfNotFound=false during retry - const perSessionDirCalls = createDirSpy.mock.calls.filter( - c => c[0].toString().includes('session-1'), - ); - expect(perSessionDirCalls).toHaveLength(0); - - // Entry should still be accessible from cache - const folder = await store.getSessionWorkspaceFolder('session-1'); - expect(folder?.fsPath).toBe(Uri.file('/workspace/a').fsPath); - store.dispose(); - }); - - it('should keep worktree cache entry unchanged when global state has same number of changes', async () => { - const cachedProps = makeWorktreeV1Props({ - changes: [{ filePath: '/a.ts', originalFilePath: '/a.ts', modifiedFilePath: '/a.ts', statistics: { additions: 1, deletions: 0 } }], - }); - const globalStateProps = makeWorktreeV1Props({ - branchName: 'from-global-state', - changes: [{ filePath: '/b.ts', originalFilePath: '/b.ts', modifiedFilePath: '/b.ts', statistics: { additions: 2, deletions: 1 } }], - }); - mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({ - 'session-wt': { worktreeProperties: cachedProps }, - })); - extensionContext.globalState.seed(WORKTREE_MEMENTO_KEY, { - 'session-wt': makeWorktreeData(globalStateProps), - }); - - const store = await createStore(); - - // Cache entry should keep its original data (not replaced by global state) - const wt = await store.getWorktreeProperties('session-wt'); - expect(wt?.branchName).toBe(cachedProps.branchName); - store.dispose(); - }); - - it('should update worktree cache entry when global state has more changes', async () => { - const cachedProps = makeWorktreeV1Props({ changes: undefined }); - const globalStateProps = makeWorktreeV1Props({ - branchName: 'from-global-state', - changes: [ - { filePath: '/a.ts', originalFilePath: '/a.ts', modifiedFilePath: '/a.ts', statistics: { additions: 1, deletions: 0 } }, - { filePath: '/b.ts', originalFilePath: '/b.ts', modifiedFilePath: '/b.ts', statistics: { additions: 2, deletions: 1 } }, - ], - }); - mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({ - 'session-wt': { worktreeProperties: cachedProps }, - })); - extensionContext.globalState.seed(WORKTREE_MEMENTO_KEY, { - 'session-wt': makeWorktreeData(globalStateProps), - }); - - const store = await createStore(); - - // Even when global state has more changes, cache entry is preserved (both paths continue) - const wt = await store.getWorktreeProperties('session-wt'); - expect(wt?.branchName).toBe(globalStateProps.branchName); - expect(wt?.changes).toEqual(globalStateProps.changes); - store.dispose(); - }); - it('should not retry entries with no workspaceFolder, worktreeProperties, or additionalWorkspaces', async () => { const existingData = { 'session-empty': {}, @@ -366,378 +225,6 @@ describe('ChatSessionMetadataStore', () => { store.dispose(); }); - it('should retry entries that only have additionalWorkspaces (not delete as invalid data)', async () => { - // A session with only additionalWorkspaces and writtenToDisc: false - // must be retried, not deleted from cache — otherwise data is lost after a crash. - const existingData = { - 'session-only-additional': { - additionalWorkspaces: [ - { workspaceFolder: { folderPath: Uri.file('/extra/workspace').fsPath, timestamp: 100 } }, - ], - }, - }; - mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify(existingData)); - - // Pre-create the session directory so the recovery write can succeed - await mockFs.createDirectory(sessionDirectoryUri('session-only-additional')); - const fileCreated = eventToPromise(mockFs.onDidCreateFile.event); - - const store = await createStore(); - await fileCreated; - - const fileUri = sessionMetadataFileUri('session-only-additional'); - const rawContent = await mockFs.readFile(fileUri); - const written = JSON.parse(new TextDecoder().decode(rawContent)); - expect(written.additionalWorkspaces).toHaveLength(1); - store.dispose(); - }); - }); - - // ────────────────────────────────────────────────────────────────────────── - // initializeStorage — migration path: bulk file missing, read from global state - // ────────────────────────────────────────────────────────────────────────── - describe('initializeStorage - migration from global state', () => { - it('should migrate workspace folder entries to bulk file', async () => { - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-1': { folderPath: Uri.file('/workspace/a').fsPath, timestamp: 100 }, - 'session-2': { folderPath: Uri.file('/workspace/b').fsPath, timestamp: 200 }, - }); - - const store = await createStore(); - - const folder1 = await store.getSessionWorkspaceFolder('session-1'); - expect(folder1?.fsPath).toBe(Uri.file('/workspace/a').fsPath); - const folder2 = await store.getSessionWorkspaceFolder('session-2'); - expect(folder2?.fsPath).toBe(Uri.file('/workspace/b').fsPath); - store.dispose(); - }); - - it('should migrate worktree entries to bulk file', async () => { - const v1Props = makeWorktreeV1Props(); - extensionContext.globalState.seed(WORKTREE_MEMENTO_KEY, { - 'session-wt': makeWorktreeData(v1Props), - }); - - const store = await createStore(); - - const wt = await store.getWorktreeProperties('session-wt'); - expect(wt).toBeDefined(); - expect(wt!.version).toBe(1); - expect(wt!.branchName).toBe(v1Props.branchName); - store.dispose(); - }); - - it('should parse version 1 worktree data with explicit version override', async () => { - // For version 1, the code spreads JSON.parse(data) and sets version: 1 - const rawProps = { baseCommit: 'c1', branchName: 'b1', repositoryPath: '/r', worktreePath: '/w', autoCommit: false }; - extensionContext.globalState.seed(WORKTREE_MEMENTO_KEY, { - 'session-v1': { data: JSON.stringify(rawProps), version: 1 } satisfies ChatSessionWorktreeData, - }); - - const store = await createStore(); - const wt = await store.getWorktreeProperties('session-v1'); - expect(wt?.version).toBe(1); - expect(wt?.baseCommit).toBe('c1'); - store.dispose(); - }); - - it('should parse version 2 worktree data directly from data string', async () => { - const v2Props = makeWorktreeV2Props(); - extensionContext.globalState.seed(WORKTREE_MEMENTO_KEY, { - 'session-v2': { data: JSON.stringify(v2Props), version: 2 } satisfies ChatSessionWorktreeData, - }); - - const store = await createStore(); - const wt = await store.getWorktreeProperties('session-v2'); - expect(wt?.version).toBe(2); - expect(wt?.version === 2 ? wt.baseBranchName : '').toBe('main'); - store.dispose(); - }); - - it('should give worktree precedence over workspace folder for same session', async () => { - const v1Props = makeWorktreeV1Props(); - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-both': { folderPath: Uri.file('/workspace/shared').fsPath, timestamp: 100 }, - }); - extensionContext.globalState.seed(WORKTREE_MEMENTO_KEY, { - 'session-both': makeWorktreeData(v1Props), - }); - - const store = await createStore(); - - // worktree takes precedence: getSessionWorkspaceFolder returns undefined when worktree exists - const folder = await store.getSessionWorkspaceFolder('session-both'); - expect(folder).toBeUndefined(); - - const wt = await store.getWorktreeProperties('session-both'); - expect(wt).toBeDefined(); - store.dispose(); - }); - - it.skip('should clear global state keys after successful migration', async () => { - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-1': { folderPath: Uri.file('/workspace/a').fsPath, timestamp: 100 }, - }); - extensionContext.globalState.seed(WORKTREE_MEMENTO_KEY, { - 'session-2': makeWorktreeData(makeWorktreeV1Props()), - }); - - const store = await createStore(); - - expect(extensionContext.globalState.has(WORKSPACE_FOLDER_MEMENTO_KEY)).toBe(false); - expect(extensionContext.globalState.has(WORKTREE_MEMENTO_KEY)).toBe(false); - store.dispose(); - }); - - it('should write migrated data to bulk metadata file', async () => { - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-1': { folderPath: Uri.file('/workspace/a').fsPath, timestamp: 100 }, - }); - - const store = await createStore(); - - // Read the bulk file directly to verify it was written - const rawContent = await mockFs.readFile(BULK_METADATA_FILE); - const written = JSON.parse(new TextDecoder().decode(rawContent)); - expect(written['session-1']).toBeDefined(); - expect(written['session-1'].workspaceFolder.folderPath).toBe(Uri.file('/workspace/a').fsPath); - store.dispose(); - }); - - it('should write per-session metadata files during migration when directory exists', async () => { - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-1': { folderPath: Uri.file('/workspace/a').fsPath, timestamp: 100 }, - }); - - // Pre-create the session directory so the write succeeds - // (migration uses createDirectoryIfNotFound=false) - await mockFs.createDirectory(sessionDirectoryUri('session-1')); - - const store = await createStore(); - // Wait for the fire-and-forget per-session writes - await vi.advanceTimersByTimeAsync(0); - - const fileUri = sessionMetadataFileUri('session-1'); - const rawContent = await mockFs.readFile(fileUri); - const written = JSON.parse(new TextDecoder().decode(rawContent)); - expect(written.workspaceFolder?.folderPath).toBe(Uri.file('/workspace/a').fsPath); - store.dispose(); - }); - - it('should skip per-session file write when directory does not exist during migration', async () => { - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-1': { folderPath: Uri.file('/workspace/a').fsPath, timestamp: 100 }, - }); - // Do NOT create the session directory - - const writeSpy = vi.spyOn(mockFs, 'writeFile'); - const store = await createStore(); - await vi.advanceTimersByTimeAsync(0); - - // No per-session file write should occur since dir is missing and createDirectoryIfNotFound=false - const perSessionWrites = writeSpy.mock.calls.filter( - c => c[0].toString().includes('session-1') && c[0].toString().includes('vscode.metadata.json'), - ); - expect(perSessionWrites).toHaveLength(0); - - // Data should still be accessible from cache - const folder = await store.getSessionWorkspaceFolder('session-1'); - expect(folder?.fsPath).toBe(Uri.file('/workspace/a').fsPath); - store.dispose(); - }); - - it('should mark migrated worktree entries with writtenToDisc false and attempt per-session write', async () => { - const v1Props = makeWorktreeV1Props(); - extensionContext.globalState.seed(WORKTREE_MEMENTO_KEY, { - 'session-wt-migrate': makeWorktreeData(v1Props), - }); - - // Pre-create the session directory so the retry write succeeds - await mockFs.createDirectory(sessionDirectoryUri('session-wt-migrate')); - const fileCreated = eventToPromise(mockFs.onDidCreateFile.event); - - const store = await createStore(); - - // Wait for the per-session file write triggered by writtenToDisc: false - await fileCreated; - const fileUri = sessionMetadataFileUri('session-wt-migrate'); - const rawContent = await mockFs.readFile(fileUri); - const written = JSON.parse(new TextDecoder().decode(rawContent)); - expect(written.worktreeProperties?.branchName).toBe(v1Props.branchName); - store.dispose(); - }); - }); - - // ────────────────────────────────────────────────────────────────────────── - // initializeStorage — filtering edge cases - // ────────────────────────────────────────────────────────────────────────── - describe('initializeStorage - filtering', () => { - it('should skip workspace folder entries with untitled- prefix', async () => { - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'untitled-session': { folderPath: Uri.file('/workspace/skip').fsPath, timestamp: 100 }, - 'session-keep': { folderPath: Uri.file('/workspace/keep').fsPath, timestamp: 200 }, - }); - - const store = await createStore(); - - const skipFolder = await store.getSessionWorkspaceFolder('untitled-session'); - expect(skipFolder).toBeUndefined(); - const keepFolder = await store.getSessionWorkspaceFolder('session-keep'); - expect(keepFolder?.fsPath).toBe(Uri.file('/workspace/keep').fsPath); - store.dispose(); - }); - - it('should skip worktree entries with untitled- prefix', async () => { - extensionContext.globalState.seed(WORKTREE_MEMENTO_KEY, { - 'untitled-wt': makeWorktreeData(makeWorktreeV1Props()), - 'session-wt': makeWorktreeData(makeWorktreeV1Props()), - }); - - const store = await createStore(); - - const skipWt = await store.getWorktreeProperties('untitled-wt'); - expect(skipWt).toBeUndefined(); - const keepWt = await store.getWorktreeProperties('session-wt'); - expect(keepWt).toBeDefined(); - store.dispose(); - }); - - it('should skip workspace folder entries that are raw strings (legacy format)', async () => { - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-legacy': Uri.file('/old/string/path').fsPath, - 'session-good': { folderPath: Uri.file('/workspace/good').fsPath, timestamp: 100 }, - }); - - const store = await createStore(); - - const legacy = await store.getSessionWorkspaceFolder('session-legacy'); - expect(legacy).toBeUndefined(); - const good = await store.getSessionWorkspaceFolder('session-good'); - expect(good?.fsPath).toBe(Uri.file('/workspace/good').fsPath); - store.dispose(); - }); - - it('should skip workspace folder entries missing folderPath', async () => { - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-no-path': { timestamp: 100 }, - }); - - const store = await createStore(); - - const folder = await store.getSessionWorkspaceFolder('session-no-path'); - expect(folder).toBeUndefined(); - store.dispose(); - }); - - it('should skip workspace folder entries missing timestamp', async () => { - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-no-ts': { folderPath: Uri.file('/workspace/no-ts').fsPath }, - }); - - const store = await createStore(); - - const folder = await store.getSessionWorkspaceFolder('session-no-ts'); - expect(folder).toBeUndefined(); - store.dispose(); - }); - - it('should skip worktree entries that are raw strings (legacy format)', async () => { - extensionContext.globalState.seed(WORKTREE_MEMENTO_KEY, { - 'session-old': 'some-old-string', - 'session-new': makeWorktreeData(makeWorktreeV1Props()), - }); - - const store = await createStore(); - - const oldWt = await store.getWorktreeProperties('session-old'); - expect(oldWt).toBeUndefined(); - const newWt = await store.getWorktreeProperties('session-new'); - expect(newWt).toBeDefined(); - store.dispose(); - }); - }); - - // ────────────────────────────────────────────────────────────────────────── - // initializeStorage — migration failure safety (CRITICAL) - // ────────────────────────────────────────────────────────────────────────── - describe('initializeStorage - failure safety', () => { - it('should NOT clear global state when writeToGlobalStorage fails', async () => { - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-1': { folderPath: Uri.file('/workspace/a').fsPath, timestamp: 100 }, - }); - extensionContext.globalState.seed(WORKTREE_MEMENTO_KEY, { - 'session-2': makeWorktreeData(makeWorktreeV1Props()), - }); - - // Make writeFile throw so writeToGlobalStorage fails - vi.spyOn(mockFs, 'writeFile').mockRejectedValue(new Error('disk full')); - - const store = await createStore(); - - // Global state should be preserved since writing to file failed - expect(extensionContext.globalState.has(WORKSPACE_FOLDER_MEMENTO_KEY)).toBe(true); - expect(extensionContext.globalState.has(WORKTREE_MEMENTO_KEY)).toBe(true); - - // Initialization failure should be logged - expect(logService.error).toHaveBeenCalled(); - store.dispose(); - }); - - it('should NOT clear global state when createDirectory fails during writeToGlobalStorage', async () => { - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-1': { folderPath: Uri.file('/workspace/a').fsPath, timestamp: 100 }, - }); - - // stat throws (dir missing) and createDirectory also throws - vi.spyOn(mockFs, 'stat').mockRejectedValue(new Error('ENOENT')); - vi.spyOn(mockFs, 'createDirectory').mockRejectedValue(new Error('permission denied')); - - const store = await createStore(); - - expect(extensionContext.globalState.has(WORKSPACE_FOLDER_MEMENTO_KEY)).toBe(true); - store.dispose(); - }); - - it.skip('should still clear global state even when per-session file writes fail', async () => { - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-1': { folderPath: Uri.file('/workspace/a').fsPath, timestamp: 100 }, - }); - - // Allow bulk file write to succeed but track per-session writes - let writeCallCount = 0; - const origWriteFile = mockFs.writeFile.bind(mockFs); - vi.spyOn(mockFs, 'writeFile').mockImplementation(async (uri, content) => { - writeCallCount++; - // Let the bulk metadata file write succeed (first or second write) - if (uri.toString().includes('copilotcli.session.metadata.json')) { - return origWriteFile(uri, content); - } - // Fail per-session writes - throw new Error('per-session write failed'); - }); - - const store = await createStore(); - - // Global state should be cleared because bulk file write succeeded - // (per-session writes are fire-and-forget via Promise.allSettled) - expect(extensionContext.globalState.has(WORKSPACE_FOLDER_MEMENTO_KEY)).toBe(false); - store.dispose(); - }); - - it('should log error when initialization fails', async () => { - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-1': { folderPath: Uri.file('/workspace/a').fsPath, timestamp: 100 }, - }); - vi.spyOn(mockFs, 'writeFile').mockRejectedValue(new Error('disk full')); - - const store = await createStore(); - - expect(logService.error).toHaveBeenCalledWith( - '[ChatSessionMetadataStore] Initialization failed: ', - expect.any(Error), - ); - store.dispose(); - }); }); // ────────────────────────────────────────────────────────────────────────── @@ -1125,7 +612,7 @@ describe('ChatSessionMetadataStore', () => { const fileUri = sessionMetadataFileUri('session-no-file'); const rawContent = await mockFs.readFile(fileUri); const written = JSON.parse(new TextDecoder().decode(rawContent)); - expect(written).toEqual({ origin: 'other' }); + expect(written).toEqual(expect.objectContaining({ origin: 'other' })); store.dispose(); }); }); @@ -1244,14 +731,14 @@ describe('ChatSessionMetadataStore', () => { // Before debounce fires, count writes to bulk file const bulkWritesBefore = writeSpy.mock.calls.filter( - c => c[0].toString().includes('copilotcli.session.metadata.json'), + c => c[0].toString().includes('vscode.session.metadata.cache.json'), ).length; // Advance past debounce await vi.advanceTimersByTimeAsync(1_100); const bulkWritesAfter = writeSpy.mock.calls.filter( - c => c[0].toString().includes('copilotcli.session.metadata.json'), + c => c[0].toString().includes('vscode.session.metadata.cache.json'), ).length; // Should have exactly one new bulk write after debounce (coalesced) @@ -1530,33 +1017,19 @@ describe('ChatSessionMetadataStore', () => { store.dispose(); }); - it('should survive crash recovery: entry with only additionalWorkspaces is re-persisted not deleted', async () => { - // Simulate VS Code crash: bulk file has the entry but writtenToDisc is falsy - // (updateSessionMetadata never completed before the crash). + it('should keep entries with additionalWorkspaces in cache even without writtenToDisc flag', async () => { + // Bulk file has the entry but writtenToDisc is falsy — it should still + // be kept in the in-memory cache and accessible via the API. mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({ 'session-crash': { additionalWorkspaces: [ { workspaceFolder: { folderPath: Uri.file('/extra/workspace').fsPath, timestamp: 100 } }, ], - // writtenToDisc intentionally absent (falsy) — simulates crash before write completed }, })); - // Pre-create session directory so recovery write can succeed - await mockFs.createDirectory(sessionDirectoryUri('session-crash')); - const fileCreated = eventToPromise(mockFs.onDidCreateFile.event); - const store = await createStore(); - await fileCreated; - - // Entry should have been re-persisted to per-session file - const fileUri = sessionMetadataFileUri('session-crash'); - const rawContent = await mockFs.readFile(fileUri); - const written = JSON.parse(new TextDecoder().decode(rawContent)); - expect(written.additionalWorkspaces).toHaveLength(1); - expect(written.additionalWorkspaces[0].workspaceFolder?.folderPath).toBe(Uri.file('/extra/workspace').fsPath); - // And still readable via the API const result = await store.getAdditionalWorkspaces('session-crash'); expect(result).toHaveLength(1); store.dispose(); @@ -1768,21 +1241,15 @@ describe('ChatSessionMetadataStore', () => { // Constructor & edge cases // ────────────────────────────────────────────────────────────────────────── describe('constructor and edge cases', () => { - it('should handle initialization failure gracefully when writeFile fails and cache was updated', async () => { - // Bulk file read fails, falls through to global state - // Global state has data, so cacheUpdated = true, writeToGlobalStorage is called but writeFile fails - extensionContext.globalState.seed(WORKSPACE_FOLDER_MEMENTO_KEY, { - 'session-1': { folderPath: Uri.file('/workspace/a').fsPath, timestamp: 100 }, - }); + it('should handle initialization gracefully when bulk file read fails', async () => { + // Bulk file read fails — store starts with empty cache, no fatal error vi.spyOn(mockFs, 'readFile').mockRejectedValue(new Error('ENOENT')); - vi.spyOn(mockFs, 'writeFile').mockRejectedValue(new Error('disk error')); const store = await createStore(); - expect(logService.error).toHaveBeenCalledWith( - '[ChatSessionMetadataStore] Initialization failed: ', - expect.any(Error), - ); + // Cache should be empty but usable + const folder = await store.getSessionWorkspaceFolder('nonexistent'); + expect(folder).toBeUndefined(); store.dispose(); }); @@ -1881,4 +1348,432 @@ describe('ChatSessionMetadataStore', () => { }); }); + // ────────────────────────────────────────────────────────────────────────── + // New behaviors: shared bulk file, refresh(), JSONL worktree index, + // last-modified-wins merge, top-N trim, legacy migration. + // Each test is intentionally small and self-contained. + // ────────────────────────────────────────────────────────────────────────── + describe('shared bulk file + refresh()', () => { + it('refresh() picks up a session written by another process to the shared bulk file', async () => { + vi.setSystemTime(new Date(0)); + // Start with empty bulk file. + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({})); + const store = await createStore(); + expect(await store.getSessionWorkspaceFolder('cross-proc-session')).toBeUndefined(); + + // Simulate "other process" rewriting the shared bulk file. `modified: 999` is + // guaranteed to be > anything stamped locally because we pinned the clock to 0. + const externalEntry = { + 'cross-proc-session': { + workspaceFolder: { folderPath: Uri.file('/external/folder').fsPath, timestamp: 42 }, + modified: 999, + }, + }; + await mockFs.writeFile(BULK_METADATA_FILE, new TextEncoder().encode(JSON.stringify(externalEntry))); + + await store.refresh(); + + expect((await store.getSessionWorkspaceFolder('cross-proc-session'))?.fsPath) + .toBe(Uri.file('/external/folder').fsPath); + store.dispose(); + }); + + it('refresh() never drops in-memory entries that are not on disk', async () => { + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({})); + const store = await createStore(); + await store.storeWorkspaceFolderInfo('local-only', { folderPath: Uri.file('/local').fsPath, timestamp: 1 }); + await vi.advanceTimersByTimeAsync(2000); // flush debounced bulk write + + // Wipe the on-disk bulk file (simulating an external truncation). + await mockFs.writeFile(BULK_METADATA_FILE, new TextEncoder().encode(JSON.stringify({}))); + + await store.refresh(); + + expect((await store.getSessionWorkspaceFolder('local-only'))?.fsPath) + .toBe(Uri.file('/local').fsPath); + store.dispose(); + }); + + it('refresh() failure does not poison subsequent reads (chained _ready)', async () => { + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({})); + const store = await createStore(); + + // Make the next bulk read throw. + const readSpy = vi.spyOn(mockFs, 'readFile').mockImplementationOnce(async () => { throw new Error('boom'); }); + await store.refresh(); // swallowed inside + + readSpy.mockRestore(); + await store.storeWorkspaceFolderInfo('after-fail', { folderPath: Uri.file('/after').fsPath, timestamp: 1 }); + expect((await store.getSessionWorkspaceFolder('after-fail'))?.fsPath).toBe(Uri.file('/after').fsPath); + store.dispose(); + }); + }); + + describe('last-modified-wins merge', () => { + it('keeps the entry with the higher `modified` timestamp', async () => { + vi.setSystemTime(new Date(0)); + // Bulk file holds an OLDER copy of session-1. + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({ + 'session-1': { + workspaceFolder: { folderPath: Uri.file('/old/path').fsPath, timestamp: 1 }, + modified: 100, + }, + })); + const store = await createStore(); + + // Another process writes a NEWER version directly to disk. + await mockFs.writeFile(BULK_METADATA_FILE, new TextEncoder().encode(JSON.stringify({ + 'session-1': { + workspaceFolder: { folderPath: Uri.file('/newer/path').fsPath, timestamp: 2 }, + modified: 5000, + }, + }))); + + await store.refresh(); + + expect((await store.getSessionWorkspaceFolder('session-1'))?.fsPath) + .toBe(Uri.file('/newer/path').fsPath); + store.dispose(); + }); + + it('does not overwrite a fresher in-memory entry with an older disk entry', async () => { + vi.setSystemTime(new Date(10_000)); + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({})); + const store = await createStore(); + await store.storeWorkspaceFolderInfo('session-fresh', { folderPath: Uri.file('/fresh').fsPath, timestamp: 100 }); + await vi.advanceTimersByTimeAsync(2000); + + // External writer puts an OLDER copy on disk (lower modified). + await mockFs.writeFile(BULK_METADATA_FILE, new TextEncoder().encode(JSON.stringify({ + 'session-fresh': { workspaceFolder: { folderPath: Uri.file('/stale').fsPath, timestamp: 1 }, modified: 1 }, + }))); + + await store.refresh(); + + expect((await store.getSessionWorkspaceFolder('session-fresh'))?.fsPath).toBe(Uri.file('/fresh').fsPath); + store.dispose(); + }); + }); + + describe('legacy bulk file migration (Step 0)', () => { + it('migrates from the legacy globalStorage path on first run', async () => { + mockFs.mockFile(LEGACY_BULK_METADATA_FILE, JSON.stringify({ + 'legacy-session': { workspaceFolder: { folderPath: Uri.file('/legacy').fsPath, timestamp: 1 } }, + })); + // New shared file does NOT exist yet. + + const store = await createStore(); + + expect((await store.getSessionWorkspaceFolder('legacy-session'))?.fsPath) + .toBe(Uri.file('/legacy').fsPath); + // Migration wrote to the new path. + const newRaw = await mockFs.readFile(BULK_METADATA_FILE); + expect(JSON.parse(new TextDecoder().decode(newRaw))).toHaveProperty('legacy-session'); + store.dispose(); + }); + + it('merges legacy entries into an existing shared file (late-joiner scenario)', async () => { + // Process A already created the shared file with session-A. + // Process B starts with its own legacy file containing session-B. + // Both should be present after migration. + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({ + 'session-A': { workspaceFolder: { folderPath: Uri.file('/a').fsPath, timestamp: 1 }, modified: 100 }, + })); + mockFs.mockFile(LEGACY_BULK_METADATA_FILE, JSON.stringify({ + 'session-B': { workspaceFolder: { folderPath: Uri.file('/b').fsPath, timestamp: 2 }, modified: 200 }, + })); + + const store = await createStore(); + + expect((await store.getSessionWorkspaceFolder('session-A'))?.fsPath).toBe(Uri.file('/a').fsPath); + expect((await store.getSessionWorkspaceFolder('session-B'))?.fsPath).toBe(Uri.file('/b').fsPath); + store.dispose(); + }); + + it('uses last-modified-wins when the same session exists in both files', async () => { + vi.setSystemTime(new Date(0)); + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({ + 'shared-session': { workspaceFolder: { folderPath: Uri.file('/old').fsPath, timestamp: 1 }, modified: 50 }, + })); + mockFs.mockFile(LEGACY_BULK_METADATA_FILE, JSON.stringify({ + 'shared-session': { workspaceFolder: { folderPath: Uri.file('/newer').fsPath, timestamp: 2 }, modified: 200 }, + })); + + const store = await createStore(); + + // Legacy had higher `modified` → its version wins + expect((await store.getSessionWorkspaceFolder('shared-session'))?.fsPath).toBe(Uri.file('/newer').fsPath); + store.dispose(); + }); + + it('sets memento flag after successful merge so it does not re-run', async () => { + mockFs.mockFile(LEGACY_BULK_METADATA_FILE, JSON.stringify({ + 'legacy-session': { workspaceFolder: { folderPath: Uri.file('/legacy').fsPath, timestamp: 1 } }, + })); + + const store = await createStore(); + expect(extensionContext.globalState.get('github.copilot.cli.legacyBulkMigrated')).toBe(true); + store.dispose(); + }); + + it('skips migration when memento flag is already set', async () => { + extensionContext.globalState.seed('github.copilot.cli.legacyBulkMigrated', true); + mockFs.mockFile(LEGACY_BULK_METADATA_FILE, JSON.stringify({ + 'legacy-session': { workspaceFolder: { folderPath: Uri.file('/legacy').fsPath, timestamp: 1 } }, + })); + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({})); + + const readSpy = vi.spyOn(mockFs, 'readFile'); + const store = await createStore(); + + // Legacy file should not have been read (migration skipped). + const legacyReads = readSpy.mock.calls.filter( + c => c[0].toString().includes('copilotcli.session.metadata.json'), + ); + expect(legacyReads).toHaveLength(0); + expect(await store.getSessionWorkspaceFolder('legacy-session')).toBeUndefined(); + store.dispose(); + }); + + it('does nothing when no legacy file exists', async () => { + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({ + 'kept-session': { workspaceFolder: { folderPath: Uri.file('/kept').fsPath, timestamp: 1 } }, + })); + // No legacy file seeded + + const store = await createStore(); + + expect((await store.getSessionWorkspaceFolder('kept-session'))?.fsPath).toBe(Uri.file('/kept').fsPath); + store.dispose(); + }); + }); + + describe('JSONL worktree index', () => { + const jsonlUri = () => Uri.file(jsonlPathHolder.get()); + + async function readJsonl(): Promise>> { + try { + const bytes = await mockFs.readFile(jsonlUri()); + const raw = new TextDecoder().decode(bytes); + return raw.split('\n').filter(Boolean).map(l => JSON.parse(l)); + } catch { + return []; + } + } + + it('appends one line per worktree session and reads it back via getSessionIdForWorktree', async () => { + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({})); + const store = await createStore(); + + const folder = Uri.file('/repo/.worktrees/wt-A'); + await store.storeWorktreeInfo('wt-session-A', makeWorktreeV1Props({ worktreePath: folder.fsPath })); + + const lines = await readJsonl(); + expect(lines).toHaveLength(1); + expect(lines[0]).toMatchObject({ id: 'wt-session-A', path: folder.fsPath }); + + // Lookup by folder works via the in-memory map. + expect(await store.getSessionIdForWorktree(folder)).toBe('wt-session-A'); + store.dispose(); + }); + + it('falls back to JSONL on disk for getSessionIdForWorktree when in-memory cache is cold', async () => { + // Pre-seed JSONL in mock fs before the store starts — simulates an entry written by another process. + const folder = Uri.file('/repo/.worktrees/wt-cold'); + mockFs.mockFile( + jsonlUri(), + JSON.stringify({ id: 'wt-session-cold', path: folder.fsPath, created: 100 }) + '\n', + ); + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({})); + + const store = await createStore(); + expect(await store.getSessionIdForWorktree(folder)).toBe('wt-session-cold'); + store.dispose(); + }); + + it('compacts duplicate JSONL lines for the same id on next rewrite', async () => { + // Two entries for the same id — last write wins, file should be rewritten. + const folder = Uri.file('/repo/.worktrees/dup'); + mockFs.mockFile( + jsonlUri(), + JSON.stringify({ id: 'dup-id', path: '/old/path', created: 1 }) + '\n' + + JSON.stringify({ id: 'dup-id', path: folder.fsPath, created: 2 }) + '\n', + ); + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({})); + + const store = await createStore(); + // Initialization should have detected the duplicate and rewritten the file. + const lines = await readJsonl(); + expect(lines).toHaveLength(1); + expect(lines[0]).toMatchObject({ id: 'dup-id', path: folder.fsPath }); + expect(await store.getSessionIdForWorktree(folder)).toBe('dup-id'); + store.dispose(); + }); + + it('removes the JSONL entry when a session is deleted', async () => { + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({})); + const store = await createStore(); + const folder = Uri.file('/repo/.worktrees/to-delete'); + await store.storeWorktreeInfo('to-delete', makeWorktreeV1Props({ worktreePath: folder.fsPath })); + expect(await readJsonl()).toHaveLength(1); + + await store.deleteSessionMetadata('to-delete'); + + expect(await readJsonl()).toHaveLength(0); + expect(await store.getSessionIdForWorktree(folder)).toBeUndefined(); + store.dispose(); + }); + }); + + describe('top-N trim (MAX_BULK_STORAGE_ENTRIES = 1000)', () => { + it('writes at most 1000 entries to the bulk file but keeps everything in memory', async () => { + // Pre-seed a bulk file with 1100 entries with varying `modified` timestamps. + const initial: Record = {}; + for (let i = 0; i < 1100; i++) { + initial[`s-${i}`] = { + workspaceFolder: { folderPath: `/w/${i}`, timestamp: i }, + modified: i, // s-0 oldest, s-1099 newest + }; + } + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify(initial)); + + const store = await createStore(); + // Trigger a write so the trim runs. + await store.storeWorkspaceFolderInfo('trigger', { folderPath: '/trigger', timestamp: Date.now() }); + await vi.advanceTimersByTimeAsync(2000); + + const onDisk = JSON.parse(new TextDecoder().decode(await mockFs.readFile(BULK_METADATA_FILE))); + expect(Object.keys(onDisk).length).toBeLessThanOrEqual(1000); + // Newest entries (highest `modified`) should still be present. + expect(onDisk['s-1099']).toBeTruthy(); + expect(onDisk['trigger']).toBeTruthy(); + // Oldest entry should have been evicted from disk. + expect(onDisk['s-0']).toBeUndefined(); + + // In-memory cache still serves the evicted entry's data via per-session file fallback, + // but the cache itself was hydrated at init so it still knows about s-0. + // (Per-session files are the source of truth for evicted entries.) + store.dispose(); + }); + }); + + describe('updateMetadataFields - stale cache write safety (Step 3b)', () => { + it('writes only the requested partial fields; other fields written by external process are preserved', async () => { + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({})); + const store = await createStore(); + + // Create the per-session file with an initial customTitle. + await store.setCustomTitle('shared-session', 'initial-title'); + await vi.advanceTimersByTimeAsync(2000); + + // Simulate another process writing a `firstUserMessage` to the same per-session file. + const sessionFile = sessionMetadataFileUri('shared-session'); + const existingRaw = await mockFs.readFile(sessionFile); + const existing = JSON.parse(new TextDecoder().decode(existingRaw)); + const externallyMerged = { ...existing, firstUserMessage: 'from-other-process' }; + await mockFs.writeFile(sessionFile, new TextEncoder().encode(JSON.stringify(externallyMerged))); + + // Now update the title from THIS process. Critically, the partial-only write + // must not stomp `firstUserMessage` even though our `_cache` does not know about it. + await store.setCustomTitle('shared-session', 'updated-title'); + await vi.advanceTimersByTimeAsync(2000); + + const finalRaw = await mockFs.readFile(sessionFile); + const final = JSON.parse(new TextDecoder().decode(finalRaw)); + expect(final.customTitle).toBe('updated-title'); + expect(final.firstUserMessage).toBe('from-other-process'); + store.dispose(); + }); + }); + + describe('timestamps', () => { + it('stamps `created` once and bumps `modified` on every write', async () => { + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({})); + vi.setSystemTime(new Date(1_000_000)); + const store = await createStore(); + + await store.setCustomTitle('ts-session', 'first'); + await vi.advanceTimersByTimeAsync(2000); + + const file = sessionMetadataFileUri('ts-session'); + const first = JSON.parse(new TextDecoder().decode(await mockFs.readFile(file))); + expect(first.created).toBeTypeOf('number'); + expect(first.modified).toBeTypeOf('number'); + const createdAt = first.created; + + vi.setSystemTime(new Date(2_000_000)); + await store.setCustomTitle('ts-session', 'second'); + await vi.advanceTimersByTimeAsync(2000); + + const second = JSON.parse(new TextDecoder().decode(await mockFs.readFile(file))); + expect(second.created).toBe(createdAt); // unchanged + expect(second.modified).toBeGreaterThan(first.modified); // bumped + store.dispose(); + }); + }); + + describe('session-state directory scan', () => { + const sessionStateDir = Uri.file('/mock/session-state'); + + it('discovers worktree sessions from per-session files not in cache or JSONL', async () => { + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({})); + // Simulate a per-session file on disk that is NOT in the bulk cache. + const folder = Uri.file('/repo/.worktrees/discovered'); + await mockFs.createDirectory(Uri.joinPath(sessionStateDir, 'orphan-session')); + mockFs.mockFile( + sessionMetadataFileUri('orphan-session'), + JSON.stringify({ worktreeProperties: makeWorktreeV1Props({ worktreePath: folder.fsPath }) }), + ); + // readDirectory returns the session dir entries. + mockFs.mockDirectory(sessionStateDir, [['orphan-session', 2 /* Directory */]]); + + const store = await createStore(); + + expect(await store.getSessionIdForWorktree(folder)).toBe('orphan-session'); + store.dispose(); + }); + + it('skips session IDs already known from the bulk cache', async () => { + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({ + 'known-session': { workspaceFolder: { folderPath: Uri.file('/known').fsPath, timestamp: 1 } }, + })); + mockFs.mockDirectory(sessionStateDir, [['known-session', 2]]); + + const readSpy = vi.spyOn(mockFs, 'readFile'); + const store = await createStore(); + + // Per-session file for known-session should NOT have been read by the scan. + const scanReads = readSpy.mock.calls.filter( + c => c[0].toString().includes('/mock/session-state/known-session/vscode.metadata.json'), + ); + expect(scanReads).toHaveLength(0); + store.dispose(); + }); + + it('sets memento flag so the scan does not re-run on next startup', async () => { + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({})); + mockFs.mockDirectory(sessionStateDir, []); + + await createStore(); + expect(extensionContext.globalState.get('github.copilot.cli.events.jsonl.scaned')).toBe(true); + }); + + it('skips scan when memento flag is already set', async () => { + extensionContext.globalState.seed('github.copilot.cli.events.jsonl.scaned', true); + mockFs.mockFile(BULK_METADATA_FILE, JSON.stringify({})); + // Even with a discoverable session, scan should be skipped. + await mockFs.createDirectory(Uri.joinPath(sessionStateDir, 'should-skip')); + mockFs.mockFile( + sessionMetadataFileUri('should-skip'), + JSON.stringify({ worktreeProperties: makeWorktreeV1Props() }), + ); + mockFs.mockDirectory(sessionStateDir, [['should-skip', 2]]); + + const store = await createStore(); + + expect(await store.getSessionIdForWorktree(Uri.file(makeWorktreeV1Props().worktreePath))).toBeUndefined(); + store.dispose(); + }); + }); + }); diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/test/worktreeSessionIndex.spec.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/test/worktreeSessionIndex.spec.ts new file mode 100644 index 0000000000000..c397ac52a180b --- /dev/null +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/test/worktreeSessionIndex.spec.ts @@ -0,0 +1,190 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { Uri } from 'vscode'; +import { MockFileSystemService } from '../../../../platform/filesystem/node/test/mockFileSystemService'; +import { ILogService } from '../../../../platform/log/common/logService'; +import { mock } from '../../../../util/common/test/simpleMock'; +import { WorktreeSessionIndex } from '../worktreeSessionIndex'; + +class MockLogService extends mock() { + override trace = vi.fn(); + override info = vi.fn(); + override warn = vi.fn(); + override error = vi.fn(); + override debug = vi.fn(); +} + +const JSONL_PATH = '/mock/copilot-home/worktree.jsonl'; +const JSONL_URI = Uri.file(JSONL_PATH); + +describe('WorktreeSessionIndex', () => { + let mockFs: MockFileSystemService; + let logService: MockLogService; + + function createIndex(): WorktreeSessionIndex { + return new WorktreeSessionIndex(mockFs, logService, JSONL_PATH); + } + + afterEach(() => { + vi.restoreAllMocks(); + }); + + // In-memory tests don't need mockFs/logService, but the constructor requires them. + describe('in-memory operations', () => { + it('adds and retrieves an entry by session id', () => { + mockFs = new MockFileSystemService(); + logService = new MockLogService(); + const index = createIndex(); + index.addEntry({ id: 's1', path: '/a', created: 1 }); + + expect(index.getSessionEntry('s1')).toMatchObject({ id: 's1', path: '/a' }); + expect(index.has('s1')).toBe(true); + expect(index.has('s2')).toBe(false); + }); + + it('looks up session id by folder Uri', () => { + mockFs = new MockFileSystemService(); + logService = new MockLogService(); + const index = createIndex(); + index.addEntry({ id: 's1', path: '/a', created: 1 }); + + expect(index.getSessionIdForFolder(Uri.file('/a'))).toBe('s1'); + expect(index.getSessionIdForFolder(Uri.file('/b'))).toBeUndefined(); + }); + + it('deletes an entry and cleans up the folder mapping', () => { + mockFs = new MockFileSystemService(); + logService = new MockLogService(); + const index = createIndex(); + index.addEntry({ id: 's1', path: '/a', created: 1 }); + index.deleteEntry('s1'); + + expect(index.has('s1')).toBe(false); + expect(index.getSessionIdForFolder(Uri.file('/a'))).toBeUndefined(); + }); + + it('clear() removes everything', () => { + mockFs = new MockFileSystemService(); + logService = new MockLogService(); + const index = createIndex(); + index.addEntry({ id: 's1', path: '/a', created: 1 }); + index.addEntry({ id: 's2', path: '/b', created: 2 }); + index.clear(); + + expect(index.has('s1')).toBe(false); + expect(index.getEntries()).toHaveLength(0); + }); + + it('getEntries() returns all entries', () => { + mockFs = new MockFileSystemService(); + logService = new MockLogService(); + const index = createIndex(); + index.addEntry({ id: 's1', path: '/a', created: 1 }); + index.addEntry({ id: 's2', path: '/b', created: 2 }); + + const entries = index.getEntries(); + expect(entries).toHaveLength(2); + expect(entries.map(e => e.id).sort()).toEqual(['s1', 's2']); + }); + + it('updating an entry with a new path removes the old path mapping', () => { + mockFs = new MockFileSystemService(); + logService = new MockLogService(); + const index = createIndex(); + index.addEntry({ id: 's1', path: '/old', created: 1 }); + expect(index.getSessionIdForFolder(Uri.file('/old'))).toBe('s1'); + + index.addEntry({ id: 's1', path: '/new', created: 1 }); + expect(index.getSessionIdForFolder(Uri.file('/new'))).toBe('s1'); + expect(index.getSessionIdForFolder(Uri.file('/old'))).toBeUndefined(); + }); + }); + + describe('JSONL persistence', () => { + it('loadFromDisk populates the index from a JSONL file', async () => { + mockFs = new MockFileSystemService(); + logService = new MockLogService(); + mockFs.mockFile(JSONL_URI, + JSON.stringify({ id: 's1', path: '/a', created: 1 }) + '\n' + + JSON.stringify({ id: 's2', path: '/b', created: 2 }) + '\n', + ); + const index = createIndex(); + await index.loadFromDisk(); + + expect(index.has('s1')).toBe(true); + expect(index.has('s2')).toBe(true); + expect(index.size).toBe(2); + }); + + it('loadFromDisk returns rewriteNeeded for duplicates', async () => { + mockFs = new MockFileSystemService(); + logService = new MockLogService(); + mockFs.mockFile(JSONL_URI, + JSON.stringify({ id: 's1', path: '/a', created: 1 }) + '\n' + + JSON.stringify({ id: 's1', path: '/b', created: 2 }) + '\n', + ); + const index = createIndex(); + const { rewriteNeeded } = await index.loadFromDisk(); + + expect(rewriteNeeded).toBe(true); + expect(index.size).toBe(1); + }); + + it('writeToDisk writes all entries to the JSONL file', async () => { + mockFs = new MockFileSystemService(); + logService = new MockLogService(); + const index = createIndex(); + index.addEntry({ id: 's1', path: '/a', created: 1 }); + index.addEntry({ id: 's2', path: '/b', created: 2 }); + await index.writeToDisk(); + + const raw = new TextDecoder().decode(await mockFs.readFile(JSONL_URI)); + const lines = raw.split('\n').filter(Boolean); + expect(lines).toHaveLength(2); + }); + + it('appendBatchToDisk adds a single entry', async () => { + mockFs = new MockFileSystemService(); + logService = new MockLogService(); + const index = createIndex(); + await index.appendBatchToDisk([{ id: 's1', path: '/a', created: 1 }]); + + expect(index.has('s1')).toBe(true); + const raw = new TextDecoder().decode(await mockFs.readFile(JSONL_URI)); + expect(raw.split('\n').filter(Boolean)).toHaveLength(1); + }); + + it('appendBatchToDisk adds multiple entries in one write', async () => { + mockFs = new MockFileSystemService(); + logService = new MockLogService(); + const index = createIndex(); + await index.appendBatchToDisk([ + { id: 's1', path: '/a', created: 1 }, + { id: 's2', path: '/b', created: 2 }, + ]); + + expect(index.has('s1')).toBe(true); + expect(index.has('s2')).toBe(true); + const raw = new TextDecoder().decode(await mockFs.readFile(JSONL_URI)); + expect(raw.split('\n').filter(Boolean)).toHaveLength(2); + }); + + it('removeAndWriteToDisk removes the entry and rewrites', async () => { + mockFs = new MockFileSystemService(); + logService = new MockLogService(); + const index = createIndex(); + index.addEntry({ id: 's1', path: '/a', created: 1 }); + index.addEntry({ id: 's2', path: '/b', created: 2 }); + await index.removeAndWriteToDisk('s1'); + + expect(index.has('s1')).toBe(false); + const raw = new TextDecoder().decode(await mockFs.readFile(JSONL_URI)); + expect(raw.split('\n').filter(Boolean)).toHaveLength(1); + expect(raw).toContain('s2'); + }); + }); +}); diff --git a/extensions/copilot/src/extension/chatSessions/vscode-node/worktreeSessionIndex.ts b/extensions/copilot/src/extension/chatSessions/vscode-node/worktreeSessionIndex.ts new file mode 100644 index 0000000000000..ca8586c639f8a --- /dev/null +++ b/extensions/copilot/src/extension/chatSessions/vscode-node/worktreeSessionIndex.ts @@ -0,0 +1,221 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Uri } from 'vscode'; +import { createDirectoryIfNotExists, IFileSystemService } from '../../../platform/filesystem/common/fileSystemService'; +import { ILogService } from '../../../platform/log/common/logService'; +import { Sequencer } from '../../../util/vs/base/common/async'; +import { ResourceMap } from '../../../util/vs/base/common/map'; +import { dirname } from '../../../util/vs/base/common/resources'; +import { WorktreeSessionEntry } from '../common/chatSessionMetadataStore'; + +/** + * In-memory index that maps session ids to {@link WorktreeSessionEntry} and + * worktree folder URIs to session ids, with JSONL file persistence. + * + * When multiple sessions share the same folder, the first-registered session + * keeps the folder → session-id mapping. + * + * All file writes are serialized through an internal {@link Sequencer} so + * concurrent appends and rewrites cannot race. + */ +export class WorktreeSessionIndex { + /** Session id → entry. */ + private readonly _byId = new Map(); + /** Worktree folder URI → session id. Uses URI-aware comparison so path casing is handled correctly. */ + private readonly _byFolder = new ResourceMap(); + /** Serializes all JSONL file writes to prevent read-modify-write races. */ + private readonly _writeSequencer = new Sequencer(); + /** Timestamp of the last {@link loadFromDisk} call; used by {@link reloadIfStale}. */ + private _lastLoadAt = 0; + + constructor( + private readonly _fileSystemService: IFileSystemService, + private readonly _logService: ILogService, + private readonly _jsonlPath: string, + ) { } + + getSessionEntry(sessionId: string): WorktreeSessionEntry | undefined { + return this._byId.get(sessionId); + } + + getSessionIdForFolder(folder: Uri): string | undefined { + return this._byFolder.get(folder); + } + + has(sessionId: string): boolean { + return this._byId.has(sessionId); + } + + get size(): number { + return this._byId.size; + } + + getAllSessionIds(): string[] { + return Array.from(this._byId.keys()); + } + + /** + * Adds or updates an entry. When the same folder path is already mapped to + * a different session, the existing mapping is preserved. + */ + addEntry(entry: WorktreeSessionEntry): void { + const folderUri = Uri.file(entry.path); + + // If this session already has an entry with a different path, clean up + // the old folder → session-id mapping before recording the new one. + const previousEntry = this._byId.get(entry.id); + if (previousEntry && previousEntry.path !== entry.path) { + const prevUri = Uri.file(previousEntry.path); + if (this._byFolder.get(prevUri) === entry.id) { + this._byFolder.delete(prevUri); + } + } + + this._byId.set(entry.id, entry); + + const existingIdForFolder = this._byFolder.get(folderUri); + if (!existingIdForFolder) { + this._byFolder.set(folderUri, entry.id); + return; + } + if (existingIdForFolder === entry.id) { + return; + } + const existingEntry = this._byId.get(existingIdForFolder); + if (existingEntry) { + return; + } + this._byFolder.set(folderUri, entry.id); + } + + deleteEntry(sessionId: string): void { + const entry = this._byId.get(sessionId); + if (!entry) { + return; + } + this._byId.delete(sessionId); + const folderUri = Uri.file(entry.path); + if (this._byFolder.get(folderUri) === sessionId) { + this._byFolder.delete(folderUri); + for (const candidate of this._byId.values()) { + if (candidate.path === entry.path) { + this._byFolder.set(folderUri, candidate.id); + break; + } + } + } + } + + clear(): void { + this._byId.clear(); + this._byFolder.clear(); + } + + getEntries(): WorktreeSessionEntry[] { + return Array.from(this._byId.values()); + } + + /** + * Loads the JSONL worktree index from disk into the in-memory maps. + * Returns `rewriteNeeded` if the file contained malformed lines or + * duplicates that should be compacted via {@link writeToDisk}. + */ + async loadFromDisk(): Promise<{ rewriteNeeded: boolean }> { + let rewriteNeeded = false; + let raw: string; + try { + const bytes = await this._fileSystemService.readFile(Uri.file(this._jsonlPath)); + raw = new TextDecoder().decode(bytes); + } catch { + this._lastLoadAt = Date.now(); + return { rewriteNeeded: false }; + } + this.clear(); + for (const line of raw.split('\n')) { + const trimmed = line.trim(); + if (!trimmed) { + continue; + } + try { + const entry = JSON.parse(trimmed) as WorktreeSessionEntry; + if (!entry?.id || !entry.path) { + rewriteNeeded = true; + continue; + } + if (this._byId.has(entry.id)) { + rewriteNeeded = true; + } + this.addEntry(entry); + } catch { + rewriteNeeded = true; + } + } + this._lastLoadAt = Date.now(); + return { rewriteNeeded }; + } + + /** Reloads from disk only if more than 1 second has passed since the last load. */ + async reloadIfStale(): Promise { + if (Date.now() - this._lastLoadAt < 1000) { + return; + } + await this.loadFromDisk(); + } + + /** Writes the entire in-memory index to the JSONL file, replacing its contents. */ + async writeToDisk(): Promise { + return this._writeSequencer.queue(async () => { + try { + const jsonlUri = Uri.file(this._jsonlPath); + await createDirectoryIfNotExists(this._fileSystemService, dirname(jsonlUri)); + const lines = this._byId.size > 0 + ? Array.from(this._byId.values()).map(e => JSON.stringify(e)).join('\n') + '\n' + : ''; + await this._fileSystemService.writeFile(jsonlUri, new TextEncoder().encode(lines)); + } catch (err) { + this._logService.error('[WorktreeSessionIndex] Failed to write JSONL: ', err); + } + }); + } + + /** Appends entries to the JSONL file and adds them to the in-memory index. */ + async appendBatchToDisk(entries: WorktreeSessionEntry[]): Promise { + if (entries.length === 0) { + return; + } + return this._writeSequencer.queue(async () => { + try { + const jsonlUri = Uri.file(this._jsonlPath); + await createDirectoryIfNotExists(this._fileSystemService, dirname(jsonlUri)); + let existing = ''; + try { + existing = new TextDecoder().decode(await this._fileSystemService.readFile(jsonlUri)); + } catch { + // File doesn't exist yet. + } + const suffix = entries.map(e => JSON.stringify(e)).join('\n') + '\n'; + await this._fileSystemService.writeFile( + jsonlUri, + new TextEncoder().encode(existing + suffix), + ); + for (const entry of entries) { + this.addEntry(entry); + } + } catch (err) { + this._logService.error('[WorktreeSessionIndex] Failed to bulk-append entries: ', err); + } + }); + } + + /** Removes an entry from the in-memory index and rewrites the JSONL file. */ + async removeAndWriteToDisk(sessionId: string): Promise { + if (!this._byId.has(sessionId)) { + return; + } + this.deleteEntry(sessionId); + await this.writeToDisk(); + } +} From 083af3b3afdb172cbd72cb3360646ee449b62758 Mon Sep 17 00:00:00 2001 From: "vs-code-engineering[bot]" <122617954+vs-code-engineering[bot]@users.noreply.github.com> Date: Tue, 21 Apr 2026 09:35:31 +0000 Subject: [PATCH 16/17] [cherry-pick] Remove ReviewPlanTool from BuiltinToolsContribution (#311638) Co-authored-by: vs-code-engineering[bot] --- src/vs/workbench/contrib/chat/common/tools/builtinTools/tools.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vs/workbench/contrib/chat/common/tools/builtinTools/tools.ts b/src/vs/workbench/contrib/chat/common/tools/builtinTools/tools.ts index 8fae6e4d033ba..4a711c404fa5f 100644 --- a/src/vs/workbench/contrib/chat/common/tools/builtinTools/tools.ts +++ b/src/vs/workbench/contrib/chat/common/tools/builtinTools/tools.ts @@ -39,7 +39,6 @@ export class BuiltinToolsContribution extends Disposable implements IWorkbenchCo const reviewPlanTool = this._register(instantiationService.createInstance(ReviewPlanTool)); this._register(toolsService.registerTool(ReviewPlanToolData, reviewPlanTool)); - this._register(toolsService.vscodeToolSet.addTool(ReviewPlanToolData)); const todoToolData = createManageTodoListToolData(); const manageTodoListTool = this._register(instantiationService.createInstance(ManageTodoListTool)); From 1f3b864525456958767d5a1542e38411403a382c Mon Sep 17 00:00:00 2001 From: Ladislau Szomoru <3372902+lszomoru@users.noreply.github.com> Date: Tue, 21 Apr 2026 10:10:05 +0000 Subject: [PATCH 17/17] Agents - fix resource order in the multi-file diff editor (#311639) --- .../contrib/changes/browser/changesView.ts | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/vs/sessions/contrib/changes/browser/changesView.ts b/src/vs/sessions/contrib/changes/browser/changesView.ts index d22d3baf5a8f0..60b1b833a5ade 100644 --- a/src/vs/sessions/contrib/changes/browser/changesView.ts +++ b/src/vs/sessions/contrib/changes/browser/changesView.ts @@ -74,6 +74,7 @@ import { ResourceTree } from '../../../../base/common/resourceTree.js'; import { structuralEquals } from '../../../../base/common/equals.js'; import { compareFileNames, comparePaths } from '../../../../base/common/comparers.js'; import { ICommandService } from '../../../../platform/commands/common/commands.js'; +import { IChatSessionFileChange, IChatSessionFileChange2, isIChatSessionFileChange2 } from '../../../../workbench/contrib/chat/common/chatSessionsService.js'; const $ = dom.$; @@ -1013,14 +1014,30 @@ export class ChangesViewPane extends ViewPane { return; } + const compare = (aChange: IChatSessionFileChange | IChatSessionFileChange2, bChange: IChatSessionFileChange | IChatSessionFileChange2): number => { + const aPath = isIChatSessionFileChange2(aChange) ? aChange.uri.fsPath : aChange.modifiedUri.fsPath; + const bPath = isIChatSessionFileChange2(bChange) ? bChange.uri.fsPath : bChange.modifiedUri.fsPath; + return comparePaths(aPath, bPath); + }; + + // Sort the changes + const resources = changes.toSorted(compare).map(d => ({ + originalUri: d.originalUri, + modifiedUri: d.modifiedUri + })); + + // Ensure the reveal resource is part of the changes + reveal = reveal + ? resources.some(r => isEqual(r.modifiedUri, reveal)) + ? reveal + : undefined + : undefined; + // Open multi-file diff editor await this.commandService.executeCommand('_workbench.openMultiDiffEditor', { multiDiffSourceUri: sessionResource.with({ scheme: sessionResource.scheme + '-session-changes' }), title: localize('sessions.changes.title', 'Session Changes'), - resources: changes.map(d => ({ - originalUri: d.originalUri, - modifiedUri: d.modifiedUri - })), + resources, reveal: reveal ? { modifiedUri: reveal } : undefined