fix(test): isolate memlab harness daemon runtime - #2197
Conversation
scripts/memlab.sh gave the profiled run a private CBM_CACHE_DIR only, so the process joined the operator's account daemon and cleanup removed the cache without stopping any daemon. Source scripts/test-runtime.sh for a harness-owned runtime and cache, and stop the private daemon before the work directory is removed. Add tests/test_memlab_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>
|
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. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
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. |
There was a problem hiding this comment.
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
Fixes memlab harness runtime isolation so profiled runs don’t attach to (or disrupt) the operator’s daemon, and adds a contract test to prevent regressions.
Changes:
- Initialize/cleanup an isolated test runtime (runtime dir + cache) in
scripts/memlab.sh. - Adjust memlab log folding to read logs from the isolated cache location.
- Add a deterministic contract test and wire it into
scripts/test.sh.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
scripts/memlab.sh |
Uses shared runtime helper to isolate daemon rendezvous/cache and updates log collection pathing. |
tests/test_memlab_runtime_isolation_contract.sh |
New contract test that asserts memlab doesn’t leak caller runtime/cache into the launched environment. |
scripts/test.sh |
Runs the new contract test as part of the shell test steps. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| CACHE_HOST="$WORK/cache" | ||
| else | ||
| export CBM_CACHE_DIR="$WORK/cache" | ||
| CACHE_HOST="$CBM_TEST_CACHE_DIR_HOST" |
| # With CBM_CACHE_DIR set the process logs to its own file rather than stderr, | ||
| # so fold that in or the census series is invisible. | ||
| cat "$WORK"/cache/logs/*.log >> "$RUN_LOG" 2>/dev/null || true | ||
| cat "$CACHE_HOST"/logs/*.log >> "$RUN_LOG" 2>/dev/null || true |
| source "$(dirname "$0")/test-runtime.sh" | ||
| cbm_test_runtime_init || exit 1 | ||
| trap 'cbm_test_runtime_cleanup "$BINARY"' EXIT | ||
|
|
||
| WORK=$(mktemp -d 2>/dev/null || mktemp -d -t memlab) |
| cleanup() { | ||
| # The helper probes with $BINARY, whose native-Windows copy lives in $WORK. | ||
| cbm_test_runtime_cleanup "$BINARY" | ||
| rm -rf "$WORK" 2>/dev/null || true | ||
| rm -rf "${WIN_ROOT:-}" 2>/dev/null || true | ||
| } | ||
| trap cleanup EXIT |
| if grep -q 'env=' "$ROOT/scripts/memlab-drive.py"; then | ||
| fail "memlab-drive.py no longer passes the harness environment through unchanged" |
| private_root="" | ||
| while IFS=$'\t' read -r child_cache_raw child_runtime_raw; do | ||
| child_cache=$(normalize_path "$child_cache_raw") | ||
| child_runtime=$(normalize_path "$child_runtime_raw") |
| "${child_runtime##*/}" != "runtime" || "${child_cache##*/}" != "cache" ]]; then | ||
| fail "memlab runtime/cache were not isolated beneath one private root" | ||
| fi | ||
| private_root="${child_runtime%/*}" |
What does this PR do?
Part of #1696 (audit ledger), follow-up to #1691/#1695.
scripts/memlab.shgave the profiled run a privateCBM_CACHE_DIRonly, so theprocess joined the operator's account daemon (refused with a cache-root conflict
when one was live; otherwise left as the account daemon while
cleanup()removedits cache underneath it).
scripts/memlab.shsourcesscripts/test-runtime.sh, callscbm_test_runtime_initand runscbm_test_runtime_cleanup "$BINARY"beforeremoving its work directory (the native-Windows copy of the binary lives there).
Off native Windows the helper's cache is the cache; the log fold-in reads from it.
tests/test_memlab_runtime_isolation_contract.sh: memlab launches the productthrough its Python driver, which passes the environment through unchanged
(
memlab-drive.pyuses noenv=; the test pins that), so the contract shimspython3and records the environment at the driver boundary. This keeps itdeterministic on Windows CI, where the native Python cannot exec a shell fixture.
Fails on
mainwithFAIL: memlab exposed the caller CBM_RUNTIME_DIR to the product; passes with this change. Wired as Step 0g2 inscripts/test.sh.Observation, not changed here:
scripts/memlab.shis tracked without an executablebit although its usage line invokes it directly; the contract runs it through
bash.Checklist
git commit -s)make -f Makefile.cbm test) — shell-only change; the newcontract plus
test_venue_parity_contractandtest_runtime_isolation_contractwere run; the full C suite was not run on this host
make -f Makefile.cbm lint-ci) — no C changes