diff --git a/devlog/_plan/260831_prio70_train_round2/020_wp2_spill_disk_budget.md b/devlog/_plan/260831_prio70_train_round2/020_wp2_spill_disk_budget.md index 98ffee3bfe..2e0f7a6a22 100644 --- a/devlog/_plan/260831_prio70_train_round2/020_wp2_spill_disk_budget.md +++ b/devlog/_plan/260831_prio70_train_round2/020_wp2_spill_disk_budget.md @@ -128,6 +128,32 @@ Suite, typecheck and privacy scan on `ssh lidge`. Residual risk, same as round 1's wp3: NTFS unlink semantics and `icacls` timeout behaviour while a path is held still want a real Windows host. + +## What implementation added beyond this plan + +Four adversarial review rounds against the built branch (findings 4, 3, 1, 0). Three of +their findings changed the design rather than the code, so they belong here: + +- **The footprint is measured, not estimated.** The plan said "exact prospective + measurement" and the first implementation used `candidate.sizeBytes`, which omits the + `version` field the published envelope carries. `prospectiveResponseSpillBytes` now + shares `serializedSpill` itself, so the two cannot drift. +- **Superseded generations are priced in two places, not one.** A same-id replacement + takes the old spill off `states` and hands it to the pending job. It is invisible to + the accounting walk at admission (the job does not exist yet) and again in the shutdown + fallback (supersession has already released the job). Both checks add it explicitly. +- **Cleanup debt is per path and repayable.** The plan did not anticipate a failed + unlink. A flat charge that never decrements is phantom debt: a Windows lock that clears, + or the async writer's own retry, removes the file while the charge stays, and two + conservative 256 MiB charges consume the whole default cap for the life of the process. + The debt is keyed by path and settled when the path is gone. +- **The shutdown fallback fails closed.** If the footprint still does not fit after + reclaim, it terminalizes with ENOSPC rather than publishing onto an over-budget volume. + +And one about verification: the first regression stayed green with the admission check +deleted, because `pruneResponses` reclaimed on another path. It proved the counter, not +the cap. There are now two tests — one for accounting during a forced copy fallback, one +for enforcement — and the enforcement test is red when admission is removed. ### Amendment after audit round 1 (`002`, blocker 3): the peak is two envelopes The first draft reserved one serialized envelope. Windows publication can fall back diff --git a/src/responses/spill-store.ts b/src/responses/spill-store.ts index f61d475c5a..ce063dfd8b 100644 --- a/src/responses/spill-store.ts +++ b/src/responses/spill-store.ts @@ -470,6 +470,26 @@ function serializedSpill( }; } +/** + * Exact on-disk payload size this spill WOULD occupy, measured before publication. + * + * Callers that reserve disk against a cap need the real envelope, not the resident + * measurement: the resident figure omits the `version` field the published payload + * carries, so pricing an admission by it undercounts and lets a request that sits exactly + * at the cap still exceed it. Shares `serializedSpill` rather than describing it, so the + * two cannot drift. + */ +export function prospectiveResponseSpillBytes( + responseId: string, + state: Omit, +): number | null { + try { + return serializedSpill(responseId, state).bytes.byteLength; + } catch { + return null; + } +} + function responseSpillWriteError(cause: unknown): NodeJS.ErrnoException { const error = new Error("Response spill write failed", { cause }) as NodeJS.ErrnoException; if (cause && typeof cause === "object" && "code" in cause) { diff --git a/src/responses/state.ts b/src/responses/state.ts index 35540a0ee7..b95a1fa2c6 100644 --- a/src/responses/state.ts +++ b/src/responses/state.ts @@ -16,6 +16,7 @@ import { responseSpillDirectory, responseSpillPayloadCap, markResponseSpillPublicationSuperseded, + prospectiveResponseSpillBytes, type ResponseSpillPublicationControl, type ResponseSpillRef, writeResponseSpillDurably, @@ -35,6 +36,30 @@ const SNAPSHOT_DEBOUNCE_MAX_MS = 30_000; * continuation chains) stores the full expanded input each turn — ~quadratic bytes per chain — * so a count cap alone cannot bound memory. Oldest-first eviction applies past this mark. */ export const MAX_STORED_RESPONSE_BYTES = 64 * 1024 * 1024; +/** + * Aggregate ceiling for the durable spill directory: the disk-side counterpart to + * the RAM ceiling above. Without it the spilled set is bounded only per-file + * (MAX_RESPONSE_SPILL_PAYLOAD_BYTES, 256 MiB) and per-entry (MAX_STORED_RESPONSES, + * 1000), whose product is 250 GiB — larger than the disk of any host this runs on. + * The only effective bound was therefore RESPONSE_TTL_MS, which makes disk use a + * function of client request rate rather than of anything this process controls. + * + * Measured on one macOS host, 2026-08-30: a client spilling ~150 MB payloads at + * ~1.4/min held 6.8 GB after 44 minutes, still climbing toward the ~12 GB an + * hour-long window implies, and filled the volume. Retention itself was correct + * throughout — the TTL evicted that whole cohort an hour later — so what was + * missing is a budget, not a sweep. + * + * 1 GiB comes from the same sample (n=31), whose spilled sizes are strongly + * bimodal: median 1.1 MiB against a p90 of 198.7 MiB, near the per-file ceiling. + * At that median the count cap and this ceiling bind within 8% of each other + * (1000 x 1.1 MiB = 1.07 GiB), so ordinary traffic sees no eviction it would not + * already have seen and only the large tail is cut. Erring small is the safe + * direction: too low costs a replay miss, an already-handled path surfaced as + * previous_response_not_found, while too high costs the host's disk and every + * unrelated process on it. + */ +export const MAX_SPILLED_RESPONSE_BYTES = 1024 * 1024 * 1024; /** Legacy snapshot selection only. Spill demotion is governed solely by the RAM cap above. */ const SNAPSHOT_ENTRY_MAX_BYTES = 2 * 1024 * 1024; const SNAPSHOT_TOTAL_MAX_BYTES = 24 * 1024 * 1024; @@ -187,12 +212,79 @@ interface PendingResponseSpill { cancelled: boolean; released: boolean; sizeBytes: number; + /** Peak on-disk bytes reserved for this publication; released exactly once on settle. */ + reservedBytes: number; publicationControl: ResponseSpillPublicationControl; } const pendingResponseSpills = new Set(); const pendingResponseSpillById = new Map(); let pendingResponseSpillBytes = 0; +/** + * On-disk bytes a queued publication is about to occupy but has not yet installed into + * `states`. + * + * `spilledResponseBytes()` walks installed spills and deferred unlinks — files that + * already exist. It cannot see one that `writeResponseSpillDurablyAsync` is in the + * middle of creating, and on Windows that middle can last as long as `icacls` takes. + * Without a reservation the cap holds only when writes are fast, which is not a cap. + * + * The reserved figure is the PEAK footprint, not the payload: publication can fall back + * from hard-linking to an exclusive copy, and during that fallback the destination copy + * and the temp file exist simultaneously. Reserving one envelope would leave the overshoot + * intact at half its magnitude. + * + * Ownership is single: a job holds its reservation from queue until + * `releasePendingResponseSpill`, which every exit from the publication path reaches + * through the `finally` in `runPendingResponseSpill` and through cancellation of a + * not-yet-running job. A leaked reservation is monotonic — it would ratchet the usable + * cap toward zero — so the release must stay on the settlement path rather than in a + * parallel bookkeeping pass. + */ +let reservedResponseSpillBytes = 0; +/** + * Paths a failed cleanup left on the volume, with the bytes each one occupies. + * + * A failed unlink leaves a real file behind, so the cap has to keep seeing it. But a + * never-decremented total would be phantom debt: a Windows lock that clears a moment + * later, or the async writer's own retry, can remove the file while the charge stays + * forever — and with 256 MiB payloads two conservative charges consume the whole default + * cap, after which nothing can spill for the life of the process. + * + * So the debt is per PATH, priced at what that path actually holds, and settled the + * moment the path is gone. `reconcileUnreclaimableSpillPaths` re-checks on every read of + * the accounted total, which is the same tick that would otherwise refuse an admission. + */ +const unreclaimableSpillPaths = new Map(); + +function chargeUnreclaimableSpillPath(path: string | null | undefined, bytes: number): void { + if (!path || bytes <= 0) return; + unreclaimableSpillPaths.set(path, bytes); +} + +/** Drop charges for paths that have since disappeared; returns the surviving total. */ +function reconcileUnreclaimableSpillPaths(): number { + let total = 0; + for (const [path, bytes] of [...unreclaimableSpillPaths]) { + if (existsSync(path)) total += bytes; + else unreclaimableSpillPaths.delete(path); + } + return total; +} + +/** + * Peak on-disk footprint of publishing this candidate: temp plus destination copy. + * + * Measured from the production serializer rather than from `candidate.sizeBytes`. The + * resident measurement omits the `version` field the published envelope carries, so + * pricing an admission by it undercounts and lets a request sitting exactly at the cap + * still exceed it. Falls back to the resident figure only when serialization fails, which + * is the same condition that will fail the publication itself. + */ +function publicationFootprintBytes(id: string, candidate: ResidentResponseState): number { + const exact = prospectiveResponseSpillBytes(id, spillPayloadForResident(candidate)); + return (exact ?? candidate.sizeBytes) * 2; +} let responseSpillPublicationTail: Promise = Promise.resolve(); let responseSpillShutdownBudgetOverride: { totalMs: number; fallbackReserveMs: number } | null = null; let responseSpillShutdownTerminalizationPassLimitOverride: number | null = null; @@ -210,6 +302,7 @@ function releasePendingResponseSpill(job: PendingResponseSpill): void { if (job.released) return; job.released = true; pendingResponseSpillBytes = Math.max(0, pendingResponseSpillBytes - job.sizeBytes); + reservedResponseSpillBytes = Math.max(0, reservedResponseSpillBytes - job.reservedBytes); pendingResponseSpills.delete(job); if (pendingResponseSpillById.get(job.id) === job) pendingResponseSpillById.delete(job.id); job.candidate = null; @@ -222,6 +315,11 @@ function cancelPendingResponseSpill(id: string): ResponseSpillRef | undefined { job.cancelled = true; markResponseSpillPublicationSuperseded(job.publicationControl); const superseded = job.supersededSpill; + // Ownership TRANSFERS to the caller. Leaving the ref on the cancelled job would let the + // accounting walk count the same physical file twice — once here and once on the + // replacement — and an overcount evicts live continuations to make room for bytes that + // are not there. + delete job.supersededSpill; // A queued job has not captured the candidate in an async frame yet, so release it now. // A running job retains its accounting until settlement and will discard its stale file. if (!job.running) releasePendingResponseSpill(job); @@ -313,6 +411,25 @@ function queuePendingResponseSpill( deferSupersededSpill(inheritedSpill); return; } + // Enforce the disk cap BEFORE the temp or destination file is created. Deleting the + // overflow afterwards is not equivalent: on Windows the file can outlive the decision + // by as long as ACL hardening takes, which is the window the measured 6.8 GiB + // accumulated in. Reclaim first, and only refuse if the peak footprint still does not + // fit — an eviction pass can free a live continuation's worth of room. + const footprint = publicationFootprintBytes(id, candidate); + // The superseded generation this job is about to own is already off `states` and not + // yet on the job, so it is invisible to the walk. Price it here or admission decides + // against a total that is short by a whole envelope. + const inheritedBytes = inheritedSpill?.payloadBytes ?? 0; + if (accountedResponseSpillBytes() + footprint + inheritedBytes > spillByteCap()) { + enforceSpilledResponseBudget(); + if (accountedResponseSpillBytes() + footprint + inheritedBytes > spillByteCap()) { + spillCounters.writeFailures += 1; + replaceWithSpillFailure(id, candidate); + deferSupersededSpill(inheritedSpill); + return; + } + } const job: PendingResponseSpill = { id, candidate, @@ -322,11 +439,13 @@ function queuePendingResponseSpill( cancelled: false, released: false, sizeBytes: candidate.sizeBytes, + reservedBytes: footprint, publicationControl: createResponseSpillPublicationControl(), }; pendingResponseSpills.add(job); pendingResponseSpillById.set(id, job); pendingResponseSpillBytes += job.sizeBytes; + reservedResponseSpillBytes += job.reservedBytes; recomputeOldestResident(); responseSpillPublicationTail = responseSpillPublicationTail .then(() => runPendingResponseSpill(job), () => runPendingResponseSpill(job)); @@ -418,7 +537,36 @@ function installShutdownFallbackSpill( aclBudgetMs: number, ): void { let ref: ResponseSpillRef | null = null; + // Supersession released this job's reservation, but the synchronous write below is the + // largest publication of the shutdown path and has its own link-then-copy fallback + // holding a temp and a destination at once. Re-reserve for its duration so the cap is + // not blind exactly where the drain does its heaviest work, and settle in `finally` so + // every return, throw and mismatch releases it. + const footprint = publicationFootprintBytes(job.id, candidate); + reservedResponseSpillBytes += footprint; try { + // Supersession released this job, so its superseded generation is no longer visible + // to the accounting walk — but the file is still on the volume until + // `deferSupersededSpill` or a delete takes it. Price it here or the fallback decides + // against a total short by that whole envelope, which is exactly the gap that lets + // `debt + footprint <= cap < old + debt + footprint` publish over budget. + const supersededBytes = job.supersededSpill?.payloadBytes ?? 0; + // The drain must not publish over the cap either. Reclaim first; if the footprint + // still does not fit — which is what unreclaimable cleanup debt looks like — the + // honest close-out is a tombstone, not another file on a volume that is already + // over budget. `replaceWithSpillFailure` is the same fail-closed ending the budget + // exhaustion path uses, so replay reports `spill_failed` and the client resends. + if (accountedResponseSpillBytes() + supersededBytes > spillByteCap()) { + enforceSpilledResponseBudget(); + if (accountedResponseSpillBytes() + supersededBytes > spillByteCap()) { + if (states.get(job.id) === candidate) { + spillCounters.writeFailures += 1; + replaceWithSpillFailure(job.id, candidate); + deferSupersededSpill(job.supersededSpill); + } + throw Object.assign(new Error("Response spill shutdown fallback exceeds the durable disk cap"), { code: "ENOSPC" }); + } + } ref = writeResponseSpillDurably(job.id, spillPayloadForResident(candidate), { aclBudgetMs }); if (ref.payloadBytes > responseSpillPayloadCap()) { deleteResponseSpill(ref); @@ -445,6 +593,8 @@ function installShutdownFallbackSpill( deferSupersededSpill(job.supersededSpill); } throw error; + } finally { + reservedResponseSpillBytes = Math.max(0, reservedResponseSpillBytes - footprint); } } @@ -477,7 +627,20 @@ function supersedeShutdownFallbackBatch( } for (const { job } of pending) { const cleanupFailure = cleanupSupersededResponseSpillPublication(job.publicationControl); - if (cleanupFailure) failures.push(cleanupFailure); + if (cleanupFailure) { + failures.push(cleanupFailure); + // Cleanup failed, so an async temp or destination is STILL on the volume. Releasing + // the reservation would un-account a file that exists, and the fallback write that + // follows reserves only its own footprint — three envelopes on disk priced as two. + // + // Charge the surviving PATHS rather than a flat two envelopes: `clearOwnedPath` + // nulls whichever it managed to remove, so one failure is one file, not two. The + // charge is settled automatically once the path disappears, which a retried unlink + // or a released Windows lock can still do. + const perPath = Math.max(1, Math.floor(job.reservedBytes / 2)); + chargeUnreclaimableSpillPath(job.publicationControl.tempPath, perPath); + chargeUnreclaimableSpillPath(job.publicationControl.destinationPath, perPath); + } releasePendingResponseSpill(job); } } @@ -603,6 +766,69 @@ export function getStoredResponseBytesForTests(): number { return storedResponseBytes; } +let spillByteCapOverride: number | null = null; + +function spillByteCap(): number { + return spillByteCapOverride ?? MAX_SPILLED_RESPONSE_BYTES; +} + +/** + * Live total of durable spill payloads. Recomputed per call rather than carried as + * a running counter: spilled entries reach `states` through several insertion paths + * (demotion swap, direct oversized admission, snapshot reload), and one missed + * increment there would silently disable the cap, where an O(MAX_STORED_RESPONSES) + * walk cannot drift. + */ +function spilledResponseBytes(): number { + let total = 0; + for (const entry of states.values()) { + if (entry.kind === "spill") total += entry.spill.payloadBytes; + } + // Superseded generations awaiting a durable snapshot are still files on disk. + // Counting only `states` would let PENDING_SPILL_UNLINKS_MAX of them sit outside + // the budget while it reports itself satisfied. + for (const ref of pendingSpillUnlinks) total += ref.payloadBytes; + return total; +} + +/** + * Accounted on-disk bytes: files that exist, plus the peak footprint of publications + * already in flight. + * + * The cap is enforced against this rather than against `spilledResponseBytes()` alone, + * because a publication that has not finished is still consuming the volume. On Windows + * the gap between "queued" and "installed" is however long `icacls` takes, and the + * measured incident this cap answers accumulated 6.8 GiB in 44 minutes. + */ +function accountedResponseSpillBytes(): number { + // Superseded generations a pending job still owns are files on disk too. A same-id + // replacement removes the old spill from `states` and hands its ref to the job, so + // counting only `states` plus `pendingSpillUnlinks` loses it for the whole publication + // — during a copy fallback that is old generation + new temp + new destination, three + // envelopes priced as two. + let ownedBySpillJobs = 0; + for (const job of pendingResponseSpills) { + if (job.supersededSpill) ownedBySpillJobs += job.supersededSpill.payloadBytes; + } + return spilledResponseBytes() + reservedResponseSpillBytes + ownedBySpillJobs + + reconcileUnreclaimableSpillPaths(); +} + +/** Test-only: lower/restore the durable spill cap (null restores the default). */ +export function setSpilledResponseByteCapForTests(bytes: number | null): void { + spillByteCapOverride = bytes; +} + +/** Test-only: current durable spill accounting (proves evictions unlink their files). */ +export function getSpilledResponseBytesForTests(): number { + return spilledResponseBytes(); +} + +/** Test-only: on-disk bytes plus in-flight publication reservations. */ +export function getAccountedResponseSpillBytesForTests(): number { + return accountedResponseSpillBytes(); +} + function serializedBytes(value: unknown): number | null { try { const serialized = JSON.stringify(value); @@ -1495,6 +1721,60 @@ export function replayOverlapSkipsForTests(): number { return replayOverlapSkips; } +/** + * Bring the durable spill set inside MAX_SPILLED_RESPONSE_BYTES, and report the + * bytes released. + * + * One owner, three callers: mutation pruning, the lazy load that follows a + * restart, and the periodic sweep. The periodic caller is not redundant — the + * mutation path only runs when traffic arrives, and a process can come up over + * budget from a snapshot written under a larger ceiling and then sit idle. That + * was observed in production at 1.8 GiB against a 1 GiB cap, held until the first + * request. + * + * NOT covered here: spill files orphaned by a crash. They are absent from + * `states`, so this function can neither see nor price them, and they stay with + * recoverOrphanedResponseSpills and its RESPONSE_SPILL_ORPHAN_GRACE_MS window. + * This ceiling therefore bounds what the store owns, which is every file it can + * account for, and not the directory as a whole. + */ +function enforceSpilledResponseBudget(): number { + // Price in-flight publications too: a file being created by + // `writeResponseSpillDurablyAsync` occupies the volume before it reaches `states`. + let spilledBytes = accountedResponseSpillBytes(); + if (spilledBytes <= spillByteCap()) return 0; + const before = spilledBytes; + // Deferred generations go first. They are already superseded, so releasing one + // costs only the crash window the queue exists to cover — the same trade + // PENDING_SPILL_UNLINKS_MAX already makes against unbounded disk. Evicting a + // live continuation to make room for a dead file would be the wrong order. + while (spilledBytes > spillByteCap() && pendingSpillUnlinks.length > 0) { + const ref = pendingSpillUnlinks.shift()!; + spilledBytes -= ref.payloadBytes; + deleteResponseSpill(ref); + } + // Ordered by createdAt, not by map order. `states` is not an age index: + // demotion and spill replacement delete and reinsert entries, and + // writeBoundedSnapshot serializes the map reversed, so map order can put a + // newer continuation first — and evicting that one spends a resume the older + // entry would not have cost. Sorting is O(k log k) over the spilled subset and + // runs only on a tick already over budget. + const spilled = [...states] + .filter((pair): pair is [string, SpilledResponseState] => pair[1].kind === "spill") + // createdAt is millisecond-resolution, so ties are ordinary under load. A + // stable sort would then fall back to insertion order — the very order this + // is avoiding — so break ties on the response id. Not localeCompare: the + // order must not depend on the host locale. + .sort((a, b) => a[1].createdAt - b[1].createdAt + || (a[0] < b[0] ? -1 : a[0] > b[0] ? 1 : 0)); + for (const [id, entry] of spilled) { + if (spilledBytes <= spillByteCap()) break; + spilledBytes -= entry.spill.payloadBytes; + deleteEntry(id); + } + return before - spilledBytes; +} + function pruneResponses(at = now()): void { for (const [id, state] of states) { if (at - state.createdAt > RESPONSE_TTL_MS) deleteEntry(id); @@ -1537,6 +1817,7 @@ function pruneResponses(at = now()): void { replaceWithSpillFailure(oldestId, entry); } } + enforceSpilledResponseBudget(); } /** Periodic TTL-only sweep; count/byte eviction remains owned by mutation paths. */ @@ -1547,7 +1828,10 @@ export function sweepExpiredResponseStates(at = now()): number { deleteEntry(id); removed += 1; } - if (removed > 0) schedulePersist(); + // The disk ceiling needs a caller that does not depend on traffic. The return + // value stays the TTL count so this function's existing contract is unchanged. + const reclaimed = enforceSpilledResponseBudget(); + if (removed > 0 || reclaimed > 0) schedulePersist(); return removed; } @@ -1995,6 +2279,8 @@ export function clearResponseStateMemoryForTests(): void { export function clearResponseStateForTests(): void { for (const entry of states.values()) deleteOwnedSpills(entry); clearResponseStateMemoryForTests(); + reservedResponseSpillBytes = 0; + unreclaimableSpillPaths.clear(); try { unlinkSync(snapshotPath()); } catch { diff --git a/structure/00_overview.md b/structure/00_overview.md index d489fb293b..d1f864c4c6 100644 --- a/structure/00_overview.md +++ b/structure/00_overview.md @@ -84,7 +84,7 @@ opencodex state root does not undo those writes. Putting native Codex back is th | `~/.opencodex/ocx.pid`, `runtime-port.json`, `system-env-port` | opencodex runtime | Live process identity and the port a client should reach; rewritten on start. `runtime-port.json` also carries the protected per-process listener-attestation key used before CLI diagnostics attach a management bearer. | | `~/.opencodex/codex-runtime.json`, `codex-runtime-clamp.json` | opencodex Codex runtime | Selected Codex executable/version state and effort-clamp diagnostics. Not process identity: these persist a resolved choice and a diagnostic, so losing them changes behavior until re-resolved. | | `~/.opencodex/service-state.json`, `service.log`, `service-api-token`, `opencodex-service-launcher.vbs`, `opencodex-service-task.xml`, `opencodex-service.cmd`, `winsw`, `tray-state.json`, `tray-heartbeat.json`, `opencodex-tray.ps1`, `opencodex-tray-*.ico`, `update-job.json` | opencodex operators | Installed-service, Windows tray, and self-update artifacts and bookkeeping. The update record carries its worker PID so a dead worker recovers instead of blocking later runs. | -| `~/.opencodex/responses-state.json`, `usage-debug.jsonl`, `crash.log`, `artifacts/` | opencodex diagnostics and artifacts | Bounded caches, diagnostics, and generated image/video artifacts served locally. | +| `~/.opencodex/responses-state.json`, `responses-state-spill/`, `usage-debug.jsonl`, `crash.log`, `artifacts/` | opencodex diagnostics and artifacts | Bounded caches, diagnostics, and generated image/video artifacts served locally. The spill directory holds continuation state demoted out of the in-memory cap and is bounded in aggregate, not only per file. | | `~/.opencodex/codex-shim.json`, `*.lock`, `kimi-device-id`, `mimo-client-id`, `.star-prompted` | opencodex bookkeeping | Shim restore obligations, cross-process locks, per-install client identifiers, one-shot UI flags. | | `~/.opencodex/.opencodex-owner.json`, `.opencodex-uninstall.json` | opencodex | Ownership marker and the manifest that bounds what uninstall may remove. Both live in the OpenCodex state root, not in `$CODEX_HOME`. | | `$CODEX_HOME/config.toml` | Codex, edited by opencodex | Active provider and provider table. | diff --git a/structure/02_config-and-codex-home.md b/structure/02_config-and-codex-home.md index 8411884b5c..ddfd1d67e0 100644 --- a/structure/02_config-and-codex-home.md +++ b/structure/02_config-and-codex-home.md @@ -157,6 +157,34 @@ Hidden` inside an already-running PowerShell script, nor to .NET/VBS process-win - 다른 대안 대신 이 방식을 선택한 이유: Names and environment paths are caller-controlled, required secret writes must not silently skip ACLs, and elevation has a larger authority boundary that should remain FFI-only. - 장점, 단점 및 영향: Default Windows ARM64 installations can start and harden secrets; non-default Windows roots continue to fail closed until Bun exposes a trustworthy native system-directory API without FFI. +The durable response-spill directory `~/.opencodex/responses-state-spill/` is bounded in +aggregate, not only per file. Continuation state demoted out of the in-memory cap +(`MAX_STORED_RESPONSE_BYTES`) is written there, and eviction past +`MAX_SPILLED_RESPONSE_BYTES` removes oldest-first through the same deletion point that serves +TTL and count eviction, so an evicted entry unlinks its file. One function owns that ceiling and +three callers drive it: mutation pruning, the lazy load that follows a restart, and the periodic +sweep. The periodic caller is not redundant — the mutation path runs only when traffic arrives, so a +process that comes up over budget from a snapshot written under a larger ceiling would otherwise +stay over it while idle. + +The ceiling bounds what the store can account for, which is every entry in the map plus the +superseded generations queued for unlink, and deliberately not the directory as a whole. Spill files +orphaned by a crash are absent from the map, so this accounting can neither see nor price them; they +remain with the `recoverOrphanedResponseSpills` grace sweep described below, which is the only +mechanism that reclaims them. A host that crashes repeatedly can therefore hold spill bytes above +this ceiling for up to `RESPONSE_SPILL_ORPHAN_GRACE_MS` past each crash. Without that aggregate bound the +directory was limited only per file (256 MiB) and per entry (1000) — a 250 GiB product — which +left `RESPONSE_TTL_MS` as the only effective limit and made disk use a function of client +request rate rather than of anything the process controls. + +[Decision Log] +- 목적과 의도: Bound the durable spill directory in aggregate so demoted continuation state cannot consume the host disk. +- 기존 구현 및 제약 조건: The resident map has an unconditional byte cap and demotes past it, but the disk it demotes onto had only a per-file ceiling and the shared 1000-entry count cap. Retention itself worked — the hour-long TTL did evict — so the gap was a missing budget, not a leak. +- 검토한 주요 대안: Lower the per-file ceiling; shorten the TTL; sweep the directory on a timer; add a configurable budget key; carry a running byte counter. +- 선택한 방식: A constant aggregate ceiling checked at the end of the existing prune, evicting oldest-first, with the total recomputed per prune rather than carried as a counter. +- 다른 대안 대신 이 방식을 선택한 이유: Per-file or TTL changes alter retention semantics other bounds depend on; a timer adds a second owner for eviction; a config key would surface a knob the sibling bounds (count, TTL, per-file) do not have; and a running counter could silently disable the cap if any of the several insertion paths missed an increment, where a walk over at most 1000 entries cannot drift. +- 장점, 단점 및 영향: Disk use stops tracking client request rate. Ordinary traffic is unaffected because the count cap binds at a comparable point for median-sized payloads; a workload of unusually large continuations loses its oldest spills earlier than the TTL would, surfacing as the existing `previous_response_not_found` continuation miss. + Response-state loading performs a bounded recovery pass for interrupted snapshot writes. It only matches regular files named `responses-state.json.ocx...tmp`, waits at least 15 minutes, and skips the current or any live PID. Eligible files are truncated before unlinking so a diff --git a/tests/responses-state.test.ts b/tests/responses-state.test.ts index 3422d8447b..8f22fd444d 100644 --- a/tests/responses-state.test.ts +++ b/tests/responses-state.test.ts @@ -38,6 +38,7 @@ import { recoverStaleResponseStateTemps, rememberResponseState, sweepAbandonedResponseStateTemps, + sweepExpiredResponseStates, responseAdmissionCountersForTests, responseStateMetrics, responseStatePersistPendingForTests, @@ -45,6 +46,9 @@ import { runPendingResponseStatePersistForTests, setResponseSpillAsyncAclAttemptBudgetForTests, setResponseStateByteCapForTests, + setSpilledResponseByteCapForTests, + getSpilledResponseBytesForTests, + getAccountedResponseSpillBytesForTests, setResponseStatePersistAttemptHookForTests, setResponseSpillShutdownBudgetForTests, getStoredResponseBytesForTests, @@ -131,6 +135,17 @@ function spillTempNames(home: string): string[] { return existsSync(dir) ? readdirSync(dir).filter(name => name.endsWith(".tmp")) : []; } +/** Real bytes the spill directory occupies, temps included. */ +function bytesOnDisk(home: string): number { + const dir = responseSpillDirectory(home); + if (!existsSync(dir)) return 0; + let total = 0; + for (const name of readdirSync(dir)) { + try { total += statSync(join(dir, name)).size; } catch { /* raced with an unlink */ } + } + return total; +} + interface ShutdownBudgetChildResult { settled: boolean; reported: boolean; @@ -282,6 +297,7 @@ describe("Responses previous_response_id state", () => { setResponseSpillShutdownBudgetForTests(null); setResponseSpillAsyncAclAttemptBudgetForTests(null); setResponseStateByteCapForTests(null); + setSpilledResponseByteCapForTests(null); clearResponseStateForTests(); rmSync(home, { recursive: true, force: true }); if (priorHome === undefined) delete process.env["OPENCODEX_HOME"]; @@ -812,6 +828,23 @@ describe("Responses previous_response_id state", () => { }) as { input: unknown[] }).input).toHaveLength(3); }); + test("evicts oldest spills once the durable set exceeds the disk cap", () => { + setResponseStateByteCapForTests(1_024); + setSpilledResponseByteCapForTests(20_000); + for (let i = 0; i < 6; i += 1) rememberLarge(`resp_spill_budget_${i}`, "x".repeat(8_000)); + // Without a disk cap all six stay on disk: the RAM cap only moves bytes out of + // memory, it never bounds where they land. + expect(getSpilledResponseBytesForTests()).toBeLessThanOrEqual(20_000); + expect(spillFileNames(home).length).toBeLessThan(6); + // The oldest entry is the one that must be gone, not merely "some" entry. + expect(spillFileNames(home).some(name => name.startsWith("resp_spill_budget_0."))).toBe(false); + // Eviction is oldest-first and must not clear the set it was asked to bound. + expect(spillFileNames(home).length).toBeGreaterThanOrEqual(1); + expect((expandPreviousResponseInput({ + previous_response_id: "resp_spill_budget_5", input: "next", + }) as { input: unknown[] }).input).toHaveLength(3); + }); + test("does not swap a resident row to a stub before fsync and no-replace publication succeed", () => { const events: string[] = []; setSpillIoForTest({ record: event => events.push(event) }); @@ -857,6 +890,126 @@ describe("Responses previous_response_id state", () => { expect(responseStateMetrics()).toMatchObject({ residentCount: 0, spillStubCount: 1, spillWrites: 1, spillWriteFailures: 0 }); }); + test("holds the disk cap while a copy-fallback publication has temp and destination on disk", async () => { + // The cap is a promise about the volume, and a file being created by + // writeResponseSpillDurablyAsync is on the volume. Accounting that walks only + // installed spills reports a satisfied budget while the directory grows — the + // incident behind this cap put 6.8 GiB on disk in 44 minutes. + // + // The peak is TWO envelopes, not one: when hard-linking fails, publication copies + // with COPYFILE_EXCL and then hardens the destination, so the temp and the copy exist + // together. This drives that exact path and measures real bytes on disk. + setPlatformForTests("win32"); + setResponseStateByteCapForTests(1_024); + + let gateDestinationHarden = false; + let release!: () => void; + let entered!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const started = new Promise(resolve => { entered = resolve; }); + let announced = false; + setAsyncIcaclsRunnerForTests(async args => { + if (gateDestinationHarden && args.some(arg => arg.endsWith(".spill.json"))) { + if (!announced) { + announced = true; + entered(); + } + await gate; + } + return { success: true, exitCode: 0, timedOut: false, stdout: "" }; + }); + + // One resident spill already on disk, so the cap has real prior occupancy. + rememberLarge("resp_cap_existing", "e".repeat(8_000)); + await flushPendingResponseSpillsForTests(); + const existingBytes = getSpilledResponseBytesForTests(); + expect(existingBytes).toBeGreaterThan(0); + expect(spillFileNames(home)).toHaveLength(1); + + // Room for the two-envelope publication and nothing more. The seeded spill does not + // fit alongside it, so correct admission must reclaim it before publishing; without + // the check, seeded + temp + destination sit on disk together and blow the cap. + // Generous enough that this publication is admitted: the point of THIS test is that + // the accounting sees the in-flight bytes. The cap-refusal behaviour is proven + // separately below, where admission is the only thing standing between the request + // and an over-budget directory. + const spillCap = existingBytes * 4; + setSpilledResponseByteCapForTests(spillCap); + + // Force the exclusive-copy fallback, then gate the destination hardening that follows + // it, so the observation below happens with BOTH files present. + setSpillIoForTest({ + link: () => { throw Object.assign(new Error("EXDEV"), { code: "EXDEV" }); }, + }); + gateDestinationHarden = true; + + rememberLarge("resp_cap_inflight", "x".repeat(8_000)); + await started; + try { + // Real disk: the temp and the copied destination coexist during hardening. + const onDisk = spillFileNames(home).length + spillTempNames(home).length; + expect(onDisk).toBeGreaterThanOrEqual(3); + // The walk over installed spills still reports only the settled file, so accounting + // built on it alone would price a three-envelope directory as one. + expect(getSpilledResponseBytesForTests()).toBe(existingBytes); + // Reservation prices the in-flight publication at its peak, so the accounted total + // covers what is actually on the volume. + expect(getAccountedResponseSpillBytesForTests()) + .toBeGreaterThanOrEqual(existingBytes * 3); + // And the bytes ACTUALLY on disk stay inside the configured cap. This is the + // assertion the admission check has to earn: without it, the seeded spill plus the + // temp plus the destination copy exceed a cap sized for two envelopes. + expect(bytesOnDisk(home)).toBeLessThanOrEqual(spillCap); + } finally { + release(); + setSpillIoForTest(null); + } + await flushPendingResponseSpillsForTests(); + // Settled: the reservation is released exactly once and accounting collapses to the + // real files. A leaked reservation would be monotonic — it would ratchet the usable + // cap toward zero until nothing could spill at all. + expect(getAccountedResponseSpillBytesForTests()).toBe(getSpilledResponseBytesForTests()); + expect(spillTempNames(home)).toHaveLength(0); + // And the newest continuation is still replayable: the cap must not have turned the + // fail-closed path into the ordinary one. + expect(JSON.stringify(expandPreviousResponseInput({ previous_response_id: "resp_cap_inflight", input: "next" }))) + .toContain("xxxxxxxx"); + }); + + test("refuses a publication whose peak footprint does not fit the disk cap", async () => { + // Enforcement, not accounting: a publication that cannot fit must be refused BEFORE + // any file is created. Deleting the overflow afterwards is not equivalent — on + // Windows the file outlives the decision by however long ACL hardening takes, which + // is the window the measured 6.8 GiB accumulated in. + setPlatformForTests("win32"); + setResponseStateByteCapForTests(1_024); + setAsyncIcaclsRunnerForTests(async () => ({ success: true, exitCode: 0, timedOut: false, stdout: "" })); + + rememberLarge("resp_cap_seed", "s".repeat(8_000)); + await flushPendingResponseSpillsForTests(); + const seededBytes = getSpilledResponseBytesForTests(); + expect(spillFileNames(home)).toHaveLength(1); + + // Room for the seeded spill and nothing else. The next publication needs two + // envelopes at peak, and the seeded file is live rather than reclaimable-on-sight, + // so admission has to refuse. + setSpilledResponseByteCapForTests(Math.floor(seededBytes * 1.5)); + + rememberLarge("resp_cap_refused", "r".repeat(8_000)); + await flushPendingResponseSpillsForTests(); + + // No second file, and no temp left behind: the refusal happened before publication. + expect(spillFileNames(home)).toHaveLength(1); + expect(spillTempNames(home)).toHaveLength(0); + expect(bytesOnDisk(home)).toBeLessThanOrEqual(Math.floor(seededBytes * 1.5)); + // Fail-closed, and it says so: the refused continuation is a spill failure, not a + // silent drop, so replay reports it and the client resends. + expect(responseStateMetrics()).toMatchObject({ spillWriteFailures: 1 }); + // The seeded continuation is untouched — a refusal must not cost an unrelated replay. + expect(JSON.stringify(expandPreviousResponseInput({ previous_response_id: "resp_cap_seed", input: "next" }))) + .toContain("ssssssss"); + }); + test("Windows spill retries one transient ACL timeout without installing a tombstone", async () => { setPlatformForTests("win32"); process.env.OPENCODEX_ACL_TIMEOUT_MS = "1000"; @@ -1120,6 +1273,57 @@ describe("Responses previous_response_id state", () => { } }); + test("shutdown fallback prices the job-owned superseded generation before publishing", async () => { + // Supersession releases the job, so its superseded generation leaves the accounting + // walk while the FILE stays on the volume. Without pricing it, the fallback decides + // against a total short by a whole envelope, and a cap sitting between + // (debt + footprint) and (old + debt + footprint) admits a publication that puts the + // directory over budget. + setPlatformForTests("win32"); + setResponseSpillShutdownBudgetForTests({ totalMs: 120, fallbackReserveMs: 80 }); + let release!: () => void; + let entered!: () => void; + const gate = new Promise(resolve => { release = resolve; }); + const started = new Promise(resolve => { entered = resolve; }); + let announced = false; + setAsyncIcaclsRunnerForTests(async () => { + if (!announced) { + announced = true; + entered(); + } + await gate; + return { success: true, exitCode: 0, timedOut: false, stdout: "" }; + }); + setIcaclsRunnerForTests(() => ({ success: true, exitCode: 0, timedOut: false, stdout: "" })); + setResponseStateByteCapForTests(1_024); + + // First generation settles to a real file, then a same-id replacement makes the job + // the owner of that superseded generation. + rememberLarge("resp_shutdown_superseded", "o".repeat(8_000)); + await flushPendingResponseSpillsForTests(); + const oldBytes = getSpilledResponseBytesForTests(); + expect(oldBytes).toBeGreaterThan(0); + + rememberLarge("resp_shutdown_superseded", "n".repeat(8_000)); + await started; + + try { + // Cap allows the new publication on its own, but not alongside the superseded + // generation the job still owns. + setSpilledResponseByteCapForTests(Math.floor(oldBytes * 2.4)); + // The refusal surfaces as a shutdown failure, which is the honest signal: the + // operator learns a continuation was dropped rather than the volume being + // silently overfilled. + await expect(flushResponseState()).rejects.toThrow(/shutdown fallback incomplete/); + // Fail-closed rather than over-budget: the drain refused to add a third envelope. + expect(bytesOnDisk(home)).toBeLessThanOrEqual(Math.floor(oldBytes * 2.4)); + expect(spillTempNames(home)).toHaveLength(0); + expect(pendingResponseSpillMetricsForTests()).toEqual({ count: 0, bytes: 0 }); + } finally { + release(); + } + }); + test("shutdown fallback spends only its reserved ACL budget", async () => { setPlatformForTests("win32"); const totalMs = 500; @@ -1535,6 +1739,147 @@ describe("Responses previous_response_id state", () => { expect(spillFileNames(home)).toHaveLength(1); }); + test("evicts by createdAt, not by insertion order", () => { + const realNow = Date.now; + try { + setResponseStateByteCapForTests(1_024); + // Insert the NEWER entry first so insertion order and createdAt order + // disagree. Map order alone would evict the newer one; `states` is not a + // reliable age index — writeBoundedSnapshot even serializes it reversed. + const base = realNow(); + Date.now = () => base; + const beforeNewer = new Set(spillFileNames(home)); + rememberLarge("resp_order_newer", "x".repeat(8_000)); + const newerFile = spillFileNames(home).find(name => !beforeNewer.has(name))!; + + Date.now = () => base - 5 * 60_000; + const beforeOlder = new Set(spillFileNames(home)); + rememberLarge("resp_order_older", "x".repeat(8_000)); + const olderFile = spillFileNames(home).find(name => !beforeOlder.has(name))!; + Date.now = () => base; + + // Each spill payload is ~16.2 KB, so 20_000 leaves room for exactly one. + setSpilledResponseByteCapForTests(20_000); + rememberLarge("resp_order_trigger", "y"); + + const dir = responseSpillDirectory(home); + expect(existsSync(join(dir, olderFile))).toBe(false); + expect(existsSync(join(dir, newerFile))).toBe(true); + } finally { + Date.now = realNow; + } + }); + + test("breaks createdAt ties on the response id, not on insertion order", () => { + const realNow = Date.now; + try { + setResponseStateByteCapForTests(1_024); + const base = realNow(); + Date.now = () => base; + // Same createdAt, inserted in reverse id order: a stable sort alone would + // keep insertion order and evict "_b" first. + const before = new Set(spillFileNames(home)); + rememberLarge("resp_tie_b", "x".repeat(8_000)); + const bFile = spillFileNames(home).find(name => !before.has(name))!; + const beforeA = new Set(spillFileNames(home)); + rememberLarge("resp_tie_a", "x".repeat(8_000)); + const aFile = spillFileNames(home).find(name => !beforeA.has(name))!; + + setSpilledResponseByteCapForTests(20_000); + rememberLarge("resp_tie_trigger", "y"); + + const dir = responseSpillDirectory(home); + expect(existsSync(join(dir, aFile))).toBe(false); + expect(existsSync(join(dir, bFile))).toBe(true); + } finally { + Date.now = realNow; + } + }); + + test("survives a single spill payload larger than the whole disk budget", () => { + setResponseStateByteCapForTests(1_024); + // Below one payload (~16.2 KB), so every spill is written and then evicted on + // the same tick and the running total goes negative. The loop must still + // terminate and leave the store usable. + setSpilledResponseByteCapForTests(1_000); + rememberLarge("resp_over_budget_a", "x".repeat(8_000)); + rememberLarge("resp_over_budget_b", "x".repeat(8_000)); + expect(spillFileNames(home)).toHaveLength(0); + expect(getSpilledResponseBytesForTests()).toBe(0); + // Raising the cap restores ordinary retention: the store is not wedged. + setSpilledResponseByteCapForTests(1_000_000); + rememberLarge("resp_over_budget_c", "x".repeat(8_000)); + expect(spillFileNames(home)).toHaveLength(1); + }); + + test("counts deferred spill generations against the disk cap and drains them first", () => { + setResponseStateByteCapForTests(1_024); + // Replacing a spilled id queues the superseded file in pendingSpillUnlinks + // instead of unlinking it, so the directory holds generations that `states` + // alone cannot see. Payloads are ~16.2 KB, so 60_000 fits about three. + setSpilledResponseByteCapForTests(60_000); + for (let i = 0; i < 8; i += 1) rememberLarge("resp_deferred_gen", "x".repeat(8_000)); + // Counting only the live entry would report ~16 KB here and evict nothing while + // eight files sat on disk. + expect(getSpilledResponseBytesForTests()).toBeLessThanOrEqual(60_000); + const onDisk = spillFileNames(home); + expect(onDisk.length).toBeLessThanOrEqual(4); + // The live continuation survives: deferred generations are released first. + expect((expandPreviousResponseInput({ + previous_response_id: "resp_deferred_gen", input: "next", + }) as { input: unknown[] }).input).toHaveLength(3); + }); + + test("reclaims an over-budget snapshot without a new continuation mutation", async () => { + const realNow = Date.now; + try { + setResponseStateByteCapForTests(4_000); + const base = realNow(); + const files: string[] = []; + for (let i = 0; i < 4; i += 1) { + Date.now = () => base - (10 - i) * 60_000; + const before = new Set(spillFileNames(home)); + rememberResponseState( + { model: "test/model", input: "z".repeat(8_000), store: false }, + fixedResponse(`resp_budget_restart_${i}`, [{ type: "message", role: "assistant", content: "stored" }]), + undefined, + { force: true, clientThreadId: "task-budget" }, + ); + files.push(spillFileNames(home).find(name => !before.has(name))!); + } + Date.now = () => base; + await flushResponseState(); + clearResponseStateMemoryForTests(); + + // Come back up under a ceiling the snapshot was not written for. Nothing has + // mutated the store yet, which is the case the mutation-path prune misses. + setResponseStateByteCapForTests(4_000); + // Four spills total ~32.9 KB, so this ceiling keeps the two newest. + setSpilledResponseByteCapForTests(20_000); + expect(spillFileNames(home).length).toBe(4); + + // A read, not a write: this drives the lazy load and its prune. + const expanded = expandPreviousResponseInput( + { previous_response_id: "resp_budget_restart_3", input: "next" }, + "task-budget", + ); + + expect(getSpilledResponseBytesForTests()).toBeLessThanOrEqual(20_000); + expect(existsSync(join(responseSpillDirectory(home), files[0]!))).toBe(false); + // The newest entry survives the reclaim and still replays. + expect(existsSync(join(responseSpillDirectory(home), files[3]!))).toBe(true); + expect((expanded as { input: unknown[] }).input).toHaveLength(3); + + // The periodic sweep owns the same ceiling: lower it again and let the tick, + // not a request, do the work. + setSpilledResponseByteCapForTests(10_000); + sweepExpiredResponseStates(); + expect(getSpilledResponseBytesForTests()).toBeLessThanOrEqual(10_000); + } finally { + Date.now = realNow; + } + }); + test("TTL and count eviction delete dedicated spill files and release stub bytes", () => { const realNow = Date.now; setResponseStateByteCapForTests(1_024);