fix(memory-core): avoid per-file watcher FD fan-out for memory directories by lukeboyett · Pull Request #86701 · openclaw/openclaw

lukeboyett · 2026-05-26T01:19:33Z

Summary

Fixes the gateway FD-leak regression reported in #86613.

A single authorized POST /tools/invoke memory_search call against a workspace with multi-thousand .md files under <workspace>/memory/ opens one read-only FD per file (~12,400 in our captures), never released until process exit. The root cause is chokidar v5's per-file fs.watch(path, {persistent: true}) fan-out (node_modules/chokidar/handler.js:126) when configured against a recursive directory tree — each node in the tree gets its own fs.watch instance, which on macOS opens a kqueue VNODE FD that lsof reports as a regular-file REG FD.

This change uses Node ≥22's native recursive fs.watch(dir, { recursive: true }) for directory watch paths (one watcher per directory via FSEvents on macOS / inotify recursive on Linux / ReadDirectoryChangesW on Windows), and keeps chokidar for individual file paths (e.g. MEMORY.md, file-typed extraPaths). Same dirty-event semantics; different watcher backend; small constant FD profile regardless of tree size.

Regression history

This is the third intervention on the memory watcher fan-out path:

fix(memory-core): fix macOS chokidar glob issue by watching memory dir directly #64711 (2204753b6214, @jasonxargs-boop, 2026-04-13) changed the chokidar watch target in manager-sync-ops.ts from glob memory/**/*.md to bare directory memory/. This made the recursive walk implicit and is where the per-file fan-out began to dominate large memory trees.
fix(memory): stop watcher write-polling fd pressure #81802 (b04e42812e, @frankekn, 2026-05-15) removed awaitWriteFinish from the same chokidar setup and added watch-settle.ts to handle event-stability. That PR reduced write-polling FD pressure but did not address the per-node fs.watch allocation; the verification used a 1,000-file tree, well under the storm-producing scale.
This PR addresses the remaining per-file allocation by routing directory paths to native recursive fs.watch instead of chokidar.

Related open PRs

fix(memory): bound INDEX_CACHE lifetime in long-running gateway daemons #86345 (@quengh, "fix(memory): bound INDEX_CACHE lifetime in long-running gateway daemons") — attacks the same [Bug]: Every memory_search call leaks ~N FDs (one per .md file in workspace memory tree) on macOS; long-lived gateways degrade toward FD exhaustion #86613 failure class from a complementary angle: it bounds how long a MemoryIndexManager (and therefore its watchers) can live in the module-level INDEX_CACHE before being evicted, so the FD leak is time-bounded. My PR attacks the per-allocation: each manager opens a small constant number of FDs rather than ~N where N is the workspace memory tree size. The two PRs are intentionally non-overlapping in manager-sync-ops.ts (this PR), and touch disjoint regions of manager.ts (their changes are in the imports/constants and search wrapper near line 715; this PR's only change there is in close() near line 957). If both land, the combined effect is defense in depth: smaller per-manager FD footprint AND bounded manager lifetime.
docs(memory): align descriptors and docs with recursive memory/**/*.md #85899 (@leafbird, "docs(memory): align descriptors and docs with recursive memory/**/*.md") — adjacent docs-only update that fixes agent-facing descriptions to use memory/**/*.md instead of memory/*.md. No code overlap; both PRs are mutually consistent.

Real behavior proof

