Skip to content

Fix invalid json files detected for claude hook paths#312016

Open
pwang347 wants to merge 3 commits intomainfrom
pawang/fixHooksPath
Open

Fix invalid json files detected for claude hook paths#312016
pwang347 wants to merge 3 commits intomainfrom
pawang/fixHooksPath

Conversation

@pwang347
Copy link
Copy Markdown
Member

@pwang347 pwang347 commented Apr 22, 2026

Refactor concept of parent to be a searchRoot. In the case of hook files, we just need to watch specific files (settings.json) not folders.

Fixes #304314

Copilot AI review requested due to automatic review settings April 22, 2026 21:47
@pwang347 pwang347 requested a review from aeschli April 22, 2026 21:50
@pwang347 pwang347 marked this pull request as ready for review April 22, 2026 21:50
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 22, 2026

Screenshot Changes

Base: 1f9cd94d Current: 711db634

Changed (1)

chat/aiCustomizations/aiCustomizationManagementEditor/McpBrowseMode/Light
Before After
before after

@pwang347 pwang347 enabled auto-merge (squash) April 22, 2026 21:50
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 fixes hook file discovery so that configured specific .json hook paths (notably Claude’s .claude/settings*.json) are treated as individual files rather than causing sibling .json files in the same directory to be picked up.

Changes:

  • Adjust hook location resolution to use a searchRoot concept (file or directory) instead of always using a “parent folder”, preventing accidental inclusion of sibling .json files.
  • Update consumers of the resolved-folder metadata to use searchRoot.
  • Add tests validating that hook discovery returns only targeted .json files (and not sibling .json files).
Show a summary per file
File Description
src/vs/workbench/contrib/chat/test/common/promptSyntax/utils/promptFilesLocator.test.ts Adds coverage for hook discovery to ensure only configured .json files are returned.
src/vs/workbench/contrib/chat/common/promptSyntax/utils/promptFilesLocator.ts Changes resolution and scanning logic to use searchRoot for hook paths (file vs directory).
src/vs/workbench/contrib/chat/common/promptSyntax/config/promptFileLocations.ts Renames resolved-folder metadata from parent to searchRoot.
src/vs/workbench/contrib/chat/browser/promptSyntax/pickers/askForPromptSourceFolder.ts Updates picker logic to reference searchRoot instead of the removed parent property.

Copilot's findings

Comments suppressed due to low confidence (1)

src/vs/workbench/contrib/chat/common/promptSyntax/utils/promptFilesLocator.ts:872

  • The JSDoc for resolveSearchLocation still references the old getParentFolder name and { parent, filePattern } shape in the example. Please update the example and wording to use resolveSearchLocation(...) and { searchRoot, filePattern } so the docs match the actual API.
 * assert.strictDeepEqual(
 *     getParentFolder(PromptsType.prompt, URI.file('/home/user/{folder1,folder2}/file.md')),
 *     { parent: URI.file('/home/user'), filePattern: '{folder1,folder2}/file.md' },
 *     'Must find correct non-glob parent dirname.',
 * );
  • Files reviewed: 4/4 changed files
  • Comments generated: 1

Comment thread src/vs/workbench/contrib/chat/common/promptSyntax/utils/promptFilesLocator.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Load hooks for claude CLI looks incorrect

2 participants