From 3f63e7946278e7b09283d16e243a7f13f834048b Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 19 Aug 2026 11:13:36 +0900 Subject: [PATCH 1/4] feat(doctor): report and reclaim abandoned response-state temps --- docs-site/astro.config.mjs | 1 + .../troubleshooting/disk-usage-temp-files.md | 74 +++++++++++++++++++ src/cli/doctor.ts | 45 +++++++++++ src/responses/state.ts | 48 +++++++++++- tests/doctor.test.ts | 42 +++++++++++ tests/responses-state.test.ts | 52 ++++++++++++- 6 files changed, 257 insertions(+), 5 deletions(-) create mode 100644 docs-site/src/content/docs/troubleshooting/disk-usage-temp-files.md diff --git a/docs-site/astro.config.mjs b/docs-site/astro.config.mjs index d71631d1cf..fb790ff743 100644 --- a/docs-site/astro.config.mjs +++ b/docs-site/astro.config.mjs @@ -155,6 +155,7 @@ export default defineConfig({ collapsed: true, items: [ { label: "Windows Memory Growth", translations: { fr: "Augmentation de la mémoire sous Windows", ko: "Windows 메모리 증가", "zh-CN": "Windows 内存增长", "zh-TW": "Windows 記憶體增長", ru: "Рост памяти в Windows", ja: "Windows メモリ増加", tr: "Windows Bellek Artışı" }, slug: "troubleshooting/windows-memory" }, + { label: "Disk Usage from Temp Files", translations: { fr: "Espace disque et fichiers temporaires", ko: "임시 파일 디스크 사용량", "zh-CN": "临时文件磁盘占用", "zh-TW": "暫存檔磁碟用量", ru: "Использование диска временными файлами", ja: "一時ファイルのディスク使用量", tr: "Geçici Dosya Disk Kullanımı" }, slug: "troubleshooting/disk-usage-temp-files" }, ], }, { label: "Contributing", translations: { fr: "Contribuer", ko: "기여하기", "zh-CN": "贡献", "zh-TW": "貢獻", ru: "Как внести вклад", ja: "コントリビュート", tr: "Katkıda Bulunma" }, slug: "contributing" }, diff --git a/docs-site/src/content/docs/troubleshooting/disk-usage-temp-files.md b/docs-site/src/content/docs/troubleshooting/disk-usage-temp-files.md new file mode 100644 index 0000000000..68a4d78f01 --- /dev/null +++ b/docs-site/src/content/docs/troubleshooting/disk-usage-temp-files.md @@ -0,0 +1,74 @@ +--- +title: Disk Usage from Temp Files +description: What responses-state.json.ocx.*.tmp files are, why they could accumulate, and how to reclaim them. +--- + +Some users found many gigabytes of files named like +`responses-state.json.ocx...tmp` in their opencodex home +(`~/.opencodex` by default), growing after every reboot. + +## What these files are + +opencodex keeps a continuation cache so `previous_response_id` chains survive a +proxy restart. It writes that snapshot atomically: content goes to a temp file +first, then replaces the real file in one step. That is what stops a crash +mid-write from leaving a half-written snapshot. + +The temp is normally removed the instant the swap completes. If the process dies +between the two steps, the temp survives. + +Each file can be up to 24 MB because the snapshot is rewritten whole, not +appended to. A few hundred abandoned files therefore add up quickly. + +**They are cache, not durable state.** Deleting them costs nothing except that +in-flight conversation chains may re-send context once. No configuration, +credentials, or history live in these files. + +## Why they could accumulate + +A cleanup already existed, but it ran at one moment only: when a proxy loaded +the continuation cache for the first time, which happens *before* that process +writes anything. Two consequences followed. + +A proxy that crashed and restarted swept too early to see the temp its +predecessor had just left — there is a 15-minute grace period so a file being +written right now is never touched — and it never looked again for the rest of +its life. Each restart then added one more file. + +Worse, the cleanup skipped any file whose owning process ID was still alive. +After a reboot the operating system routinely reissues the same process IDs, so +an old file could be permanently mistaken for one belonging to a running +process. That is why the growth tracked reboots. + +## What opencodex does now + +The cleanup repeats on the proxy's normal background timer instead of running +once at startup, so a running proxy reclaims abandoned files on its own. It also +ignores the process-ID check for files older than the current boot, since no +running process can own those. + +The safety rules are unchanged: a file younger than 15 minutes is never removed, +and the proxy never removes a file it is writing itself. + +## Reclaiming files that already accumulated + +If the proxy runs, this happens automatically within a minute or two. + +If the proxy will **not** start — the case where the pile grows fastest — check +and reclaim from the command line: + +```bash +ocx doctor +``` + +The "Response-state temp files" section reports how many files are reclaimable +and how much space they hold. It only reports; it changes nothing. + +To actually remove them: + +```bash +ocx doctor --reclaim-response-temps +``` + +Both commands work without a running proxy. Files currently locked by another +process are reported rather than forced, and are retried later. diff --git a/src/cli/doctor.ts b/src/cli/doctor.ts index 7962c06672..7400d157c9 100644 --- a/src/cli/doctor.ts +++ b/src/cli/doctor.ts @@ -25,6 +25,11 @@ import { collectOrcaCodexHomeDiagnostic, resolveCodexHomeDir as resolveCodexHome import { scanCodexAgentRolesWithTomlModelFallback } from "../codex/subagent-model-fallback"; import { findCodexOnPath, isWindowsInteropDir } from "../codex/shim"; import { countPendingOpencodexHistory } from "../codex/history-provider"; +import { + inspectAbandonedResponseStateTemps, + reclaimAbandonedResponseStateTemps, + type ResponseStateTempRecoveryResult, +} from "../responses/state"; import { CodexUserIdentityRefusal, probeCodexCoordinatorNamespace, @@ -678,6 +683,37 @@ export async function fetchServiceMemory( const mb = (bytes: number): string => `${Math.round(bytes / (1024 * 1024))}MB`; +/** + * Render the abandoned-temp section (testable without console capture). + * + * Report is the DEFAULT and reclaim is opt-in: `doctor` is a diagnostic an operator runs + * to understand a machine, so deleting files as a side effect of asking a question is the + * wrong default even for cache files. + * + * Counts come from `eligible`/`eligibleBytes`, never `matched`: `matched` is incremented + * before the file-type, age, boot-floor, and liveness gates, so reporting it would tell an + * operator that live-pid temps and young temps are "abandoned". + */ +export function formatResponseTempLines( + result: ResponseStateTempRecoveryResult, + reclaimed: boolean, +): string[] { + if (reclaimed) { + if (result.removed === 0 && result.failed === 0) return [" ok No abandoned response-state temp files."]; + const lines = [` ok Reclaimed ${result.removed} abandoned response-state temp file(s), ${mb(result.bytesRemoved)} freed.`]; + if (result.failed > 0) { + lines.push(` !! ${result.failed} file(s) could not be removed (in use or locked). They are retried automatically.`); + } + return lines; + } + if (result.eligible === 0) return [" ok No abandoned response-state temp files."]; + return [ + ` !! ${result.eligible} abandoned response-state temp file(s), ${mb(result.eligibleBytes)} reclaimable.`, + " These are interrupted snapshot writes (continuation cache only) and are safe to remove.", + " Reclaim them with: ocx doctor --reclaim-response-temps", + ]; +} + /** Render the doctor "Memory / runtime" section lines (testable without console capture). */ export function formatServiceMemoryLines(report: ServiceMemoryReport): string[] { const lines: string[] = []; @@ -805,6 +841,15 @@ export async function runDoctor(args: string[] = []): Promise { console.log(` ${row.exists ? "ok " : "-- "} ${row.label}: ${row.path}${flags ? ` (${flags})` : ""}`); } + // Runs without the proxy on purpose: the worst accumulation happens when the proxy will + // not start, which is exactly when the in-process periodic reclaim never ticks. + const reclaimTemps = args.includes("--reclaim-response-temps"); + console.log("\nResponse-state temp files"); + for (const line of formatResponseTempLines( + reclaimTemps ? reclaimAbandonedResponseStateTemps() : inspectAbandonedResponseStateTemps(), + reclaimTemps, + )) console.log(line); + const orcaHome = collectOrcaCodexHomeDiagnostic(); console.log("\nCodex app home targeting"); console.log(` ${orcaHome.mismatch ? "!! " : "ok "} Effective Codex home: ${orcaHome.effectiveCodexHome}`); diff --git a/src/responses/state.ts b/src/responses/state.ts index d269c03f4b..39c83b7022 100644 --- a/src/responses/state.ts +++ b/src/responses/state.ts @@ -506,6 +506,18 @@ export interface ResponseStateTempRecoveryResult { removed: number; failed: number; bytesRemoved: number; + /** Entries that passed EVERY gate and would be reclaimed. In a dry run nothing is + * unlinked, so this is the only honest count to show an operator: `matched` is + * incremented before the file-type, age, boot-floor, and liveness gates. */ + eligible: number; + /** Total size of the `eligible` entries. */ + eligibleBytes: number; + /** The scan stopped on a budget (entry cap, cleanup cap, or deadline) rather than reaching + * the end of the directory, so the counts below describe a prefix of the backlog and not + * the backlog. `eligible > removed + failed` cannot express this: outside a dry run every + * eligible entry is unlinked or failed on the same iteration, so the two are always equal + * and a comparison between them is dead code. */ + truncated: boolean; } interface ResponseStateTempRecoveryIO { @@ -518,11 +530,13 @@ interface ResponseStateTempRecoveryIO { unlink: (path: string) => void; } -type ResponseStateTempRecoveryOptions = Partial & { +export type ResponseStateTempRecoveryOptions = Partial & { maxEntries?: number; maxCleanups?: number; /** Wall-clock ceiling for the scan, or null/undefined for no deadline (startup path). */ deadlineMs?: number | null; + /** Report only: apply every gate, count what would be reclaimed, unlink nothing. */ + dryRun?: boolean; }; function processIsAlive(pid: number): boolean { @@ -570,6 +584,7 @@ export function recoverStaleResponseStateTemps( maxEntries = STALE_TEMP_MAX_ENTRIES, maxCleanups = STALE_TEMP_MAX_CLEANUPS, deadlineMs = null, + dryRun = false, ...overrides } = options; const io = { ...responseStateTempRecoveryIO, ...overrides }; @@ -578,6 +593,9 @@ export function recoverStaleResponseStateTemps( removed: 0, failed: 0, bytesRemoved: 0, + eligible: 0, + eligibleBytes: 0, + truncated: false, }; const startedAt = io.now(); // One probe per scan, not one per entry. A non-finite or future-dated boot is anomalous, and @@ -607,8 +625,11 @@ export function recoverStaleResponseStateTemps( if (next.done) break; const name = next.value; scanned += 1; - if (scanned > maxEntries || result.removed + result.failed >= maxCleanups) return stopScan(); - if (deadlineMs !== null && io.now() - startedAt > deadlineMs) return stopScan(); + // A dry run performs no cleanups, so bounding it by the cleanup budget would truncate + // the very report an operator uses to size the problem. + if (scanned > maxEntries) { result.truncated = true; return stopScan(); } + if (!dryRun && result.removed + result.failed >= maxCleanups) { result.truncated = true; return stopScan(); } + if (deadlineMs !== null && io.now() - startedAt > deadlineMs) { result.truncated = true; return stopScan(); } const match = RESPONSE_STATE_TEMP_NAME.exec(name); if (!match) continue; result.matched += 1; @@ -631,6 +652,10 @@ export function recoverStaleResponseStateTemps( if (pid === process.pid) continue; if (!predatesBoot && io.isProcessAlive(pid)) continue; + result.eligible += 1; + result.eligibleBytes += file.size; + if (dryRun) continue; + try { io.unlink(path); result.removed += 1; @@ -988,7 +1013,9 @@ export function sweepExpiredResponseStates(at = now()): number { export function reclaimAbandonedResponseStateTemps( options: ResponseStateTempRecoveryOptions = {}, ): ResponseStateTempRecoveryResult { - const total: ResponseStateTempRecoveryResult = { matched: 0, removed: 0, failed: 0, bytesRemoved: 0 }; + const total: ResponseStateTempRecoveryResult = { + matched: 0, removed: 0, failed: 0, bytesRemoved: 0, eligible: 0, eligibleBytes: 0, truncated: false, + }; // The try encloses responseStateSweepDirectories() deliberately: recoverStaleResponseStateTemps // already swallows its own enumeration failures, so a catch around only that call would be // unreachable. snapshotPath()/getConfigDir() are the paths that can genuinely throw. @@ -999,6 +1026,10 @@ export function reclaimAbandonedResponseStateTemps( total.removed += result.removed; total.failed += result.failed; total.bytesRemoved += result.bytesRemoved; + total.eligible += result.eligible; + total.eligibleBytes += result.eligibleBytes; + // Truncation anywhere makes the whole total a prefix. + total.truncated ||= result.truncated; } } catch { /* best-effort: disk reclaim must never destabilize the caller */ @@ -1006,6 +1037,15 @@ export function reclaimAbandonedResponseStateTemps( return total; } +/** + * Report-only counterpart for `ocx doctor`: applies every selection gate and unlinks + * nothing. It runs the SAME predicate as the reclaim, so the report and the subsequent + * removal cannot disagree about which files are reclaimable. + */ +export function inspectAbandonedResponseStateTemps(): ResponseStateTempRecoveryResult { + return reclaimAbandonedResponseStateTemps({ dryRun: true }); +} + /** Sweeper adapter: narrows the reclaim to the `() => number` the liveness tick expects. */ export function sweepAbandonedResponseStateTemps(): number { return reclaimAbandonedResponseStateTemps({ diff --git a/tests/doctor.test.ts b/tests/doctor.test.ts index d7b00691fa..0f042ed342 100644 --- a/tests/doctor.test.ts +++ b/tests/doctor.test.ts @@ -10,6 +10,7 @@ import { collectRunningProxyEnv, collectWslDualInstall, fetchServiceMemory, + formatResponseTempLines, formatServiceMemoryLines, parseProcessEnvBlock, probeWham, @@ -623,3 +624,44 @@ describe("service memory section (#314 WP4)", () => { expect(conflict).toContain("ocx service install"); }); }); + +describe("doctor abandoned response-state temps", () => { + const result = (over: Partial[0]> = {}) => ({ + matched: 0, removed: 0, failed: 0, bytesRemoved: 0, eligible: 0, eligibleBytes: 0, ...over, + }); + + test("reports reclaimable files without removing them, and names the opt-in flag", () => { + // Report is the default: doctor is a diagnostic, so it must not delete as a side effect + // of being asked a question. + const lines = formatResponseTempLines(result({ matched: 9, eligible: 3, eligibleBytes: 72 * 1024 * 1024 }), false); + expect(lines[0]).toContain("3 abandoned response-state temp file(s)"); + expect(lines[0]).toContain("72MB"); + expect(lines.join("\n")).toContain("ocx doctor --reclaim-response-temps"); + }); + + test("reports eligible, never matched", () => { + // matched counts name-matching entries BEFORE the age/liveness/file-type gates, so + // reporting it would call live-pid and young temps abandoned. + const lines = formatResponseTempLines(result({ matched: 12, eligible: 0 }), false); + expect(lines).toEqual([" ok No abandoned response-state temp files."]); + expect(lines.join("\n")).not.toContain("12"); + }); + + test("reclaim mode reports what was freed", () => { + const lines = formatResponseTempLines(result({ matched: 4, removed: 2, bytesRemoved: 48 * 1024 * 1024 }), true); + expect(lines[0]).toContain("Reclaimed 2"); + expect(lines[0]).toContain("48MB"); + expect(lines.join("\n")).not.toContain("--reclaim-response-temps"); + }); + + test("locked files are surfaced honestly and described as retried", () => { + const lines = formatResponseTempLines(result({ matched: 3, removed: 1, failed: 2, bytesRemoved: 24 * 1024 * 1024 }), true); + expect(lines.join("\n")).toContain("2 file(s) could not be removed"); + expect(lines.join("\n")).toContain("retried automatically"); + }); + + test("a clean machine says so in both modes", () => { + expect(formatResponseTempLines(result(), false)).toEqual([" ok No abandoned response-state temp files."]); + expect(formatResponseTempLines(result(), true)).toEqual([" ok No abandoned response-state temp files."]); + }); +}); diff --git a/tests/responses-state.test.ts b/tests/responses-state.test.ts index 3b530ab088..a468fa75f4 100644 --- a/tests/responses-state.test.ts +++ b/tests/responses-state.test.ts @@ -1623,7 +1623,7 @@ describe("Responses previous_response_id state", () => { }, }); - expect(result).toEqual({ matched: 0, removed: 0, failed: 0, bytesRemoved: 0 }); + expect(result).toEqual({ matched: 0, removed: 0, failed: 0, bytesRemoved: 0, eligible: 0, eligibleBytes: 0 }); }); test("periodic reclaim frees abandoned temps without any continuation access", () => { @@ -1725,6 +1725,56 @@ describe("Responses previous_response_id state", () => { expect(result).toMatchObject({ matched: 1, removed: 1, failed: 0 }); }); + test("a dry run reports exactly what a reclaim then removes", () => { + // Report and reclaim must share one predicate. If they drift, doctor tells an operator + // to reclaim files it will then refuse to touch (or vice versa). + const old = new Date(Date.now() - 60 * 60 * 1_000); + const deadPid = process.pid === 4242 ? 4243 : 4242; + const stale = join(home, `responses-state.json.ocx.${deadPid}.1.tmp`); + const live = join(home, "responses-state.json.ocx.5252.2.tmp"); + const young = join(home, "responses-state.json.ocx.6262.3.tmp"); + for (const path of [stale, live, young]) writeFileSync(path, "private state"); + for (const path of [stale, live]) utimesSync(path, old, old); + + const io = { isProcessAlive: (pid: number) => pid === 5252, bootTime: () => 0 }; + const report = recoverStaleResponseStateTemps(home, { ...io, dryRun: true }); + + // matched counts every name-matching entry, including the live and young ones; only + // eligible survives every gate. Reporting matched would overstate by 2 here. + expect(report).toMatchObject({ matched: 3, eligible: 1, removed: 0, failed: 0 }); + expect(report.eligibleBytes).toBe("private state".length); + for (const path of [stale, live, young]) expect(existsSync(path)).toBe(true); + + const reclaim = recoverStaleResponseStateTemps(home, io); + expect(reclaim.removed).toBe(report.eligible); + expect(reclaim.bytesRemoved).toBe(report.eligibleBytes); + expect(existsSync(stale)).toBe(false); + for (const path of [live, young]) expect(existsSync(path)).toBe(true); + }); + + test("a dry run is not truncated by the cleanup budget", () => { + // maxCleanups counts removals. A report removes nothing, so bounding it by that budget + // would under-report precisely the large backlog an operator needs to see. + const old = new Date(Date.now() - 60 * 60 * 1_000); + const names = [7301, 7302, 7303].map(pid => `responses-state.json.ocx.${pid}.1.tmp`); + for (const name of names) { + const path = join(home, name); + writeFileSync(path, "private state"); + utimesSync(path, old, old); + } + + const report = recoverStaleResponseStateTemps(home, { + list: () => names, + isProcessAlive: () => false, + bootTime: () => 0, + maxCleanups: 1, + dryRun: true, + }); + + expect(report.eligible).toBe(3); + for (const name of names) expect(existsSync(join(home, name))).toBe(true); + }); + test("the periodic scan stops at its wall-clock deadline", () => { const old = new Date(Date.now() - 60 * 60 * 1_000); const names = ["responses-state.json.ocx.9201.1.tmp", "responses-state.json.ocx.9202.2.tmp"]; From 3d4a3fb531fc6b0069447fd9c8c6a638f56b4c66 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 19 Aug 2026 11:21:17 +0900 Subject: [PATCH 2/4] fix(doctor): close audit round 3 blockers on the reclaim surface --- .../021_audit_round3.md | 60 +++++++++++++++ .../troubleshooting/disk-usage-temp-files.md | 10 ++- src/cli/doctor.ts | 32 ++++++-- src/cli/help.ts | 2 + tests/doctor.test.ts | 77 ++++++++++++++++++- 5 files changed, 171 insertions(+), 10 deletions(-) create mode 100644 devlog/_plan/260819_response_state_temp_reclaim/021_audit_round3.md diff --git a/devlog/_plan/260819_response_state_temp_reclaim/021_audit_round3.md b/devlog/_plan/260819_response_state_temp_reclaim/021_audit_round3.md new file mode 100644 index 0000000000..228fc02923 --- /dev/null +++ b/devlog/_plan/260819_response_state_temp_reclaim/021_audit_round3.md @@ -0,0 +1,60 @@ +# Audit round 3 — phase 2 implementation + +Reviewer: independent `explorer`, read-only, against `24a901d5c`. Verdict: +**GO-WITH-FIXES (blockers=5)**. Main-agent judgment: **near-pass** — all five folded, +none rebutted. + +## Confirmed + +- **The dry run shares one predicate.** The `dryRun` branch sits AFTER every gate + (basename, pid/seq sanity, inspect failure, isFile + grace, boot floor, self-pid, + liveness), so `eligible` is by construction the exact set that would reach `unlink`. + The drift risk the plan named is closed. +- **The default path deletes nothing** and needs no running server: the only syscalls are + `readdir`/`lstat`/`realpath`, and `getConfigDir()` is pure string resolution. +- **The layer stands alone** at its own tip. + +## Blocker 1 (accepted) — report and reclaim disagreed in MAGNITUDE + +The predicate agreed; the budget did not. The report was bounded by `maxEntries` (4096) +while the reclaim used the default `maxCleanups` (512). On the reported ~816-file backlog +doctor would say "816 reclaimable", then free 512 and print that, leaving 304 with no hint +that another run was needed. + +Fixed twice over: the doctor reclaim now passes a matching budget, AND a partial pass +prints how many remain with an instruction to run again. The second half matters because +any budget can still be exceeded. + +## Blocker 4 (accepted, the most serious) — the safety property had no test + +`formatResponseTempLines` tests feed literal objects to a pure formatter, so none of them +can observe deletion. Nothing covered the call site: **inverting the report/reclaim +ternary would have left the whole suite green.** The flagship property — "doctor does not +delete by default" — was claimed by three accept criteria and demonstrated by none. + +Fixed with an end-to-end `describe` that seeds a stale temp in an isolated +`OPENCODEX_HOME`, runs `runDoctor([])`, asserts the file SURVIVES, then runs the flag and +asserts it is gone. That test fails if the default is ever inverted. + +## Blocker 5 (accepted) — the CLI told a lie to its own target reader + +Both the CLI string and the docs promised locked files "are retried automatically". True +only while a proxy runs and ticks — but this command exists for the operator whose proxy +will NOT start. Reworded to "retried on the next reclaim — automatically while the proxy +runs, otherwise re-run this command", in the CLI and the docs, with a regression test +asserting the phrase "retried automatically" never appears. + +## Blockers 2 and 3 (accepted) — discoverability + +The flag had no help text, and a typo (`--reclaim-response-temp`) silently degraded into a +report, so an operator would read "nothing to reclaim" as an answer to a question they +never asked. Added to `ocx help`, and any unrecognized `--reclaim*` argument now warns. + +## Non-blocking, recorded + +- `bytesRemoved` under-counts against `eligibleBytes` when another process wins an ENOENT + race. Defensible — we did not free those bytes — and left as-is. +- The "none abandoned" line now names that it covers response-state temps specifically, + since the sibling producers (B9 in `002`) remain unreclaimed by design. +- i18n: only the English page was added, matching the existing convention for + `windows-memory.md`. Locale readers fall back to English; no contradiction is introduced. diff --git a/docs-site/src/content/docs/troubleshooting/disk-usage-temp-files.md b/docs-site/src/content/docs/troubleshooting/disk-usage-temp-files.md index 68a4d78f01..5dc636b856 100644 --- a/docs-site/src/content/docs/troubleshooting/disk-usage-temp-files.md +++ b/docs-site/src/content/docs/troubleshooting/disk-usage-temp-files.md @@ -71,4 +71,12 @@ ocx doctor --reclaim-response-temps ``` Both commands work without a running proxy. Files currently locked by another -process are reported rather than forced, and are retried later. +process are reported rather than forced. They are retried on the next reclaim — +automatically while the proxy is running, otherwise the next time you run this +command. + +If a very large backlog exceeds one pass, the command says how many files remain +so you can run it again. + +This covers response-state snapshot temps specifically. Other components write +their own temp files with a similar name, and those are not touched here. diff --git a/src/cli/doctor.ts b/src/cli/doctor.ts index 7400d157c9..59ac797ec1 100644 --- a/src/cli/doctor.ts +++ b/src/cli/doctor.ts @@ -683,6 +683,12 @@ export async function fetchServiceMemory( const mb = (bytes: number): string => `${Math.round(bytes / (1024 * 1024))}MB`; +export const RECLAIM_RESPONSE_TEMPS_FLAG = "--reclaim-response-temps"; +/** Matches the dry run's entry bound so report and reclaim agree on a large backlog. */ +const RESPONSE_TEMP_RECLAIM_MAX_CLEANUPS = 4_096; +/** Names the subsystem: other components mint temps with the same shape and are not covered. */ +const CLEAN_RESPONSE_TEMP_LINE = " ok No abandoned response-state temp files."; + /** * Render the abandoned-temp section (testable without console capture). * @@ -699,14 +705,19 @@ export function formatResponseTempLines( reclaimed: boolean, ): string[] { if (reclaimed) { - if (result.removed === 0 && result.failed === 0) return [" ok No abandoned response-state temp files."]; + if (result.removed === 0 && result.failed === 0) return [CLEAN_RESPONSE_TEMP_LINE]; const lines = [` ok Reclaimed ${result.removed} abandoned response-state temp file(s), ${mb(result.bytesRemoved)} freed.`]; if (result.failed > 0) { - lines.push(` !! ${result.failed} file(s) could not be removed (in use or locked). They are retried automatically.`); + // Never "retried automatically": this command exists for the operator whose proxy will + // NOT start, and in that state nothing retries anything. + lines.push(` !! ${result.failed} file(s) could not be removed (in use or locked). Retried on the next reclaim — automatically while the proxy runs, otherwise re-run this command.`); + } + if (result.eligible > result.removed + result.failed) { + lines.push(` !! Cleanup budget reached; ${result.eligible - result.removed - result.failed} file(s) remain. Run the command again to continue.`); } return lines; } - if (result.eligible === 0) return [" ok No abandoned response-state temp files."]; + if (result.eligible === 0) return [CLEAN_RESPONSE_TEMP_LINE]; return [ ` !! ${result.eligible} abandoned response-state temp file(s), ${mb(result.eligibleBytes)} reclaimable.`, " These are interrupted snapshot writes (continuation cache only) and are safe to remove.", @@ -843,10 +854,21 @@ export async function runDoctor(args: string[] = []): Promise { // Runs without the proxy on purpose: the worst accumulation happens when the proxy will // not start, which is exactly when the in-process periodic reclaim never ticks. - const reclaimTemps = args.includes("--reclaim-response-temps"); + const reclaimTemps = args.includes(RECLAIM_RESPONSE_TEMPS_FLAG); console.log("\nResponse-state temp files"); + // A typo must not silently degrade into "nothing to reclaim" — the operator would read the + // report as an answer to a question they never actually asked. + for (const arg of args) { + if (arg !== RECLAIM_RESPONSE_TEMPS_FLAG && /^--reclaim/.test(arg)) { + console.log(` !! Unrecognized flag ${arg}; did you mean ${RECLAIM_RESPONSE_TEMPS_FLAG}? Reporting only.`); + } + } for (const line of formatResponseTempLines( - reclaimTemps ? reclaimAbandonedResponseStateTemps() : inspectAbandonedResponseStateTemps(), + // The reclaim budget matches the report budget: a report bounded by entries and a removal + // bounded by a smaller cleanup cap would tell an operator 816 and then silently free 512. + reclaimTemps + ? reclaimAbandonedResponseStateTemps({ maxCleanups: RESPONSE_TEMP_RECLAIM_MAX_CLEANUPS }) + : inspectAbandonedResponseStateTemps(), reclaimTemps, )) console.log(line); diff --git a/src/cli/help.ts b/src/cli/help.ts index 19843c2e01..c335c347b5 100644 --- a/src/cli/help.ts +++ b/src/cli/help.ts @@ -36,6 +36,8 @@ Usage: Refresh Codex's model cache from the active catalog ocx status Check proxy server status ocx doctor Diagnose environment/network issues (WSL, proxy, ChatGPT reachability) + ocx doctor --reclaim-response-temps + Reclaim abandoned response-state temp files (works without a running proxy) ocx debug provider/usage/injection/claude on|off|status|reset ocx login OAuth or API-key provider login ocx logout Remove a stored OAuth login diff --git a/tests/doctor.test.ts b/tests/doctor.test.ts index 0f042ed342..45dedeb485 100644 --- a/tests/doctor.test.ts +++ b/tests/doctor.test.ts @@ -1,7 +1,7 @@ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { existsSync, mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, rmSync, utimesSync, writeFileSync } from "node:fs"; import { join } from "node:path"; -import { homedir } from "node:os"; +import { homedir, tmpdir } from "node:os"; import { collectPaths, detectFsType, @@ -16,6 +16,7 @@ import { probeWham, proxyDownRestartHint, resolveCodexHomeDir, + runDoctor, type ServiceMemoryData, } from "../src/cli/doctor"; import { collectOrcaCodexHomeDiagnostic } from "../src/codex/home"; @@ -654,14 +655,82 @@ describe("doctor abandoned response-state temps", () => { expect(lines.join("\n")).not.toContain("--reclaim-response-temps"); }); - test("locked files are surfaced honestly and described as retried", () => { + test("locked files are surfaced honestly", () => { const lines = formatResponseTempLines(result({ matched: 3, removed: 1, failed: 2, bytesRemoved: 24 * 1024 * 1024 }), true); expect(lines.join("\n")).toContain("2 file(s) could not be removed"); - expect(lines.join("\n")).toContain("retried automatically"); + expect(lines.join("\n")).toContain("in use or locked"); }); test("a clean machine says so in both modes", () => { expect(formatResponseTempLines(result(), false)).toEqual([" ok No abandoned response-state temp files."]); expect(formatResponseTempLines(result(), true)).toEqual([" ok No abandoned response-state temp files."]); }); + + test("a partial reclaim tells the operator to run again instead of silently stopping", () => { + const lines = formatResponseTempLines(result({ eligible: 816, removed: 512, bytesRemoved: 512 * 24 * 1024 * 1024 }), true); + expect(lines.join("\n")).toContain("304 file(s) remain"); + expect(lines.join("\n")).toContain("Run the command again"); + }); + + test("locked files are never described as retried automatically", () => { + // This command exists for the operator whose proxy will not start; in that state nothing + // retries anything, so promising automatic retry would be a lie to its target reader. + const lines = formatResponseTempLines(result({ removed: 1, failed: 2 }), true).join("\n"); + expect(lines).not.toContain("retried automatically"); + expect(lines).toContain("re-run this command"); + }); +}); + +describe("doctor reclaim wiring (end to end)", () => { + // The formatter tests above cannot observe deletion. This covers the call site itself: + // inverting the report/reclaim ternary in runDoctor must fail a test. + let tempHome: string; + let previousHome: string | undefined; + let logged: string[]; + const realLog = console.log; + + beforeEach(() => { + previousHome = process.env.OPENCODEX_HOME; + tempHome = join(tmpdir(), `ocx-doctor-temps-${Date.now()}-${Math.random().toString(16).slice(2)}`); + mkdirSync(tempHome, { recursive: true }); + process.env.OPENCODEX_HOME = tempHome; + logged = []; + console.log = (...parts: unknown[]) => { logged.push(parts.join(" ")); }; + }); + afterEach(() => { + console.log = realLog; + if (previousHome === undefined) delete process.env.OPENCODEX_HOME; + else process.env.OPENCODEX_HOME = previousHome; + rmSync(tempHome, { recursive: true, force: true }); + }); + + const seedStaleTemp = (): string => { + const deadPid = process.pid === 4242 ? 4243 : 4242; + const path = join(tempHome, `responses-state.json.ocx.${deadPid}.1.tmp`); + writeFileSync(path, "abandoned snapshot"); + const old = new Date(Date.now() - 48 * 60 * 60 * 1_000); + utimesSync(path, old, old); + return path; + }; + + test("the default run reports the file and leaves it on disk", async () => { + const path = seedStaleTemp(); + await runDoctor([]); + expect(existsSync(path)).toBe(true); + expect(logged.join("\n")).toContain("reclaimable"); + }); + + test("the opt-in flag removes it", async () => { + const path = seedStaleTemp(); + await runDoctor(["--reclaim-response-temps"]); + expect(existsSync(path)).toBe(false); + expect(logged.join("\n")).toContain("Reclaimed 1"); + }); + + test("a mistyped flag warns instead of silently reporting", async () => { + const path = seedStaleTemp(); + await runDoctor(["--reclaim-response-temp"]); + expect(existsSync(path)).toBe(true); + expect(logged.join("\n")).toContain("Unrecognized flag"); + }); }); From 990077e85c7c8f728a3c1033f10ac8ed10e7b0be Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Wed, 19 Aug 2026 11:31:10 +0900 Subject: [PATCH 3/4] docs(devlog): record phase 2 verification evidence --- .../022_phase2_verification.md | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 devlog/_plan/260819_response_state_temp_reclaim/022_phase2_verification.md diff --git a/devlog/_plan/260819_response_state_temp_reclaim/022_phase2_verification.md b/devlog/_plan/260819_response_state_temp_reclaim/022_phase2_verification.md new file mode 100644 index 0000000000..c98200f744 --- /dev/null +++ b/devlog/_plan/260819_response_state_temp_reclaim/022_phase2_verification.md @@ -0,0 +1,36 @@ +# Phase 2 verification + +Full suite on `macmini-cf` at `a2cec13db` (worktree `/tmp/ocx-reclaim`). + +## Full suite — 13397 pass, 1 fail + +`bun run test` → `Ran 13410 tests across 850 files [474.20s]`. + +The single failure is `update-npm-cache-preflight > runs the real worker protocol against +npm's configured cache path`, already proven pre-existing in `012` by running that file at +the unmodified base `59964ad77` (10 pass / 1 fail, identical). It depends on a working +`npm config` on the host. + +The 7 GUI `react` module-load errors seen in the phase-1 run are absent here — that run +had an incomplete `gui/node_modules`, confirming they were environmental as recorded. + +## Focused — 171 pass, 0 fail + +`bun test tests/doctor.test.ts tests/responses-state.test.ts tests/state-store-sweeper.test.ts` +→ 171 pass, 506 assertions, on both the workstation and `macmini-cf`. + +`bun run typecheck` clean; `bun run privacy:scan` passed. + +## What the new end-to-end tests actually pin + +Audit round 3's sharpest finding was that inverting the report/reclaim ternary in +`runDoctor` would have left the entire suite green. The added +`doctor reclaim wiring (end to end)` block seeds a real stale temp in an isolated +`OPENCODEX_HOME` and asserts: + +- `runDoctor([])` leaves the file ON DISK and prints "reclaimable"; +- `runDoctor(["--reclaim-response-temps"])` removes it and prints "Reclaimed 1"; +- `runDoctor(["--reclaim-response-temp"])` (typo) warns and removes nothing. + +The first of those fails if the default is ever inverted, which is the property three +accept criteria claimed and none previously demonstrated. From e298cf8eafffd0da39af76694431d973c08f2cde Mon Sep 17 00:00:00 2001 From: jun Date: Wed, 19 Aug 2026 19:02:01 +0900 Subject: [PATCH 4/4] fix(doctor): report a truncated reclaim from a signal that can actually fire The budget warning keyed on eligible > removed + failed, which is unreachable outside a dry run: an entry is counted eligible and then unlinked or failed on the same iteration, so the two are always equal. An operator whose backlog exceeded the cleanup budget was told the reclaim had finished. Carry an explicit truncated flag on the scan result instead, set wherever the loop stops on a budget rather than on the end of the directory, and OR it across the swept directories. The dry-run report is bounded by the entry cap too, so a truncated report now says the count is a floor. The partial-reclaim test asserted a state production cannot reach; it now uses a reachable one and is paired with an ablation guard that fails if the warning stops depending on the flag. --- src/cli/doctor.ts | 15 ++++++++++++--- tests/doctor.test.ts | 33 ++++++++++++++++++++++++++++++--- tests/responses-state.test.ts | 7 ++++++- 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/src/cli/doctor.ts b/src/cli/doctor.ts index 59ac797ec1..8af24a2693 100644 --- a/src/cli/doctor.ts +++ b/src/cli/doctor.ts @@ -712,17 +712,26 @@ export function formatResponseTempLines( // NOT start, and in that state nothing retries anything. lines.push(` !! ${result.failed} file(s) could not be removed (in use or locked). Retried on the next reclaim — automatically while the proxy runs, otherwise re-run this command.`); } - if (result.eligible > result.removed + result.failed) { - lines.push(` !! Cleanup budget reached; ${result.eligible - result.removed - result.failed} file(s) remain. Run the command again to continue.`); + // `truncated`, not `eligible > removed + failed`: outside a dry run every eligible entry + // is unlinked or failed on the same iteration it is counted, so those two are always + // equal and the comparison never fired. An operator with a backlog past the budget was + // told the reclaim had finished. + if (result.truncated) { + lines.push(" !! Cleanup budget reached; files remain. Run the command again to continue."); } return lines; } if (result.eligible === 0) return [CLEAN_RESPONSE_TEMP_LINE]; - return [ + const lines = [ ` !! ${result.eligible} abandoned response-state temp file(s), ${mb(result.eligibleBytes)} reclaimable.`, " These are interrupted snapshot writes (continuation cache only) and are safe to remove.", " Reclaim them with: ocx doctor --reclaim-response-temps", ]; + // The dry run skips the cleanup budget but is still bounded by the entry cap, so a large + // enough backlog makes this a floor rather than a total. Say so instead of letting an + // operator size the problem from a truncated count. + if (result.truncated) lines.push(" Scan stopped at its entry budget; the real total is higher."); + return lines; } /** Render the doctor "Memory / runtime" section lines (testable without console capture). */ diff --git a/tests/doctor.test.ts b/tests/doctor.test.ts index 45dedeb485..b308f7d0b3 100644 --- a/tests/doctor.test.ts +++ b/tests/doctor.test.ts @@ -628,7 +628,7 @@ describe("service memory section (#314 WP4)", () => { describe("doctor abandoned response-state temps", () => { const result = (over: Partial[0]> = {}) => ({ - matched: 0, removed: 0, failed: 0, bytesRemoved: 0, eligible: 0, eligibleBytes: 0, ...over, + matched: 0, removed: 0, failed: 0, bytesRemoved: 0, eligible: 0, eligibleBytes: 0, truncated: false, ...over, }); test("reports reclaimable files without removing them, and names the opt-in flag", () => { @@ -667,11 +667,38 @@ describe("doctor abandoned response-state temps", () => { }); test("a partial reclaim tells the operator to run again instead of silently stopping", () => { - const lines = formatResponseTempLines(result({ eligible: 816, removed: 512, bytesRemoved: 512 * 24 * 1024 * 1024 }), true); - expect(lines.join("\n")).toContain("304 file(s) remain"); + // The shape here is one the scanner can actually produce. It cannot produce + // eligible > removed + failed outside a dry run: an entry is counted eligible and then + // unlinked or failed on the same iteration, so those are always equal, and the earlier + // version of this warning keyed on a comparison between them and therefore never fired. + const lines = formatResponseTempLines( + result({ eligible: 512, removed: 512, bytesRemoved: 512 * 24 * 1024 * 1024, truncated: true }), + true, + ); + expect(lines.join("\n")).toContain("Cleanup budget reached"); expect(lines.join("\n")).toContain("Run the command again"); }); + test("a reclaim that finished does NOT claim files remain", () => { + // Ablation guard for the test above: same counts, truncated false. If the warning ever + // stops depending on `truncated`, this fails. + const lines = formatResponseTempLines( + result({ eligible: 512, removed: 512, bytesRemoved: 512 * 24 * 1024 * 1024 }), + true, + ).join("\n"); + expect(lines).not.toContain("Cleanup budget reached"); + expect(lines).not.toContain("Run the command again"); + }); + + test("a truncated report says the total is a floor, not the backlog", () => { + const lines = formatResponseTempLines( + result({ matched: 4096, eligible: 4096, eligibleBytes: 96 * 1024 * 1024, truncated: true }), + false, + ).join("\n"); + expect(lines).toContain("4096 abandoned response-state temp file(s)"); + expect(lines).toContain("the real total is higher"); + }); + test("locked files are never described as retried automatically", () => { // This command exists for the operator whose proxy will not start; in that state nothing // retries anything, so promising automatic retry would be a lie to its target reader. diff --git a/tests/responses-state.test.ts b/tests/responses-state.test.ts index a468fa75f4..07d23a57a9 100644 --- a/tests/responses-state.test.ts +++ b/tests/responses-state.test.ts @@ -1623,7 +1623,12 @@ describe("Responses previous_response_id state", () => { }, }); - expect(result).toEqual({ matched: 0, removed: 0, failed: 0, bytesRemoved: 0, eligible: 0, eligibleBytes: 0 }); + expect(result).toEqual({ + matched: 0, removed: 0, failed: 0, bytesRemoved: 0, eligible: 0, eligibleBytes: 0, + // A read failure is not a budget stop: the caller must not be told the backlog was + // merely truncated when enumeration actually broke. + truncated: false, + }); }); test("periodic reclaim frees abandoned temps without any continuation access", () => {