-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: Add external folder context for monorepos and full-stack workflows #1693
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Implement github.copilot.chat.addSystemContext and systemContextService. Add related contributions and i18n updates (package.json, package.nls.json). Add helper tools/utilities used by the feature (listDirTool, toolUtils) and wire into prompt/request handler. Note: To surface this choice in the Copilot Chat 'Add Context' QuickPick, a follow-up UI change is required in the chat/workbench UI.
…rvice Implements github.copilot.chat.addExternalContext command, refactors associated contributions and services, enforces 3-folder cap, and adds dedicated test coverage. Safe sync commit — local testing pending.
…ogic Aligns naming, command wiring, and prompt/tool references with ExternalContext design. Enforces 3-folder cap via IExternalContextService, adds exclusion guardrails and status-bar badge. Includes package and i18n updates.
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds external context support to GitHub Copilot Chat, allowing users to add folders outside their workspace that Copilot can access. Users can manage up to 3 external folders via commands and a status bar indicator, with these paths automatically included as chat references.
Key changes:
- Implements
ExternalContextServiceto manage external folder paths with events and path comparison logic - Adds UI contribution with commands for adding/managing external folders and a status bar item
- Integrates external paths into tool validation (
toolUtils,listDirTool) and chat references (chatParticipantRequestHandler)
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/extension/context/node/externalContextService.ts | Core service managing external paths with events, max limit enforcement, and path comparison logic |
| src/extension/context/vscode-node/externalContextContribution.ts | UI contribution providing commands, status bar item, and folder picker with exclusion rules |
| src/extension/context/node/test/externalContextService.spec.ts | Unit tests for external context service covering max paths, events, and removal |
| src/extension/tools/node/toolUtils.ts | Updates file validation to allow external paths in tools |
| src/extension/tools/node/listDirTool.tsx | Updates directory listing tool to allow external paths |
| src/extension/prompt/node/chatParticipantRequestHandler.ts | Injects external paths as chat references for each request |
| src/extension/extension/vscode-node/services.ts | Registers external context service in DI container |
| src/extension/extension/vscode-node/contributions.ts | Registers external context contribution |
| package.nls.json | Adds localization strings for new commands |
| package.json | Adds command definitions for external context management |
| replaceExternalPaths(paths: readonly URI[]): void { | ||
| this._paths.clear(); | ||
| for (const path of paths) { | ||
| this._paths.set(path.toString(), path); |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The replaceExternalPaths method does not enforce the MAX_EXTERNAL_PATHS limit, allowing more paths to be set than the maximum. This is inconsistent with addExternalPaths which respects the limit. Add a check to only add paths up to MAX_EXTERNAL_PATHS.
| this._paths.set(path.toString(), path); | |
| if (this._paths.size >= MAX_EXTERNAL_PATHS) { | |
| break; | |
| } | |
| const key = path.toString(); | |
| if (!this._paths.has(key)) { | |
| this._paths.set(key, path); | |
| } |
| if (potentialParent === child) { | ||
| return true; | ||
| } | ||
|
|
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The equality check at lines 115-117 is redundant since the same comparison is already performed by the caller at line 98 (candidateComparable === storedComparable). Remove this check to avoid duplicated logic.
| if (potentialParent === child) { | |
| return true; | |
| } |
| this.request = await this.sanitizeVariables(); | ||
| this.turn.request.message = this.request.prompt; | ||
|
|
||
|
|
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extra blank line at line 228 should be removed to maintain consistent spacing according to the project's coding style.
| private appendExternalContextReferences(): void { | ||
| const externalUris = this._externalContextService.getExternalPaths(); | ||
| if (!externalUris.length) { | ||
| return; | ||
| } | ||
|
|
||
| const existingRefs = this.request.references ?? []; | ||
|
|
||
| const newRefs: ChatPromptReference[] = []; | ||
| let counter = 0; | ||
| for (const uri of externalUris) { | ||
| const alreadyPresent = existingRefs.some(ref => this.matchesReference(ref, uri)) || newRefs.some(ref => this.matchesReference(ref, uri)); | ||
| if (!alreadyPresent) { | ||
| const id = `${EXTERNAL_CONTEXT_REFERENCE_PREFIX}.${counter++}`; | ||
| newRefs.push({ | ||
| id, | ||
| name: uri.fsPath, | ||
| value: uri, | ||
| modelDescription: `External context path ${uri.fsPath}`, | ||
| }); | ||
| } | ||
| } |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The nested loop at line 304 creates O(n*m) complexity checking if URIs are already present. The second check newRefs.some(ref => this.matchesReference(ref, uri)) is unnecessary since newRefs is being built in this loop and cannot contain duplicates from externalUris. Consider using a Set-based approach or remove the redundant newRefs.some() check.
Co-authored-by: Copilot <[email protected]>
…rnal context reference handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
| isExternalPath(uri: URI): boolean { | ||
| const candidateComparable = this.toComparablePath(uri); | ||
| for (const stored of this._paths.values()) { | ||
| const storedComparable = this.toComparablePath(stored); | ||
|
|
||
| if (candidateComparable === storedComparable) { | ||
| return true; | ||
| } | ||
|
|
||
| if (this.isSubPath(candidateComparable, storedComparable) || this.isSubPath(storedComparable, candidateComparable)) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| } |
Copilot
AI
Oct 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The toComparablePath method is called repeatedly inside the loop, converting the same stored paths on every iteration. Consider caching the comparable paths in a Map alongside the URIs to avoid redundant path normalization operations.
Problem
@workspaceis limited to opened folders. Full-stack and multiple repo developers cannot reference external code (e.g. backend, shared utils) without workspace reloads or manual#filedrags.Solution
github.copilot.chat.addExternalContextQuickPick + folder browseChatPromptReference(vscode.prompt.file.external)listDir,edit, etc. access external paths viaisExternalPathnode_modules,.git,dist,build,.log,.tmp+files.exclude$(folder) 2/3github.copilot.chat.manageExternalContextsremove via QuickPickEXTERNAL_CONTEXT_PATHS(fallback:SYSTEM_CONTEXT_PATHS)No impact on existing
@workspaceor open files.Implementation
ExternalContextService: path management, dedup, cap, eventsExternalContextContribution: commands, UI, badge, filteringchatParticipantRequestHandler,listDirTool,toolUtilsTesting
vitestsuite: add/cap/remove/eventsLimitations
→ Status bar + manage command provide full visibility and control.
Ready for review and merge.
All functionality delivered within open-source constraints.