Repository navigation
test: deterministic teardowns for the three observed CI flakes (#1208) - #1224
Conversation
…rip, e2e-export-handoff (#1208)
📦 Built Package ArtifactBranch: Option A — Install from npm PR tag (recommended)npm install -g billion-context@pr-1224Each push to this PR publishes a new version under the Option B — Download artifact
tar xzf billion-context-pr1224.tgz
npm install -g packageInstall instructions are refreshed on each push. |
一句话总结:审核了 PR #1224(#1208 三个 CI flake 的测试加固)——三个根因均在代码层面独立验证成立、对其他 enabled-store 测试的审计结论复核无误、CI 四平台全绿、纯测试改动零生产行为变更,可以合并。 ① 原 issue 方向:#1208 建议「逐一定位 + 确定性等待」,本 PR 对三个观察到的 flake 全部照此实施;明确不做建议 2(CI retry-on-flake)——理由成立:根因已确定性消除,retry 只会掩盖未来真实回归,建议 3(标记 known-flaky)随之不需要。第三个 flake(ubuntu-22 master@8daba7fd)确认为 ② 回归审查(既有行为清点):
③ 代码/机制审查(逐条读了相关代码路径,不只读 diff):
④ 测试有效性:三处都是真测试、断言有牙—— ⑤ diff 整洁度:+11/−2、3 个文件、单个线性 commit 直接坐在当前 master head(aae4de1c)上,每行都与目的相关,注释统一带 #1208 出处。无 version bump、无 lockfile/生成物 churn。干净。 验证证据:
结论:可以合并。按惯例 Agent 不合并 PR,请人工操作:#1224 |
Fixes #1208
What changed (test-only, zero src/ changes)
tests/logger-rotation.test.ts—settle()now requires a stable non-zero size before returning and throws on timeout instead of silently returning. Previouslylastinitialized to-1, which is exactly what a missing file reads as, so if the stream's asyncopenhad not completed within ~60ms (plausible under windows-latest load), settle returned early whilestreamFdwas still undefined. The clobber then raced the in-flight open:isOrphaned()short-circuits to "not orphaned" while an open is in flight, both post-clobber logs reused the pending stream, and the expected single[warn]never fired (warns.length0 ≠ 1 — the exact PR fix: register watcher pid for every native client attaching to a shared proxy (#1190) #1200 windows-24 failure). With the precondition guaranteed (open completed + line flushed), the clobber/reopen path is deterministic on every OS: libuv opens files withFILE_SHARE_DELETEon Windows, so the clobber succeeds either way,statSync(logPath)then throws ENOENT, orphan detection fires, the reopen fails with ENOTDIR, and exactly one warn is emitted.tests/session-gc.test.ts("records rawInputTokens per turn and persists it ([Feature] Default state file cleanup strategy #1082)") — drains in-flight debounced saves withawait store.flushAll([])before theflushSyncround-trip assertion (same pattern as Flaky: #1101 T7 cleanup races in-flight persist writes — ENOTEMPTY kills the release gate #1194). WithdebounceMs: 10, a turn-1 timer can fire between turns and start an asyncwriteNowwhose payload snapshot predates turn 2'srawInputTokens; its rename can land after the synchronousflushSync, leaving stale bytes on disk →recorded value survives persistence round-tripfails (the master@e8be19a1 windows-24 failure). AfterflushAllno competing writer remains.tests/e2e-export-handoff.test.ts— same one-line drain before the teardownrmSync: this test drives real proxy traffic (markDirty → scheduleSave, debounceMs 0) over an enabled store and deleted the dir without draining, the identical race class as Flaky: #1101 T7 cleanup races in-flight persist writes — ENOTEMPTY kills the release gate #1194/T7. Hardened proactively so it cannot become the next random red.Not changed (verified safe)
Audited every other enabled-store test for the same race class (debounced save in flight at teardown
rmSync):fix-401-storage,persist-tail,cli-export,search-context-cross-session,store.test.tsenvelope round-trip: use awaitedwriteNowor sync-onlyflushSync— no in-flight writer at teardown.issue1086T3/T8: foreign sessions never create local state →scheduleSavenever called.fix-config-session: sync eviction→flushSync only.proxy-persist,host-usage-postfold: teardowns either drain (flushAll) or cancel before any write can be in flight; the residual fire-and-forget case is covered by the kernel-side unhandled-rejection fix from release v0.1.42 #158.The third flake reported in #1208 (ubuntu-22 master@8daba7fd) was identified via the CI job log as
issue1086-chain-self-exemption.test.tsT7 (ENOTEMPTY rmdir at teardown) — already fixed on master by 3f61cc1 (#1194); the failing run predated that commit. No action needed.Behavior changes
None in production code — all three edits are inside
tests/. No config schema, wire shape, persistence format, or default changes.Pre-flight
npm run typecheck✓npm test: 2377 pass / 3 fail — the 3 failures are intests/claude-native.test.ts(liveps-walk e2e tests) and fail identically on the untouched tree in this sandbox (verified via stash); they are environment-specific here, not regressions. All 3 touched files pass 5/5 standalone repeated runs.npm run build✓Note: I did not add blanket CI retry-on-flake (issue suggestion 2) — with the root causes removed deterministically, retries would only mask future real regressions.