feat: detect VS Code location settings in readiness checks#98
Conversation
- Parse chat.instructionsFilesLocations, chat.agentFilesLocations, and chat.agentSkillsLocations from .vscode/settings.json and *.code-workspace - Extend stripJsonComments with trailing comma support for JSONC - Fix file-based instructions without areas reporting as pass - Delete monolithic readiness.ts, activate modular readiness/ directory - Add 14 new tests (11 VS Code locations + 3 JSONC trailing commas) - Reject path traversal, absolute paths, and repo root in location settings
There was a problem hiding this comment.
Pull request overview
Adds support for discovering Copilot/agent instruction, agent, and skill locations from VS Code workspace settings as part of readiness checks, and improves JSONC parsing to match VS Code’s common file formats (comments + trailing commas).
Changes:
- Parse
chat.*Locationsfrom.vscode/settings.jsonand root*.code-workspacefiles and thread the results into readiness checks. - Extend JSONC parsing (
stripJsonComments) to also remove trailing commas. - Refactor readiness implementation by removing the legacy monolithic
readiness.tsin favor of the modularservices/readiness/implementation; add tests for the new behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/tests/readiness.test.ts | Adds readiness tests covering VS Code location settings for instructions/agents/skills discovery. |
| src/services/tests/fs.test.ts | Adds unit tests validating trailing-comma stripping behavior in JSONC parsing. |
| packages/core/src/utils/fs.ts | Extends stripJsonComments to strip trailing commas in addition to comments (JSONC compatibility). |
| packages/core/src/services/readiness/types.ts | Adds VscodeLocationSettings and threads it through ReadinessContext. |
| packages/core/src/services/readiness/index.ts | Populates vscodeLocations in readiness context via parseVscodeLocations. |
| packages/core/src/services/readiness/criteria.ts | Uses VS Code-derived locations for custom-instructions, custom-agents, and copilot-skills. |
| packages/core/src/services/readiness/checkers.ts | Implements VS Code settings parsing + validation and extends checkers to accept extra directories. |
| packages/core/src/services/readiness.ts | Deletes the legacy monolithic readiness implementation. |
| if (!trimmed || path.isAbsolute(trimmed)) return undefined; | ||
| const segments = trimmed.split(/[/\\]+/u); | ||
| if (segments.some((s) => s === "..")) return undefined; | ||
| let normalized = path.normalize(trimmed).replace(/\\/gu, "/"); | ||
| normalized = normalized.replace(/\/+$/u, ""); | ||
| if (!normalized || normalized === ".") return undefined; |
There was a problem hiding this comment.
validateAndNormalize() can incorrectly accept Windows-style absolute paths when running on POSIX. Example: a setting path like \\etc\\agents is not considered absolute by path.isAbsolute() on POSIX, but the later replace(/\\/g, "/") turns it into /etc/agents (absolute). That would make subsequent path.join(repoPath, dir) ignore repoPath and allow filesystem access outside the repo. Consider normalizing separators before the absolute-path check and/or rejecting any normalized path that starts with / or \\, and add a test case for leading-backslash inputs.
| if (!trimmed || path.isAbsolute(trimmed)) return undefined; | |
| const segments = trimmed.split(/[/\\]+/u); | |
| if (segments.some((s) => s === "..")) return undefined; | |
| let normalized = path.normalize(trimmed).replace(/\\/gu, "/"); | |
| normalized = normalized.replace(/\/+$/u, ""); | |
| if (!normalized || normalized === ".") return undefined; | |
| if (!trimmed) return undefined; | |
| // Normalize separators first so absolute-path detection is reliable across platforms. | |
| const sepNormalized = trimmed.replace(/\\/gu, "/"); | |
| // Reject absolute or root-based paths (POSIX, Windows drive letters, UNC, etc.). | |
| if (path.isAbsolute(sepNormalized) || sepNormalized.startsWith("/")) return undefined; | |
| const segments = sepNormalized.split(/\/+/u); | |
| if (segments.some((s) => s === "..")) return undefined; | |
| let normalized = path.normalize(sepNormalized).replace(/\\/gu, "/"); | |
| normalized = normalized.replace(/\/+$/u, ""); | |
| if (!normalized || normalized === "." || normalized.startsWith("/")) return undefined; |
| @@ -321,6 +324,15 @@ export function buildCriteria(): ReadinessCriterion[] { | |||
| }; | |||
There was a problem hiding this comment.
In custom-instructions, fileBasedInstructions now includes .instructions.md files from VS Code chat.instructionsFilesLocations, but the surrounding comment and success reason still describe these as "area instruction(s)". This can be misleading when the extra locations are not area-specific. Consider updating the wording (and possibly the variable name) to reflect "file-based instructions" rather than "area instructions" when non-.github/instructions/ locations are included.
…de-location-settings # Conflicts: # packages/core/src/services/readiness.ts
Summary
Adds detection of VS Code
chat.*Locationssettings (instructionsFilesLocations,agentFilesLocations,agentSkillsLocations) from.vscode/settings.jsonand*.code-workspacefiles in readiness checks.Changes
custom-instructions,custom-agents, andcopilot-skillsreadiness criteriastripJsonCommentsto handle trailing commas (common in VS Code settings files)readiness.ts(1381 lines), activating the equivalent modularreadiness/directorySecurity
..) and absolute paths rejected viavalidateAndNormalize.) rejected to prevent false-positive detection