diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index d52add4f8..ccf43586c 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -187,7 +187,7 @@ For a **9.0 root lock**, the CLI ensures `pnpm-workspace.yaml` carries `trustLoc **Agent-flow run-level warnings (additive).** An agent-mode apply (`--mode agent` / `--sync`, `--json`) may add a top-level `warnings[]` array of `{code, detail}` entries to the scan envelope (absent when none fired; each is also mirrored to stderr unless `--silent`). They surface cross-mode state the apply cannot change — never a status or exit-code change (hosted refusals set the precedent: exit 0 + warning). Codes (stable; new codes are additive/MINOR): `vendored_ownership_retained` — vendor-owned package(s) were skipped before download (the per-patch `skipped`/`vendored` records in `apply.patches[]` are unchanged); the detail names the purls and the migration path (`remove `, or `vendor --revert` which unwinds every vendored package, then re-run). `hosted_wiring_retained` — the lockfiles still pin scanned package(s) to a hosted patch (the agent run does not unwind hosted wiring — as of v5.0 that is `socket-patch rollback`'s job, which restores the upstream registry entries, or `remove ` per package); the detail names the purls and the options (stay `--mode hosted`, migrate via `scan --mode vendored`, or `socket-patch rollback`). The warning keys on the hosted pins lockfile discovery finds at scan time, so a flow that restored the upstream entries retires it. The human path prints the same `hosted_wiring_retained` text to stderr after an apply; the vendored counterpart is already covered by its per-package `[skip] … (vendored …)` lines. `ownership_not_restored` (v5.0; `apply` and `rollback` `warnings[]` alike) — a file WAS patched (or restored) but its ownership could not be put back to the original uid/gid (the mode is still restored last); the detail is `: : patched, but ownership could not be restored to uid N gid M: ` and the human line `Warning: ` (stderr, muted by `--silent`); never a status or exit change. `cargo_build_cache_stale` (v5.0; `apply` and `rollback` `warnings[]` alike) — a cargo crate's bytes changed but its compiled copy in a project build directory could not be invalidated (a fingerprint directory that could not be removed, or a `build.build-dir` with a `{workspace-path-hash}` template); the detail names the crates to `cargo clean -p` before the next build, which may otherwise link the stale code; never a status or exit change. -`scan --prune` opts into garbage collection. When set, `scan` removes manifest entries for packages no longer present in the crawl, then deletes orphan blob files, and every obsolete diff and package archive, from `.socket/`. A blob is an orphan when no patch left in the manifest references it: the afterHash and beforeHash blobs of every remaining patch are kept, the same retention policy `repair` uses (the beforeHash blobs are an offline rollback's only restore data, and `repair` downloads afterHash blobs only). Diff and legacy package archives are never kept, even for a remaining patch: nothing reads them any more. Off by default (v3.0) so a temporary uninstall doesn't silently destroy manifest state. Only entries whose ecosystem this run actually crawled are eligible: a `pkg:/` with no crawler in this build (a newer CLI's ecosystem in the committed manifest) is exempt — the crawl never looked for them, so their absence is not evidence of removal (same fail-safe as the `--ecosystems` filter, which narrows the query but never the prune's installed set). A Cargo entry is also exempt while its agent-mode copy is still patched: the project crawl looks up only the crates `Cargo.lock` resolves, but a crate the lock bumped or dropped keeps its patched copy in the machine-wide `$CARGO_HOME/registry/src` cache (nothing deletes it), and the entry holds the only blobs that can restore that copy. The wet pass keeps it with a `cargo_cache_patch_kept` warning naming the `socket-patch rollback ` that restores the copy and drops the entry; the preview leaves it out of `prunableManifestEntries`. The pass also reconciles vendored state (runs FIRST, under ONE apply-lock acquisition shared with the manifest prune — lock contention skips the whole pass without failing the scan; `--lock-timeout` is honored and a lock I/O error is reported rather than swallowed; the existence gate — a manifest file OR a vendor ledger file, both cheap stats; an emptied ledger is deleted on save, so its presence is its content proxy — runs BEFORE the lock, so a bare project never gets a `.socket/`; in the vendored scan arms the pass runs AFTER the vendor step): (a) ledger entries still tracked by a manifest record (manifest-mode entries written by standalone `vendor`) whose patch is gone from the manifest are reverted — `detached` entries (every `scan`/`get --mode vendored` entry, v5.0) have no manifest record to lose and are exempt from this leg; (b) EVERY ledger entry whose dependency is no longer in the lockfile graph is reverted and any manifest entry it still had dropped (v5.0: the check is about the lockfile, not the manifest, so embedded-record entries are no longer exempt; a missing or undeterminable lockfile keeps the entry, fail-safe); and (c) orphan `.socket/vendor//` dirs with no ledger entry are swept. The prune never deletes a zero-patch `.socket/manifest.json` (its `{"patches": {}}` + `setup` block stay). The JSON `gc` sub-object gains `revertedVendoredEntries` + `keptVendoredEntries` + `failedVendoredEntries` + `removedVendorOrphanDirs` (wet) / `revertableVendoredEntries` + `vendorOrphanDirs` (preview), plus two ADDITIVE wet-only keys: `skipped: {code, message}` — present exactly when the pass was skipped at the lock (`lock_held` | `lock_io`; every count is then zero) — and `warnings: [{code, detail}]` — `vendor_state_write_failed` / `manifest_write_failed` (entries were reverted but the ledger or manifest rewrite failed), `cleanup_failed` (an orphan sweep failed mid-way), `cargo_cache_patch_kept` (see above), and the reinstall advisories of the vendored reverts (`vendor_bun_reinstall_required`, `vendor_vlt_reinstall_required`, `vendor_pypi_reinstall_required`; a revert's other warnings, such as `vendor_lock_entry_removed` or a drift keep's, are not repeated here). Human mode prints `GC: skipped (): .`, one `GC: .` line per warning, and `GC: failed to revert N vendored entries: …` (singular for one) for `failedVendoredEntries`. `keptVendoredEntries` lists drift-kept entries the revert deliberately preserved (`vendor_artifact_kept` — undo the drift and re-run `vendor --revert` to finish); the preview cannot see drift (backends return before the wiring replay on dry runs), so `revertableVendoredEntries` may over-promise what a wet run will actually reclaim. +`scan --prune` opts into garbage collection. When set, `scan` removes manifest entries for packages no longer present in the crawl, then deletes orphan blob files, and every obsolete diff and package archive, from `.socket/`. A blob is an orphan when no patch left in the manifest references it: the afterHash and beforeHash blobs of every remaining patch are kept, the same retention policy `repair` uses (the beforeHash blobs are an offline rollback's only restore data, and `repair` downloads afterHash blobs only). Diff and legacy package archives are never kept, even for a remaining patch: nothing reads them any more. Off by default (v3.0) so a temporary uninstall doesn't silently destroy manifest state. Only entries whose ecosystem this run actually crawled are eligible: a `pkg:/` with no crawler in this build (a newer CLI's ecosystem in the committed manifest) is exempt — the crawl never looked for them, so their absence is not evidence of removal (same fail-safe as the `--ecosystems` filter, which narrows the query but never the prune's installed set). A Cargo entry is also exempt while its agent-mode copy is still patched: the project crawl looks up only the crates `Cargo.lock` resolves, but a crate the lock bumped or dropped keeps its patched copy in the machine-wide `$CARGO_HOME/registry/src` cache (nothing deletes it), and the entry holds the only blobs that can restore that copy. The wet pass keeps it with a `cargo_cache_patch_kept` warning naming the `socket-patch rollback ` that restores the copy and drops the entry; the `--dry-run` preview leaves it out of `prunedManifestEntries` (and, like every `warnings` entry, reports no warning for it). The pass also reconciles vendored state (runs FIRST, under ONE apply-lock acquisition shared with the manifest prune — lock contention skips the whole pass without failing the scan; `--lock-timeout` is honored and a lock I/O error is reported rather than swallowed; the existence gate — a manifest file OR a vendor ledger file, both cheap stats; an emptied ledger is deleted on save, so its presence is its content proxy — runs BEFORE the lock, so a bare project never gets a `.socket/`; in the vendored scan arms the pass runs AFTER the vendor step): (a) ledger entries still tracked by a manifest record (manifest-mode entries written by standalone `vendor`) whose patch is gone from the manifest are reverted — `detached` entries (every `scan`/`get --mode vendored` entry, v5.0) have no manifest record to lose and are exempt from this leg; (b) EVERY ledger entry whose dependency is no longer in the lockfile graph is reverted and any manifest entry it still had dropped (v5.0: the check is about the lockfile, not the manifest, so embedded-record entries are no longer exempt; a missing or undeterminable lockfile keeps the entry, fail-safe); and (c) orphan `.socket/vendor//` dirs with no ledger entry are swept. The prune never deletes a zero-patch `.socket/manifest.json` (its `{"patches": {}}` + `setup` block stay). The JSON `gc` sub-object gains `revertedVendoredEntries` + `removedVendorOrphanDirs` (on a `--dry-run` preview, what the pass would revert and sweep, under the same keys — see "One GC shape") + the wet-only `keptVendoredEntries` + `failedVendoredEntries`, plus two ADDITIVE wet-only keys: `skipped: {code, message}` — present exactly when the pass was skipped at the lock (`lock_held` | `lock_io`; every count is then zero) — and `warnings: [{code, detail}]` — `vendor_state_write_failed` / `manifest_write_failed` (entries were reverted but the ledger or manifest rewrite failed), `cleanup_failed` (an orphan sweep failed mid-way), `cargo_cache_patch_kept` (see above), and the reinstall advisories of the vendored reverts (`vendor_bun_reinstall_required`, `vendor_vlt_reinstall_required`, `vendor_pypi_reinstall_required`; a revert's other warnings, such as `vendor_lock_entry_removed` or a drift keep's, are not repeated here). Human mode prints `GC: skipped (): .`, one `GC: .` line per warning, and `GC: failed to revert N vendored entries: …` (singular for one) for `failedVendoredEntries`. `keptVendoredEntries` lists drift-kept entries the revert deliberately preserved (`vendor_artifact_kept` — undo the drift and re-run `vendor --revert` to finish); the preview cannot see drift (backends return before the wiring replay on dry runs), so the preview's `revertedVendoredEntries` may over-promise what a wet run will actually reclaim. `scan` queries the patch API in `--batch-size` chunks. Authenticated runs POST `/v0/orgs/{slug}/patches/batch`; token-less runs POST `{proxy}/patch/batch` on the public proxy and degrade to per-package `GET /patch/by-package/:purl` requests in two cases: the deployed proxy predates the batch endpoint (legacy proxies answer the POST with their `400 "Unsupported endpoint"` catch-all), or the all-or-nothing batch validation rejects the chunk (e.g. a crawled PURL type the server doesn't recognize, such as `pkg:jsr/…` — the per-package path tolerates those individually, preserving the pre-batch scan semantics). Rate limits and over-capacity 503s surface instead of silently degrading. @@ -1052,9 +1052,11 @@ v5.0 replaces v4's per-purl reverts and whole-ledger reverse replay (`revert_rem | `vendoredFailed` | `[{purl, error}]` | Vendored reverts that errored — entry, artifact, and manifest record all survive for a retry; drives exit 1 | | `hosted` | `{reverted: [purl], failed: [{purl, error}], unsupported: [purl], editedFiles: N}` | The hosted leg (v5.0: the upstream restore). `reverted` lists the pins restored (would-be on dry-run); `failed` the refused pins with the version-control remedy in `error` (the pseudo-purl `files` for a write failure); `unsupported` is kept for shape and is always empty (every ecosystem has a restore); `editedFiles` counts distinct files rewritten | | `manifest` | `{removedEntries: [purl], preserved: bool}` | Entries removed from the manifest (would-be removals on dry-run); `preserved` mirrors `--preserve-state` | -| `gc` | `{skipped: true}` \| `{removedBlobs, removedDiffArchives, removedPackageArchives, bytesFreed}` | Skipped under `--preserve-state`, after a blob-gate abort, and under a corrupt vendor ledger | +| `gc` | `{skipped: true}` \| `{removedBlobs, removedDiffArchives, removedPackageArchives, bytesFreed}` | The shared GC shape (see "One GC shape"). Skipped under `--preserve-state`, after a blob-gate abort, and under a corrupt vendor ledger | | `paths` | `[string]` | The path-glob targets verbatim (empty when none) | +**Counters (v5.0, #1066)**: `rolledBack` and `failed` span every leg. `rolledBack` counts agent results that restored files, plus `vendoredReverted`, `vendoredPreserved` and `hosted.reverted`; `failed` counts failed agent results, plus `vendoredKept`, `vendoredFailed`, `hosted.failed` and `hosted.unsupported`. A package wired through two legs counts once per leg. `alreadyOriginal` stays agent-only. When something failed and nothing was rolled back or already original, the run failed as a whole: `status: "error"` with `error: {code: "rollback_failed", message}` (exit 1) instead of `partial_failure`. + **Exit rules**: not-installed entries never flip the exit (the documented apply/rollback asymmetry — even an all-not-installed run exits 0 `success`). Everything that leaves the system still patched DOES flip it to `partial_failure` exit 1: agent-leg failures, vendored drift-keeps and revert failures, hosted refusals, a corrupt vendor ledger, and a failed manifest write. GC failures never affect the exit. ## Self-update contract (`socket-patch --update`) @@ -1248,7 +1250,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified ```jsonc { - "command": "scan" | "apply" | "vex" | "vendor" | "rollback" | "get" | "list" | "remove" | "repair", + "command": "scan" | "apply" | "vex" | "vendor" | "rollback" | "get" | "list" | "remove" | "repair" | "update", "status": "success" | "partialFailure" | "error" | "noManifest" | "notFound", "dryRun": false, "events": [ , ... ], @@ -1261,20 +1263,28 @@ Every `--json` invocation emits a single JSON object that follows the **unified "failed": 0, "removed": 0, "verified": 0, - "bytesDownloaded": 0, - "bytesFreed": 0 + "rebuilt": 0, // omitted while zero (repair / vendor only) + "bytesFreed": 0 // = gc.bytesFreed; 0 when no GC ran + }, + "gc": { // only when the run swept .socket/ (repair, remove) + "removedBlobs": 0, + "removedDiffArchives": 0, + "removedPackageArchives": 0, + "bytesFreed": 0 }, "error": { "code": "...", "message": "..." } // only on status=error } ``` -`events` is the load-bearing payload. `summary` is pre-computed from `events` so consumers don't have to walk the array. `error` is set only on top-level failures (e.g. `manifest_not_found`); per-patch failures appear as `events[*]` with `action: "failed"`. +`events` is the load-bearing payload. `summary` is pre-computed from `events` so consumers don't have to walk the array; its action counters count patch-level events, so a GC carrier event (the artifact-level `removed`, or `verified` on a dry run, that `repair` and `remove` emit for a sweep) bumps none of them. The sweep itself is reported once, in `gc`. `error` is set only on top-level failures (e.g. `manifest_not_found`); per-patch failures appear as `events[*]` with `action: "failed"`. + +**One GC shape (v5.0).** Every command that sweeps orphan artifacts from `.socket/blobs`, `.socket/diffs` and `.socket/packages` reports the pass as the same `gc` object, `{removedBlobs, removedDiffArchives, removedPackageArchives, bytesFreed}`: the envelope's `gc` (`repair`, `remove`), rollback's `gc` and the `gc` of `scan --prune` / `--sync` (which adds its manifest and vendored keys beside them). On a dry run the counts are what the pass would remove, under the same keys (v5.0, MAJOR: scan's `--dry-run` preview no longer prints `prunableManifestEntries` / `orphanBlobs` / `orphanDiffArchives` / `orphanPackageArchives` / `revertableVendoredEntries` / `vendorOrphanDirs` / `bytesReclaimable`; it prints `prunedManifestEntries`, `removedBlobs`, `removedDiffArchives`, `removedPackageArchives`, `revertedVendoredEntries`, `removedVendorOrphanDirs` and `bytesFreed`, and leaves out only the keys a real pass alone can fill: `keptVendoredEntries`, `failedVendoredEntries`, `skipped`, `warnings`). `summary.bytesFreed` mirrors `gc.bytesFreed` on the envelope commands (0 when no GC ran: `repair --download-only`, `remove --preserve-state`, and every command without a GC pass). `gc` is absent when no sweep ran. v5.0 removes `summary.bytesDownloaded`, which no command ever emitted. ### `PatchEvent` shape ```jsonc { - "action": "discovered" | "downloaded" | "applied" | "updated" | "skipped" | "failed" | "removed" | "verified", + "action": "discovered" | "downloaded" | "applied" | "updated" | "skipped" | "failed" | "removed" | "verified" | "rebuilt", "purl": "pkg:npm/foo@1.2.3", // omitted on artifact-level events "uuid": "", // optional "oldUuid": "", // only when action=updated @@ -1285,7 +1295,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified "appliedVia": "blob" // only on action=applied; v5.0 drops "package" and "diff" } ], - "bytes": 1234, // optional (downloaded/removed) + "bytes": 1234, // only on the GC carrier event and --update's downloaded "reason": "Files match afterHash", // human-readable explanation (skipped) "errorCode": "already_patched", // stable snake_case routing tag "error": "", // only when action=failed @@ -1301,15 +1311,17 @@ Every `--json` invocation emits a single JSON object that follows the **unified | Action | Emitted by | Meaning | |--------------|---------------------------------------|---------| -| `discovered` | `scan`, `list` | Patch exists upstream / in the manifest — no work taken. | -| `downloaded` | `get`, `repair`, `scan --mode agent` | Patch bytes were fetched from the registry. `bytes` set. | -| `applied` | `apply`, `scan --sync` | Patch was written to disk. `files` enumerates what changed. | -| `updated` | `apply`, `scan --sync`, `get` | A different UUID replaced an older one for this PURL. `oldUuid` set. | -| `skipped` | every command | No-op — already patched, not in scope, filtered, etc. `errorCode` carries the reason. | -| `failed` | every command | A specific patch attempt failed. `errorCode` + `error` set. | -| `removed` | `repair`, `remove`, `rollback` | Data was removed from `.socket/` (or files rolled back). `bytes` optional. | -| `verified` | `apply --dry-run`, `scan --dry-run` | The patch *would* apply cleanly. `files` lists previewed changes. | -| `rebuilt` | `repair` | A missing/corrupt vendored artifact was restored from its exact server download (v5.0: never a lost ledger entry — see `vendor_ledger_missing`). `summary.rebuilt` counts these (the field is omitted while zero). | +| `discovered` | `list` | Patch recorded in the manifest, the vendor ledger or a hosted lockfile pin — no work taken. | +| `downloaded` | `repair`, `--update` | `repair`: artifacts were fetched (one aggregate event, `details.count`). `--update`: the release archive was fetched (`bytes` = archive size). | +| `applied` | `apply`, `vendor` | Patch was written to disk (`vendor`: vendored). `files` enumerates what changed. | +| `updated` | `--update` | The binary was replaced (`details.from` / `details.to`). | +| `skipped` | every envelope command | No-op — already patched, not in scope, filtered, etc. `errorCode` carries the reason. | +| `failed` | `apply`, `repair`, `vendor` | A specific attempt failed. `errorCode` + `error` set. | +| `removed` | `remove`, `repair`, `vendor` | A manifest entry or vendored state was removed, or (artifact-level, no `purl`) `.socket/` artifacts were swept — that GC carrier sets `bytes` and is not counted in `summary.removed`; the sweep's totals are the envelope's `gc`. | +| `verified` | `apply`, `remove`, `repair`, `vendor` (dry run); `vex`; `--update --dry-run` | The action *would* succeed cleanly (`files` lists previewed changes); `vex`: the patch verified and was attested. | +| `rebuilt` | `repair`, `vendor` | A missing/corrupt vendored artifact was restored from its exact server download (v5.0: never a lost ledger entry — see `vendor_ledger_missing`). `summary.rebuilt` counts these (the field is omitted while zero). | + +`scan`, `get` and `rollback` print their legacy shapes, not events (see [Migration status](#migration-status-v30)). ### Stable `errorCode` tags @@ -1532,11 +1544,12 @@ Every `--json` invocation emits a single JSON object that follows the **unified | Subcommand | Emits | |--------------|---| -| `apply` | `Applied` · `Updated` · `Skipped` (already_patched / package_not_installed / vendored) · `Failed` · `Verified` (dry-run) | -| `vendor` | `Applied` (= vendored; `command` routes) · `Skipped` (refusals, warnings, unsupported ecosystems) · `Failed` · `Removed` (reconcile + `--revert`) · `Verified` (dry-run) | +| `apply` | `Applied` · `Skipped` (already_patched / package_not_installed / vendored) · `Failed` · `Verified` (dry-run) | +| `vendor` | `Applied` (= vendored; `command` routes) · `Rebuilt` (a reused artifact restored) · `Skipped` (refusals, warnings, unsupported ecosystems) · `Failed` · `Removed` (reconcile + `--revert`) · `Verified` (dry-run) | | `list` | `Discovered` (with `details.vulnerabilities`, `details.tier`, `details.license`, `details.description`, `details.exportedAt`; hosted pins (v5.0: one per `(purl, uuid)` the lockfiles wire) additionally carry `details.mode: "hosted"` and `details.lockfiles: []` (no `details.ledger` — hosted mode keeps no ledger; the human listing labels them `Mode: hosted (wired in )`), both additive and absent on manifest entries; v5.0: vendor-ledger records carry `details.mode: "vendored"` + `details.ledger: ".socket/vendor/state.json"` the same way, and the human listing labels them `Mode: vendored (recorded in .socket/vendor/state.json)`; a `state.json` that cannot be read or parsed degrades to nothing-to-consult with the stderr line `Warning: unreadable vendor ledger (); its vendored patches are not listed` — muted by `--silent`, exit unchanged) | -| `repair` | `Downloaded` (or `Verified` on dry-run; `details: {count, mode: "file"}` — `mode` is always `"file"` since v5.0 removed the diff download path) · `Rebuilt` (vendored artifacts; `Verified` previews on dry-run) · `Skipped` (vendor_uuid_mismatch) · `Removed` (or `Verified`) · `Failed` events | -| `remove` | `Removed` (per purl; `Verified` on dry-run) · artifact-level `Removed`/`Verified` event (with `details.blobsRemoved`, `details.rolledBack`) | +| `repair` | `Downloaded` (or `Verified` on dry-run; `details: {count, mode: "file"}` — `mode` is always `"file"` since v5.0 removed the diff download path) · `Rebuilt` (vendored artifacts; `Verified` previews on dry-run) · `Skipped` (vendor_uuid_mismatch, cleanup_failed) · artifact-level `Removed` (or `Verified`) GC carrier (`details.count`, `details.checked`, `bytes`) · `Failed` events · top-level `gc` (absent under `--download-only`) | +| `vex` | `Verified` (one per attested subcomponent) · `Skipped` (omissions) — only under `--json --output` | +| `remove` | `Removed` (per purl; `Verified` on dry-run) · artifact-level `Removed`/`Verified` event (with `details.blobsRemoved`, `details.archivesRemoved`, `details.rolledBack`; `bytes` when the sweep removed something) · top-level `gc` (absent under `--preserve-state`) | | `--update` | `Downloaded` → `Updated` (success) · `Skipped` (already_latest) · `Verified` (dry-run check, reason update_check) — see the Self-update contract section for details fields and top-level error codes | ### Migration status (v3.0) @@ -1793,13 +1806,14 @@ socket-patch apply --json | jq ' ' ``` -GC summary (after `repair --json`): +GC summary (after `repair --json`; `remove --json` prints the same `gc`): ```bash socket-patch repair --json | jq '{ - removed: .summary.removed, - bytesFreed: .summary.bytesFreed, - failed: .summary.failed + removedBlobs: .gc.removedBlobs, + removedArchives: (.gc.removedDiffArchives + .gc.removedPackageArchives), + bytesFreed: .summary.bytesFreed, + failed: .summary.failed }' ``` diff --git a/crates/socket-patch-cli/src/commands/remove.rs b/crates/socket-patch-cli/src/commands/remove.rs index c8729a408..c2833ce27 100644 --- a/crates/socket-patch-cli/src/commands/remove.rs +++ b/crates/socket-patch-cli/src/commands/remove.rs @@ -1,7 +1,10 @@ use clap::Args; +use socket_patch_core::api::blob_fetcher::{DIFF_ARCHIVE, PACKAGE_ARCHIVE}; use socket_patch_core::api::client::get_api_client_with_overrides; use socket_patch_core::ledgers::hosted_pins_matching; -use socket_patch_core::manifest::cleanup_blobs::{format_bytes, ArtifactReferences}; +use socket_patch_core::manifest::cleanup_blobs::{ + format_bytes, format_cleanup_result_for, ArtifactReferences, +}; use socket_patch_core::manifest::operations::{read_manifest, write_manifest}; use socket_patch_core::manifest::schema::PatchManifest; use socket_patch_core::patch::redirect::upstream::HostedPin; @@ -21,7 +24,9 @@ use crate::commands::lock_cli::acquire_or_emit; use crate::commands::vendored_backend::{ KeepCause, RevertedEntry, VendorRevertStep, VendoredBackend, }; -use crate::json_envelope::{Command, Envelope, EnvelopeError, PatchAction, PatchEvent, Status}; +use crate::json_envelope::{ + Command, Envelope, EnvelopeError, GcReport, PatchAction, PatchEvent, Status, +}; use crate::ui::short_uuid; use crate::ui::{plural, sweep_failure}; @@ -962,8 +967,8 @@ pub async fn run(args: RemoveArgs) -> i32 { &updated_manifest, retained_not_installed.iter().copied(), ); - let mut blobs_removed = 0; - let mut archives_removed = 0; + // `None` under `--preserve-state` (no sweep ran): no `gc` in the JSON. + let mut gc: Option = None; if !args.preserve_state { let sweep = references.sweep(&socket_dir, args.common.dry_run).await; // repair's posture: a failed pass (or a pass that could not unlink @@ -974,9 +979,11 @@ pub async fn run(args: RemoveArgs) -> i32 { eprintln!("Warning: {detail}"); } } - if let Ok(r) = sweep.blobs { - blobs_removed = r.blobs_removed; + // The GC lines are one block, opened by a blank line. + let mut gc_printed = false; + if let Ok(r) = &sweep.blobs { if loud && r.blobs_removed > 0 { + gc_printed = true; println!( "\n{}", format_blob_sweep( @@ -990,16 +997,35 @@ pub async fn run(args: RemoveArgs) -> i32 { } // Obsolete diff and package archives are swept whole (parity with // repair and scan --prune). - for (dir, result) in [("diffs", sweep.diffs), ("packages", sweep.packages)] { - if let Some(detail) = sweep_failure(dir, &result) { + for (dir, noun, result) in [ + ("diffs", DIFF_ARCHIVE, &sweep.diffs), + ("packages", PACKAGE_ARCHIVE, &sweep.packages), + ] { + if let Some(detail) = sweep_failure(dir, result) { if loud { eprintln!("Warning: {detail}"); } } + // The archives the sweep took are named like repair names them, + // so the human run accounts for everything `gc` reports. if let Ok(r) = result { - archives_removed += r.blobs_removed; + if loud && r.blobs_removed > 0 { + if !gc_printed { + println!(); + } + gc_printed = true; + println!( + "{}", + format_cleanup_result_for(r, args.common.dry_run, noun) + ); + } } } + gc = Some(GcReport::from_passes( + sweep.blobs.as_ref().ok(), + sweep.diffs.as_ref().ok(), + sweep.packages.as_ref().ok(), + )); } // The dry-run footer closes the whole preview, the blob-cleanup @@ -1097,14 +1123,25 @@ pub async fn run(args: RemoveArgs) -> i32 { // single-patch removal that happened to sweep an orphan blob. // Consumers read the blob/rollback totals from `details`, never // from `summary.removed`. - if blobs_removed > 0 || rollback_count > 0 || archives_removed > 0 { - env.events.push( + // The sweep's per-kind totals and byte count are also the + // envelope's `gc` (`summary.bytesFreed`), the shape every GC-running + // command prints. + let report = gc.unwrap_or_default(); + if report.total_removed() > 0 || rollback_count > 0 { + let mut carrier = PatchEvent::artifact(removal_action).with_details(serde_json::json!({ - "blobsRemoved": blobs_removed, + "blobsRemoved": report.removed_blobs, "rolledBack": rollback_count, - "archivesRemoved": archives_removed, - })), - ); + "archivesRemoved": report.removed_diff_archives + + report.removed_package_archives, + })); + if report.total_removed() > 0 { + carrier = carrier.with_bytes(report.bytes_freed); + } + env.events.push(carrier); + } + if let Some(gc) = gc { + env.set_gc(gc); } // Any drift-kept entry means part of the requested removal did // NOT happen: the run is a partialFailure (exit 1) even when diff --git a/crates/socket-patch-cli/src/commands/repair.rs b/crates/socket-patch-cli/src/commands/repair.rs index 9c9abea9e..cc6bad26a 100644 --- a/crates/socket-patch-cli/src/commands/repair.rs +++ b/crates/socket-patch-cli/src/commands/repair.rs @@ -16,7 +16,7 @@ use std::time::Duration; use crate::args::{apply_env_toggles, parse_bool_flag, GlobalArgs}; use crate::commands::lock_cli::{acquire_or_emit, error_envelope}; -use crate::json_envelope::{Command, Envelope, PatchAction, PatchEvent, Status}; +use crate::json_envelope::{Command, Envelope, GcReport, PatchAction, PatchEvent, Status}; use crate::ui::sweep_failure; #[derive(Args)] @@ -477,9 +477,8 @@ async fn repair_inner( let mut downloaded_count = 0usize; let mut download_failed_count = 0usize; - let mut blobs_cleaned = 0usize; let mut blobs_checked = 0usize; - let mut bytes_freed = 0u64; + let mut gc: Option = None; // The envelope is built up-front: the vendored-artifact phase records // its events inline; the download/cleanup aggregates are appended at @@ -597,7 +596,8 @@ async fn repair_inner( ("package", PACKAGE_ARCHIVE, sweep.packages), ]; let mut results: Vec<(ArtifactNoun, CleanupResult)> = Vec::new(); - for (label, noun, result) in passes { + let mut ok: [Option; 3] = [None, None, None]; + for (slot, (label, noun, result)) in ok.iter_mut().zip(passes) { // A failed cleanup — the pass aborted, or it could not unlink // every orphan — is error output: `--silent` (suppress // NON-error output) must not mute it, and the JSON envelope @@ -618,15 +618,13 @@ async fn repair_inner( ); } if let Ok(cleanup_result) = result { + blobs_checked += cleanup_result.blobs_checked; + *slot = Some(results.len()); results.push((noun, cleanup_result)); } } - - for (_, r) in &results { - blobs_checked += r.blobs_checked; - blobs_cleaned += r.blobs_removed; - bytes_freed += r.bytes_freed; - } + let pass = |i: usize| ok[i].map(|at| &results[at].1); + gc = Some(GcReport::from_passes(pass(0), pass(1), pass(2))); if !quiet { if stdout_started { println!(); @@ -690,25 +688,37 @@ async fn repair_inner( )); env.mark_partial_failure(); } - if blobs_cleaned > 0 { + // `None` when no sweep ran (`--download-only`, no manifest): no `gc`. + let swept = gc.is_some(); + let gc = gc.unwrap_or_default(); + if gc.total_removed() > 0 { let cleanup_action = if args.common.dry_run { PatchAction::Verified } else { PatchAction::Removed }; - env.record( - PatchEvent::artifact(cleanup_action).with_details(serde_json::json!({ - "count": blobs_cleaned, - "checked": blobs_checked, - })), + // Pushed directly rather than via `env.record`, as remove's GC + // carrier is: `summary.removed` / `summary.verified` count patch + // entries, and the sweep's totals live in `gc` (with the byte + // count mirrored into `summary.bytesFreed`). + env.events.push( + PatchEvent::artifact(cleanup_action) + .with_bytes(gc.bytes_freed) + .with_details(serde_json::json!({ + "count": gc.total_removed(), + "checked": blobs_checked, + })), ); } + if swept { + env.set_gc(gc); + } Ok(( env, RepairCounts { downloaded: downloaded_count, - cleaned: blobs_cleaned, - bytes_freed, + cleaned: gc.total_removed(), + bytes_freed: gc.bytes_freed, }, )) } @@ -893,8 +903,26 @@ mod tests { // The referenced blob survives; the orphan is gone. assert!(socket.join("blobs").join(REFERENCED_HASH).exists()); assert!(!socket.join("blobs").join(&orphan_hash).exists()); - // A Removed event is recorded for the swept orphan. - assert_eq!(env.summary.removed, 1); + // The sweep is the envelope's `gc` (bytes mirrored into the + // summary) plus one carrier event; `summary.removed` counts patch + // entries, so the carrier does not bump it. + assert_eq!( + env.gc, + Some(GcReport { + removed_blobs: 1, + removed_diff_archives: 0, + removed_package_archives: 0, + bytes_freed: orphan_bytes.len() as u64, + }) + ); + assert_eq!(env.summary.bytes_freed, orphan_bytes.len() as u64); + assert_eq!(env.summary.removed, 0); + let carrier = env + .events + .iter() + .find(|e| e.action == PatchAction::Removed) + .expect("a Removed carrier event"); + assert_eq!(carrier.bytes, Some(orphan_bytes.len() as u64)); } /// `--download-only` skips the cleanup pass, so an orphan blob survives @@ -913,12 +941,13 @@ mod tests { args.common.offline = false; args.download_only = true; - let (_env, counts) = + let (env, counts) = repair_inner(&args, &socket.join("manifest.json"), &mut None, Vec::new()) .await .expect("repair_inner"); assert_eq!(counts.cleaned, 0, "download-only must skip cleanup"); + assert_eq!(env.gc, None, "no sweep ran, so no `gc`"); assert_eq!(counts.bytes_freed, 0); assert!( socket.join("blobs").join(&orphan_hash).exists(), @@ -974,11 +1003,20 @@ mod tests { (orphan_diff.len() + orphan_pkg.len() + legacy_pkg.len() + stale_diff.len()) as u64, "bytes_freed must aggregate diff + package reclaim" ); - // Cleanup is reported as a SINGLE batched `removed` artifact event whose - // `details.count` carries the tally — so the event-count summary is 1 - // (`Summary::bump` increments once per event), and the 4-artifact count - // is asserted via `counts.cleaned` above and the event details here. - assert_eq!(env.summary.removed, 1, "one batched removal event"); + // Cleanup is reported once, per kind, in `gc`, plus a SINGLE batched + // `removed` carrier event whose `details.count` carries the tally. + // The carrier is not a removed patch entry, so `summary.removed` + // stays 0. + assert_eq!( + env.gc, + Some(GcReport { + removed_blobs: 0, + removed_diff_archives: 2, + removed_package_archives: 2, + bytes_freed: counts.bytes_freed, + }) + ); + assert_eq!(env.summary.removed, 0, "the carrier bumps no counter"); let removed = env .events .iter() diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs index 77859bd16..2f00a1f0b 100644 --- a/crates/socket-patch-cli/src/commands/rollback.rs +++ b/crates/socket-patch-cli/src/commands/rollback.rs @@ -1578,26 +1578,22 @@ pub async fn run(args: RollbackArgs) -> i32 { .map(String::as_str), ); let sweep = references.sweep(&socket_dir, args.common.dry_run).await; - let mut removed_counts = [0usize; 3]; - for (slot, (label, result)) in removed_counts.iter_mut().zip([ - ("blob", sweep.blobs), - ("diffs", sweep.diffs), - ("packages", sweep.packages), - ]) { - if let Some(detail) = crate::ui::sweep_failure(label, &result) { + for (label, result) in [ + ("blob", &sweep.blobs), + ("diffs", &sweep.diffs), + ("packages", &sweep.packages), + ] { + if let Some(detail) = crate::ui::sweep_failure(label, result) { run_warnings.push(("cleanup_failed".into(), detail)); } - if let Ok(r) = result { - *slot = r.blobs_removed; - gc_bytes_freed += r.bytes_freed; - } } - gc_json = serde_json::json!({ - "removedBlobs": removed_counts[0], - "removedDiffArchives": removed_counts[1], - "removedPackageArchives": removed_counts[2], - "bytesFreed": gc_bytes_freed, - }); + let report = crate::json_envelope::GcReport::from_passes( + sweep.blobs.as_ref().ok(), + sweep.diffs.as_ref().ok(), + sweep.packages.as_ref().ok(), + ); + gc_bytes_freed = report.bytes_freed; + gc_json = report.to_value(); } // ── run-level warnings ─────────────────────────────────────── @@ -1715,6 +1711,29 @@ pub async fn run(args: RollbackArgs) -> i32 { .filter(|r| r.success && all_files_already_original(r)) .count(); let failed_count = results.iter().filter(|r| !r.success).count(); + // The top-level counters span every leg (#1066): a vendored or + // hosted unwind is a rollback, and a drift-keep, failure or + // unsupported hosted target is a failure, exactly as each one + // drives the status. A package wired through two legs counts + // once per leg; the per-leg arrays below say which. + let rolled_back_total = rolled_back_count + + vendored_leg.reverted.len() + + vendored_leg.preserved.len() + + hosted_leg.reverted.len(); + let failed_total = failed_count + + vendored_leg.kept.len() + + vendored_leg.failed.len() + + hosted_leg.failed.len() + + hosted_leg.unsupported.len(); + // Something failed and nothing reached the unpatched end state + // (rolled back, already original, or not installed): the run + // failed as a whole, so the status is an error (with `error`), + // not a partial failure. + let total_failure = !success + && failed_total > 0 + && rolled_back_total == 0 + && already_original_count == 0 + && not_installed.is_empty(); if let Some(e) = &manifest_write_failed { if !args.common.json { @@ -1731,11 +1750,12 @@ pub async fn run(args: RollbackArgs) -> i32 { // always present so consumers never null-check. println!( "{}", - serde_json::to_string_pretty(&serde_json::json!({ + serde_json::to_string_pretty(&{ + let mut out = serde_json::json!({ "status": if success { "success" } else { "partial_failure" }, - "rolledBack": rolled_back_count, + "rolledBack": rolled_back_total, "alreadyOriginal": already_original_count, - "failed": failed_count, + "failed": failed_total, "dryRun": args.common.dry_run, "warnings": run_warnings .iter() @@ -1791,7 +1811,21 @@ pub async fn run(args: RollbackArgs) -> i32 { .map(result_to_json) .chain(not_installed.iter().map(|p| skipped_not_installed_json(p))) .collect::>(), - })) + }); + if total_failure { + crate::json_envelope::set_error( + &mut out, + crate::json_envelope::EnvelopeError::new( + "rollback_failed", + format!( + "nothing was rolled back: {}", + plural(failed_total, "patch failed", "patches failed") + ), + ), + ); + } + out + }) .expect("serializing an in-memory JSON value cannot fail") ); } else if !args.common.silent && !results.is_empty() { @@ -1945,7 +1979,7 @@ pub async fn run(args: RollbackArgs) -> i32 { } if success { - track_patch_rolled_back(rolled_back_count, &telemetry).await; + track_patch_rolled_back(rolled_back_total, &telemetry).await; } else { track_patch_rollback_failed("One or more rollbacks failed", &telemetry).await; } diff --git a/crates/socket-patch-cli/src/commands/scan/gc.rs b/crates/socket-patch-cli/src/commands/scan/gc.rs index 1d78d1846..3826320ae 100644 --- a/crates/socket-patch-cli/src/commands/scan/gc.rs +++ b/crates/socket-patch-cli/src/commands/scan/gc.rs @@ -14,6 +14,7 @@ use std::time::Duration; use crate::args::GlobalArgs; use crate::commands::lock_cli::lock_failure; use crate::commands::vendor::{run_vendor_gc, VendorGcSummary}; +use crate::json_envelope::GcReport; use crate::ui::sweep_failure; /// Aggregated outcome of a GC pass (or preview). Serialized into the @@ -93,21 +94,24 @@ impl GcSummary { self.vendor_orphan_dirs = v.orphan_dirs; } - /// Serialize for a *mutating* GC pass (post-apply). `skipped` and - /// `warnings` are additive: present only when the lock could not be - /// taken / a post-revert rewrite failed. - fn to_apply_json(&self) -> serde_json::Value { - let mut json = serde_json::json!({ - "prunedManifestEntries": self.pruned, - "removedBlobs": self.blobs.blobs_removed, - "removedDiffArchives": self.diffs.blobs_removed, - "removedPackageArchives": self.packages.blobs_removed, - "revertedVendoredEntries": self.vendored_reverted, - "keptVendoredEntries": self.vendored_kept, - "failedVendoredEntries": self.vendored_failed, - "removedVendorOrphanDirs": self.vendor_orphan_dirs, - "bytesFreed": self.total_bytes(), - }); + /// The `gc` sub-object. One shape for both passes: the artifact half is + /// the `gc` object every GC-running command prints (`GcReport`), the + /// manifest and vendored halves are scan's. On a `--dry-run` preview the + /// counts are what the pass would remove, and the keys only a real pass + /// can fill (`keptVendoredEntries`, `failedVendoredEntries`, `skipped`, + /// `warnings`) are left out rather than reported as an empty check. + pub(super) fn to_json(&self, preview: bool) -> serde_json::Value { + let mut json = + GcReport::from_passes(Some(&self.blobs), Some(&self.diffs), Some(&self.packages)) + .to_value(); + json["prunedManifestEntries"] = serde_json::json!(self.pruned); + json["revertedVendoredEntries"] = serde_json::json!(self.vendored_reverted); + json["removedVendorOrphanDirs"] = serde_json::json!(self.vendor_orphan_dirs); + if preview { + return json; + } + json["keptVendoredEntries"] = serde_json::json!(self.vendored_kept); + json["failedVendoredEntries"] = serde_json::json!(self.vendored_failed); if let Some((code, message)) = &self.skipped { json["skipped"] = serde_json::json!({ "code": code, "message": message }); } @@ -120,29 +124,6 @@ impl GcSummary { } json } - - /// The `gc` sub-object: [`Self::to_preview_json`] for a `--dry-run` - /// pass, [`Self::to_apply_json`] otherwise. - pub(super) fn to_json(&self, preview: bool) -> serde_json::Value { - if preview { - self.to_preview_json() - } else { - self.to_apply_json() - } - } - - /// Serialize for a *non-mutating* GC pass (read-only preview). - fn to_preview_json(&self) -> serde_json::Value { - serde_json::json!({ - "prunableManifestEntries": self.pruned, - "orphanBlobs": self.blobs.blobs_removed, - "orphanDiffArchives": self.diffs.blobs_removed, - "orphanPackageArchives": self.packages.blobs_removed, - "revertableVendoredEntries": self.vendored_reverted, - "vendorOrphanDirs": self.vendor_orphan_dirs, - "bytesReclaimable": self.total_bytes(), - }) - } } /// The orphan blob/diff/package sweep against the (post-prune) manifest. @@ -338,9 +319,8 @@ pub(super) async fn run_vendor_only_gc( GcSummary::vendor_only(run_vendor_gc(common, manifest_path, false).await) } -/// Dry-run preview of the apply-mode GC pass. Same shape as -/// [`run_apply_gc`] but emits `prunable*`/`orphan*` field names and -/// performs no mutation. +/// Dry-run preview of the apply-mode GC pass. Same summary as +/// [`run_apply_gc`] (what the pass would remove), with no mutation. async fn preview_apply_gc( common: &GlobalArgs, manifest_path: &Path, @@ -390,8 +370,7 @@ async fn preview_apply_gc( } /// The `gc` sub-object for the JSON paths: a read-only preview under -/// `--dry-run`, the mutating pass otherwise, serialized with the matching -/// (`prunable*`/`orphan*` vs `pruned*`/`removed*`) field names. +/// `--dry-run`, the mutating pass otherwise, both in the one `gc` shape. pub(super) async fn gc_json( common: &GlobalArgs, manifest_path: &Path, @@ -403,11 +382,11 @@ pub(super) async fn gc_json( if dry_run { preview_apply_gc(common, manifest_path, socket_dir, scanned_purls, vendored) .await - .to_preview_json() + .to_json(true) } else { run_apply_gc(common, manifest_path, socket_dir, scanned_purls, vendored) .await - .to_apply_json() + .to_json(false) } } @@ -880,7 +859,7 @@ mod tests { let blobs_dir = socket_dir.join("blobs"); std::fs::create_dir_all(&blobs_dir).unwrap(); let blob_path = blobs_dir.join(after_hash); - // Non-trivial size so `bytesReclaimable`/`bytesFreed` is observably > 0. + // Non-trivial size so `bytesFreed` is observably > 0. std::fs::write(&blob_path, vec![0u8; 64]).unwrap(); let manifest_path = socket_dir.join("manifest.json"); @@ -942,7 +921,7 @@ mod tests { ); assert!( preview.total_bytes() > 0, - "bytesReclaimable must be > 0 when an orphan blob would be freed" + "bytesFreed must be > 0 when an orphan blob would be freed" ); // Preview is non-mutating: blob and manifest untouched. assert!( @@ -1007,7 +986,7 @@ mod tests { )), "a lock-contended pass must say why it pruned nothing" ); - let json = gc.to_apply_json(); + let json = gc.to_json(false); assert_eq!(json["skipped"]["code"], "lock_held", "{json}"); assert_eq!( json["prunedManifestEntries"], @@ -1088,7 +1067,7 @@ mod tests { assert!(blob_path.exists(), "nothing may be swept on a lock fault"); let m = read_manifest(&manifest_path).await.unwrap().unwrap(); assert!(m.patches.contains_key("pkg:npm/gone@1.0.0")); - assert_eq!(gc.to_apply_json()["skipped"]["code"], "lock_io"); + assert_eq!(gc.to_json(false)["skipped"]["code"], "lock_io"); } #[tokio::test] @@ -1296,10 +1275,10 @@ mod tests { "preview must not create a manifest file" ); // The serialized degenerate preview is the normal all-zero shape. - let json = gc.to_preview_json(); - assert_eq!(json["prunableManifestEntries"], serde_json::json!([])); - assert_eq!(json["orphanBlobs"], serde_json::json!(0)); - assert_eq!(json["bytesReclaimable"], serde_json::json!(0)); + let json = gc.to_json(true); + assert_eq!(json["prunedManifestEntries"], serde_json::json!([])); + assert_eq!(json["removedBlobs"], serde_json::json!(0)); + assert_eq!(json["bytesFreed"], serde_json::json!(0)); // Corrupt manifest, fresh tempdir. let tmp = tempfile::tempdir().unwrap(); @@ -1405,7 +1384,7 @@ mod tests { ); assert!( gc.total_bytes() > 0, - "bytesReclaimable must include the unused entry's blob" + "bytesFreed must include the unused entry's blob" ); assert_eq!(gc.vendor_orphan_dirs, 0, "no orphan uuid dirs on disk"); // Preview is non-mutating: blob, manifest entry, and ledger intact. @@ -1525,7 +1504,7 @@ mod tests { preview listed as revertable was deliberately not reclaimed" ); assert_eq!( - gc.to_apply_json()["keptVendoredEntries"], + gc.to_json(false)["keptVendoredEntries"], serde_json::json!([PURL]), "scan --prune --json must carry the keep" ); @@ -1533,9 +1512,9 @@ mod tests { // `vendor_artifact_kept`) are already said by the keep above; they // must not also land in `gc.warnings[]`. assert!( - gc.to_apply_json().get("warnings").is_none(), + gc.to_json(false).get("warnings").is_none(), "a drift keep adds no gc warning: {}", - gc.to_apply_json() + gc.to_json(false) ); // Nothing reclaimed: manifest record, blob, ledger entry, and // artifacts all survive (the drift-keep contract). @@ -1597,7 +1576,7 @@ mod tests { Some(("lock_held", LOCK_MARKER.to_string())), "absorbing the vendored half leaves this pass's own skip reason alone" ); - let apply = gc.to_apply_json(); + let apply = gc.to_json(false); assert_eq!( apply["keptVendoredEntries"], serde_json::json!(["pkg:npm/a@1.0.0", "pkg:npm/b@1.0.0"]) @@ -1617,7 +1596,7 @@ mod tests { }]), "{apply}" ); - let preview = gc.to_preview_json(); + let preview = gc.to_json(true); for key in [ "keptVendoredEntries", "failedVendoredEntries", @@ -1635,7 +1614,7 @@ mod tests { // absent, not null). let clean = GcSummary::vendor_only(VendorGcSummary::default()); assert!(clean.skipped.is_none()); - let clean_json = clean.to_apply_json(); + let clean_json = clean.to_json(false); assert!(clean_json.get("skipped").is_none(), "{clean_json}"); assert!(clean_json.get("warnings").is_none(), "{clean_json}"); // A failed rewrite in the vendored half is a warning, never a skip @@ -1647,7 +1626,7 @@ mod tests { }); assert!(io.skipped.is_none(), "the vendored half never sets skipped"); assert_eq!( - io.to_apply_json()["warnings"][0]["code"], + io.to_json(false)["warnings"][0]["code"], "manifest_write_failed" ); assert!(io.vendored_failed.is_empty()); diff --git a/crates/socket-patch-cli/src/commands/update.rs b/crates/socket-patch-cli/src/commands/update.rs index 52ac94797..9fd8fef5e 100644 --- a/crates/socket-patch-cli/src/commands/update.rs +++ b/crates/socket-patch-cli/src/commands/update.rs @@ -378,11 +378,13 @@ pub async fn run(args: UpdateArgs) -> i32 { if args.common.json { let mut env = Envelope::new(Command::Update); env.record( - PatchEvent::artifact(PatchAction::Downloaded).with_details(serde_json::json!({ - "asset": outcome.asset, - "bytes": outcome.archive_bytes, - "sha256": outcome.archive_sha256, - })), + PatchEvent::artifact(PatchAction::Downloaded) + .with_bytes(outcome.archive_bytes) + .with_details(serde_json::json!({ + "asset": outcome.asset, + "bytes": outcome.archive_bytes, + "sha256": outcome.archive_sha256, + })), ); env.record( PatchEvent::artifact(PatchAction::Updated).with_details(serde_json::json!({ diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index bfeb93664..5f18386c7 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -5797,7 +5797,7 @@ mod gc_tests { /// listed exactly once. The wet pass removes it from the ledger in (a) /// before (b) runs; the dry-run preview leaves the ledger untouched, so /// without excluding (a)-handled purls from (b) the same purl lands in - /// both lists and `scan --prune`'s `revertableVendoredEntries` preview + /// both lists and `scan --prune --dry-run`'s `revertedVendoredEntries` preview /// duplicates it (breaking preview/wet parity). #[tokio::test] async fn vendor_gc_dry_run_lists_dropped_and_unused_entry_once() { diff --git a/crates/socket-patch-cli/src/json_envelope.rs b/crates/socket-patch-cli/src/json_envelope.rs index 446b68c5e..e9d3f5b67 100644 --- a/crates/socket-patch-cli/src/json_envelope.rs +++ b/crates/socket-patch-cli/src/json_envelope.rs @@ -27,6 +27,7 @@ //! `jq` recipes. use serde::Serialize; +use socket_patch_core::manifest::cleanup_blobs::CleanupResult; pub use socket_patch_core::patch::sidecars::{SidecarFile, SidecarFileAction, SidecarRecord}; @@ -90,6 +91,61 @@ pub struct Envelope { /// (and flips the exit code), not here. #[serde(skip_serializing_if = "Option::is_none")] pub vex: Option, + /// The artifact GC pass's outcome — the same `gc` object (same keys) + /// `rollback` and `scan --prune` print. Set by [`Envelope::set_gc`], + /// which also mirrors `bytesFreed` into `summary.bytesFreed`. Omitted + /// for runs that swept nothing (`repair --download-only`, `remove + /// --preserve-state`, every command without a GC pass). + #[serde(skip_serializing_if = "Option::is_none")] + pub gc: Option, +} + +/// One artifact GC pass — the orphan sweeps of `.socket/blobs`, +/// `.socket/diffs` and `.socket/packages` — serialized identically by +/// every command that runs one: the envelope's `gc` (`repair`, `remove`), +/// rollback's legacy `gc` and the `gc` of `scan --prune` / `--sync`. On a +/// dry run the counts are what the pass would remove. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct GcReport { + pub removed_blobs: usize, + pub removed_diff_archives: usize, + pub removed_package_archives: usize, + pub bytes_freed: u64, +} + +impl GcReport { + /// Fold the three passes' results; a pass that failed outright + /// (`None`) counts as empty — its `cleanup_failed` warning is the + /// caller's to report. + pub fn from_passes( + blobs: Option<&CleanupResult>, + diffs: Option<&CleanupResult>, + packages: Option<&CleanupResult>, + ) -> Self { + let count = |r: Option<&CleanupResult>| r.map_or(0, |r| r.blobs_removed); + Self { + removed_blobs: count(blobs), + removed_diff_archives: count(diffs), + removed_package_archives: count(packages), + bytes_freed: [blobs, diffs, packages] + .into_iter() + .flatten() + .map(|r| r.bytes_freed) + .sum(), + } + } + + /// Blobs plus diff and package archives removed. + pub fn total_removed(&self) -> usize { + self.removed_blobs + self.removed_diff_archives + self.removed_package_archives + } + + /// The `gc` object as a JSON value, for the legacy shapes that extend + /// it with command-specific keys (`scan --prune`). + pub fn to_value(&self) -> serde_json::Value { + serde_json::to_value(self).expect("GcReport serializes") + } } /// Summary of an OpenVEX document emitted as a side-effect of an @@ -131,9 +187,17 @@ impl Envelope { sidecars: Vec::new(), warnings: Vec::new(), vex: None, + gc: None, } } + /// Attach the run's artifact GC outcome (`gc`) and mirror its byte + /// count into `summary.bytesFreed`. + pub fn set_gc(&mut self, gc: GcReport) { + self.summary.bytes_freed = gc.bytes_freed; + self.gc = Some(gc); + } + /// Append an event and bump the matching summary counter. Centralizes /// the "events list must agree with summary counts" invariant so per- /// command code can't drift. @@ -255,6 +319,11 @@ pub struct PatchEvent { /// Empty for actions that don't operate on files (e.g. `Downloaded`). #[serde(skip_serializing_if = "Vec::is_empty")] pub files: Vec, + /// Byte count of the artifact-level GC event (`removed`, or `verified` + /// on a dry run: bytes freed) and of `--update`'s `downloaded` event + /// (archive size). Omitted everywhere else. + #[serde(skip_serializing_if = "Option::is_none")] + pub bytes: Option, /// Human-readable explanation for `Skipped` or `Failed` events. /// Machine consumers should prefer `error_code` for routing decisions. #[serde(skip_serializing_if = "Option::is_none")] @@ -302,6 +371,7 @@ impl PatchEvent { uuid: None, old_uuid: None, files: Vec::new(), + bytes: None, reason: None, error_code: None, error: None, @@ -327,6 +397,11 @@ impl PatchEvent { self } + pub fn with_bytes(mut self, bytes: u64) -> Self { + self.bytes = Some(bytes); + self + } + pub fn with_reason(mut self, code: impl Into, message: impl Into) -> Self { self.error_code = Some(code.into()); self.reason = Some(message.into()); @@ -472,6 +547,10 @@ pub struct Summary { /// every other command's summary shape is unchanged. #[serde(skip_serializing_if = "u32_is_zero")] pub rebuilt: u32, + /// Bytes the run's artifact GC freed (would free, on a dry run) — the + /// envelope's `gc.bytesFreed`, 0 when no GC ran. Not derived from + /// `events`: GC is reported once, in `gc`. + pub bytes_freed: u64, } fn u32_is_zero(n: &u32) -> bool { @@ -806,6 +885,7 @@ mod tests { (PatchAction::Failed, "failed"), (PatchAction::Removed, "removed"), (PatchAction::Verified, "verified"), + (PatchAction::Rebuilt, "rebuilt"), ] { let serialized = serde_json::to_string(&action).unwrap(); assert_eq!(serialized, format!("\"{tag}\"")); @@ -1297,4 +1377,146 @@ mod tests { ); } } + + /// The ```jsonc block under `heading` in CLI_CONTRACT.md. + /// CLI_CONTRACT.md with LF line endings: a Windows checkout may carry + /// CRLF, which the `"```jsonc\n"` fence match below would miss. + fn contract_doc() -> &'static str { + static DOC: std::sync::OnceLock = std::sync::OnceLock::new(); + DOC.get_or_init(|| include_str!("../CLI_CONTRACT.md").replace("\r\n", "\n")) + } + + fn contract_block(heading: &str) -> &'static str { + let doc = contract_doc(); + let at = doc + .find(heading) + .unwrap_or_else(|| panic!("{heading} missing")); + let body = &doc[at..]; + let start = body.find("```jsonc\n").expect("jsonc block") + "```jsonc\n".len(); + let end = start + body[start..].find("```").expect("block end"); + &body[start..end] + } + + /// The `"key":` names at exactly `indent` spaces in `block`. + fn keys_at(block: &str, indent: usize) -> std::collections::BTreeSet { + block + .lines() + .filter(|l| l.len() > indent && l[..indent].trim().is_empty()) + .filter_map(|l| l[indent..].strip_prefix('"')) + .filter_map(|l| l.split_once('"').map(|(k, _)| k.to_string())) + .collect() + } + + fn object_keys(value: serde_json::Value) -> std::collections::BTreeSet { + value.as_object().unwrap().keys().cloned().collect() + } + + /// The contract's envelope schema names exactly the `summary`, `gc` and + /// top-level keys the envelope serializes — a documented counter no + /// command emits (as `bytesDownloaded` was) fails here (#1257). + #[test] + fn contract_envelope_block_matches_serialized_keys() { + let block = contract_block("### Envelope shape"); + let section = |name: &str| { + let from = block.find(&format!("\"{name}\":")).expect(name); + let rest = &block[from..]; + &rest[..rest.find("\n }").expect("section end")] + }; + let mut env = Envelope::new(Command::Repair); + env.summary.rebuilt = 1; + env.set_gc(GcReport::default()); + env.mark_error(EnvelopeError::new("x", "y")); + let value = serde_json::to_value(&env).unwrap(); + assert_eq!( + keys_at(section("summary"), 4), + object_keys(value["summary"].clone()) + ); + assert_eq!(keys_at(section("gc"), 4), object_keys(value["gc"].clone())); + let top: std::collections::BTreeSet = object_keys(value) + .into_iter() + // Additive keys documented in their own sections. + .filter(|k| !matches!(k.as_str(), "sidecars" | "warnings" | "vex")) + .collect(); + assert_eq!(keys_at(block, 2), top); + } + + /// Every `PatchEvent` key the contract documents is serialized, and + /// every serialized key is documented; every action has a row. + #[test] + fn contract_patch_event_block_matches_serialized_keys() { + let block = contract_block("### `PatchEvent` shape"); + let event = PatchEvent::new(PatchAction::Updated, "pkg:npm/a@1.0.0") + .with_uuid("u") + .with_old_uuid("o") + .with_files(vec![PatchEventFile { + path: "package/index.js".into(), + verified: true, + applied_via: Some(AppliedVia::Blob), + }]) + .with_bytes(1) + .with_reason("c", "r") + .with_error("c", "e") + .with_details(serde_json::json!({})); + let value = serde_json::to_value(&event).unwrap(); + assert_eq!(keys_at(block, 2), object_keys(value.clone())); + assert_eq!( + keys_at(block, 6), + object_keys(value["files"][0].clone()), + "files[] keys" + ); + let doc = contract_doc(); + for action in [ + PatchAction::Discovered, + PatchAction::Downloaded, + PatchAction::Applied, + PatchAction::Updated, + PatchAction::Skipped, + PatchAction::Failed, + PatchAction::Removed, + PatchAction::Verified, + PatchAction::Rebuilt, + ] { + let tag = serde_json::to_value(action).unwrap(); + let tag = tag.as_str().unwrap(); + assert!( + block.contains(&format!("\"{tag}\"")), + "{tag} in the action enum" + ); + assert!( + doc.contains(&format!("| `{tag}`")), + "{tag} has a vocabulary row" + ); + } + } + + #[test] + fn gc_report_folds_passes_and_serializes_shared_keys() { + let pass = |removed, bytes| CleanupResult { + blobs_removed: removed, + bytes_freed: bytes, + ..CleanupResult::default() + }; + let (blobs, packages) = (pass(3, 30), pass(1, 4)); + let report = GcReport::from_passes(Some(&blobs), None, Some(&packages)); + assert_eq!(report.total_removed(), 4); + assert_eq!( + report.to_value(), + serde_json::json!({ + "removedBlobs": 3, + "removedDiffArchives": 0, + "removedPackageArchives": 1, + "bytesFreed": 34, + }) + ); + let mut env = Envelope::new(Command::Remove); + assert!(serde_json::to_value(&env).unwrap().get("gc").is_none()); + assert_eq!( + serde_json::to_value(&env).unwrap()["summary"]["bytesFreed"], + 0 + ); + env.set_gc(report); + let value = serde_json::to_value(&env).unwrap(); + assert_eq!(value["gc"], report.to_value()); + assert_eq!(value["summary"]["bytesFreed"], 34); + } } diff --git a/crates/socket-patch-cli/tests/apply/bun_global_store.rs b/crates/socket-patch-cli/tests/apply/bun_global_store.rs index 2f1ffa26e..af971c940 100644 --- a/crates/socket-patch-cli/tests/apply/bun_global_store.rs +++ b/crates/socket-patch-cli/tests/apply/bun_global_store.rs @@ -188,7 +188,10 @@ fn rollback_refuses_bun_global_store_packages() { ); let v = run(&proj, "rollback"); - assert_eq!(v["status"], "partial_failure", "{v}"); + // Every package refused, nothing rolled back: the run failed as a + // whole (#1066). + assert_eq!(v["status"], "error", "{v}"); + assert_eq!(v["error"]["code"], "rollback_failed", "{v}"); assert_refused(&v, "pkg:npm/left-pad@1.3.0"); assert_refused(&v, "pkg:npm/is-number@6.0.0"); assert_eq!(std::fs::read(left_pad.join("index.js")).unwrap(), AFTER); diff --git a/crates/socket-patch-cli/tests/covgap_commands_rollback.rs b/crates/socket-patch-cli/tests/covgap_commands_rollback.rs index d6f1585d0..ce97ce3a9 100644 --- a/crates/socket-patch-cli/tests/covgap_commands_rollback.rs +++ b/crates/socket-patch-cli/tests/covgap_commands_rollback.rs @@ -931,7 +931,11 @@ fn vendored_unknown_ecosystem_fails_leg_in_both_modes() { "an unknown-backend entry must fail the run; stdout=\n{stdout}\nstderr=\n{stderr}" ); let v = parse_envelope(&stdout, &stderr); - assert_eq!(v["status"], "partial_failure", "stdout=\n{stdout}"); + // The vendored failure is the run's only outcome: a total failure + // (#1066), counted in the top-level `failed`. + assert_eq!(v["status"], "error", "stdout=\n{stdout}"); + assert_eq!(v["error"]["code"], "rollback_failed", "stdout=\n{stdout}"); + assert_eq!(v["failed"], 1, "stdout=\n{stdout}"); let failed = v["vendoredFailed"] .as_array() .expect("vendoredFailed array"); @@ -1301,7 +1305,11 @@ fn vendored_ledger_save_failure_fails_closed() { "a ledger save failure must exit 1; stdout=\n{stdout}\nstderr=\n{stderr}" ); let v = parse_envelope(&stdout, &stderr); - assert_eq!(v["status"], "partial_failure", "stdout=\n{stdout}"); + // The vendored failure is the run's only outcome: a total failure + // (#1066), counted in the top-level `failed`. + assert_eq!(v["status"], "error", "stdout=\n{stdout}"); + assert_eq!(v["error"]["code"], "rollback_failed", "stdout=\n{stdout}"); + assert_eq!(v["failed"], 1, "stdout=\n{stdout}"); let failed = v["vendoredFailed"] .as_array() .expect("vendoredFailed array"); @@ -1797,7 +1805,11 @@ fn legacy_ledger_beside_a_live_pin_is_never_the_revert_source() { ); assert_eq!(code, 1, "stdout=\n{stdout}\nstderr=\n{stderr}"); let v = parse_envelope(&stdout, &stderr); - assert_eq!(v["status"], "partial_failure", "stdout=\n{stdout}"); + // Nothing was rolled back: a total failure (#1066), and the refused + // hosted pin counts in the top-level `failed`. + assert_eq!(v["status"], "error", "stdout=\n{stdout}"); + assert_eq!(v["error"]["code"], "rollback_failed", "stdout=\n{stdout}"); + assert_eq!(v["failed"], 1, "stdout=\n{stdout}"); assert_eq!( v["hosted"]["failed"][0]["purl"], LP_PURL, "stdout=\n{stdout}" @@ -3185,7 +3197,9 @@ fn pypi_variant_group_with_no_installed_match_attempts_every_variant() { "a drifted install cannot roll back; stdout=\n{stdout}\nstderr=\n{stderr}" ); let v = parse_envelope(&stdout, &stderr); - assert_eq!(v["status"], json!("partial_failure"), "stdout=\n{stdout}"); + // Both variants failed and nothing was rolled back: a total failure. + assert_eq!(v["status"], json!("error"), "stdout=\n{stdout}"); + assert_eq!(v["error"]["code"], "rollback_failed", "stdout=\n{stdout}"); assert_eq!(v["failed"], json!(2), "stdout=\n{stdout}"); let results = v["results"].as_array().expect("results array"); let mut result_purls: Vec<&str> = results diff --git a/crates/socket-patch-cli/tests/covgap_commands_scan_hosted.rs b/crates/socket-patch-cli/tests/covgap_commands_scan_hosted.rs index 46695f131..aabccb264 100644 --- a/crates/socket-patch-cli/tests/covgap_commands_scan_hosted.rs +++ b/crates/socket-patch-cli/tests/covgap_commands_scan_hosted.rs @@ -1405,7 +1405,10 @@ async fn native_bun_lockb_hosting_dry_run_rerun_and_rollback_without_bun() { "a binary bun.lockb pin is refused: {stdout}\n{stderr}" ); let doc: Value = serde_json::from_str(&stdout).unwrap_or_else(|e| panic!("{e}: {stdout}")); - assert_eq!(doc["status"], "partial_failure", "{doc:#}"); + // The refused pin is the only outcome: a total failure (#1066). + assert_eq!(doc["status"], "error", "{doc:#}"); + assert_eq!(doc["error"]["code"], "rollback_failed", "{doc:#}"); + assert_eq!(doc["failed"], 1, "{doc:#}"); let failed = doc["hosted"]["failed"] .as_array() .unwrap_or_else(|| panic!("{doc:#}")); diff --git a/crates/socket-patch-cli/tests/covgap_commands_scan_mod.rs b/crates/socket-patch-cli/tests/covgap_commands_scan_mod.rs index 545a778e8..33d3a9337 100644 --- a/crates/socket-patch-cli/tests/covgap_commands_scan_mod.rs +++ b/crates/socket-patch-cli/tests/covgap_commands_scan_mod.rs @@ -2883,7 +2883,7 @@ async fn scan_dry_run_prune_previews_gc_in_human_and_json() { if json { let v: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid JSON"); assert!( - v["gc"]["prunableManifestEntries"] + v["gc"]["prunedManifestEntries"] .as_array() .is_some_and(|a| a.iter().any(|p| p == stale)), "{extra:?}: {v}" diff --git a/crates/socket-patch-cli/tests/e2e_bun_lockb.rs b/crates/socket-patch-cli/tests/e2e_bun_lockb.rs index b441f9bc8..ce183b5d7 100644 --- a/crates/socket-patch-cli/tests/e2e_bun_lockb.rs +++ b/crates/socket-patch-cli/tests/e2e_bun_lockb.rs @@ -122,7 +122,11 @@ fn cli_code(project: &Path, args: &[&str]) -> (i32, Value) { /// `rollback` REFUSES the pin, naming the checkout remedy, and leaves the /// lock exactly as found; the test then applies that remedy (`git checkout -- /// bun.lockb`, here: the original bytes written back). -fn rollback_refuses_binary_hosted_pin_then_checkout(fixture: &Fixture, server: &MockServer) { +fn rollback_refuses_binary_hosted_pin_then_checkout( + fixture: &Fixture, + server: &MockServer, + copy_already_original: bool, +) { let hosted_lock = fixture.lock(); let uri = server.uri(); let (code, env) = cli_code( @@ -130,7 +134,28 @@ fn rollback_refuses_binary_hosted_pin_then_checkout(fixture: &Fixture, server: & &["rollback", "--yes", "--patch-server-url", &uri], ); assert_eq!(code, 1, "a binary hosted pin cannot be restored: {env}"); - assert_eq!(env["status"], "partial_failure", "{env}"); + // The refused pin counts as a failure (#1066). After a takeover a + // manifest record makes the agent leg find the installed copy already + // original, so the run is a partial failure; a hosted-only project has + // no other outcome, so the run failed as a whole (`rollback_failed`). + if copy_already_original { + assert_eq!(env["status"], "partial_failure", "{env}"); + assert_eq!(env["alreadyOriginal"], 1, "{env}"); + } else { + assert_eq!(env["status"], "error", "{env}"); + assert_eq!(env["error"]["code"], "rollback_failed", "{env}"); + assert_eq!(env["alreadyOriginal"], 0, "{env}"); + } + // `failed` spans every leg (#1066): the refused hosted pin plus any + // agent copy that could not be restored (a bundled copy the patch never + // touched reports `hash_mismatch`). + let agent_failed = env["results"] + .as_array() + .into_iter() + .flatten() + .filter(|r| r["success"] == false) + .count(); + assert_eq!(env["failed"], 1 + agent_failed, "{env}"); let failed = env["hosted"]["failed"] .as_array() .cloned() @@ -1074,7 +1099,7 @@ async fn native_binary_hosted_vendored_takeover_roundtrip() { ); fixture.frozen("hosted-again", &fixture.patched, "minimist"); fixture.manifestless_vex("hosted-again", bun_vex::BunMode::Hosted, &server.uri()); - rollback_refuses_binary_hosted_pin_then_checkout(&fixture, &server); + rollback_refuses_binary_hosted_pin_then_checkout(&fixture, &server, true); fixture.pristine(); fixture.frozen("rolled-back", &fixture.original, "minimist"); } @@ -1184,7 +1209,7 @@ async fn binary_shared_bundled_record_hosted_pin_is_managed() { hosted["redirect"]["redirected"], 1, "hosted again: {hosted}" ); - rollback_refuses_binary_hosted_pin_then_checkout(&fixture, &server); + rollback_refuses_binary_hosted_pin_then_checkout(&fixture, &server, true); assert_eq!(fixture.lock(), fixture.original_lock); } @@ -1291,7 +1316,7 @@ async fn native_binary_alias_and_transitive() { bun_vex::BunMode::Hosted, &server.uri(), ); - rollback_refuses_binary_hosted_pin_then_checkout(&fixture, &server); + rollback_refuses_binary_hosted_pin_then_checkout(&fixture, &server, false); fixture.pristine(); fixture.stage(); let result = cli(&fixture.project, &["vendor", "--offline"]); diff --git a/crates/socket-patch-cli/tests/e2e_cargo.rs b/crates/socket-patch-cli/tests/e2e_cargo.rs index 0bd74b050..8b3e00ecb 100644 --- a/crates/socket-patch-cli/tests/e2e_cargo.rs +++ b/crates/socket-patch-cli/tests/e2e_cargo.rs @@ -361,7 +361,8 @@ async fn sync_keeps_entry_whose_shared_cache_copy_is_still_patched() { .unwrap() }; - // The preview prunes only the pristine one. + // The preview prunes only the pristine one (v5.0 one GC shape: a dry + // run reports would-be prunes under the wet pass's key). let out = run( &[ "scan", @@ -380,9 +381,9 @@ async fn sync_keeps_entry_whose_shared_cache_copy_is_still_patched() { let json: serde_json::Value = serde_json::from_str(&stdout) .unwrap_or_else(|e| panic!("scan --dry-run --sync JSON ({e}):\n{stdout}")); assert_eq!( - json["gc"]["prunableManifestEntries"], + json["gc"]["prunedManifestEntries"], serde_json::json!([ryu]), - "{json:#}" + "the preview must keep the still-patched cargo entry: {json:#}" ); let out = run( diff --git a/crates/socket-patch-cli/tests/e2e_scan.rs b/crates/socket-patch-cli/tests/e2e_scan.rs index 10285fc69..727c99dd4 100644 --- a/crates/socket-patch-cli/tests/e2e_scan.rs +++ b/crates/socket-patch-cli/tests/e2e_scan.rs @@ -638,7 +638,8 @@ fn test_scan_apply_prune_cleans_orphan_blobs() { /// `scan --json --dry-run --sync --yes` previews the full sync action: /// `apply.patches[]` is populated with would-be actions and `gc` -/// reports `prunable*`/`orphan*` counts, but nothing on disk changes. +/// reports the would-be `pruned*`/`removed*` counts, but nothing on disk +/// changes. #[test] #[ignore] fn test_scan_dry_run_sync_previews_apply_and_gc() { @@ -673,15 +674,15 @@ fn test_scan_dry_run_sync_previews_apply_and_gc() { let v = parse_scan_json(&stdout); // Preview output present. - let prunable = v["gc"]["prunableManifestEntries"] + let prunable = v["gc"]["prunedManifestEntries"] .as_array() - .expect("gc.prunableManifestEntries array"); + .expect("gc.prunedManifestEntries array"); assert!( prunable.iter().any(|p| p == NPM_PURL), "preview should list minimist as prunable; got {prunable:?}" ); assert!( - v["gc"]["orphanBlobs"].as_u64().unwrap_or(0) >= 1, + v["gc"]["removedBlobs"].as_u64().unwrap_or(0) >= 1, "preview should count at least 1 orphan blob" ); assert_eq!(v["apply"]["dryRun"], true); diff --git a/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs b/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs index 44001ef37..702258197 100644 --- a/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs +++ b/crates/socket-patch-cli/tests/in_process_rollback_hosted.rs @@ -1114,7 +1114,12 @@ async fn a_refused_pin_fails_closed_beside_a_restored_one() { let wired = std::fs::read_to_string(tmp.path().join("yarn.lock")).unwrap(); let (code, envelope) = run_rollback_subprocess(tmp.path(), &[]); assert_eq!(code, 1, "{envelope}"); - assert_eq!(envelope["status"], "partial_failure", "{envelope}"); + // Both pins refused, nothing restored: a total failure whose + // counters span the hosted leg (#1066). + assert_eq!(envelope["status"], "error", "{envelope}"); + assert_eq!(envelope["error"]["code"], "rollback_failed", "{envelope}"); + assert_eq!(envelope["failed"], 2, "{envelope}"); + assert_eq!(envelope["rolledBack"], 0, "{envelope}"); assert_eq!(envelope["hosted"]["reverted"], serde_json::json!([])); let failed: Vec<&str> = envelope["hosted"]["failed"] .as_array() @@ -1142,6 +1147,9 @@ async fn a_refused_pin_fails_closed_beside_a_restored_one() { let (code, envelope) = run_rollback_subprocess_online(tmp.path(), &server, &[]); assert_eq!(code, 1, "{envelope}"); assert_eq!(envelope["status"], "partial_failure", "{envelope}"); + // The top-level counters span the hosted leg (#1066): they were 0/0. + assert_eq!(envelope["rolledBack"], 1, "{envelope}"); + assert_eq!(envelope["failed"], 1, "{envelope}"); assert_eq!(envelope["hosted"]["reverted"], serde_json::json!([IO_PURL])); assert_eq!( envelope["hosted"]["failed"][0]["purl"], LP_PURL, diff --git a/crates/socket-patch-cli/tests/in_process_rollback_vendored.rs b/crates/socket-patch-cli/tests/in_process_rollback_vendored.rs index a31a86bbc..42715a561 100644 --- a/crates/socket-patch-cli/tests/in_process_rollback_vendored.rs +++ b/crates/socket-patch-cli/tests/in_process_rollback_vendored.rs @@ -534,7 +534,11 @@ async fn drift_keep_exits_partial_failure_and_holds_manifest() { // fixture replays identically through the binary ── let (code, env) = rollback_cli(root, &[]); assert_eq!(code, 1, "drift-keep exits 1: {env:#}"); - assert_eq!(env["status"], "partial_failure", "{env:#}"); + // The drift-keep is the run's only outcome: a total failure, counted + // in the top-level `failed` (#1066). + assert_eq!(env["status"], "error", "{env:#}"); + assert_eq!(env["error"]["code"], "rollback_failed", "{env:#}"); + assert_eq!(env["failed"], 1, "{env:#}"); let kept = env["vendoredKept"].as_array().expect("vendoredKept array"); assert_eq!(kept.len(), 1, "{env:#}"); assert_eq!(kept[0]["purl"], DRIFT_PURL, "{env:#}"); diff --git a/crates/socket-patch-cli/tests/repair/covgap_commands_repair.rs b/crates/socket-patch-cli/tests/repair/covgap_commands_repair.rs index 0dce2b57a..86ee2de21 100644 --- a/crates/socket-patch-cli/tests/repair/covgap_commands_repair.rs +++ b/crates/socket-patch-cli/tests/repair/covgap_commands_repair.rs @@ -500,11 +500,13 @@ fn repair_archive_cleanup_failure_warns_and_continues() { .contains("diff cleanup failed"), "the skip reason must name the failing archive pass; got {skip}" ); - // The packages pass still swept its orphan: one batched removal event. + // The packages pass still swept its orphan: `gc` reports it (the + // failed diffs pass counts as empty). assert_eq!( - v["summary"]["removed"], 1, + v["gc"]["removedPackageArchives"], 1, "the packages sweep must still be recorded; envelope={v}" ); + assert_eq!(v["gc"]["removedDiffArchives"], 0, "envelope={v}"); assert!( !orphan_pkg_path.exists(), "json: the packages orphan must be swept despite the diffs failure" diff --git a/crates/socket-patch-cli/tests/repair/repair_invariants.rs b/crates/socket-patch-cli/tests/repair/repair_invariants.rs index d41ade41b..082beab9a 100644 --- a/crates/socket-patch-cli/tests/repair/repair_invariants.rs +++ b/crates/socket-patch-cli/tests/repair/repair_invariants.rs @@ -385,7 +385,32 @@ fn repair_offline_removes_orphan_blob() { assert_eq!(code, 0, "expected exit 0; stdout=\n{stdout}"); let v: serde_json::Value = serde_json::from_str(&stdout).expect("envelope JSON"); assert_eq!(v["status"], "success"); - assert_eq!(v["summary"]["removed"], 1, "one orphan should be removed"); + // The paths CLI_CONTRACT.md's "GC summary" jq recipe reads (#1257): the + // shared `gc` object, its bytes mirrored into `summary.bytesFreed`. + let freed = b"orphaned content".len() as u64; + assert_eq!( + v["gc"], + serde_json::json!({ + "removedBlobs": 1, + "removedDiffArchives": 0, + "removedPackageArchives": 0, + "bytesFreed": freed, + }), + "{v:#}" + ); + assert_eq!(v["summary"]["bytesFreed"], freed); + assert_eq!(v["summary"]["failed"], 0); + // The GC carrier event carries the bytes but is not a removed patch + // entry, so `summary.removed` stays 0. + assert_eq!(v["summary"]["removed"], 0, "{v:#}"); + let carrier = v["events"] + .as_array() + .unwrap() + .iter() + .find(|e| e["action"] == "removed") + .expect("GC carrier event"); + assert_eq!(carrier["bytes"], freed); + assert_eq!(carrier["details"]["count"], 1); // The referenced blob must survive; the orphan must be gone. assert!( @@ -437,8 +462,10 @@ fn repair_dry_run_does_not_remove_orphan_blob() { "dry-run must report both blobs as checked; got {}", verified[0] ); - // Summary must mirror the preview: one verified, zero actually removed. - assert_eq!(v["summary"]["verified"], 1); + // The preview is reported in `gc` (would-remove counts, like rollback's + // dry-run `gc`); the carrier bumps no counter, and nothing was removed. + assert_eq!(v["gc"]["removedBlobs"], 1, "{v:#}"); + assert_eq!(v["summary"]["verified"], 0); assert_eq!( v["summary"]["removed"], 0, "dry-run must not record any actual removals" @@ -495,6 +522,8 @@ fn repair_download_only_skips_cleanup() { .all(|e| e["action"] != "removed" && e["action"] != "verified"), "--download-only must emit no cleanup event; got events={events:?}" ); + assert!(v.get("gc").is_none(), "no sweep ran, so no `gc`: {v:#}"); + assert_eq!(v["summary"]["bytesFreed"], 0); // Both the referenced blob and the orphan must survive untouched. assert!( socket.join("blobs").join(REFERENCED_HASH).exists(), diff --git a/crates/socket-patch-cli/tests/rollback/rollback_invariants.rs b/crates/socket-patch-cli/tests/rollback/rollback_invariants.rs index 690906f7a..b041f2fe2 100644 --- a/crates/socket-patch-cli/tests/rollback/rollback_invariants.rs +++ b/crates/socket-patch-cli/tests/rollback/rollback_invariants.rs @@ -162,7 +162,9 @@ fn rollback_offline_with_missing_before_blob_partial_failure() { "offline + missing blob must exit 1; stdout=\n{stdout}" ); let v: serde_json::Value = serde_json::from_str(&stdout).expect("valid JSON"); - assert_eq!(v["status"], "partial_failure"); + // Nothing could be rolled back: a total failure (#1066). + assert_eq!(v["status"], "error"); + assert_eq!(v["error"]["code"], "rollback_failed"); assert_eq!(v["rolledBack"], 0); assert_eq!(v["alreadyOriginal"], 0); assert_eq!(v["dryRun"], false, "not a dry-run"); @@ -293,7 +295,9 @@ fn rollback_undownloadable_blob_envelope_names_blob_and_remedy() { "undownloadable blob must exit 1; stdout=\n{stdout}\nstderr=\n{stderr}" ); let v: serde_json::Value = serde_json::from_str(&stdout).expect("valid JSON"); - assert_eq!(v["status"], "partial_failure"); + // The only patch failed: a total failure (#1066). + assert_eq!(v["status"], "error", "stdout=\n{stdout}"); + assert_eq!(v["error"]["code"], "rollback_failed", "stdout=\n{stdout}"); assert_eq!(v["failed"], 1, "stdout=\n{stdout}"); let results = v["results"].as_array().expect("results array"); assert_eq!(results.len(), 1, "stdout=\n{stdout}"); diff --git a/crates/socket-patch-cli/tests/scan/covgap_commands_scan_vendor_flow.rs b/crates/socket-patch-cli/tests/scan/covgap_commands_scan_vendor_flow.rs index 62de0897b..23c19572c 100644 --- a/crates/socket-patch-cli/tests/scan/covgap_commands_scan_vendor_flow.rs +++ b/crates/socket-patch-cli/tests/scan/covgap_commands_scan_vendor_flow.rs @@ -347,8 +347,8 @@ async fn scan_vendor_dry_run_reports_already_vendored() { /// `scan --json --mode vendored --dry-run --prune` (a legal combination — /// `--mode vendored` conflicts only with `--mode agent`/`--sync`): the vendor JSON -/// path's dry-run arm must emit the GC PREVIEW (`prunable*`/`orphan*` -/// field names, per `to_preview_json`) and mutate nothing on disk. +/// path's dry-run arm must emit the GC PREVIEW (the one `gc` shape minus +/// the wet-only keys) and mutate nothing on disk. #[tokio::test] async fn scan_vendor_dry_run_prune_previews_gc_without_mutating() { let mock = MockServer::start().await; @@ -377,17 +377,17 @@ async fn scan_vendor_dry_run_prune_previews_gc_without_mutating() { .as_object() .unwrap_or_else(|| panic!("--prune must emit a gc sub-object; envelope={v}")); assert_eq!( - gc["prunableManifestEntries"], + gc["prunedManifestEntries"], serde_json::json!([STALE_PURL]), "envelope={v}" ); assert!( - gc.contains_key("bytesReclaimable") && gc.contains_key("orphanBlobs"), - "dry+prune must use the preview field names; gc={gc:?}" + gc.contains_key("bytesFreed") && gc.contains_key("removedBlobs"), + "dry+prune uses the one gc shape; gc={gc:?}" ); assert!( - !gc.contains_key("prunedManifestEntries") && !gc.contains_key("bytesFreed"), - "dry+prune must not use the mutating pass's field names; gc={gc:?}" + !gc.contains_key("keptVendoredEntries") && !gc.contains_key("failedVendoredEntries"), + "dry+prune must not claim the wet-only vendored checks; gc={gc:?}" ); // Nothing mutated: the stale entry survives byte-for-byte. @@ -417,7 +417,7 @@ async fn scan_vendor_dry_run_prune_previews_gc_without_mutating() { let v: serde_json::Value = serde_json::from_str(stdout.trim()).expect("valid JSON"); assert_eq!(v["vendor"]["dryRun"], true, "envelope={v}"); assert_eq!( - v["gc"]["prunableManifestEntries"], + v["gc"]["prunedManifestEntries"], serde_json::json!([STALE_PURL]), "the lock-free preview lists under a held lock: {v}" ); diff --git a/crates/socket-patch-cli/tests/scan/scan_ecosystems_scope_e2e.rs b/crates/socket-patch-cli/tests/scan/scan_ecosystems_scope_e2e.rs index a7391de50..be0b26e92 100644 --- a/crates/socket-patch-cli/tests/scan/scan_ecosystems_scope_e2e.rs +++ b/crates/socket-patch-cli/tests/scan/scan_ecosystems_scope_e2e.rs @@ -253,7 +253,7 @@ async fn gc_scan_crawls_the_unselected_ecosystems() { ] { let (v, _) = scan(tmp.path(), extra).await; assert_eq!( - v["gc"]["prunableManifestEntries"], + v["gc"]["prunedManifestEntries"], serde_json::json!(["pkg:npm/orphan-npm@9.9.9"]), "{extra:?}: the installed crate must not read as uninstalled: {v}" ); diff --git a/crates/socket-patch-cli/tests/scan/scan_invariants.rs b/crates/socket-patch-cli/tests/scan/scan_invariants.rs index 44ae7b34b..e644a9afe 100644 --- a/crates/socket-patch-cli/tests/scan/scan_invariants.rs +++ b/crates/socket-patch-cli/tests/scan/scan_invariants.rs @@ -888,11 +888,10 @@ async fn scan_prune_dry_run_reports_prunable_manifest_entries() { let gc = v["gc"] .as_object() .unwrap_or_else(|| panic!("--prune must emit gc field; full envelope was: {v}")); - // Dry-run uses the *prunable*/* orphan* preview field names per the - // CLI contract. - let prunable = gc["prunableManifestEntries"] + // Dry-run reports the would-be removals under the one `gc` shape. + let prunable = gc["prunedManifestEntries"] .as_array() - .expect("prunableManifestEntries present in dry-run gc"); + .expect("prunedManifestEntries present in dry-run gc"); assert_eq!(prunable.len(), 1); assert_eq!(prunable[0], "pkg:npm/uninstalled@1.0.0");