refactor(sidecars): typed envelope contract with structured per-file + advisory data

Replaces the previous `event.details.sidecarsUpdated` / `event.details.sidecarAdvisory`
free-form JSON bag with a typed, top-level `Envelope.sidecars[]` list.

## New types (`socket-patch-core/src/patch/sidecars/types.rs`)

  pub struct SidecarRecord { purl, ecosystem, files, advisory }
  pub struct SidecarFile   { path, action: SidecarFileAction }
  pub enum SidecarFileAction { Rewritten | Deleted | Created }
  pub struct SidecarAdvisory { code, severity, message }
  pub enum SidecarAdvisoryCode {
      PypiRecordStale | GemBundleInstallReverts | GoModVerifyFails
      | NugetSignedPackageTampered | SidecarFixupFailed
  }
  pub enum SidecarSeverity { Info | Warning | Error }

All derive `serde::Serialize`. Structs use camelCase; enums use
snake_case. Unit tests pin the JSON contract.

## JSON shape (consumer view)

```json
{
  "command": "apply",
  "events": [...],
  "sidecars": [
    { "purl": "pkg:cargo/...", "ecosystem": "cargo",
      "files": [{"path":".cargo-checksum.json","action":"rewritten"}] },
    { "purl": "pkg:nuget/...", "ecosystem": "nuget",
      "files": [{"path":".nupkg.metadata","action":"deleted"}],
      "advisory": { "code":"nuget_signed_package_tampered",
                    "severity":"warning", "message":"..." } }
  ]
}
```

  - `sidecars` omitted from JSON when empty.
  - `files` always present (possibly `[]` for advisory-only).
  - `advisory` omitted when absent.
  - `code` / `severity` are stable snake_case enum tags; `message`
    is human text.
  - `purl` joins to `events[].purl` for per-event context.

## Three real improvements over the old design

  1. **No more lossy collapse.** NuGet's "deleted `.nupkg.metadata`
     AND has a `.nupkg.sha512` signature" case now carries BOTH
     a file entry AND an advisory. Before, the advisory was
     silently lost when the file entry took its slot.
  2. **Stable codes + severity.** Consumers (CI bots, dashboards,
     telemetry, jq pipelines) can switch on `code` and route on
     `severity` without regex-matching free-form strings.
  3. **Decoupled from events.** Sidecar reporting is a top-level
     `Envelope.sidecars` list. `PatchEvent.details` is no longer
     mixed with `list` / `repair` / `remove`'s command-specific
     bags — sidecar consumers have a typed schema all their own.

## Internal refactor

  - `SidecarOutcome` removed. Per-ecosystem fixups return
    `Result<Option<SidecarPayload>, SidecarError>` (internal
    `SidecarPayload = { files, advisory }`); the dispatcher in
    `sidecars/mod.rs` wraps the payload with PURL + ecosystem to
    produce the `SidecarRecord`.
  - `ApplyResult.sidecars_updated: Vec<String>` and
    `sidecar_advisory: Option<String>` consolidated into a single
    `sidecar: Option<SidecarRecord>` field.
  - Apply CLI's `result_to_event` no longer attaches to
    `event.details`; the run loop now calls
    `env.record_sidecar(record.clone())` after each apply result.
  - `Envelope` gains `sidecars: Vec<SidecarRecord>` field +
    `record_sidecar` method.
  - The error path (`SidecarError` returned by a fixup) is
    converted at the apply boundary into a `SidecarRecord` with
    `advisory.code = SidecarFixupFailed`, `severity = Error`.
    Single uniform shape for consumers.

## Pre-existing test fixups

`in_process_remote_ecosystems_apply.rs` and `in_process_rollback_all_ecosystems.rs`
now set `SOCKET_EXPERIMENTAL_MAVEN=1` / `SOCKET_EXPERIMENTAL_NUGET=1`
when they explicitly exercise those paths. These were broken
silently by the Maven/NuGet runtime gates added in the prior
rebase (the gate was always there in commit 39a2321; tests just
happened not to exercise the maven/nuget paths to a depth where
the skip mattered).

## Test results

  - cargo build --workspace --all-features: clean
  - cargo build --release --workspace: clean (no warnings)
  - cargo clippy --workspace --all-features -- -D warnings: clean
  - cargo test --workspace --all-features: 1021 passed, 0 failed
  - cargo test --features cargo --test e2e_safety_cargo_build --
    --ignored: 5 passed (includes traitobject real-patch round trip)

The e2e cargo test `apply_reports_cargo_checksum_in_sidecars_updated`
tightened from a substring match to a structured-shape assertion
on `envelope.sidecars[].ecosystem=="cargo"` +
`files[].path=".cargo-checksum.json"` + `files[].action=="rewritten"`.

