feat(missions): hold every producing phase to delivering, and read markdown instead of PDFs
Two changes the portal review asked for.
1. `benchmark` and `security_hardening` had no delivery guarantee.
`empty_delivery_is_a_failure` tested `kind == "coding"`, on the reasoning that
"research phases legitimately write nothing to the tree" — which the research
directive three modules over contradicts, since it tells the agent to save
findings under /mission/repo/research/. The cost: a `benchmark` mission is ONE
benchmark phase, and with that phase exempt nothing in the platform could fail
it. Same for `security_hardening`, whose first two phases are security_scan and
research.
Now keyed on PRODUCING_KINDS = coding, research, benchmark, security_scan.
`review` stays exempt — a reviewing phase that changes nothing has done its job,
the same distinction `vm_stop_gate::per_node` makes. The test that encoded the
old rule is rewritten rather than deleted, with the reasoning that replaced it.
All 8 harness fixtures are coding phases, so harness behaviour is unchanged.
2. PDFs are dropped; markdown is the deliverable.
Rendering a PDF meant asking an LLM to convert markdown to HTML — a paid API
call per document, on the critical path of "let me read my research", which
failed on depleted Gemini credits and left every artifact unreadable. Styling at
render time is free, offline, instant and cannot 429.
- `mission_outputs` no longer requests a render.
- New `GET /api/missions/{id}/artifacts/{artifact_id}/content`. The frontend had
no way to READ an artifact at all: it listed paths and offered a PDF preview
that never rendered (and whose `rendered_pdf_path` had no route serving it).
Two containment rules, both enforced: the artifact must belong to a mission in
the caller's workspace, and the CANONICALISED path must stay under `_outputs`
— canonicalise first, because checking the string before resolving `..` is the
classic hole.
- `MarkdownBlock` now uses react-markdown + remark-gfm + rehype-slug. It was a
deliberate zero-dep renderer for "the subset the refiner emits", and that
subset stopped matching reality: agent briefs are largely GFM pipe tables,
which it showed as literal pipes. MissionOutputReader and RefineDiffModal use
the same component and gain tables for free.
- Heading ids come from rehype-slug and `outlineOf` slugs with the same
GithubSlugger, so the outline rail's anchors still resolve. A test pins that
invariant, including duplicate headings.
Styles live in globals.css under `.md-view`: the markup is generated so there
are no class hooks, and this project has no styled-jsx registry — the app-router
requirement is documented in next/dist/docs/01-app/02-guides/css-in-js.md, which
frontend/AGENTS.md exists to make me read.
The artifacts tab moved to `MissionArtifacts.tsx`. MissionCanvas was 1341 lines
against a 1250 limit BEFORE this change — already failing lint; it is now 1248.
238 backend lib tests, 20 backend test binaries, 89 frontend tests, clean tsc,
clean eslint on every file touched, production build succeeds.
This commit is contained in:
@@ -111,7 +111,6 @@ pub async fn capture_repo_less_phases(pool: &PgPool) -> Result<(), String> {
|
||||
}
|
||||
};
|
||||
|
||||
let wants_pdf = produces_pdf(&config);
|
||||
for file in &captured {
|
||||
let rel = match file.strip_prefix(missions_root()) {
|
||||
Ok(r) => r.to_string_lossy().to_string(),
|
||||
@@ -131,9 +130,13 @@ pub async fn capture_repo_less_phases(pool: &PgPool) -> Result<(), String> {
|
||||
mime: Some(mime_for(file)),
|
||||
title: Some(&title),
|
||||
generated_by_run: None,
|
||||
// `produces` used to be inert — declared by the recipe and
|
||||
// read by nothing. This is the code that reads it.
|
||||
render_pdf: wants_pdf && is_markdown(file),
|
||||
// No PDF. The renderer converted Markdown to HTML by
|
||||
// calling an LLM — a paid API call, per document, on the
|
||||
// critical path of "save my research", which promptly
|
||||
// failed on depleted credits. Markdown IS the deliverable;
|
||||
// it is served by `artifact_content` and styled at render
|
||||
// time, which is free, offline, and cannot 429.
|
||||
render_pdf: false,
|
||||
metadata: Some(serde_json::json!({
|
||||
"bytes": std::fs::metadata(file).map(|m| m.len()).unwrap_or(0),
|
||||
"captured_from": "/mission/repo",
|
||||
@@ -291,19 +294,22 @@ fn outputs_dir(mission_id: Uuid, phase_id: Uuid) -> PathBuf {
|
||||
.join(phase_id.to_string())
|
||||
}
|
||||
|
||||
/// The missions root, for callers that resolve artifact paths against it.
|
||||
pub fn missions_root_dir() -> PathBuf {
|
||||
missions_root()
|
||||
}
|
||||
|
||||
/// The only directory an artifact may be read from.
|
||||
pub fn outputs_root_dir() -> PathBuf {
|
||||
missions_root().join("_outputs")
|
||||
}
|
||||
|
||||
fn missions_root() -> PathBuf {
|
||||
std::env::var("CLAWMATES_MISSIONS_ROOT")
|
||||
.map(PathBuf::from)
|
||||
.unwrap_or_else(|_| PathBuf::from("/var/lib/clawmates-missions"))
|
||||
}
|
||||
|
||||
fn is_markdown(p: &Path) -> bool {
|
||||
matches!(
|
||||
p.extension().and_then(|e| e.to_str()),
|
||||
Some("md") | Some("markdown")
|
||||
)
|
||||
}
|
||||
|
||||
fn mime_for(p: &Path) -> &'static str {
|
||||
match p.extension().and_then(|e| e.to_str()) {
|
||||
Some("md") | Some("markdown") => "text/markdown",
|
||||
@@ -314,16 +320,6 @@ fn mime_for(p: &Path) -> &'static str {
|
||||
}
|
||||
}
|
||||
|
||||
/// Did the recipe ask for a PDF? `research_only.toml` declares
|
||||
/// `produces = ["md","pdf"]`.
|
||||
fn produces_pdf(config: &serde_json::Value) -> bool {
|
||||
config
|
||||
.get("produces")
|
||||
.and_then(|v| v.as_array())
|
||||
.map(|a| a.iter().any(|v| v.as_str() == Some("pdf")))
|
||||
.unwrap_or(false)
|
||||
}
|
||||
|
||||
fn allow_empty(config: &serde_json::Value) -> bool {
|
||||
config.get("allow_empty").and_then(|v| v.as_bool()) == Some(true)
|
||||
}
|
||||
@@ -370,22 +366,48 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// `produces` was registered as an INERT key — declared by the recipe and
|
||||
/// read by nothing, which is why `research_only`'s promised PDF never
|
||||
/// appeared. This is the code that makes it mean something.
|
||||
/// Markdown is the deliverable, so it must be labelled as markdown — the
|
||||
/// viewer decides how to render from the mime type.
|
||||
#[test]
|
||||
fn the_recipes_produces_key_decides_pdf_rendering() {
|
||||
let with = serde_json::json!({ "produces": ["md", "pdf"] });
|
||||
let without = serde_json::json!({ "produces": ["md"] });
|
||||
let absent = serde_json::json!({});
|
||||
assert!(produces_pdf(&with));
|
||||
assert!(!produces_pdf(&without));
|
||||
assert!(!produces_pdf(&absent));
|
||||
fn markdown_is_labelled_so_the_viewer_can_style_it() {
|
||||
assert_eq!(mime_for(Path::new("/x/01_notes.md")), "text/markdown");
|
||||
assert_eq!(mime_for(Path::new("/x/data.json")), "application/json");
|
||||
}
|
||||
|
||||
// Only markdown is rendered; a captured JSON side-file is not a document
|
||||
// to typeset.
|
||||
assert!(is_markdown(Path::new("/x/01_notes.md")));
|
||||
assert!(!is_markdown(Path::new("/x/data.json")));
|
||||
/// The containment rule the content endpoint enforces: everything readable
|
||||
/// lives under `_outputs`, and nothing else does.
|
||||
///
|
||||
/// Artifact paths are written by this server, but they are DATA in a table,
|
||||
/// and a row saying `../../../etc/passwd` must be a 404 rather than a file
|
||||
/// read. The endpoint canonicalises before comparing — checking the string
|
||||
/// first would pass `_outputs/../../etc/passwd` straight through.
|
||||
#[test]
|
||||
fn everything_readable_lives_under_the_outputs_root() {
|
||||
let root = outputs_root_dir();
|
||||
assert!(root.ends_with("_outputs"), "{root:?}");
|
||||
assert!(root.starts_with(missions_root_dir()), "{root:?}");
|
||||
|
||||
// A real capture is inside it...
|
||||
let inside = outputs_dir(Uuid::now_v7(), Uuid::now_v7());
|
||||
assert!(inside.starts_with(&root), "{inside:?}");
|
||||
|
||||
// ...and the traversal shape this guards against is not, once resolved.
|
||||
let escaped = root.join("..").join("..").join("etc/passwd");
|
||||
let normalised: PathBuf = escaped
|
||||
.components()
|
||||
.fold(PathBuf::new(), |mut acc, c| {
|
||||
match c {
|
||||
std::path::Component::ParentDir => {
|
||||
acc.pop();
|
||||
}
|
||||
other => acc.push(other),
|
||||
}
|
||||
acc
|
||||
});
|
||||
assert!(
|
||||
!normalised.starts_with(&root),
|
||||
"a traversal must not resolve back inside the outputs root: {normalised:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// A phase that produced nothing must still leave a marker, or the
|
||||
|
||||
Reference in New Issue
Block a user