Skip to content

feat(server): persist agent sessions on SSD with a disk budget and TTL - #839

Merged
davide221 merged 3 commits into
Luce-Org:mainfrom
Graffioh:feat/disk-session-cache
Oct 13, 2026
Merged

davide221 merged 3 commits into
Luce-Org:mainfrom
Graffioh:feat/disk-session-cache

Conversation

@Graffioh

@Graffioh Graffioh commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

What this does

A long coding-agent session can lose its RAM snapshots in three ways:

  • another session evicts them;
  • the server restarts;
  • the user switches model.

luce_server now keeps those sessions on disk, and the disk cache is on by default. A session whose RAM snapshot is gone restores its raw KV from disk and prefills only the new suffix. This works both while the server runs and after a restart.

Defaults and flags

Flag Default Meaning
--kv-cache-dir <path>|off $XDG_CACHE_HOME/lucebox/kv-cache, else ~/.cache/lucebox/kv-cache Cache directory. off (or --no-kv-cache) disables the cache.
--kv-cache-budget <MB> 131072 (128 GB) Hard cap on all snapshot files, with LRU deletion. Clamped to the free space minus a reserve (the larger of 1 GiB and 5% of the disk, at most 16 GiB).
--kv-cache-ttl <hours> 72 Deletes files unused for this long. 0 = never.
--kv-cache-shutdown-flush <s> 120 Longest time a graceful shutdown spends writing RAM snapshots. -1 = no limit.

The other disk flags are unchanged: --kv-cache-min-tokens, --kv-cache-interval, --kv-cache-cold-max and --disk-prefix-cache (default full). /props → full_cache reports disk_enabled, disk_budget_mb and disk_ttl_hours.

Dashboard: Luce-Org/luce_box#391 (branch feat/session-cache-ssd-setting) still proposes 16 GB, off by default. Its default should change to match this PR: on, 128 GB. That repo is not touched here.

When something is written: only when it leaves RAM

