fix: close the three remaining gaps, and repair a test I silently disabled
FIRST, the self-inflicted one. My edit ina20702dinserted a test between an existing `#[test]` and the function it belonged to. The result compiled and looked fine: `every_anthropic_spelling_is_one_family` lost its attribute and STOPPED BEING A TEST, its doc comment ended up describing my test instead, and my test carried two `#[test]`s. It has not run since — in already-deployed code. Nothing failed, which is the point: a test that does not run is indistinguishable from one that passes. Found via a compiler warning I had not read. The commit message ona20702dsaid "241 lib tests pass". 240 ran. Then the three gaps. 1. A security scan could not read history. `ensure_checkout` clones with `--filter=blob:none` — full commits, blobs on demand — and the agent environment has NO network route to the forge. Measured: gitleaks on a 4-commit repo reported "1 commits scanned" and "could not fetch <sha> from promisor remote". A credential committed and later deleted is exactly what a scanner looks for and exactly what a lazy blob withholds. Missions with a `security_scan` phase now clone fully; everything else keeps the cheap path. (I first blamed `--depth 1`, from a stale module doc comment. The code has said `--filter=blob:none` since it was written, and the comment at `clone` explains why NOT shallow — a shallow clone cannot push a branch back. Both the comment and my claim are fixed.) 2. `benchmark_runner` never ran as part of a benchmark phase. It was reachable only from an operator button, so the `author_and_baseline` recipe authored benchmarks and measured nothing — `benchmark_snapshots` stayed empty. Now baselined from the sweep, SPAWNED not awaited: BENCH_TIMEOUT is 30 minutes and that loop also starts, closes, evaluates and captures every phase on the platform. A `NOT EXISTS` guard on iteration 0 makes per-tick firing safe. A repo with no bench harness logs and does NOT fail the phase — but it logs, because "no baseline" must not read like "not attempted". 3. The World's rich layer was empty for every mission. `run_events::append` is called only from the a2a path, and `world.rs` tailed only that table — while mission per-step detail has always lived in `topology_runs.checkpoint.records`, which `topology::run_events_sse` streams. The data was never missing; the viz read the one source missions never write. Now both are tailed, mapped through the existing `step_started` vocabulary so no new event types are needed. 242 lib tests, 20 test binaries, zero warnings.
This commit is contained in:
@@ -812,9 +812,6 @@ pub async fn latest(
|
|||||||
mod cross_provider_tests {
|
mod cross_provider_tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
/// A family, not a model. Two Claude models share a lineage and most of their
|
|
||||||
/// failure modes, so `opus` judging `sonnet` is not an independent check.
|
|
||||||
#[test]
|
|
||||||
/// A bare model name must never be accepted as a validator spec.
|
/// A bare model name must never be accepted as a validator spec.
|
||||||
///
|
///
|
||||||
/// `resolve_provider` falls back to the DEFAULT provider for anything it
|
/// `resolve_provider` falls back to the DEFAULT provider for anything it
|
||||||
@@ -836,6 +833,9 @@ mod cross_provider_tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A family, not a model. Two Claude models share a lineage and most of their
|
||||||
|
/// failure modes, so `opus` judging `sonnet` is not an independent check.
|
||||||
|
#[test]
|
||||||
fn every_anthropic_spelling_is_one_family() {
|
fn every_anthropic_spelling_is_one_family() {
|
||||||
for spec in [
|
for spec in [
|
||||||
"claude-opus-4-8",
|
"claude-opus-4-8",
|
||||||
|
|||||||
@@ -11,7 +11,10 @@
|
|||||||
//! - no repo_id → no-op (Ok(None))
|
//! - no repo_id → no-op (Ok(None))
|
||||||
//! - dir already a git repo → `fetch + reset --hard origin/<branch>`
|
//! - dir already a git repo → `fetch + reset --hard origin/<branch>`
|
||||||
//! to bring it in sync
|
//! to bring it in sync
|
||||||
//! - dir missing → `git clone --depth 1 <url> <path>`
|
//! - dir missing → `git clone --filter=blob:none --single-branch` (NOT
|
||||||
|
//! `--depth 1`: a shallow clone cannot push a new branch back, and delivery
|
||||||
|
//! needs exactly that — see `clone`). A mission with a `security_scan` phase
|
||||||
|
//! gets a FULLY HYDRATED clone instead; see `wants_full_history`.
|
||||||
//!
|
//!
|
||||||
//! Auth: for `git.redclaw.dev` clones we inject the ambient
|
//! Auth: for `git.redclaw.dev` clones we inject the ambient
|
||||||
//! `GITEA_TOKEN` (already provisioned in the server container's env)
|
//! `GITEA_TOKEN` (already provisioned in the server container's env)
|
||||||
@@ -99,7 +102,7 @@ pub async fn ensure_checkout(
|
|||||||
fetch_and_reset(&path, default_branch, &auth_url).await?;
|
fetch_and_reset(&path, default_branch, &auth_url).await?;
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
clone(&path, &auth_url).await?;
|
clone(&path, &auth_url, wants_full_history(pool, mission_id).await).await?;
|
||||||
}
|
}
|
||||||
Ok(Some(path))
|
Ok(Some(path))
|
||||||
}
|
}
|
||||||
@@ -220,21 +223,59 @@ pub(crate) fn no_terminal_prompt(cmd: &mut Command) -> &mut Command {
|
|||||||
cmd.env("GIT_TERMINAL_PROMPT", "0")
|
cmd.env("GIT_TERMINAL_PROMPT", "0")
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn clone(path: &std::path::Path, url: &str) -> Result<(), String> {
|
/// Does any phase of this mission need history it can READ, not just reference?
|
||||||
|
///
|
||||||
|
/// `--filter=blob:none` keeps every commit but fetches file contents on demand,
|
||||||
|
/// which is nearly free for a repo that gets read once — and silently useless to
|
||||||
|
/// a tool that walks history, because the agent environment has NO network route
|
||||||
|
/// to the forge. Measured: gitleaks on a 4-commit repo reported
|
||||||
|
/// "1 commits scanned" and "could not fetch <sha> from promisor remote". It was
|
||||||
|
/// not misconfigured; the blobs simply were not there and could not be got.
|
||||||
|
///
|
||||||
|
/// A security scan is the phase kind whose entire value is old content — a
|
||||||
|
/// credential committed and later deleted is exactly what it looks for, and that
|
||||||
|
/// is precisely what a lazy blob is. So those missions pay for a full clone and
|
||||||
|
/// everything else keeps the cheap one.
|
||||||
|
///
|
||||||
|
/// Best-effort: an unreadable phase list yields `false`, i.e. today's behaviour.
|
||||||
|
async fn wants_full_history(pool: &sqlx::PgPool, mission_id: Uuid) -> bool {
|
||||||
|
sqlx::query_scalar::<_, i64>(
|
||||||
|
"SELECT count(*) FROM mission_phases WHERE mission_id = $1 AND kind = 'security_scan'",
|
||||||
|
)
|
||||||
|
.bind(mission_id)
|
||||||
|
.fetch_one(pool)
|
||||||
|
.await
|
||||||
|
.map(|n| n > 0)
|
||||||
|
.unwrap_or(false)
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The `git clone` flags, split out so the strategy is testable without a forge.
|
||||||
|
fn clone_args(full_history: bool) -> Vec<&'static str> {
|
||||||
|
let mut a = vec!["clone"];
|
||||||
|
if !full_history {
|
||||||
|
a.push("--filter=blob:none");
|
||||||
|
}
|
||||||
|
a.push("--single-branch");
|
||||||
|
a
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn clone(path: &std::path::Path, url: &str, full_history: bool) -> Result<(), String> {
|
||||||
// `--filter=blob:none` rather than `--depth 1`. A shallow clone cannot
|
// `--filter=blob:none` rather than `--depth 1`. A shallow clone cannot
|
||||||
// usually push a new branch back ("shallow update not allowed"), and
|
// usually push a new branch back ("shallow update not allowed"), and
|
||||||
// mission delivery needs exactly that. A partial clone keeps full history
|
// mission delivery needs exactly that. A partial clone keeps full history
|
||||||
// — so the base commit stays meaningful and a diff has something to be
|
// — so the base commit stays meaningful and a diff has something to be
|
||||||
// relative to — while fetching file contents only on demand, which is
|
// relative to — while fetching file contents only on demand, which is
|
||||||
// nearly as cheap as a shallow clone for a repo that gets read once.
|
// nearly as cheap as a shallow clone for a repo that gets read once.
|
||||||
|
if full_history {
|
||||||
|
eprintln!(
|
||||||
|
"mission_workspace: cloning {} with full history — a security_scan phase \
|
||||||
|
reads old file contents, which a partial clone cannot supply offline",
|
||||||
|
path.display()
|
||||||
|
);
|
||||||
|
}
|
||||||
let mut cmd = Command::new("git");
|
let mut cmd = Command::new("git");
|
||||||
cmd.args([
|
cmd.args(clone_args(full_history));
|
||||||
"clone",
|
cmd.args([url, &path.display().to_string()]);
|
||||||
"--filter=blob:none",
|
|
||||||
"--single-branch",
|
|
||||||
url,
|
|
||||||
&path.display().to_string(),
|
|
||||||
]);
|
|
||||||
let out = no_terminal_prompt(&mut cmd)
|
let out = no_terminal_prompt(&mut cmd)
|
||||||
.output()
|
.output()
|
||||||
.await
|
.await
|
||||||
@@ -718,6 +759,28 @@ async fn fetch_and_reset(
|
|||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
|
/// The clone strategy is a fact worth pinning: `--depth 1` breaks delivery
|
||||||
|
/// (a shallow clone cannot push a new branch — "shallow update not
|
||||||
|
/// allowed"), and `--filter=blob:none` breaks history-reading tools offline.
|
||||||
|
/// Both failure modes are real and were both hit.
|
||||||
|
#[test]
|
||||||
|
fn the_clone_strategy_is_partial_by_default_and_never_shallow() {
|
||||||
|
let partial = clone_args(false);
|
||||||
|
assert!(partial.contains(&"--filter=blob:none"), "{partial:?}");
|
||||||
|
assert!(!partial.iter().any(|a| a.starts_with("--depth")), "{partial:?}");
|
||||||
|
|
||||||
|
// A security_scan mission must NOT get the lazy-blob filter: its scanner
|
||||||
|
// walks old file contents and cannot reach the forge to fetch them.
|
||||||
|
let full = clone_args(true);
|
||||||
|
assert!(!full.contains(&"--filter=blob:none"), "{full:?}");
|
||||||
|
assert!(!full.iter().any(|a| a.starts_with("--depth")), "{full:?}");
|
||||||
|
|
||||||
|
// Both keep --single-branch: the mission only ever works one branch.
|
||||||
|
for args in [partial, full] {
|
||||||
|
assert!(args.contains(&"--single-branch"), "{args:?}");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
use super::*;
|
use super::*;
|
||||||
|
|
||||||
const TOK: Option<&str> = Some("secret123");
|
const TOK: Option<&str> = Some("secret123");
|
||||||
|
|||||||
@@ -71,6 +71,8 @@ async fn sweep_once(
|
|||||||
// The same, for missions with no repository to diff. Without this the
|
// The same, for missions with no repository to diff. Without this the
|
||||||
// container holding a research phase's only output is reaped unread.
|
// container holding a research phase's only output is reaped unread.
|
||||||
crate::mission_outputs::capture_repo_less_phases(pool).await?;
|
crate::mission_outputs::capture_repo_less_phases(pool).await?;
|
||||||
|
// Benchmark phases record their baseline once the work exists to measure.
|
||||||
|
baseline_finished_benchmark_phases(pool).await?;
|
||||||
// A failed phase makes every later phase unreachable, and saying so is what
|
// A failed phase makes every later phase unreachable, and saying so is what
|
||||||
// lets the mission finish at all.
|
// lets the mission finish at all.
|
||||||
skip_unreachable_phases(pool).await?;
|
skip_unreachable_phases(pool).await?;
|
||||||
@@ -253,6 +255,64 @@ mod repo_less_text_tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Record the baseline for benchmark phases that have finished and have none.
|
||||||
|
///
|
||||||
|
/// `benchmark_runner` has existed since Slice 7 and was reachable only from
|
||||||
|
/// `POST /api/missions/{id}/benchmark` — an operator button. So a `benchmark`
|
||||||
|
/// mission, whose recipe declares `mode = "author_and_baseline"`, authored
|
||||||
|
/// benchmarks and then recorded nothing: `benchmark_snapshots` stayed empty and
|
||||||
|
/// the canvas had nothing to render. The measuring half of "author + baseline"
|
||||||
|
/// simply never ran.
|
||||||
|
///
|
||||||
|
/// Spawned, never awaited in the sweep: `BENCH_TIMEOUT` is 30 minutes, and this
|
||||||
|
/// loop also starts phases, closes them, evaluates and captures. Blocking it on
|
||||||
|
/// a benchmark would stall every mission on the platform behind one `cargo
|
||||||
|
/// bench`.
|
||||||
|
///
|
||||||
|
/// The `NOT EXISTS` guard is what makes that safe to fire per tick: a phase with
|
||||||
|
/// a snapshot is never selected again, so a spawned run cannot be started twice
|
||||||
|
/// while the first is still going.
|
||||||
|
async fn baseline_finished_benchmark_phases(pool: &PgPool) -> Result<(), String> {
|
||||||
|
let rows = sqlx::query(
|
||||||
|
"SELECT mp.id, mp.mission_id
|
||||||
|
FROM mission_phases mp
|
||||||
|
WHERE mp.kind = 'benchmark'
|
||||||
|
AND mp.status IN ('completed', 'failed')
|
||||||
|
AND NOT EXISTS (
|
||||||
|
SELECT 1 FROM benchmark_snapshots b
|
||||||
|
WHERE b.phase_id = mp.id AND b.iteration = 0
|
||||||
|
)
|
||||||
|
ORDER BY mp.completed_at DESC NULLS LAST
|
||||||
|
LIMIT 2",
|
||||||
|
)
|
||||||
|
.fetch_all(pool)
|
||||||
|
.await
|
||||||
|
.map_err(|e| format!("select benchmark phases: {e}"))?;
|
||||||
|
|
||||||
|
for row in rows {
|
||||||
|
let phase_id: Uuid = row.get("id");
|
||||||
|
let mission_id: Uuid = row.get("mission_id");
|
||||||
|
let pool = pool.clone();
|
||||||
|
tokio::spawn(async move {
|
||||||
|
match crate::benchmark_runner::baseline(&pool, mission_id, phase_id).await {
|
||||||
|
Ok(()) => eprintln!(
|
||||||
|
"phase_runner: recorded benchmark baseline for phase {phase_id} \
|
||||||
|
of mission {mission_id}"
|
||||||
|
),
|
||||||
|
// Not a phase failure. A repo with no bench harness is a normal
|
||||||
|
// outcome, and failing the phase for it would punish a mission
|
||||||
|
// that did exactly what it was asked. Said out loud, though —
|
||||||
|
// "no baseline" must not be indistinguishable from "not tried".
|
||||||
|
Err(e) => eprintln!(
|
||||||
|
"phase_runner: no benchmark baseline for phase {phase_id} \
|
||||||
|
of mission {mission_id}: {e}"
|
||||||
|
),
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
|
Ok(())
|
||||||
|
}
|
||||||
|
|
||||||
/// Did this phase finish without delivering the work it exists to produce?
|
/// Did this phase finish without delivering the work it exists to produce?
|
||||||
///
|
///
|
||||||
/// A coding phase that changes no files has done nothing, and until now that
|
/// A coding phase that changes no files has done nothing, and until now that
|
||||||
|
|||||||
@@ -341,6 +341,11 @@ pub async fn world_live(State(state): State<AppState>, Authed(user): Authed) ->
|
|||||||
let mut last: std::collections::HashMap<String, String> = std::collections::HashMap::new();
|
let mut last: std::collections::HashMap<String, String> = std::collections::HashMap::new();
|
||||||
// Per-run journal cursor so we stream only NEW run_events each poll.
|
// Per-run journal cursor so we stream only NEW run_events each poll.
|
||||||
let mut cursors: std::collections::HashMap<String, i64> = std::collections::HashMap::new();
|
let mut cursors: std::collections::HashMap<String, i64> = std::collections::HashMap::new();
|
||||||
|
// How many checkpoint step-records of each run have already been sent.
|
||||||
|
// Separate from `cursors`: that tracks `run_events.seq`, and the two
|
||||||
|
// sources are populated by different paths — see the loop below.
|
||||||
|
let mut step_cursors: std::collections::HashMap<String, usize> =
|
||||||
|
std::collections::HashMap::new();
|
||||||
// Audit-log cursor for edge-initiated inter-agent events (delegation,
|
// Audit-log cursor for edge-initiated inter-agent events (delegation,
|
||||||
// A2A) that bypass the run loop. -1 until seeded on the first pass.
|
// A2A) that bypass the run loop. -1 until seeded on the first pass.
|
||||||
let mut audit_cursor: i64 = -1;
|
let mut audit_cursor: i64 = -1;
|
||||||
@@ -426,6 +431,54 @@ pub async fn world_live(State(state): State<AppState>, Authed(user): Authed) ->
|
|||||||
// Tail the run's journal for richer real events (reasoning, tool
|
// Tail the run's journal for richer real events (reasoning, tool
|
||||||
// convergence, doors). On first sight, jump the cursor to the
|
// convergence, doors). On first sight, jump the cursor to the
|
||||||
// current max so we stream forward without replaying the backlog.
|
// current max so we stream forward without replaying the backlog.
|
||||||
|
// Mission runs write NO `run_events`: `run_events::append` is called
|
||||||
|
// only from the a2a path. Their per-step detail lives in
|
||||||
|
// `topology_runs.checkpoint.records`, which is what
|
||||||
|
// `topology::run_events_sse` has always streamed. So the World's
|
||||||
|
// rich layer was empty for every mission on the platform — not
|
||||||
|
// because the data was missing, but because this read the one
|
||||||
|
// source missions never write.
|
||||||
|
//
|
||||||
|
// Both sources are tailed, because both are real: a2a runs
|
||||||
|
// populate the table, topology runs populate the checkpoint.
|
||||||
|
let steps: Vec<Value> = sqlx::query_scalar::<_, Option<Value>>(
|
||||||
|
"SELECT checkpoint FROM topology_runs WHERE id = $1::uuid",
|
||||||
|
)
|
||||||
|
.bind(run_id)
|
||||||
|
.fetch_optional(&pool)
|
||||||
|
.await
|
||||||
|
.ok()
|
||||||
|
.flatten()
|
||||||
|
.flatten()
|
||||||
|
.and_then(|c| c.get("records").cloned())
|
||||||
|
.and_then(|r| r.as_array().cloned())
|
||||||
|
.unwrap_or_default();
|
||||||
|
let already = *step_cursors.get(run_id).unwrap_or(&0);
|
||||||
|
if already < steps.len() {
|
||||||
|
for rec in &steps[already..] {
|
||||||
|
// A node turn IS a step. `step_started` is what
|
||||||
|
// `normalize_run_event` already maps to a tool call, so
|
||||||
|
// the viz needs no new event vocabulary.
|
||||||
|
let role = rec
|
||||||
|
.get("role")
|
||||||
|
.or_else(|| rec.get("node_id"))
|
||||||
|
.and_then(|v| v.as_str())
|
||||||
|
.unwrap_or("turn");
|
||||||
|
for (t, d) in normalize_run_event(
|
||||||
|
agent_id,
|
||||||
|
"step_started",
|
||||||
|
&json!({ "tool": role, "input": rec.get("output").cloned().unwrap_or(Value::Null) }),
|
||||||
|
) {
|
||||||
|
yield sse(t, d);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
step_cursors.insert(run_id.clone(), steps.len());
|
||||||
|
} else if !step_cursors.contains_key(run_id) {
|
||||||
|
// First sight: jump to the end rather than replaying the
|
||||||
|
// backlog, matching how the run_events cursor below behaves.
|
||||||
|
step_cursors.insert(run_id.clone(), steps.len());
|
||||||
|
}
|
||||||
|
|
||||||
if let Some(&after) = cursors.get(run_id) {
|
if let Some(&after) = cursors.get(run_id) {
|
||||||
let rows = sqlx::query(
|
let rows = sqlx::query(
|
||||||
"SELECT seq, event_type, payload FROM run_events
|
"SELECT seq, event_type, payload FROM run_events
|
||||||
|
|||||||
Reference in New Issue
Block a user