Assisted-by: Claude Code:claude-opus-4-7

Author: mikolalysenko

crates/socket-patch-cli/src/commands/apply.rs

@@ -133,36 +133,12 @@ pub(crate) fn result_to_event(result: &ApplyResult, dry_run: bool) -> PatchEvent
                 .map(AppliedVia::from_core),
         })
         .collect();
-    let mut event = PatchEvent::new(PatchAction::Applied, purl).with_files(files);
-    // Carry ecosystem sidecar fixup outcomes under `details` —
-    // narrower JSON contract than first-class fields (see plan).
-    // Consumers read `event.details.sidecarsUpdated` and
-    // `event.details.sidecarAdvisory`. Only attach when either is
-    // non-empty so events for ecosystems with no sidecar (npm,
-    // yarn) stay quiet.
-    if !result.sidecars_updated.is_empty() || result.sidecar_advisory.is_some() {
-        let mut details = serde_json::Map::new();
-        if !result.sidecars_updated.is_empty() {
-            details.insert(
-                "sidecarsUpdated".to_string(),
-                serde_json::Value::Array(
-                    result
-                        .sidecars_updated
-                        .iter()
-                        .map(|s| serde_json::Value::String(s.clone()))
-                        .collect(),
-                ),
-            );
-        }
-        if let Some(ref advisory) = result.sidecar_advisory {
-            details.insert(
-                "sidecarAdvisory".to_string(),
-                serde_json::Value::String(advisory.clone()),
-            );
-        }
-        event = event.with_details(serde_json::Value::Object(details));
-    }
-    event
+    // Sidecar data is NOT attached here — it's surfaced at the
+    // envelope level under `Envelope.sidecars[]` by the run loop.
+    // See `Envelope::record_sidecar`. Keeping events clean of
+    // sidecar info means each event describes only the apply
+    // action; sidecar reporting is a separate, JOIN-able list.
+    PatchEvent::new(PatchAction::Applied, purl).with_files(files)
 }
 
 pub async fn run(args: ApplyArgs) -> i32 {
@@ -253,6 +229,13 @@ pub async fn run(args: ApplyArgs) -> i32 {
                 env.dry_run = args.common.dry_run;
                 for result in &results {
                     env.record(result_to_event(result, args.common.dry_run));
+                    // Sidecar records live on the envelope, not on
+                    // individual events. Consumers iterate
+                    // `envelope.sidecars[]` and JOIN against
+                    // `events[]` by `purl` for per-package context.
+                    if let Some(ref sidecar) = result.sidecar {
+                        env.record_sidecar(sidecar.clone());
+                    }
                 }
                 // Manifest entries that targeted in-scope ecosystems but
                 // had no installed package on disk — emit one Skipped
@@ -792,8 +775,7 @@ mod tests {
             files_patched: vec!["package/index.js".to_string()],
             applied_via,
             error: None,
-            sidecars_updated: Vec::new(),
-            sidecar_advisory: None,
+            sidecar: None,
         }
     }
 
@@ -868,8 +850,7 @@ mod tests {
             ],
             applied_via,
             error: None,
-            sidecars_updated: Vec::new(),
-            sidecar_advisory: None,
+            sidecar: None,
         };
 
         let event = result_to_event(&result, false);

crates/socket-patch-cli/src/json_envelope.rs

@@ -26,6 +26,11 @@
 
 use serde::Serialize;
 
+pub use socket_patch_core::patch::sidecars::{
+    SidecarAdvisory, SidecarAdvisoryCode, SidecarFile, SidecarFileAction, SidecarRecord,
+    SidecarSeverity,
+};
+
 /// Top-level JSON envelope emitted by every `--json` invocation.
 #[derive(Debug, Clone, Serialize)]
 #[serde(rename_all = "camelCase")]
@@ -53,6 +58,22 @@ pub struct Envelope {
     /// mode, etc.). Implies `events` is empty.
     #[serde(skip_serializing_if = "Option::is_none")]
     pub error: Option<EnvelopeError>,
+    /// Per-package sidecar fixup records. Each entry describes what
+    /// the post-apply integrity fixup did for one package — rewriting
+    /// `.cargo-checksum.json`, deleting `.nupkg.metadata`, surfacing
+    /// an advisory for PyPI / gem / Go, etc.
+    ///
+    /// Top-level (not per-event) so consumers can iterate sidecar
+    /// outcomes directly with `jq '.sidecars[]'`. Records carry
+    /// `purl` so a consumer that needs the matching apply event can
+    /// JOIN against `events[]`.
+    ///
+    /// Empty (and omitted from JSON via `skip_serializing_if`) for
+    /// commands that don't produce sidecar work — `rollback`,
+    /// `repair`, `list`, etc. — and for apply runs against ecosystems
+    /// with no sidecar contract (e.g. npm).
+    #[serde(skip_serializing_if = "Vec::is_empty")]
+    pub sidecars: Vec<SidecarRecord>,
 }
 
 impl Envelope {
@@ -67,6 +88,7 @@ impl Envelope {
             events: Vec::new(),
             summary: Summary::default(),
             error: None,
+            sidecars: Vec::new(),
         }
     }
 
