fix(test): isolate benchmark harness daemon runtimes

[astandrik]

What does this PR do?

Part of #1696 (audit ledger), follow-up to #1691/#1695.

scripts/benchmark-index.sh and scripts/benchmark-search-graph.sh ran the product
with no runtime or cache of their own: the benchmark repository was indexed into the
operator's live store, every one-shot joined the operator's account daemon, and the
timings depended on whatever that daemon was doing.

• Both scripts source scripts/test-runtime.sh, call cbm_test_runtime_init and
  run cbm_test_runtime_cleanup "$BINARY" from the EXIT trap.
• Comparability: benchmark-index.sh starts the private daemon before timing, so
  index-time.txt measures the index alone, and records setup-time.txt (daemon
  activation) and total-time.txt (their sum — the figure comparable with earlier
  runs, which paid activation inside the index timing whenever no daemon was warm).
  benchmark-search-graph.sh now takes <repo-path> instead of <project-name>:
  it indexes into the private cache (untimed) and times the queries against a daemon
  it keeps warm, so no timing includes activation. No performance claim is made.
• tests/test_benchmark_runtime_isolation_contract.sh: environment-probe fixture
  for both scripts plus the presence of the three timing files, the keep handoff
  (kept only after a successful index) and the refused-index contract (non-zero
  exit, cause surfaced, no root, no handoff). Fails on main with FAIL: benchmark-index exposed the caller CBM_RUNTIME_DIR to a product process; passes
  with this change. Wired as Step 0i2 in scripts/test.sh.

