[Afloat16]

Fixes #6175

Problem

Automatic decompression spill paths use a block ID and Date.now(); range exports add the line range. Two calls in the same millisecond can therefore write the same path. Different views of a block can overwrite the first result after its path has already been returned, and repeated range exports can register the same pathname more than once in retention tracking.

Changes

Give both automatic spill paths a UUID suffix and create them exclusively with wx, preserving 0600 permissions. Explicit toFile destinations keep their existing behavior. The retention policy and its timestamps are unchanged.

Validation

• Rebased onto master e3220adc0f5412af83163666ca70a6afa05e3247 (package version 0.1.213). The intervening [PR] thirdparty-scan: flag dsh-live-compact as a known conflict (#5984) #6166 changes only third-party plugin detection, its tests, announcements and configuration docs; none of this patch's source or test files changed. The complete patch is byte-identical to the locally validated patch on 33104b1f9545caeb0344193009917ceef9969682 and applies cleanly.
• Negative control on 33104b1f9545caeb0344193009917ceef9969682: 2 new regression cases still fail before the production fix.
• Independent source tree based on 33104b1f9545caeb0344193009917ceef9969682 (28647ae92eb4502bb9aaebd7ef5d1d5550f02732): 27/27 focused tests passed, with no failures, skips or cancellations.
• The new spill regressions select TEMP on Windows and TMPDIR on other platforms so each test checks the directory actually used by os.tmpdir().
• Full project TypeScript checking and the standard npm run build passed on the temporary integration tree e186774d3428c85e2a4974c6e609b2a7381d6153 containing the seven independently submitted fixes on 33104b1f9545caeb0344193009917ceef9969682. The repository compiler options and include set are unchanged.
• git diff --check passed. Local runtime: Node.js 24.22.1 on Linux. Full cross-platform remote CI remains pending.

Open #5038 refactors these automatic spill paths into an autoSpillReceipt helper but retains timestamp-only names; it does not address collisions. If that refactor lands first, this naming and exclusive-create fix should be carried into its helper. No kernel or configuration changes.

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b)

Review of this PR is in progress (my session was restarted mid-review; reporting stage results now, final structured verdict follows shortly).

Verified so far:

• Base: single commit 68429e17 sitting directly on current master e3220adc — zero drift, mergeable. Diff is exactly 2 files: src/decompress-shared.ts (+8/−5) and tests/fix-decompress-tofile.test.ts (+49).
• Scope check: read the full module — there are exactly TWO automatic spill paths today (whole-block src/decompress-shared.ts:251, range src/decompress-shared.ts:456); both get the UUID suffix + exclusive wx create, 0600 kept. Explicit toFile (toFilePointer) deliberately unchanged, matching the documented caller-artifact semantics. A wx failure routes into the existing exportFailureReceipt — loud failure, no crash, no silent clobber.
• Local evidence (independent run, PR-head worktree): targeted suites tests/fix-decompress-tofile.test.ts + tests/fix-decompress-reaper.test.ts → 9/9 pass. Negative control reproduced myself: swapping in the pre-fix src/decompress-shared.ts makes both new tests fail (# pass 6 / # fail 2), restoring the fix makes them pass — the new tests have real teeth. tsc --noEmit clean.
• Duplicate screening: no duplicate of this fix among open/closed issues or PRs. One coordination point: open PR [PR] fix: derived-parent decompress honors toFile and spills oversized restores (#5013) #5038 (owner) refactors these same two lines into an autoSpillReceipt() helper while keeping timestamp-only names — you've already flagged it correctly in the PR body; I'll state the sequencing implication in the final verdict.

Remaining: live CI status, PR labels, one trivial comment-provenance nit I'm fixing directly on the branch, then the full 八问 review.

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b)

Review verdict: mergeable — CI green, tests have teeth, diff clean

查重: 不重复。#6183 is the only fix PR for #6175 (same author); no other open/closed issue or PR addresses automatic-spill path collisions. Related-but-distinct: open PR #5038 (owner) refactors these same two lines into autoSpillReceipt() while keeping timestamp-only names — coordination note below.

① Original issue direction. #6175 is a source-level report correctly pinned to v0.1.213 @ 33104b1f95. I verified the mechanism in code at that revision: both auto-spill paths derived filenames from (blockId[, span], Date.now()) — a millisecond clock as the uniqueness source — and wrote with the default truncate flag. Two spills in one millisecond ⇒ identical path; the second overwrote the first after its path was already returned to the model, and repeated range exports double-registered the same pathname in retention tracking. The proposed fix hits the root cause (path uniqueness at the decision point), not a symptom. Direction correct.

② Regression audit (existing behaviors on the touched paths, read pre-change code, not just the diff):

• Filename format -<epoch-ms>.txt → -<uuid>.txt (whole-block), -<start>-<end>-<uuid>.txt (range). Prefix acp-decompress- preserved; repo-wide grep shows only prefix-based consumers (tests/fix-decompress-reaper.test.ts:59, test filters) — nothing parses the timestamp portion, no doc mentions the pattern. Intentional cosmetic change, disclosed here per policy.
• Permissions: mode: 0o600 unchanged; wx guarantees a fresh file so the mode actually applies (previously an overwrite inherited the existing file's mode).
• Retention policy: cap/timestamps/beforeExit logic untouched; each call still registers exactly one entry; entries are now guaranteed distinct (the double-registration defect is gone).
• Error semantics: a collision can no longer silently clobber; the residual theoretical race throws EEXIST into the existing exportFailureReceipt ([decompress FAILED …]) — fail-loud, no crash, and on the range path it returns before any injection is queued.
• Explicit toFile: deliberately unchanged (caller-named artifact, documented overwrite semantics decompress: toFile is documented but never read — every restore goes to os.tmpdir() #1691/[decompress] toFile 将虚拟 URI 当磁盘路径，导出写入失败仍返回成功 #2204); all five pre-existing decompress: toFile is documented but never read — every restore goes to os.tmpdir() #1691 tests still green.
• Small-body inline path (<10000 chars): untouched. Both compression modes: resolveDecompress is host-side tool execution shared by plugin/proxy modes; wire shape unchanged (ack text identical, only path bytes differ). Kernel / config / env: untouched.

③ Code review. Minimal and correct: randomUUID from node:crypto (no new dependency), exclusive create via flag: "wx" on both paths. One trivial nit found: the fix replaced the range-spill comment and dropped its [#1207 review F4] provenance marker and the 0600 rationale — fixed directly on your branch as labeled commit 9179dc7f (comment only, no code change).

④ Test validity. Both new tests have real teeth — I independently reproduced the negative control: swapping in the pre-fix src/decompress-shared.ts makes both fail (# pass 6 / # fail 2); at head they pass (9/9 across fix-decompress-tofile.test.ts + fix-decompress-reaper.test.ts). They freeze Date.now() via t.mock.method (deterministic, no timing luck), assert distinct paths AND exact per-view content, and pick TEMP/TMPDIR platform-correctly (better than the pre-existing test in the same file, which only sets TMPDIR). No tautologies; a regression of the collision would turn them red.

⑤ Diff cleanliness. Two files, +55/−7, every line in scope; single linear commit; base = current master e3220adc (zero drift).

Coordination note (#5038). #5038 extracts autoSpillReceipt() over exactly these lines and promises byte-identical timestamp names. Whichever merges first must carry this fix forward: if #5038 lands first, rebase this PR onto it and move UUID+wx into the helper (one place then covers all three spill legs including derived-parent — strictly better); if this lands first, #5038's "byte-identical names" line goes stale. Both are mergeable today; flagging for sequencing, no action needed on this PR itself.

CI & labels. Green on reviewed head 68429e17: 30 checks, 0 failed, 0 pending (test lanes ubuntu/windows × node 22/24 all success; red-signal/codex-e2e/registry-e2e skipped by path trigger). My comment-only supplement 9179dc7f will re-run CI. mergeable_state=blocked reflects branch protection awaiting human approval, not failing checks. Labels applied on my own judgment of this diff: bug, p2, scope/storage — p2 rather than the suggested p1 because the trigger window is sub-millisecond, the consequence is recoverable (re-decompress), and there is no unavailability, cache leak, or unbounded growth.

---

结构化八问

1. 是什么问题：decompress 自动把大内容落盘时，文件名由 blockId（+行范围）+毫秒时间戳拼成；同一毫秒内的两次调用会生成同一路径，后写者覆盖先写者——模型手里拿着先返回的路径，读到的却是后一次的内容（静默内容错换）；重复的 range 导出还会在保留台账里把同一路径登记两次。
2. 什么原因导致的：根因在命名方案本身——用毫秒时钟充当唯一性来源（本质是非唯一键），叠加 writeFileSync 默认 w 标志对已存在文件直接截断覆盖，于是碰撞必然表现为静默覆盖而不是报错。
3. 影响范围与严重程度：仅当同一毫秒内发生两次自动 spill（并行工具调用/客户端重试）才触发；后果是模型从已返回路径读到错误恢复内容（可重新 decompress 恢复），外加保留台账重复登记（表面问题）。触发窗口极窄、无数据丢失、无只增不减 → p2，不升 p1。
4. 怎么修复的：每次 spill 加 UUID 后缀独占路径 + wx 独占创建——理论上的残余碰撞也会经既有 exportFailureReceipt 走失败回执（fail-loud），永不覆盖；显式 toFile 保持原语义（调用者按名指定的产物）；保留策略、时间戳逻辑、0600 权限均不变。
5. 是否有回归问题：逐项审过——文件名前缀 acp-decompress- 保留（reaper 测试过滤不受影响，全仓无任何消费者解析时间戳部分）；0600 保持；小于 10K 的 inline 路径不动；错误路径从「静默覆盖」变为「失败回执」（本修复的预期变更）；显式 toFile 行为不变（既有 decompress: toFile is documented but never read — every restore goes to os.tmpdir() #1691 五个测试全绿）；kernel/配置/env/wire 形状未动；两种压缩模式共用此宿主侧工具执行路径，无差异。无意回归：无；唯一有意变更是文件名格式（时间戳→UUID），已披露。
6. 是否破坏缓存：不涉及任何缓存/重试/去重键（range 注入去重用的 injId 未动）。
7. 是否改了设计方向：没有，spill 命名方案的局部加固。
8. 是否引入了新的配置：没有。

收工判定：可合并。CI 绿、新测试有牙（负向对照已独立复现）、diff 干净、基线即当前 master。唯一需要 owner 留意的是与 #5038 的合入顺序协调（见上）。

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b)

Status addendum (live-verified just now): master moved to dc041dd1 (merge of #5774) after my review above — the PR now shows behind. The new master commit touches kernel/*, src/absorb.ts, src/acp-block-ops.ts, docs and lockfiles only; it does not touch this PR's two files (src/decompress-shared.ts, tests/fix-decompress-tofile.test.ts), so no rebase is needed and "behind" is not a merge blocker here. CI is re-running on the new head 9179dc7f (my comment-only supplement). Verdict unchanged: mergeable.

[ranxianglei]

🤖 Powered by ework · qwen3.8-27b

[bot] 🏷 (ai: vllm-qwen/qwen3.8-27b)

Closing status (live-verified just now): CI on the current head 9179dc7f is green — 30 checks, 0 failed, 0 pending. Review is complete; nothing further is outstanding from my side.

Recap of where things stand:

• Verdict: mergeable. Fix is correct at the root cause (UUID-suffixed unique path + exclusive wx create on both auto-spill paths), regression-audited clean, new tests independently verified to have teeth (negative control reproduced), diff clean.
• Labels applied by me: bug, p2, scope/storage.
• One comment-only supplement commit (9179dc7f) pushed to your branch restoring dropped provenance in the range-spill comment.
• Remaining for the owner: merge when ready (branch protection makes it human-only). The only coordination item is open PR [PR] fix: derived-parent decompress honors toFile and spills oversized restores (#5013) #5038 touching the same lines — if [PR] fix: derived-parent decompress honors toFile and spills oversized restores (#5013) #5038 merges first, this PR should be rebased onto it and the UUID+wx naming moved into its autoSpillReceipt() helper; if this merges first, [PR] fix: derived-parent decompress honors toFile and spills oversized restores (#5013) #5038's "byte-identical names" claim goes stale. Both are currently mergeable against master.

[Afloat16]

Could a maintainer rerun Ubuntu 24 shard 2 on the current head 69ab7eec2cf970f0606a1f9dbeb75fe26c45cb0c? The latest full CI run failed a synchronous burst retains every record in order across rotations at tests/logger-rotate.test.ts:61: the collected sequence omitted record 11 from the expected 0–39 sequence.

That shard completed with 3,451 passing tests, one failure and eight skipped tests. The other 26 jobs passed, including the Windows 22 shard 2 that failed in the earlier run. This PR still changes only src/decompress-shared.ts and tests/fix-decompress-tofile.test.ts; the logger implementation and rotation tests are unchanged. I have not established the cause of this failure, so the check remains unresolved.