Analyzed and fixed fs.watch error crash
Public
Analyzed and fixed fs.watch error crash
Agent
Pi
Model
Claude Opus 4.6
Started
Apr 4, 6:40 PM
Messages103
Downloads0
Added in-progress label to GitHub issue.
EditReviewed code and verified root cause independently.
UnderstandCreated small test to reproduce the bug.
BuildDrafted and committed fix for the issue.
EditPushed changes and confirmed issue closure.
Verify
Analyze GitHub issue(s): https://github.com/badlogic/pi-mono/issues/2791
For each issue:
Add the inprogress label to the issue via GitHub CLI before analysis starts. If adding the label fails, report that explicitly and continue.
Read the issue in full, including all comments and linked issues/PRs.
Do not trust analysis written in the issue. Independently verify behavior and derive your own analysis from the code and execution path.
For bugs:
For feature requests:
Do NOT implement unless explicitly asked. Analyze and propose only.
Label added. Now let me read the actual source file to verify the issue independently.
Let me check if there are other fs.watch usages with the same pattern in the codebase.
The example file has the same pattern (no .on('error') handler), but it's just an example, not production code.
Issue: #2791 — fs.watch error event crashes all pi sessions simultaneously
Type: Bug
Independently verified: Yes. The issue is real and the root cause analysis in the issue is correct.
In packages/coding-agent/src/modes/interactive/theme/theme.ts, the startThemeWatcher() function (lines ~420-470) creates an fs.watch() handle on the custom themes directory. The code wraps the call in try/catch, but this only catches synchronous errors from fs.watch() itself.
fs.watch() returns an FSWatcher (an EventEmitter). Runtime errors (directory renamed/deleted, OS handle invalidation, handle limit exhaustion) are emitted asynchronously as error events. Without an .on('error', ...) handler, Node.js treats these as uncaught exceptions and terminates the process.
The specific code:
try { themeWatcher = fs.watch(customThemesDir, (_eventType, filename) => { // ... callback ... }); // No .on('error', ...) handler} catch (_error) { // Only catches synchronous errors}Add an error event handler to themeWatcher immediately after creation, inside the try block. On error, close the watcher and set it to undefined (theme hot-reload becomes unavailable, but pi stays alive):
try { themeWatcher = fs.watch(customThemesDir, (_eventType, filename) => { // ... existing callback ... }); themeWatcher.on("error", () => { try { themeWatcher?.close(); } catch { /* ignore */ } themeWatcher = undefined; });} catch (_error) { // Ignore errors starting watcher}packages/coding-agent/src/modes/interactive/theme/theme.ts — single change in startThemeWatcher()initTheme()/setTheme() call, but all sessions remain alivecan you reproduce this in a small test?
Let me look at the test infrastructure to understand how to write this properly.
Good, now I understand the test patterns. Let me also check how getCustomThemesDir works to know how to set up the environment.
Now let me look at a dark.json theme to use as a template for the test:
Now let me write the regression test:
Can't spy on ESM namespace. Let me use a child process approach instead, which is actually more faithful to the bug (it tests whether the process actually crashes).
The test confirms the bug: exit code 3 means emit("error") threw because there's no error handler. The test is working correctly: it fails with the current code (bug present) and will pass once the fix is applied.
Let me clean up the test slightly to make the failure output clearer:
Test works correctly. The failure message is clear:
BUG: FSWatcher has no error handler (issue #2791)error event was unhandled and threwThis confirms the bug: the FSWatcher from fs.watch() has zero error listeners, so any async error event crashes the process.
Summary:
packages/coding-agent/test/suite/regressions/2791-fswatch-error-crash.test.tsFSWatcher handle, and emits a synthetic error event on it. Without an error handler, EventEmitter.emit('error') throws/crashes. With a handler, the child exits cleanly.The fix remains the same one-liner addition in startThemeWatcher() in theme.ts:
themeWatcher.on("error", () => { try { themeWatcher?.close(); } catch { /* ignore */ } themeWatcher = undefined;});ok fix it
Test passes. Now run the full check:
All checks pass. Now update the changelog:
Run the test one more time to confirm everything is solid:
All done. Here's a summary of the changes:
Files modified:
packages/coding-agent/src/modes/interactive/theme/theme.ts — Added .on("error", ...) handler to the FSWatcher in startThemeWatcher(). On error, the watcher is closed and set to undefined. Theme hot-reload stops, but pi stays alive.
packages/coding-agent/test/suite/regressions/2791-fswatch-error-crash.test.ts — New regression test. Spawns a child process that sets up the theme watcher, finds the FSWatcher handle, and emits a synthetic error event. Verifies the process survives instead of crashing.
packages/coding-agent/CHANGELOG.md — Added entry under [Unreleased] > Fixed.
Wrap it.
Additional instructions:
Determine context from the conversation history first.
Rules for context detection:
/is or /pr, assume the issue or PR context is already known from the conversation and from the analysis work already done.Unless I explicitly override something in this request, do the following in order:
## [Unreleased] using the repo changelog rules.closes #<issue> in the commit message. If it is tied to multiple issues, stop and ask which one to use. If it is not tied to any issue, do not include closes # or fixes # in the commit message.main, stop and ask what to do. Do not push from another branch unless I explicitly say so.Constraints:
git add . or git add -A.On main. My files are: theme.ts, CHANGELOG.md, and the new test file. The other changes are from other work.
Now draft the issue comment:
Looks good. Posting and committing:
Only my 3 files staged.
Pushed. Issue #2791 will be auto-closed by the commit.
try { themeWatcher = fs.watch(customThemesDir, (_eventType, filename) => { // ... callback ... }); // No .on('error', ...) handler} catch (_error) { // Only catches synchronous errors}try { themeWatcher = fs.watch(customThemesDir, (_eventType, filename) => { // ... existing callback ... }); themeWatcher.on("error", () => { try { themeWatcher?.close(); } catch { /* ignore */ } themeWatcher = undefined; });} catch (_error) { // Ignore errors starting watcher}BUG: FSWatcher has no error handler (issue #2791)error event was unhandled and threwthemeWatcher.on("error", () => { try { themeWatcher?.close(); } catch { /* ignore */ } themeWatcher = undefined;});