fix(infra): scope Windows path realpath caches by 211-lee · Pull Request #108073 · openclaw/openclaw

211-lee · 2026-07-15T06:39:16Z

Summary

Windows lstat/realpath calls are expensive enough that repeated path-boundary checks can dominate startup and agent-path flows. The original version tried to fix that with process-wide realpath caches, but those caches sit on plugin, skill, hook, and sandbox containment checks. That lifetime is too broad: stale symlink or ancestor answers can survive after the filesystem changes.

This revision keeps the performance direction while moving the cache boundary to caller-owned operation scopes:

src/infra/path-guards.ts remains the direct @openclaw/fs-safe/path export surface; no process-wide realpath cache is installed there.
src/infra/boundary-path.ts accepts an optional caller-owned cache for resolvePathViaExistingAncestorSync.
Plugin hook/skill resolution and sandbox host-path validation now pass per-operation caches so repeated checks in one operation avoid duplicate canonicalization work.
Regression tests assert stale symlink/junction and ancestor changes are detected without global invalidation, including native Windows directory junction cleanup.

Closes #85262

Real behavior proof

Behavior addressed: reduces repeated Windows path canonicalization in plugin hook/skill and sandbox host-path operations without adding stale process-wide path answers to security-sensitive containment checks.

Real environment tested: local linked worktree for focused tests/lint/review; AWS Crabbox native Windows for focused runtime tests; AWS Crabbox Linux for check:changed; WSL2 runner acquisition was attempted on both AWS and Azure but did not produce a usable WSL2 test lane.

Exact steps or command run after this patch:

node scripts/run-vitest.mjs src/infra/path-safety.test.ts src/infra/boundary-path.test.ts src/hooks/plugin-hooks.test.ts src/skills/loading/plugin-skills.test.ts src/agents/sandbox/host-paths.test.ts src/agents/sandbox/validate-sandbox-security.test.ts src/agents/sandbox/workspace-mounts.test.ts src/agents/sandbox/fs-paths.test.ts
node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.core.json src/infra/path-safety.test.ts src/infra/boundary-path.test.ts src/hooks/plugin-hooks.ts src/skills/loading/plugin-skills.ts src/agents/sandbox/host-paths.ts src/agents/sandbox/validate-sandbox-security.ts src/agents/sandbox/workspace-mounts.ts src/agents/sandbox/fs-paths.ts src/agents/sandbox/fs-paths.test.ts
git diff --check origin/main...HEAD
.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main
node scripts/crabbox-wrapper.mjs run --provider aws --target windows --id cbx_dc20a584615f --reclaim --no-sync --label windows-realpath-85264-native-tests --idle-timeout 45m --ttl 90m --timing-json --stop-after never --shell -- 'node scripts/run-vitest.mjs src/infra/path-safety.test.ts src/infra/boundary-path.test.ts src/hooks/plugin-hooks.test.ts src/skills/loading/plugin-skills.test.ts src/agents/sandbox/host-paths.test.ts src/agents/sandbox/validate-sandbox-security.test.ts src/agents/sandbox/workspace-mounts.test.ts src/agents/sandbox/fs-paths.test.ts'
node scripts/crabbox-wrapper.mjs run --provider aws --target linux --label windows-realpath-85264-check-changed --idle-timeout 90m --ttl 180m --timing-json --stop-after always --shell -- 'env OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 CI=1 corepack pnpm check:changed'

Evidence after fix:

Local focused Vitest at pushed head b1d8191b27: 5 shards passed, 114 tests.
Local oxlint/diff check at pushed head b1d8191b27: passed.
Final autoreview at pushed head b1d8191b27: clean, no accepted/actionable findings.
AWS Crabbox native Windows focused proof: provider=aws, lease=cbx_dc20a584615f, run=run_b4c210ac8166, hydrate run https://github.com/openclaw/openclaw/actions/runs/26719083996. Focused Vitest passed 5 shards: 111 passed, 3 skipped.
AWS Crabbox changed gate: provider=aws, lease=cbx_ab9a2c3903d5, run=run_4137bbc8c85d, pnpm check:changed passed.

Observed result after fix: repeated path canonicalization inside a single plugin/sandbox operation can reuse one caller-owned cache, while a later operation re-reads current filesystem state and blocks stale symlink/junction escapes.

What was not tested: full end-to-end Windows startup profiling at the current rebased head b1d8191b27 (performance data was collected at prior heads; the rebase was conflict-resolution only with no logic changes). The manifest-registry-installed lstat hotspot is intentionally out of scope and tracked in #90362.

Remaining performance gap

The largest remaining lstat gap is in manifest-registry-installed.ts, where buildInstalledManifestRegistryIndexKey() is called ~179 times per config validation, each time creating a new scoped Map(). This is intentionally left as a follow-up in #90362 — the current PR focuses on the plugin hook/skill and sandbox containment paths. The manifest-registry optimization can be addressed in a separate PR without changing the security-boundary cache lifetime design landed here.

Runner notes

The AWS native Windows image had Node after GitHub Actions hydration but not node_modules; pnpm install --frozen-lockfile hit Windows EMFILE twice near finalization. Vitest was nevertheless present and resolvable, and the focused native Windows test command passed.
The current branch was rebased and pushed to b1d8191b27; origin/main continued moving during validation, so the final broad gate is recorded against the rebased head before that later churn.

Windows Performance Proof

Environment

Windows 10 Pro, NTFS, Node.js v24.15.0
OpenClaw v2026.5.18 (installed version)
94 bundled plugins, 5 enabled (browser, device-pair, phone-control, talk-voice, octo)

Method

CPU profiling via Node.js inspector API, 120s coverage: startup → ready → idle (no webchat connection).

Scoped Cache vs Global Cache vs No Cache

Metric	No cache (baseline)	Global cache	Scoped cache (current head)
`lstat` total	47,834ms	8,016ms (-83%)	21,376ms (-55%)
Startup to ready	~17s	8.7s	14.0s
Profile	`gateway-1779357547869.cpuprofile`	`gateway-1779440772524.cpuprofile`	`gateway-1780375536440.cpuprofile`

Why scoped cache is slower than global cache (+13s)

The gap is not from "other callers not passing a cache" — it is from the largest single caller manifest-registry-installed.ts:

function buildInstalledManifestRegistryIndexKey(index) {
  const realpathCache = new Map(); // ← new Map on every call
  index.plugins.map((record) => resolvePackageJsonPath(record, realpathCache));
}
// Called ~179 times (once per config validation), each time starting with an empty Map

Global cache: 1st call fills ~188 paths, next 178 calls all hit → actual lstat ~188 times
Scoped cache: 179 calls × ~188 misses each (new Map) → actual lstat ~33,600 times
33,600 × ~500µs ≈ 16.8s — closely matches the measured 13.4s gap

The scoped-cache approach still delivers a 55% reduction in lstat time and 3s faster startup compared to no cache, while avoiding stale process-wide cache answers in security-sensitive containment checks.

Cache Behavior (instrumented dist, global-cache version, call counting)

Metric	Value
`safeRealpathSync` total calls	66,634
Cache hits	65,784 (98.7%)
Cache misses (actual lstat)	850
`isPathInsideWithRealpath` calls	15,937
Global cache entries	292

Per-phase breakdown:

Phase	safeRealpath	CacheHit	HitRate	isPathInside
startup (→ready)	10,300	9,942	96.5%	2,353
ready-idle	6,706	6,666	99.4%	1,566
webchat-api	9,874	9,822	99.5%	2,614
chat-processing	35,907	35,531	99.0%	8,273
post-reply	3,847	3,823	99.4%	1,131

Cache entries = 3 per plugin (root dir + package.json + entry file).
292 entries × ~200 bytes = ~58KB memory. Bounded by plugin count.

Cache growth test (after installing @openclaw/feishu)

Calls	cacheSize	Hits	HitRate
1	0	0	—
1,000	292	699	early fill phase
5,000	292	4,651	93%
10,000	292	9,635	96.4%
50,000	301	49,338	98.7%

+1 plugin = +3 cache entries (+~600 bytes). Cache is bounded.

Reproducing

# Profile (requires gateway not already running):
node profile-gateway-internal.mjs
# → opens webchat, send a message, wait 120s
# → outputs .cpuprofile file

# Analyze:
node analyze-lstat-deep.mjs <file>.cpuprofile

Rebase note (2026-07-06): Rebased onto latest origin/main (e5a605424e) — conflict-resolution only, no logic changes. New head: b1d8191b27.

Note: This is a re-submission of #85264 which was auto-closed by the stale bot due to inactivity. The code changes are identical — only the Android camera logging file has been excluded from this PR.

Move realpath cache boundary from process-wide to caller-owned scopes to prevent stale symlink/ancestor answers in security-sensitive containment checks while maintaining performance. Changes: - Add hostPathCache to fs-paths.ts mount resolution - Accept cache parameter in resolvePathViaExistingAncestorSync - Create per-operation caches in plugin hooks and skills resolution - Use scoped caches in sandbox host-path validation - Fix boundary-path.ts export conflict (remove duplicate export) - Add regression tests for stale symlink/junction detection Performance: 55% reduction in lstat time on Windows (21,376ms vs 47,834ms baseline) Fixes openclaw#85262

clawsweeper · 2026-07-16T06:48:43Z

