Skip to content

feat(cli): add background cleanup for OpenAI API logs - #8862

Merged
doudouOUC merged 2 commits into
QwenLM:mainfrom
doudouOUC:feat/openai-logs-housekeeping
Aug 10, 2026
Merged

feat(cli): add background cleanup for OpenAI API logs#8862
doudouOUC merged 2 commits into
QwenLM:mainfrom
doudouOUC:feat/openai-logs-housekeeping

Conversation

@doudouOUC

@doudouOUC doudouOUC commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

This PR adds OpenAI API log retention to the existing interactive background-housekeeping pipeline. It removes expired flat-file logs at most once per resolved directory per day, uses the UTC date in writer-owned filenames to avoid a stat call for every file, streams large directories with bounded deletion concurrency, and preserves the log directory itself. Deletion accepts only the exact timestamp-and-ID filename shape emitted by OpenAILogger, so unrelated openai-*.json files in project-local or custom directories are not touched.

It adds model.openAILogRetentionDays with a default of seven days and a minimum-retention value of 0 (approximately one hour). The first-pass scheduler now considers the resolved OpenAI log marker as well as the file-history marker, so a missing or old per-directory marker gets the one-minute catch-up delay. Default per-workspace directories use the merged retention setting; a custom directory uses a user- or system-owned retention policy, and cleanup is skipped when a trusted workspace would otherwise supply an ambiguous directory-wide policy.

Why it's needed

With model.enableOpenAILogging enabled, every OpenAI-compatible API call writes the full request and response to a new JSON file with no rotation or retention. Heavy use was observed to create approximately 342,000 files and 95 GB in two months, including full prompts and responses. A short dedicated retention window reduces disk, inode, and sensitive-data accumulation while preserving recent logs used for debugging and the in-tree latest-session lookup.

Reviewer Test Plan

How to verify

  • In a temporary log directory, place an expired real writer-format log, a fresh real writer-format log, and an expired lookalike such as openai-my-export.json; run housekeeping with seven-day retention and confirm only the expired writer-owned log is removed while the directory remains.
  • With a fresh file-history marker, check an absent OpenAI marker, an OpenAI marker older than seven days, and a fresh OpenAI marker; expect a one-minute initial delay for the first two cases and ten minutes only when both applicable markers are fresh.
  • Point two trusted workspaces with different workspace-scoped retention values at one custom directory; confirm cleanup is skipped and no success marker is written. Then define retention at user or system scope and confirm the directory is cleaned under that single policy.
  • Confirm an unavailable content-generator configuration falls back to the settings-provided log directory, 0 retains files younger than approximately one hour, two different directories receive independent throttle markers, and a non-ENOENT root scan failure does not write a success marker.

Evidence (Before & After)

Before: an expired user-owned openai-my-export.json matched the prefix-only predicate and was deleted (removed=1, existsAfter=false); a fresh file-history marker also forced a ten-minute initial delay even when the OpenAI marker was absent.

After: the same lookalike remains, an expired file generated under the real OpenAILogger contract is removed, and missing or more-than-seven-day-old OpenAI markers select the one-minute catch-up delay. Focused configuration and housekeeping verification passes 251 tests.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

macOS; Node.js 22.22.3; repository version 0.21.8; focused Vitest and ESLint checks. The full build reaches the CLI package and then hits the pre-existing Ink selection-type errors on main in untouched UI files.

Risk & Scope

  • Main risk or tradeoff: The deletion predicate intentionally mirrors the current writer contract rather than sharing the broader reader discovery predicate; a regression test generates a real OpenAILogger filename so writer drift fails the cleaner suite. Custom directories favor safe, machine-owned retention over workspace-specific policies because their flat files do not encode workspace ownership.
  • Not validated / out of scope: Headless (-p) and SDK-only processes do not start interactive housekeeping, so they can still accumulate logs; write-path cleanup remains follow-up work. Windows and Linux were not tested locally.
  • Breaking changes / migration notes: No API breaking changes. Users who combine a custom model.openAILoggingDir with workspace-scoped model.openAILogRetentionDays must move the retention value to user or system scope for cleanup to run.

Linked Issues

Addresses #8860

中文说明

本 PR 的改动

本 PR 在现有的交互式后台 housekeeping 流水线中加入 OpenAI API 日志保留机制。它针对每个解析后的日志目录最多每天清理一次过期的扁平日志文件,利用 writer 自有文件名中的 UTC 日期避免为每个文件执行一次 stat,以有界删除并发流式扫描大目录,并始终保留日志根目录。删除只接受 OpenAILogger 实际生成的“时间戳 + ID”精确文件名格式,因此不会触碰项目目录或自定义目录中无关的 openai-*.json 文件。

本 PR 新增 model.openAILogRetentionDays,默认值为 7 天,0 表示约 1 小时的最短保留期。首次调度现在同时检查解析后的 OpenAI 日志 marker 和 file-history marker,因此缺失或过旧的目录级 marker 会使用 1 分钟追赶延迟。默认的工作区目录使用合并后的保留设置;自定义目录使用用户级或系统级拥有的保留策略,如果可信工作区会提供一个有歧义的目录级策略,则跳过清理。

为什么需要

启用 model.enableOpenAILogging 后,每次 OpenAI 兼容 API 调用都会把完整请求与响应写入新的 JSON 文件,当前没有轮转或保留机制。重度使用下曾观测到两个月约产生 34.2 万个文件、占用 95 GB,其中包含完整 prompt 和响应。独立且较短的保留窗口可以减少磁盘、inode 和敏感数据的累积,同时保留调试以及代码库内“查找当前会话最新日志”所需的近期文件。

评审者测试计划

如何验证

  • 在临时日志目录中放入一个过期的真实 writer 格式日志、一个新鲜的真实 writer 格式日志,以及一个过期的同名前缀文件(例如 openai-my-export.json);以 7 天保留期运行 housekeeping,确认只删除过期的 writer 自有日志,并保留目录本身。
  • 在 file-history marker 新鲜的情况下,分别测试 OpenAI marker 缺失、超过 7 天和新鲜三种状态;前两种应使用 1 分钟首次延迟,只有所有适用 marker 都新鲜时才使用 10 分钟。
  • 让两个具有不同工作区级保留值的可信工作区指向同一个自定义目录;确认清理被跳过且不会写成功 marker。随后在用户级或系统级定义保留值,确认该目录按照这一单一策略清理。
  • 确认 content-generator 配置尚不可用时会回退到设置提供的日志目录,0 会保留约 1 小时内的文件,两个不同目录拥有相互独立的节流 marker,且根目录发生非 ENOENT 扫描错误时不会写成功 marker。

证据(改动前后)

改动前:过期的用户自有 openai-my-export.json 会命中过宽的前缀谓词并被删除(removed=1existsAfter=false);即使 OpenAI marker 缺失,只要 file-history marker 新鲜,首次调度仍会被延迟 10 分钟。

