fix(watch): bound config file watching and prevent ENOSPC / uncaughtException crash

[jstokke]

Problem

When configDir (~/.pi/agent/pi-data-masking or <cwd>/.pi/pi-data-masking) does not exist at session start, watchConfigFile currently traverses ancestors until it finds an existing directory (while (!existsSync(target)) target = dirname(target);), and attaches a watcher with { recursive: true } without an 'error' listener.

On Linux:

1. Node.js implements recursive: true via node:internal/fs/recursive_watch by issuing individual inotify_add_watch calls for every file and folder in the tree.
2. For global config, this places watches across all of ~/.pi/agent/ (including npm/node_modules/ with 34,000+ files, sessions/, etc.), quickly exhausting system inotify limits (fs.inotify.max_user_watches) with ENOSPC.
3. When any file is created (e.g. subagents writing session artifacts in sessions/<id>/subagent-artifacts/), Node's recursive watcher attempts to watch the new file, hits ENOSPC, and emits an 'error' event on the FSWatcher.
4. Because no .on("error", ...) handler is attached, Node's EventEmitter contract triggers an uncaughtException, crashing the entire Pi agent harness (pi exiting due to uncaughtException: Error: ENOSPC: System limit for number of file watchers reached...).

Solution

1. Eliminate Recursive Watching: All watchers are strictly non-recursive (recursive: false).
2. Bound Ancestor Traversal: Never climb beyond the immediate parent directory (parentDir = dirname(configDir)). Large ancestor directories like <cwd>, ~, or / are never watched.
3. Dynamic Promotion & Cleanup:
   • If configDir does not exist yet, watch at most parentDir non-recursively, filtered to dirName = basename(configDir). Sibling churn (e.g. sessions/, npm/) is ignored.
   • When configDir is created, parentWatcher is cleanly detached/closed, and a non-recursive watcher is attached to configDir.
   • The 250ms polling loop serves as a zero-inotify fallback when intermediate directories do not exist yet, promoting watchers once files are created.
4. Resilient Error Handling (safeWatch):
   • Always attach .on("error", ...) immediately upon watcher creation.
   • Safely close and remove failed watchers on ENOSPC, ENOENT, or EPERM without allowing unhandled exceptions to take down the host process.
5. Lifecycle Guard:
   • Add stopped flag in watchConfigPaths so pending timers never execute after session shutdown.

Testing

• npm test: All 164 tests pass (159 pre-existing + 5 new regression tests).
• npm run check: tsc clean (0 errors).
• Regression tests added to tests/config-loader.test.ts:
  • watchConfigPaths attaches error handlers to all FSWatchers and survives ENOSPC / ENOENT error events
  • watchConfigFile watches parentDir non-recursively when configDir does not exist yet and ignores sibling churn
  • parentWatcher is cleanly detached when configDir is created and dirWatcher is attached
  • watchConfigPaths does not climb beyond immediate parent and detects config when intermediate directories are created later
  • watchConfigPaths survives watched directory deletion without throwing uncaught exceptions

[cattyhouse]

Thanks — this also fixes a second, distinct crash mode with the same root cause on Linux, which I can reproduce deterministically. Posting the evidence in case it helps get this merged.

Same root cause, different errno: ELOOP

recursive: true on the ancestor watcher, with no 'error' listener. Your ENOSPC case is about the number of watches. The case below is Node's recursive watcher walking a tree that contains a self-referential symlink: statSync throws ELOOP inside node:internal/fs/recursive_watch, escapes as uncaughtException, and kills the host process:

pi exiting due to uncaughtException:
Error: ELOOP: too many symbolic links encountered,
       stat '<cwd>/build/wolfssl-aarch64/conf198529.dir/conf198529.file'
    at statSync (node:fs:1795:25)
    at #watchFile (node:internal/fs/recursive_watch:181:28)
    at #watchFolder (node:internal/fs/recursive_watch:150:28)
    at FSWatcher.<anonymous> (node:internal/fs/recursive_watch:213:26)
    at FSWatcher.emit (node:events:514:28)
    at FSWatcher._handle.onchange (node:internal/fs/watchers:281:12)

Pi dies mid-session with no prompt and no chance to save. The user's workaround was mkdir -p <cwd>/.pi/pi-data-masking per project (so the climb stops at the immediate parent and nothing is watched recursively) — which is exactly the ergonomics this PR removes.

Deterministic minimal reproduction

Autoconf's configure creates that symlink shape while probing the filesystem (conf$$.file → itself, conf$$.dir → conf$$.file). Any repo that builds autoconf/CMake-based dependencies has it transiently inside its build tree:

mkdir -p repo/build
ln -s conf.file repo/build/conf.file   # self-referential
ln -s conf.file repo/build/conf.dir    # chain that resolves to it
touch repo/build/x                      # make the recursive watcher rescan the tree

The old code climbs from the project config dir to the nearest existing ancestor and watches that recursively. So the crash needs the config dir's immediate parent to be missing as well:

<cwd>/.pi	<cwd>/.pi/pi-data-masking	old code recursively watches	result
missing	missing	<cwd> — the whole repo, incl. build/	CRASH: ELOOP
present	missing	<cwd>/.pi	OK (no such symlink there)
present	present	<cwd>/.pi/pi-data-masking (non-recursive)	OK

i.e. a fresh clone of any project that has never had a project-level masking config, plus a dependency build that creates autoconf-style symlinks anywhere in the tree → pi dies.

A/B verification

Bundled both versions with esbuild and drove them through an identical harness that mirrors the real wiring (global config dir exists like ~/.pi/agent/pi-data-masking; project config dir missing), with the symlink fixture above in build/:

version	fixture	result
v0.6.2 (bfe415b3, before)	.pi missing	CRASH ELOOP, exit 9
v0.6.2 (before)	.pi present	OK — exit 0
PR head (5436cd0)	.pi missing	OK — no crash, hot reload still fired
PR head (5436cd0)	.pi present	OK — no crash, hot reload still fired

Environment: Linux, Node v24.20.0, pi 0.85.1, extension v0.6.2.

Notes that may help review / merge

1. Applies cleanly, no upstream drift. The delta between the PR's config-loader.ts and the tagged v0.6.2 file is exactly +186/-54, matching the PR's own diffstat. git diff between v0.6.2 and current main shows config-loader.ts untouched, so this rebases without conflicts.
2. Backward compatible exports. The only "removed" export is watchConfigs, re-added with an optional third parameter, so existing callers such as watchConfigs(ctx.cwd, cb) in index.ts are unaffected; only WatchConfigHooks is added.
3. Hot reload does not regress — worth stating in the PR description. When neither the config dir nor its parent exists, no watcher can be attached at all, and detection falls back to the existing 250 ms signature poller, which now also calls refresh(). I confirmed the reload still fires in that configuration. That case is the very reason the recursive watcher was introduced (f8b6984, v0.2.0), so it's the first thing a maintainer will ask about.
4. Minor, optional: matchesFile / matchesDir now return true when filename == null (previously false), so events without a filename cause a debounced reload instead of being ignored. I read that as a deliberate conservative choice and it's harmless (at worst one extra reload), but flagging it since it is a behavior change.
5. The package-lock.json hunk adds @earendil-works/pi-ai to peerDependencies — only needed for the tests/typings, not at runtime.

One suggestion: since the 250 ms poller already covers the "created later" case on Linux, dropping recursive: true entirely (as this PR does) looks strictly better than keeping recursion behind a platform check.

@Sevten

[Sevten]

Thanks for the fix — the approach looks right (bounded traversal, non-recursive watchers, error handlers on every watcher, lifecycle guard), and the tests cover the crash scenarios well.

One blocker: main has since extracted the watch logic from config-loader.ts into a new config-watcher.ts (6db6424, v0.7.0), so this PR no longer applies — could you rebase and port the changes to config-watcher.ts? The watch code itself is unchanged on main, so the port should be mechanical. Happy to approve once that's in.