fix(missions): the security scan phase now scans, and task upserts work
Four defects, found by checking the audit's claims instead of trusting them. Two of the audit's own findings turned out to be wrong, and the registry that exists to record which config keys are read was itself inaccurate — so the corrections are part of the change. upsert_task raised 42P10 on every call, for every caller `mission_tasks_external_uniq` is a PARTIAL unique index (WHERE external_id IS NOT NULL). Postgres will not match a partial index to an ON CONFLICT target unless the statement repeats the predicate, so the upsert failed on its first row. Both callers — the task-card parser that turns INT markers into tasks, and the security scanner — map the error to a string their caller logs. Two features were broken and nothing was red. Regression test in cm-db with a negative control: reverting the WHERE reproduces 42P10 exactly. the security scan never ran `security_scan::run` was reachable only from an operator button, so security_hardening.toml — a workflow whose entire first phase is a scan — ran an agent that was never told to scan and never fired the scanner either. phase_runner now sweeps finished security_scan phases, mirroring the benchmark baseline sweep that was added for the identical defect. Guarded on a new completion marker rather than on findings: a clean scan writes no findings, so a findings-guard would rescan forever. The marker also answers the question an operator actually asks, which is not "how many findings" but "was this looked at, by what, and when". two recipes could not fail security_hardening.toml and benchmark.toml carried no `task` and no `done_when` on any phase. A phase without done_when never enters evaluating, is never judged, and reports completed whatever it did — so a security mission could scan nothing and go green, and a benchmark mission could record no baseline that the next refactor would then compare against. Both now state the work and the condition, with inert keys annotated inline rather than deleted, so the gap between what a recipe asks for and what a phase receives stays visible. the config registry was wrong in both directions `harness` was listed NOT IMPLEMENTED while benchmark_runner reads it and phase_runner runs a baseline through it. `tools` was listed NOT IMPLEMENTED while security_scan::run reads it. A registry that exists so an operator can trust what a recipe does is worse than useless when it is inaccurate. Both corrected, `bench_name` and `cmd` added, and `test_command` deleted — it had neither a reader nor a writer, so it described a situation that could not arise. Also: CLAWMATES_JUDGE_MODEL had two different defaults (opus-4-8 in routes/topology.rs vs opus-5 in cm_runtime::judge_model) and a doc comment naming a third; topology now calls the one function. GITEA_TOKEN's absence in mission_plan is stated rather than degrading to the same "could not be read" string a private repo produces. BRAINHUB_API_KEY needed no change — hub::push already rejects an unset key with a named error. That half of the finding was overstated. Co-Authored-By: Claude Opus 5 <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
4358964c05
commit
18dc0b964b
@@ -53,6 +53,33 @@ pub const KNOWN_KEYS: &[KnownKey] = &[
|
|||||||
phase that changes no files still completes; also vm_stop_gate::\
|
phase that changes no files still completes; also vm_stop_gate::\
|
||||||
StopGate::for_phase, where it drops the in-loop delivery check",
|
StopGate::for_phase, where it drops the in-loop delivery check",
|
||||||
},
|
},
|
||||||
|
KnownKey {
|
||||||
|
key: "tools",
|
||||||
|
read_by: "security_scan::run — gates which of cargo_audit / gitleaks / \
|
||||||
|
trivy_fs / semgrep run against the phase's checkout; absent \
|
||||||
|
means all four. Listed here as NOT IMPLEMENTED while wired, \
|
||||||
|
which understated the recipe: the key was real, what was \
|
||||||
|
missing was anything that FIRED the scan outside an operator \
|
||||||
|
button — now phase_runner::scan_finished_security_phases",
|
||||||
|
},
|
||||||
|
KnownKey {
|
||||||
|
key: "harness",
|
||||||
|
read_by: "benchmark_runner::harness_from_config — selects criterion / \
|
||||||
|
cargo_bench / vitest_bench / pytest_bench / shell, with \
|
||||||
|
`bench_name` (criterion) and `cmd` (shell) as its arguments. \
|
||||||
|
phase_runner's benchmark sweep runs the baseline through it. \
|
||||||
|
This key was listed as NOT IMPLEMENTED while being fully \
|
||||||
|
wired, which is worse than an unread key: the registry exists \
|
||||||
|
so an operator can trust what a recipe does, and it was wrong",
|
||||||
|
},
|
||||||
|
KnownKey {
|
||||||
|
key: "bench_name",
|
||||||
|
read_by: "benchmark_runner::harness_from_config — the criterion bench target",
|
||||||
|
},
|
||||||
|
KnownKey {
|
||||||
|
key: "cmd",
|
||||||
|
read_by: "benchmark_runner::harness_from_config — the shell harness command line",
|
||||||
|
},
|
||||||
KnownKey {
|
KnownKey {
|
||||||
key: "done_when_check",
|
key: "done_when_check",
|
||||||
read_by: "vm_stop_gate::StopGate::for_phase — a shell command the agent's \
|
read_by: "vm_stop_gate::StopGate::for_phase — a shell command the agent's \
|
||||||
@@ -84,14 +111,6 @@ pub const DECLARED_BUT_UNREAD: &[KnownKey] = &[
|
|||||||
key: "mode",
|
key: "mode",
|
||||||
read_by: "NOT IMPLEMENTED — benchmark/refactor mode selection",
|
read_by: "NOT IMPLEMENTED — benchmark/refactor mode selection",
|
||||||
},
|
},
|
||||||
KnownKey {
|
|
||||||
key: "harness",
|
|
||||||
read_by: "NOT IMPLEMENTED — benchmark harness selection",
|
|
||||||
},
|
|
||||||
KnownKey {
|
|
||||||
key: "tools",
|
|
||||||
read_by: "NOT IMPLEMENTED — per-phase tool selection",
|
|
||||||
},
|
|
||||||
KnownKey {
|
KnownKey {
|
||||||
key: "benchmark",
|
key: "benchmark",
|
||||||
read_by: "NOT IMPLEMENTED — nested benchmark settings",
|
read_by: "NOT IMPLEMENTED — nested benchmark settings",
|
||||||
@@ -104,11 +123,6 @@ pub const DECLARED_BUT_UNREAD: &[KnownKey] = &[
|
|||||||
setting this per phase changes nothing: security_hardening.toml \
|
setting this per phase changes nothing: security_hardening.toml \
|
||||||
asks for gitea_forge + security_scan and its phase gets neither",
|
asks for gitea_forge + security_scan and its phase gets neither",
|
||||||
},
|
},
|
||||||
KnownKey {
|
|
||||||
key: "test_command",
|
|
||||||
read_by: "NOT IMPLEMENTED — mission_delivery::discover_test_command infers \
|
|
||||||
from the repo and does not consult config",
|
|
||||||
},
|
|
||||||
];
|
];
|
||||||
|
|
||||||
fn is_listed(key: &str, list: &[KnownKey]) -> bool {
|
fn is_listed(key: &str, list: &[KnownKey]) -> bool {
|
||||||
|
|||||||
@@ -73,6 +73,8 @@ async fn sweep_once(
|
|||||||
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.
|
// Benchmark phases record their baseline once the work exists to measure.
|
||||||
baseline_finished_benchmark_phases(pool).await?;
|
baseline_finished_benchmark_phases(pool).await?;
|
||||||
|
// Security phases run the scanners once the checkout exists to scan.
|
||||||
|
scan_finished_security_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?;
|
||||||
@@ -352,6 +354,74 @@ async fn baseline_finished_benchmark_phases(pool: &PgPool) -> Result<(), String>
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Run the scanners for `security_scan` phases that have finished without one.
|
||||||
|
///
|
||||||
|
/// The same defect the benchmark sweep above was written for, in the other
|
||||||
|
/// half of the platform. `security_scan::run` — which reads `phase.config.tools`
|
||||||
|
/// to pick scanners and upserts each finding as a `mission_task` — was
|
||||||
|
/// reachable only from `POST /api/missions/{id}/security-scan`, an operator
|
||||||
|
/// button. So `security_hardening.toml`, a workflow whose entire first phase is
|
||||||
|
/// a scan, ran an agent that was never told to scan and then never fired the
|
||||||
|
/// scanner either. The phase reported `completed` having scanned nothing, and
|
||||||
|
/// the two facts that would have exposed it — zero findings, and a green
|
||||||
|
/// phase — are exactly what a genuinely clean repository also looks like.
|
||||||
|
///
|
||||||
|
/// Guarded on the marker row rather than on the presence of findings, because
|
||||||
|
/// a clean scan writes no findings: without it, a clean phase would be
|
||||||
|
/// rescanned on every tick for as long as the mission existed.
|
||||||
|
///
|
||||||
|
/// Spawned, not awaited, for the same reason as the benchmark baseline: four
|
||||||
|
/// scanners against a large tree take minutes, and this loop also starts,
|
||||||
|
/// closes and evaluates every phase on the platform.
|
||||||
|
/// The selection half of the sweep, split out so the marker guard is testable
|
||||||
|
/// without a container: the scan itself needs Docker, the guard is the part
|
||||||
|
/// that decides whether it runs twice or never.
|
||||||
|
pub async fn unscanned_security_phases(pool: &PgPool) -> Result<Vec<(Uuid, Uuid)>, String> {
|
||||||
|
let rows = sqlx::query(
|
||||||
|
"SELECT mp.id, mp.mission_id
|
||||||
|
FROM mission_phases mp
|
||||||
|
WHERE mp.kind = 'security_scan'
|
||||||
|
AND mp.status IN ('completed', 'failed')
|
||||||
|
AND NOT EXISTS (
|
||||||
|
SELECT 1 FROM mission_tasks t
|
||||||
|
WHERE t.phase_id = mp.id AND t.external_id = $1
|
||||||
|
)
|
||||||
|
ORDER BY mp.completed_at DESC NULLS LAST
|
||||||
|
LIMIT 2",
|
||||||
|
)
|
||||||
|
.bind(crate::security_scan::SCAN_MARKER)
|
||||||
|
.fetch_all(pool)
|
||||||
|
.await
|
||||||
|
.map_err(|e| format!("select security_scan phases: {e}"))?;
|
||||||
|
Ok(rows
|
||||||
|
.into_iter()
|
||||||
|
.map(|r| (r.get("id"), r.get("mission_id")))
|
||||||
|
.collect())
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn scan_finished_security_phases(pool: &PgPool) -> Result<(), String> {
|
||||||
|
for (phase_id, mission_id) in unscanned_security_phases(pool).await? {
|
||||||
|
let pool = pool.clone();
|
||||||
|
tokio::spawn(async move {
|
||||||
|
match crate::security_scan::run(&pool, mission_id, phase_id).await {
|
||||||
|
Ok(n) => eprintln!(
|
||||||
|
"phase_runner: security scan recorded {n} finding(s) for phase \
|
||||||
|
{phase_id} of mission {mission_id}"
|
||||||
|
),
|
||||||
|
// Not a phase failure, for the same reason the benchmark
|
||||||
|
// baseline is not: a repo the scanners cannot read is a real
|
||||||
|
// outcome. Said out loud, though — "no findings" must not be
|
||||||
|
// indistinguishable from "never scanned".
|
||||||
|
Err(e) => eprintln!(
|
||||||
|
"phase_runner: security scan did not run 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
|
||||||
|
|||||||
@@ -48,7 +48,12 @@ async fn repo_digest(pool: &sqlx::PgPool, mission_id: uuid::Uuid) -> String {
|
|||||||
return "(this mission has no repository)".to_string();
|
return "(this mission has no repository)".to_string();
|
||||||
};
|
};
|
||||||
let branch = branch.unwrap_or_else(|| "main".to_string());
|
let branch = branch.unwrap_or_else(|| "main".to_string());
|
||||||
|
// Distinguish "no credential" from "the forge said no". Both used to
|
||||||
|
// arrive as the same "(could not be read)" string, so an unconfigured
|
||||||
|
// deployment looked identical to a private repo — and the planner, told
|
||||||
|
// only that the read failed, cannot say which.
|
||||||
let token = std::env::var("GITEA_TOKEN").unwrap_or_default();
|
let token = std::env::var("GITEA_TOKEN").unwrap_or_default();
|
||||||
|
let unauthenticated = token.trim().is_empty();
|
||||||
let Ok(client) = reqwest::Client::builder()
|
let Ok(client) = reqwest::Client::builder()
|
||||||
.timeout(std::time::Duration::from_secs(20))
|
.timeout(std::time::Duration::from_secs(20))
|
||||||
.build()
|
.build()
|
||||||
@@ -70,6 +75,11 @@ async fn repo_digest(pool: &sqlx::PgPool, mission_id: uuid::Uuid) -> String {
|
|||||||
);
|
);
|
||||||
let tree: serde_json::Value = match auth(client.get(&tree_url)).send().await {
|
let tree: serde_json::Value = match auth(client.get(&tree_url)).send().await {
|
||||||
Ok(r) if r.status().is_success() => r.json().await.unwrap_or_default(),
|
Ok(r) if r.status().is_success() => r.json().await.unwrap_or_default(),
|
||||||
|
_ if unauthenticated => {
|
||||||
|
return "(the repository tree could not be read: GITEA_TOKEN is unset, \
|
||||||
|
so this read was unauthenticated)"
|
||||||
|
.to_string()
|
||||||
|
}
|
||||||
_ => return "(the repository tree could not be read)".to_string(),
|
_ => return "(the repository tree could not be read)".to_string(),
|
||||||
};
|
};
|
||||||
let entries: Vec<crate::repo_digest::FileEntry> = tree
|
let entries: Vec<crate::repo_digest::FileEntry> = tree
|
||||||
|
|||||||
@@ -109,14 +109,17 @@ pub async fn compare_topologies(
|
|||||||
Json(req): Json<CompareRequest>,
|
Json(req): Json<CompareRequest>,
|
||||||
) -> Result<Json<Comparison>, ApiError> {
|
) -> Result<Json<Comparison>, ApiError> {
|
||||||
// Execution turns run on the exec model (default = configured model, e.g.
|
// Execution turns run on the exec model (default = configured model, e.g.
|
||||||
// sonnet); the judge uses the judge model (default claude-opus-4-8). Either
|
// sonnet); the judge uses the judge model (cm_runtime::judge_model). Either
|
||||||
// can name a registry provider as "<name>:<model>" (e.g. "glm:glm-4.6",
|
// can name a registry provider as "<name>:<model>" (e.g. "glm:glm-4.6",
|
||||||
// "kimi:kimi-k2") to run on GLM/Kimi instead.
|
// "kimi:kimi-k2") to run on GLM/Kimi instead.
|
||||||
let exec_spec = std::env::var("CLAWMATES_TOPOLOGY_EXEC_MODEL")
|
let exec_spec = std::env::var("CLAWMATES_TOPOLOGY_EXEC_MODEL")
|
||||||
.unwrap_or_else(|_| state.runtime.model().to_string());
|
.unwrap_or_else(|_| state.runtime.model().to_string());
|
||||||
let (exec_provider, exec_model) = state.runtime.resolve_provider(&exec_spec);
|
let (exec_provider, exec_model) = state.runtime.resolve_provider(&exec_spec);
|
||||||
let judge_spec =
|
// `cm_runtime::judge_model()`, not a second read of the same variable: this
|
||||||
std::env::var("CLAWMATES_JUDGE_MODEL").unwrap_or_else(|_| "claude-opus-4-8".to_string());
|
// line and that function disagreed on the default (opus-4-8 vs opus-5), so
|
||||||
|
// an unconfigured deployment scored topology comparisons on a different
|
||||||
|
// model than the door governor and nothing recorded which.
|
||||||
|
let judge_spec = cm_runtime::judge_model();
|
||||||
let (judge_provider, judge_model) = state.runtime.resolve_provider(&judge_spec);
|
let (judge_provider, judge_model) = state.runtime.resolve_provider(&judge_spec);
|
||||||
let executor = ProviderExecutor::new(exec_provider, exec_model, state.runtime.max_tokens());
|
let executor = ProviderExecutor::new(exec_provider, exec_model, state.runtime.max_tokens());
|
||||||
let scorer = JudgeScorer::new(judge_provider, judge_model, 16);
|
let scorer = JudgeScorer::new(judge_provider, judge_model, 16);
|
||||||
|
|||||||
@@ -36,6 +36,9 @@ use uuid::Uuid;
|
|||||||
|
|
||||||
use cm_db::repo::missions::UpsertTask;
|
use cm_db::repo::missions::UpsertTask;
|
||||||
|
|
||||||
|
/// `external_id` of the marker row proving a scan ran against a phase.
|
||||||
|
pub const SCAN_MARKER: &str = "security_scan:complete";
|
||||||
|
|
||||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||||
pub struct Finding {
|
pub struct Finding {
|
||||||
pub external_id: String,
|
pub external_id: String,
|
||||||
@@ -92,6 +95,32 @@ pub async fn run(pool: &PgPool, mission_id: Uuid, phase_id: Uuid) -> Result<usiz
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A completion marker, always written — including for a scan that found
|
||||||
|
// nothing. Without it "we scanned and the repo is clean" and "no scan ever
|
||||||
|
// ran" are both zero rows, and the sweep in `phase_runner` that fires this
|
||||||
|
// would have no way to tell whether it had already run: a clean phase would
|
||||||
|
// be rescanned on every tick, forever. It is also the answer to the
|
||||||
|
// question an operator actually asks, which is not "how many findings"
|
||||||
|
// but "was this looked at, by what, and when".
|
||||||
|
let scanned_with = tools.join(", ");
|
||||||
|
cm_db::repo::missions::upsert_task(
|
||||||
|
pool,
|
||||||
|
UpsertTask {
|
||||||
|
mission_id,
|
||||||
|
phase_id,
|
||||||
|
external_id: SCAN_MARKER,
|
||||||
|
title: &format!(
|
||||||
|
"security scan complete — ran [{scanned_with}], {} finding(s)",
|
||||||
|
all_findings.len()
|
||||||
|
),
|
||||||
|
assigned_agent_id: None,
|
||||||
|
status: "created",
|
||||||
|
run_id: None,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.map_err(|e| format!("upsert scan marker: {e}"))?;
|
||||||
|
|
||||||
for f in &all_findings {
|
for f in &all_findings {
|
||||||
cm_db::repo::missions::upsert_task(
|
cm_db::repo::missions::upsert_task(
|
||||||
pool,
|
pool,
|
||||||
|
|||||||
@@ -0,0 +1,128 @@
|
|||||||
|
//! The guard on the security-scan sweep.
|
||||||
|
//!
|
||||||
|
//! `phase_runner::scan_finished_security_phases` fires `security_scan::run`
|
||||||
|
//! for finished `security_scan` phases. The scan needs Docker; the part that
|
||||||
|
//! decides whether it runs twice, once, or never is pure SQL, and it is the
|
||||||
|
//! part that fails silently in both directions — rescanning forever, or never
|
||||||
|
//! scanning at all and leaving a phase that looks identical to a clean repo.
|
||||||
|
|
||||||
|
use cm_domain::{Workspace, WorkspaceId};
|
||||||
|
use uuid::Uuid;
|
||||||
|
|
||||||
|
async fn seed_mission(pool: &sqlx::PgPool) -> Uuid {
|
||||||
|
let ws = Workspace {
|
||||||
|
id: WorkspaceId::new(),
|
||||||
|
name: "Security Sweep Test".into(),
|
||||||
|
plan: "team".into(),
|
||||||
|
};
|
||||||
|
cm_db::repo::workspaces::insert(pool, &ws).await.unwrap();
|
||||||
|
let id = Uuid::now_v7();
|
||||||
|
sqlx::query(
|
||||||
|
"INSERT INTO missions (id, workspace_id, title, template_kind, status)
|
||||||
|
VALUES ($1, $2, 'security test', 'security_hardening', 'running')",
|
||||||
|
)
|
||||||
|
.bind(id)
|
||||||
|
.bind(ws.id.as_uuid())
|
||||||
|
.execute(pool)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
id
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn seed_phase(
|
||||||
|
pool: &sqlx::PgPool,
|
||||||
|
mission_id: Uuid,
|
||||||
|
kind: &str,
|
||||||
|
status: &str,
|
||||||
|
order_idx: i32,
|
||||||
|
) -> Uuid {
|
||||||
|
let id = Uuid::now_v7();
|
||||||
|
sqlx::query(
|
||||||
|
"INSERT INTO mission_phases (id, mission_id, kind, order_idx, status, completed_at)
|
||||||
|
VALUES ($1, $2, $3, $4, $5, now())",
|
||||||
|
)
|
||||||
|
.bind(id)
|
||||||
|
.bind(mission_id)
|
||||||
|
.bind(kind)
|
||||||
|
.bind(order_idx)
|
||||||
|
.bind(status)
|
||||||
|
.execute(pool)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
id
|
||||||
|
}
|
||||||
|
|
||||||
|
async fn mark_scanned(pool: &sqlx::PgPool, mission_id: Uuid, phase_id: Uuid) {
|
||||||
|
cm_db::repo::missions::upsert_task(
|
||||||
|
pool,
|
||||||
|
cm_db::repo::missions::UpsertTask {
|
||||||
|
mission_id,
|
||||||
|
phase_id,
|
||||||
|
external_id: cm_api::security_scan::SCAN_MARKER,
|
||||||
|
title: "security scan complete — ran [gitleaks], 0 finding(s)",
|
||||||
|
assigned_agent_id: None,
|
||||||
|
status: "created",
|
||||||
|
run_id: None,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn a_finished_security_phase_is_selected_until_it_carries_a_scan_marker() {
|
||||||
|
let pool = cm_testkit::test_pool().await;
|
||||||
|
let mission = seed_mission(&pool).await;
|
||||||
|
let phase = seed_phase(&pool, mission, "security_scan", "completed", 0).await;
|
||||||
|
|
||||||
|
let selected = cm_api::phase_runner::unscanned_security_phases(&pool)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert!(
|
||||||
|
selected.iter().any(|(p, _)| *p == phase),
|
||||||
|
"a finished security_scan phase with no marker must be selected — \
|
||||||
|
otherwise the scan never runs and the phase reports completed having \
|
||||||
|
scanned nothing"
|
||||||
|
);
|
||||||
|
|
||||||
|
// A clean scan writes NO findings, so the marker is the only evidence the
|
||||||
|
// scan happened. This is the case that would otherwise rescan forever.
|
||||||
|
mark_scanned(&pool, mission, phase).await;
|
||||||
|
|
||||||
|
let selected = cm_api::phase_runner::unscanned_security_phases(&pool)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert!(
|
||||||
|
!selected.iter().any(|(p, _)| *p == phase),
|
||||||
|
"a phase carrying the scan marker must not be selected again, even \
|
||||||
|
though it has zero findings"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn only_finished_security_phases_are_selected() {
|
||||||
|
let pool = cm_testkit::test_pool().await;
|
||||||
|
let mission = seed_mission(&pool).await;
|
||||||
|
|
||||||
|
let running = seed_phase(&pool, mission, "security_scan", "running", 0).await;
|
||||||
|
let coding = seed_phase(&pool, mission, "coding", "completed", 1).await;
|
||||||
|
let failed = seed_phase(&pool, mission, "security_scan", "failed", 2).await;
|
||||||
|
|
||||||
|
let selected: Vec<Uuid> = cm_api::phase_runner::unscanned_security_phases(&pool)
|
||||||
|
.await
|
||||||
|
.unwrap()
|
||||||
|
.into_iter()
|
||||||
|
.map(|(p, _)| p)
|
||||||
|
.collect();
|
||||||
|
|
||||||
|
assert!(
|
||||||
|
!selected.contains(&running),
|
||||||
|
"scanning a phase still running would scan a half-written checkout"
|
||||||
|
);
|
||||||
|
assert!(!selected.contains(&coding), "only security_scan phases scan");
|
||||||
|
assert!(
|
||||||
|
selected.contains(&failed),
|
||||||
|
"a FAILED security phase is exactly the one worth scanning — the \
|
||||||
|
scanners are how we find out what state it left behind"
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -515,7 +515,15 @@ pub async fn upsert_task(pool: &PgPool, t: UpsertTask<'_>) -> Result<Uuid, DbErr
|
|||||||
assigned_agent_id, status, run_id, completed_at)
|
assigned_agent_id, status, run_id, completed_at)
|
||||||
VALUES ($1,$2,$3,$4,$5,$6,$7,$8,
|
VALUES ($1,$2,$3,$4,$5,$6,$7,$8,
|
||||||
CASE WHEN $9 THEN now() ELSE NULL END)
|
CASE WHEN $9 THEN now() ELSE NULL END)
|
||||||
ON CONFLICT (phase_id, external_id)
|
-- The WHERE clause is REQUIRED, not decoration: the index it infers
|
||||||
|
-- (mission_tasks_external_uniq) is PARTIAL, and Postgres will not
|
||||||
|
-- match a partial index to an ON CONFLICT target unless the statement
|
||||||
|
-- repeats its predicate. Without it every call raised 42P10, no
|
||||||
|
-- unique or exclusion constraint matching the ON CONFLICT
|
||||||
|
-- specification, so both callers -- the task-card parser and the
|
||||||
|
-- security scanner -- failed on their first row and surfaced it as a
|
||||||
|
-- log line rather than as a broken feature.
|
||||||
|
ON CONFLICT (phase_id, external_id) WHERE external_id IS NOT NULL
|
||||||
DO UPDATE SET
|
DO UPDATE SET
|
||||||
title = EXCLUDED.title,
|
title = EXCLUDED.title,
|
||||||
assigned_agent_id = COALESCE(EXCLUDED.assigned_agent_id, mission_tasks.assigned_agent_id),
|
assigned_agent_id = COALESCE(EXCLUDED.assigned_agent_id, mission_tasks.assigned_agent_id),
|
||||||
|
|||||||
@@ -0,0 +1,107 @@
|
|||||||
|
//! `upsert_task` against the partial unique index it depends on.
|
||||||
|
//!
|
||||||
|
//! `mission_tasks_external_uniq` is declared `WHERE external_id IS NOT NULL`.
|
||||||
|
//! Postgres will not match a partial index to an `ON CONFLICT` target unless
|
||||||
|
//! the statement repeats that predicate, so the upsert raised 42P10 on its
|
||||||
|
//! first row for every caller — the task-card parser (INT markers) and the
|
||||||
|
//! security scanner. Both map the error to a string their caller logs, so the
|
||||||
|
//! feature was broken and the platform stayed green.
|
||||||
|
//!
|
||||||
|
//! The insert half and the update half are tested separately because they fail
|
||||||
|
//! differently: a missing conflict target breaks both, but a wrong DO UPDATE
|
||||||
|
//! breaks only the second write, which is the one that happens on re-run.
|
||||||
|
|
||||||
|
use cm_db::repo::missions::{upsert_task, UpsertTask};
|
||||||
|
use cm_domain::{Workspace, WorkspaceId};
|
||||||
|
use uuid::Uuid;
|
||||||
|
|
||||||
|
async fn seed_phase(pool: &sqlx::PgPool) -> (Uuid, Uuid) {
|
||||||
|
let ws = Workspace {
|
||||||
|
id: WorkspaceId::new(),
|
||||||
|
name: "Task Upsert Test".into(),
|
||||||
|
plan: "team".into(),
|
||||||
|
};
|
||||||
|
cm_db::repo::workspaces::insert(pool, &ws).await.unwrap();
|
||||||
|
let mission = Uuid::now_v7();
|
||||||
|
sqlx::query(
|
||||||
|
"INSERT INTO missions (id, workspace_id, title, template_kind, status)
|
||||||
|
VALUES ($1, $2, 'upsert test', 'security_hardening', 'running')",
|
||||||
|
)
|
||||||
|
.bind(mission)
|
||||||
|
.bind(ws.id.as_uuid())
|
||||||
|
.execute(pool)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
let phase = Uuid::now_v7();
|
||||||
|
sqlx::query(
|
||||||
|
"INSERT INTO mission_phases (id, mission_id, kind, order_idx, status)
|
||||||
|
VALUES ($1, $2, 'security_scan', 0, 'completed')",
|
||||||
|
)
|
||||||
|
.bind(phase)
|
||||||
|
.bind(mission)
|
||||||
|
.execute(pool)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
(mission, phase)
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn the_same_external_id_updates_in_place_instead_of_erroring() {
|
||||||
|
let pool = cm_testkit::test_pool().await;
|
||||||
|
let (mission_id, phase_id) = seed_phase(&pool).await;
|
||||||
|
|
||||||
|
let first = upsert_task(
|
||||||
|
&pool,
|
||||||
|
UpsertTask {
|
||||||
|
mission_id,
|
||||||
|
phase_id,
|
||||||
|
external_id: "RUSTSEC-2026-0001",
|
||||||
|
title: "advisory: something",
|
||||||
|
assigned_agent_id: None,
|
||||||
|
status: "created",
|
||||||
|
run_id: None,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.expect("first upsert must succeed — 42P10 here means the ON CONFLICT \
|
||||||
|
target no longer matches the partial unique index");
|
||||||
|
|
||||||
|
// The re-run. A scanner that finds the same advisory twice must not
|
||||||
|
// duplicate the row, and must not fail.
|
||||||
|
let second = upsert_task(
|
||||||
|
&pool,
|
||||||
|
UpsertTask {
|
||||||
|
mission_id,
|
||||||
|
phase_id,
|
||||||
|
external_id: "RUSTSEC-2026-0001",
|
||||||
|
title: "advisory: something (rescanned)",
|
||||||
|
assigned_agent_id: None,
|
||||||
|
status: "complete",
|
||||||
|
run_id: None,
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.expect("second upsert must succeed");
|
||||||
|
|
||||||
|
assert_eq!(first, second, "the same (phase_id, external_id) must be one row");
|
||||||
|
|
||||||
|
let (title, status, completed): (String, String, Option<time::OffsetDateTime>) =
|
||||||
|
sqlx::query_as("SELECT title, status, completed_at FROM mission_tasks WHERE id = $1")
|
||||||
|
.bind(first)
|
||||||
|
.fetch_one(&pool)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(title, "advisory: something (rescanned)");
|
||||||
|
assert_eq!(status, "complete");
|
||||||
|
assert!(
|
||||||
|
completed.is_some(),
|
||||||
|
"a task upserted as `complete` must get its completed_at stamped"
|
||||||
|
);
|
||||||
|
|
||||||
|
let n: i64 = sqlx::query_scalar("SELECT count(*) FROM mission_tasks WHERE phase_id = $1")
|
||||||
|
.bind(phase_id)
|
||||||
|
.fetch_one(&pool)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(n, 1, "two upserts of one external_id left {n} rows");
|
||||||
|
}
|
||||||
@@ -28,7 +28,7 @@ const APPROVAL_TTL: time::Duration = time::Duration::hours(24);
|
|||||||
|
|
||||||
/// Model used for LLM-as-judge roles (the door governor and the topology
|
/// Model used for LLM-as-judge roles (the door governor and the topology
|
||||||
/// comparison scorer). Judges use the strongest model — default
|
/// comparison scorer). Judges use the strongest model — default
|
||||||
/// `claude-opus-4-8` — while everything else runs on the configured default
|
/// `claude-opus-5` — while everything else runs on the configured default
|
||||||
/// model (`claude-sonnet-4-6`). Override with `CLAWMATES_JUDGE_MODEL`.
|
/// model (`claude-sonnet-4-6`). Override with `CLAWMATES_JUDGE_MODEL`.
|
||||||
pub fn judge_model() -> String {
|
pub fn judge_model() -> String {
|
||||||
std::env::var("CLAWMATES_JUDGE_MODEL").unwrap_or_else(|_| "claude-opus-5".to_string())
|
std::env::var("CLAWMATES_JUDGE_MODEL").unwrap_or_else(|_| "claude-opus-5".to_string())
|
||||||
|
|||||||
@@ -0,0 +1,166 @@
|
|||||||
|
# Provenance: what we can answer today, and what we cannot
|
||||||
|
|
||||||
|
*Assessment only. No schema, no migration, nothing built. Written 2026-08-19.*
|
||||||
|
|
||||||
|
The question this exists to answer is narrow and practical:
|
||||||
|
|
||||||
|
> An agent said something. Where did it come from?
|
||||||
|
|
||||||
|
That is the question a provenance layer has to make answerable. Everything
|
||||||
|
below is measured against it.
|
||||||
|
|
||||||
|
## The short answer
|
||||||
|
|
||||||
|
We can reconstruct **what an agent did**, on both execution paths, for seven
|
||||||
|
days. We cannot reconstruct **why it said what it said** on any path, at any
|
||||||
|
retention, because no store links a statement to the evidence that produced it.
|
||||||
|
There is no claim as a first-class object anywhere in the system.
|
||||||
|
|
||||||
|
That is a design gap, not a bug. Nothing is broken; the edge was never built.
|
||||||
|
|
||||||
|
## What each store actually holds
|
||||||
|
|
||||||
|
### `mission_events` — the action record
|
||||||
|
|
||||||
|
The main one. Structured rows for the mission path: tool calls, phase
|
||||||
|
transitions, agent lifecycle.
|
||||||
|
|
||||||
|
- **Retention: 7 days** (`EVENT_RETENTION_DAYS`, `mission_gc.rs`). This is the
|
||||||
|
single most consequential fact in this document. The richest signal we have
|
||||||
|
expires before most retrospectives happen, and a level-up proposal citing an
|
||||||
|
event id older than a week points at nothing.
|
||||||
|
- `reasoning` rows **are written** (`topology_worker.rs`) and **are read by
|
||||||
|
nothing**. `routes/world.rs` explicitly excludes them, with a comment saying
|
||||||
|
so. The model's stated rationale is recorded and then discarded unread.
|
||||||
|
- No foreign key to `run_id` — `0075` dropped it deliberately. Events reference
|
||||||
|
runs by convention, so nothing enforces that the reference resolves.
|
||||||
|
|
||||||
|
### `steps` — the rich record, on the wrong path
|
||||||
|
|
||||||
|
Structurally the best provenance we have: `kind`, `tool_name`, `input`,
|
||||||
|
`output`, **and `taint[]`** — per tool call, with the taint sources carried
|
||||||
|
through.
|
||||||
|
|
||||||
|
It is **chat-path only**, and not by omission. `steps.message_id` references a
|
||||||
|
chat message, and `runtime::record_step` is the sole writer. A mission phase
|
||||||
|
has no message, so it cannot write a step even in principle.
|
||||||
|
|
||||||
|
The consequence is worth stating plainly: the execution path that does the
|
||||||
|
substantial work — missions — produces the *poorer* provenance record, and the
|
||||||
|
path that produces the good one is the conversational one. Any real provenance
|
||||||
|
work starts by resolving that asymmetry, and it is a schema change, not a call
|
||||||
|
site.
|
||||||
|
|
||||||
|
### `audit_log` — genuinely immutable, narrowly used
|
||||||
|
|
||||||
|
A `BEFORE UPDATE OR DELETE` trigger (`0001_init.sql`) makes rows append-only for
|
||||||
|
real, not by convention. It is the only tamper-evident store in the system.
|
||||||
|
|
||||||
|
It is used mostly as a rate-limit counter. The mechanism we would want for
|
||||||
|
provenance already exists here and holds almost none of the content we would
|
||||||
|
want in it.
|
||||||
|
|
||||||
|
### Approvals — the best-shaped record we have
|
||||||
|
|
||||||
|
Approval rows carry `taint_sources[]` and the exact preview a human was shown.
|
||||||
|
For the narrow slice of actions that pass a gate, we can answer "what was the
|
||||||
|
human told, what did they decide, and what data influenced it" — completely.
|
||||||
|
|
||||||
|
This is the shape to copy. It is also, currently, an island.
|
||||||
|
|
||||||
|
### The `.brain` — memory, versioned, chat-only
|
||||||
|
|
||||||
|
Full `.onion` revision history with `commit`/`revisions`/`rollback`, so a
|
||||||
|
definition's evolution is fully recoverable. Memory is BM25 keyword recall and
|
||||||
|
is written on the **chat** path; mission work writes none, so an agent that ran
|
||||||
|
missions for a week has an empty memory section.
|
||||||
|
|
||||||
|
`cm-brain/src/lib.rs:234-239` exposes `set_provenance` / `provenance` — a wired
|
||||||
|
slot with **zero callers** in the entire workspace. It is a place to put this,
|
||||||
|
already plumbed, already versioned, already per-agent.
|
||||||
|
|
||||||
|
### `mission_phase_summaries` — prose, and overwritten
|
||||||
|
|
||||||
|
A model's post-hoc narrative of a phase, **overwritten on retry**. A summary is
|
||||||
|
a model's later account of its own behaviour, and the retry that changed the
|
||||||
|
outcome erases the account of the attempt that failed — which is precisely the
|
||||||
|
one worth reading.
|
||||||
|
|
||||||
|
## The questions we cannot answer
|
||||||
|
|
||||||
|
Concretely, with what breaks each:
|
||||||
|
|
||||||
|
| Question | Why not |
|
||||||
|
|---|---|
|
||||||
|
| "Why did the agent claim X?" | No claim object; no edge from a statement to a tool result |
|
||||||
|
| "Which tool output led to this file change?" | Mission path writes no `steps`, so no input/output pair exists |
|
||||||
|
| "What did the agent believe when it decided?" | `reasoning` rows are written and read by nothing |
|
||||||
|
| "Was this conclusion derived from tainted input?" | `taint[]` exists only on the chat path |
|
||||||
|
| "What happened on the attempt that failed?" | Phase summaries are overwritten on retry |
|
||||||
|
| "Why did this mission go wrong last month?" | 7-day retention |
|
||||||
|
|
||||||
|
The recurring shape: **the pieces exist, individually, on the wrong path or
|
||||||
|
unread.** This is not a system that lacks provenance primitives. It is one where
|
||||||
|
they were never connected into something that answers a question.
|
||||||
|
|
||||||
|
## Two candidate paths
|
||||||
|
|
||||||
|
### A. Postgres claim/edge tables
|
||||||
|
|
||||||
|
Add a claim as a first-class row, and edges from claim → evidence (tool call,
|
||||||
|
file, event). Extend `steps` off the mission path so mission tool calls record
|
||||||
|
input/output/taint the way chat ones do.
|
||||||
|
|
||||||
|
- **For:** stays in the store we already query, back up and migrate; the
|
||||||
|
approvals record already proves the shape works here; no new dependency.
|
||||||
|
- **Against:** a real schema addition on a 79-migration database; retention
|
||||||
|
policy must be decided deliberately (7 days makes the whole thing pointless);
|
||||||
|
graph queries in SQL get awkward exactly when they get interesting.
|
||||||
|
|
||||||
|
### B. Adopt `clawhdf5-agent::knowledge` + `::provenance`
|
||||||
|
|
||||||
|
Already in the dependency graph via the clawsync patch — declared in the
|
||||||
|
workspace `Cargo.toml`, and **no crate depends on it**. Roughly 21k lines:
|
||||||
|
typed entities, relations including `RelationType::Causal`, BFS and spreading
|
||||||
|
activation; `provenance.rs` with `MemorySource` / `content_hash` /
|
||||||
|
`session_id`.
|
||||||
|
|
||||||
|
- **For:** the causal structure is the thing we lack, and it is written; it is
|
||||||
|
per-agent and per-file, matching the `.brain` model; `set_provenance` is
|
||||||
|
already the slot it would fill.
|
||||||
|
- **Against:** 21k lines of unexercised code entering a critical path; it is
|
||||||
|
file-local, so cross-agent queries need a second mechanism; the `.brain` is
|
||||||
|
reaped with the agent, which is the wrong lifetime for an audit record.
|
||||||
|
|
||||||
|
### What the research says about choosing
|
||||||
|
|
||||||
|
`MemoryLake on MemoryArena` (2026-08-14) compared memory backends with the
|
||||||
|
backend as the only changed component. Structured beat vector RAG and
|
||||||
|
long-context — on success rates of 9/40, 12/20 and 4/20, with every system
|
||||||
|
scoring zero somewhere. `Harness the Memory` (08-15) found no substrate
|
||||||
|
dominates, and that *excessive* retrieval actively harms agent decision-making
|
||||||
|
even while helping factual QA.
|
||||||
|
|
||||||
|
Read together: a substrate swap is not where the win is, and adopting a graph
|
||||||
|
memory because it is more sophisticated is not supported by the evidence. That
|
||||||
|
argues for **A first** — make the record complete and correctly retained on the
|
||||||
|
path that matters — and to treat B as a question to measure later, against a
|
||||||
|
baseline captured beforehand.
|
||||||
|
|
||||||
|
`D²ACCI` (08-18) is the sharper finding for us: "end-to-end evaluation reveals
|
||||||
|
that an error occurred, but not which stage caused it", and its **DCR** metric
|
||||||
|
grades whether failures stay *localizable*. That is this project's recurring
|
||||||
|
defect class stated as a research problem, and localizability — not
|
||||||
|
completeness — is the property a provenance layer here should be judged on.
|
||||||
|
|
||||||
|
## The cheapest thing that would help, if we do nothing else
|
||||||
|
|
||||||
|
1. **Read the `reasoning` rows we already write.** They exist. Nothing consumes
|
||||||
|
them. This is a query, not a schema.
|
||||||
|
2. **Raise retention for a subset.** Seven days is right for volume, wrong for
|
||||||
|
audit. The distinction is which rows, not how long.
|
||||||
|
3. **Stop overwriting phase summaries on retry.** The erased attempt is the
|
||||||
|
informative one.
|
||||||
|
|
||||||
|
None of these is the provenance layer. All three are cheap, and each closes a
|
||||||
|
question we currently cannot answer at all.
|
||||||
@@ -5,16 +5,59 @@ requires_repo = true
|
|||||||
|
|
||||||
default_team_template = "rust_sdlc"
|
default_team_template = "rust_sdlc"
|
||||||
|
|
||||||
|
# ── What this recipe actually does ───────────────────────────────────
|
||||||
|
#
|
||||||
|
# `harness` is REAL and was mislabelled: `benchmark_runner::harness_from_config`
|
||||||
|
# reads it, and `phase_runner`'s benchmark sweep runs a baseline through it
|
||||||
|
# automatically, recording a `benchmark_snapshots` row. It was listed in
|
||||||
|
# `phase_config.rs` as NOT IMPLEMENTED — now corrected there. `mode` genuinely
|
||||||
|
# is inert.
|
||||||
|
#
|
||||||
|
# The real gap was that the phase carried no `task` and no `done_when` — so a
|
||||||
|
# benchmark mission ran
|
||||||
|
# one unjudged phase with a generic directive and reported `completed` whether
|
||||||
|
# it wrote a benchmark or not. Since the entire point of this workflow is to
|
||||||
|
# produce numbers a later refactor is measured against, a run that silently
|
||||||
|
# produced none is worse than no run: the next mission compares against a
|
||||||
|
# baseline that does not exist.
|
||||||
|
#
|
||||||
|
# The instructions therefore live in `task`/`done_when`, which ARE read, rather
|
||||||
|
# than in `mode`/`harness`, which are not. `benchmark` is in PRODUCING_KINDS, so
|
||||||
|
# the empty-delivery rule applies once there is something to deliver.
|
||||||
|
|
||||||
[[phases]]
|
[[phases]]
|
||||||
kind = "benchmark"
|
kind = "benchmark"
|
||||||
order_idx = 0
|
order_idx = 0
|
||||||
[phases.config]
|
[phases.config]
|
||||||
# Slice 7 runs the benchmark_snapshots baseline pass here. This
|
produces = ["md"]
|
||||||
# workflow's job is to AUTHOR the benchmarks + establish the
|
# NOTE: `mode` is INERT — no code selects behaviour from it.
|
||||||
# baseline; subsequent refactor missions consume them.
|
|
||||||
mode = "author_and_baseline"
|
mode = "author_and_baseline"
|
||||||
# Which harness to use; per-stack defaults if unset:
|
# READ by benchmark_runner. "auto" lets it detect the stack; naming a harness
|
||||||
# rust → criterion / cargo bench
|
# explicitly ("criterion" + `bench_name`, "cargo_bench", "vitest_bench",
|
||||||
# ts/js → vitest --bench / mitata
|
# "pytest_bench", or "shell" + `cmd`) pins it. The automatic baseline this
|
||||||
# py → pytest-benchmark
|
# drives is separate from, and a check on, the numbers the agent reports.
|
||||||
harness = "auto"
|
harness = "auto"
|
||||||
|
task = """
|
||||||
|
Author benchmarks for this repository and record a baseline that a later \
|
||||||
|
refactor can be measured against.
|
||||||
|
|
||||||
|
Pick the harness that matches the stack — criterion for Rust, `vitest --bench` \
|
||||||
|
or mitata for TS/JS, pytest-benchmark for Python — and say in the report which \
|
||||||
|
you chose and why.
|
||||||
|
|
||||||
|
Benchmark what the project's own hot path is, not what is easy to measure. A \
|
||||||
|
microbenchmark of a function nobody calls produces a number that will never \
|
||||||
|
change and teaches a future refactor nothing. Read the code first and name the \
|
||||||
|
operation you are measuring and why it is the one that matters.
|
||||||
|
|
||||||
|
Record the baseline in BENCHMARKS.md: the machine, the command, the numbers, \
|
||||||
|
AND the run-to-run spread from at least three runs. The spread is not optional \
|
||||||
|
detail — without it, nobody can tell whether a later 5% "improvement" is real \
|
||||||
|
or noise, which makes the entire baseline unusable for its only purpose.
|
||||||
|
|
||||||
|
Commit the benchmark code itself, not just the results. The next mission has to \
|
||||||
|
be able to re-run exactly what you ran.
|
||||||
|
"""
|
||||||
|
done_when = "BENCHMARKS.md exists and records, for each benchmark authored, the operation measured, the exact command to re-run it, the baseline numbers from at least three runs including the observed spread, and the machine they were taken on — and the benchmark source is committed to the repository"
|
||||||
|
max_iterations = 2
|
||||||
|
commit_policy = "always"
|
||||||
|
|||||||
@@ -5,29 +5,120 @@ requires_repo = true
|
|||||||
|
|
||||||
default_team_template = "rust_sdlc"
|
default_team_template = "rust_sdlc"
|
||||||
|
|
||||||
|
# ── What is real here, and what is decoration ────────────────────────
|
||||||
|
#
|
||||||
|
# The scanners themselves are REAL: `gitleaks`, `trivy`, `semgrep` and
|
||||||
|
# `cargo-audit` are installed in the runtime image and `runtime_preflight`
|
||||||
|
# probes for all four at boot, naming the consequence when one is absent. An
|
||||||
|
# agent in a security_scan phase can run them from Bash today.
|
||||||
|
#
|
||||||
|
# `tools` is REAL too: `security_scan::run` reads it to pick which scanners to
|
||||||
|
# run and upserts each finding as a mission_task. What was missing was anything
|
||||||
|
# that FIRED it — it was reachable only from an operator button, so this
|
||||||
|
# workflow's scan phase never scanned. `phase_runner::scan_finished_security_phases`
|
||||||
|
# now runs it when the phase finishes, guarded on a completion marker so a
|
||||||
|
# clean repo is not rescanned forever.
|
||||||
|
#
|
||||||
|
# What remains decoration is annotated inline below. And the part that actually
|
||||||
|
# mattered: no phase carried a `task` or a `done_when`.
|
||||||
|
# A phase with no `done_when` never enters `evaluating`, is never judged, and
|
||||||
|
# reports `completed` whatever it did. So this recipe could run all three
|
||||||
|
# phases, scan nothing, patch nothing, and go green. That is the same defect
|
||||||
|
# `research_and_code.toml` was fixed for, and it is why the tasks below name
|
||||||
|
# the scanners explicitly rather than trusting `tools` to deliver them.
|
||||||
|
|
||||||
[[phases]]
|
[[phases]]
|
||||||
kind = "security_scan"
|
kind = "security_scan"
|
||||||
order_idx = 0
|
order_idx = 0
|
||||||
[phases.config]
|
[phases.config]
|
||||||
# Slice 8 wires these tools as an MCP bundle. Each finding becomes
|
# READ by `security_scan::run` — this list gates which scanners the platform
|
||||||
# a mission_task with external_id = CVE/RUSTSEC/gitleaks fingerprint.
|
# runs against the checkout after the phase finishes. The agent ALSO runs them
|
||||||
|
# itself during the phase (see the task): the platform pass is the independent
|
||||||
|
# record, the agent pass is what lets it write a report about what it found.
|
||||||
tools = ["cargo_audit", "gitleaks", "trivy_fs", "semgrep"]
|
tools = ["cargo_audit", "gitleaks", "trivy_fs", "semgrep"]
|
||||||
|
produces = ["md"]
|
||||||
|
task = """
|
||||||
|
Scan this repository for security problems and write findings down.
|
||||||
|
|
||||||
|
Run the scanners that are installed in your container — `cargo audit`, \
|
||||||
|
`gitleaks detect`, `trivy fs .` and `semgrep --config auto` — from the repo \
|
||||||
|
root. If one is missing or errors, say so explicitly in the report rather than \
|
||||||
|
omitting it: a section absent because a tool failed reads identically to a \
|
||||||
|
section absent because nothing was found, and those are opposite conclusions.
|
||||||
|
|
||||||
|
Write SECURITY-FINDINGS.md with one entry per finding: the identifier \
|
||||||
|
(CVE / RUSTSEC / rule id), the file and line, what an attacker could actually \
|
||||||
|
do with it, and whether it is reachable in our code or sits in an unused \
|
||||||
|
dependency path. Rank by exploitability, not by the scanner's severity field.
|
||||||
|
|
||||||
|
A clean scan is a real and useful result. Say which tools ran, on what, and \
|
||||||
|
that they found nothing — do not manufacture findings to fill the report.
|
||||||
|
"""
|
||||||
|
done_when = "SECURITY-FINDINGS.md exists and states, for each of the four scanners, whether it ran and what it found, with every reported finding carrying an identifier, a location, and a reachability judgement"
|
||||||
|
max_iterations = 2
|
||||||
|
commit_policy = "always"
|
||||||
|
|
||||||
[[phases]]
|
[[phases]]
|
||||||
kind = "research"
|
kind = "research"
|
||||||
order_idx = 1
|
order_idx = 1
|
||||||
[phases.config]
|
[phases.config]
|
||||||
produces = ["md", "pdf"]
|
# `pdf` dropped: PDF rendering was removed from the delivery path (artifacts are
|
||||||
|
# served as Markdown), so asking for it produced a format nothing generates.
|
||||||
|
produces = ["md"]
|
||||||
default_topology = "hub_spoke"
|
default_topology = "hub_spoke"
|
||||||
# The research phase reads the security_scan phase's findings from
|
# NOTE: `input_from_phase` is INERT — DECLARED_BUT_UNREAD. Phases do not receive
|
||||||
# mission_tasks and produces a patch strategy per finding.
|
# a structured hand-off from a named predecessor; the next phase reads the
|
||||||
|
# previous phase's committed FILES out of the shared checkout. That is why the
|
||||||
|
# task names SECURITY-FINDINGS.md by path.
|
||||||
input_from_phase = "security_scan"
|
input_from_phase = "security_scan"
|
||||||
|
task = """
|
||||||
|
Turn the scan findings into a patch strategy.
|
||||||
|
|
||||||
|
Read SECURITY-FINDINGS.md from the repo root — that is the previous phase's \
|
||||||
|
output, committed to this mission's branch. For each finding that is actually \
|
||||||
|
reachable, write into SECURITY-PLAN.md: the fix, the specific file and \
|
||||||
|
function it touches, what could break, and how the fix will be verified.
|
||||||
|
|
||||||
|
Where the fix is a dependency bump, check what the new version changes — a \
|
||||||
|
major bump presented as "update the version" is how a security patch becomes \
|
||||||
|
an outage. Where a finding is not worth fixing, say so and say why; an \
|
||||||
|
unreachable advisory in a dev-dependency is a legitimate "no action".
|
||||||
|
"""
|
||||||
|
done_when = "SECURITY-PLAN.md exists and gives, for every reachable finding in SECURITY-FINDINGS.md, either a named fix with the file it touches and how it will be verified, or an explicit justification for taking no action"
|
||||||
|
max_iterations = 2
|
||||||
|
commit_policy = "always"
|
||||||
|
|
||||||
[[phases]]
|
[[phases]]
|
||||||
kind = "coding"
|
kind = "coding"
|
||||||
order_idx = 2
|
order_idx = 2
|
||||||
[phases.config]
|
[phases.config]
|
||||||
|
# NOTE: `loop` is INERT — DECLARED_BUT_UNREAD ("phase iteration uses
|
||||||
|
# max_iterations + done_when"). Kept so the intent stays visible beside the two
|
||||||
|
# keys that actually drive the loop.
|
||||||
loop = "until_all_findings_closed"
|
loop = "until_all_findings_closed"
|
||||||
|
max_iterations = 3
|
||||||
# Security requires reviewer approval on top of green tests.
|
# Security requires reviewer approval on top of green tests.
|
||||||
commit_policy = "on_reviewer_approval"
|
commit_policy = "on_reviewer_approval"
|
||||||
|
# NOTE: `mcp_bundles` is INERT AT PHASE LEVEL — bundles come from the TEAM
|
||||||
|
# template (`mission_orchestrator` binds `template.mcp_bundles`). This phase
|
||||||
|
# gets neither `gitea_forge` nor `security_scan` from this line, and there is no
|
||||||
|
# `security_scan` bundle to get. Left visible rather than deleted because the
|
||||||
|
# gap between what a recipe asks for and what a phase receives is the thing
|
||||||
|
# worth being able to see.
|
||||||
mcp_bundles = ["clawmates_door", "clawmates_skills", "gitea_forge", "security_scan"]
|
mcp_bundles = ["clawmates_door", "clawmates_skills", "gitea_forge", "security_scan"]
|
||||||
|
task = """
|
||||||
|
Apply the patch strategy and prove it worked.
|
||||||
|
|
||||||
|
Work through SECURITY-PLAN.md. Make the smallest change that closes each \
|
||||||
|
finding, and run the test suite after each one so a regression is attributable \
|
||||||
|
to a single fix rather than to the batch.
|
||||||
|
|
||||||
|
Then RE-RUN the scanner that produced each finding and record the new output in \
|
||||||
|
SECURITY-FINDINGS.md under a "after remediation" heading. A fix that was not \
|
||||||
|
re-scanned is a claim, not a result — and this phase's whole value is the \
|
||||||
|
difference between those two.
|
||||||
|
|
||||||
|
If a fix cannot be made safely, leave the finding open and say why. An open \
|
||||||
|
finding that is documented is worth more than a closed one that is not true.
|
||||||
|
"""
|
||||||
|
done_when = "every finding in SECURITY-PLAN.md marked for fixing is either closed with a re-run of the scanner that found it recorded in SECURITY-FINDINGS.md, or left open with a stated reason, and the test suite passes"
|
||||||
|
|||||||
Reference in New Issue
Block a user