feat(cli): clean up OpenAI logs in non-interactive sessions - #8893
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Non-interactive OpenAI log retention E2EBaselineGlobal VerificationUse a temporary
Run and verify these paths against the built CLI:
For each path, assert the expired log is deleted, the recent and lookalike files remain, and the per-directory cleanup marker exists. For two concurrent processes sharing the directory, assert only one obtains the lock and both exit without errors. For cancellation, use the focused scheduler tests rather than fixed sleeps: an interrupted scan must release its lock when possible and must not write a completion marker. ResultAll cases passed against the built
The last case covers graceful CLI exit cleanup, not |
|
Thanks for the follow-up — gate check below. Re-run after the two feedback-batch commits (
Moving on to code review. 🔍 中文说明感谢这个后续 PR,以下是准入检查结果。本次为 re-run:此前审查后新增的两个 commit(
进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-run focus: the two commits since the last reviewed revision are test-only — production code is byte-for-byte unchanged (verified via the compare API), so the earlier walkthrough carries over, and this pass checked whether the new tests actually pin what the review asked for. They do:
The not-taken items carry coherent reasons that match the design doc: R1-3 (no ceiling on failure retries — a ceiling could leave cleanup permanently idle once a directory recovers), R1-4 (frozen retention per target — retention changes take effect after restart by documented contract), R1-5 (no job eviction — pruning could drop a target before its first log write). The design itself still holds up on re-read:
One standing watch-item: sequenceDiagram
participant P1 as CLI or ACP lifecycle
participant P2 as FIFO queue (one worker)
participant P3 as runThrottledOnce
participant P4 as cleanupOldOpenAILogs
P1->>P2: start with resolved log dir (dedup)
P2->>P3: one directory at a time
P3->>P3: check marker freshness, then lock
P3->>P4: scan with AbortSignal
P4-->>P3: removed, errors, completed
P3-->>P2: completed, fresh, locked or incomplete
P2->>P2: reschedule (daily, remaining interval, 1 min or 10 min)
P1->>P2: exit (abort active scan, drop queue, wait up to 250 ms)
Files changed (14)
Test evidence (PR's own CI — this is an unattended run; no PR code was executed here)Both
Sandboxed verification of the remaining gap is in flight: the sponsored 中文说明代码审查Re-run 关注点:上次审查后的两个 commit 均为纯测试改动——生产代码逐字节未变(已用 compare API 核实),因此此前的走查结论延续有效,本次重点核对新测试是否真正钉住了评审要求。结论是钉住了:
未采纳项的理由与设计文档一致、成立:R1-3(失败重试无上限——设上限可能在目录恢复可用后让清理永久闲置)、R1-4(目标冻结保留期——按文档约定保留期变更重启后生效)、R1-5(任务不逐出——逐出可能在首次日志写入前丢掉有效目标)。 设计本身复读后依然成立:
一个持续的关注点: 时序图与文件清单见英文部分(14 个文件:核心队列为 scheduler.ts;throttledOnce 改为结果枚举;cleanup 支持中止;gemini.tsx 负责生命周期接线;acpAgent.ts 仅两行注册;其余为测试、文档与重新生成的 schema)。 测试证据(PR 自身 CI——本次为无人值守运行,未执行任何 PR 代码)被审 commit 上两个 剩余缺口的沙盒验证正在进行:赞助的 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — solid, focused follow-up; the re-run only added test strength, no new concerns surfaced. Stepping back: this remains the second half of a fix the project already committed to, against a problem with hard observational evidence (95 GB / 340k files in #8860). What changed since the last reviewed revision is exactly what a healthy review loop should produce: two test-only commits that close real mutation holes — the exit-time stop registration, both retry-clamp branches, the queued-duplicate dedup path, the content-generator precedence, the partial-batch settling, and the post-lock marker recheck. I spot-checked the two load-bearing premises of those tests at head (the final The one substantive reservation is unchanged: the end-to-end wiring (a real session deleting files and writing the marker) is still only covered mock-on-both-sides in unit tests plus the author's macOS black-box run. That is what the in-flight sponsored 中文说明置信度:4/5 —— 扎实、聚焦的后续 PR;re-run 只增强了测试,没有出现新的疑虑。 退一步看:这仍是项目已承诺修复的后半段,针对的是有硬观测证据的问题(#8860 中 95 GB / 34 万文件)。相比上次审查的版本,变化恰恰是健康评审循环应有的产物:两个纯测试 commit 补上了真实的变异漏洞——退出时的停止注册、重试 clamp 的两个边界、排队重复的去重路径、content-generator 优先级、部分批次的落定、以及获锁后的 marker 复查。我在 head 上抽查了这些测试依赖的两个关键前提(清理器末尾的 唯一实质性的保留意见未变:端到端接线(真实会话删除文件并写入 marker)在单测中仍是"双侧 mock"覆盖,外加作者的 macOS 黑盒验证。正在进行中的赞助 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (macOS/Windows unit-matrix jobs also skipped; unit suites ran locally on Linux only).
Not explored to full depth (tool budget reached): You are review agent reverse-audit — Reverse audit agen...: none — all checks I started were completed within budget.; This PR (#8893) extends OpenAI API log retention cleanup ...: none — all checks in my dimension completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed within budget.; You are review agent reverse-audit — Reverse audit agen...: none — all checks above were completed within budget.; chunk 2: none — all checks I identified were completed within budget., and 3 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): 582 assertions should pass — this review observed 481 passed.
中文说明
已审查。 建议见行内评论。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (macOS/Windows unit-matrix jobs also skipped; unit suites ran locally on Linux only)。
未探索到全部深度(达到工具调用预算):You are review agent reverse-audit — Reverse audit agen...:none — all checks I started were completed within budget.;This PR (#8893) extends OpenAI API log retention cleanup ...:none — all checks in my dimension completed within budget.;You are review agent reverse-audit — Reverse audit agen...:none — all checks above completed within budget.;You are review agent reverse-audit — Reverse audit agen...:none — all checks above were completed within budget.;chunk 2:none — all checks I identified were completed within budget.,另有 3 条。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan(非阻断):582 assertions should pass — this review observed 481 passed。
— qwen3.8-max via Qwen Code /review (v0.21.9)
wenshao
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — the full packages/cli test command timed out and packages/vscode-ide-companion tests did not run.
中文说明
已审查。 建议见行内评论。
未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未审查:build-and-test — the full packages/cli test command timed out and packages/vscode-ide-companion tests did not run。
— gpt-5.6-sol via Qwen Code /review (v0.21.9)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Review feedback batch (23ab89a)
Verification: npm run build; npm run typecheck; 88 focused Vitest tests; focused Prettier and ESLint checks. |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Review feedback batch (f437346)
Verification: npm run build; npm run typecheck; 126 focused Vitest tests; focused Prettier and ESLint checks. |
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. The deletion predicate is unchanged and byte-for-byte the #8862 writer-contract regex — unparseable names skipped (never mtime-deleted), boundary day strictly-older via mtime, and headless/stream-json/ACP all funnel through the same cleanupOldOpenAILogs with no broader predicate anywhere; the cross-package test generates a real OpenAILogger filename and asserts deletion. Lock/marker/cancellation semantics are correct: the lock always releases in finally, the marker is written only on completion, a cancelled scan returns completed:false with no marker so a later long-lived process resumes, abrupt death is covered by the 1-hour stale-lock takeover, and in-flight unlink batches settle before the lock releases. The FIFO worker serializes per-dir jobs with dedup and a bounded stop drain. All nine prior suggestions are resolved (six fixed with tests, three declined with rationale consistent with the documented contract). CI green on head. Nothing blocks merge.
|
@qwen-code /triage |
|
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: 795 passed · 0 failed · 795 total 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:795 通过 · 0 失败 · 795 总计 Verification reportPR 8893 Deep Verification — feat(cli): clean up OpenAI logs in non-interactive sessionsVerdict: merge-ready — scripted assertions: 795 pass / 0 fail (ledger notes below). Verified head: 中文摘要结论:merge-ready(可合并)。795 项脚本化断言全部通过,0 失败。
ScopeCentral claim: one-shot headless invocations now run OpenAI log retention cleanup — expired writer-format files removed, lookalikes preserved, one completion marker, lock released — where base does nothing. Secondary claims: (1) an interrupted short process releases the lock without a false marker and a later process resumes to completion; (2) ownership/lifecycle ordering — workspace-only retention on a custom dir skips; cleanup starts only after auth success and (one-shot) after non-empty input; concurrent processes on one dir produce exactly one completed scan. Central claim — A/B tableIdentical scenario per arm: scratch
9/9 head assertions, 5/5 base assertions (the base arm's expected inaction is encoded as assertions, so its predicted "failure to clean" counts as passed control cells). Witness: The same seed shape was driven through the stream-json transport ( Interrupt / resume (secondary claim 1)Calibrated against the real compiled cleaner (20,000 files in 147 ms ⇒ N = 200,000). Phase 1 (mock delay 0): process lived 1.4 s, removed 1,632 files, was aborted by its own exit cleanup — no marker, no lock, 198,368 residue, exit 0, debug log observed Load-bearing proof (mutation A/B). The compiled
The mutant reproduces exactly the bug this PR exists to fix (false success marker suppressing a later complete scan). Witness: Ownership + lifecycle ordering (secondary claim 2) — 13/13Witness:
Cells C/D confirm cleanup starts only after auth and non-empty input (housekeeping never ran — no marker, no deletions, no mock traffic). Lock contention — 9/9Process A (400,001 seeded files, slow mock) held the directory lock while scanning; process B started inside that window: exit 0, no marker, cleaned nothing (A's lock asserted held at B's exit — only the lock holder can delete), then A completed the single scan, released the lock, wrote the single marker (count over Targeted gates
Vacuity / mutation matrix
Positive controls: the head arm of the A/B (cleanup demonstrably happens) and the mock-server request counts ( FindingsNone. No blocking, no non-blocking defects produced by this round. (The 12 settings-suite failures are reported under Targeted gates as attributed pre-existing container behavior, not as PR findings.) Not covered
MethodologyOne Evidence imagesHarness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.21.10. |
|
PR 已合并(merged)并已通过评审(APPROVED by yiliang114, qwen-code-ci-bot),CI 为绿色。以下是我对变更的评审总结: Scope: 将 OpenAI API 日志保留从交互式 housekeeping 扩展到 headless、stream-json SDK 和 ACP daemon 会话。14 个文件,+1028/-86 行。 Key design observations:
No blockers. 所有先前的评审建议均已解决(6 个已修复+测试,3 个已保留并有合理理由)。CI 在 head 上为绿色。 |








What this PR does
This PR extends OpenAI API log retention from interactive sessions to one-shot headless invocations, stream-json SDK transports, and ACP-based daemon sessions. Non-interactive targets are resolved from the initialized model configuration with settings fallbacks, deduplicated by absolute log directory, and processed through one process-local FIFO worker so multiple ACP workspaces never multiply disk concurrency. Interactive and non-interactive processes continue to share the existing per-directory marker and exclusive lock.
The cleaner now supports cooperative cancellation and reports whether it reached the end of the directory. An interrupted scan releases its lock without writing a completion marker, allowing a later long-lived process to resume cleanup. Persistent processes retry completed work daily, fresh markers when their remaining interval expires, lock contention after one minute, and filesystem failures after ten minutes. Exit cleanup cancels active work, drops queued targets, and waits for at most 250 milliseconds without blocking model requests or ACP session creation.
The CLI lifecycle starts cleanup only after authentication succeeds and, for one-shot modes, after non-empty input is confirmed. Stream-json starts immediately before its protocol loop, while ACP registers each successfully initialized workspace session and intentionally ignores the bootstrap working directory. Successful and exceptional exits now both run the existing cleanup chain. No SDK transport, core writer, public API, log format, directory layout, or user-facing option is changed.
Why it's needed
The initial retention implementation in #8862 runs only from interactive background housekeeping. TypeScript, Python, and Java process-based SDKs use stream-json, while daemon SDKs use ACP, so workloads that primarily use headless or SDK entry points can continue accumulating full OpenAI request and response logs indefinitely. Short one-shot processes also need cancellation-aware best-effort cleanup so they do not leave a false success marker that suppresses a later complete scan.
Reviewer Test Plan
How to verify
openai-my-export.json, then run one-shot headless, stream-json, and ACP flows. Confirm only the expired writer-owned file is removed and the directory receives one completion marker.Evidence (Before & After)
N/A — this changes background housekeeping behavior and has no TUI or visual output.
Tested on
Environment (optional)
macOS 26.4.1 arm64; Node.js 22.22.3; npm 10.9.8; repository version 0.21.9; built CLI with a local fake OpenAI server for black-box headless, stream-json, ACP, ownership, lock-contention, and interrupted-scan verification.
Risk & Scope
SIGKILL, crashes, and external termination that bypasses exit cleanup rely on the existing one-hour stale-lock recovery. Direct@qwen-code/qwen-code-coreembeddings and in-memory channels that bypass the CLI lifecycle remain outside this change.Linked Issues
Closes #8860
Follow-up to #8862
中文说明
本 PR 的改动
本 PR 将 OpenAI API 日志保留能力从交互式会话扩展到一次性 headless 调用、stream-json SDK transport 和基于 ACP 的 daemon 会话。非交互目标从已初始化的模型配置及设置回退中解析,按绝对日志目录去重,并通过单个进程内 FIFO worker 处理,因此多个 ACP 工作区不会放大磁盘并发。交互式与非交互式进程继续共享现有的目录级 marker 和排他锁。
清理器现在支持协作式取消,并报告是否扫描到了目录末尾。被中断的扫描会释放锁但不写完成 marker,使后续长驻进程能够继续完成清理。长驻进程会在完成后每日重试、在 fresh marker 的剩余有效期结束后重试、锁竞争后一分钟重试,并在文件系统失败后十分钟重试。退出清理会取消当前任务、丢弃排队目标,并最多等待 250 毫秒,同时不会阻塞模型请求或 ACP session 创建。
CLI 生命周期只会在认证成功后启动清理;对于一次性模式,还要求已经确认输入非空。stream-json 在进入协议循环前启动,而 ACP 会为每个成功初始化的工作区 session 登记目标,并有意忽略 bootstrap 工作目录。成功返回和异常退出现在都会运行现有退出清理链。本 PR 不修改 SDK transport、core writer、公共 API、日志格式、目录结构或用户可见参数。
为什么需要
#8862 中的初始日志保留实现只从交互式后台 housekeeping 启动。TypeScript、Python 和 Java 进程型 SDK 使用 stream-json,而 daemon SDK 使用 ACP,因此主要采用 headless 或 SDK 入口的负载仍可能无限累积完整的 OpenAI 请求与响应日志。短生命周期的一次性进程也需要支持取消的尽力清理,避免留下错误的成功 marker,进而阻止后续进程执行完整扫描。
评审者测试计划
如何验证
openai-my-export.json),然后分别运行一次性 headless、stream-json 和 ACP。确认只删除过期的 writer 自有文件,并且目录只产生一个完成 marker。证据(改动前后)
N/A——本 PR 修改后台 housekeeping 行为,不包含 TUI 或视觉输出。
测试平台
环境(可选)
macOS 26.4.1 arm64;Node.js 22.22.3;npm 10.9.8;仓库版本 0.21.9;使用构建后的 CLI 和本地 fake OpenAI server 完成 headless、stream-json、ACP、配置归属、锁竞争及中断扫描的黑盒验证。
风险与范围
SIGKILL、崩溃及绕过退出清理的外部终止依赖现有的一小时陈旧锁恢复。直接嵌入@qwen-code/qwen-code-core以及绕过 CLI 生命周期的内存 channel 不在本次范围内。关联问题
Closes #8860
#8862 的后续 PR