Commit Graph
8 Commits
Author SHA1 Message Date
Omar SobhandClaude Opus 5 91fbd2dc88 refactor: the missions root has one definition, not five
`mission_workspace::missions_root()` is now the only place that answers "where
does mission state live". It had fragmented into five: this function, private
`env::var("CLAWMATES_MISSIONS_ROOT")` copies in security_scan, benchmark_runner
and mission_outputs, and a hardcoded `MISSIONS_HOST_ROOT` const in
mission_runtime that read no env at all.

They agree on the deployed value, so nothing has broken. The risk is entirely
in what comes next: anything that sweeps or reclaims this tree has to be
sweeping the same tree the writers use, and five definitions cannot promise
that — a reaper written against one would silently leave the others' directories
behind forever, which is how the orphans got there in the first place.

A source-walk test fails any module outside `mission_workspace` that reads the
env var itself.

Co-Authored-By: Claude Opus 5 <[email protected]>
2026-08-09 06:18:01 -07:00
Omar Sobh a8b8efba6a fix(delivery): the on_green_tests gate ran the suite in the live checkout
Fourth instance of the same defect, and the last of the three commands that run
as root against a mission tree.

`verify_tests` execs the project's test command with `workdir = repo` — the live
checkout — inside a container running as ROOT. `cargo test` writes `target/`, so
the checkout ends up owned by two uids and the next phase's cargo hits
permission-denied. The harness reported `uids=0,65532` the first time this gate
ever ran end to end.

It survived because it had never run. Every one of the ten harness fixtures used
`commit_policy: "always"`; `on_green_tests` and `on_reviewer_approval` were
parsed, implemented, and never exercised — and `Gate`'s own doc already records
that three recipes carried this policy while it "did precisely nothing" for want
of a reader. A policy that is never exercised is indistinguishable from one that
is ignored.

Consolidated rather than fixed a third time. `root_copy` now owns the pattern —
copy through `mission_fs::pack_dir` into a SIBLING of the mission dir, run there,
and purge FROM INSIDE THE CONTAINER, because the copy's `target/` is root-owned
and the server (uid 65532) cannot delete it. `benchmark_runner` moved onto it;
`evaluator_tools::Sandbox` keeps its own copy logic for now (it carries an
allow-list and a judge-facing API, so folding it in is a larger change than this
moment warrants — noted, not done).

The gate fails CLOSED if the copy cannot be made: an unverifiable suite must not
license a push.

Also adds the `refactor` scenario, which is what found this. I had written it off
as "structurally identical to four existing scenarios" — wrong: it is the only
recipe carrying `on_green_tests`, and that made it the only one testing this
code path at all.

245 lib tests, 20 test binaries.
2026-08-07 17:48:29 -07:00
Omar Sobh e89a32ffef fix(benchmark): the bench copy leaked because only root could delete it
The copy fix in a93a411 restored the checkout's single-writer invariant but
stranded the copy: 1.2 MB per run, growing forever.

`cargo bench` runs as root inside the container and writes `target/` there, so
the copy is root-owned. The server process is uid 65532; its
`remove_dir_all` cannot delete those files, and `Drop` discarded the error — so
the tree survived and nothing said so. The same "cleanup that cannot clean up"
shape as the container leak in the runtime tests, and invisible for the same
reason: a swallowed error on a path nobody reads.

`purge_copy` removes it from INSIDE the container, as root, where it was
written. Called on BOTH the success and failure paths before `Drop`, and again
before creating a copy, since a stale one from a previous run is root-owned too.
`Drop` stays as a fallback for the early-error paths where nothing ran as root
yet, and now says in its doc comment that it cannot do the real job.

Found by checking `_bench` after the uid probe went green — the invariant it
asserts was satisfied while the fix that satisfied it was leaking.

243 lib tests.
2026-08-07 17:07:36 -07:00
Omar Sobh a93a4111e1 fix(benchmark): the baseline runner was writing root-owned files into the checkout
Caught by the harness: `benchmark: checkout has multiple writers (uids=0,65532)`.
The previous full run passed that same check, so this was introduced by wiring
`benchmark_runner` into the sweep one commit ago.

`docker_exec` enters a container running as ROOT with the missions root
bind-mounted, and `cargo bench` writes `target/`. Run in the live tree it leaves
root-owned build output in a checkout owned by uid 65532 — the single-writer
invariant broken, and the next phase's cargo hitting permission-denied on a
directory it cannot write.

This is the SAME defect `evaluator_tools::Sandbox` was written for, found by the
same probe, and fixed the same way: benchmark a COPY. `BenchCopy` packs the
checkout through `mission_fs::pack_dir` (so it excludes exactly what the
delivered diff excludes — one exclusion list, now four consumers) into
`<missions_root>/_bench/<mission>`, a SIBLING of the per-mission dirs like
`_verify` and `_outputs`, so a mission reap cannot race a running bench. Removed
on drop, including on error paths.

The operator-triggered path (POST /api/missions/{id}/benchmark) had this bug
from the start and is fixed by the same change — it shares `run`.

Worth naming the pattern: measurement must not mutate what it measures. It
applies to the judge, to the `verifier` subagent that has no Edit or Write, and
now to the benchmark runner.