改动后:同一个同名前缀文件会被保留,按照真实 OpenAILogger 契约生成的过期文件会被删除,并且缺失或超过 7 天的 OpenAI marker 会选择 1 分钟追赶延迟。配置与 housekeeping 的聚焦验证共 251 个测试通过。

测试平台

操作系统 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

macOS;Node.js 22.22.3;仓库版本 0.21.8;运行了聚焦 Vitest 与 ESLint 检查。完整构建到达 CLI 包后,在未改动的 UI 文件中遇到了 main 已存在的 Ink selection 类型错误。

风险与范围

  • 主要风险或权衡:删除谓词有意匹配当前 writer 契约,而不复用更宽松的 reader 发现谓词;回归测试会生成真实的 OpenAILogger 文件名,因此 writer 漂移会使清理器测试失败。由于自定义目录中的扁平文件不包含工作区归属,自定义目录优先采用安全的机器级保留策略,而不是工作区特有策略。
  • 未验证 / 范围外:Headless(-p)和仅 SDK 的进程不会启动交互式 housekeeping,因此仍可能累积日志;在写入路径上清理属于后续工作。Windows 和 Linux 未在本地测试。
  • 破坏性变更 / 迁移说明:没有 API 破坏性变更。同时使用自定义 model.openAILoggingDir 和工作区级 model.openAILogRetentionDays 的用户,需要把保留值移动到用户级或系统级,清理才会运行。

关联问题

Addresses #8860

With model.enableOpenAILogging on, every API call appends a full
request/response JSON under logs/openai with no rotation — heavy usage
accumulates hundreds of thousands of files (tens of GB) within months.

Register a third cleaner in the existing background housekeeping
pipeline that sweeps openai-*.json files older than the new
model.openAILogRetentionDays setting (default 7 days). The
filename-embedded UTC date is used as a fast path to avoid one stat()
per file; the boundary day and unparseable names fall back to mtime.
Throttling is keyed on the resolved log dir, so both the default
per-CWD layout and a shared custom openAILoggingDir are swept at most
once a day. The sweep runs regardless of whether logging is currently
enabled, so residue from earlier debugging sessions still gets cleaned.

Scope note: housekeeping only starts for interactive sessions, so
headless (-p) / SDK processes are not covered yet.
@github-actions github-actions Bot added the review/self-reported The linked issue was opened by the PR author (self-reported) label Aug 10, 2026
@doudouOUC
doudouOUC requested a review from wenshao August 10, 2026 09:28
@doudouOUC
doudouOUC enabled auto-merge August 10, 2026 09:28
@doudouOUC
doudouOUC requested a review from yiliang114 August 10, 2026 09:29
@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the PR, @doudouOUC — the underlying problem is real and your description is detailed, but the PR body doesn't follow the PR template, so I have to pause it here. This is a formatting gate, not a code concern.

None of the template's required sections are present (the body currently uses a custom ## Summary / ## Changes / ## Test plan / ## Scope note structure):

  • What this PR does — prose description of the change; note the template asks for prose, not a per-file rundown
  • Why it's needed — motivation and user-facing benefit
  • Reviewer Test Plan, with its three subsections:
    • How to verify — the behaviors a reviewer should confirm and what to expect, not just the test commands you ran
    • Evidence (Before & After) — e.g. a directory listing of logs/openai before/after a sweep, since this change is about files on disk
    • Tested on — the OS matrix (🍏/🪟/🐧 with ✅/⚠️/N/A); right now it's unclear which OS your vitest/eslint/typecheck runs happened on
  • Risk & Scope — the three bullets: main risk or tradeoff / not validated / breaking changes; your headless-session limitation note belongs under "Not validated / out of scope"
  • Linked IssuesCloses #8860

Could you restructure the body to follow the template? The content you already wrote is largely good — most of it can be moved into the right sections as-is. Please keep each paragraph or list item as one long line (the template notes that GitHub renders single newlines as <br>, so hard-wrapped text displays as a narrow column).

Once the body is updated, a maintainer can re-run triage with @qwen-code /triage to continue.

中文说明

感谢提交 PR,@doudouOUC —— 问题是真实存在的,描述也很详细,但 PR 正文没有遵循 PR 模板,所以需要先停在这里。这是一次格式上的拦截,而不是对代码的质疑。

模板要求的章节全部缺失(目前正文使用了自定义的 ## Summary / ## Changes / ## Test plan / ## Scope note 结构):

  • What this PR does —— 用散文描述改动;注意模板要求散文描述,而不是按文件罗列
  • Why it's needed —— 动机与用户收益
  • Reviewer Test Plan,包含三个子章节:
    • How to verify —— 评审者应确认的行为和预期结果,而不只是你跑过的测试命令
    • Evidence (Before & After) —— 例如清扫前后 logs/openai 的目录列表,因为这个改动针对的就是磁盘上的文件
    • Tested on —— 操作系统矩阵(🍏/🪟/🐧 加 ✅/⚠️/N/A);目前无法判断你的 vitest/eslint/typecheck 是在哪个操作系统上运行的
  • Risk & Scope —— 三个要点:主要风险或权衡 / 未验证项 / 破坏性变更;关于 headless 会话的局限应放在 "Not validated / out of scope" 下
  • Linked Issues —— Closes #8860

能否按模板重构正文?你已经写好的内容大部分是好的 —— 多数可以直接挪到对应章节。请保持每个段落或列表项为一长行(模板注明 GitHub 会把单个换行渲染成 <br>,硬换行的文字会显示成窄列)。

正文更新后,维护者可以用 @qwen-code /triage 重新触发 triage 继续流程。

Qwen Code · qwen3.8-max

@doudouOUC doudouOUC self-assigned this Aug 10, 2026
// Matches the filenames OpenAILogger writes:
// `openai-<ISO timestamp>[_<diagnostic suffix>].json` — same predicate the
// reader side uses (see packages/cli/src/utils/sessionPaths.ts).
const OPENAI_LOG_FILE_PATTERN = /^openai-.*\.json$/;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This predicate accepts every openai-*.json file, while the sweep runs against the project-local default directory even when OpenAI logging is disabled. A user-owned old file such as logs/openai/openai-project-data.json is therefore deleted. Please match the logger’s exact timestamp/UUID filename contract, preferably through a shared ownership predicate or directory marker.

Comment thread docs/users/configuration/settings.md Outdated
| `model.skipStartupContext` | boolean | Skips sending the startup workspace context (environment summary and acknowledgement) at the beginning of each session. Enable this if you prefer to provide context manually or want to save tokens on startup. | `false` |
| `model.enableOpenAILogging` | boolean | Enables logging of OpenAI API calls for debugging and analysis. When enabled, API requests and responses are logged to JSON files. | `false` |
| `model.openAILoggingDir` | string | Custom directory path for OpenAI API logs. If not specified, defaults to `logs/openai` in the current working directory. Supports absolute paths, relative paths (resolved from current working directory), and `~` expansion (home directory). | `undefined` |
| `model.openAILogRetentionDays` | number | Days to retain OpenAI API log files written when `model.enableOpenAILogging` is on. Log files older than this are removed by a background housekeeping pass that runs at most once per day. `0` = minimum retention (~1 hour). Changes take effect after restart. | `7` |

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The documented retention setting only runs through the interactive-only housekeeping startup path. Headless CLI and SDK processes can therefore continue producing OpenAI logs indefinitely. Please either cover those writers, or document the limitation here and keep the broader issue open until it is tracked separately.

qqqys
qqqys previously requested changes Aug 10, 2026

@qqqys qqqys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Requesting changes for the unresolved Critical data-deletion finding already documented in the current-head inline comment on packages/cli/src/utils/housekeeping/cleanup.ts:135.

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The mechanics are right — boundary-day mtime disambiguation (strictly-older-than, both UTC), the 0/negative/NaN clamp to ~1h, top-level-only enumeration with isFile() (dirs/symlinks skipped), root dir never removed, per-dir sha256 marker so dir changes re-run, graceful skip on resolve failure, and no overlap with the session-layout cleaners — all verified correct and well pinned. But one P0 blocks:

The ownership predicate /^openai-.*.json$/ (cleanup.ts:135) is broader than the logger's actual filename contract (openai--<8-hex-uuid>[-].json per core/src/utils/openaiLogger.ts). Consequences, all verified: a date-less user file like openai-not-a-date.json in the log dir falls to the mtime fallback and is unlinked once >7 days old (the test actually pins this unsafe behavior as intended); a file like openai-2025-01-01-eval-data.json is deleted on filename string alone with no mtime check. Blast radius is amplified because the default dir /logs/openai lives inside the user's project checkout, the sweep runs even with logging disabled, and a custom openAILoggingDir may point at any user-chosen directory. The 'same predicate as the reader' justification doesn't transfer: sessionPaths.ts only reads. Fix: match the writer contract exactly (export the pattern from core alongside the existing UUID_PATTERN) and make unparseable names SKIP rather than mtime-delete. This matches the Critical qqqys already filed; the PR is one predicate away from approve.

Two smaller items: docs/settings.md:184 — retention only runs in interactive sessions (startup-prefetch.ts gates startBackgroundHousekeeping behind config.isInteractive()), so headless/SDK writers grow logs forever; document or track. And cleanup.ts:185 counts ENOENT-on-unlink as an error; treat it as benign success. Test fixtures also use a _suffix separator the writer never produces (writer uses -), and the load-bearing ownership test is currently pinned in reverse (look-alike user names deletable).

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed — no blockers. Suggestions are inline. Test Plan (not a blocker): src/config/settingsSchema.test.tsno such file or directory; src/config/settings.test.tsno such file or directory; 46 passed — this review observed 18781, 481 passed; 201 passed — this review observed 18781, 481 passed.

— qwen3.8-max via Qwen Code /review (v0.21.8)

Comment thread packages/cli/src/utils/housekeeping/cleanup.ts Outdated
Comment thread packages/cli/src/utils/housekeeping/cleanup.ts Outdated
Comment thread packages/cli/src/utils/housekeeping/cleanup.ts Outdated
Comment thread packages/cli/src/utils/housekeeping/cleanup.ts Outdated
Comment thread packages/cli/src/utils/housekeeping/cleanup.ts
Comment thread packages/cli/src/utils/housekeeping/cleanup.ts
Comment thread packages/cli/src/utils/housekeeping/scheduler.ts
Comment thread packages/cli/src/utils/housekeeping/scheduler.ts Outdated
Comment thread packages/cli/src/utils/housekeeping/scheduler.ts Outdated
Comment thread packages/cli/src/utils/housekeeping/scheduler.test.ts

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ This run could not certify that any of this diff was reviewed. Suggestions are inline. Not reviewed: build-and-test — the packages/cli full test suite exceeded the 300-second local harness deadline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally. Not reviewed: coverage — no plan was given, so this run cannot show that any of the diff was read. [Critical] R1-2 First-pass scheduling ignores the OpenAI log marker: the startup delay checks only the global file-history marker. If that marker is fresh while the current directory's OpenAI marker is absent or stale, repeated interactive sessions shorter than ten minutes exit before cleanup and the log backlog is never swept.

— Qwen Code via Qwen Code /review (v0.21.8)

Comment thread packages/cli/src/utils/housekeeping/scheduler.ts
Comment thread packages/cli/src/utils/housekeeping/cleanup.ts Outdated
Comment thread packages/cli/src/utils/housekeeping/scheduler.ts Outdated
Comment thread packages/cli/src/utils/housekeeping/scheduler.test.ts
Comment thread packages/cli/src/utils/housekeeping/scheduler.ts Outdated
Comment thread packages/cli/src/utils/housekeeping/cleanup.ts
Comment thread packages/cli/src/utils/housekeeping/scheduler.test.ts
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@github-actions github-actions Bot removed the review/self-reported The linked issue was opened by the PR author (self-reported) label Aug 10, 2026
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Addressed the current review round in b6e45cd568.

Review area Action
Destructive filename ownership Fixed: deletion now requires the exact OpenAILogger timestamp/8-hex-ID contract; prefix-only lookalikes are preserved and a real-writer regression test guards drift.
First-pass scheduling Fixed: the resolved OpenAI marker participates in catch-up selection; missing or >7-day markers use 1 minute, all-fresh markers use 10 minutes.
Shared custom-directory retention Fixed: custom directories require a user/system owner; an effective trusted workspace policy skips cleanup without a success marker. System-owner behavior is order-independent in verification.
Large first sweep Fixed: directory entries stream through opendir with one bounded batch instead of materializing the full listing.
Scan and race failures Fixed: non-ENOENT root scan errors reject without a marker; disappearing files are benign; other per-file failures remain counted and isolated.
Defaults, config fallback, and edge values Fixed: the schema owns the default constant, unavailable content-generator config falls back to settings, 0 has scheduler coverage, and oversized retention cannot create an invalid Date.
Marker and fallback coverage Fixed: behavioral two-directory isolation, settings-directory fallback, custom policy ownership, and stale-marker cases are covered.
Shared reader/deleter predicate Not taken: reader discovery may remain permissive, while destructive ownership must be strict; a real-writer test guards naming drift without widening the change into core.
Generic batching/hash helpers Not taken: the streaming file sweep and session-directory sweep now have materially different lifecycles, and two small marker builders do not justify a speculative abstraction.
Synthetic per-file failure injection Not taken: portable injection would add a production test seam solely for this suggestion; the direct catch remains, while root failure and ENOENT semantics are covered.
Headless/SDK cleanup Deferred: the limitation is now explicit in settings docs and the PR template; #8860 remains referenced without an auto-close keyword so write-path cleanup can continue separately.

Verification: 251 focused tests passed, affected-file ESLint passed, generated settings schema is current, and an independent smoke pass confirmed lookalike preservation, real-log removal, catch-up timing, and shared-directory policy order. The full build still stops only on the known main Ink selection-type errors in untouched UI files.

Replied to every eligible unresolved thread; the two threads already authored by the PR author were skipped by the dedup guard.

@qwen-code /triage

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 100 passed · 0 failed · 100 total

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:100 通过 · 0 失败 · 100 总计

Verification report

PR #8862 — feat(cli): add background cleanup for OpenAI API logs

Verdict: merge-ready — 100/100 scripted assertions passed, 0 unexpected failures. Verified head: b6e45cd568f29a9749b9e2765b5916b389610bd4 (== snapshot headRefOid, no drift). Base: 358091833bfa2ebf0e05e55e9ee19a4cb6018c82 (HEAD^1 of the CI merge-ref checkout).

中文摘要
  • 结论merge-ready。100 条脚本化断言全部通过,0 个意外失败;未发现阻塞性问题。
  • A/B 结论:同一份 fixture 日志目录(2 个过期 writer 格式日志、1 个新 writer 日志、openai-my-export.json 等 5 个无关/相似条目、目录、符号链接)分别跑 head 与 base 的 _runHousekeepingForTesting:head 恰好删除 2 个过期的 writer 自有日志并保留其余全部、写入按目录的 marker、同日第二次 pass 被节流(13/13);base 什么都不删、无 marker、且不存在 cleanupOldOpenAILogs 导出(7/7)。中心声明被证明为 load-bearing。
  • 矩阵:文件名谓词 13 个相邻形状 + 文件名/mtime 权威性 + 255B 最坏名 + 正则 ladder(2k/20k/200k,均 <1ms)27/27;保留期所有权策略 16/16(workspace 级 + 自定义目录 → 跳过且不写 marker;system/user 级 → 按机器级策略清理,双向验证 system 优先;0 ≈ 1 小时;MAX_VALUE 钳制);节流/失败路径 12/12(EACCES 不写成功 marker 且可恢复、外部锁静默跳过);首通延迟矩阵 7/7;批边界 55+5 文件 4/4。
  • 测试有效性:8 个单点变异全部被 PR 新增测试以行为性断言杀死(无幸存者),未变异对照 50/50 绿。
  • :housekeeping + settingsSchema 套件全绿;tsc --noEmit 0 错误;eslint 干净且活性已证。settings.test.ts 的 12 个失败在 base 上逐名相同(环境性,非本 PR 引入)。
  • 未覆盖:逐 commit 归因(depth-2,2 个 commit 中 1 个本地不可达);真实 10 分钟/24 小时定时器链;headless/SDK 累积(设计内 out-of-scope);Windows;与当前 main 的 trial merge(无网络)。

Central claim + A/B

Central claim: interactive housekeeping removes expired writer-owned OpenAI logs at most once per resolved dir per day, while preserving lookalikes, fresh files, directories, symlinks, and the log root.

Fixture (identical in both cells): 2 expired writer-format logs (10d, 12d+suffix), 1 fresh writer log, expired openai-my-export.json, expired openai-<date>.json (missing 8-hex id), notes.txt, a directory with a writer-format name, and a symlink with a writer-format name pointing outside the log dir. Oracle: before/after directory listings + marker files in a fresh QWEN_HOME. Witness: evidence/01-ab-cells-base-vs-head.png.

Cell Build Removed Kept Marker Assertions
head CI tsc dist at b6e45cd exactly the 2 expired writer logs (removed=2) all 6 others + dir + symlink + root .openai-logs-cleanup-<hash> written; same-day 2nd pass throttled (planted expired file survives) 13/13
base esbuild-transpiled HEAD^1 sources nothing (removed=0) all 8 entries none; cleanupOldOpenAILogs export absent 7/7

The base control differs only by the PR's code: package.json/lockfile untouched; packages/core byte-identical on both arms (verified git diff HEAD^1..HEAD -- packages/core empty), so the base worktree's upward resolution of @qwen-code/qwen-code-core into the head tree is a no-op confound.

Reviewer Test Plan, step by step

Plan step Result Backing cells
1. expired writer log + fresh writer log + expired lookalike, 7d retention → only expired writer removed, dir remains 01-ab head H1/H3/H4/H9
2. fresh file-history marker + {absent, >7d-old, fresh} OpenAI marker → 60s, 60s, 600s 05-delay D1/D3/D2 (plus D4/D5)
3. two trusted workspaces, different workspace retention, one custom dir → skipped, no marker; user/system scope → cleaned under that policy 03-policy P1 (skip, no marker), P2/P3 (system owner, both directions), P4 (user scope)
4. cgc-unavailable fallback to settings dir; 0 ≈ 1h; independent per-dir markers; non-ENOENT scan failure writes no success marker 03-policy P10, P6; 04-throttle T1, T2

Findings

No blocking findings. Non-blocking observations, for completeness:

  1. Batch retention is verified behaviorally but not pinned by a test (informational). The design doc says the streaming loop was "inspected directly"; no unit test asserts the 20-concurrency batching. My 06-batch probe (55 expired + 5 fresh, crossing two batch boundaries) passes 4/4, so the behavior is right today — a future refactor of the batch.length === SWEEP_CONCURRENCY flush would go untested. Coverage gap, not a defect.
  2. Syntactically valid but semantically invalid filename dates (informational). openai-2026-13-01T00-00-00.000Z-a1b2c3d4.json matches the regex and is compared lexicographically; a month-13 date sorting older than the cutoff would be deleted. The writer can never emit such a name, and a hand-placed full-shape file is by the PR's stated design "writer-owned", so this is inherent to the contract, not a hole in it.
  3. Skip is logged at error level (nit). workspace-scoped openAILogRetentionDays is unsafe… is a deliberate user configuration, not a fault; debugLogger is debug-gated so there is no production noise, but the level reads as failure.

Not covered

  • Per-commit attribution: the snapshot lists 2 commits (82dd2e17, b6e45cd5); at depth 2 only b6e45cd5 (HEAD^2) is reachable, so the aggregate HEAD^1..HEAD diff was verified and per-commit claims were not separated.
  • Real timer chain: the 10-min/24-h setTimeout chain is pre-existing and untouched; delay selection and pass logic were driven through the exported seams, not by waiting out real timers.
  • Headless/SDK accumulation: out of scope by the PR's own design (documented limitation).
  • Windows: not tested here; Linux behavior (EACCES, symlinks, chmod, NAME_MAX) was exercised and passes, upgrading the author's ⚠️ Linux cell for the tested paths.
  • Trial merge into current main: no network in this environment; base 0d56e50/3580918 vs current main not re-measured.
  • settings.test.ts 12 failures: environmental — byte-identical failing-test set on base and head (compared by name; files untouched by the PR). Root cause not diagnosed (container HOME/.env quirks); attributed as pre-existing by the A/A comparison.
  • Base control build: esbuild-transpiled (no typecheck) because a from-scratch tsc --build in a bare worktree fails on pre-existing third-party type resolution once the per-package nested node_modules (e.g. packages/core/node_modules/ajv@8.20.0) are absent — the same trap that made my first mutant-tree vitest run fail to collect. The head arm uses the CI typechecked tsc dist.
  • Mutation-matrix capture (02-mutation-matrix.png) is a replay of the recorded run logs; the live runs are in logs/mutation-*.log.

Methodology

Environment: node:22-bookworm CI container, uid 1000, merge-ref checkout at depth 2. Harnesses (01-ab.mjs06-batch.mjs, lib.mjs) import the compiled packages/cli/dist (head) and an esbuild transpile of HEAD^1's four housekeeping modules (base) directly, with fresh QWEN_HOME and temp dirs per cell; fake Config/LoadedSettings objects match the shapes the PR's own tests use — nothing in the unit under test is stubbed. Mutation matrix ran in a scratch worktree at HEAD (tmp/pr8862-mutant, nested node_modules symlinked) with vitest run over the two affected test files, one mutant per run, restored via git checkout after each. Gates: focused vitest (housekeeping + settingsSchema + settings), tsc --noEmit, and eslint on the five changed files, with a planted-violation liveness check for eslint and a base-arm run for failure attribution. Raw logs: logs/; captures: evidence/01-ab-cells-base-vs-head.png, 02-mutation-matrix.png, 03-policy-matrix.png, 04-gates-vitest-typecheck.png.

Evidence images

01-ab-cells-base-vs-head

02-mutation-matrix

03-policy-matrix

04-gates-vitest-typecheck

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-review at head b6e45cd — the P0 is fixed and verified adversarially (13/13 name walk plus a real OpenAILogger write→delete regression test): the predicate now matches the writer contract exactly (ISO-ts with .sssZ, 8-hex uuid, optional sanitized suffix with leading/trailing-dash rule), everything non-matching is skipped, never mtime-deleted — openai-not-a-date.json and openai-2025-01-01-eval-data.json survive, real logger names (with and without suffix) are deletable when old, uppercase-UUID/9-hex/.json.bak variants rejected, boundary day still strictly-older-than via mtime. The sweep also hardened nicely: streaming opendir with bounded batches, ENOENT benign, non-ENOENT root-scan failure throws with the marker only written on success so the timer chain survives. All P2/P3 items addressed: interactive-only limitation documented in settings.md, both schema descriptions, and the new design doc; fixture separator matches the writer; ownership tests pin both directions (workspace-only retention on a custom dir skips with no marker). Non-blocking notes: the contract regex lives in cli with the single home being a cross-package regression test rather than an exported constant (deliberate, acceptable); custom-dir + workspace-scoped retention silently disables cleanup (fail-safe, documented); the EACCES test would not hold under root (runners are non-root). CI green on all runnable jobs. Ship it.

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — the earlier stage-1a template gate is resolved by the rewritten body.

Problem: observed, not theoretical. The linked issue #8860 documents ~342,000 files / ~95 GB accumulated in ~2 months of heavy use (~2–3 GB/day, disk reaching 97%), and the code confirms there is no rotation or retention anywhere on the OpenAI logging path.

Direction: aligned. The change extends the existing background housekeeping pipeline — whose comment already reserved a slot for a debug-log cleaner — instead of adding new machinery. Supporting signal from claude-code's CHANGELOG: retention sweeps being extended to more file classes ("The cleanupPeriodDays retention sweep now also covers ~/.claude/tasks/, ~/.claude/shell-snapshots/, and ~/.claude/backups/").

Size: core paths touched — packages/cli/src/config/settingsSchema.ts matches packages/*/src/config/**. Production logic ≈ 208 lines (settingsSchema 15, cleanup.ts 76, scheduler.ts 117), tests 470, docs 39, generated schema 6 — well under the 500-line threshold; no size escalation.

Approach: scope feels right. Reusing runThrottledOnce with a per-resolved-dir marker, deriving the date from the writer-owned filename instead of stat-ing every file, and preserving the log root are all the calls I'd have made. The custom-directory ownership policy (user/system-scoped retention only, skip on workspace ambiguity) is real complexity, but it was pulled in by a Critical review finding about shared directories, not speculative hardening. The four declined suggestions each carry a reasoned response.

Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ —— 先前的 stage-1a 模板关卡已被重写后的 PR 正文解决。

问题:已观测到的真实问题,不是理论加固。关联的 issue #8860 记录了重度使用约两个月累积 ~34.2 万个文件 / ~95 GB(约 2–3 GB/天,磁盘达 97%),代码确认 OpenAI 日志路径上没有任何轮转或保留机制。

方向:对齐。改动扩展现有后台 housekeeping 流水线(其注释本就为 debug-log 清理器预留了位置),而不是新增机制。参考信号:claude-code 的 CHANGELOG 中有同样的模式——保留期清理扩展到更多文件类别("The cleanupPeriodDays retention sweep now also covers ~/.claude/tasks/, ~/.claude/shell-snapshots/, and ~/.claude/backups/")。

规模:触及核心路径——packages/cli/src/config/settingsSchema.ts 匹配 packages/*/src/config/**。生产逻辑约 208 行(settingsSchema 15、cleanup.ts 76、scheduler.ts 117),测试 470 行,文档 39 行,生成 schema 6 行——远低于 500 行阈值,无需规模升级。

方案:范围合理。复用 runThrottledOnce 并按解析后的目录做 marker、从 writer 自有文件名取日期避免逐文件 stat、保留日志根目录——这些都是我会做的选择。自定义目录的归属策略(仅用户/系统级保留期,工作区级有歧义时跳过)是实打实的复杂度,但它来自针对共享目录的 Critical 评审发现,不是猜测性加固。四条被拒绝的建议都附有理由说明。

风险:无升级风险信号——改动文件均未命中与 revert 相关的高风险路径。

进入代码审查。🔍

Qwen Code · qwen3.8-max

Reviewed at b6e45cd568f29a9749b9e2765b5916b389610bd4 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code review

No critical blockers. I re-verified the current head independently rather than relying on the prior review thread:

  • Deletion predicate vs writer contract — the regex in cleanup.ts matches OpenAILogger.logInteraction's actual output exactly: ISO timestamp with :-, uuidv4().slice(0, 8) lowercase-hex ID, and an optional suffix whose charset and leading/trailing-dash strip mirror sanitizeDiagnosticSuffix. The regression test writes a real OpenAILogger file and deletes it, so writer drift fails the suite. Keeping the destructive predicate stricter than the permissive reader-side discovery (startsWith('openai-') && endsWith('.json')) is the right call — sharing the reader predicate is exactly what caused the round-1 P0. Symlinks and directories are excluded via entry.isFile() on Dirents, and unlink never follows symlinks.
  • Failure semanticsrunThrottledOnce writes the marker only after successful task completion (taskCompleted flag), and cleanupOldOpenAILogs rethrows non-ENOENT opendir failures, so a scan failure retries next cycle instead of suppressing cleanup for 24 h. A vanished file mid-sweep is benign (ENOENT swallowed); other per-file errors are counted without stopping later batches.
  • Shared getCutoffDate clamp — the new MIN_DATE_MS floor only changes behavior for astronomically large hand-edited values (where the old code produced an Invalid Date and swept nothing anyway); sane inputs yield identical cutoffs, so the existing file-history and subagent cleaners are unaffected.
  • Config plumbing — the LoadedSettings fields used (system/user/workspace/systemDefaults, isTrusted) match the real class shape; Config.getWorkingDir() / getContentGeneratorConfig() exist; the startup-prefetch.ts call site is unchanged. Custom dirs require user/system-owned retention and skip — without writing a marker — when a trusted workspace would supply an ambiguous policy; the ownership chain reads correctly and both workspace orders are tested.
  • Schema additions follow the cleanupPeriodDays precedent exactly (requiresRestart: true, minimum: 0, same rationale comment), and the vscode companion schema matches.

Two non-blocking gaps remain: the per-file error branch still has no direct test (the author declined adding a production injection seam solely for testing; the root-scan failure path is covered), and unlink behavior on locked files under Windows degrades to "count the error, retry next cycle" but wasn't exercised in CI (matrix skipped, below).

Test evidence — the PR's own CI at b6e45cd via API (no PR code executed locally)

Check Conclusion
Test (ubuntu-latest, Node 22.x) ✅ success
Test (windows-latest, Node 22.x) ⏭️ skipped
Test (macos-latest, Node 22.x) ⏭️ skipped
Integration Tests (CLI, No Sandbox) ⏭️ skipped
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success

The Qwen Code CI workflow run (pull_request event) is completed: everything that ran is green, including the full unit suite with the 470 new test lines. The Windows/macOS unit matrix and integration tests were skipped for this fork PR, so cross-platform behavior rests on inspection plus the author's macOS run — flagging it rather than hiding it.

Sandboxed verification for the behavioral claims is already in flight on this triage trigger — its report will post in this thread. The claims it should pin: only exact-writer-format files older than retention are deleted while prefix lookalikes survive, and workspace-scoped retention on a custom directory skips cleanup without writing a marker. Not verified here: Windows/Linux runtime behavior (author tested on macOS only; CI matrix skipped).

中文说明

代码审查

无关键阻塞。本轮我独立复核了当前 head,而不是只依赖此前的评审线程:

  • 删除谓词 vs writer 契约 —— cleanup.ts 中的正则与 OpenAILogger.logInteraction 的实际输出精确匹配:冒号替换为 - 的 ISO 时间戳、uuidv4().slice(0, 8) 小写十六进制 ID、可选后缀的字符集与首尾去 - 规则同 sanitizeDiagnosticSuffix 一致。回归测试通过真实 OpenAILogger 写入并删除文件,writer 契约漂移会让测试失败。删除侧谓词比读取侧发现谓词(startsWith('openai-') && endsWith('.json'))更严格是正确的——共用读取侧谓词正是第一轮 P0 的成因。符号链接与目录通过 Dirent 的 entry.isFile() 排除,且 unlink 不会跟随符号链接。
  • 失败语义 —— runThrottledOnce 仅在任务成功完成后写 marker(taskCompleted 标志),且 cleanupOldOpenAILogs 对非 ENOENT 的 opendir 失败向上抛出,因此扫描失败会在下个周期重试,而不是压制清理 24 小时。清理过程中文件消失视为良性(吞掉 ENOENT),其他单文件错误计数且不阻断后续批次。
  • 共享的 getCutoffDate 钳制 —— 新增的 MIN_DATE_MS 下限只改变手工编辑超大值的场景(旧代码产生 Invalid Date、本来也什么都不删);正常输入产生完全相同的 cutoff,现有 file-history 与 subagent 清理器不受影响。
  • 配置接线 —— 用到的 LoadedSettings 字段(system/user/workspace/systemDefaultsisTrusted)与真实类结构一致;Config.getWorkingDir() / getContentGeneratorConfig() 存在;startup-prefetch.ts 调用点未变。自定义目录要求用户/系统级拥有的保留期,可信工作区提供歧义策略时跳过且不写 marker;归属链读取正确,两种工作区顺序均有测试。
  • Schema 新增完全沿用 cleanupPeriodDays 先例(requiresRestart: trueminimum: 0、相同理由注释),vscode 伴侣 schema 一致。

剩余两处非阻塞缺口:单文件错误分支仍无直接测试(作者拒绝仅为测试引入生产注入缝;根扫描失败路径已覆盖);Windows 下对被占用文件 unlink 的行为退化为"计数错误、下周期重试",但 CI 矩阵被跳过,未实际运行(见下)。

测试证据 —— b6e45cd 上 PR 自己的 CI,通过 API 获取(未本地执行 PR 代码)

CI 表格见上。Qwen Code CI(pull_request 事件)已完成:实际运行的任务全绿,包含新增 470 行测试在内的完整单元测试通过。该 fork PR 的 Windows/macOS 单元矩阵与集成测试被跳过,跨平台行为依赖代码审查与作者的 macOS 运行——如实标注而非掩盖。行为类声明的沙箱验证已随本次 triage 触发在运行中,报告会发在本线程;它应钉住:只删除精确 writer 格式的过期文件、同名前缀文件幸存;自定义目录上的工作区级保留期跳过清理且不写 marker。此处未验证:Windows/Linux 运行时行为(作者仅在 macOS 测试,CI 矩阵被跳过)。

Qwen Code · qwen3.8-max

Reviewed at b6e45cd568f29a9749b9e2765b5916b389610bd4 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean review across every stage; the only reservations are environmental (the fork PR's CI skips the Windows/macOS unit matrix) and one untested per-file error branch, neither blocking.

Stepping back: this is what a good contribution looks like. The problem is observed and quantified (95 GB / 342k files, #8860), the fix rides the existing housekeeping pipeline instead of inventing parallel machinery, the deletion predicate is pinned to the writer contract by a real-OpenAILogger regression test, and every round-1 finding — including the P0 where a broad openai-*.json prefix match would have deleted user files — has a verified fix in b6e45cd. My independent re-check of the head found nothing new: marker-on-success semantics, cutoff clamping, the settings-ownership chain, and the catch-up scheduling all hold up against the base code. It also matches my independent proposal for this problem almost exactly; I found no simpler path it missed.

@yiliang114's adversarial re-review at this head reached the same conclusion and stands as an approval. The outstanding CHANGES_REQUESTED votes — @qqqys's Critical on the old predicate and my own round-1 template gate — are addressed by the commits on this head; a re-dismiss from the respective reviewers would clear the board. Approving, pinned to the reviewed commit.

中文说明

置信度:4/5 —— 各阶段审查均干净;仅有的保留是环境性的(fork PR 的 CI 跳过 Windows/macOS 单元矩阵)和一个未覆盖的单文件错误分支,均不阻塞。

整体来看:这是一次高质量的贡献。问题来自真实观测且有量化数据(95 GB / 34.2 万文件,#8860),修复复用了现有 housekeeping 流水线而非另起炉灶,删除谓词通过真实 OpenAILogger 回归测试钉死在 writer 契约上,第一轮的所有发现——包括宽泛 openai-*.json 前缀匹配会误删用户文件的 P0——都在 b6e45cd 中有已验证的修复。我对当前 head 的独立复核没有发现新问题:成功才写 marker 的语义、cutoff 钳制、设置归属链、追赶调度都与基础代码吻合。它也几乎完全符合我对该问题的独立方案,我没有找到它遗漏的更简路径。

@yiliang114 在同一 head 上的对抗性复审得出相同结论并已批准。尚未撤销的 CHANGES_REQUESTED —— @qqqys 针对旧谓词的 Critical 与我第一轮的模板关卡——都已被当前 head 的提交解决;请相关评审者确认后撤销,即可清场。批准,并钉在所审提交上。

Qwen Code · qwen3.8-max

Reviewed at b6e45cd568f29a9749b9e2765b5916b389610bd4 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship. ✅

@doudouOUC
doudouOUC dismissed qqqys’s stale review August 10, 2026 12:20

already have 2 approved, 3ks.

@doudouOUC
doudouOUC added this pull request to the merge queue Aug 10, 2026
Merged via the queue into QwenLM:main with commit a292c89 Aug 10, 2026
455 of 471 checks passed
@wenshao

wenshao commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Local verification report (real build, real session, real API traffic)

I built this PR and its merge-base into two separate bundles and exercised the cleaner the way a user actually hits it: a real interactive qwen TUI (tmux, isolated QWEN_HOME) talking to a real OpenAI-compatible HTTP server, with model.enableOpenAILogging on, so every fixture log file below was produced by the shipped OpenAILogger — no hand-written filenames, no mocked fs.

Head b6e45cd (feat/openai-logs-housekeeping)
Base 9aec40f (merge-base with main)
Env macOS 26.6 (arm64), Node v24.18.1, repo 0.21.8
Harness two npm ci + npm run bundle trees · mock OpenAI SSE server · tmux TUI · QWEN_DEBUG_LOG_FILE=1 for the [HOUSEKEEPING] trace

Verdict: works as described. I did not find a correctness bug. Three non-blocking notes at the bottom.


1. End-to-end sweep, head vs. base

Real headless turn writes real logs → genuinely old logs minted through the real OpenAILogger at a shifted clock → decoys added → interactive session started and left idle so the catch-up pass fires.

sweep

  • head: exactly the 5 expired writer-owned logs removed (openai-logs: removed=5 errors=0), including the -subagent-* and -side-query-* suffixed shapes.
  • preserved: openai-my-export.json, …-ZZZZZZZZ.json (non-hex id), …deadbeef.json.bak, openai_2026-01-01.json, README.md, a directory named like a log file, and both fresh logs. The log root itself survives.
  • base: same fixture, removed list empty, no .openai-logs-cleanup-* marker — the cleaner genuinely does not exist before this PR.

2. Writer ↔ deleter contract

The real risk with a stricter-than-reader predicate is the opposite of over-deletion: a writer shape the predicate misses leaks forever. I drove the shipped OpenAILogger over 29 promptId shapes (internal ids, side-query: ids, subagent a#b#c ids, unicode, leading/trailing dashes, 120-char ids) × 5 wall clocks and tested every emitted filename against the PR's predicate:

145 files written, 145 matched, 0 unmatched.

3. First-pass delay matrix

.file-history-cleanup is fresh in every row — only the per-directory OpenAI marker varies. This is the regression the PR fixes.

scheduler

build OpenAI marker observed
head missing first pass in 60s
head 8 days old first pass in 60s
head fresh first pass in 600s
base missing first pass in 600s ← backlog ignored

Same shot also covers the shared-custom-directory policy: with retention at workspace scope the sweep is skipped, nothing is deleted and no success marker is written; moving the same value to user scope cleans the directory (removed=2 errors=0).

4. Scale — the case from #8860

65,000 flat files (60k expired / 5k fresh, 254 MB) in one directory, swept from inside a live session:

scale

  • 60,000 / 60,000 removed in a single pass, ~8.3 s wall, errors=0, exactly the 5,000 fresh files left, 254 MB → 20 MB.
  • Process fd table flat at 14–15 across the whole sweep — the bounded batching holds.
  • Notably, unlinking while the opendir stream is still being iterated did not skip entries on APFS; a single pass was complete. (Worth knowing, because a partial pass would only be retried 24 h later.)

5. Concurrency, throttling, and retention: 0

concurrency

  • Three sessions started simultaneously on one log dir: one swept (removed=40), one lost the O_EXCL lock, one found the fresh marker. 0 files left, 0 errors, no double-unlink noise.
  • Two different projects → two distinct .openai-logs-cleanup-<hash> markers, both cleaned independently; a repeat session in the same project is throttled (openai-logs-cleanup: skipping, ran …ms ago).
  • retention: 0 behaves as documented: files with today's filename date but a 2 h-old mtime are removed, files written minutes earlier are kept — i.e. the mtime tie-break on the cutoff day works on real writer output.

6. Static checks

npm ci (full build incl. tsc --noEmit) exit 0 on both trees · eslint clean on all changed files · focused suites 97/97 pass (cleanup 24, scheduler 26, throttledOnce 7, settingsSchema 40) · re-running npm run generate:settings-schema produces no diff, so the vscode schema is in sync.

FYI on the PR description: the full build did not hit the Ink selection type errors here — it completed cleanly on this merge-base.


Non-blocking notes

a) An invalid openAILogRetentionDays silently becomes ~1 hour, not the 7-day default.

retention

-1 and "abc" both fall through getCutoffDate's cleanupPeriodDays > 0 ? … : MS_PER_HOUR branch, so real 3-day-old logs — comfortably inside the documented window — were deleted with no warning. The schema declares minimum: 0, but nothing enforces it at runtime. This is inherited behaviour shared with general.cleanupPeriodDays, so it is not a regression; but since this is a brand-new setting that deletes data, clamping non-finite/negative values back to DEFAULT_OPENAI_LOG_RETENTION_DAYS (rather than to the most aggressive branch) seems worth doing before merge.

b) The workspace-scope skip is invisible to the user. It is logged only via debugLogger.error, which writes nothing unless QWEN_DEBUG_LOG_FILE is set. Affected users get no cleanup and no signal. The guard also fires when the workspace value is identical to what user/system scope would resolve to, which is a safe-but-unnecessary skip. Consider surfacing it once in the UI, or only skipping when the workspace value actually differs.

c) getOpenAILogCleanupTarget() is evaluated twice per session (once in getFirstPassDelay, once in runHousekeeping), so the skip diagnostic in (b) is emitted twice per session. Cosmetic.

And a restatement of the author's own scope note, confirmed here: my log-generating runs used -p, which never starts housekeeping. Users who accumulate logs from scripted/headless runs will still not get cleanup until they open an interactive session in the same directory.

Real interactive session used for the sweeps (screenshot)

tui

中文版报告

本地验证报告(真实构建、真实会话、真实 API 流量)

我把本 PR 和它的 merge-base 各自完整构建成一份 bundle,并按用户真正会触发的路径来验证:真实的交互式 qwen TUI(tmux + 独立 QWEN_HOME)连接真实的 OpenAI 兼容 HTTP 服务,开启 model.enableOpenAILogging。下面所有日志 fixture 都是由线上同一份 OpenAILogger 写出来的——没有手写文件名,也没有 mock 文件系统。

Head b6e45cdfeat/openai-logs-housekeeping
Base 9aec40f(与 main 的 merge-base)
环境 macOS 26.6 (arm64)、Node v24.18.1、仓库 0.21.8
手段 两套 npm ci + npm run bundle 目录 · mock OpenAI SSE 服务 · tmux TUI · QWEN_DEBUG_LOG_FILE=1[HOUSEKEEPING] 日志

结论:行为与描述一致,没有发现正确性缺陷。文末有三条不阻塞合并的建议。

1. 端到端清理,head 对比 base

真实 headless 轮次写出真实日志 → 用真实 OpenAILogger 在偏移时钟下铸造真正过期的日志 → 放入各类干扰文件 → 启动交互式会话并保持空闲,等追赶(catch-up)pass 触发。

  • head:正好删除 5 个过期的 writer 自有日志(openai-logs: removed=5 errors=0),其中包含 -subagent-*-side-query-* 后缀形态。
  • 保留openai-my-export.json…-ZZZZZZZZ.json(非十六进制 id)、…deadbeef.json.bakopenai_2026-01-01.jsonREADME.md、一个与日志同名的目录,以及两个新鲜日志。日志根目录本身也保留。
  • base:同一份 fixture,删除列表为空,也没有 .openai-logs-cleanup-* marker——本 PR 之前确实完全没有这个清理器。

2. writer ↔ deleter 契约

比"误删"更危险的其实是反向:删除谓词漏掉某种 writer 形态,日志就会永久泄漏。我用真实 OpenAILogger 跑了 29 种 promptId 形态(内部 id、side-query: id、subagent a#b#c id、unicode、首尾连字符、120 字符长 id)× 5 个时钟,把它实际写出的每个文件名拿去匹配本 PR 的谓词:

写出 145 个文件,145 个全部命中,0 漏网。

3. 首次调度延迟矩阵

下表每一行 .file-history-cleanup 都是新鲜的,只有 per-directory 的 OpenAI marker 在变。这正是本 PR 修的问题。

build OpenAI marker 实测
head 缺失 first pass in 60s
head 8 天前 first pass in 60s
head 新鲜 first pass in 600s
base 缺失 first pass in 600s ← 积压被忽略

同一张截图还覆盖了共享自定义目录的策略:保留值放在 workspace scope 时清理被跳过,没有删除任何文件,也没有写成功 marker;把同一个值移到 user scope 后目录被正常清理(removed=2 errors=0)。

4. 规模验证 —— 对应 #8860 的场景

单目录 65,000 个扁平文件(60k 过期 / 5k 新鲜,254 MB),在活跃会话中清理:

  • 单次 pass 删除 60,000 / 60,000,耗时约 8.3 秒,errors=0,恰好留下 5,000 个新鲜文件,254 MB → 20 MB。
  • 整个清理过程中进程 fd 数稳定在 14–15,有界并发生效。
  • 值得一提:在 APFS 上边遍历 opendir 流边 unlink 没有导致条目被跳过,一次 pass 就扫干净了。(这点很重要,因为漏扫的部分要等 24 小时后才会再试。)

5. 并发、节流与 retention: 0

  • 三个会话同时启动、指向同一日志目录:一个执行清理(removed=40),一个抢 O_EXCL 锁失败,一个发现 marker 新鲜。最终剩 0 文件、0 错误,没有重复 unlink 的噪音。
  • 两个不同项目 → 两个独立的 .openai-logs-cleanup-<hash> marker,各自清理;同一项目再开会话会被节流(openai-logs-cleanup: skipping, ran …ms ago)。
  • retention: 0 行为与文档一致:文件名是当天日期但 mtime 为 2 小时前的文件被删除,几分钟前写入的文件保留——即"截止当天用 mtime 兜底"这条分支在真实 writer 输出上确实生效。

6. 静态检查

两套目录 npm ci(含完整构建与 tsc --noEmit)退出码 0 · 所有改动文件 eslint 无告警 · 聚焦测试 97/97 通过cleanup 24、scheduler 26、throttledOnce 7、settingsSchema 40)· 重新执行 npm run generate:settings-schema 无 diff,vscode schema 已同步。

另外补充一点:这里的完整构建并没有遇到 PR 描述中提到的 Ink selection 类型错误,在当前 merge-base 上是干净通过的。

不阻塞合并的建议

a) 非法的 openAILogRetentionDays 会静默变成约 1 小时,而不是回落到 7 天默认值。

-1"abc" 都会落进 getCutoffDatecleanupPeriodDays > 0 ? … : MS_PER_HOUR 分支,于是真实的 3 天前日志——明明在文档描述的窗口内——被无声删除。schema 里写了 minimum: 0,但运行时并没有强制。这是与 general.cleanupPeriodDays 共用的既有行为,不算回归;但既然这是一个会删数据的新设置,建议在合并前把非有限值/负值 clamp 回 DEFAULT_OPENAI_LOG_RETENTION_DAYS,而不是落到最激进的那条分支。

b) workspace scope 的跳过对用户不可见。 它只通过 debugLogger.error 记录,而这在未设置 QWEN_DEBUG_LOG_FILE 时不会写任何东西。受影响的用户既得不到清理,也得不到任何提示。此外,当 workspace 的值与 user/system scope 解析结果完全相同时也会触发跳过,属于安全但没必要的跳过。建议在 UI 中提示一次,或者只在值确实不同的时候才跳过。

c) getOpenAILogCleanupTarget() 每个会话被求值两次getFirstPassDelay 一次、runHousekeeping 一次),所以 (b) 中的诊断日志每个会话会打印两遍。属于观感问题。

最后复述并确认作者自己列出的范围外事项:我用来生成日志的运行都是 -p,它完全不会启动 housekeeping。也就是说,靠脚本/headless 运行积累日志的用户,只有在同一目录开一次交互式会话之后才会被清理。

Fixtures, harness scripts and full logs kept locally; screenshots published to pr-assets/8862-verify.

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.21.9.

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.

5 participants