refactor(agent-core-v2): unify fs watching into a single xstate watch service - #3502
Conversation
|
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f29952f8bf
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "optionalDependencies": { | ||
| "@parcel/watcher-darwin-arm64": "2.5.1", | ||
| "@parcel/watcher-darwin-x64": "2.5.1", | ||
| "@parcel/watcher-linux-arm64-glibc": "2.5.1", |
There was a problem hiding this comment.
For users installing @moonshot-ai/kimi-code from npm, these optional dependencies are never installed: agent-core-v2 is private and bundled into apps/kimi-code/dist/main.mjs, while the published package contains only the app's own dependency manifest. The computed runtime require('@parcel/watcher-…') therefore fails and every recursive Linux watch falls back to chokidar, leaving the advertised single-inotify fix unavailable outside the SEA build. Declare or ship the platform bindings from the published CLI package as well.
Useful? React with 👍 / 👎.
| const ignorePaths = resolveIgnorePaths(root, options?.ignorePaths); | ||
| this.subscribeOptions = { | ||
| backend: 'inotify', | ||
| ...(ignorePaths.length > 0 ? { ignorePaths } : {}), |
There was a problem hiding this comment.
This conditional spread violates the repository rule for optional object properties. Assign ignorePaths directly, using undefined when it should be absent, rather than conditionally spreading a temporary object.
AGENTS.md reference: AGENTS.md:L52-L54
Useful? React with 👍 / 👎.
| "@moonshot-ai/kimi-code": patch | ||
| --- | ||
|
|
||
| On Linux, recursive directory watching now uses the native @parcel/watcher binding — one inotify instance for the whole tree instead of one watcher per directory — so watching large workspaces no longer fails with `EMFILE: too many open files`. Heavy generated directories (`.git`, `node_modules`, `.tower`, `.worktrees`, `.jj`) are now pruned at the OS watch level. When the native binding is unavailable, watching falls back to the previous behavior. |
There was a problem hiding this comment.
The changeset body contains three detailed sentences, but repository policy requires exactly one short user-facing sentence stating only what changed. Condense this entry before release.
AGENTS.md reference: AGENTS.md:L85-L87
Useful? React with 👍 / 👎.
4afae8a to
ad86368
Compare
sailist
changed the title
ad86368 to
56fab0d
Compare
sailist
changed the title
There was a problem hiding this comment.
💡 Codex Review
kimi-code/packages/protocol/src/ws-control.ts
Lines 446 to 447 in 56fab0d
Preserve the fs-watch protocol until a major release
When an external client uses the documented watch_fs_add or watch_fs_remove frames, removing both schemas from this union causes the existing protocol-v2 server to reject those requests and stops delivery of event.fs.changed, while WS_PROTOCOL_VERSION remains unchanged. The repository requires breaking changes to go through a changeset with a major bump, but this commit includes neither; retain compatibility or explicitly version and release the removal as a major change.
AGENTS.md reference: AGENTS.md:L61-L61
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
56fab0d to
9c85dec
Compare
9c85dec to
9b8468f
Compare
1 participant
Related Issue
No linked issue — internal refactor of the fs-watch stack (follow-up to EMFILE/wd investigations in large Linux workspaces).
Problem
Fs watching was scattered across four separate implementations: the os-layer
HostFsWatchService(chokidar + native signal handle),WorkspaceFsWatchService(whole-tree workspace watch),FileStorageService.watch(document-store watch chain), and kap-server'sFsWatchBridgeplus the WSwatch_fs_add/watch_fs_remove/event.fs.changedprotocol surface. The whole WS watch chain has no real client (no sender ofwatch_fs_addanywhere), and each service re-implements watcher lifecycle, readiness, retry and filtering in its own way.What changed
packages/agent-core-v2/src/human/utils/watch.ts: a module-level singleton whose side effects are managed by xstate — a root machine spawns one child machine per subscription, withfromCallbacklegs owning the chokidarFSWatcher, the native recursivefs.watch(signal mode on darwin/win32) and the backoff retry timer. Capabilities match the previous implementation:recursive/ignored(native-level pruning)/depth/signal(FSEvents / ReadDirectoryChangesW coarse invalidation, chokidar fallback onERR_FEATURE_UNAVAILABLE_ON_PLATFORM, 1s→30s exponential retry)/ready/default.gitexclusion.HostFsWatchService(interface + impl),WorkspaceFsWatchService,FileStorageService.watchand the whole document-store watch chain, kap-serverFsWatchBridge, the WSwatch_fs_add/watch_fs_remove/event.fs.changedprotocol schemas (no consumers), and their tests (-2648/+819 lines).subtreeWatchFilterpruning are unchanged, so hot-reload behavior is identical.No new dependencies (chokidar 4 and xstate 5 were already dependencies of agent-core-v2).
Checklist
/approve).gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.