Codex review: needs maintainer review before merge. Reviewed July 15, 2026, 10:45 PM ET / July 16, 2026, 02:45 UTC.

Summary
The PR adds caller-scoped realpath caches to plugin, skill, and sandbox path-containment operations, with regression tests for stale symlink, junction, and ancestor changes.

PR surface: Source +77, Tests +127. Total +204 across 10 files.

Reproducibility: yes. at the source-and-profile level: repeated uncached canonicalization is clear from the call paths and the contributor supplied baseline and after-fix Windows profiles, but this review did not independently reproduce the current-main slowdown.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: persistent cache schema: src/agents/sandbox/host-paths.ts, persistent cache schema: src/hooks/plugin-hooks.ts, persistent cache schema: src/infra/boundary-path.test.ts, persistent cache schema: src/infra/boundary-path.ts, persistent cache schema: src/infra/path-safety.test.ts, persistent cache schema: src/skills/loading/plugin-skills.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: canonical
Canonical: #108073
Summary: This PR is the active replacement implementation for the closed performance report and stale predecessor; the manifest-registry hotspot is a distinct follow-up.

Members:

fixed_by_candidate: Windows: lstat bottleneck causes 2-3x slower performance vs Mac (59% of CPU time) #85262 - The issue reports the Windows lstat bottleneck this PR is intended to fix.
superseded: fix(infra): scope Windows path realpath caches #85264 - The closed predecessor contains the same central patch lineage; this resubmission removes unrelated churn and carries the revised scoped-cache design.
adjacent_distinct: perf: optimize manifest-registry-installed lstat hotspot on Windows #90362 - That issue concerns the separate manifest-registry-installed hotspot intentionally excluded from this branch.

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

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster ✨ media proof bonus
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:

Rebase onto current main, preserve the reviewed operation-scoped lifetime, and rerun the focused Windows path and sandbox tests on the resolved head.

Risk before merge

[P1] The branch currently conflicts with main; conflict resolution could accidentally widen cache lifetime or omit newer sandbox-path behavior unless the resolved diff receives a fresh review.
[P1] These caches participate in plugin and sandbox containment decisions, so a future refactor that retains a Map beyond one synchronous operation could reintroduce stale security decisions even though this head avoids process-wide caching.
[P1] The linked performance profiles were collected across earlier equivalent heads rather than a full startup profile after the latest rebase, so the exact end-to-end gain on the eventual merge head remains unconfirmed.

Maintainer options:

Rebase without widening cache lifetime (recommended)
Resolve conflicts against current main, preserve one-operation Map ownership, and rerun the focused path, plugin, skill, and sandbox validation before owner review.
Pause if the resolved lifetime is ambiguous
Do not merge if conflict resolution requires shared or long-lived caches without an explicit invalidation and lifecycle contract.

Next step before merge

[P2] A maintainer should review the conflict-resolved head and approve the security-boundary cache lifetime; there is no narrow code defect for an automated repair worker on the reviewed head.

Maintainer decision needed

Question: After the branch is rebased, should OpenClaw accept these caller-scoped cache lifetimes for plugin and sandbox containment checks?
Rationale: The implementation appears technically sound, but the affected checks enforce security boundaries and the current merge conflicts could change the reviewed cache ownership or caller behavior.
Likely owner: vincentkoc — The current infra path-wrapper history and recent adjacent sandbox work make this the strongest available owner signal for the cross-cutting cache contract.
Options:
- Rebase and approve scoped caches (recommended): Resolve the main conflicts without widening cache lifetime, rerun the focused Windows and sandbox tests, and obtain an owner review of the resolved containment paths.
- Pause the optimization: Keep live uncached checks until maintainers define a different lifecycle-owned caching contract for these security-sensitive paths.

Security
Cleared: No concrete security regression was found: caches are caller-owned and operation-scoped, while later operations return to live fs-safe resolution.

Review details

Best possible solution:

Land only an operation-owned cache design that is created and discarded within one synchronous resolution pass, preserves live filesystem reads between operations, and keeps stale-alias regression coverage around every security-sensitive caller.

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

Yes at the source-and-profile level: repeated uncached canonicalization is clear from the call paths and the contributor supplied baseline and after-fix Windows profiles, but this review did not independently reproduce the current-main slowdown.

Is this the best way to solve the issue?

Yes, subject to rebase review: caller-owned per-operation caches are narrower and safer than the rejected process-wide approach while retaining measurable Windows performance benefit.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

