fix(usage-bar): bound template file cache to prevent unbounded watche… by chenyangjun-xy · Pull Request #98990 · openclaw/openclaw

chenyangjun-xy · 2026-07-02T07:36:41Z

What Problem This Solves

Fixes #98960: The module-level fileCache Map in src/auto-reply/usage-bar/template.ts holds a live fs.watch watcher for each cached template file path. The cache grows without bound — entries are never evicted, and watchers are only closed in clearUsageBarTemplateCacheForTest(). Over time this leaks file descriptors.

Root Cause

cacheTemplateFile() inserts without checking cache size. Each new file path creates a persistent fs.watch watcher that is never released in production.

Why This Change Was Made

Add a maximum cache size (MAX_CACHED_TEMPLATE_FILES = 64). When inserting a new cache key that would exceed the limit, evict the oldest entry (closing its fs.watch watcher) before allocating a watcher for the new entry. This bounds watcher count and prevents unbounded FD growth.

Key design decisions:

Eviction runs before watcher allocation so we never create a watcher only to close it immediately.
Eviction triggers only when inserting a new key (!fileCache.has(path)). Retries for an existing key (re-reading after a prior miss) must not evict other entries.
Eviction uses Map.keys().next() for oldest-entry ordering (Map maintains insertion order).

Changes

src/auto-reply/usage-bar/template.ts: +13 lines
- Added MAX_CACHED_TEMPLATE_FILES = 64 constant
- Added oldest-entry eviction with watcher cleanup in cacheTemplateFile(), before watcher allocation, guarded by !fileCache.has(path)
src/auto-reply/usage-bar/template.test.ts: +79/-1 lines
- Two new focused tests in a cache eviction describe block

Evidence

Verification environment

Item	Value
OS	Linux 4.19.112 x86_64
Node.js	v22.19.0
Vitest	v4.1.8

Regression tests (12/12, 2 new)

 ✓ returns the built-in template when unset
 ✓ returns an inline template object when usable
 ✓ falls back to the built-in template for an unusable inline object
 ✓ falls back quietly for an empty inline template
 ✓ loads and parses a template file
 ✓ falls back to the built-in template for invalid JSON
 ✓ falls back to the built-in template for an empty template file
 ✓ reloads a path after an initial miss
 ✓ reloads a path after invalid JSON is fixed
 ✓ serves the cached template without re-reading the file
 ✓ evicts the oldest entry and closes its watcher when inserting a new key over the limit  ← NEW
 ✓ does not evict when retrying the same key after a prior miss                         ← NEW
 Test Files  1 passed (1) | Tests  12 passed (12)

The two new tests cover:

Overflow eviction: 65 paths loaded → oldest evicted → re-read from disk proves eviction + watcher closure.
Watcher closure: Evicted entry's watcher is closed; re-access triggers fresh readFileSync instead of serving stale data. Non-evicted entry remains cached.
Same-key retry: Cache full → invalid file loaded (slot consumed) → file fixed → retry same path → does NOT evict other entries.

Real-process proof: 65-template load (production code, no mocking)

Standalone script (scripts/verify-template-cache-bound.mjs) imports the production loadUsageBarTemplate module via tsx and exercises it with 65 real template files. No mocking — this is a real Node process running the actual production code path.

========================================================================
OpenClaw Template Cache Bound — Real-Process Verification
========================================================================
PID:       681079
Node:      v22.19.0
Platform:  linux x64
Started:   2026-07-03T02:41:33.053Z
Temp dir:  /tmp/usage-template-proof-pxStOa
Cache cap: 64
Files:     65

── Phase 1: Fill cache (files 0–63) ──
  Duration: 6ms
  Result:   64 files loaded and cached

── Phase 2: Load 65th file (triggers eviction of oldest entry) ──
  Duration: 0ms
  File 64:  {"segments":[{"text":"v1-64"}]}

── Phase 3: Non-evicted file 1 — prove watcher still alive ──
  File 1 reloaded: "CHANGED-VIA-WATCHER"
  Result:          PASS (live watcher updated cache)

── Phase 4: Verify files 2–63 still cached ──
  Cached: 62/62
  Result: PASS

── Phase 5: Prove eviction (modify file 0 on disk, re-read) ──
  File 0 reloaded: "V2-EVICTED-RELOADED"
  Result:          PASS (disk re-read — evicted watcher was closed)

── Phase 6: Cleanup ──
  Result: clearUsageBarTemplateCacheForTest called