docs/EVALUATION_PLAN.md §7 runs benchmark-index.sh and then answers graph
questions from its own MCP session, which assumed the index stayed in the live store.
The harness therefore has an explicit, opt-in handoff: with CBM_BENCH_KEEP_RUNTIME=1
a run whose index succeeded stops its daemon, leaves the run-private root in place and
records the paths that reach it in <results>/<lang>/runtime-root.txt (sourceable);
ownership of that root, including its removal, passes to the caller. A failed or
refused index exits non-zero after the timing files are written, keeps nothing (no
root, no handoff — a stale one from an earlier run in the same results directory
included) and names its cause under --- index stderr ---. §7 sets the flag, sources
the file before the graph session, stops the daemon and removes the root in step 8;
§13 no longer clears ~/.cache/codebase-memory-mcp/*.db. Never on by default.

Checklist

• Every commit is signed off (git commit -s)
• Tests pass locally (make -f Makefile.cbm test) — full C suite (ASan/UBSan,
  sequential) on Linux/gcc 13.3 at f913f0e: 8114 passed, 8 skipped, exit 0; scripts/test.sh
  default (143/143 suites) and --tsan (1180 passed, 8 skipped) on the merge result
  with main e783f73
• Lint passes (make -f Makefile.cbm lint-ci) — clang-format 20.1.8 + cppcheck
  2.20.0 (the CI toolchain) at f913f0e, clean
• New behavior is covered by a test (reproduce-first for bug fixes)

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Pull request overview

This PR updates the benchmark harness scripts to run against a harness-owned daemon runtime and cache, preventing benchmark runs from contaminating or depending on the operator’s live store/daemon state, and adds a contract test to enforce that isolation.

Changes:

• Add a benchmark runtime isolation contract test that verifies both harnesses don’t leak caller CBM_* dirs into product processes and that benchmark-index.sh writes new timing artifacts.
• Update benchmark-index.sh to initialize/cleanup a private runtime, start a private daemon before timing, and emit setup-time.txt and total-time.txt.
• Update benchmark-search-graph.sh to accept <repo-path>, index into a private cache (untimed), and run queries against a warm private daemon.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
tests/test_benchmark_runtime_isolation_contract.sh	Adds an isolation contract test + timing file presence checks for benchmark harnesses.
scripts/test.sh	Wires the new contract test into the shell test runner.
scripts/benchmark-search-graph.sh	Switches to repo-path input and makes indexing/queries use a harness-owned runtime/daemon.
scripts/benchmark-index.sh	Runs index benchmark under harness-owned runtime/cache, starts private daemon before timing, and writes setup/total timing files.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[astandrik]

Taken, in both scripts (8f0ff31): REPO_JSON=$(python3 -c 'import json,sys; print(json.dumps(sys.argv[1]))' "$REPO"), the spelling scripts/soak-test.sh already uses for the same reason.

benchmark-index.sh carried the identical unescaped interpolation (pre-existing, not flagged); fixing one file and leaving its twin looked worse than the small extra diff.

Fail-before is in the contract now: with a repository path containing " and \, the old request came out as {"repo_path":"/private/tmp/.../re"po\dir","mode":"full"}, which is not JSON. The test asserts the recorded request parses and that its repo_path is the resolved repository. POSIX-only case — NTFS rejects both characters in a path component.

[astandrik]

Taken (8f0ff31). All three readings had to move together — they are subtracted from each other, so mixing clocks was not an option — through one helper, bench_now_ms, using time.monotonic_ns() // 1000000. Its reference point is fixed per boot on every platform this runs on, so the three readings stay comparable even though each comes from its own python3 process.

[astandrik]

Taken (8f0ff31). All three readings had to move together — they are subtracted from each other, so mixing clocks was not an option — through one helper, bench_now_ms, using time.monotonic_ns() // 1000000. Its reference point is fixed per boot on every platform this runs on, so the three readings stay comparable even though each comes from its own python3 process.

[astandrik]

Taken (8f0ff31). Both the index call and the parse now write stderr to a run-private temp file (removed by the same EXIT trap), and the failure path prints that stderr plus the first 500 bytes of the response under the existing "did not report a project" line. Covered in the contract: the fixture refuses the index with a stderr line, and the test requires both the section header and the cause to reach the output — it fails against the previous head of this PR.

As of 07ee617 benchmark-index.sh keeps the index stderr the same way: the maintainer's round asked for a refused index to exit non-zero, and a failure that names only its symptom would have been half of it.

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[DeusData]

Thank you for the benchmark isolation work. The evaluation plan has a later graph phase that expects the indexed data to remain available, so cleanup and retained-runtime ownership need an explicit handoff before this is integrated. We need more time to review that lifecycle contract and keep the reported total-time comparison meaningful; no benchmark redesign is being accepted here.

[DeusData]

Thank you for the three follow-up commits — this answers the handoff question exactly the way I hoped: opt-in only, the daemon stopped before the root is handed over, a sourceable runtime-root.txt so the graph session never has to guess paths, and ownership stated in one sentence. I verified it on the merge result with today's main (e783f73d): merge clean; the contract test passes as Step 0i2 in 1.2 s; with either benchmark script alone restored to main it fails on the right assertion (benchmark-index exposed the caller CBM_RUNTIME_DIR to a product process, and the search-graph twin), so it binds to both fixes; bash -n and shellcheck -x -P scripts clean on every touched script; no delay added anywhere; and the caller's runtime and cache never reach a product process, with zero cbmrt.* roots left behind after every run. The scripts/test.sh hunk is one purely additive step, no environment or ordering change, and I have noted its scope on our side.

One thing to close before it merges, and it is in the contract you wrote rather than in the isolation: retention keys on the script's exit status, but an index failure is swallowed. bench_finish keeps the root when $? -eq 0, yet the index call is … || echo '{"error":"index failed"}', so a refused index still exits 0. With CBM_BENCH_KEEP_RUNTIME=1 that keeps the root and writes runtime-root.txt for an index that does not exist — reproduced here: exit 0, 00-index.json = {"error":"index failed"}, root present, handoff written. That contradicts the comment above bench_finish ("a failed run cleans up regardless: there is no index worth keeping") and it is the case §7 would hit hardest, because the graph session would then run against an empty store and every question would look like a miss. The contract test enshrines exactly this path: the keep run uses the probe fixture, which refuses cli index_repository, so the assertions at the keep step check that a root is kept after a failed index.

What I would ask for, one small round:

1. Make retention depend on the index having succeeded, not on the exit status — either require a non-empty project from the index in bench_finish, or let an index failure exit non-zero after the timing files are written (my preference: the second, so the caller's loop notices too).
2. In the contract test, let the keep-run fixture answer cli index_repository with a minimal envelope (e.g. {"project":"probe"}) so the keep assertions cover the successful-index handoff, and add one assertion that CBM_BENCH_KEEP_RUNTIME=1 with a refused index leaves no root and no handoff file.
3. Two doc lines if you are in there anyway: §7 step 8 removes a root whose daemon the graph session (steps 4–7) may still have running — a daemon stop under the kept CBM_RUNTIME_DIR belongs before the rm -rf; and §13 Reproducibility still clears ~/.cache/codebase-memory-mcp/*.db and calls the harness unflagged, which no longer describes what the harness does.

Incidental and not yours to fix: run_case in benchmark-search-graph.sh uses date +%s%3N, which BSD date on macOS does not support; the contract test never reaches it, so CI is unaffected, and I have recorded it on our side. The Windows guard failure on your run is the daemon endpoint cold-start race (#2275), not this diff; I will rerun it once that has landed. After this round the harness change is the one we want.

[astandrik]

Thank you — reproduced exactly as you describe, and the retention contract now keys on the index, not on
the exit status alone.

benchmark-index.sh (07ee617): the index call records the CLI's exit status; once every per-run file is
written — the three timing files included — the run exits 1 when the CLI failed or reported no project
(the search-graph twin's own test), so bench_finish sees rc=1 and cleans up as its comment promised, and
the caller's loop notices. 00-index.json stays valid JSON on failure ({"error":"index failed","exit":N}).
The call also keeps its stderr in a run-private temp file, the twin's shape, and the failure prints it under
--- index stderr --- with the first 500 bytes of the response: for a one-shot cli the refusal is reported
on stderr only, and with 2>/dev/null the operator was left with the symptom. One extra line: a stale
runtime-root.txt from an earlier run in the same results directory is removed at start, since §7 reuses
/tmp/eval-results/<lang> and sources that file unconditionally — a failed re-run must not hand the graph
session the previous run's paths.

Contract test: the fixture answers cli index_repository with {"project":"probe"} when
CBM_BENCH_PROBE_INDEX_OK is set, so the keep run now covers the successful-index handoff (and asserts
project.txt); the unflagged run asserts a non-zero exit and the surfaced cause; a new keep run with a
refused index asserts a non-zero exit, the timing files, no surviving root and no handoff — a pre-seeded
stale one included. Against 9574c31 it fails on benchmark-index exited 0 after a refused index.

Docs (f913f0e): §7 step 8 stops the daemon under the sourced CBM_RUNTIME_DIR before rm -rf;
§13 step 2 no longer clears ~/.cache/…/*.db, calls the harness with CBM_BENCH_KEEP_RUNTIME=1 and points
at §7 for the handoff and removal.

Verified with a real binary built from this head (Linux, gcc 13.3) and with the release binary on macOS:
default run — exit 0, root gone; keep — exit 0, runtime-root.txt sourceable, cli list_projects under it
sees the index, a daemon started under it stops with the §7 step-8 recipe and the root goes away, no process
left; keep + refused index (CBM_ALLOWED_ROOT outside the repo) — exit 1, timing files present, no handoff,
no root, the refusal in the output; the same into a reused results directory — the stale handoff is gone.
bash -n and shellcheck -x -P scripts clean; merge with today's main (e783f73) is clean, and on that merge
result scripts/test.sh (default lane, 143/143 suites) and scripts/test.sh --tsan (1180 passed, 8 skipped, no
TSan reports) both pass end to end on Linux/gcc, Step 0i2 included. On this head, make -f Makefile.cbm lint-ci
with the CI toolchain (clang-format 20.1.8, cppcheck 2.20.0) is clean, and CI is green across the matrix
(https://github.com/DeusData/codebase-memory-mcp/actions/runs/35730933863), the Windows guards included this time.

Not touched, as you noted: date +%s%3N in run_case. One thing I saw while at it and left alone: both
harnesses still pass raw JSON to cli index_repository, which the CLI now warns is deprecated (--repo-path
is the flag form) — happy to switch both in a follow-up if you want it.