@@ -78,6 +100,13 @@ impl Envelope {
         self.events.push(event);
     }
 
+    /// Append a sidecar fixup record. Called once per `ApplyResult`
+    /// whose `sidecar` field is `Some`. Order matches the order
+    /// `apply` processed packages, which is best-effort.
+    pub fn record_sidecar(&mut self, sidecar: SidecarRecord) {
+        self.sidecars.push(sidecar);
+    }
+
     /// Mark the run as a partial failure. Idempotent.
     pub fn mark_partial_failure(&mut self) {
         if !matches!(self.status, Status::Error) {

crates/socket-patch-cli/tests/e2e_safety_cargo_build.rs

@@ -372,8 +372,17 @@ fn apply_then_cargo_check_succeeds() {
 }
 
 /// JSON envelope sanity check on the same scenario: assert apply
-/// reports the sidecar in `sidecarsUpdated`. Locked in as part of
-/// the JSON contract.
+/// reports the cargo sidecar in the new top-level `envelope.sidecars[]`
+/// list with the structured shape.
+///
+/// Locks in the typed JSON contract that downstream consumers
+/// (jq pipelines, dashboards, telemetry) rely on:
+///   envelope.sidecars[].ecosystem == "cargo"
+///   envelope.sidecars[].files[i].path == ".cargo-checksum.json"
+///   envelope.sidecars[].files[i].action == "rewritten"
+///
+/// If a refactor flips key names or moves the data elsewhere, this
+/// test fires loudly.
 #[test]
 #[ignore]
 fn apply_reports_cargo_checksum_in_sidecars_updated() {
@@ -392,14 +401,38 @@ fn apply_reports_cargo_checksum_in_sidecars_updated() {
         &["apply", "--json", "--cwd", consumer.to_str().unwrap()],
     );
 
-    // Apply may exit 0 (success) or surface a warning event; the
-    // contract we pin here is "the per-package result reports the
-    // cargo checksum file under sidecarsUpdated".
     let env = parse_json_envelope(&stdout);
-    let serialized = serde_json::to_string(&env).unwrap();
+    let sidecars = env["sidecars"]
+        .as_array()
+        .unwrap_or_else(|| panic!(
+            "envelope must carry `sidecars` array.\nstdout:\n{stdout}\nstderr:\n{stderr}"
+        ));
+    let cargo_record = sidecars
+        .iter()
+        .find(|s| s["ecosystem"] == "cargo")
+        .unwrap_or_else(|| panic!(
+            "envelope.sidecars must contain a record with ecosystem=cargo.\nstdout:\n{stdout}"
+        ));
+    let files = cargo_record["files"].as_array().expect("files array");
+    assert!(
+        files.iter().any(|f| {
+            f["path"] == ".cargo-checksum.json" && f["action"] == "rewritten"
+        }),
+        "expected files[] to contain {{path:.cargo-checksum.json, action:rewritten}}; got {cargo_record}"
+    );
+    // No advisory expected for the cargo success path.
+    assert!(
+        cargo_record.get("advisory").is_none()
+            || cargo_record["advisory"].is_null(),
+        "cargo success path should not carry an advisory; got {cargo_record}"
+    );
+    // PURL is denormalized into the record for jq filtering.
     assert!(
-        serialized.contains(".cargo-checksum.json"),
-        "apply --json should mention .cargo-checksum.json in sidecarsUpdated.\nstdout:\n{stdout}\nstderr:\n{stderr}"
+        cargo_record["purl"]
+            .as_str()
+            .map(|p| p.starts_with("pkg:cargo/"))
+            .unwrap_or(false),
+        "sidecar record must carry the PURL; got {cargo_record}"
     );
 }
 

crates/socket-patch-cli/tests/in_process_remote_ecosystems_apply.rs

@@ -200,6 +200,10 @@ async fn maven_handcrafted_install_apply_patches_file() {
     let after_hash = git_sha256(&patched);
 
     std::env::set_var("MAVEN_REPO_LOCAL", &repo);
+    // Maven crawler is runtime-gated behind this env var (see
+    // `ecosystem_dispatch::maven_runtime_enabled`). The test
+    // deliberately exercises the Maven apply path, so opt in.
+    std::env::set_var("SOCKET_EXPERIMENTAL_MAVEN", "1");
 
     let server = MockServer::start().await;
     setup_apply_mock(
@@ -225,6 +229,7 @@ async fn maven_handcrafted_install_apply_patches_file() {
     );
 
     std::env::remove_var("MAVEN_REPO_LOCAL");
+    std::env::remove_var("SOCKET_EXPERIMENTAL_MAVEN");
 }
 
 // ---------------------------------------------------------------------------
@@ -319,6 +324,10 @@ async fn nuget_handcrafted_install_apply_patches_file() {
     let after_hash = git_sha256(&patched);
 
     std::env::set_var("NUGET_PACKAGES", &packages);
+    // NuGet crawler is runtime-gated behind this env var (see
+    // `ecosystem_dispatch::nuget_runtime_enabled`). The test
+    // deliberately exercises the NuGet apply path, so opt in.
+    std::env::set_var("SOCKET_EXPERIMENTAL_NUGET", "1");
 
     let server = MockServer::start().await;
     setup_apply_mock(
@@ -344,6 +353,7 @@ async fn nuget_handcrafted_install_apply_patches_file() {
     );
 
     std::env::remove_var("NUGET_PACKAGES");
+    std::env::remove_var("SOCKET_EXPERIMENTAL_NUGET");
 }
 
 // ---------------------------------------------------------------------------
@@ -389,6 +399,7 @@ async fn maven_handcrafted_discovery() {
     std::fs::create_dir_all(&version_dir).unwrap();
     std::fs::write(version_dir.join("foo-1.0.0.pom"), "<project/>").unwrap();
     std::env::set_var("MAVEN_REPO_LOCAL", &repo);
+    std::env::set_var("SOCKET_EXPERIMENTAL_MAVEN", "1");
 
     let server = MockServer::start().await;
     Mock::given(method("POST"))
@@ -403,6 +414,7 @@ async fn maven_handcrafted_discovery() {
     args.sync = false;
     assert_eq!(scan_run(args).await, 0);
     std::env::remove_var("MAVEN_REPO_LOCAL");
+    std::env::remove_var("SOCKET_EXPERIMENTAL_MAVEN");
 }
 
 #[tokio::test]
@@ -414,6 +426,7 @@ async fn nuget_handcrafted_discovery() {
     std::fs::create_dir_all(&dir).unwrap();
     std::fs::write(dir.join("foo.nuspec"), "<package/>").unwrap();
     std::env::set_var("NUGET_PACKAGES", &pkgs);
+    std::env::set_var("SOCKET_EXPERIMENTAL_NUGET", "1");
 
     let server = MockServer::start().await;
     Mock::given(method("POST"))
@@ -428,6 +441,7 @@ async fn nuget_handcrafted_discovery() {
     args.sync = false;
     assert_eq!(scan_run(args).await, 0);
     std::env::remove_var("NUGET_PACKAGES");
+    std::env::remove_var("SOCKET_EXPERIMENTAL_NUGET");
 }
 
 // Helper kept around so `PathBuf` import is used in case of future tests.

crates/socket-patch-cli/tests/in_process_rollback_all_ecosystems.rs

@@ -351,10 +351,13 @@ async fn rollback_maven_restores_original_content() {
     std::fs::write(blobs.join(&before_hash), original).unwrap();
 
     std::env::set_var("MAVEN_REPO_LOCAL", &repo);
+    // Maven crawler is runtime-gated; opt in for the test.
+    std::env::set_var("SOCKET_EXPERIMENTAL_MAVEN", "1");
     let mut args = default_rollback_args(tmp.path(), "maven");
     args.common.global = true;
     let _ = rollback_run(args).await;
     std::env::remove_var("MAVEN_REPO_LOCAL");
+    std::env::remove_var("SOCKET_EXPERIMENTAL_MAVEN");
 
     assert_eq!(
         std::fs::read(version_dir.join("LICENSE.txt")).unwrap(),
@@ -440,10 +443,13 @@ async fn rollback_nuget_restores_original_content() {
     std::fs::write(blobs.join(&before_hash), original).unwrap();
 
     std::env::set_var("NUGET_PACKAGES", &packages);
+    // NuGet crawler is runtime-gated; opt in for the test.
+    std::env::set_var("SOCKET_EXPERIMENTAL_NUGET", "1");
     let mut args = default_rollback_args(tmp.path(), "nuget");
     args.common.global = true;
     let _ = rollback_run(args).await;
     std::env::remove_var("NUGET_PACKAGES");
+    std::env::remove_var("SOCKET_EXPERIMENTAL_NUGET");
 
     assert_eq!(
         std::fs::read(pkg_dir.join("LICENSE.md")).unwrap(),

crates/socket-patch-core/src/patch/apply.rs

@@ -92,15 +92,15 @@ pub struct ApplyResult {
     /// populated for files in `files_patched`.
     pub applied_via: HashMap<String, AppliedVia>,
     pub error: Option<String>,
-    /// Ecosystem sidecar files that were rewritten or deleted as part
-    /// of this apply (e.g. `.cargo-checksum.json`, `.nupkg.metadata`).
-    /// Paths are relative to `pkg_path`. Empty when no sidecar
-    /// applied or when the ecosystem only emits an advisory.
-    pub sidecars_updated: Vec<String>,
-    /// One-line advisory for the operator about post-apply tooling
-    /// behavior (e.g. "PyPI: pip check may flag RECORD inconsistency").
-    /// None when no advisory applies.
-    pub sidecar_advisory: Option<String>,
+    /// Ecosystem sidecar fixup outcome — a typed
+    /// [`SidecarRecord`](crate::patch::sidecars::SidecarRecord) carrying
+    /// per-file actions (rewritten / deleted / created) and an
+    /// optional structured advisory. `None` when no sidecar
+    /// applied (e.g. npm) or when no files were patched.
+    ///
+    /// Surfaced in the CLI JSON envelope under
+    /// `Envelope.sidecars[]` (top-level, not per-event).
+    pub sidecar: Option<crate::patch::sidecars::SidecarRecord>,
 }
 
 /// Normalize file path by removing the "package/" prefix if present.
@@ -456,8 +456,7 @@ pub async fn apply_package_patch(
         files_patched: Vec::new(),
         applied_via: HashMap::new(),
         error: None,
-        sidecars_updated: Vec::new(),
-        sidecar_advisory: None,
+        sidecar: None,
     };
 
     // First, verify all files
@@ -629,30 +628,32 @@ pub async fn apply_package_patch(
 
     // Ecosystem sidecar fixup. Best-effort: a failing sidecar does
     // NOT undo the patch (the bytes were committed atomically via
-    // stage+rename; nothing to roll back). Errors surface as an
-    // advisory string so the CLI envelope can carry them under
-    // `event.details`.
+    // stage+rename; nothing to roll back). The error path is
+    // converted at this boundary into a `SidecarRecord` carrying
+    // `SidecarAdvisoryCode::SidecarFixupFailed` so downstream
+    // consumers see a uniform shape regardless of whether the
+    // fixup succeeded, was advisory-only, or raised an error.
     if !result.files_patched.is_empty() {
-        match crate::patch::sidecars::dispatch_fixup(
-            package_key,
-            pkg_path,
-            &result.files_patched,
-            files,
-        )
-        .await
-        {
-            Ok(crate::patch::sidecars::SidecarOutcome::Updated(touched)) => {
-                result.sidecars_updated = touched;
-            }
-            Ok(crate::patch::sidecars::SidecarOutcome::Advisory(msg)) => {
-                result.sidecar_advisory = Some(msg);
-            }
-            Ok(crate::patch::sidecars::SidecarOutcome::None) => {}
+        use crate::patch::sidecars::{
+            dispatch_fixup, SidecarAdvisory, SidecarAdvisoryCode, SidecarRecord, SidecarSeverity,
+        };
+        match dispatch_fixup(package_key, pkg_path, &result.files_patched, files).await {
+            Ok(Some(record)) => result.sidecar = Some(record),
+            Ok(None) => {}
             Err(e) => {
-                result.sidecar_advisory = Some(format!(
-                    "sidecar fixup failed (patch still applied): {}",
-                    e
-                ));
+                let ecosystem = crate::crawlers::Ecosystem::from_purl(package_key)
+                    .map(|eco| eco.cli_name().to_string())
+                    .unwrap_or_else(|| "unknown".to_string());
+                result.sidecar = Some(SidecarRecord {
+                    purl: package_key.to_string(),
+                    ecosystem,
+                    files: Vec::new(),
+                    advisory: Some(SidecarAdvisory {
+                        code: SidecarAdvisoryCode::SidecarFixupFailed,
+                        severity: SidecarSeverity::Error,
+                        message: format!("sidecar fixup failed (patch still applied): {}", e),
+                    }),
+                });
             }
         }
     }