Nothing is written after a turn. A snapshot is written in two cases:

  • RAM eviction. A new snapshot takes the slot of an older one (its reservation's victim), the resident budget evicts several to make room for a large one (below), or the budget evicts one after a commit. This applies to the single-sequence path and to the concurrent (paged) scheduler.
  • Graceful shutdown (SIGTERM/SIGINT). Shutdown is one last eviction of everything still resident, most recently used first. It is bounded by --kv-cache-shutdown-flush. One log line gives the sessions, edit backups not written, files, MB and seconds, plus anything the limit dropped.

Caveat: a crash, kill -9 or power loss loses whatever was only in RAM. States written before that still restore.

An active session that stays resident writes nothing. That matters because a 90-120K-token Qwen3.8-27B session with q4_0 KV is 1.5-2.2 GB per snapshot. The 30 s idle write-behind of earlier revisions is gone.

What is persisted

  • Each conversation's newest snapshot: the inline snapshot of its last request, or the agent-turn state with the generated reply (tool path).
  • The shared system/tools heads.

Edit-backup rule. Since #835, a new turn keeps the previous turn's snapshot in RAM so that an edit of the last message can resume there.

  • That kept turn is not written while a deeper snapshot of its conversation is resident (PrefixCache::persist_on_evict). It only serves an edit, which the deeper file cannot, and writing it would double the bytes per conversation.
  • Once the deeper snapshot is gone, the kept turn becomes the conversation's newest snapshot and is written normally.
  • Pruned turns are never written.
  • Writing a state deletes the same session's shallower files on disk, except the head.

Compressed prompts (PFlash, FlowKV) are not raw history and never get a disk key.

How it works

Writes. Eviction hands the snapshot buffer to a writer thread without a copy, and the request continues at once.

  • The snapshot is indexed immediately and served from memory until its file exists.
  • Queued payload is bounded at 4 GiB; past that, the evicting request waits, and the wait is logged.
  • Each write goes to <file>.tmp, is fdatasynced and renamed, and the directory is synced.
  • A failed write (for example, a full disk) is logged and skipped.

Restore. With the full policy, a lookup probes three keys: the whole prompt, every chat boundary, and the deepest stored key that is a prefix of the prompt. These are index lookups; only a hit reads a file.

  • A disk state deeper than the best RAM hit wins over it. A session evicted to disk usually still matches the shared head in RAM. If the read fails, the RAM hit is used.
  • The paged scheduler reads the file on a reader thread while live streams keep decoding.
  • Each file stores the tool-call replay text (ToolMemory), and startup loads it back. Without it, a new process would render earlier tool calls differently and no deep state would match.

Safety: fail closed.

  • Format v5 ends with an XXH64 checksum of the whole file. A corrupt or partial file is a logged miss, is deleted, and the prompt is prefilled.
  • Startup deletes temporaries, files of older formats and truncated files.
  • A file restores only when its identity matches:
    • the model's path, size and mtime, plus a content fingerprint (SHA-1 of the first and last MiB);
    • the KV types and K rotation;
    • --max-ctx, the chat template and the draft model;
    • every tensor's name, type and shape. Qwen also checks each tensor against the live cache and the paged page table.
  • Each file stores the exact key token ids, and a hit compares them with the prompt.
  • One server per directory, enforced by an exclusive lock.
  • LUCE_DISK_CACHE_DIGEST=1 logs an XXH64 of every restored state, tagged ssd or ram.

A large capture evicts several older entries (own commit). Upstream skipped a capture when no single evictable entry freed enough room. A long session that arrived after many short ones (subagent threads) was then never captured, so it reached neither RAM nor disk. Now a capture that fits the budget at all plans several evictions, least recently used first: edit backups first, then leaves. The rule is the same on the single-sequence and paged paths.

  • Upstream used one victim so a capture never disturbs the committed cache before it lands. The plan keeps that: it is all or nothing and decided before anything is evicted. Its entries leave RAM (to disk) only when the capture commits, so a capture that fails or is cancelled evicts nothing.
  • The plan never takes protected pins, heads that other entries extend, the capture's own ancestors or restore source, or entries that another capture in flight writes, evicts or counts as freed.
  • A capture larger than the whole budget is still skipped (budget_skips).

Pre-existing bug fixed (qwen35). snapshot_adopt() never set the snapshot layout, so every Qwen disk restore was rejected. The caller then decoded on an empty cache, which produced garbage output. Adopted snapshots are now dense, and a failed restore fails the request.

Commits

Folded into exactly 3 commits on main (5f402c61f), including the review follow-ups. The fold preserves the verified final tree.

# Commit Change
1 5ee22b574 feat(server): persist evicted sessions to disk with a budget, TTL and…
2 5f744cd83 docs(server): document the disk session cache, its defaults and evict…
3 d6774ae46 fix(server): harden disk cache lifecycle, scope and capture budgets

Review fixes and validation (13 Oct)

Corrupt tensor metadata is rejected before GGML tensor construction: unsupported types, invalid quantized dimensions, overflowing shapes and out-of-file payloads become logged misses. The original corruption regression reproduced a floating-point exception; four malformed metadata cases now pass during live lookup and restart.

The follow-up fixes also disable a failed cache initialization so RAM serving continues without an unstarted writer queue; refresh duplicate-save TTL timestamps on disk; respect request disk scope for agent-turn saves and concurrent reads/writes; reap completed abandoned reader threads during serving; and provide an exclusive Windows directory lock and native Windows default cache directory. /props.full_cache documents the effective disk enablement, budget and TTL. The disk write-failure regression is portable to Windows and root-owned test runs.

The reported snapshot_adopt / restore_and_generate_impl failure is fixed here: adopted Qwen snapshots are dense, and restore failure returns an error before decoding from cleared KV. A CPU-backed test uses the real Qwen backend to load a .dkv, adopt it, restore exact tensor values, and reject an incompatible restore without generating tokens.

The shared-ancestor reservation concern did not reproduce: two capture orders are tested at tight budgets. Reservations retain each capture's full pending bytes, so the shared ancestor is not credited twice. No accounting workaround was added.

Validation on the final tree:

CUDA and Windows builds and R9700/Strix GPU CI passed on the preceding published head; the DGX job was cancelled before running steps. CI is rerunning for these follow-ups. This review did not run a new live GPU workload; the hardware measurements below are the earlier gates.

Numerics

SSD restore vs RAM restore of the same state: byte-identical.

  • 0 digest differences over 38 paired SSD restores in the final gate run (c4, with the multi-entry eviction fix), across the single-sequence path (including tool-call agent-turn states) and the paged path.
  • c2 and c3 (27 pairs) give the same.
  • Greedy answers are identical for every request restored at the same depth.

A RAM restore and a fresh prefill can still differ, because prefill runs over different chunk boundaries. That is a prefix-cache property, unchanged here. In the gate, every answer that differs from the reference belongs to a request restored at a different depth.

Gate results: Modal L40S

Setup:

  • Plan frozen before serving, with readings pre-registered as labels, not vetoes.
  • Qwen3.8-27B IQ4_XS, q4_0 KV, max-ctx 131072, AR decode, temperature 0.
  • Public MIT ggml corpus.
  • The cache dir was on the container's local disk (gVisor overlay).
  • All timings are L40S timings. The page-cache drop before each restart is not verified to take effect under gVisor, so the read times are container-disk times, not cold-NVMe claims.
Reading Result
R1: writes on eviction WRITES-ON-EVICTION. Single-sequence: 10 eviction writes; paged: 12; 0 write failures.
R2: same depth as RAM c3: DEPTH-DIFFERS(9), every one a state the RAM budget never captured (one-victim rule), so it was never on disk. c4, with the multi-entry eviction fix: none differ. All 9 requests (for example the 32K-token thread's 36698/39172) now restore at the reference depth.
R3: numerics BYTE-IDENTICAL. 0 digest differences (c3: 27 pairs; c4: 38).
R4: TTFT, cold prefill / RAM follow-up / SSD follow-up after graceful restart / reference RAM restore of the same follow-up 15.8K tokens: 7.61 / 0.76 / 0.98 / 0.30 s (read 628 MB in 0.21 s)
44K: 22.97 / 1.04 / 1.10 / 0.91 s (1124 MB, 0.32 s)
88K: 64.30 / 0.77 / 2.03 / 1.60 s (1895 MB, 0.61 s)
R5: disk bytes per evicted state ~18.4 KB per token, plus a fixed ~340 MB single-sequence or ~140 MB paged. 554 MB at 11.6K tokens, 833 MB at 27.5K, 1.9 GB at 88K. Each state is written once.
R6: TTFT during a write burst c2: NO-STALL. Six 40-70K prompts that each evict the previous one; disk-on/off TTFT ratio median 1.001, max 1.009.
c4 (fix): median 1.003, but one request is over the 1.15 line, so the pre-registered label is STALL-SUSPECT. That request is B3-probe at 0.99 vs 0.86 s. The 70K B3 state, skipped before the fix, is now captured: it pushes out 1.6 GB, and the next probe pushes it out again (1.6 GB written while that probe prefills).
R7: graceful restart RESTART-RESTORES. Shutdown flushes took 0.44 s for 1.5 GB and 1.60 s for 5.4 GB, well under the 120 s limit; the process stopped in 1.1-2.2 s. Restarts restored from SSD for 6/6, 3/3 and 5/6 requests in c3; in c4 for 6/6 and 6/6.

The gate found one bug. On the single-sequence path, a RAM hit on the shared 512-token head ended the restore search. A session evicted to disk was therefore never restored while the server ran: 0 SSD restores in the first run. It is fixed in this PR, with a unit test that fails without the fix. After the fix, the same group made 9 SSD restores, all byte-identical where paired.

Unit tests: test_server_unit 679 passed, 0 failed, 2 skipped on this branch (HIP build for gfx1201, GPU hidden). The suite covers:

  • TTL and the LRU budget across restarts;
  • checksum and corrupt-file rejection;
  • identity mismatch;
  • persist-on-evict and the edit-backup rule;
  • shutdown flush, then restart;
  • a deeper disk state winning over a RAM head hit;
  • agent-turn eviction writes;
  • a large capture evicting several entries, written to disk, on the single-sequence and paged paths (disk on and off); one larger than the budget skipped; protected, head, ancestor and in-use entries never taken; edit backups first and not written; a failed or cancelled capture evicting nothing.

R9700 restart check

One AMD Radeon AI PRO R9700, 12 Oct, built with the PFlash release branch merged in (release defaults on). A 45K-token
agent session (public MIT ggml tool calls and results): turn 1, graceful shutdown, restart, turn 2.

Turn 2 time to first token Restored from disk Answer
Disk cache on, after restart 2.58 s 44,721 of 45,136 tokens (1.1 GB read in 0.29 s) byte-identical to full prefill
Full prefill, cold 56.2 s none reference

Paths not covered

  • States the RAM budget cannot hold. A capture larger than the whole budget, or one whose only possible victims are protected or in use, is still skipped. It is then neither in RAM nor on disk. In c4 this happened once: the last agent-turn save of a restart arm, next to its own restore source.
  • Crash or kill -9. Loses what was only in RAM, by design. Not exercised.
  • Single-sequence --paged-attention and the concurrent qwen4exp engine. They keep their checkpoints outside the backend's snapshot slots and run without a disk cache (logged at startup).
  • Unit-tested only, not served: TTL and disk-budget LRU eviction, the free-space clamp, corruption, identity mismatch, the second-server lock, and the edit-backup exclusion (the gate workload had no edits).
  • Not exercised: R9700/HIP serving beyond the one restart check above, the Auto/Fixed disk scopes, non-Qwen backends, and compressed prompts.

PFlash integration limit. The combined #839 + #746 unit suite passes, but compressed PFlash views are not persisted to SSD. This PR excludes shortened snapshots from disk keys; it restores raw session states and ToolMemory replay metadata. Disk-aware PFlash admission and compressed-view restart persistence require a separate integration change.

🤖 Generated with Claude Code

View guided diff

… validated restores

Keep each conversation's newest snapshot and shared heads on disk when RAM
evicts them, and flush resident sessions on graceful shutdown. Enable the
cache by default with a hard LRU budget, a TTL, bounded asynchronous writes,
checksums, exact token keys and model/config identity validation.

Restore deeper disk states ahead of shallow RAM hits, load paged snapshots
without blocking decoding, and replay tool-call text after restart. Validate
Qwen snapshot layout and every disk tensor's type, alignment, dimensions and
payload bounds before adoption or allocation; corrupt files become misses.

Includes persistence, restart, eviction, corruption and identity regression
tests. Compressed prompts do not receive raw disk keys.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Graffioh
Graffioh force-pushed the feat/disk-session-cache branch from 53b8c22 to 8dcdee6 Compare October 13, 2026 08:43
@Graffioh
Graffioh marked this pull request as ready for review October 13, 2026 08:43

@davide221 davide221 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code review

Found 8 issues.

Copy to agent

Use the copy button in the top-right corner of the block, then paste into your agent.

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.

Repository: Luce-Org/lucebox
Reviewed commit: 8dcdee6461ec8fb6ea60c744d0aee54ada2d7f56
Verify the findings against the current code, run relevant checks for any fixes, and explain any findings you reject.

<file name="server/src/server/disk_prefix_cache.cpp">

<violation number="1" line="616" end_line="629">
Exclusive one-server-per-directory lock is POSIX-only; on Windows two servers share the cache and delete each other's fi

The PR's stated safety contract is "One server per directory, enforced by an exclusive lock." The lock block in `init()` is compiled only under `#if !defined(_WIN32)` and there is no `#else` fallback: on Windows, `init()` proceeds with no lock at all and no warning. The rest of the file is explicitly written to support Windows (`_fseeki64` in `seek_to`, `_commit` in `sync_file`, the `(void)dir` branch in `sync_directory`), so a Windows build is a supported configuration.

Failure sequence: start two luce_server instances on Windows with the same `--kv-cache-dir`. The second instance's `scan_all()` (lines 749-757) deletes every `*.dkv.tmp` it finds — including the temporary file the first server's writer thread is actively writing (writes go to `&lt;file&gt;.tmp` and are renamed only after `fdatasync`). The first server's rename then fails, so its evicted-session snapshot is lost (logged as a write failure). Both instances also independently run `make_room_locked` LRU eviction and in-place `last_used` rewrites over the same files, so each deletes files the other still has indexed.

Fix: add a Windows equivalent of the exclusive lock (e.g. `CreateFile` on `&lt;dir&gt;/.lock` with `dwShareMode = 0`, or `LockFileEx(..., LOCKFILE_EXCLUSIVE_LOCK | LOCKFILE_FAIL_IMMEDIATELY, ...)`), with the same fail-off-and-log behavior as the POSIX path. Fact I could not check: whether the server is officially shipped for Windows; if it is not, the Windows-specific code paths elsewhere in this file suggest it is, and the lock should match.

<suggestion>
#if !defined(_WIN32)
    const std::string lock_path = config_.cache_dir + "/.lock";
    lock_fd_ = ::open(lock_path.c_str(), O_RDWR | O_CREAT | O_CLOEXEC, 0644);
    if (lock_fd_ &lt; 0 || ::flock(lock_fd_, LOCK_EX | LOCK_NB) != 0) {
        std::fprintf(stderr,
                     "[disk-cache] disabled: %s is in use by another server "
                     "(pass --kv-cache-dir &lt;other dir&gt; to give this one its own)\n",
                     config_.cache_dir.c_str());
        if (lock_fd_ &gt;= 0) ::close(lock_fd_);
        lock_fd_ = -1;
        config_.cache_dir.clear();
        return true;
    }
#else
    // Windows: exclusive hold on .lock via a share-mode-0 handle.
    const std::string lock_path = config_.cache_dir + "/.lock";
    lock_handle_ = CreateFileA(lock_path.c_str(), GENERIC_READ | GENERIC_WRITE,
                               0, nullptr, OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr);
    if (lock_handle_ == INVALID_HANDLE_VALUE) {
        std::fprintf(stderr,
                     "[disk-cache] disabled: %s is in use by another server "
                     "(pass --kv-cache-dir &lt;other dir&gt; to give this one its own)\n",
                     config_.cache_dir.c_str());
        config_.cache_dir.clear();
        return true;
    }
#endif
</suggestion>
</violation>

<violation number="2" line="1153" end_line="1158">
Duplicate-save touch updates last_used only in memory, so TTL/LRU after a restart ignore recent use

`save_async()`'s contract (like `load()`'s) is that using an entry refreshes its `last_used`, and the header comment at the `load()` touch site states the file field is rewritten in place precisely so that "LRU order and the TTL survive a restart." When a snapshot is saved but its key is already on disk (line 1154 finds the entry), the code updates `entries_[existing].last_used` in memory only and never rewrites `DISK_CACHE_LAST_USED_OFFSET` in the file. The same pattern exists in `save()` (lines 1213-1217). Concretely: a shared system/tools head or a session snapshot that is written once and then repeatedly re-saved (every later eviction of the same prefix hits the duplicate branch — including the graceful-shutdown flush, which also goes through `save_async`) keeps its original on-disk `last_used` forever. After a restart, `scan_all()` restores that stale timestamp, and `expire_locked()` at startup (line 633) or `make_room_locked()` LRU eviction will delete the file as "unused for ttl_seconds" (default 72 h) even though it was actively used right up to the shutdown. Fix: mirror the `load()` touch path and persist the new `last_used` to the file when the entry is not pending.

<suggestion>
    const int existing = find_entry(hash);
    if (existing &gt;= 0) {
        DiskEntry &amp; e = entries_[(size_t)existing];
        e.last_used = now();
        if (!e.pending) {
            // Persist like load() does, so LRU order and the TTL survive a restart.
            if (FILE * f = std::fopen(e.path.c_str(), "r+b")) {
                if (std::fseek(f, DISK_CACHE_LAST_USED_OFFSET, SEEK_SET) == 0) {
                    write_u64(f, e.last_used);
                }
                std::fclose(f);
            }
        }
        return true;
    }
</suggestion>
</violation>

</file>

<file name="server/src/server/prefix_cache.cpp">

<violation number="1" line="991" end_line="993">
Reclaimed (superseded) bytes are double-counted by concurrent reservations

The new `credited_slots` mechanism exists so that "two captures never count the same bytes": slots an in-flight reservation already counts as freed are pushed into `credited_slots` (here and at lines 998-999) and `plan_several` skips them via `slot_credited()` (line 937-938). But the `reclaimed` computation above (lines 830-837), which credits this reservation with the bytes of the entries it supersedes, only skips `slot_reserved(...)` — it never checks `slot_credited(...)`. Sequence that breaks the invariant: reservation A on one conversation thread computes `reclaimed` containing ancestor entry X and pushes X's slot into its `credited_slots`; reservation B (a sibling/deeper prompt sharing ancestor X, two concurrent requests) then computes its own `reclaimed` including X again, because X is not `slot_reserved`. Both reservations are admitted on the assumption that X's bytes are freed twice. The budget check `fits()`/`room()` therefore admits captures whose real post-commit resident bytes exceed `max_resident_bytes_`; the overshoot is only corrected later by `enforce_resident_budget()` evicting entries that were never planned (possibly including a restore source or the shared head that the plan was careful to keep). Fix: in the `reclaimed` loop, also skip entries whose slot is `slot_credited(entries_[i].slot)` (and conversely skip already-reclaimed slots), so a superseded ancestor is credited to exactly one in-flight reservation.

<suggestion>
        for (size_t i = 0; i &lt; reclaimed.size(); ++i) {
            // Skip slots another in-flight reservation already credits: the
            // reclaimed loop above only filters slot_reserved, so without
            // this two concurrent captures can count the same ancestor's
            // bytes as freed.
            if (reclaimed[i] &amp;&amp; !slot_credited(entries_[i].slot))
                credited_slots.push_back(entries_[i].slot);
        }
</suggestion>
</violation>

</file>

<file name="server/src/server/http_server.cpp">

<violation number="1" line="4668" end_line="4670">
Agent-turn snapshots get a disk key even when the request's disk scope is not full

`save_generated_turn()` unconditionally records `slot_tokens_[slot]` and calls `note_session_slot(slot)` for the saved generated turn. `slot_tokens_` is the disk key: `persist_evicted_slot()` (line 4359) writes any slot that has a key to disk on RAM eviction or shutdown, and its own comment says a missing key means "a compressed view, a non-`full` disk scope". The inline-snapshot path in `finalize_generation_cache()` (lines 4293-4299) gates exactly this keying on `!prepared.compressed &amp;&amp; cache.disk_policy.mode == DiskPrefixCacheMode::Full`, but the agent-turn path here (and the identical one in `remember_agent_turn()`, lines 4816-4818, which also has `cache` in scope via the caller) skips that check. A client that sends a request with disk-cache scope `off`/`auto`/`fixed` (parsed into `req.disk_cache_policy`) while the server-level disk cache is enabled will still have its generated agent turn written to the on-disk KV cache when the snapshot is evicted or the server shuts down — persisting a conversation the request explicitly scoped out of the disk cache. Gate the `slot_tokens_` assignment and `note_session_slot()` on `cache.disk_policy.mode == DiskPrefixCacheMode::Full` (and `!disk_cache_.disabled()`) the same way `finalize_generation_cache()` does.

<suggestion>
    if (cache.disk_policy.mode == DiskPrefixCacheMode::Full) {
        slot_tokens_[slot] = std::vector&lt;int32_t&gt;(
            canonical.begin(), canonical.begin() + saved_pos);
        note_session_slot(slot);
    }
</suggestion>
</violation>

</file>

<file name="server/test/unit/test_server_unit.cpp">

<violation number="1" line="10106" end_line="10106">
Unguarded ::geteuid() breaks the Windows build of the test file

The new `test_disk_cache_write_failure_is_not_fatal` test is compiled in every configuration (it sits in the unguarded region lines 9757-10696), yet line 10106 calls `::geteuid()`, a POSIX-only function declared in `&lt;unistd.h&gt;`. This same diff shows the project treats Windows as a supported build for this test file: the paged-scheduler tests directly below (lines 10698-10982) are wrapped in `#if !defined(_WIN32)` precisely because they use `socketpair`/`AF_UNIX`/`close`. On an MSVC build `::geteuid` is not declared, so the unit-test target fails to compile. Even where it compiles, the root check is meant to skip the permission-based assertions when the write can't fail; the surrounding `fs::permissions(owner_read|owner_exec)` trick also has different semantics on Windows (it maps to the read-only attribute), so the whole block should be platform-guarded. What I could not check: whether the test file (or a test support header) defines a `geteuid` compatibility shim for `_WIN32`; if it does, the compile concern is void but the permission-based assertions would still silently not exercise the failure path on Windows.

<suggestion>
#if !defined(_WIN32)
    if (::geteuid() != 0) {
        TEST_ASSERT(cache.stats().write_failures == 1);
        TEST_ASSERT(cache.entry_count() == 1);
        TEST_ASSERT(dkv_files(dir).size() == 1);
        TEST_ASSERT(!cache.lookup(disk_test_key(40, 800), 1));
    }
#endif
</suggestion>
</violation>

</file>

<file name="docs/specs/props-endpoint.md">

<violation number="1" line="390" end_line="391">
New full_cache disk fields (disk_enabled, disk_budget_mb, disk_ttl_hours) are not documented in the /props spec

This PR adds `disk_enabled`, `disk_budget_mb` and `disk_ttl_hours` to the `full_cache` object reported by `/props` (per the PR description and the http_server.cpp wiring reviewed in part 4), and §4.7 of this spec is the normative description of `full_cache` semantics — it already documents `capacity`, `max_resident_bytes`, `resident_bytes` and `budget_skips` in the paragraph this diff edits. The diff only updates the `budget_skips` wording and never adds the three new fields, so the spec now under-documents the endpoint: a client written against docs/specs/props-endpoint.md has no way to know these keys exist, what units they use (MB vs bytes — note `disk_budget_mb` vs `max_resident_bytes`), or what values mean the disk cache is disabled (`disk_enabled` false vs a `0` budget, which this same paragraph defines as 'disabled'/`unlimited' for other fields). Add a sentence to §4.7 documenting the three fields, their units and their disabled-state values. I could not verify the exact wire names/units emitted by http_server.cpp beyond the PR description, so the fix should be checked against that code.
</violation>

</file>

<file name="server/src/server/scheduler.cpp">

<violation number="1" line="139" end_line="145">
Dropped disk loads leak unjoined reader threads until server shutdown

`drop_disk_load()` moves a still-joinable reader `std::thread` into `finished_readers`, and that vector is only joined in the scheduler teardown path (lines 1334-1336). A joinable thread that has already exited keeps its stack and thread-control resources allocated until it is joined or detached (pthreads semantics), so every dropped load leaks on the order of a full thread stack (typically 8 MB) for the remaining process lifetime. Drops happen in normal use: line 516 drops a pending load whenever a different job is admitted, and a retry whose probe no longer qualifies (a deeper RAM entry was committed meanwhile, or the disk entry expired/was evicted) drops it too. On a long-running server with the disk cache on by default, repeated drops accumulate leaked stacks unboundedly. Fix: reap finished readers periodically in the scheduler loop (join and erase any whose `result-&gt;done` is set), rather than only at shutdown — joining inside `drop_disk_load()` itself is not an option because the reader may still be blocked in `disk_cache_.load()`, and detaching would let a reader outlive `disk_cache_` at shutdown.
</violation>

</file>

<file name="server/src/server/server_main.cpp">

<violation number="1" line="429" end_line="435">
Default disk-cache directory is empty on Windows, silently disabling the on-by-default cache

The PR makes the disk session cache on by default via `default_kv_cache_dir()`, which only consults `XDG_CACHE_HOME` and `HOME` and returns an empty string otherwise, and `parse_model_options()` assigns that empty result to `sconfig.disk_cache_dir` (line 459), which is the disabled sentinel. On Windows — a build configuration this repository supports (`#if defined(_WIN32)` paths throughout the server) — `XDG_CACHE_HOME` is never set and `HOME` is normally unset, so every Windows launch gets the cache silently disabled, contradicting the documented default (on, 128 GB) with no log line telling the user it is off. Use a Windows fallback (e.g. `%LOCALAPPDATA%` / `%USERPROFILE%` under `lucebox\kv-cache`) so the default is actually on there, or at least log that the cache defaulted to off.
</violation>

</file>
Issue overview
  1. Exclusive one-server-per-directory lock is POSIX-only; on Windows two servers share the cache and delete each other's fi · likely (server/src/server/disk_prefix_cache.cpp:616-629)

  2. Duplicate-save touch updates last_used only in memory, so TTL/LRU after a restart ignore recent use · likely (server/src/server/disk_prefix_cache.cpp:1153-1158)

  3. Reclaimed (superseded) bytes are double-counted by concurrent reservations · likely (server/src/server/prefix_cache.cpp:991-993)

  4. Agent-turn snapshots get a disk key even when the request's disk scope is not full · likely (server/src/server/http_server.cpp:4668-4670)

  5. Unguarded ::geteuid() breaks the Windows build of the test file · confirmed (server/test/unit/test_server_unit.cpp:10106)

  6. New full_cache disk fields (disk_enabled, disk_budget_mb, disk_ttl_hours) are not documented in the /props spec · likely (docs/specs/props-endpoint.md:390-391)

  7. Dropped disk loads leak unjoined reader threads until server shutdown · likely (server/src/server/scheduler.cpp:139-145)

  8. Default disk-cache directory is empty on Windows, silently disabling the on-by-default cache · likely (server/src/server/server_main.cpp:429-435)

Reviewed 8dcdee64 · 11 candidate issues checked, kept at ≥70% confidence · add the code-review label to review again

Comment thread server/src/server/disk_prefix_cache.cpp Outdated
Comment thread server/src/server/disk_prefix_cache.cpp
Comment thread server/src/server/prefix_cache.cpp
Comment thread server/src/server/http_server.cpp Outdated
Comment thread server/test/unit/test_server_unit.cpp Outdated
Comment thread docs/specs/props-endpoint.md
Comment thread server/src/server/scheduler.cpp
Comment thread server/src/server/server_main.cpp
Graffioh and others added 2 commits October 13, 2026 09:36
…ion-only writes

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Allow large captures to evict several eligible entries transactionally.
Respect request scopes, refresh duplicate-save TTL metadata, disable failed
cache initialization, and reap asynchronous readers during serving.
Use an exclusive Windows directory lock and native cache defaults.
Cover dense Qwen disk adoption, failed restores, shared capture reservations,
and compressed-state exclusion with model-free regressions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Graffioh
Graffioh force-pushed the feat/disk-session-cache branch from 8dcdee6 to d6774ae Compare October 13, 2026 09:39
@davide221
davide221 merged commit fbcefbc into Luce-Org:main Oct 13, 2026
8 of 9 checks passed
dusterbloom added a commit to dusterbloom/lucebox-hub that referenced this pull request Oct 14, 2026
Prerequisite PR stack from the branch, F milestones M1-M9 with tests and 3090 gates against the Modal R1b F rows, multi-round compaction answer, risks (Luce-Org#839 disk, kernel bias on CUDA/HIP, ragged pool vs Luce-Org#652 verify views), and the origin/main merge strategy.
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.

2 participants