Skip to content

fix(coding-agent): incremental single-flight session metadata scans - #2043

Open
snimu wants to merge 4 commits into
mainfrom
fix/incremental-session-info-scans
Open

fix(coding-agent): incremental single-flight session metadata scans#2043
snimu wants to merge 4 commits into
mainfrom
fix/incremental-session-info-scans

Conversation

@snimu

@snimu snimu commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Purpose

Session-list metadata scans treated append-only session files as immutable documents: every (size, mtime) change triggered a full re-read from byte 0, the read stream had no end bound (so a scan of an actively-appended file chases the moving EOF and holds its FD), and concurrent callers of readSessionInfo for the same path each opened their own duplicate stream. With ten daemon call sites (catalog list, RLM tree walks, ledger, supervisor) this is the read-amplification and FD-stampede half of the large-tree incidents.

Mechanism

One owner per session file for metadata scanning, in session-manager.ts:

  • Per-file scan state caches a fold accumulator plus the consumed byte offset; a changed file resumes scanning from that offset instead of byte 0.
  • Rewrites are detected by size shrink, same-size mtime change, or a consumed prefix that no longer ends with the recorded 16-byte tail; any of these restarts from byte 0.
  • Reads are bounded to the stat snapshot (readLinesAsBuffers gained an optional {start, end} range), so a growing file cannot extend a scan.
  • Concurrent readers of the same path share one in-flight scan.
  • A final line without its terminating newline (a torn in-progress append) folds into that scan's snapshot only, never into the resumable accumulator, so a later scan of the completed line cannot double-count.

All ten readSessionInfo call sites become readers of this one derivation; no call-site changes.

Measurement (72MB fixture: 200 sessions + one 24MB hot session)

scenario main this PR
initial catalog scan 72.1MB read, 138ms 72.1MB read, 172ms
15 refreshes, 11 files appended between each 396MB read, 634ms 0.1MB read, 86ms
30 concurrent readers of the freshly-appended hot file 720MB read, 30 streams, 1083ms 1 shared incremental scan, <1ms

Validation

  • Two pins verified fail-unfixed on main (shared in-flight scan identity; prefix not re-read after consumption), plus two guards for the rewrite-detection and torn-tail invariants the new mechanism must keep; one pass-through test for the bounded range.
  • test/session-manager (157), rlm-ledger (31 with file-lines) pass; root npm run check passes.

LOC

Total src: +323/−138 (net +185); tests: +137/−1 (net +136).
Src +219/−139 (net +80): mechanism change in one owner — full-rescan cache replaced by resumable accumulator + single-flight; no deletions elsewhere. Tests +115.

Squashes discussion #1536 and the per-child scan cost of #1671.

Linear: RES-1272 https://linear.app/primeintellect/issue/RES-1272


Note

Medium Risk
Core daemon hot path for session catalog metadata; incorrect resume or torn-tail handling could show stale counts or usage, though rewrite detection and tests mitigate this.

Overview
Replaces full-file session-list metadata rescans on every size/mtime change with resumable per-file scan state in session-manager.ts. Each path keeps a fold accumulator plus consumed byte offset, inode identity, and a short prefix tail so appends only read new bytes; shrink, inode/rename, or prefix mismatch forces a full rescan.

readSessionInfo now serializes concurrent readers on the same path (queued follow-up scans) and caps retained scan memory via LRU eviction of whole states. Scans are bounded to the file size at stat time through optional { start, end } on readLinesAsBuffers and a new readBytesSync helper in file-lines.ts. Incomplete trailing JSONL lines contribute to the returned snapshot only, not the persistent offset, avoiding double-count when the line completes.

Session directory listing drops scan state for removed or missing files. Tests cover concurrency, incremental resume, rewrite detection, and file recreation.

Reviewed by Cursor Bugbot for commit df032c1. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add incremental resumable single-flight session metadata scans in session-manager

  • Session metadata reads now persist per-file scan state (parsed metadata, usage aggregates, consumed-byte offset, prefix tail) so subsequent scans process only appended bytes instead of re-reading the whole file
  • Concurrent reads for the same path are serialized via per-file promise queues; unchanged followers return the cached result without re-scanning
  • Scans are bounded by the file size observed at scan start, and inode changes during a read trigger a single retry against the replacement file
  • readLinesAsBuffers and a new readBytesSync helper in file-lines.ts support byte-range reads, enabling prefix validation and range-bounded line streaming
  • Directory refreshes in listSessionsFromDir evict scan state for deleted or missing session files to prevent recreated paths from inheriting stale metadata
  • Risk: scans rely on inode-based identity and prefix-tail validation; same-length in-place edits within the already-consumed prefix are not detected as replacements and will not trigger a rescan

Macroscope summarized df032c1.

…ne shared in-flight scan per file

readSessionInfo re-read every session file from byte 0 whenever (size, mtime)
changed, streamed with no end bound past the stat snapshot on actively growing
files, and let concurrent callers stampede duplicate scans of the same path.
Scans now fold into a per-file accumulator resumed from the last consumed byte
offset (rewrites detected by shrink, same-size mtime change, or a changed
prefix tail), are bounded to the size seen at scan start, and concurrent
readers share one in-flight scan. A torn trailing line folds into the snapshot
only, never into the resumable accumulator. Fixes the defects reported in
discussions #1536 and the per-child scan cost of #1671.
…d bound resumable scan state