========================================================================
VERDICT
========================================================================
  64 files loaded → all cached                         PASS
  65th file → eviction + watcher close                    PASS
  Non-evicted watcher still alive                          PASS
  62 files remain cached                                  PASS
  cleanup → all watchers closed                            PASS

Direct watcher create/close measurement (Vitest)

Verification test (src/auto-reply/usage-bar/template.watcher-proof.test.ts) uses vi.mock + vi.hoisted to intercept fs.watch before module resolution and directly count every FSWatcher creation and close() call.

========================================================================
Template Cache Bound — Direct Watcher & Eviction Measurement
========================================================================
Phase 1: Load 64 files → 64 watchers created, 0 closed
Phase 2: Load 65th file → 1 created, 1 closed (eviction)
  Content: {"segments":[{"text":"v1-64"}]}
  Oldest watcher CLOSED, new watcher CREATED
Phase 3: Re-read all 63 cached files → 0 created, 0 closed
Phase 4: clearUsageBarTemplateCacheForTest → 64 watchers closed
  Remaining active: 0

========================================================================
VERDICT — Direct watcher measurement
========================================================================
  Watchers created : 65
  Watchers closed  : 65
  Leaked           : 0

  ✅ 64 files → 64 watchers
  ✅ 65th file → 1 watcher CLOSED (eviction) + 1 CREATED
  ✅ Cache hits → 0 created/closed
  ✅ Cleanup → all remaining closed, 0 leaked

Measurement methodology:

Intercept point	What it counts	Phase 2 result
Mock `fs.watch`	`state.created++` per watcher allocated	+1 (65th file)
Instance `w.close = fn`	`state.closed++` per watcher closed	+1 (oldest evicted)

Claim	Measurement	Result
Cache bound at 64	64 watchers created in Phase 1, eviction triggers on 65th	PASS
Oldest watcher CLOSED on eviction	`state.closed` increments by 1 in Phase 2	PASS
New watcher CREATED for 65th file	`state.created` increments by 1 in Phase 2	PASS
Cache hits: no new watchers	0 created, 0 closed in Phase 3	PASS
Cleanup closes all (no leaks)	64 closed in Phase 4, `created - closed = 0`	PASS

User Impact

Watcher/FD usage bounded at 64 entries instead of growing indefinitely
Transparent to users; no config changes required
Evicted template files are automatically reloaded (with a fresh watcher) on next access

🤖 Generated with Claude Code

clawsweeper · 2026-07-02T08:12:13Z

Codex review: needs changes before merge. Reviewed July 6, 2026, 4:05 AM ET / 08:05 UTC.

Summary
The branch caps the usage-bar template file cache at 64 entries, evicts and closes the oldest watcher before creating a new one, and adds regression plus verification coverage.

PR surface: Source +13, Tests +204, Other +123. Total +340 across 4 files.

Reproducibility: yes. Source inspection shows current main and v2026.6.11 store watcher-backed usage-template cache entries without a size cap, and CI reproduces the PR's new watcher-proof test crash on the latest head.

Review metrics: 1 noteworthy metric.

Runtime cache cap: 1 cap added: 64 template file paths. The selected bound intentionally changes long-running watcher retention, so maintainers should notice the cap before merge.

Stored data model
Persistent data-model change detected: persistent cache schema: scripts/verify-template-cache-bound.mjs, persistent cache schema: src/auto-reply/usage-bar/template.test.ts, persistent cache schema: src/auto-reply/usage-bar/template.watcher-proof.test.ts, serialized state: scripts/verify-template-cache-bound.mjs, serialized state: src/auto-reply/usage-bar/template.watcher-proof.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #98960
Summary: This PR is a candidate fix for the canonical usage-template fileCache/fs.watch watcher leak issue, with one narrow test-harness repair still needed before merge.

Members:

canonical: usage-bar: template file cache grows unbounded with live fs.watch watchers #98960 - The issue describes the same unbounded usage-template file cache and live fs.watch watcher behavior, and this PR uses closing syntax for it.
partial_overlap: fix(usage-bar): evict oldest template cache entry when full #98853 - The older open PR targets the same cache-bound problem but has weaker proof and a competing implementation shape.
superseded: fix(usage-bar): evict oldest template cache entry when size exceeds limit #99358 - That closed unmerged PR attempted the same one-file cache cap and was superseded by stronger current candidates.
adjacent_distinct: fix(gateway-lock): close fd and remove partial lock file on writeFile failure #99356 - That closed PR is another file-descriptor cleanup item, but it targets gateway-lock write failure handling rather than the usage-template watcher cache.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦞 diamond lobster
Patch quality: 🦪 silver shellfish
Result: blocked by patch quality or review findings.

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

Rank-up moves:

Pass afterEach into useAutoCleanupTempDirTracker in the new watcher-proof test and rerun the focused usage-bar tests.
Rerun or wait for the affected CI checks so the latest head has green test evidence.

Next step before merge

[P2] A narrow automated repair can fix the incorrect test-helper call; maintainer review can continue once focused tests and CI are green.

Security
Cleared: The diff changes internal cache lifecycle code, tests, and a local verification script without adding dependencies, workflows, permissions, downloads, package resolution, or secret handling.

Review findings

[P2] Pass the cleanup registrar to the temp-dir tracker — src/auto-reply/usage-bar/template.watcher-proof.test.ts:45

Review details

Best possible solution:

Keep the bounded cache fix, but repair the new watcher-proof test so it uses the shared temp-dir helper correctly and the focused node shard is green before merge.

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

Yes. Source inspection shows current main and v2026.6.11 store watcher-backed usage-template cache entries without a size cap, and CI reproduces the PR's new watcher-proof test crash on the latest head.

Is this the best way to solve the issue?

No as submitted. The runtime fix is the right narrow owner-module solution, but the new watcher-proof test must pass afterEach to useAutoCleanupTempDirTracker before the PR is merge-ready.

Full review comments:

[P2] Pass the cleanup registrar to the temp-dir tracker — src/auto-reply/usage-bar/template.watcher-proof.test.ts:45
useAutoCleanupTempDirTracker requires a cleanup registrar, and existing call sites pass afterEach. This new test calls it with no argument, so the latest node shard fails before the suite runs with TypeError: registerCleanup is not a function; import/pass afterEach here so the watcher proof actually executes.
Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.96

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against d0fdfe845b61.

Label changes

Label changes:

add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦞 diamond lobster and patch quality is 🦪 silver shellfish. Replaced prior rating: 🐚 platinum hermit.
remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.

Label justifications:

P2: This PR fixes a concrete resource-growth bug in a limited usage-bar runtime path, but the latest head has a narrow blocking test failure.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦞 diamond lobster and patch quality is 🦪 silver shellfish. Replaced prior rating: 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes after-fix live output from a real Node process plus watcher create/close measurement; the remaining blocker is the committed test harness crash, not missing behavior proof.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes after-fix live output from a real Node process plus watcher create/close measurement; the remaining blocker is the committed test harness crash, not missing behavior proof.

Evidence reviewed

PR surface:

Source +13, Tests +204, Other +123. Total +340 across 4 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	13	0	+13
Tests	2	212	8	+204
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	1	123	0	+123
Total	4	348	8	+340

Acceptance criteria:

[P1] pnpm test src/auto-reply/usage-bar/template.watcher-proof.test.ts.
[P1] pnpm test src/auto-reply/usage-bar/template.test.ts.
[P1] pnpm check:test-types.

What I checked:

Current main still has unbounded watcher-backed cache insertion: cacheTemplateFile creates an fs.watch watcher for valid templates and stores every path in fileCache with no size cap or production eviction path. (src/auto-reply/usage-bar/template.ts:123, d0fdfe845b61)
Latest release still has the same unbounded path: v2026.6.11 contains the same watcher creation and fileCache.set(path, entry) behavior without eviction, so the requested bug fix has not shipped. (src/auto-reply/usage-bar/template.ts:123, e085fa1a3ffd)
Runtime caller reaches the template cache: resolveResponseUsageLine calls loadUsageBarTemplate for full usage rendering, so the cache path is reachable from reply usage output. (src/auto-reply/reply/agent-runner-usage-line.ts:98, d0fdfe845b61)
PR implements the bounded-cache fix: The PR adds MAX_CACHED_TEMPLATE_FILES = 64 and evicts/closes the oldest cached watcher before allocating the watcher for a new key. (src/auto-reply/usage-bar/template.ts:130, 989823f66469)
Helper contract requires a cleanup registrar: useAutoCleanupTempDirTracker requires a registerCleanup argument; existing call sites pass afterEach. (test/helpers/temp-dir.ts:58, d0fdfe845b61)
Latest CI reproduces the new test failure: The current head's node test shard fails before running template.watcher-proof.test.ts with TypeError: registerCleanup is not a function at line 45. (src/auto-reply/usage-bar/template.watcher-proof.test.ts:45, 989823f66469)

Likely related people:

Peter Lindsey: Authored the native usage-template renderer and the later watcher-backed cache performance change that this PR bounds. (role: introduced behavior; confidence: high; commits: 64d0fc83364e, 069cb8d63687; files: src/auto-reply/usage-bar/template.ts, src/auto-reply/usage-bar/template.test.ts, src/auto-reply/reply/agent-runner-usage-line.ts)
obviyus: Prior ClawSweeper/GitHub metadata and local history connect this handle to follow-up usage-template fixes in the same module. (role: recent area contributor; confidence: medium; commits: 069cb8d63687, ff6940036b43, afe75b3387d2; files: src/auto-reply/usage-bar/template.ts, src/auto-reply/usage-bar/template.test.ts, docs/concepts/usage-tracking.md)
Vincent Koc: Authored the shared temp-dir helper contract that the new watcher-proof test uses incorrectly. (role: adjacent helper owner; confidence: medium; commits: ce50b97c8661; files: test/helpers/temp-dir.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.

Review history (1 earlier review cycle)

reviewed 2026-07-03T07:23:06.425Z sha 782974f :: needs maintainer review before merge. :: none

chenyangjun-xy · 2026-07-02T08:58:24Z

@clawsweeper re-review

clawsweeper · 2026-07-02T08:58:28Z

🦞🧹
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.

…r growth Add MAX_CACHED_TEMPLATE_FILES=64 limit; evict the oldest entry (closing its fs.watch watcher) before allocating a watcher for a new key when the cache is full. Eviction runs before watcher allocation so we never create a watcher only to close it immediately. Eviction triggers only when inserting a new key (!fileCache.has(path)) — retries for an existing key must not evict other entries. Fixes openclaw#98960 Co-Authored-By: Claude <[email protected]>

…rn in template.test.ts Co-Authored-By: Claude <[email protected]>

Co-Authored-By: Claude <[email protected]>

openclaw-barnacle Bot added the size: XS label Jul 2, 2026

clawsweeper Bot mentioned this pull request Jul 2, 2026

fix(usage-bar): evict oldest template cache entry when full #98853

Closed

openclaw-barnacle Bot added size: S and removed size: XS labels Jul 2, 2026

clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jul 2, 2026

chenyangjun-xy force-pushed the fix/issue-98960-template-cache-unbounded branch from f69f8f6 to 54da7f8 Compare July 2, 2026 11:51

openclaw-barnacle Bot added scripts Repository scripts size: M and removed size: S labels Jul 2, 2026

clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 2, 2026

chenyangjun-xy force-pushed the fix/issue-98960-template-cache-unbounded branch 3 times, most recently from 2bb1975 to c978b0f Compare July 2, 2026 12:49

chenyangjun-xy force-pushed the fix/issue-98960-template-cache-unbounded branch from 6e123f9 to a5eb6f4 Compare July 3, 2026 03:06

This was referenced Jul 3, 2026

fix(gateway-lock): close fd and remove partial lock file on writeFile failure #99356

Closed

fix(usage-bar): evict oldest template cache entry when size exceeds limit #99358

Closed

chenyangjun-xy and others added 4 commits July 6, 2026 01:05

refactor(test): replace mutable dir variable with cleanup-stack patte…

0a9dfc2

…rn in template.test.ts Co-Authored-By: Claude <[email protected]>

chore(test): add curly braces for eslint curly rule

a09612d

Co-Authored-By: Claude <[email protected]>

test(usage-bar): register watcher proof cleanup

d744d6f

clawsweeper Bot mentioned this pull request Jul 6, 2026

build(deps): bump github.com/steipete/commander from 0.2.2 to 0.2.3 in /apps/swabble in the swift-deps group across 1 directory #100759

Closed

zenglingbiao mentioned this pull request Jul 18, 2026

fix(usage-bar): clear dead watcher reference after transient error #109462

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(usage-bar): bound template file cache to prevent unbounded watche…#98990

fix(usage-bar): bound template file cache to prevent unbounded watche…#98990
vincentkoc merged 4 commits into
openclaw:mainfrom
chenyangjun-xy:fix/issue-98960-template-cache-unbounded

chenyangjun-xy commented Jul 2, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

Uh oh!

chenyangjun-xy commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

chenyangjun-xy commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Root Cause

Why This Change Was Made

Changes

Evidence

Verification environment

Regression tests (12/12, 2 new)

Real-process proof: 65-template load (production code, no mocking)

Direct watcher create/close measurement (Vitest)

User Impact

Uh oh!

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

Uh oh!

Uh oh!

chenyangjun-xy commented Jul 2, 2026

Uh oh!

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

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chenyangjun-xy commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading