fix(test): isolate benchmark harness daemon runtimes by astandrik · Pull Request #2198 · DeusData/codebase-memory-mcp

astandrik · 2026-09-13T10:52:21Z

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. 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.

To decide: docs/EVALUATION_PLAN.md §7 runs benchmark-index.sh and then answers
graph questions from the main MCP session, i.e. it assumes the index stays in the
live store (rm -f ~/.cache/codebase-memory-mcp/*.db between languages). With a
harness-owned cache that assumption no longer holds; either the graph phase runs
against the harness root, or the harness needs an explicit keep-root mode. Not
changed here.

Checklist

Every commit is signed off (git commit -s)
Tests pass locally (make -f Makefile.cbm test) — shell-only change; the new
contract plus test_venue_parity_contract and test_runtime_isolation_contract
were run; the full C suite was not run on this host
Lint passes (make -f Makefile.cbm lint-ci) — no C changes
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.

+    echo "private daemon did not start" >&2
+    exit 1
+fi
+INDEX_JSON=$("$BINARY" cli index_repository "{\"repo_path\":\"$REPO\",\"mode\":\"full\"}" 2>/dev/null || echo '{}')


+# alone. setup-time.txt keeps the activation cost attributable and
+# total-time.txt is their sum — the figure comparable with earlier runs, which
+# paid activation inside the index timing whenever no daemon was already warm.
+SETUP_START_MS=$(python3 -c "import time; print(int(time.time()*1000))")


+fi
+
 # Index via CLI and capture timing
 START_MS=$(python3 -c "import time; print(int(time.time()*1000))")


+INDEX_JSON=$("$BINARY" cli index_repository "{\"repo_path\":\"$REPO\",\"mode\":\"full\"}" 2>/dev/null || echo '{}')
+PROJECT=$(echo "$INDEX_JSON" | python3 -c "
+import json, sys
+d = json.load(sys.stdin)
+if 'content' in d:
+    d = json.loads(d['content'][0]['text'])
+print(d.get('project', ''))
+" 2>/dev/null || echo "")
+if [ -z "$PROJECT" ]; then
+    echo "index of $REPO did not report a project" >&2
+    exit 1
+fi



github-actions · 2026-09-13T11:26:26Z

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 · 2026-09-19T20:25:57Z

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.

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 and every one-shot joined the operator's account daemon. Source scripts/test-runtime.sh in both, start the private daemon before timing, record setup-time.txt and total-time.txt beside index-time.txt, and index from a repository path instead of querying a project in the live store. Add tests/test_benchmark_runtime_isolation_contract.sh, which fails before this change, and wire it into scripts/test.sh. Part of DeusData#1696. Signed-off-by: Anton Standrik <astandrik@yandex-team.ru>

Review follow-up for the benchmark harness isolation. - Both benchmark scripts build the index request with a JSON-escaped repository path (python3 json.dumps, the spelling soak-test.sh already uses): a path containing a quote or a backslash produced a payload the server could not parse. - benchmark-index.sh reads its three timestamps from one helper backed by time.monotonic_ns(); an NTP step mid-run no longer skews or negates a figure whose purpose is comparison across runs. - benchmark-search-graph.sh keeps the stderr of the index call and of the response parse in a run-private temp file (removed by the EXIT trap) and prints it, plus the first 500 bytes of the response, under the existing "did not report a project" line. - The contract test now refuses the index from its fixture and requires the cause to reach the output, and (POSIX only) drives a repository path with a quote and a backslash and requires the recorded request to parse as JSON with the resolved path. Both assertions fail against the previous head of this branch. Signed-off-by: Anton Standrik <astandrik@yandex-team.ru>

The evaluation plan (docs/EVALUATION_PLAN.md §7) indexes a language with benchmark-index.sh and then answers graph questions against that index from its own MCP session. With a run-private runtime (DeusData#1696) the index was gone before that session could start, so the handoff is now explicit and opt-in. With CBM_BENCH_KEEP_RUNTIME set, a SUCCESSFUL run stops its daemon, leaves the private root in place and records the paths that reach it in <results>/<lang>/runtime-root.txt (sourceable: CBM_BENCH_RUNTIME_ROOT, CBM_RUNTIME_DIR, CBM_CACHE_DIR). Ownership of that root, including its removal, passes to the caller. A failed run cleans up regardless: there is no index worth keeping and nothing may leak. Nothing changes in the default path or in the three timing files. §7's skeleton sets the flag, sources the file before the graph session and removes the root in step 8 instead of the live-store *.db files. The contract test drives a kept run and requires the root to survive, the handoff file to be sourceable, and its paths to be the ones the product processes actually used; the unflagged run still asserts the root is gone. The kept-run assertions fail against the previous head. Signed-off-by: Anton Standrik <astandrik@yandex-team.ru>

Copilot AI lite review requested due to automatic review settings September 13, 2026 10:52

astandrik requested a review from DeusData as a code owner September 13, 2026 10:52

astandrik mentioned this pull request Sep 13, 2026

Audit smoke, soak, profiling, and benchmark harnesses for account daemon runtime isolation #1696

Open

Copilot AI reviewed Sep 13, 2026

View reviewed changes

Copilot started reviewing on behalf of astandrik September 13, 2026 11:26 View session

DeusData added bug Something isn't working stability/performance Server crashes, OOM, hangs, high CPU/memory priority/high Needs near-term maintainer attention; high-impact bug, regression, safety issue, or release blocker. labels Sep 19, 2026

This was referenced Sep 20, 2026

fix(test): isolate soak harness daemon runtime #2196

Merged

fix(test): isolate memlab harness daemon runtime #2197

Merged

astandrik added 3 commits September 21, 2026 12:00

astandrik force-pushed the fix/1696-bench-runtime-isolation branch from b5b69af to 9574c31 Compare September 21, 2026 09:50

Navigation Menu

fix(test): isolate benchmark harness daemon runtimes - #2198

fix(test): isolate benchmark harness daemon runtimes#2198
astandrik wants to merge 3 commits into
DeusData:mainfrom
astandrik:fix/1696-bench-runtime-isolation

astandrik commented Sep 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

github-actions Bot commented Sep 13, 2026

Uh oh!

DeusData commented Sep 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

astandrik commented Sep 13, 2026

What does this PR do?

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

github-actions Bot commented Sep 13, 2026

Uh oh!

DeusData commented Sep 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants