Repository navigation
Deploy plumbing for the codex author (install-on-init + tick sources .env) - #126
Conversation
…urces .env - scripts/install_codex.sh: idempotent install of the harness-verified codex binary (0.130.0). Fast path is a local 'codex --version'; only downloads (codex-x86_64-unknown-linux-musl.tar.gz) on a version mismatch or missing binary. Atomic replace; x86_64-only (the pinned target). - tick_chain.sbatch: sources ~/.config/autoresearch/.env each tick (so a live config change like AUTORESEARCH_AUTHOR_BACKEND=codex needs no chain restart), and runs install_codex.sh best-effort when the fleet author is codex. Both are fail-safe: a missing .env or a failed install logs and the chain continues. Enabling codex on the live loop is now a .env edit + provisioning the codex key file — no code change, reversible. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Round 1 — reviewed head 0493d7cc — reviewer hermes/gpt-5.6-terra.
second opinion — terra
Advisory findings from autoresearch — the code owner decides. Reply to disagree; the autoresearch:no-review label opts this PR out.
Verdict: 1 blocking, 0 advisory.
1 finding attached to the lines below.
There was a problem hiding this comment.
Round 1 — reviewed head 0493d7cc — reviewer claude/claude-opus-5.
Advisory findings from autoresearch — the code owner decides. Reply to disagree; the autoresearch:no-review label opts this PR out.
Verdict: nothing blocking — 6 advisory notes.
4 findings attached to the lines below.
Advisory (non-blocking):
- a failed mv leaves a stale temp binary next to the target (
scripts/install_codex.sh:45; low) - binary lookup is depth-1 only (
scripts/install_codex.sh:41; low)
I cannot verify the actual layout or checksum of the openai/codex rust-v0.130.0 release asset from this checkout, so the find-depth point is stated as uncertain. Submitted jobs do inherit the sourced env: compute.py builds sbatch argv with no --export flag, so Slurm's default --export=ALL applies. Tilde handling is consistent: climb.py and followup.py expanduser the AUTORESEARCH_CODEX_BIN default, matching the script's ~/.local/bin/codex target. No tests were added, and none exist for shell scripts here.
- [terra blocking] The tick sourced .env checking only readability — a group/ world-writable .env would be code execution as the tick user. Now it refuses to source unless the file is owned by us AND not group/world-writable (fails closed to 'not sourced'). - [claude] curl now has --connect-timeout/--max-time/--retry, so a stalled download can't hang the tick until Slurm kills it. - [claude] install verifies the installed codex reports the pinned version after the swap (the release asset is mutable) — a wrong/corrupt binary fails loudly. - [claude] the staged temp binary is cleaned on every exit (trap), not just the work dir; the tarball binary lookup no longer assumes archive depth. - [claude] documented that .env is per-tick config only — chain-scheduling vars (AUTORESEARCH_CADENCE_MIN, HOME/ROOT/ACCOUNT/PARTITION) come from --export and are read before the .env source, so setting them there has no effect. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
All #126 round-1 findings addressed in 616d7b0:
bash -n + pre-commit (large-files, whitespace, gitleaks) clean. |
There was a problem hiding this comment.
Round 2 — reviewed head 616d7b0c — reviewer hermes/gpt-5.6-terra.
second opinion — terra
Advisory findings from autoresearch — the code owner decides. Reply to disagree; the autoresearch:no-review label opts this PR out.
Verdict: 1 blocking, 0 advisory.
1 finding attached to the lines below.
There was a problem hiding this comment.
Round 2 — reviewed head 616d7b0c — reviewer claude/claude-opus-5.
Advisory findings from autoresearch — the code owner decides. Reply to disagree; the autoresearch:no-review label opts this PR out.
Verdict: nothing blocking — 6 advisory notes.
4 findings attached to the lines below.
Advisory (non-blocking):
- An unset-variable reference in .env kills the tick despite the
|| echoguard (scripts/tick_chain.sbatch:125; low) - Cache dirs set in .env are never created (
scripts/tick_chain.sbatch:104; low)
I could not verify from the checkout that the release asset name codex-x86_64-unknown-linux-musl.tar.gz exists for tag rust-v0.130.0, nor that codex 0.130.0 prints a version string the grep -oE matches; both are external facts. The AUTORESEARCH_CODEX_BIN default path matches climb.py:2015 and followup.py:686, so the install target is consistent with the code.
… vars (#126 R2) - [terra blocking] The version check ran AFTER mv, so a wrong/corrupt download was already at the author's codex path on exit 1. Now the staged binary is verified against the pinned version BEFORE the atomic mv — a bad binary never lands. - [terra] Tightened the download bound (--max-time 120 --retry 1) so a stalled fetch can't eat the ~15-min tick job it runs inside. - [terra] Corrected the false 'no effect on HOME/ROOT' claim: .env IS referenced later (install path, tick --root), so the chain-identity vars (HOME/ROOT/ ACCOUNT/PARTITION + cache dirs) are now snapshotted and RESTORED around the source — .env genuinely can't hijack them. - [claude] Source .env under set +u so an unset-var reference can't abort the tick (|| echo can't catch an unbound-variable exit). Cache dirs are covered by the restore, so a .env override can't leave them uncreated. Deferred (suggestion): no checksum/sigstore on the download — it comes from the official GitHub release over HTTPS at a pinned tag + a version assertion; full signature verification is a follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Round 3 — reviewed head e5e663e1 — reviewer hermes/gpt-5.6-terra.
second opinion — terra
Advisory findings from autoresearch — the code owner decides. Reply to disagree; the autoresearch:no-review label opts this PR out.
Verdict: 1 blocking, 0 advisory.
1 finding attached to the lines below.
There was a problem hiding this comment.
Round 3 — reviewed head e5e663e1 — reviewer claude/claude-opus-5.
Advisory findings from autoresearch — the code owner decides. Reply to disagree; the autoresearch:no-review label opts this PR out.
Verdict: 1 blocking, 5 advisory.
3 findings attached to the lines below.
Advisory (non-blocking):
- Some documented knobs are consumed before .env is sourced, so setting them there silently does nothing (
scripts/tick_chain.sbatch:118; medium) - The chain-var restore can itself be overridden by .env, and HOME/PATH are not restored (
scripts/tick_chain.sbatch:137; low) - --max-time is per attempt, so the fetch cap is double what the comment claims (
scripts/install_codex.sh:42; low)
I could not verify host-side facts (whether ~/.local/bin already exists on the cluster, the actual layout of the codex 0.130.0 tarball, or that codex 0.130.0 is already installed there) from the repository; those claims come only from the PR description. No Python or tests changed, and no shell tests exist in the repo, so nothing here is test-gaming.
… fix install dir order (#126 R3) - [terra/claude blocking] install wrote the staged binary into the target dir BEFORE mkdir -p created it — a host without ~/.local/bin couldn't install. Reordered: mkdir -> stage -> verify(staged) -> atomic mv. (The verify already ran before the mv; the reorder makes the dir exist first too.) - [terra/claude, .env footgun class] Stop SOURCING .env — that executed it and let it set any variable (chain HOME/ROOT/PATH/cadence, or arbitrary code). Instead extract an ALLOWLIST of author knobs (AUTORESEARCH_AUTHOR_BACKEND/ _MODEL, CODEX_BIN, {CODEX,HARNESS}_KEY_FILE) via grep — no execution, no chain-var hijack, no unset-var abort. Verified a KEY=$(cmd) line is read as a literal, not run. Perms check kept (a writable .env could still inject a bad value like a malicious codex path). - [claude] curl --retry 0 (single bounded attempt; the next tick retries the idempotent install), so the fetch cap is a true 120s. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Round 3 addressed in cb94b6c:
bash -n + pre-commit clean. |
There was a problem hiding this comment.
Round 4 — reviewed head cb94b6ce — reviewer hermes/gpt-5.6-terra.
second opinion — terra
Advisory findings from autoresearch — the code owner decides. Reply to disagree; the autoresearch:no-review label opts this PR out.
Verdict: 1 blocking, 0 advisory.
1 finding attached to the lines below.
There was a problem hiding this comment.
Round 4 — reviewed head cb94b6ce — reviewer claude/claude-opus-5.
Advisory findings from autoresearch — the code owner decides. Reply to disagree; the autoresearch:no-review label opts this PR out.
Verdict: nothing blocking — 4 advisory notes.
3 findings attached to the lines below.
Advisory (non-blocking):
- .env value parsing only strips double quotes, not single quotes, inline comments, or trailing whitespace (
scripts/tick_chain.sbatch:128; low)
I verified that tick_chain.sbatch uses only set -u (no set -e), so the _v=$(grep ...) assignment failing on a missing key and the [ -n "$_v" ] && export short-circuit do not abort the tick. I also verified the tick submits jobs with no --export override (compute.py:97), so the exported .env knobs do reach climb/followup. Whether the pinned release asset and its layout match the script's assumptions cannot be checked from the repo.
…ownload (#126 R4) [terra blocking] Running `$staged --version` to verify was itself executing the unverified download on the host — a swapped release asset could run arbitrary code as the tick user and still print 0.130.0. Now the tarball's sha256 is checked against a pin (WANT_SHA256, verified to be the real 0.130.0 asset: it extracts to the same 218,718,240-byte binary the host runs) BEFORE anything is extracted or executed. The --version check remains as a secondary sanity, now running only verified bytes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… low) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Round 4 addressed:
bash -n + pre-commit clean. |
There was a problem hiding this comment.
Round 5 — reviewed head fa3cf2e1 — reviewer hermes/gpt-5.6-terra.
second opinion — terra
Advisory findings from autoresearch — the code owner decides. Reply to disagree; the autoresearch:no-review label opts this PR out.
Verdict: nothing blocking — 1 advisory note.
1 finding attached to the lines below.
There was a problem hiding this comment.
Round 5 — reviewed head fa3cf2e1 — reviewer claude/claude-opus-5.
Advisory findings from autoresearch — the code owner decides. Reply to disagree; the autoresearch:no-review label opts this PR out.
Verdict: nothing blocking — 4 advisory notes.
3 findings attached to the lines below.
Advisory (non-blocking):
- .env lines that do not match the strict pattern are dropped with no log (
scripts/tick_chain.sbatch:127; low)
The pinned SHA256 for codex 0.130.0 cannot be verified from the checkout; it must be confirmed out of band. There are no shell tests in this repo, so neither script is covered by automated tests. I checked that sbatch submissions in compute.py pass no --export, so the exported author variables do reach climb/followup jobs, and that successors are queued before the .env read, so a reverted .env takes effect on the next tick.
|
Review-clean: both reviewers nothing-blocking (terra converged from one blocking/round). All substantive findings fixed across the rounds — install dir order, curl bounds, verify-before-install, allowlist extraction instead of sourcing .env (no code execution / chain-var hijack), and a pinned SHA256 verified before extract/run (no executing an unverified download). Remaining notes are low/by-design (the allowlist silently ignoring non-author keys is the intended security property) or acknowledged repo limits (no shell tests; the SHA is an out-of-band pin, verified to be the real 0.130.0 asset). Holding the merge for the coordinated flip: merging arms the flip (the live .env has the codex vars), so it lands only after the codex key file is provisioned on Torch (currently unreachable). Order: provision |
What
Deploy plumbing so the config-driven codex author (#124, #125) can be enabled on
the live loop by editing
.env— no code change, no chain restart.Changes
scripts/install_codex.sh— idempotent install of the harness-verifiedcodex binary (pinned
0.130.0: no code-mode-host helper, no bubblewrap on thedanger-full-access path). Fast path is a local
codex --version; it only hitsthe network (
codex-x86_64-unknown-linux-musl.tar.gzfrom the pinned release)on a version mismatch or missing binary. Atomic install (temp +
mv).x86_64-only — the only pinned target.
tick_chain.sbatch— sources~/.config/autoresearch/.env(0600) eachtick, so a live config change (e.g.
AUTORESEARCH_AUTHOR_BACKEND=codex+ itskey file) needs no chain restart. Then runs
install_codex.shbest-effortonly when
AUTORESEARCH_AUTHOR_BACKEND=codex.Both are fail-safe: a missing
.envor a failed install logs and the chaincontinues on the previous state.
Enabling codex (the flip), for reference
~/.config/autoresearch/codex_key(0600) — theresolve_author_key_filedefault for codex; coexists with the claudeharness_key..env:AUTORESEARCH_AUTHOR_BACKEND=codex,AUTORESEARCH_AUTHOR_MODEL=gpt-5.6-terra..env, installs codex if needed, and authors on codex.Reverting is deleting those two
.envlines.Test plan
bash -non both scripts (syntax); pre-commit (large-files, whitespace,gitleaks) clean. No Python changed. Shell scripts aren't unit-tested in this
repo; the install is validated by running it on the host (codex 0.130.0 is
already present there, so the fast path no-ops).
No secrets / no large files
Confirmed. (The key file itself is provisioned by the operator out-of-band; this
change only references its path.)
🤖 Generated with Claude Code