Fix sandbox classifier null contributor email handling #10
Loading…
Reference in a new issue
No description provided.
Delete branch "claude/wi-019e535b-fix-sandbox-classifier-allow-null-contri"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Implements Anima WI
019e535b-0ef4-7b70-bc8e-ab4c84f8f6acon current main.BodyVerdict::NewContributornow accepts JSONnullforcontributor_email. Routing skips only the person upsert when email is absent and preserves the remaining Seshat action. The sandbox prompt advertises the nullable field.Regression coverage parses an actual JSON null and verifies routing retains non-person actions. The canonical branch history is preserved additively; the resulting PR diff contains only the four focused source/test files.
Validation:
cargo test -p sia-core new_contributor_accepts_null_emailcargo test -p sia-mail new_contributor_without_email_keeps_non_person_actionscargo test -p sia-mail new_contributor_with_99347d465e23eb41587823eb415878b35a987e6bb35a987e6ba13a37010da13a37010d16d9c270ca16d9c270ca3f56ed24ae3f56ed24ae24262f286024262f286061e5cd4a9261e5cd4a924d87c219cb4d87c219cbd098d1866cWIP: Fix sandbox classifier: allow null contributor_email in BodyVerdictto Fix sandbox classifier null contributor email handling`get_project_ci_status` reported `resolvable: false` for Sia — no `default_ci_steps` and no `.solstice/job.sh` — so every native dispatch failed at config resolution before running anything. That failure is not neutral. anima-server's solstice.rs routes it to `handle_config_failure`, which parks the work item with `ParkedReason::CiConfigUnresolvable`. The ABSENCE of this file reads as "this work item's CI is broken" and parks real work. akh-medu learned this the expensive way (8/8 native dispatches failed, the WI 1/2/3 series parked) and its job.sh states the rule: a missing file must not read as "the code is red". Sia had CI only on the Forgejo-Actions lane, so the native lane — the one Anima's exact-head gate reads — had never produced a verdict at all. One definition of the checks, two lanes --------------------------------------- scripts/ci-checks.sh is now the only place the cargo commands are written. Both .solstice/job.sh and .forgejo/workflows/ci.yml call it; neither re-lists them. akh-medu's rule is that a native verdict must never be red for work the Actions lane would call green, because the disagreement parks work items on phantom failures. Two inline copies drift the first time someone edits one; sharing the script makes that structurally impossible rather than a reviewer's responsibility. job.sh only provisions. It deliberately differs from the Anima/akh-medu scripts in two places: - It does not assume passwordless sudo. Those scripts call `sudo apt-get` unconditionally because their image grants it; a runner without sudo would fail with a bare "sudo: command not found" that classifies as a code failure and parks the item. Privilege is resolved once, only missing packages are installed, and the no-privilege case exits with a message naming the packages and the reason. - It does not `rm -rf ../anima`. The workflow did, and on a persistent or shared runner that deletes a checkout the job does not own. An existing sibling is reused; a non-git path there is refused rather than removed. The resolved revision is always logged so an upstream anima-proto change is attributable. ANIMA_SIBLING_REV pins a run without freezing CI to a revision the shipped image does not use. protoc keeps ci.yml's pinned 25.3 release, since Ubuntu's 3.12 rejects anima-proto's proto3 `optional` fields, and installs into the checkout so it needs no privileges. The build is debug: release codegen is what OOM-killed rustc on the Solstice VMs for akh-medu, and an OOM reads as red on healthy code. Sia's release build is still covered by the `docker` job on push to main. Validation ---------- scripts/test-ci-job.sh replaces every external command with a logging stub and asserts on the control flow — the branches that only execute on a runner in an unusual state, i.e. never until the day they matter. 10 scenarios, 36 assertions, all passing; it needs no toolchain and runs in seconds, so it is wired into ci-checks.sh and guards the CI definition on both lanes. It earned its keep immediately, catching three defects before any runner saw the script: `"${profile_args[@]}"` on an empty array is an unbound-variable error under `set -u` on bash < 4.4, so the script would have worked on the runner but not locally, defeating the point of sharing it; the openssl probe hardcoded /usr/include/openssl/ssl.h and reported a false "missing" on any other layout, triggering a pointless privileged install; and the harness's own no-sudo case was inheriting the host's cached sudo credentials, leaving the no-privilege branch untested exactly where it was most likely to be wrong. No application change: no Cargo.toml edit, no dependency version change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>e5ead22All four were real. Reproduced each before fixing. 1. The install died as root. `$SUDO DEBIAN_FRONTEND=noninteractive apt-get` looks like an assignment prefix but is not one: bash decides that from the text BEFORE expansion, and the first word is `$SUDO`. Once that expands to nothing — i.e. running as root, the case that should always work — the assignment becomes the command name: $ SUDO=""; $SUDO DEMO_ONLY=1 true bash: DEMO_ONLY=1: command not found (rc=127) Routed through `env`, which is a real command in both shapes. The harness now exercises root explicitly via an `id` stub rather than only covering sudo/non-root, so the escalation-free path has real coverage. 2. A git worktree's `.git` is a FILE holding a gitdir pointer, so `[ -d .git ]` rejected a perfectly valid shared worktree — and the next branch then refused to run at all. This checkout is itself a worktree, so the script would have rejected the very tree it was written in. Now asks git (`rev-parse --is-inside-work-tree`) and confirms the path is that worktree's ROOT by comparing realpaths, so a subdirectory of an unrelated repo above it is still refused — non-destructively. 3. ANIMA_SIBLING_REV claimed a pin it never enforced: it was only applied on the clone path, so with an existing checkout the requested revision was silently ignored and the verdict was attributed to a revision that was never built. Now validated against the existing checkout and REFUSED on mismatch, without touching a checkout this job does not own. An unknown rev is likewise refused rather than fetched into it. ANIMA_SIBLING_DIR is gone. It was misleading: cargo reads the override path from the workspace [patch] table, so pointing the variable elsewhere provisioned a directory cargo would never look at while the build still failed on the real path. The sibling location is now derived from Cargo.toml, which is the only thing that actually decides it, and a missing [patch] path fails loudly instead of guessing. 4. `clippy --workspace --all-targets -- -D warnings` was stricter than the ci.yml it replaced (libs/bins only). Pulling tests, benches and examples into a -D warnings gate for the first time risks an unrelated baseline failure landing as a red verdict on unrelated work — the phantom-failure park this work item exists to stop. Restored to the previously-verified scope. Widening is a separate change that needs a clean baseline run and then moves both lanes together. Also fixes unbounded recursion introduced when the harness was wired into ci-checks.sh: the harness runs job.sh, which execs ci-checks.sh, which ran the harness. Guarded with CI_SKIP_JOB_HARNESS, which the harness sets for its children, and the guard itself is now covered. Harness: 18 scenarios, 62 assertions, all passing. New fixtures are real, not mocked around — an actual shared worktree created with `git worktree add`, an actual non-root checkout, actual commit SHAs for the pin cases, and a real Cargo.toml with a [patch] table so the derivation runs the same code path the runner takes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.