Repository navigation
fix(test): serialise optimizer_bench fixture writes (flaky recall_file/no_match on main) - #43
Merged
Merged
Conversation
…ead never sees a truncated file measure() rewrites target/bench-fixtures/<name> with fs::write (truncate, then fill) and then lets the tool read it. Two tests in the file call measure() on parallel threads, so one thread's truncate could land under the other's read: the tool saw an empty file, reported full=0 and fell back to would_inflate. That failed every_mcp_tool_is_measured_against_the_committed_baseline on macos-14 (recall_file/no_match, full=0 returned=25 against 7495/230). measure() now holds a process-wide mutex from the write to the last tool call. The path stays the shared one because it is part of the reply header and so of the baselined token counts. A new test keeps one measure() inside its write-to-read window and fails if a second walks in.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cause
measure()inoptimizer_bench.rsrewrites the sharedtarget/bench-fixtures/<name>withfs::write(truncate, then fill) and then lets the tool read it. Two tests in the file callmeasure()on parallel threads, so one thread's truncate can land under the other's read. The tool sees an empty file, reportsfull=0, falls back towould_inflate. That is the red job: https://github.com/HackPoint/lumen/actions/runs/38170443513 (recall_file/no_match, full=0 returned=25 vs baseline 7495/230).Hypothesis confirmed. Throwaway repro on unfixed code (a thread rewriting
big.rsin a loop while the main thread ranrecall_file/no_match60 times): 1, 2 and 5 bad runs of 60 over three runs, every one exactlyfull_tokens: 0, returned_tokens: 25, route: would_inflate.Fix
A process-wide
Mutexheld bymeasure()from the write to the last tool call (poison ignored). The path stays shared because it is part of the reply header and so of the baselined token counts. A rename-over-file swap would fail on Windows while the other thread has the file open. Baseline file and assertions untouched, no retries, no new dependency.Red, then green
New test
measure_calls_never_share_the_fixture_window. It keeps onemeasure()inside its write-to-read window (via a scenarioargshook) and waits up to 400 ms for a second one to walk in. No race to win. Unfixed (3 of 3 runs, 0.37 s):Fixed: passes in 0.96 s. The whole bench binary passed 8 of 8 consecutive runs.
Audit (crates//tests, #[cfg(test)] in crates//src, src-tauri)
No other instance of parallel tests writing then reading one shared path.
escape_valve,hook_cli,repeat_reads,efficiency,protocol,ranked_metering,e2e_*and the daemon/stats/core tests use per-test tempdirs, and the hook harnesses override TMPDIR per test. Suspected, not fixed:LUMEN_BENCH_BLESS=1(manual only): the baseline test rewritesoptimizer_baseline.jsonwhilethe_fixtures_match_…reads it. The two accesses are seconds apart.report.rs(GH_TOKEN),update.rs(LUMEN_UPDATE_CHECK),faults.rs(LUMEN_FAULT_SPOOL) and daemon (LUMEN_LOG) tests mutate process env. I checked that nothing else in those binaries reads the vars today.report.rsstages long issue bodies at a fixedtemp_dir()/lumen-issue-body.md. No test reaches that branch.Local gate
/tmp/lumen-verify.shon this HEAD: GATE GREEN (660 s). Includes silent_fallbacks, fmt and clippy -D warnings, MSRV, hook e2e and the verify-install legs.