243 lib tests, zero warnings.
2026-08-07 17:01:36 -07:00
Omar SobhandClaude Opus 5 c812b714f4 fix(evaluator): the verification sandbox never ran a command
ci / gates (push) Failing after 5s
ci / rust (push) Skipped
ci / frontend (push) Skipped
ci / e2e (push) Skipped
ci / publish (push) Skipped
`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]>
2026-08-01 18:33:32 -07:00
Omar Sobh d8c8793c4a ci fixes: cargo fmt, eslint entities, max-lines split
ci / gates (push) Successful in 8s
ci / frontend (push) Successful in 26s
ci / rust (push) Successful in 4m25s
ci / e2e (push) Skipped
ci / publish (push) Successful in 2m46s
CI on 6ffbe97 failed on two auto-fixable gates. Both fixed:

  * cargo fmt --all — rustfmt applied across the surface touched
    by the last ~20 commits (world.rs, security_scan.rs,
    routes/{missions,nodes,terminal}.rs, fleet_herdr.rs,
    mission_workspace.rs, benchmark_runner.rs, mission_refiner.rs,
    lib.rs, tests/mission_orchestrator.rs, cm-db/repo/{missions,teams}.rs,
    bins/clawmates-node/src/main.rs)
  * eslint apostrophe escapes in HerdrSessions + MissionWizard
  * eslint max-lines: extracted EditMissionModal + RefineDiffModal
    (each ~200 LoC) into their own files. MissionCanvas drops from
    1424 to 1026, comfortably under both the 1250 eslint cap and the
    1500 CI budget.

New files:
  frontend/src/components/dashboard/EditMissionModal.tsx  (211 LoC)
  frontend/src/components/dashboard/RefineDiffModal.tsx   (208 LoC)

Verified locally: cargo fmt --check clean, cargo check clean,
mission_orchestrator test 3/3 pass, tsc + eslint --quiet both silent.
2026-07-20 12:03:43 -07:00
Omar Sobh 854a617777 task #23: retire per-team ZeroClaw container coords (Option A)
Missions never populated teams.zeroclaw_container /
teams.zeroclaw_gateway_url — those were research/loops-era columns
for long-lived per-team containers. Every mission-materialized team
runs inside the SHARED runtime as claws-as-agents provisioned via
RuntimeProvisioner. Reading zeroclaw_container on a mission row
always came up NULL, making security_scan + benchmark_runner
silently fail with "mission has no team container yet."

Changes:
  - migrations/0054_drop_teams_zeroclaw_columns.sql — DROP both
    columns.
  - cm-db/src/repo/teams.rs — delete dead helpers
    team_container_coords + set_team_container_coords.
  - cm-api/src/security_scan.rs — replace team_container_for_mission
    with exec_target(pool, mission_id): container from env
    CLAWMATES_RUNTIME_CONTAINER (default clawmates-runtime); workdir
    from env CLAWMATES_MISSIONS_ROOT + /{mission_id}/repo
    (same convention pdf_renderer uses); precondition that mission
    must have repo_id bound.
  - cm-api/src/benchmark_runner.rs — same shape.

Follow-up (not in this commit): mission_orchestrator + compose stack
still need to wire a per-mission repo checkout under
CLAWMATES_MISSIONS_ROOT before scan/bench actually produce findings.
Columns cleanup here removes the misleading silent-fail; the
missing-checkout gap is now surfaced with a clear error.

Verified: SQLX_OFFLINE=true cargo check --workspace + cargo test
-p cm-api --test mission_orchestrator both green.

Closes task #23.
2026-07-19 23:18:28 -07:00
Omar SobhandClaude Opus 4.7 f843c9ddb1 slice 7: before/after benchmark runner
ci / frontend (push) Successful in 26s
ci / publish (push) Successful in 4m52s
ci / gates (push) Successful in 4s
ci / rust (push) Successful in 4m19s
ci / e2e (push) Skipped
Executes a benchmark harness inside the mission's team container
and records the resulting metrics as a benchmark_snapshots row keyed
on (phase_id, iteration). Baseline pass (iteration=0) captures
before_metrics; each post-iteration call captures after_metrics +
computes delta vs baseline.

Rust surface:
  - cm_db::repo::missions::upsert_benchmark_snapshot / benchmark_snapshots_for
  - cm_api::benchmark_runner::{baseline, after_iteration, run}
  - Harness enum: Auto | Criterion | CargoBench | VitestBench |
    PytestBench | Shell (each with a command() vector)
  - Auto detection peeks at the repo layout inside the container
    (Cargo.toml → CargoBench, package.json → VitestBench, pyproject
    → PytestBench). Falls back to a Shell echo when nothing
    identifiable.
  - Bencher-format line parser extracts (name, ns_per_iter,
    plusminus) so criterion + `cargo bench` output become structured
    samples the canvas can diff.
  - compute_delta pairs samples by name, emits {before_ns, after_ns,
    delta_pct, direction: improved|regressed}.

API:
  - POST /api/missions/{id}/benchmark { phase_id, slot, iteration? }
    triggers baseline or after run and returns the mission's full
    snapshot list.
  - GET /api/missions/{id} now includes `benchmarks[]` in the detail
    payload.

Frontend:
  - New Benchmarks tab on MissionCanvas with iteration + driver
    header, plus a 4-column grid (bench / before / after / Δ%) when
    delta samples are present. Improved deltas render green,
    regressions red.
  - TS types + triggerBenchmark() helper in lib/api/missions.ts.

Wiring notes:
  - team_container_for_mission reads teams.zeroclaw_container — that's
    populated by topology_worker::try_team_gateway_url on first run,
    so trigger baseline AFTER the mission's first phase spawns the
    container.
  - Not auto-fired yet by phase execution; that's the "template phase
    executor" work that spans Slices 4-8. Manual API trigger works
    today; automated hook is a follow-up.

Co-Authored-By: Claude Opus 4.7 <[email protected]>
2026-07-19 15:32:47 -07:00