Repository navigation
fix(test): isolate the test suite from a live B12 install - #212
Conversation
Installer tests rebound the live com.b12.mcp.daemon launchd job to a temporary plist and stopped the live embed daemon through its /tmp PID file, and a test MCP session wrote its summary into the live database, because launchd, /tmp sockets and the HOME-derived DB path are shared with the developer's real install. Honor B12_EMBED_RUNTIME_DIR and B12_MCP_DAEMON_SOCK at every hook, script and installer site (defaults unchanged), and add a root conftest.py that points HOME and the runtime knobs at a private temp tree and puts no-op launchctl/crontab stubs first on PATH before any test module is imported. A self-check test fails if the isolation is not active.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
LoCoMo bench — skippedlocomo10.json fixture not available on this runner The regression gate runs against the monthly scheduled bench and explicit |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0945ee5f9d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
install_mcp_daemon() waited for ${B12_MCP_DAEMON_SOCK} but the rendered plist did not carry it, so the daemon bound the default socket and the installer timed out. Write B12_MCP_DAEMON_SOCK and B12_EMBED_RUNTIME_DIR into the job's EnvironmentVariables when they are set. Also make the suite-isolation DB check independent of test order.
|
@codex review |
LoCoMo bench — skippedlocomo10.json fixture not available on this runner The regression gate runs against the monthly scheduled bench and explicit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 064d842354
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
pwd and os.getuid are Unix-only, so importing the module failed on native Windows. Skip the login-home checks when they are unavailable and the POSIX stub check on Windows.
|
@codex review |
LoCoMo bench — skippedlocomo10.json fixture not available on this runner The regression gate runs against the monthly scheduled bench and explicit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 97b68b3487
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The isolated HOME hid ~/.local/b12-venv, so the pagerank guard tests skipped themselves. The conftest now exports the installed interpreter as B12_TEST_VENV_PYTHON (data paths stay isolated) and the pagerank tests use it.
|
@codex review |
LoCoMo bench — skippedlocomo10.json fixture not available on this runner The regression gate runs against the monthly scheduled bench and explicit |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ecc0934051
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… home
On native Windows os.path.expanduser() reads USERPROFILE, not HOME, so module-level DB paths could still resolve to the live AppData database. Point USERPROFILE at the isolated home and add a platform-neutral self-check that expanduser('~') resolves there.
|
@codex review |
LoCoMo bench — skippedlocomo10.json fixture not available on this runner The regression gate runs against the monthly scheduled bench and explicit |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
Running the test suite on a machine where B12 is installed reached the live install:
install.shwith a temporaryHOME, but the launchd job domain is per account.reload_daemon_if_running()saw the livecom.b12.mcp.daemon, andinstall_mcp_daemon()unloaded it through the temporary plist (same label) and loaded the temporary one, so launchd kept respawning a daemon from a pytest temp dir.restart_embed_daemon_if_running()read the live PID file under/tmpand stopped the live embed daemon.HOMEat import time, so a test MCP session wrote its session summary into the live database.B12_EMBED_RUNTIME_DIRandB12_MCP_DAEMON_SOCKexisted but were only honored by the daemons and a few helpers; about 30 hook and script call sites, plus the installer's socket wait and uninstall, still used hardcoded/tmp/b12-*paths. So even a custom runtime dir would not have isolated clients from the live daemon.This change:
B12_EMBED_RUNTIME_DIR(embed socket, PID and lock) andB12_MCP_DAEMON_SOCK(socket and the PID file next to it) everywhere; default paths are unchangedconftest.pywhosepytest_configureruns before any test module is imported:HOME, the embed runtime dir and the MCP socket point into a private temp tree, inheritedB12_*settings are dropped, no-oplaunchctlandcrontabstubs come first onPATH, and the Hugging Face cache keeps its real location; the tree is removed atpytest_unconfiguretests/test_suite_isolation.py, which fails if the isolation is not activeCONTRIBUTING.mdTest plan
bash -nfor all hooks andinstall.sh; Python heredoc syntax check passesosis imported at module level (or earlier in the same heredoc) at every changed Python site/tmp/b12-mcp-<UID>.pid,/tmp/b12-embed-<UID>.sock)mainon macOS (they pass in CI). Before/after snapshots of the live launchd job (plist path and PID), embed daemon PID, crontab, DB max id / row count / deleted count, LaunchAgents and setup settings files were identical. The stub log shows 14launchctl list com.b12.mcp.daemoncalls during the run, each of which used to trigger the hijacktests/test_suite_isolation.py: 4 passed with the conftest, 4 failed with--noconftestDocs sync
README.mdupdated: not neededdocs/architecture.mdupdated: not neededCHANGELOG.mdentries added under Unreleased / FixedCONTRIBUTING.mddocuments the isolation and the rule for new codeAnti-Claude trailer check
Co-Authored-BytrailersDependencies