perf(usage): OpenCode legacy walk stats entries before checking for symlinks

[tris203]

Problem

After #17254, a usage scan no longer reads OpenCode legacy message files older than the window, but it still walks them, and the walk is now most of the OpenCode scan. For every entry under <data dir>/storage/message it calls readLink and then stat, one entry at a time. readLink fails for everything that is not a symlink, so on a store of 29,998 legacy files in 796 session directories that is about 30,800 failed calls, each building an error, before the 30,800 stats.

Measured over those entries in isolation (sequential, Node v24.3.0, warm cache):

Per-entry work	Wall time
Failing readLink through Effect FileSystem	~1000 ms
stat through Effect FileSystem	~500 ms
Effect.fn span around the pair	~250 ms

Change

Stat entries first, four at a time in batches of 256, then handle each batch in directory order. Ask readLink only where the answer changes the result:

• before descending into a directory;
• before reading a file, or before letting a file be the proof that the store exists;
• when stat fails with something other than not-found, so a symlink loop or a link to an unreadable target is still skipped rather than reported as an error.

On the store above that is 30,794 stats and 796 readLink calls instead of 30,794 of each. Results are unchanged: same records, same file entries in the same order, same missing/error reporting, and symlinks are still never followed.

It stays on Effect's FileSystem, in line with #17615. That service has no lstat and readDirectory returns names only, so readLink remains the symlink test. A node:fs walk using dirent types measured about 90 ms faster for the walk alone, which did not seem worth leaving the service for.

Left out on purpose: skipping a whole session directory by its own mtime. OpenCode wrote these files in place (Bun.write(target, ...) in storage.ts from sst/opencode f993541e0b, September 2025, onwards), which does not update the parent directory's mtime, so that skip could drop in-window messages.

Scope and approval

Focused performance fix with no intended behavior change: one loop in one reader in packages/provider-opencode, plus tests. No prior issue or discussion.

Stacked on #17254. This branch contains that PR's commit (f8c09d0743) and one commit of its own (5309804d63); only the second is this change. The gain depends on #17254: without its mtime prefilter every legacy file is still read, so every file still needs its symlink check, and I measured no difference on main alone. Whichever of the two lands second can be rebased.

Verification

Environment: Linux, Node v24.3.0, real OpenCode data directory (~/.local/share/opencode, opened read-only, 29,998 legacy files / 500 MB, all older than the window), 30 day window (sinceMs = now - 30d - 36h). readOpenCodeUsage timed five times in one process with NodeServices.layer, with perf_hooks.monitorEventLoopDelay for the event-loop figure. The parent is #17254's head, f8c09d0743. Three alternating rounds; the machine was running other CPU-bound work at the time, which is why rounds differ.

	Wall time (ms)	Max event-loop delay (ms)
Parent, round 1	2448, 2279, 2321, 2276, 2775	37, 34, 34, 33, 2
This change, round 1	657, 566, 506, 622, 492	45, 49, 37, 37, 39
Parent, round 2	3231, 3089, 3101, 3056, 3026	42, 43, 44, 41, 3
This change, round 2	668, 601, 510, 539, 540	48, 37, 40, 42, 5
Parent, round 3	3551, 3008, 3069, 2985, 3038	40, 41, 43, 38, 2
This change, round 3	661, 549, 524, 558, 558	47, 39, 41, 9, 39

The ~35-45 ms event-loop delay is present in both and comes from the SQLite read. An earlier revision of this change (same walk, before the batches of 256) showed 1-4 ms when the legacy walk ran on its own, at concurrency 1, 4 and 8; I did not repeat that isolated run on the final revision.

Same results as the parent on the real store: for the 30 day window the 172 serialised records and their dedupe keys are identical. With sinceMs = 0, which reads the 3,358 legacy files not already in SQLite, the 84,050 records are identical.

Tests in packages/provider-opencode/src/server/usage.test.ts:

• never follows symlinks in the legacy OpenCode store: a symlinked message, a symlinked stale message, a symlinked directory, a cycle, a dangling link and a self-referencing link are all ignored without an error, and a store made only of links is reported missing. It fails if the symlink check is disabled.
• reads a large legacy OpenCode session in directory order: 600 messages in one directory come back as 600 file entries in readDirectory order, across batch boundaries.
• Both new tests also pass against the parent commit, so they pin existing behavior rather than new behavior.

