Use extension handle to identify extensions in file-watcher#7224
Open
isaacroldan wants to merge 3 commits intomainfrom
Open
Use extension handle to identify extensions in file-watcher#7224isaacroldan wants to merge 3 commits intomainfrom
isaacroldan wants to merge 3 commits intomainfrom
Conversation
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
4 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the dev file-watcher event model to attribute file changes to extensions by extension handle rather than only by extension directory path, enabling correct handling of shared files across multiple extensions.
Changes:
- Adds
extensionHandle?: stringtoWatcherEventand threads it through event creation/deduping. - Changes watched-file ownership tracking to map file paths → extension handles.
- Updates watcher event handling logic (and tests) to support multiple events for shared files.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/app/src/cli/services/dev/app-events/file-watcher.ts | Emits watcher events with extensionHandle and tracks watched files by handle. |
| packages/app/src/cli/services/dev/app-events/file-watcher.test.ts | Adjusts tests to allow multiple emitted events for a single FS change. |
| packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts | Resolves affected extensions by extensionHandle when present, otherwise by directory. |
Comments suppressed due to low confidence (1)
packages/app/src/cli/services/dev/app-events/file-watcher.test.ts:383
- When
expectedEventCountis 2, the test still only asserts againstactualEvents[0]. Since the watcher can emit multiple events in either order (and the whole point of this PR is to differentiate them viaextensionHandle), this can become order-dependent/flaky and it also doesn't validate thatextensionHandleis set correctly. Prefer asserting thatactualEventscontains an event matchingexpectedEvent(and, when multiple events are expected, assert the full set ofextensionHandlevalues or the full list of expected events) rather than assuming index 0.
const calls = onChange.mock.calls
const actualEvents = calls.find((call) => call[0].length > 0)?.[0]
if (!actualEvents) {
throw new Error('Expected onChange to be called with events, but all calls had empty arrays')
}
const eventCount = expectedEventCount ?? 1
expect(actualEvents).toHaveLength(eventCount)
const actualEvent = actualEvents[0]
expect(actualEvent.type).toBe(expectedEvent.type)
expect(actualEvent.path).toBe(normalizePath(expectedEvent.path))
expect(actualEvent.extensionPath).toBe(normalizePath(expectedEvent.extensionPath))
expect(Array.isArray(actualEvent.startTime)).toBe(true)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
b5640a9 to
8053877
Compare
4 tasks
4 tasks
Co-authored-by: Claude Code <claude-code@anthropic.com>
8053877 to
cc5abaa
Compare
Co-authored-by: Claude Code <claude-code@anthropic.com>
6a83b0d to
7403589
Compare
Co-authored-by: Claude Code <claude-code@anthropic.com>
7403589 to
296ba04
Compare
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

WHY are these changes introduced?
The file watcher was using extension directory paths to identify which extensions are affected by file changes. This approach has limitations when dealing with shared files or when extensions need to be uniquely identified beyond their filesystem location.
WHAT is this pull request doing?
Refactors the file watcher system to use extension handles instead of directory paths for tracking file ownership and identifying affected extensions. The changes include:
extensionHandlefield toWatcherEventinterface to uniquely identify extensionsThe
extensionPathfield is retained for backward compatibility and folder-level events where no extension handle exists yet.How to test your changes?
Checklist
pnpm changeset add