The QA runner's home is not the repo, and a claim PIN is not a repo file - #509
The QA runner's home is not the repo, and a claim PIN is not a repo file#509emooreatx wants to merge 1 commit into
Conversation
Same defect CIRISAgent's `server_manager` just hit on a desktop first-run setup:
`env.setdefault("CIRIS_HOME", project_root)`. Ours was
`if "CIRIS_HOME" not in env: env["CIRIS_HOME"] = str(project_root)` — the same
thing spelled out.
TWO FAILURES FOLLOW FROM POINTING HOME AT THE CHECKOUT.
First, anything reading home at the PRODUCT default (`/var/lib/ciris`, or the
desktop app's `~/ciris`) looks somewhere else and finds nothing. On a first run
that means the claim fails with the one-time PIN sitting in the repo, written by
the same run that could not find it. The two halves disagree about where home is,
and neither is wrong on its own.
Second, a one-time claim SECRET lands in a git checkout — alongside the node's
store, logs and identity material.
The QA home is now a per-run temp dir: isolated between runs, cleaned by the OS,
and incapable of disagreeing with a product default because nothing else claims
it. An explicit `CIRIS_HOME` from the caller still wins; that is the knob for
pointing a run at a real home deliberately. `stop()` removes the temp home, and
ONLY one we created — a caller-supplied home is theirs and may hold a real node.
BELT AS WELL AS BRACES. `.gitignore` now covers `claim_pin`, `ciris_engine.db`
(+wal/shm), `logs/`, `secrets/`, `keys/` and `identity/` at the repo root, so a
run that somehow still writes there cannot leave a claim secret one `git add -A`
away from being committed. `data/` is deliberately NOT listed: it holds tracked
mesh fixtures, and a node home would collide with them — a second reason home does
not belong in the checkout.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BSa6L2MEDAXmoJuBVUZNVg
![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: f5ef0083a9
ℹ️ 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".
| env["CIRIS_HOME"] = str(project_root) | ||
| self.console.print(f"[dim]Setting CIRIS_HOME={project_root}[/dim]") | ||
| qa_home = Path(tempfile.mkdtemp(prefix="ciris-qa-home-")) | ||
| env["CIRIS_HOME"] = str(qa_home) |
There was a problem hiding this comment.
When the handler or CIRISNode modules run, this redirects the agent's state to <qa_home>/data, but the tests still inspect repository-relative paths: handler_tests.py:233 opens data/ciris_audit.db for every handler audit assertion, and cirisnode_tests.py:248 checks data/agent_signing.key. Those checks therefore report missing files even though the server created them successfully in the temporary home. Expose the selected home to these validators or update them to resolve paths through CIRIS_HOME as part of this change.
Useful? React with 👍 / 👎.
| /ciris_engine.db | ||
| /ciris_engine.db-wal | ||
| /ciris_engine.db-shm |
There was a problem hiding this comment.
When CIRIS_HOME is pointed at the checkout—the exact fallback scenario these rules are meant to protect—the database is written to data/ciris_engine.db, not the repository root: ServerConfig::from_home sets data_dir = home.join("data") and db_path() appends ciris_engine.db (src/config.rs:204,251-252), which also matches server.py:574. Consequently these root-only patterns do not match the database or its WAL/SHM files, and git add can still stage the node store. Add specific /data/ciris_engine.db* exclusions while leaving the tracked mesh fixtures visible.
Useful? React with 👍 / 👎.
| qa_home = getattr(self, "_qa_home", None) | ||
| if qa_home is not None: | ||
| import shutil as _sh | ||
|
|
||
| _sh.rmtree(qa_home, ignore_errors=True) |
There was a problem hiding this comment.
If startup raises after mkdtemp()—for example, Popen fails in staged mode, or opening the console log fails after the child starts—the exception handler at start():1182 returns False without calling stop(). Moreover, this cleanup is nested under if self.process, so even a caller that invokes stop() cannot remove a home when failure occurred before process assignment. Such failures leave per-run directories behind, and post-spawn failures can leave the database and claim PIN in them; move home cleanup outside the process conditional and run it on the startup exception path.
Useful? React with 👍 / 👎.
| env["CIRIS_HOME"] = str(project_root) | ||
| self.console.print(f"[dim]Setting CIRIS_HOME={project_root}[/dim]") | ||
| qa_home = Path(tempfile.mkdtemp(prefix="ciris-qa-home-")) | ||
| env["CIRIS_HOME"] = str(qa_home) |
There was a problem hiding this comment.
When the standalone identity_update module runs, it kills the manager-owned server and starts a replacement at identity_update_tests.py:246-298 using {**os.environ, "CIRIS_CONFIGURED": "true"}. The temporary CIRIS_HOME exists only in this manager's private child environment and is never added to os.environ, so the replacement cannot locate the database and identity created by the original process; the post-restart login at lines 342-362 consequently targets a different node state and fails. Make the selected home available to restart workflows or have this module restart through the manager with the same environment.
Useful? React with 👍 / 👎.
1 participant
Mirrors the fix CIRISAgent needed after a desktop first-run setup failed to claim:
server_managerdidenv.setdefault("CIRIS_HOME", project_root), so the backend used the repo as home while the app used the product default~/ciris. The app looked for a PIN the backend had written somewhere else.We had the same line, spelled out:
if "CIRIS_HOME" not in env: env["CIRIS_HOME"] = str(project_root)inqa/qa_runner/server.py.Two failures follow from pointing home at the checkout:
Fix: the QA home is a per-run temp dir — isolated, OS-cleaned, and incapable of disagreeing with a product default because nothing else claims it. An explicit
CIRIS_HOMEfrom the caller still wins.stop()removes the temp home and only one we created; a caller-supplied home is theirs and may hold a real node.Belt as well as braces:
.gitignorenow coversclaim_pin,ciris_engine.db(+wal/shm),logs/,secrets/,keys/,identity/at the repo root — verified to bite.data/is deliberately excluded from that list because it holds tracked mesh fixtures, which is itself a second reason home does not belong in the checkout.🤖 Generated with Claude Code
https://claude.ai/code/session_01BSa6L2MEDAXmoJuBVUZNVg