Behavior or issue addressed: Gateway accumulates >12,000 read-only REG-type file descriptors on .md files under <workspace>/memory/ whenever memory_search is invoked against a multi-thousand-file memory tree (issue [Bug]: Every memory_search call leaks ~N FDs (one per .md file in workspace memory tree) on macOS; long-lived gateways degrade toward FD exhaustion #86613). One FD per file, never released until process exit; on macOS this is chokidar v5's per-file fs.watch opening a kqueue VNODE descriptor per node in the watched tree. Long-running gateways degrade toward FD exhaustion / EBADF in child-process spawns.
Real environment tested:
- Environment 1 (deployed gateway): macOS 14.7.6 (Darwin 23.6.0), Node 22.x, OpenClaw 2026.5.19 on upstream/main + 14 local patches (none in extensions/memory-core/), running under launchd. Workspace memory tree has 12,591 .md files across transcripts/, transcripts.archived/, and structured-md/{lessons,procedures,decisions,projects}/ plus their .archived/ siblings.
- Environment 2 (clean upstream-main lab): HEAD 01c5ab8d13, macOS 14.7.6, Node 22.19, dedicated OPENCLAW_HOME, isolated gateway runtime on port 28789, no channels, no provider keys, memorySearch.sync.watch defaulting to true. Synthetic workspace with 12,391 .md files mirroring the per-subdirectory composition of the original storms (9,394 / 1,695 / 268 / 215 / 214 / 213 / 151 / 126 / 81 / 34).
Exact steps or command run after the patch: Build the patched dist (pnpm build in this checkout). Launch the isolated lab gateway against the synthetic workspace. Run one authorized HTTP call, sample FDs via lsof -p $GATEWAY_PID at intervals 0/5/15/30/45/60/90/120 seconds, then send a second invocation and re-sample. Probe command:
```
curl -X POST http://127.0.0.1:28789/tools/invoke \
  -H "Authorization: Bearer test-token" \
  -H "Content-Type: application/json" \
  --data '{"tool":"memory_search","args":{"query":"FD-leak-probe-sentinel-xyzzy-nomatch"}}'
```
Same probe was used against the deployed gateway at port 18789 with its bearer token sourced from ~/.openclaw/.env.
Evidence after fix (deployed gateway, before this patch was applied, for the baseline):

curl -X POST http://127.0.0.1:18789/tools/invoke \
  -H "Authorization: Bearer $OPENCLAW_GATEWAY_TOKEN" \
  -H "Content-Type: application/json" \
  --data '{"tool":"memory_search","args":{"query":"FD-leak-probe-sentinel-xyzzy-nomatch"}}'

time	total FDs	REG FDs	mem-tree REG
baseline	159	91	0
t+0s (curl `--max-time 30` timed out; call never returned)	308	239	142
t+15s	1,875	1,806	1,712
t+45s	5,201	5,131	5,037
t+93s	7,632	7,562	7,469
t+157s	7,786	7,716	7,613
t+7m (no further probes; natural agent traffic only)	—	—	12,591

Mid-storm V8 heap snapshot at t+17s (341 MB, retainer chain WatchHelper → fsw → FSWatcher → _closers → Map) available on request. Full setup, sample lines, and second-call behaviour in #86613 comment 4537621029.

Environment 2 — clean upstream-main lab, before fix: HEAD 01c5ab8d13, dedicated OPENCLAW_HOME, isolated gateway runtime on port 28789, no channels, no provider keys, memorySearch.sync.watch defaulting to true. Synthetic workspace with 12,391 .md files mirroring the per-subdirectory composition of the original storms (9,394 / 1,695 / 268 / 215 / 214 / 213 / 151 / 126 / 81 / 34).

time	total FDs	REG FDs	workspace REG
baseline	69	46	0
t+0s (after single `memory_search` call; curl 30s timeout fired)	12,474	12,447	12,392
t+5s … t+120s	unchanged	unchanged	12,392 (plateau)

12,392 = 12,391 (memory/**/*.md count) + 1 (MEMORY.md).

Second invocation: 200 OK in <30s, zero additional FDs. Confirms the fan-out cost is paid once per (gateway, workspace) and reused thereafter — but never released for the process lifetime.

Environment 2 — clean upstream-main lab, WITH this fix applied (same OPENCLAW_HOME, same synthetic workspace, same probe):

time	total FDs	REG FDs	workspace REG	KQUEUE FDs
baseline	69	46	0	4
t+0s (after `memory_search`; curl 30s timeout fired again — see below)	83	56	1	4
t+5s … t+110s	unchanged	unchanged	1 (plateau)	4

The remaining 1 workspace REG is MEMORY.md, opened by chokidar (single-file path; chokidar's per-node fs.watch here costs one FD which is correct). The memory tree (12,391 files) is now covered by one native recursive fs.watch, visible as 1 of the 4 KQUEUE FDs (the other 3 are Node libuv internals present at baseline). 99.99% reduction in REG FDs.

The HTTP timeout at 30s is unrelated to the leak — it's first-time memory indexing on a 12k-file workspace. The second invocation returns 200 OK in <30s and does not add any FDs.

Observed result after fix: On the patched clean upstream-main lab gateway with the synthetic 12,391-file memory tree, a single memory_search call opens 1 REG FD (MEMORY.md, via chokidar single-file path) instead of 12,392, and that count holds flat through the 120-second observation window. The memory tree itself is now covered by exactly one native recursive fs.watch (visible as 1 of 4 total KQUEUE FDs; the other 3 are Node libuv internals present at baseline). Second invocation returns 200 OK in <30s with zero additional FDs, confirming the fan-out cost is paid once per (gateway, workspace) lifetime via FSEvents on macOS. 99.99% reduction in REG FDs versus before-fix baseline.
What was not tested:
- Linux behavior on a real Linux gateway (no Linux test bed; patch explicitly gates native recursive watch to process.platform === "darwin" || "win32" and routes Linux + others through chokidar fallback to match pre-PR behavior on those platforms).
- Windows behavior on a real Windows gateway (no test bed; native recursive watch is supported via dependency contract on Windows but not validated live).
- The native-creation-failure fallback path on a real unsupported filesystem (covered by unit test mock only).
- Behavior under sustained event flood (10k events/s on the watched dir). The existing watch-settle.ts queue should handle this; not exercised in lab.

What changes

extensions/memory-core/src/memory/manager-sync-ops.ts (+128/-15):

ensureWatcher() splits watchPaths into file vs directory sets. MEMORY.md stays a file path; memory/ and dir-typed extraPaths become directory paths.
Directory paths route to fsSync.watch(dir, { recursive: true }) via a new test-injectable factory TEST_MEMORY_NATIVE_WATCH_FACTORY_KEY, parallel to the existing TEST_MEMORY_WATCH_FACTORY_KEY.
The native watcher's event callback reuses shouldIgnoreMemoryWatchPath for filtering (no duplicated filter logic), with an lstatSync({ throwIfNoEntry: false }) for stats. null filename (Node platform caveat) triggers a broad markDirty() rather than a silent drop.
If fsSync.watch throws at creation (e.g. unsupported filesystem, ERR_FEATURE_UNAVAILABLE_ON_PLATFORM), the directory falls back into the chokidar set — coverage preserved, FD cost degraded to the previous behavior for that path only.
On a runtime error event the native watcher is closed, removed from the array, and a broad markDirty() is issued; the existing interval sync continues to maintain freshness.
Symlink policy preserved: only extraPaths entries are lstat-skipped (matching pre-PR behavior); memory/ and MEMORY.md continue to follow symlinks.
Idempotence guard expanded to this.watcher || this.nativeMemoryWatchers.length > 0 so a re-entrant ensureWatcher() doesn't double-allocate native watchers.

extensions/memory-core/src/memory/manager.ts (+10/-0):

close() tears down the native watchers in addition to the chokidar watcher.

extensions/memory-core/src/memory/manager.watcher-config.test.ts (+230/-67):

Adds a nativeWatchMock paired with the new factory key.
Existing assertions split: chokidar mock receives only file paths (MEMORY.md); native mock receives directories (memory/, dir-typed extraPaths) with recursive: true.
New test cases:
- Native rename / change event → markDirty.
- Null filename event → broad markDirty (Node platform caveat).
- Native creation failure → fallback into chokidar's path set, with the warning logged.
- Native runtime error → watcher closed, removed, broad markDirty fires.
- ensureWatcher() re-entrant call → no duplicate native watcher allocation.

Compatibility expectations

Memory freshness semantics preserved. Every path that previously triggered markDirty continues to trigger it. The shared shouldIgnoreMemoryWatchPath predicate is the sole filter for both watcher backends.
No config knobs added or removed. sync.watch resolution at src/agents/memory-search.ts is unchanged; this is purely an internal watcher backend swap. The "watcher policy choice" raised in clawsweeper's review on [Bug]: Every memory_search call leaks ~N FDs (one per .md file in workspace memory tree) on macOS; long-lived gateways degrade toward FD exhaustion #86613 (disable / cap / lifetime change) is intentionally sidestepped — this PR is a transparent infrastructure swap.
No new dependencies. Uses only node:fs recursive watch, available in our engines: node >= 22.19.
Cross-platform. Recursive fs.watch is supported in Node ≥22.19 on macOS (FSEvents), Linux (since v19.1, refined through v20+), and Windows (always supported). On any path where the native call throws or the FSWatcher emits an error, the chokidar fallback preserves coverage. Real lab verification was done on macOS only (see "What I did not test" above).

Acceptance criteria from clawsweeper review on #86613

Real-chokidar large-tree /tools/invoke memory_search repro that samples gateway FD counts before and after the call — done in lab (Environment 2 above)
manager.watcher-config.test.ts updated for the watcher split + new behavior cases
tools.test.ts runs clean against the patch (untouched file; verified locally)
tools-invoke-http.test.ts runs clean against the patch (untouched file; verified locally)

Test plan

pnpm tsgo:core:test clean on the patched source
pnpm test:extension memory-core clean
pnpm check clean
pnpm vitest run extensions/memory-core/src/memory/manager.watcher-config.test.ts clean
pnpm vitest run extensions/memory-core/src/tools.test.ts clean
pnpm vitest run src/gateway/tools-invoke-http.test.ts clean
Local codex review --base origin/main run; findings addressed (see "AI assistance" below)
Lab probe against patched dist: 12,392 → 1 REG FD reduction confirmed, second-call returns 200 OK with zero FD delta (numbers above)

AI assistance

AI-assisted: drafted with Claude (Sonnet 4.6 / Opus 4.7). Patch design, test scaffolding, and PR body all went through structured codex peer review.
Human-run real behavior proof included (lab + deployed-gateway numbers above; manual curl POST /tools/invoke and lsof sampling done by hand on a real Mac).
Codex review session log available on request.
I understand what the code does and the trade-offs (see "Compatibility expectations" + "What I did not test").
Local codex review --base origin/main run before opening.
Will resolve any bot review conversations rather than leaving them for maintainers.

Notes for review

This is a regression fix. The previous behavior (chokidar's per-file fs.watch fan-out) was opportunistically introduced when the watch target changed from glob to bare directory in fix(memory-core): fix macOS chokidar glob issue by watching memory dir directly #64711; this PR restores the small-constant-FD-cost property the gateway had before that change, without reverting the directory-watch semantics.
The PR scope is intentionally narrow: only the watcher backend selection changes. No changes to sync.watch default, no per-tree cap, no manager lifetime change. The "maintainer policy direction" called out by clawsweeper is sidestepped because freshness semantics are preserved.
Reviewers may want to consider whether memorySearch.sync.watch: false should still be the safer default for very large workspaces as defense-in-depth (separately from this PR). That's Feature: sync.watch should default to false in gateway mode #71335's domain.

Codex review findings deferred to follow-up

close() / sync race (pre-existing). codex review --base origin/main flagged a P2 race in extensions/memory-core/src/memory/manager.ts around close() lines 940–941: when close() races with a first sync() that's still awaiting ensureProviderInitialized(), the pendingSync snapshot taken before the await can be stale (this.syncing may not yet be assigned). After provider init resolves, that sync continues and can run against the just-closed provider/DB. This race is in pre-existing code; my changes in this file are limited to the native watcher teardown block at lines 958–967 (a disjoint region). Tracked as separate follow-up in MemoryIndexManager.close() races with in-flight sync — provider/DB closed before sync settles #86702.
Watched directory replacement (edge case, surfaced by post-fallback-fix codex review). When an existing memory/ directory is deleted and recreated mid-process (e.g. rm -rf memory && mkdir memory), native fs.watch remains bound to the old inode and subsequent changes in the recreated directory are not observed. With intervalMinutes defaulting to 0, auto-sync would stay stale until restart. The previous chokidar-on-bare-directory setup may handle this case via parent-dir or rediscovery semantics (not fully verified). Fixing this here would require either parent-directory watching, periodic inode-stat checks, or readdirp-style root rediscovery — meaningful scope expansion beyond the FD-leak fix. The scenario is uncommon at runtime (live gateways rarely have their memory roots replaced) and is also relevant to Feature: sync.watch should default to false in gateway mode #71335 (sync.watch policy: the safer default for high-churn workspaces is sync.watch: false plus a non-zero intervalMinutes). Happy to file as a separate follow-up issue if maintainers consider it merge-blocking.

Update — clawsweeper review addressed

clawsweeper's PR review (2026-05-26 01:32 UTC) requested one fix before merge: the native watcher error handler closed and removed the failed watcher but did not restore coverage, leaving the directory un-watched. This is now fixed in the follow-up commit on this branch:

manager-sync-ops.ts — added attachMemoryChokidarFallback(dir, markDirty). On a native watcher runtime error, the directory is reattached to the existing chokidar watcher via this.watcher.add(dir); if no chokidar watcher exists yet, one is spun up for that directory.
manager.watcher-config.test.ts — the existing runtime-error test now asserts the directory is added to chokidar after the error, AND that a subsequent chokidar event on the fallback path schedules sync. New test added for the "no chokidar yet → spin one up" branch.

All 16 tests in manager.watcher-config.test.ts pass; lab probe still shows 12,392 → 1 REG FD on the synthetic workspace.

Closes #86613.

clawsweeper · 2026-05-26T01:22:00Z

Codex review: needs changes before merge. Reviewed May 26, 2026, 12:13 PM ET / 16:13 UTC.

Summary
The PR routes memory-core directory watches to native recursive fs.watch on macOS/Windows, keeps chokidar for file and fallback paths, closes native watcher pairs, expands watcher tests, and adds a gateway memory FD repro helper script.

PR surface: Source +287, Tests +477, Config +1, Other +553. Total +1318 across 5 files.

Reproducibility: yes. The linked issue and PR body provide a high-confidence clean-main macOS reproduction using an authorized memory_search call against a 12,391-file memory tree, and current main source still routes the memory directory through chokidar; I did not rerun the large live repro in this read-only pass.

Review metrics: 1 noteworthy metric.

Package metadata delta: 1 script added, 0 dependency entries changed, 0 lockfiles changed. The dependency-change label came from package.json, but the inspected diff only adds a local repro command and does not change package resolution.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Fix or explicitly waive the issue-sized repro-helper false failure on first-invocation timeout.

Risk before merge

The macOS FD reduction is strongly proven, but the Windows native recursive watch path is covered by Node contract evidence and mocked tests rather than a real Windows gateway probe.
Changing the memory directory watch backend on macOS/Windows can affect memory index freshness if a native, parent, or fallback watcher edge misses events in a long-running gateway.
The new issue-sized repro helper can still false-fail fixed runs when first-time indexing exceeds the default HTTP timeout even though FD samples prove the fan-out is fixed.

Maintainer options:

Repair repro-helper timeout handling (recommended)
Update the helper so fixed mode can pass on good FD samples when the first indexing call times out, or performs the documented second probe before requiring ok.
Accept the native watcher proof boundary
Maintainers can land after checks if they accept macOS live proof plus Node contract and mocked Windows coverage for the native watcher path.
Request Windows runtime proof
Maintainers can ask for a real Windows gateway probe before merge if mocked ReadDirectoryChangesW coverage is not enough for this availability-sensitive backend change.

Copy recommended automerge instruction

@clawsweeper automerge

Special instructions:
Repair scripts/check-memory-fd-repro.mjs so fixed mode does not fail solely because the first memory_search invocation times out during initial indexing when FD counts stay under threshold; preserve leak-mode behavior and do not change memory-core runtime code.

Next step before merge
A narrow automated repair can fix the repro helper without changing the watcher runtime path; the remaining watcher-backend risk is maintainer acceptance rather than a code-repair task.

Security
Cleared: Cleared: the diff adds a local repro script and package script entry, with no dependency version, lockfile, external download, secret-handling, or CI permission change.

Review findings

[P3] Separate first-index timeouts from FD failures — scripts/check-memory-fd-repro.mjs:523-526

Review details

Best possible solution:

Fix the repro-helper timeout false-negative, then land the watcher-backend change once maintainers accept the macOS-live and Windows-contract proof boundary; keep broader sync.watch default policy work in the existing follow-up thread.

Do we have a high-confidence way to reproduce the issue?

Yes. The linked issue and PR body provide a high-confidence clean-main macOS reproduction using an authorized memory_search call against a 12,391-file memory tree, and current main source still routes the memory directory through chokidar; I did not rerun the large live repro in this read-only pass.

Is this the best way to solve the issue?

Mostly yes. The runtime fix preserves the existing sync.watch contract, leaves Linux on the pre-PR chokidar path, and uses native recursive watching only where the dependency contract supports the intended constant watcher shape; the repro-helper false-negative is the remaining patch issue.

Full review comments:

[P3] Separate first-index timeouts from FD failures — scripts/check-memory-fd-repro.mjs:523-526
In fixed mode the helper requires invoke.ok before accepting the FD samples. The PR's own full-tree proof says the fixed build can still hit the 30s first-index timeout while REG FDs stay low and a second invocation succeeds, so --full can report a false regression for indexing latency instead of watcher FD fan-out. Please allow an aborted first invoke when the FD peak is under threshold, or perform the documented second invocation before requiring ok.
Confidence: 0.86

Overall correctness: patch is correct
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 0973a7e4e4cc.

Label changes

Label changes:

add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal/lsof proof from a real macOS gateway lab showing the after-fix 12k-file workspace REG FD count dropping from 12,392 to 1 and staying flat through the observation window.

Label justifications:

P1: The PR targets a real gateway FD-exhaustion regression that can degrade long-running memory_search workflows and process availability.
merge-risk: 🚨 session-state: Changing memory watcher backends can leave memory index state stale if native, parent, or fallback events are missed.
merge-risk: 🚨 availability: The changed watcher lifecycle directly affects file descriptor and handle pressure in long-running gateway processes.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): The PR body includes terminal/lsof proof from a real macOS gateway lab showing the after-fix 12k-file workspace REG FD count dropping from 12,392 to 1 and staying flat through the observation window.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes terminal/lsof proof from a real macOS gateway lab showing the after-fix 12k-file workspace REG FD count dropping from 12,392 to 1 and staying flat through the observation window.

Evidence reviewed

PR surface:

Source +287, Tests +477, Config +1, Other +553. Total +1318 across 5 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	2	307	20	+287
Tests	1	525	48	+477
Docs	0	0	0	0
Config	1	1	0	+1
Generated	0	0	0	0
Other	1	553	0	+553
Total	5	1386	68	+1318

Acceptance criteria:

node --check scripts/check-memory-fd-repro.mjs
node scripts/check-memory-fd-repro.mjs --help
node scripts/check-memory-fd-repro.mjs --files 16 --allow-non-darwin --mode report
node scripts/run-vitest.mjs extensions/memory-core/src/memory/manager.watcher-config.test.ts

What I checked:

Root policy applied: Root AGENTS.md was read fully and its OpenClaw PR review, dependency-contract, validation, plugin-boundary, and merge-risk guidance affected this review. (AGENTS.md:1, 0973a7e4e4cc)
Scoped extension policy applied: extensions/AGENTS.md was read and applies because the runtime changes are inside bundled plugin code. (extensions/AGENTS.md:1, 0973a7e4e4cc)
Scoped scripts policy applied: scripts/AGENTS.md was read and applies because the PR adds a local gateway repro helper and package script entry. (scripts/AGENTS.md:1, 0973a7e4e4cc)
Current main still uses chokidar for the memory directory: On current main, ensureWatcher() passes both MEMORY.md and memory/ into a single chokidar watcher when sync.watch is enabled, so the backend swap is not already implemented on main. (extensions/memory-core/src/memory/manager-sync-ops.ts:436, 0973a7e4e4cc)
PR head splits file and directory watcher backends: At PR head, files stay on chokidar while directory paths attempt native recursive watch only on darwin/win32, with unsupported platforms and creation failures falling back to chokidar. (extensions/memory-core/src/memory/manager-sync-ops.ts:450, c156fd561026)
Native watcher lifecycle is covered in code: The PR closes failed native watcher pairs, marks memory dirty, attaches chokidar fallback, and uses parent watchers to handle watched-root replacement by reattaching or falling back. (extensions/memory-core/src/memory/manager-sync-ops.ts:592, c156fd561026)

Likely related people:

jasonxargs-boop: Authored the merged macOS chokidar change that moved memory watching from memory/**/*.md to the bare memory directory, which is the watch shape this PR repairs at large-tree scale. (role: introduced adjacent behavior; confidence: high; commits: 2204753b6214; files: extensions/memory-core/src/memory/manager-sync-ops.ts, extensions/memory-core/src/memory/manager.watcher-config.test.ts)
frankekn: Authored the recent memory watcher FD-pressure fix and watch-settle path that this PR preserves while changing the directory watcher backend. (role: recent area contributor; confidence: high; commits: b04e42812eca; files: extensions/memory-core/src/memory/manager-sync-ops.ts, extensions/memory-core/src/memory/watch-settle.ts, extensions/memory-core/src/memory/manager.watcher-config.test.ts)
osolmaz: The current PR is assigned to osolmaz, and the latest PR commits add the memory FD repro helper and watcher-pair refactor under review. (role: assigned reviewer and repro-helper contributor; confidence: medium; commits: 321c77040157, c156fd561026; files: scripts/check-memory-fd-repro.mjs, package.json, extensions/memory-core/src/memory/manager-sync-ops.ts)
Vincent Koc: Recent current-main memory-core refactors touched manager-sync-ops.ts and manager.ts, so this person is a useful routing candidate for lifecycle and typing drift context. (role: recent adjacent contributor; confidence: medium; commits: ca26489fe882, 620537914b8c; files: extensions/memory-core/src/memory/manager-sync-ops.ts, extensions/memory-core/src/memory/manager.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ff5e61a4f8

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

clawsweeper · 2026-05-26T01:32:13Z

ClawSweeper PR egg

✨ Hatched: 🥚 common Mossy Proofling

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

Merged PRs are hatchable.
Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🥚 common.
Trait: guards the happy path.
Image traits: location review cove; accessory commit compass; palette amber, ink, and glacier blue; mood focused; pose pointing at a small proof artifact; shell paper lantern shell; lighting warm desk-lamp glow; background tiny shells and proof notes.
Share on X: post this hatch
Copy: My PR egg hatched a 🥚 common Mossy Proofling in ClawSweeper.

What is this egg doing here?

Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

…e watcher error Addresses clawsweeper PR review on openclaw#86701: when the native recursive fs.watch emits an error and the watcher is closed/removed, no replacement watcher is attached. Because intervalMinutes defaults to 0, subsequent memory changes under that directory would stop driving watch sync until process restart. After this commit, the native watcher error handler closes and removes the dead FSWatcher (as before), forces one broad markDirty to cover the gap, AND reattaches the directory to chokidar via the new attachMemoryChokidarFallback helper. If the existing this.watcher slot holds a chokidar instance (handling MEMORY.md and/or other file paths), the directory is added via this.watcher.add(dir). If no chokidar watcher exists yet, one is spun up specifically for that directory. The fallback is intentional: it accepts chokidar's per-file FD cost for the failed directory only (and only after the native watcher already died once), rather than dropping watcher coverage entirely. The FD-leak fix property for the healthy-watcher path is unchanged. Tests: - existing "runtime error" test in manager.watcher-config.test.ts now asserts the directory is attached to chokidar after the error and that a subsequent chokidar event on the fallback path schedules sync - new test exercises the "no chokidar yet" branch where the fallback creates a fresh chokidar watcher for the directory - all 16 tests in manager.watcher-config.test.ts pass; lab probe still shows 12,392 → 1 REG FD reduction on the synthetic workspace Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

lukeboyett · 2026-05-26T02:08:47Z

@clawsweeper review

Addressed the P2 finding from your earlier review (native watcher error path losing directory coverage). New commit e153c2fc24 adds attachMemoryChokidarFallback(dir, markDirty):

On native watcher runtime error: close + remove + markDirty() (as before), then either this.watcher.add(dir) if a chokidar watcher already exists for file paths, or spin up a fresh chokidar watcher for dir if not.
Two test updates in manager.watcher-config.test.ts: existing "runtime error" test now asserts the dir is attached to chokidar after error AND that a subsequent chokidar event on the fallback path schedules sync; new test exercises the no-pre-existing-chokidar branch.
16/16 tests in that file pass; lab probe still shows 12,392 → 1 REG FD on synthetic 12k-file workspace.

A separate codex review surfaced a different P2 concerning watched-directory replacement (rm -rf memory && mkdir memory while the gateway runs) — left as a deferred follow-up in the PR body since it's a different scenario from your fallback finding and likely belongs with the broader sync.watch policy discussion in #71335. Happy to address here if you prefer.

clawsweeper · 2026-05-26T02:08:50Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26428259930
Updated: 2026-05-26T02:20:31.365Z

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e153c2fc24

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

…d fallback against shutdown race Two follow-up findings from PR review on openclaw#86701: 1. Linux gating (clawsweeper [P2], also codex round-1 #1). On non-macOS/non-Windows platforms, Node routes `fs.watch(dir, { recursive: true })` through `internal/fs/recursive_watch`, which walks the tree and attaches one watcher per discovered entry. That defeats the constant-watcher- profile goal of the parent commit's fix without throwing, so the creation-failure fallback never fires. Restrict native attachment to `process.platform === "darwin" || "win32"`; Linux and others go straight to the chokidar fallback, matching pre-PR behavior on those platforms. 2. Shutdown-race guard (codex-connector inline reviews 4359699784 + 4359929223 [P2]). The native watcher `error` handler called `attachMemoryChokidarFallback` unconditionally, including after `close()` had set `this.closed = true` and begun teardown. That left a window where an error fired mid-shutdown could spin up a new chokidar watcher after the normal watcher-close step had already run, leaking watch handles past manager close. Both the error handler and the `attachMemoryChokidarFallback` helper now short-circuit when `this.closed` is true. Tests: - new case in manager.watcher-config.test.ts verifies that `process.platform === "linux"` routes both `memory/` and dir-typed `extraPaths` directly to chokidar (`nativeWatchMock` not called at all) — covers the Linux gate - all 17 tests in manager.watcher-config.test.ts pass Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

lukeboyett · 2026-05-26T02:30:26Z

Pushed 5df68c5674 covering the second review pass.

1. Linux gating — addresses clawsweeper's [P2] Gate native recursive watches off Linux and codex round-1 #1. Native recursive fs.watch is now restricted to process.platform === "darwin" || "win32"; Linux and others route directly to the chokidar fallback. Comment in source explains the reasoning: Node's non-Darwin/non-Windows recursive watch path walks the tree per-entry under the hood without throwing, so the creation-failure fallback would never catch it.

2. Shutdown-race guard — addresses codex-connector inline reviews 4359699784 + 4359929223 ([P2] Avoid reattaching fallback watchers during shutdown). Both the native watcher error handler and the attachMemoryChokidarFallback helper now short-circuit on this.closed, so an error firing during/after close() cannot leak a new chokidar watcher past manager teardown.

Validation:

pnpm tsgo:core:test: clean
pnpm vitest run extensions/memory-core/src/memory/manager.watcher-config.test.ts: 17/17 pass (new "routes directories through chokidar on non-macOS/non-Windows" case asserts nativeWatchMock is not called when process.platform === "linux")
codex review --base origin/main: clean — "No discrete, actionable regressions were identified"
Lab probe on macOS dist still shows 12,392 → 1 REG FD on synthetic 12k-file workspace

The remaining dir-replacement edge case from the earlier codex final review (watched root deleted/recreated mid-process) stays as a deferred follow-up in the PR body — happy to file as a separate issue if maintainers want it merge-blocking.

@clawsweeper review

clawsweeper · 2026-05-26T02:30:29Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/26428877439
Updated: 2026-05-26T02:42:35.826Z

…e watcher error Addresses clawsweeper PR review on openclaw#86701: when the native recursive fs.watch emits an error and the watcher is closed/removed, no replacement watcher is attached. Because intervalMinutes defaults to 0, subsequent memory changes under that directory would stop driving watch sync until process restart. After this commit, the native watcher error handler closes and removes the dead FSWatcher (as before), forces one broad markDirty to cover the gap, AND reattaches the directory to chokidar via the new attachMemoryChokidarFallback helper. If the existing this.watcher slot holds a chokidar instance (handling MEMORY.md and/or other file paths), the directory is added via this.watcher.add(dir). If no chokidar watcher exists yet, one is spun up specifically for that directory. The fallback is intentional: it accepts chokidar's per-file FD cost for the failed directory only (and only after the native watcher already died once), rather than dropping watcher coverage entirely. The FD-leak fix property for the healthy-watcher path is unchanged. Tests: - existing "runtime error" test in manager.watcher-config.test.ts now asserts the directory is attached to chokidar after the error and that a subsequent chokidar event on the fallback path schedules sync - new test exercises the "no chokidar yet" branch where the fallback creates a fresh chokidar watcher for the directory - all 16 tests in manager.watcher-config.test.ts pass; lab probe still shows 12,392 → 1 REG FD reduction on the synthetic workspace Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…d fallback against shutdown race Two follow-up findings from PR review on openclaw#86701: 1. Linux gating (clawsweeper [P2], also codex round-1 #1). On non-macOS/non-Windows platforms, Node routes `fs.watch(dir, { recursive: true })` through `internal/fs/recursive_watch`, which walks the tree and attaches one watcher per discovered entry. That defeats the constant-watcher- profile goal of the parent commit's fix without throwing, so the creation-failure fallback never fires. Restrict native attachment to `process.platform === "darwin" || "win32"`; Linux and others go straight to the chokidar fallback, matching pre-PR behavior on those platforms. 2. Shutdown-race guard (codex-connector inline reviews 4359699784 + 4359929223 [P2]). The native watcher `error` handler called `attachMemoryChokidarFallback` unconditionally, including after `close()` had set `this.closed = true` and begun teardown. That left a window where an error fired mid-shutdown could spin up a new chokidar watcher after the normal watcher-close step had already run, leaking watch handles past manager close. Both the error handler and the `attachMemoryChokidarFallback` helper now short-circuit when `this.closed` is true. Tests: - new case in manager.watcher-config.test.ts verifies that `process.platform === "linux"` routes both `memory/` and dir-typed `extraPaths` directly to chokidar (`nativeWatchMock` not called at all) — covers the Linux gate - all 17 tests in manager.watcher-config.test.ts pass Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…atisfy check-test-types clawsweeper review on PR openclaw#86701 (P2 finding) reported the `check-test-types` lane failing on `extensions/memory-core/src/memory/manager.watcher-config.test.ts` because several assertions index `watchMock.mock.calls[0][...]` directly. TypeScript treats vi.fn's `.mock.calls` as a possibly-empty tuple, so direct positional indexing fails the strict type-check lane. Apply the same `as unknown as [string[], Record<string, unknown>]` cast pattern already used elsewhere in this file for the first chokidar destructure, to all remaining `mock.calls[N]` accesses in the new tests added by the previous commit. No runtime behavior change; tests still pass (16/16 in the focused file). `pnpm check:test-types` now clean. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…ment Two rank-up moves identified by clawsweeper review on PR openclaw#86701: 1. **Windows native-watcher coverage in tests.** Add a mocked `process.platform === "win32"` case that mirrors the existing Linux gate test: assert that BOTH `memory/` and dir-typed `extraPaths` route to native recursive fs.watch (Windows uses ReadDirectoryChangesW for the recursive option, like macOS uses FSEvents), and that chokidar receives only the file path (`MEMORY.md`). Verifies the `process.platform === "darwin" || "win32"` gate keeps Windows on the native path. 2. **Detect memory-root replacement and reattach.** When the native recursive watcher is bound to a directory inode and that directory is replaced mid-process (`rm -rf memory && mkdir memory`), Node's fs.watch silently keeps watching the dead inode and never emits an error, so the existing creation-failure + runtime-error fallbacks cannot recover. Codex inline reviews 4360224380 (and codex final) flagged this as a freshness risk because `intervalMinutes` defaults to `0`, leaving the directory unindexed until restart. This commit pairs every native memory directory watch with a non-recursive `fs.watch(parentDir, { recursive: false })`. When that parent watcher fires for the watched basename AND the current inode of the dir differs from the recorded inode, the existing main+parent pair is closed and a fresh `attachNativeMemoryWatchForDir` invocation reattaches on the new inode. If the dir was removed (not replaced), the chokidar fallback takes over to preserve coverage. Implementation: - New `nativeMemoryParentWatchers: fsSync.FSWatcher[]` paired with the existing `nativeMemoryWatchers`. - New helper `attachNativeMemoryWatchForDir(dir, markDirty)` factored out of `ensureWatcher()` so the parent-event handler can re-enter cleanly for reattach. - Both arrays are torn down in `manager.ts` `close()`. - All existing close-time / error-path guards preserved (the parent handler also respects `this.closed`). Tests: - "routes directories through native recursive watch on Windows" — mocked-platform mirror of the existing Linux gate test. - "attaches a non-recursive parent-directory watcher for root-replacement detection" — asserts the main+parent pair is created with the correct recursive flags. - "ignores parent-directory events for unrelated basenames" — asserts the parent handler only reacts to the watched basename and leaves the main watcher untouched for sibling events. - Existing tests updated for the new call shape: each watched dir now produces 2 nativeWatchMock calls (1 main recursive + 1 parent non-recursive). Assertions filter by `recursive: true` to identify main watchers. - Updated `createMockNativeWatcher` mock to expose the `options` argument so tests can distinguish main vs parent watchers. Local validation: - `pnpm tsgo:core:test`: clean - `pnpm check:test-types`: clean - `pnpm vitest run manager.watcher-config.test.ts`: 20/20 pass - `codex review --base origin/main`: no actionable regressions - Lab probe unchanged: 12,392 → 1 REG FD on macOS Closes most of the "merge-risk: 🚨 availability" exposure clawsweeper flagged. Windows live proof (real Windows runner, not mock) and Linux live proof remain available as separate live-validation steps that need real VMs. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…ter root replacement codex-connector inline review 4360397128 on PR openclaw#86701 found a small race in the root-replacement handler: after tearing down the old main+parent native watcher pair, we attempt to reattach with `attachNativeMemoryWatchForDir(dir, markDirty)` if the inode-check reported a non-null inode. But that helper runs its OWN statSync internally — if the directory disappears between our inode check and the helper's check (e.g., `rm -rf memory && mkdir memory && rm -rf memory` racing), the helper returns `false` and we silently lose watcher coverage instead of falling back to chokidar. Check the boolean return from `attachNativeMemoryWatchForDir` and invoke `attachMemoryChokidarFallback` on failure, matching the behavior of the corresponding initial-attach path in `ensureWatcher()`. Local validation: - `pnpm tsgo:core:test`: clean - `pnpm vitest run manager.watcher-config.test.ts`: 20/20 pass - `codex review --base origin/main`: no actionable regressions Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…ories Closes openclaw#86613. A single authorized POST /tools/invoke memory_search call against a workspace with multi-thousand .md files under <workspace>/memory/ opens one read-only FD per file (~12,400 in our captures), never released until process exit. Root cause: chokidar v5 calls fs.watch(path) per filesystem node (handler.js:126), no recursive option. On macOS each per-file fs.watch opens a kqueue VNODE FD that lsof reports as a regular-file REG FD. So one watcher per .md file = one FD per file = 12k+ FDs. Switch directory watch paths to Node >= 22 native recursive fs.watch(dir, { recursive: true }) — one watcher per directory via FSEvents on macOS / inotify recursive on Linux / ReadDirectoryChangesW on Windows. Individual file paths (MEMORY.md, file-typed extraPaths) continue through chokidar. Same dirty-event semantics, different backend, small constant FD profile regardless of tree size. Regression introduced by openclaw#64711 (jasonxargs-boop, 2026-04-13) which changed the chokidar watch target from glob memory/**/*.md to bare directory memory/, making the recursive walk implicit. PR openclaw#81802 (frankekn, 2026-05-15) removed awaitWriteFinish and added watch-settle on the same path; that reduced write-polling pressure but did not address the per-node fs.watch allocation (verification used 1,000 files, well under the storm-producing scale). Lab verification on clean upstream-main + synthetic 12,391-file workspace: 0 → 1 REG FD (MEMORY.md only; 12,391 file events covered by one recursive kqueue/FSEvents watcher), flat plateau, second memory_search returns 200 OK with zero additional FDs. Complementary to openclaw#86345 which attacks the same failure class by bounding INDEX_CACHE lifetime; this PR is non-overlapping in manager-sync-ops.ts and touches a disjoint region of manager.ts. Changes: - manager-sync-ops.ts: split watchPaths by dir/file; native recursive fs.watch for dirs with shouldIgnoreMemoryWatchPath filter and per-event lstat; chokidar retained for files. New TEST_MEMORY_NATIVE_WATCH_FACTORY_KEY for test injection. Native creation failure falls back into chokidar set. Null filename (Node platform caveat) triggers broad markDirty. Runtime error closes + removes + marks dirty. Idempotence guard expanded. Symlink policy preserved (extraPaths-only skip). - manager.ts: close() tears down nativeMemoryWatchers in addition to chokidar watcher. - manager.watcher-config.test.ts: nativeWatchMock paired with new factory key; assertions split between chokidar (files) and native (dirs); new cases for rename/change dispatch, null filename, native creation failure fallback, runtime error close, ensureWatcher re-entrancy guard. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…e watcher error Addresses clawsweeper PR review on openclaw#86701: when the native recursive fs.watch emits an error and the watcher is closed/removed, no replacement watcher is attached. Because intervalMinutes defaults to 0, subsequent memory changes under that directory would stop driving watch sync until process restart. After this commit, the native watcher error handler closes and removes the dead FSWatcher (as before), forces one broad markDirty to cover the gap, AND reattaches the directory to chokidar via the new attachMemoryChokidarFallback helper. If the existing this.watcher slot holds a chokidar instance (handling MEMORY.md and/or other file paths), the directory is added via this.watcher.add(dir). If no chokidar watcher exists yet, one is spun up specifically for that directory. The fallback is intentional: it accepts chokidar's per-file FD cost for the failed directory only (and only after the native watcher already died once), rather than dropping watcher coverage entirely. The FD-leak fix property for the healthy-watcher path is unchanged. Tests: - existing "runtime error" test in manager.watcher-config.test.ts now asserts the directory is attached to chokidar after the error and that a subsequent chokidar event on the fallback path schedules sync - new test exercises the "no chokidar yet" branch where the fallback creates a fresh chokidar watcher for the directory - all 16 tests in manager.watcher-config.test.ts pass; lab probe still shows 12,392 → 1 REG FD reduction on the synthetic workspace Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…d fallback against shutdown race Two follow-up findings from PR review on openclaw#86701: 1. Linux gating (clawsweeper [P2], also codex round-1 #1). On non-macOS/non-Windows platforms, Node routes `fs.watch(dir, { recursive: true })` through `internal/fs/recursive_watch`, which walks the tree and attaches one watcher per discovered entry. That defeats the constant-watcher- profile goal of the parent commit's fix without throwing, so the creation-failure fallback never fires. Restrict native attachment to `process.platform === "darwin" || "win32"`; Linux and others go straight to the chokidar fallback, matching pre-PR behavior on those platforms. 2. Shutdown-race guard (codex-connector inline reviews 4359699784 + 4359929223 [P2]). The native watcher `error` handler called `attachMemoryChokidarFallback` unconditionally, including after `close()` had set `this.closed = true` and begun teardown. That left a window where an error fired mid-shutdown could spin up a new chokidar watcher after the normal watcher-close step had already run, leaking watch handles past manager close. Both the error handler and the `attachMemoryChokidarFallback` helper now short-circuit when `this.closed` is true. Tests: - new case in manager.watcher-config.test.ts verifies that `process.platform === "linux"` routes both `memory/` and dir-typed `extraPaths` directly to chokidar (`nativeWatchMock` not called at all) — covers the Linux gate - all 17 tests in manager.watcher-config.test.ts pass Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…atisfy check-test-types clawsweeper review on PR openclaw#86701 (P2 finding) reported the `check-test-types` lane failing on `extensions/memory-core/src/memory/manager.watcher-config.test.ts` because several assertions index `watchMock.mock.calls[0][...]` directly. TypeScript treats vi.fn's `.mock.calls` as a possibly-empty tuple, so direct positional indexing fails the strict type-check lane. Apply the same `as unknown as [string[], Record<string, unknown>]` cast pattern already used elsewhere in this file for the first chokidar destructure, to all remaining `mock.calls[N]` accesses in the new tests added by the previous commit. No runtime behavior change; tests still pass (16/16 in the focused file). `pnpm check:test-types` now clean. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…ment Two rank-up moves identified by clawsweeper review on PR openclaw#86701: 1. **Windows native-watcher coverage in tests.** Add a mocked `process.platform === "win32"` case that mirrors the existing Linux gate test: assert that BOTH `memory/` and dir-typed `extraPaths` route to native recursive fs.watch (Windows uses ReadDirectoryChangesW for the recursive option, like macOS uses FSEvents), and that chokidar receives only the file path (`MEMORY.md`). Verifies the `process.platform === "darwin" || "win32"` gate keeps Windows on the native path. 2. **Detect memory-root replacement and reattach.** When the native recursive watcher is bound to a directory inode and that directory is replaced mid-process (`rm -rf memory && mkdir memory`), Node's fs.watch silently keeps watching the dead inode and never emits an error, so the existing creation-failure + runtime-error fallbacks cannot recover. Codex inline reviews 4360224380 (and codex final) flagged this as a freshness risk because `intervalMinutes` defaults to `0`, leaving the directory unindexed until restart. This commit pairs every native memory directory watch with a non-recursive `fs.watch(parentDir, { recursive: false })`. When that parent watcher fires for the watched basename AND the current inode of the dir differs from the recorded inode, the existing main+parent pair is closed and a fresh `attachNativeMemoryWatchForDir` invocation reattaches on the new inode. If the dir was removed (not replaced), the chokidar fallback takes over to preserve coverage. Implementation: - New `nativeMemoryParentWatchers: fsSync.FSWatcher[]` paired with the existing `nativeMemoryWatchers`. - New helper `attachNativeMemoryWatchForDir(dir, markDirty)` factored out of `ensureWatcher()` so the parent-event handler can re-enter cleanly for reattach. - Both arrays are torn down in `manager.ts` `close()`. - All existing close-time / error-path guards preserved (the parent handler also respects `this.closed`). Tests: - "routes directories through native recursive watch on Windows" — mocked-platform mirror of the existing Linux gate test. - "attaches a non-recursive parent-directory watcher for root-replacement detection" — asserts the main+parent pair is created with the correct recursive flags. - "ignores parent-directory events for unrelated basenames" — asserts the parent handler only reacts to the watched basename and leaves the main watcher untouched for sibling events. - Existing tests updated for the new call shape: each watched dir now produces 2 nativeWatchMock calls (1 main recursive + 1 parent non-recursive). Assertions filter by `recursive: true` to identify main watchers. - Updated `createMockNativeWatcher` mock to expose the `options` argument so tests can distinguish main vs parent watchers. Local validation: - `pnpm tsgo:core:test`: clean - `pnpm check:test-types`: clean - `pnpm vitest run manager.watcher-config.test.ts`: 20/20 pass - `codex review --base origin/main`: no actionable regressions - Lab probe unchanged: 12,392 → 1 REG FD on macOS Closes most of the "merge-risk: 🚨 availability" exposure clawsweeper flagged. Windows live proof (real Windows runner, not mock) and Linux live proof remain available as separate live-validation steps that need real VMs. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…ter root replacement codex-connector inline review 4360397128 on PR openclaw#86701 found a small race in the root-replacement handler: after tearing down the old main+parent native watcher pair, we attempt to reattach with `attachNativeMemoryWatchForDir(dir, markDirty)` if the inode-check reported a non-null inode. But that helper runs its OWN statSync internally — if the directory disappears between our inode check and the helper's check (e.g., `rm -rf memory && mkdir memory && rm -rf memory` racing), the helper returns `false` and we silently lose watcher coverage instead of falling back to chokidar. Check the boolean return from `attachNativeMemoryWatchForDir` and invoke `attachMemoryChokidarFallback` on failure, matching the behavior of the corresponding initial-attach path in `ensureWatcher()`. Local validation: - `pnpm tsgo:core:test`: clean - `pnpm vitest run manager.watcher-config.test.ts`: 20/20 pass - `codex review --base origin/main`: no actionable regressions Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…parent watcher on native errors clawsweeper review on commit 5df68c5 surfaced two parent-watcher lifecycle issues introduced by the dir-replacement fix: 1. **[P2] Null parent-watcher filename** (line 623). Node fs.watch may call its listener with `filename === null` on some platforms even when the parent watcher is otherwise supported. The previous handler returned silently on `filename !== baseName`, so a root replacement reported with a null filename would leave the native watcher bound to the dead inode. Now treat null as an unknown event and fall through to the inode check; sibling events (non-null, non-baseName) still return early. 2. **[P3] Paired parent watcher leaks on native error** (line 600). When the main native watcher emits `error`, the previous handler closed only the main watcher and added the chokidar fallback, leaving the paired parent watcher alive. A later root-replacement event would then reattach native coverage on top of the already-installed chokidar watcher, creating duplicate handles and event paths. Forward-declared `parentWatcherRef` so the main error handler can close + remove + null-out the paired parent watcher before the chokidar fallback runs. Parent watcher's own error handler also clears the ref so a stale reference doesn't persist after parent-side teardown. Tests added (22 total now, up from 20): - "treats null parent-watcher filename as an unknown event and re-checks the inode" — verifies null filename falls through to the inode check and that no spurious teardown happens when the inode hasn't changed. - "closes the paired parent watcher when the native main watcher errors" — verifies the paired parent watcher's `close()` is invoked when the main watcher errors out (preventing double-attach if a later root replacement occurs). Local validation: - `pnpm tsgo:core:test`: clean - `pnpm vitest run manager.watcher-config.test.ts`: 22/22 pass - `codex review --base origin/main`: no actionable regressions Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

osolmaz · 2026-05-26T16:49:00Z

Merged via squash.

Prepared head SHA: e27c28a3a10ee72a181c082210070a12f5801c2f
Merge commit: 9e43d0327fc884183958bc29df09a47d9b03725d

Thanks @lukeboyett!

openclaw-barnacle Bot added extensions: memory-core Extension: memory-core size: M labels May 26, 2026

lukeboyett mentioned this pull request May 26, 2026

[Bug]: Every memory_search call leaks ~N FDs (one per .md file in workspace memory tree) on macOS; long-lived gateways degrade toward FD exhaustion #86613

Closed

openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 26, 2026

lukeboyett mentioned this pull request May 26, 2026

MemoryIndexManager.close() races with in-flight sync — provider/DB closed before sync settles #86702

Closed

chatgpt-codex-connector Bot reviewed May 26, 2026

View reviewed changes

Comment thread extensions/memory-core/src/memory/manager-sync-ops.ts Outdated

openclaw-barnacle Bot removed the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 26, 2026

clawsweeper Bot added the merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. label May 26, 2026

github-actions Bot mentioned this pull request May 26, 2026

🦞 OpenClaw 生态日报 2026-05-26 gsscsd/big_model_radar#396

Open

openclaw-barnacle Bot added size: L and removed size: M labels May 26, 2026

openclaw-barnacle Bot added triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. labels May 26, 2026

chatgpt-codex-connector Bot reviewed May 26, 2026

View reviewed changes

Comment thread extensions/memory-core/src/memory/manager-sync-ops.ts Outdated

clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 26, 2026

openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 26, 2026

github-actions Bot mentioned this pull request May 26, 2026

🦞 OpenClaw 生态日报 2026-05-26 ivanweng2077/big_model_radar#92

Open

osolmaz force-pushed the fix/memory-fd-fan-out-86613 branch from 803df6c to baa8161 Compare May 26, 2026 16:46

lukeboyett and others added 9 commits May 27, 2026 00:46

test(memory-core): add memory fd repro helper

e91d017

refactor(memory-core): pair native memory watchers

e27c28a

osolmaz force-pushed the fix/memory-fd-fan-out-86613 branch from baa8161 to e27c28a Compare May 26, 2026 16:47

github-actions Bot mentioned this pull request May 26, 2026

📡 Upstream Digest — 2026-05-26 18:18 UTC curtismercier/openclaw-mods#951

Open

quengh mentioned this pull request May 27, 2026

fix(memory): bound INDEX_CACHE lifetime in long-running gateway daemons #86345

Closed

This was referenced Jun 1, 2026

Linux memory watcher still allocates per-file watch descriptors #89182

Closed

fix(memory): warn on gateway watcher FD risk #89185

Merged

fix(memory-core): reduce Linux watcher fan-out #89188

Merged

clawsweeper Bot mentioned this pull request Jul 5, 2026

fix(memory): report close-time pending work failures #100308

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(memory-core): avoid per-file watcher FD fan-out for memory directories#86701

fix(memory-core): avoid per-file watcher FD fan-out for memory directories#86701
osolmaz merged 9 commits into
openclaw:mainfrom
lukeboyett:fix/memory-fd-fan-out-86613

lukeboyett commented May 26, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented May 26, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

clawsweeper Bot commented May 26, 2026 •

edited

Loading

Uh oh!

lukeboyett commented May 26, 2026

Uh oh!

clawsweeper Bot commented May 26, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

lukeboyett commented May 26, 2026

Uh oh!

clawsweeper Bot commented May 26, 2026 •

edited

Loading

Uh oh!

osolmaz commented May 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

lukeboyett commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Regression history

Related open PRs

Real behavior proof

What changes

Compatibility expectations

Acceptance criteria from clawsweeper review on #86613

Test plan

AI assistance

Notes for review

Codex review findings deferred to follow-up

Update — clawsweeper review addressed

Uh oh!

clawsweeper Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

clawsweeper Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Hatch command

Uh oh!

lukeboyett commented May 26, 2026

Uh oh!

clawsweeper Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

lukeboyett commented May 26, 2026

Uh oh!

clawsweeper Bot commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

osolmaz commented May 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

lukeboyett commented May 26, 2026 •

edited

Loading

clawsweeper Bot commented May 26, 2026 •

edited

Loading

clawsweeper Bot commented May 26, 2026 •

edited

Loading

clawsweeper Bot commented May 26, 2026 •

edited

Loading

clawsweeper Bot commented May 26, 2026 •

edited

Loading