Review fixes for the incremental scan owner: a caller arriving after an
append could join an earlier in-flight scan and observe the pre-append
snapshot, so per-path scans now chain instead of joining (unchanged files
settle with one stat in the cached-hit path); rename rewrites that grow the
file while preserving the 16-byte tail window were resumed stale, so resume
now also requires an unchanged dev+ino; and resumable accumulators are LRU
bounded (1024 files) with eviction when a listed directory disappears, so
transcript-sized scan state cannot grow the daemon heap monotonically.
@snimu

snimu commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Addressed all three review findings in 41427fa:

  • Join-after-append staleness: readSessionInfo no longer joins an in-flight scan. Per-path scans chain strictly one at a time, so every caller's pass stats at or after its call time and sees every preceding append; unchanged files settle in the cached-hit path with a single stat, keeping the stampede collapse. Pin: reader arriving mid-scan after an append gets the post-append count (fails on the previous head with 20000 vs 20001).
  • Rewrite detection: resume now also requires unchanged dev+ino, which identifies every rename-based rewrite (_rewriteFile) including same-length prefix edits that grow the file and preserve the 16-byte tail window. Pin: temp+rename rewrite with preserved tail bytes rescans from byte 0 (fails on the previous head with the stale name). The prior in-place-edit test now pokes bytes via a positional writeSync and documents that same-inode interior edits are outside the writer model (append + rename rewrite) and intentionally not re-read.
  • Retention: resumable states are LRU-bounded to 1024 files (an evicted file pays one full rescan), a missing listed directory evicts its states before the early return, and ENOENT eviction is pinned by a delete/recreate-same-(size,mtime) test. The per-file assistantUsageById map stays: child_usage_attributed replaces the target's usage, so the fold needs the previous per-id value; the two unbounded arrays were already folded to running totals.

Bench after the changes (72MB fixture): 15 refresh cycles 0.1MB read / 44ms (main: 396MB / 634ms); 30 concurrent hot readers still collapse to one incremental scan.

Comment thread packages/coding-agent/src/core/session-manager.ts
Comment thread packages/coding-agent/src/core/session-manager.ts

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 41427fa. Configure here.

Comment thread packages/coding-agent/src/core/session-manager.ts
…rify inode after each scan

Review fixes for the resumable scan cache: the 1024-file LRU bounded entry
count, not memory (a huge transcript retains one usage record per assistant
message), and a listing larger than the cap evicted its own earlier entries,
re-paying full rescans every refresh. The cache is now bounded by total
retained usage entries (100k, roughly a few tens of MB worst case) with
whole-state LRU eviction only while over the bound, so small states never
thrash regardless of catalog size. And a rename rewrite racing a scan between
the pre-scan stat and the reads could mix two files' bytes into one cached
accumulator: the inode is now re-verified after each scan, discarding the
state and rescanning once when it changed.
@snimu

snimu commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the three findings in 8467701:

  • LRU bounds entries, not memory — fixed (merged with the thrash finding). Re-examined per-id retention first: dropping "settled" ids is not sound — an append-only file admits a future child_usage_attributed naming any prior assistant id (the attribution target is the parent's last assistant message at spawn time, and resident children keep flushing to that same old target indefinitely), so exact incremental folds need the previous per-id value with no settlement horizon. The bound is therefore on memory: total retained usage entries across the cache (100k; a usage record is a few hundred bytes, so roughly a few tens of MB worst case), with whole-state LRU eviction only while over the bound. Accounting is store-time (states grow between stores), and every eviction path (ENOENT, scan error, missing dir, listing sweep, LRU) routes through one accounting owner.
  • LRU thrash on listings larger than the cap — fixed by the same change. The fixed 1024-file cap is gone; states with small usage maps never evict, so a listing of any file count no longer evicts its own earlier entries. Eviction now only happens under real memory pressure.
  • stat -> prefix-check race — fixed. A rename rewrite landing between the pre-scan stat and the reads could mix two files' bytes into one cached accumulator. The inode is re-verified after every scan; a change discards the state and rescans once from scratch (bounded retry), so the mixed snapshot is neither cached nor served after the retry. No deterministic pin: the interleaving needs a seam between the stat and the read, which stays out of production code — same decline rationale as the mid-walk-append race on fix(coding-agent): memoize the passive RLM topology derivation #2051, and the mechanism (post-verify + drop) is the fix itself.

Resource bounds remain unpinned by prior agreement pattern (no behavioral observable without instrumentation); the bound and accounting are stated in the code. session-manager + file-lines + rlm-ledger suites 191/191; bench unchanged (15 refreshes: 0.1MB/46ms vs main 396MB/634ms).

…ts pins

Comment blocks collapse to one- or two-line invariants; the concurrency pins
merge into one serialized-scan test, the two rewrite-detection pins become one
table (rename vs truncate mode), and the torn-tail pin folds into the resume
test. Every fail-unfixed behavior keeps its assertion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant