test: fix stale malformed-label e2e test, split by failure site

crates/outrig/src/container/embedded.rs

@@ -672,6 +672,15 @@ mod tests {
         assert_eq!(labels[LABEL_SCHEMA], LABEL_SCHEMA_VERSION);
     }
 
+    #[test]
+    fn merged_mcp_config_labels_reject_malformed_inherited_label() {
+        let inherited = BTreeMap::from([(LABEL_MCP.to_string(), r#"{"fs": ["#.to_string())]);
+
+        let err = merged_mcp_config_to_labels("img", &inherited, &BTreeMap::new()).unwrap_err();
+
+        assert!(matches!(err, OutrigError::EmbeddedImageConfigParse { .. }));
+    }
+
     #[test]
     fn standalone_image_labels_read_metadata_and_mcp() {
         let mut mcp = BTreeMap::new();

crates/outrig/tests/embedded_image.rs

@@ -51,6 +51,35 @@ fn mcp_label_json(mcp: &BTreeMap<String, McpServerSpec>) -> String {
     serde_json::to_string(mcp).expect("serialize mcp label json")
 }
 
+/// Tempdir image context whose Dockerfile carries only a malformed
+/// `org.outrig.mcp` label on the alpine base -- the parse failure under test
+/// never touches the layer contents, so no tooling is installed.
+fn malformed_label_context() -> tempfile::TempDir {
+    let ctx = tempfile::tempdir().expect("tempdir image context");
+    let dockerfile = format!(
+        "FROM docker.io/library/alpine:latest\n{}",
+        label_line(embedded::LABEL_MCP, r#"{"fs": ["#),
+    );
+    std::fs::write(ctx.path().join("Dockerfile"), dockerfile).expect("write Dockerfile");
+    ctx
+}
+
+/// `ensure_image` over a Build-source config rooted at `project_dir` (which
+/// must contain a `Dockerfile`). Returns the result so tests can assert on
+/// build-time failures.
+async fn try_ensure_built_image(project_dir: &Path) -> outrig::error::Result<ImageTag> {
+    let cfg = ImageConfig {
+        image_name: None,
+        dockerfile: Some("Dockerfile".into()),
+        context: Some(".".into()),
+        build_args: BTreeMap::new(),
+        security: Default::default(),
+        mcp: BTreeMap::new(),
+        sidecars: BTreeMap::new(),
+    };
+    Ok(image::ensure_image(&cfg, project_dir, false).await?.tag)
+}
+
 /// Build an alpine + filesystem-MCP fixture image carrying `labels` (stamped via
 /// Dockerfile `LABEL`, mirroring what `outrig image build` stamps via
 /// `--label`). An empty slice builds a label-free image.
@@ -65,20 +94,9 @@ async fn ensure_image_with_labels(labels: &[(&str, &str)]) -> ImageTag {
         dockerfile.push_str(&label_line(key, value));
     }
     std::fs::write(ctx.path().join("Dockerfile"), dockerfile).expect("write Dockerfile");
-
-    let cfg = ImageConfig {
-        image_name: None,
-        dockerfile: Some("Dockerfile".into()),
-        context: Some(".".into()),
-        build_args: BTreeMap::new(),
-        security: Default::default(),
-        mcp: BTreeMap::new(),
-        sidecars: BTreeMap::new(),
-    };
-    image::ensure_image(&cfg, ctx.path(), false)
+    try_ensure_built_image(ctx.path())
         .await
         .expect("ensure embedded fixture image")
-        .tag
 }
 
 /// Convenience over [`ensure_image_with_labels`] for the common case: stamp just
@@ -91,19 +109,9 @@ async fn ensure_image_with_mcp(mcp: &BTreeMap<String, McpServerSpec>) -> ImageTa
 /// Build a label-free image (the committed mcp-fs fixture has the tool binaries
 /// but no `org.outrig.*` labels) -- runtime must fall back to repo config.
 async fn ensure_label_free_image() -> ImageTag {
-    let cfg = ImageConfig {
-        image_name: None,
-        dockerfile: Some("Dockerfile".into()),
-        context: Some(".".into()),
-        build_args: BTreeMap::new(),
-        security: Default::default(),
-        mcp: BTreeMap::new(),
-        sidecars: BTreeMap::new(),
-    };
-    image::ensure_image(&cfg, &fixture_mcp_fs_dir(), false)
+    try_ensure_built_image(&fixture_mcp_fs_dir())
         .await
         .expect("ensure mcp-fs fixture image")
-        .tag
 }
 
 async fn start_and_bootstrap(image: &ImageTag, host_ws: &Path) -> Container {
@@ -237,19 +245,64 @@ async fn missing_label_falls_back_to_config() {
 }
 
 #[tokio::test]
-async fn malformed_mcp_label_is_hard_error() {
+async fn malformed_mcp_label_fails_ensure_image() {
     common::init_tracing();
     let _guard = E2E_LOCK.lock().await;
-    let image = ensure_image_with_labels(&[(embedded::LABEL_MCP, r#"{"fs": ["#)]).await;
-    let host_ws = tempfile::tempdir().expect("tempdir host_ws");
-    let container = start_and_bootstrap(&image, host_ws.path()).await;
+    // Repo builds re-merge inherited labels into the cache tag
+    // (stamp_repo_image_labels), so a malformed org.outrig.mcp label fails the
+    // build itself rather than the later runtime read.
+    let ctx = malformed_label_context();
 
-    let err = embedded::merged_mcp(&container, &BTreeMap::new())
+    let err = try_ensure_built_image(ctx.path())
         .await
-        .expect_err("malformed org.outrig.mcp label should fail");
+        .expect_err("malformed org.outrig.mcp label should fail ensure_image");
     assert!(matches!(err, OutrigError::EmbeddedImageConfigParse { .. }));
+}
 
-    container.stop(Duration::from_secs(2)).await.expect("stop");
+#[tokio::test]
+async fn malformed_mcp_label_on_raw_image_fails_runtime_read() {
+    common::init_tracing();
+    let _guard = E2E_LOCK.lock().await;
+    // A raw (non-built) image ref skips the build path's label re-merge, so a
+    // malformed label must still hard-error at the runtime read. The standalone
+    // build stamps nothing beyond the Dockerfile's own labels, and the fixed
+    // tag is rebuilt on every run, so nothing accumulates in local storage.
+    let ctx = malformed_label_context();
+    let tag = ImageTag("outrig-raw-malformed:e2e".to_string());
+    image::build_standalone(
+        ctx.path(),
+        Path::new("Dockerfile"),
+        Path::new("."),
+        &tag,
+        false,
+        &BTreeMap::new(),
+    )
+    .await
+    .expect("build raw fixture image");
+
+    // ensure_image on an image-ref config only probes/pulls -- it must not
+    // re-validate labels, so the malformed image passes here.
+    let cfg = ImageConfig {
+        image_name: Some(tag.0.clone()),
+        dockerfile: None,
+        context: None,
+        build_args: BTreeMap::new(),
+        security: Default::default(),
+        mcp: BTreeMap::new(),
+        sidecars: BTreeMap::new(),
+    };
+    let image = image::ensure_image(&cfg, ctx.path(), false)
+        .await
+        .expect("raw image ref should pass ensure_image without label validation")
+        .tag;
+
+    // merged_mcp reads labels off the handle's image tag without ever exec'ing
+    // into the container, so an attached handle stands in for a running one.
+    let container = Container::attach("raw-malformed-runtime-read", image, None, None);
+    let err = embedded::merged_mcp(&container, &BTreeMap::new())
+        .await
+        .expect_err("malformed org.outrig.mcp label should fail the runtime read");
+    assert!(matches!(err, OutrigError::EmbeddedImageConfigParse { .. }));
 }
 
 #[tokio::test]

plan/done/0083-malformed-label-e2e-stale.md

@@ -0,0 +1,50 @@
+# 0083 -- Fix stale `malformed_mcp_label_is_hard_error` e2e test
+
+## Goal
+
+Repair the stale e2e test. `cargo test -p outrig --features e2e --test embedded_image
+malformed_mcp_label` fails on trunk (verified against a clean tree during task 0079): the
+test builds an image whose Dockerfile stamps a malformed `org.outrig.mcp` label and
+expects `ensure_image` to succeed so the *runtime* read (`embedded::merged_mcp`) can fail.
+Since the build path started re-merging inherited labels into the cache tag
+(`merged_mcp_config_to_labels` at `image.rs:658`), the malformed label hard-errors during
+`ensure_image` itself and the test's `.expect("ensure embedded fixture image")` panics
+before the assertion runs.
+
+## Deliverables
+
+The behavior is arguably better (fail at build), so the fix is probably to update the
+test to assert `ensure_image` fails with `EmbeddedImageConfigParse` -- and add a separate
+runtime-read case using a raw (non-built) image ref if that path still needs coverage.
+
+## Acceptance
+
+- `cargo test -p outrig --features e2e --test embedded_image malformed_mcp_label` passes
+  on a clean tree.
+- The runtime-read failure path (`embedded::merged_mcp`) either keeps coverage via a
+  raw-image-ref case or is shown to be unreachable.
+
+## Dependencies
+
+None (follows up on completed task 0079).
+
+## Decisions
+
+- Split the stale test into two, both keeping the `malformed_mcp_label` filter substring:
+  `malformed_mcp_label_fails_ensure_image` (build path hard-errors during the label
+  re-merge) and `malformed_mcp_label_on_raw_image_fails_runtime_read` (raw image ref
+  passes `ensure_image` untouched, fails at `embedded::merged_mcp`). The old name was
+  dropped -- it collided with the unit test `embedded_mcp_malformed_label_is_hard_error`.
+- The runtime-read path is reachable only via the `ImageSourceRef::Image` branch, which
+  never re-validates labels; the raw fixture is built with `image::build_standalone`
+  (stamps nothing beyond the Dockerfile's own labels) rather than a hand-rolled buildah
+  invocation, under a fixed tag rebuilt each run so local storage doesn't accumulate.
+- `merged_mcp` only reads labels off the handle's image tag, so the runtime-read test
+  uses `Container::attach` instead of a full start/bootstrap/stop cycle, and both
+  malformed-label fixtures are bare `FROM alpine` + `LABEL` (no node toolchain, no
+  shadow) -- the parse failure never touches layer contents.
+- Runtime coverage stays library-level in `crates/outrig/tests/embedded_image.rs` (it
+  exercises `embedded::merged_mcp` directly); the CLI test file keeps its own raw-image
+  CLI cases.
+- Bonus non-e2e regression test: `merged_mcp_config_labels_reject_malformed_inherited_label`
+  pins the new hard-error site (`merged_mcp_config_to_labels`) in plain `cargo test`.

plan/todo/README.md

@@ -8,7 +8,6 @@ See [`.claude/CLAUDE.md`](../../.claude/CLAUDE.md) for the workflow conventions.
 
 | Task | Title                                              | Dependencies |
 |------|----------------------------------------------------|--------------|
-| 0083 | Fix stale `malformed_mcp_label_is_hard_error` test | --           |
 | 0084 | REPL slash-command dispatcher                      | --           |
 | 0085 | from_image_config sidecar translation              | --           |
 | 0086 | Sidecar startup and clean-sweep performance        | --           |