Fix workspace watchers with Parcel watcher#1769
Open
zerob13 wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughReplaces Changes
Sequence Diagram(s)sequenceDiagram
participant WorkspacePresenter
participant FileWatcherService
participant WatcherPool
participant WatcherHostClient
participant UtilityProcess as FileWatcherHost (utility process)
participant Renderer as WorkspacePanel (renderer)
WorkspacePresenter->>FileWatcherService: watch(request, onBatch, onStatus)
FileWatcherService->>WatcherPool: watch(request, onBatch, onStatus)
WatcherPool->>WatcherHostClient: watch(request) via IPC RPC
WatcherHostClient->>UtilityProcess: fork + file-watcher:request watch
UtilityProcess-->>WatcherHostClient: file-watcher:status healthy/native
WatcherHostClient-->>WorkspacePresenter: onStatus(healthy)
WorkspacePresenter->>Renderer: sendToAllWindows workspace.watch.status.changed
UtilityProcess-->>WatcherHostClient: file-watcher:event-batch
WatcherHostClient-->>WatcherPool: onBatch(batch)
WatcherPool-->>WorkspacePresenter: filtered onBatch(batch)
WorkspacePresenter->>WorkspacePresenter: schedule invalidation (fs/git/full)
WorkspacePresenter->>Renderer: sendToAllWindows workspace.invalidated
UtilityProcess--xWatcherHostClient: unexpected exit
WatcherHostClient->>WatcherHostClient: handleHostExit → emitDegraded + scheduleRestart
WatcherHostClient-->>WorkspacePresenter: onStatus(degraded/utility-exit)
WorkspacePresenter->>Renderer: sendToAllWindows workspace.watch.status.changed (degraded)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/renderer/src/components/sidepanel/composables/useWorkspaceSync.ts (1)
421-427:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRegister watch-status listeners before immediate watcher startup.
Line [397] runs
ensureWatcherState(...)immediately, but the new watch-status listener is attached at Line [425]. Earlyworkspace.watch.status.changedevents can be missed, so initial degraded/failed state may never surface in the panel.Suggested fix
-import { computed, onBeforeUnmount, onMounted, ref, watch, type ComputedRef, type Ref } from 'vue' +import { computed, onBeforeUnmount, ref, watch, type ComputedRef, type Ref } from 'vue' - onMounted(() => { - stopWorkspaceInvalidatedListener = options.workspaceClient.onInvalidated( - handleWorkspaceInvalidated - ) - stopWorkspaceWatchStatusListener = options.workspaceClient.onWatchStatusChanged( - handleWorkspaceWatchStatusChanged - ) - }) + stopWorkspaceInvalidatedListener = options.workspaceClient.onInvalidated( + handleWorkspaceInvalidated + ) + stopWorkspaceWatchStatusListener = options.workspaceClient.onWatchStatusChanged( + handleWorkspaceWatchStatusChanged + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/src/components/sidepanel/composables/useWorkspaceSync.ts` around lines 421 - 427, The watch-status listener registration is happening too late in the initialization sequence. In the onMounted hook in useWorkspaceSync.ts, the stopWorkspaceWatchStatusListener assignment (which registers the handler for workspace watch-status changes) needs to be moved to occur before the ensureWatcherState function is called, as ensureWatcherState triggers the watcher immediately and could emit status-change events before the listener is attached, causing initial degraded or failed states to be missed by the panel.src/renderer/src/i18n/es-ES/chat.json (1)
1-1:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTranslate watchStatus strings into native languages.
All five non-English locale files contain English placeholder text for the new
workspace.files.watchStatuskeys instead of native-language translations:
src/renderer/src/i18n/es-ES/chat.json#L264-267: translate thedegradedandfailedstrings into Spanish.src/renderer/src/i18n/fa-IR/chat.json#L214-217: translate thedegradedandfailedstrings into Farsi/Persian.src/renderer/src/i18n/fr-FR/chat.json#L214-217: translate thedegradedandfailedstrings into French.src/renderer/src/i18n/he-IL/chat.json#L214-217: translate thedegradedandfailedstrings into Hebrew.src/renderer/src/i18n/id-ID/chat.json#L264-267: translate thedegradedandfailedstrings into Indonesian.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/renderer/src/i18n/es-ES/chat.json` at line 1, Replace the English placeholder text for the workspace.files.watchStatus keys in the five non-English locale files with native-language translations. In src/renderer/src/i18n/es-ES/chat.json (lines 264-267), translate the degraded and failed strings into Spanish. In src/renderer/src/i18n/fa-IR/chat.json (lines 214-217), translate the degraded and failed strings into Farsi/Persian. In src/renderer/src/i18n/fr-FR/chat.json (lines 214-217), translate the degraded and failed strings into French. In src/renderer/src/i18n/he-IL/chat.json (lines 214-217), translate the degraded and failed strings into Hebrew. In src/renderer/src/i18n/id-ID/chat.json (lines 264-267), translate the degraded and failed strings into Indonesian.Source: Coding guidelines
src/shared/types/presenters/workspace.d.ts (1)
97-102:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep
WorkspaceInvalidationEvent.versionrequired to match the contract.Line 101 makes
versionoptional, butworkspace.invalidateddefinesversionas required. This weakens type safety across the shared event boundary.Suggested fix
export type WorkspaceInvalidationEvent = { workspacePath: string kind: WorkspaceInvalidationKind source: WorkspaceInvalidationSource - version?: number + version: number }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shared/types/presenters/workspace.d.ts` around lines 97 - 102, The WorkspaceInvalidationEvent type marks the version field as optional with the ? modifier, but the workspace.invalidated contract requires version to be a non-optional required field. Remove the ? from the version property declaration in the WorkspaceInvalidationEvent type definition to make it a required field matching the actual contract requirement and restore type safety at the shared event boundary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/issues/parcel-watcher-issue-1764/plan.md`:
- Around line 210-227: The fallback modes section documents
`git-metadata-polling` as a polling strategy, but this mode is missing from the
earlier `WatcherMode` union type definition and the status-event example that
shows the `mode` field values as `native | snapshot-polling | lifecycle`. To fix
this inconsistency, either add `git-metadata-polling` to the `WatcherMode` union
and update the status-event example to include `git-metadata-polling` as a valid
mode value, or remove `git-metadata-polling` from the Fallback modes section if
it is intended to be internal only. Whichever approach you choose, ensure that
the `WatcherMode` contract, the status-event example, and the documented
fallback modes all describe the same set of possible modes.
In `@docs/issues/parcel-watcher-issue-1764/spec.md`:
- Line 138: Remove the phrase "currently the latest npm release" from line 138
in the spec.md file. Keep only the part about the recommended dependency version
with the semantic versioning constraint "`@parcel/watcher`@^2.5.6". This
eliminates time-bound language that will become outdated when newer versions are
released, while maintaining the useful version specification information.
In `@docs/issues/parcel-watcher-issue-1764/tasks.md`:
- Around line 17-23: The verification checklist includes `pnpm run typecheck`
but the acceptance gate specification requires `pnpm run typecheck:node`. Update
the task list to use the correct command `pnpm run typecheck:node` that matches
the spec, or if both commands are required, add a note clarifying that both
should be executed by contributors.
In `@src/main/lib/fileWatcher/eventCoalescer.ts`:
- Around line 57-69: The code is using path.normalize on lines 59 and 66 to
normalize paths in the deletedParents array and when checking descendants, but
this is weaker than the normalizeEventKey normalization used elsewhere. Replace
both occurrences of path.normalize(event.path) in the deletedParents mapping and
the normalized variable assignment in the filter function with
normalizeEventKey(event.path) to ensure consistent and stronger path
normalization that prevents missing equivalent paths and leaking redundant
delete events.
In `@src/main/lib/fileWatcher/watcherHost.ts`:
- Around line 166-179: After detecting and reporting the terminal `root-deleted`
state in the root path existence check (where fs.existsSync fails for
activeWatch.request.rootPath), prevent any future polling from being scheduled
for this activeWatch. Identify the polling scheduling mechanism (likely a
timeout or interval setup) and ensure it is cancelled or prevented from being
re-scheduled once the `root-deleted` condition is detected and the status is set
to failed. This issue appears at two locations: the primary check at lines
166-179 and a secondary occurrence at lines 215-218; both sites need the same
fix to prevent polling from continuing after the terminal failure state is
reached.
In `@src/main/lib/fileWatcher/watcherHostClient.ts`:
- Around line 91-115: The request method does not have a timeout mechanism,
which means if the utility host becomes unresponsive and never replies, the
promise will hang indefinitely and leave entries in the pendingRequests Map
unresolved. Add a timeout to the promise created in the request method that
rejects after a reasonable duration (typically a few seconds). When the timeout
fires, delete the corresponding entry from pendingRequests using the id as the
key and reject the promise with an appropriate timeout error. This ensures that
hung RPC calls are cleaned up and do not block watch, unwatch, or shutdown
operations.
In `@src/main/lib/fileWatcher/watcherPool.ts`:
- Around line 108-130: When the watch promise in `entry.ready` rejects, the
entry remains in both the `entriesByKey` and `entriesByWatchId` maps, causing
future callers with the same key to inherit the failed promise and keep failing.
Add error handling around the `await entry.ready` statement to catch rejection
and clean up by removing the poisoned entry from both `entriesByKey` and
`entriesByWatchId` maps before re-throwing the error, allowing subsequent
requests with the same key to create a fresh entry and retry.
In `@src/main/lib/fileWatcher/watcherService.ts`:
- Around line 34-35: In the resetFileWatcherServiceForTests function, before
setting sharedWatcherService to null, you need to properly clean up the existing
watcher service instance to prevent watcher clients and processes from remaining
alive across tests. Add logic to check if sharedWatcherService exists and call
its cleanup method (such as destroy() or close()) before assigning null to
sharedWatcherService.
In `@src/main/presenter/skillPresenter/index.ts`:
- Around line 1955-1961: The code in this section calls discoverSkills() which
already publishes the skills.catalog.changed event internally, and then
immediately publishes the same event again at lines 1956-1960 with duplicate
data. Remove the redundant publishDeepchatEvent call for skills.catalog.changed
since the discoverSkills() method already handles this publication. Keep the
const skills assignment and the return statement.
In `@src/main/presenter/workspacePresenter/index.ts`:
- Around line 230-233: The fire-and-forget calls to
`this.refreshGitWatcher(runtime)` at lines 232 and 242 lack rejection handlers,
which can cause unhandled promise rejections. Add a `.catch()` handler to both
invocations of `refreshGitWatcher(runtime)` to properly handle any promise
rejections, ensuring errors are either logged or handled gracefully rather than
becoming unhandled rejections that could destabilize the runtime.
- Around line 168-176: The runtime is being added to the watchRuntimes map at
the beginning of the async setup sequence, before createContentWatcher and
refreshGitWatcher complete. If either of these async operations throws an error,
the runtime remains in the map but with incomplete or failed initialization,
causing subsequent watchWorkspace calls to find it in the map and only increment
refCount instead of retrying setup. Move the this.watchRuntimes.set(normalized,
runtime) call to execute only after both createContentWatcher and
refreshGitWatcher have completed successfully, ensuring the insertion is atomic
with the full initialization. This way, if setup fails at any point, the runtime
is not added to the map and can be properly retried on the next watchWorkspace
call.
In `@src/renderer/src/i18n/da-DK/chat.json`:
- Around line 214-217: The watchStatus object containing "degraded" and "failed"
messages remain in English across multiple non-English locale files. Replace the
English strings with proper translations in each file: in
src/renderer/src/i18n/da-DK/chat.json at lines 214-217 replace with Danish
translations, in src/renderer/src/i18n/de-DE/chat.json at lines 264-267 replace
with German translations, in src/renderer/src/i18n/pt-BR/chat.json at lines
214-217 replace with Brazilian Portuguese translations, in
src/renderer/src/i18n/ru-RU/chat.json at lines 214-217 replace with Russian
translations, and in src/renderer/src/i18n/tr-TR/chat.json at lines 264-267
replace with Turkish translations. For each file, translate both the
watchStatus.degraded and watchStatus.failed string values into the corresponding
target language.
In `@src/renderer/src/i18n/it-IT/chat.json`:
- Around line 264-267: The workspace watch-status strings contain English
placeholder text in five locale files instead of proper translations. Translate
both the degraded and failed keys under workspace.files.watchStatus to the
appropriate language at each location: in src/renderer/src/i18n/it-IT/chat.json
(lines 264-267) translate to Italian, in src/renderer/src/i18n/ja-JP/chat.json
(lines 214-217) translate to Japanese, in src/renderer/src/i18n/ko-KR/chat.json
(lines 214-217) translate to Korean, in src/renderer/src/i18n/ms-MY/chat.json
(lines 264-267) translate to Malay, and in src/renderer/src/i18n/pl-PL/chat.json
(lines 264-267) translate to Polish. For each locale, replace the English text
for both degraded ("Watching in fallback mode. Changes may refresh slower.") and
failed ("File watching is unavailable. Refresh or reselect the workspace.") with
their native language equivalents.
In `@src/renderer/src/i18n/vi-VN/chat.json`:
- Around line 264-266: The watchStatus strings (degraded and failed) in the
locale files are using placeholder text that was not properly localized for each
target language. In src/renderer/src/i18n/vi-VN/chat.json at lines 264-266,
replace the English watchStatus.degraded and watchStatus.failed messages with
Vietnamese-localized translations. In src/renderer/src/i18n/zh-HK/chat.json at
lines 222-224, replace the Simplified Chinese watchStatus text with proper
Traditional Chinese (Hong Kong) phrasing. In
src/renderer/src/i18n/zh-TW/chat.json at lines 222-224, replace the Simplified
Chinese watchStatus text with proper Traditional Chinese (Taiwan) phrasing.
Ensure each locale has culturally appropriate and grammatically correct
translations for both the degraded and failed status messages.
---
Outside diff comments:
In `@src/renderer/src/components/sidepanel/composables/useWorkspaceSync.ts`:
- Around line 421-427: The watch-status listener registration is happening too
late in the initialization sequence. In the onMounted hook in
useWorkspaceSync.ts, the stopWorkspaceWatchStatusListener assignment (which
registers the handler for workspace watch-status changes) needs to be moved to
occur before the ensureWatcherState function is called, as ensureWatcherState
triggers the watcher immediately and could emit status-change events before the
listener is attached, causing initial degraded or failed states to be missed by
the panel.
In `@src/renderer/src/i18n/es-ES/chat.json`:
- Line 1: Replace the English placeholder text for the
workspace.files.watchStatus keys in the five non-English locale files with
native-language translations. In src/renderer/src/i18n/es-ES/chat.json (lines
264-267), translate the degraded and failed strings into Spanish. In
src/renderer/src/i18n/fa-IR/chat.json (lines 214-217), translate the degraded
and failed strings into Farsi/Persian. In src/renderer/src/i18n/fr-FR/chat.json
(lines 214-217), translate the degraded and failed strings into French. In
src/renderer/src/i18n/he-IL/chat.json (lines 214-217), translate the degraded
and failed strings into Hebrew. In src/renderer/src/i18n/id-ID/chat.json (lines
264-267), translate the degraded and failed strings into Indonesian.
In `@src/shared/types/presenters/workspace.d.ts`:
- Around line 97-102: The WorkspaceInvalidationEvent type marks the version
field as optional with the ? modifier, but the workspace.invalidated contract
requires version to be a non-optional required field. Remove the ? from the
version property declaration in the WorkspaceInvalidationEvent type definition
to make it a required field matching the actual contract requirement and restore
type safety at the shared event boundary.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dbc3bff8-cecf-4d26-b6b2-d1d43a7873f2
📒 Files selected for processing (56)
docs/issues/parcel-watcher-issue-1764/plan.mddocs/issues/parcel-watcher-issue-1764/spec.mddocs/issues/parcel-watcher-issue-1764/tasks.mdelectron-builder.ymlelectron.vite.config.tspackage.jsonscripts/afterPack.jssrc/main/fileWatcherUtilityHostEntry.tssrc/main/lib/fileWatcher/eventCoalescer.tssrc/main/lib/fileWatcher/fileWatcherUtilityHost.tssrc/main/lib/fileWatcher/index.tssrc/main/lib/fileWatcher/watcherHost.tssrc/main/lib/fileWatcher/watcherHostClient.tssrc/main/lib/fileWatcher/watcherPool.tssrc/main/lib/fileWatcher/watcherService.tssrc/main/lib/fileWatcher/watcherTypes.tssrc/main/presenter/index.tssrc/main/presenter/skillPresenter/index.tssrc/main/presenter/workspacePresenter/index.tssrc/renderer/api/WorkspaceClient.tssrc/renderer/src/components/sidepanel/WorkspacePanel.vuesrc/renderer/src/components/sidepanel/composables/useWorkspaceSync.tssrc/renderer/src/i18n/da-DK/chat.jsonsrc/renderer/src/i18n/de-DE/chat.jsonsrc/renderer/src/i18n/en-US/chat.jsonsrc/renderer/src/i18n/es-ES/chat.jsonsrc/renderer/src/i18n/fa-IR/chat.jsonsrc/renderer/src/i18n/fr-FR/chat.jsonsrc/renderer/src/i18n/he-IL/chat.jsonsrc/renderer/src/i18n/id-ID/chat.jsonsrc/renderer/src/i18n/it-IT/chat.jsonsrc/renderer/src/i18n/ja-JP/chat.jsonsrc/renderer/src/i18n/ko-KR/chat.jsonsrc/renderer/src/i18n/ms-MY/chat.jsonsrc/renderer/src/i18n/pl-PL/chat.jsonsrc/renderer/src/i18n/pt-BR/chat.jsonsrc/renderer/src/i18n/ru-RU/chat.jsonsrc/renderer/src/i18n/tr-TR/chat.jsonsrc/renderer/src/i18n/vi-VN/chat.jsonsrc/renderer/src/i18n/zh-CN/chat.jsonsrc/renderer/src/i18n/zh-HK/chat.jsonsrc/renderer/src/i18n/zh-TW/chat.jsonsrc/shared/contracts/domainSchemas.tssrc/shared/contracts/events.tssrc/shared/contracts/events/workspace.events.tssrc/shared/types/presenters/index.d.tssrc/shared/types/presenters/workspace.d.tssrc/shared/types/skill.tstest/e2e/specs/30-workspace-watcher-events.smoke.spec.tstest/main/lib/fileWatcher/eventCoalescer.test.tstest/main/lib/fileWatcher/watcherPool.test.tstest/main/presenter/skillPresenter/skillPresenter.test.tstest/main/presenter/workspacePresenter.test.tstest/main/routes/contracts.test.tstest/main/scripts/afterPack.test.tstest/renderer/components/WorkspacePanel.test.ts
Comment on lines
+210
to
+227
| Fallback modes: | ||
|
|
||
| - `snapshot-polling`: use `@parcel/watcher.writeSnapshot()` and `getEventsSince()` from the | ||
| watcher host on a 5000 ms interval for workspace content. | ||
| - `git-metadata-polling`: stat `HEAD`, `index`, `packed-refs`, and scan `refs` mtimes from the git | ||
| watcher host on a 1000 ms interval. | ||
| - `lifecycle`: emit a full fallback invalidation when the workspace panel activates or the | ||
| workspace path changes. | ||
|
|
||
| Degraded mode emits a typed status event: | ||
|
|
||
| ```text | ||
| workspace.watch.status.changed | ||
| workspacePath | ||
| mode: native | snapshot-polling | lifecycle | ||
| health: healthy | degraded | failed | ||
| reason | ||
| ``` |
Contributor
There was a problem hiding this comment.
Keep the fallback mode contract consistent.
git-metadata-polling appears here, but the earlier WatcherMode union and status-event example only cover native, snapshot-polling, and lifecycle. Please either add git-metadata-polling to the shared contract or keep it internal; otherwise the renderer state model won’t match the documented fallback behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/issues/parcel-watcher-issue-1764/plan.md` around lines 210 - 227, The
fallback modes section documents `git-metadata-polling` as a polling strategy,
but this mode is missing from the earlier `WatcherMode` union type definition
and the status-event example that shows the `mode` field values as `native |
snapshot-polling | lifecycle`. To fix this inconsistency, either add
`git-metadata-polling` to the `WatcherMode` union and update the status-event
example to include `git-metadata-polling` as a valid mode value, or remove
`git-metadata-polling` from the Fallback modes section if it is intended to be
internal only. Whichever approach you choose, ensure that the `WatcherMode`
contract, the status-event example, and the documented fallback modes all
describe the same set of possible modes.
|
|
||
| ## Review Decisions | ||
|
|
||
| - Recommended dependency version: `@parcel/watcher@^2.5.6`, currently the latest npm release. |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n docs/issues/parcel-watcher-issue-1764/spec.mdRepository: ThinkInAIXYZ/deepchat
Length of output: 8662
🌐 Web query:
@parcel/watcher npm latest version release history
💡 Result:
As of June 15, 2026, the latest version of @parcel/watcher on npm is 2.5.6, which was published on January 23, 2026 [1][2]. Release history summary: - The package was first published on December 18, 2018 [1]. - It has seen consistent updates throughout its lifecycle, with the 2.x series beginning around October 2021 [1]. - Recent releases leading up to the current version include: - v2.5.6 (January 23, 2026): Added stack-protector-strong flag and fixed a Windows ia32 build issue [2]. - v2.5.4 (January 11, 2026): Included various engineering updates and test improvements [3]. The package maintains a history of 39 versions as of this date [1][4].
Citations:
- 1: https://www.npmjs.com/package/@parcel/watcher
- 2: https://github.com/parcel-bundler/watcher/releases/tag/v2.5.6
- 3: https://github.com/parcel-bundler/watcher/releases/tag/v2.5.4
- 4: https://npmx.dev/package/@parcel/watcher/versions
Remove "currently the latest npm release" to avoid time-bound language.
Line 138 uses language that will become stale as soon as a new version is released. Since the spec uses semantic versioning (^2.5.6), rephrase to reference only the version chosen for this implementation without claiming it's currently latest, for example: "Recommended dependency version: @parcel/watcher@^2.5.6."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/issues/parcel-watcher-issue-1764/spec.md` at line 138, Remove the phrase
"currently the latest npm release" from line 138 in the spec.md file. Keep only
the part about the recommended dependency version with the semantic versioning
constraint "`@parcel/watcher`@^2.5.6". This eliminates time-bound language that
will become outdated when newer versions are released, while maintaining the
useful version specification information.
Comment on lines
+17
to
+23
| - `pnpm run format` | ||
| - `pnpm run i18n` | ||
| - `pnpm run lint` | ||
| - `pnpm run typecheck` | ||
| - `pnpm test` | ||
| - `pnpm run build` | ||
| - `pnpm exec playwright test -c test/e2e/playwright.config.ts test/e2e/specs/30-workspace-watcher-events.smoke.spec.ts` |
Contributor
There was a problem hiding this comment.
Align the verification checklist with the acceptance gate.
The spec calls for pnpm run typecheck:node, but this checklist records pnpm run typecheck. Please make the task list match the command you expect contributors to run, or note that both are required.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/issues/parcel-watcher-issue-1764/tasks.md` around lines 17 - 23, The
verification checklist includes `pnpm run typecheck` but the acceptance gate
specification requires `pnpm run typecheck:node`. Update the task list to use
the correct command `pnpm run typecheck:node` that matches the spec, or if both
commands are required, add a note clarifying that both should be executed by
contributors.
Comment on lines
+57
to
+69
| const deletedParents = mergedEvents | ||
| .filter((event) => event.type === 'delete') | ||
| .map((event) => path.normalize(event.path)) | ||
|
|
||
| return mergedEvents.filter((event) => { | ||
| if (event.type !== 'delete') { | ||
| return true | ||
| } | ||
|
|
||
| const normalized = path.normalize(event.path) | ||
| return !deletedParents.some( | ||
| (deletedParent) => deletedParent !== normalized && isDescendantOf(normalized, deletedParent) | ||
| ) |
Contributor
There was a problem hiding this comment.
Use normalizeEventKey for descendant-delete filtering too.
Line 59 and Line 66 use path.normalize, which is weaker than the merge-key normalization. That can miss dedupe for equivalent paths and leak redundant delete events.
Suggested fix
const mergedEvents = Array.from(byPath.values())
const deletedParents = mergedEvents
.filter((event) => event.type === 'delete')
- .map((event) => path.normalize(event.path))
+ .map((event) => normalizeEventKey(event.path))
return mergedEvents.filter((event) => {
if (event.type !== 'delete') {
return true
}
- const normalized = path.normalize(event.path)
+ const normalized = normalizeEventKey(event.path)
return !deletedParents.some(
(deletedParent) => deletedParent !== normalized && isDescendantOf(normalized, deletedParent)
)
})🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/lib/fileWatcher/eventCoalescer.ts` around lines 57 - 69, The code is
using path.normalize on lines 59 and 66 to normalize paths in the deletedParents
array and when checking descendants, but this is weaker than the
normalizeEventKey normalization used elsewhere. Replace both occurrences of
path.normalize(event.path) in the deletedParents mapping and the normalized
variable assignment in the filter function with normalizeEventKey(event.path) to
ensure consistent and stronger path normalization that prevents missing
equivalent paths and leaking redundant delete events.
Comment on lines
+166
to
+179
| if (!fs.existsSync(activeWatch.request.rootPath)) { | ||
| this.enqueueEvents(activeWatch, [ | ||
| { | ||
| path: activeWatch.request.rootPath, | ||
| type: 'root-deleted' | ||
| } | ||
| ]) | ||
| this.sendStatus(activeWatch, { | ||
| health: 'failed', | ||
| mode: activeWatch.mode, | ||
| reason: 'root-deleted' | ||
| }) | ||
| return | ||
| } |
Contributor
There was a problem hiding this comment.
Stop polling after terminal root-deleted state to avoid repeated failure churn.
When rootPath is missing, the host emits root-deleted/failed but keeps scheduling polling, which can repeatedly fire the same failure path every interval and retrigger downstream invalidation work.
Suggested fix
@@
- void poll()
- activeWatch.pollTimer = setInterval(() => {
+ activeWatch.pollTimer = setInterval(() => {
void poll()
}, SNAPSHOT_POLL_INTERVAL_MS)
+ void poll()
@@
if (!fs.existsSync(activeWatch.request.rootPath)) {
this.enqueueEvents(activeWatch, [
{
path: activeWatch.request.rootPath,
type: 'root-deleted'
}
])
this.sendStatus(activeWatch, {
health: 'failed',
mode: activeWatch.mode,
reason: 'root-deleted'
})
+ if (activeWatch.pollTimer) {
+ clearInterval(activeWatch.pollTimer)
+ activeWatch.pollTimer = null
+ }
return
}Also applies to: 215-218
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/lib/fileWatcher/watcherHost.ts` around lines 166 - 179, After
detecting and reporting the terminal `root-deleted` state in the root path
existence check (where fs.existsSync fails for activeWatch.request.rootPath),
prevent any future polling from being scheduled for this activeWatch. Identify
the polling scheduling mechanism (likely a timeout or interval setup) and ensure
it is cancelled or prevented from being re-scheduled once the `root-deleted`
condition is detected and the status is set to failed. This issue appears at two
locations: the primary check at lines 166-179 and a secondary occurrence at
lines 215-218; both sites need the same fix to prevent polling from continuing
after the terminal failure state is reached.
Comment on lines
168
to
176
| this.watchRuntimes.set(normalized, runtime) | ||
| runtime.contentWatcher = await this.createContentWatcher(normalized) | ||
| if (runtime.disposed || this.watchRuntimes.get(normalized) !== runtime) { | ||
| await runtime.contentWatcher.close() | ||
| runtime.contentWatcher = null | ||
| return | ||
| } | ||
| await this.refreshGitWatcher(runtime) | ||
| } |
Contributor
There was a problem hiding this comment.
Make watcher startup atomic to avoid wedged runtimes.
At Line 168, runtime is inserted into watchRuntimes before async setup. If createContentWatcher (Line 169) or refreshGitWatcher (Line 175) throws, the map retains a runtime with no active watchers; later watchWorkspace calls hit Line 150-154 and only increment refCount, so watching may never recover.
Suggested fix
- this.watchRuntimes.set(normalized, runtime)
- runtime.contentWatcher = await this.createContentWatcher(normalized)
- if (runtime.disposed || this.watchRuntimes.get(normalized) !== runtime) {
- await runtime.contentWatcher.close()
- runtime.contentWatcher = null
- return
- }
- await this.refreshGitWatcher(runtime)
+ this.watchRuntimes.set(normalized, runtime)
+ try {
+ runtime.contentWatcher = await this.createContentWatcher(normalized)
+ if (runtime.disposed || this.watchRuntimes.get(normalized) !== runtime) {
+ await runtime.contentWatcher.close()
+ runtime.contentWatcher = null
+ return
+ }
+ await this.refreshGitWatcher(runtime)
+ } catch (error) {
+ this.watchRuntimes.delete(normalized)
+ await this.disposeRuntime(runtime)
+ throw error
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/presenter/workspacePresenter/index.ts` around lines 168 - 176, The
runtime is being added to the watchRuntimes map at the beginning of the async
setup sequence, before createContentWatcher and refreshGitWatcher complete. If
either of these async operations throws an error, the runtime remains in the map
but with incomplete or failed initialization, causing subsequent watchWorkspace
calls to find it in the map and only increment refCount instead of retrying
setup. Move the this.watchRuntimes.set(normalized, runtime) call to execute only
after both createContentWatcher and refreshGitWatcher have completed
successfully, ensuring the insertion is atomic with the full initialization.
This way, if setup fails at any point, the runtime is not added to the map and
can be properly retried on the next watchWorkspace call.
Comment on lines
+230
to
+233
| for (const event of batch.events) { | ||
| if (event.type === 'overflow' || event.type === 'root-deleted') { | ||
| void this.refreshGitWatcher(runtime) | ||
| this.scheduleInvalidation(runtime, 'full', source) |
Contributor
There was a problem hiding this comment.
Catch refreshGitWatcher failures in fire-and-forget paths.
Line 232 and Line 242 call void this.refreshGitWatcher(runtime) without a rejection handler. If that promise rejects, it can become an unhandled rejection and destabilize shutdown/runtime behavior.
Suggested fix
+ private refreshGitWatcherSafely(runtime: WorkspaceWatchRuntime): void {
+ void this.refreshGitWatcher(runtime).catch((error) => {
+ console.warn('[Workspace] Failed to refresh git watcher', {
+ workspacePath: runtime.workspacePath,
+ error
+ })
+ })
+ }
...
- void this.refreshGitWatcher(runtime)
+ this.refreshGitWatcherSafely(runtime)
this.scheduleInvalidation(runtime, 'full', source)
return
...
- void this.refreshGitWatcher(runtime)
+ this.refreshGitWatcherSafely(runtime)
this.scheduleInvalidation(runtime, 'full', source)
returnAlso applies to: 241-243
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/presenter/workspacePresenter/index.ts` around lines 230 - 233, The
fire-and-forget calls to `this.refreshGitWatcher(runtime)` at lines 232 and 242
lack rejection handlers, which can cause unhandled promise rejections. Add a
`.catch()` handler to both invocations of `refreshGitWatcher(runtime)` to
properly handle any promise rejections, ensuring errors are either logged or
handled gracefully rather than becoming unhandled rejections that could
destabilize the runtime.
Comment on lines
+214
to
217
| "watchStatus": { | ||
| "degraded": "Watching in fallback mode. Changes may refresh slower.", | ||
| "failed": "File watching is unavailable. Refresh or reselect the workspace." | ||
| } |
Contributor
There was a problem hiding this comment.
Local watch-status strings are still English in non-English locale bundles.
The same English fallback/failed messages were inserted in multiple locale files, so localized UIs will display mixed-language status text.
src/renderer/src/i18n/da-DK/chat.json#L214-L217: replace bothwatchStatusvalues with Danish translations.src/renderer/src/i18n/de-DE/chat.json#L264-L267: replace bothwatchStatusvalues with German translations.src/renderer/src/i18n/pt-BR/chat.json#L214-L217: replace bothwatchStatusvalues with Brazilian Portuguese translations.src/renderer/src/i18n/ru-RU/chat.json#L214-L217: replace bothwatchStatusvalues with Russian translations.src/renderer/src/i18n/tr-TR/chat.json#L264-L267: replace bothwatchStatusvalues with Turkish translations.
📍 Affects 5 files
src/renderer/src/i18n/da-DK/chat.json#L214-L217(this comment)src/renderer/src/i18n/de-DE/chat.json#L264-L267src/renderer/src/i18n/pt-BR/chat.json#L214-L217src/renderer/src/i18n/ru-RU/chat.json#L214-L217src/renderer/src/i18n/tr-TR/chat.json#L264-L267
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/src/i18n/da-DK/chat.json` around lines 214 - 217, The
watchStatus object containing "degraded" and "failed" messages remain in English
across multiple non-English locale files. Replace the English strings with
proper translations in each file: in src/renderer/src/i18n/da-DK/chat.json at
lines 214-217 replace with Danish translations, in
src/renderer/src/i18n/de-DE/chat.json at lines 264-267 replace with German
translations, in src/renderer/src/i18n/pt-BR/chat.json at lines 214-217 replace
with Brazilian Portuguese translations, in src/renderer/src/i18n/ru-RU/chat.json
at lines 214-217 replace with Russian translations, and in
src/renderer/src/i18n/tr-TR/chat.json at lines 264-267 replace with Turkish
translations. For each file, translate both the watchStatus.degraded and
watchStatus.failed string values into the corresponding target language.
Comment on lines
+264
to
267
| "watchStatus": { | ||
| "degraded": "Watching in fallback mode. Changes may refresh slower.", | ||
| "failed": "File watching is unavailable. Refresh or reselect the workspace." | ||
| } |
Contributor
There was a problem hiding this comment.
Missing translations for workspace watch-status strings across five locales. All five non-English locale files contain English placeholder text for workspace.files.watchStatus.degraded and workspace.files.watchStatus.failed. Users who have selected these locales will see English text in the workspace watch-status banner instead of their expected language.
src/renderer/src/i18n/it-IT/chat.json#L264-L267: translate both keys to Italiansrc/renderer/src/i18n/ja-JP/chat.json#L214-L217: translate both keys to Japanesesrc/renderer/src/i18n/ko-KR/chat.json#L214-L217: translate both keys to Koreansrc/renderer/src/i18n/ms-MY/chat.json#L264-L267: translate both keys to Malaysrc/renderer/src/i18n/pl-PL/chat.json#L264-L267: translate both keys to Polish
📍 Affects 5 files
src/renderer/src/i18n/it-IT/chat.json#L264-L267(this comment)src/renderer/src/i18n/ja-JP/chat.json#L214-L217src/renderer/src/i18n/ko-KR/chat.json#L214-L217src/renderer/src/i18n/ms-MY/chat.json#L264-L267src/renderer/src/i18n/pl-PL/chat.json#L264-L267
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/src/i18n/it-IT/chat.json` around lines 264 - 267, The workspace
watch-status strings contain English placeholder text in five locale files
instead of proper translations. Translate both the degraded and failed keys
under workspace.files.watchStatus to the appropriate language at each location:
in src/renderer/src/i18n/it-IT/chat.json (lines 264-267) translate to Italian,
in src/renderer/src/i18n/ja-JP/chat.json (lines 214-217) translate to Japanese,
in src/renderer/src/i18n/ko-KR/chat.json (lines 214-217) translate to Korean, in
src/renderer/src/i18n/ms-MY/chat.json (lines 264-267) translate to Malay, and in
src/renderer/src/i18n/pl-PL/chat.json (lines 264-267) translate to Polish. For
each locale, replace the English text for both degraded ("Watching in fallback
mode. Changes may refresh slower.") and failed ("File watching is unavailable.
Refresh or reselect the workspace.") with their native language equivalents.
Comment on lines
+264
to
+266
| "watchStatus": { | ||
| "degraded": "Watching in fallback mode. Changes may refresh slower.", | ||
| "failed": "File watching is unavailable. Refresh or reselect the workspace." |
Contributor
There was a problem hiding this comment.
Localize new watchStatus strings per target locale. The same root issue appears in multiple locale packs: copied fallback text was not translated/adapted for each locale.
src/renderer/src/i18n/vi-VN/chat.json#L264-L266: replace EnglishwatchStatus.degraded/failedmessages with Vietnamese-localized text.src/renderer/src/i18n/zh-HK/chat.json#L222-L224: replace Simplified ChinesewatchStatustext with Traditional Chinese (Hong Kong) phrasing.src/renderer/src/i18n/zh-TW/chat.json#L222-L224: replace Simplified ChinesewatchStatustext with Traditional Chinese (Taiwan) phrasing.
📍 Affects 3 files
src/renderer/src/i18n/vi-VN/chat.json#L264-L266(this comment)src/renderer/src/i18n/zh-HK/chat.json#L222-L224src/renderer/src/i18n/zh-TW/chat.json#L222-L224
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/renderer/src/i18n/vi-VN/chat.json` around lines 264 - 266, The
watchStatus strings (degraded and failed) in the locale files are using
placeholder text that was not properly localized for each target language. In
src/renderer/src/i18n/vi-VN/chat.json at lines 264-266, replace the English
watchStatus.degraded and watchStatus.failed messages with Vietnamese-localized
translations. In src/renderer/src/i18n/zh-HK/chat.json at lines 222-224, replace
the Simplified Chinese watchStatus text with proper Traditional Chinese (Hong
Kong) phrasing. In src/renderer/src/i18n/zh-TW/chat.json at lines 222-224,
replace the Simplified Chinese watchStatus text with proper Traditional Chinese
(Taiwan) phrasing. Ensure each locale has culturally appropriate and
grammatically correct translations for both the degraded and failed status
messages.
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
1 participant
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.
Summary
Closes #1764.
@parcel/watcherservice.Technical Design
WatcherServiceis the feature-facing facade. Presenters subscribe throughWatchRequestobjects and do not import native watcher code.WatcherHostClientowns utility-process lifecycle, RPC correlation, restart status, and request replay.WatcherPooldeduplicates equivalent subscriptions, normalizes macOS/private/varpaths, applies include/exclude filters, and fans batches out to subscribers.watcherHostruns inside the utility process, owns@parcel/watchersubscriptions, coalesces regular content events, chunks large batches, and switches to snapshot polling when native watching fails or overflows.Validation
pnpm run formatpnpm run i18npnpm run lintpnpm run typecheckpnpm test(388 files passed, 3222 tests passed; existing skipped suites unchanged)pnpm run buildpnpm exec playwright test -c test/e2e/playwright.config.ts test/e2e/specs/30-workspace-watcher-events.smoke.spec.tsSummary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation
Tests