Commands, run from packages/provider-opencode:

• vp test run src/server/usage.test.ts: 4 passed.
• vp lint and vp fmt on the two touched files: clean. tsc --noEmit: no errors.

Not checked: macOS or Windows; a cold page cache or a slow or network filesystem; the end-to-end Usage page load in a client. There is no committed test for a non-not-found stat failure on a regular entry (it needs a permission-denied fixture, which does not fail when tests run as root); a reviewer's scratch comparison of parent and this change covered EACCES, ELOOP and ENOTDIR cases with matching results.

Model: Claude Opus 5.5. Harness: Claude Code in T3 Code. Reviewed by GPT-6.1-Sol via Codex.

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This PR changes a production OpenCode usage/metering scan by adding an mtime-based skip gate and concurrent batched filesystem traversal, altering which legacy files reach aggregation. The scope is focused and tested, but its metering-path impact and automatic suppression of substantial processing warrant human review.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: 438eface-9382-41db-a18a-da8a48fdee04

---

📥 Commits

Reviewing files that changed from the base of the PR and between c85dc64 and 5309804.

---

📒 Files selected for processing (2)

• packages/provider-opencode/src/server/usage.test.ts
• packages/provider-opencode/src/server/usage.ts

---

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.

---

---

📝 Walkthrough

Walkthrough

Legacy OpenCode usage discovery now batches filesystem stats, skips symbolic links and stale or already-recorded JSON files, and processes eligible files. Tests cover modification-time filtering, symlink handling, and ordering across 600 files.

Changes

Legacy usage discovery

Layer / File(s)	Summary
Filter and process legacy store entries packages/provider-opencode/src/server/usage.ts, packages/provider-opencode/src/server/usage.test.ts	Discovery stats entries in batches, skips symbolic links and missing entries, and filters JSON files already in database records or modified before sinceMs. Other stat failures for non-symlinks propagate. Tests cover time filtering, symlink cases, and ordering across 600 messages.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Suggested reviewers: juliusmarminge

---

Merge Risk

Merge Risk: ⚪ Minimal · up to 53098

The reviewed batching change has no identified issue that needs resolution before merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 53098

The scan remains read-only and rejects linked files and directories before reading their contents or traversing them. However, it now accesses linked-target metadata before rejection. This is a limited boundary relaxation; content disclosure or privilege escalation has not been established.

Retained concerns

• Low · security · observed: Legacy symlinks are now followed for target metadata before rejection. A party able to place a symlink under a scanned store can cause target resolution outside that store under the scanner's existing OS permissions, although normal linked contents remain unread.

Security review details

Security Blast Radius

• inferred — The introduced reach is conditional on write access sufficient to create a symlink inside a scanned legacy store. Its target can lie outside that store, but access remains constrained by the scanner's existing filesystem permissions. The inspected path does not return target metadata or establish linked-content disclosure.

Security Findings and Attack Paths

• observed — The changed path is legacy entry to followed stat to subsequent symlink rejection. The base rejected a detected link before stat. This supports the limited metadata-boundary concern, not a verified content-disclosure or privilege-escalation finding.

Trust Boundaries and Controls

• observed — For stable paths, directory links are rejected before being queued and eligible JSON links are rejected before content reads or establishing store existence. Links whose stat fails are skipped when link detection succeeds. The symlink fixture checks output exclusion and missing/error behavior, but does not assert that target metadata was never accessed.

Resilience and Maintainability Implications

• inferred — Files, deduplication keys, and discovery/error flags belong to each invocation. Ordered consumption preserves earlier results before a later stat failure; non-not-found failures mark partial results while other queued directories can continue. Repetition rebuilds state, and interruption introduces no persistent write to roll back. The reader does not provide an atomic filesystem snapshot, and concurrent path replacement remains outside the tested guarantees.

Hardening Proposals

• proposed — If the intended no-follow guarantee includes metadata access, preserve link-first classification or use a service-supported non-following metadata operation. Otherwise, explicitly scope the guarantee to linked contents and directory traversal.

---

---

---

Pre-merge checks | 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check		The title clearly and concisely identifies the main performance change: statting OpenCode legacy-store entries before checking for symlinks.
Description check		The description is complete and follows the required template. It explains the problem, implementation, scope, approval rationale, focused verification, measured results, test coverage, and known limi…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

---

---

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}