add P2: This is a normal-priority Windows performance improvement with meaningful but limited platform impact and no demonstrated current outage.
add merge-risk: 🚨 security-boundary: The changed realpath and ancestor-resolution calls decide whether plugin, skill, and sandbox paths remain inside permitted roots.
add proof: sufficient: Contributor real behavior proof is sufficient. The PR provides after-fix native Windows focused results, Linux changed-gate output, and linked CPU-profile artifacts showing both performance improvement and stale-alias protection; private details should remain redacted in any additional proof.
add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The PR provides after-fix native Windows focused results, Linux changed-gate output, and linked CPU-profile artifacts showing both performance improvement and stale-alias protection; private details should remain redacted in any additional proof.

Label justifications:

P2: This is a normal-priority Windows performance improvement with meaningful but limited platform impact and no demonstrated current outage.
merge-risk: 🚨 security-boundary: The changed realpath and ancestor-resolution calls decide whether plugin, skill, and sandbox paths remain inside permitted roots.
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 (linked_artifact): The PR provides after-fix native Windows focused results, Linux changed-gate output, and linked CPU-profile artifacts showing both performance improvement and stale-alias protection; private details should remain redacted in any additional proof.
proof: sufficient: Contributor real behavior proof is sufficient. The PR provides after-fix native Windows focused results, Linux changed-gate output, and linked CPU-profile artifacts showing both performance improvement and stale-alias protection; private details should remain redacted in any additional proof.

Evidence reviewed

PR surface:

Source +77, Tests +127. Total +204 across 10 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	7	103	26	+77
Tests	3	131	4	+127
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	10	234	30	+204

What I checked:

Scoped cache design: The branch passes new Maps from bounded plugin, skill, and sandbox operations instead of installing the process-wide realpath caches rejected during the earlier review. (src/hooks/plugin-hooks.ts:45, b1d8191b2712)
Live-filesystem regression coverage: The added path tests replace symlinks or junctions between operations and assert that later uncached operations observe the changed target. (src/infra/path-safety.test.ts:32, b1d8191b2712)
Dependency contract: The upstream fs-safe implementation accepts an optional caller-owned Map, reads the live filesystem on cache misses, and does not establish a process-global cache. (github.com) (src/infra/path-safety.ts:1, b1d8191b2712)
Prior review continuity: The earlier PR review specifically requested request-local or operation-scoped caching plus stale-symlink and ancestor regression tests; this resubmission implements that requested shape. (github.com) (src/infra/boundary-path.ts:5, b1d8191b2712)
Windows behavior proof: The PR body reports native Windows focused execution with 111 passing tests and 3 platform skips, Linux changed-gate success, and linked CPU profiles showing scoped-cache improvement over the uncached baseline. (b1d8191b2712)
Current merge state: GitHub reports the PR head as dirty and not cleanly mergeable against the current base, so conflict resolution must be reviewed before the branch can land. (b1d8191b2712)

Likely related people:

vincentkoc: Prior path-safety history attributes the current infra wrappers to this contributor, and recent sandbox refactoring also touched the affected owner boundary. (role: introduced current wrapper boundary; confidence: high; commits: a4c81c6f3583, 53ff3085f9f3; files: src/infra/path-safety.ts, src/infra/boundary-path.ts, src/agents/sandbox/validate-sandbox-security.ts)
mmaps: Authored the recent bind-source validation hardening that defines the security policy this PR now accelerates. (github.com) (role: sandbox security contributor; confidence: high; commits: a90eb93452f1; files: src/agents/sandbox/validate-sandbox-security.ts, src/agents/sandbox/validate-sandbox-security.test.ts)
steipete: Recent commits maintain and refactor the sandbox validation surface, including the latest recorded touch before this PR. (github.com) (role: recent area contributor; confidence: medium; commits: 9092e59a43b1, 00d8d7e, 5acfc89; files: src/agents/sandbox/validate-sandbox-security.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.

211-lee requested a review from a team as a code owner July 15, 2026 06:39

openclaw-barnacle Bot added agents Agent runtime and tooling size: M labels Jul 15, 2026

BradGroux mentioned this pull request Jul 15, 2026

WORKING: All Microsoft Issues and PRs (refresh) #74163

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(infra): scope Windows path realpath caches#108073

fix(infra): scope Windows path realpath caches#108073
211-lee wants to merge 1 commit into
openclaw:mainfrom
211-lee:fix/windows-lstat-global-cache-v2

211-lee commented Jul 15, 2026

Uh oh!

clawsweeper Bot commented Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

211-lee commented Jul 15, 2026

Summary

Real behavior proof

Remaining performance gap

Runner notes

Windows Performance Proof

Environment

Method

Scoped Cache vs Global Cache vs No Cache

Why scoped cache is slower than global cache (+13s)

Cache Behavior (instrumented dist, global-cache version, call counting)

Cache growth test (after installing @openclaw/feishu)

Reproducing

Uh oh!

clawsweeper Bot commented Jul 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant