# Handoff — the `test_dev.ml` "never bound agent.sock" flake ## Root cause `test_dev.ml`'s first block checked `Sys.file_exists agent_sock` without having completed a single round-trip on the daemon socket, so nothing had ordered the check after the bind. The evidence is in `lib/dev.ml`: - `merged_setup` (lib/dev.ml:2264) binds and listens on the **dev** socket (`Unix.bind ls` at lib/dev.ml:2300) and returns. The program's main thread — the thread that calls `agent/start`, which is what binds `agent.sock` (`vendor/agent/flan_agent.c:1097` `flan_agent_start`) — only runs *after* `merged_setup` returns. - So the test's `await (fun () -> Sys.file_exists sock)` (test/test_dev.ml:76) says nothing about `agent.sock`, and `connect sock` (test/test_dev.ml:82) succeeds off the **listen backlog** without anyone having accepted: `merged_serve` (lib/dev.ml:2311) does not reach `accept_loop` until after its own `await ~ms:10000 (fun () -> Sys.file_exists t.agent)` (lib/dev.ml:2322). - The comment at lib/dev.ml:2314-2320 states the ordering outright. There is a **second race on the same path**, which the timings confirmed. The test unlinks `agent.sock` immediately after the check (test/test_dev.ml, `Unix.unlink agent_sock`). Both the test and `merged_serve` poll that path at 5ms. When the test won and unlinked first, `merged_serve` burned its full 10s timeout and printed the "does it call (agent/start ...)?" warning before accepting — the test still passed, ~10s slower. Measured: failing runs clustered at ~24s, passing runs at ~30-36s. ## What changed One file: `test/test_dev.ml`. The existing `describe` request and its status check were **moved up** to sit immediately before the `agent_sock` existence check (and removed from their old position a few lines below). No new sleep, no lengthened timeout, no change to `lib/dev.ml`. A completed reply proves `merged_serve` got past its `await` — i.e. it has *already observed* `agent.sock` — which makes the existence check deterministic and puts the unlink safely after that observation, killing both races at once. The genuine-failure property is preserved: if a program really never binds, `merged_serve` warns after 10s and accepts anyway, the reply arrives, and the existence check fails as it should. ## Verified Run as `./test_dev.exe` from `_build/default/test`, serially, whole binary, detecting the flake by grepping stdout for `never bound`: - **Before: 2/8 runs failed (25%)** — matches the rate NEXT.md records. Failing runs 24.7s and 24.0s; passing runs 30.0-36.0s (the 10s stall described above). - **After: 0/10 runs failed (0%)**, and every run landed in the fast band, 17.0-26.7s — the 10s stall band is gone as well, which is the independent confirmation that the second race is also closed. Run counts are small (8 and 10) because the session was cut short for budget; each run is ~25s. A longer sweep was **not measured**. `dune test --root .` was run twice after the change. The second run was green. The first run reported `1 failure(s)` with `FAIL the daemon never listened` — see item 1 below. ## What remains 1. **Decide whether `merged_serve`'s 10s warning path deserves a test.** Nothing currently exercises lib/dev.ml:2322-2326 (a program that never calls `agent/start`). It was reachable only by accident here, via the unlink race that is now closed. ## Closed since: the other "the daemon never listened", which was not a race at all The previous item 1 sent two lanes after a contention race. **There is none**, and the reading it was built on was wrong twice over: - The wording is **not** unique to `test_emacs.ml:50` and `test_repl.ml:71`. `test/test_dev.ml:77` — the *dev* socket check in the same first block this handoff fixed the *agent* socket check in — emitted exactly `the daemon never listened` as well. Three sites, not two, which is why a sighting in a full-run log could not be attributed. - The cause is a **bound that is too short for a build, not a race**. `flan dev` does an llc-and-link of the whole program *before* it binds its socket, so the await covers a build. Warm and idle that is ~600ms; under dune's own parallelism it was measured at 6.5s here and at 6.6s and 6.8s by the lane that fixed it — against a 5000ms await in `test_dev.ml` and 8000ms in the other two. Nothing about it is occasional; it reproduces whenever the build is cold, and `Build.cachedir` sits under dune's per-run `TMPDIR`, so the object cache starts empty on **every** `dune test` invocation. Fixed on another branch: all three awaits now wait a minute, bounded by the existing watchdog rather than by a guess, and `test_dev.ml` carries a named `listening` helper so the reason is written once for its thirteen daemons. Two measurements from this lane that stand whatever else changes: **30 sequential full `dune test` runs, 0 failures and no `never bound` in any of them** — the `agent.sock` ordering fix above is confirmed at a sweep wide enough to close item 3 of the old list, so `NEXT.md`'s "One flaky test, measured rather than suspected" section is deleted. The reproduction that came after it, on the 31st run, was the *dev* socket at `test_dev.ml:77`, with `built dev-loop.flan in 6525ms` sitting four lines below the FAIL in the same log — which is what turned the hypothesis into the finding above. ## Tried and rejected - **Wrapping the check in the file's `await` helper** (`await (fun () -> Sys.file_exists agent_sock)`). It looks like the obvious fix and it is wrong: it makes the test observe the file at the *earliest* possible instant, which makes it *more* likely to win the unlink race against `merged_serve` and more likely to trigger the silent 10s stall. Do not reintroduce it. - **Lengthening any timeout** — *for the `agent.sock` check*. Not needed there; the ordering was missing, not the time. This does **not** generalise, and reading it as a rule cost two lanes: the socket-appears checks above are a wait for a *build*, and there lengthening the bound was exactly the fix. See the section before this one. ## Environment note for whoever picks this up Running `_build/default/test/test_dev.exe` directly requires the dune-copied deps (`test/programs/*.flan`) to be present in `_build`. A bare `dune build --root .` does **not** copy them — every daemon then dies with `Sys_error("programs/dev-loop.flan: No such file or directory")` and all eleven blocks report "never listened". Run `dune build --root . @test/runtest` once first to populate `_build/default/test/programs/`. Also run the loop **strictly serially**: the socket names under `$TMPDIR` are fixed (`flan-devtest-*.sock`), so parallel runs collide and manufacture failures.