Fix manifest read errors treated as absent (#998, #1063, #1123)

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -1734,7 +1734,7 @@ outcome for a selected record, rather than inferring a write from exit 0.
 | `manifest_not_found`  | remove, repair, rollback, vex, apply (only when the manifest vanishes between apply's existence probe and its locked read) (not `list` since v5.0: a missing manifest is an empty list) | `.socket/manifest.json` doesn't exist. For `vex` (and `scan --vex`) it fires only when, in addition, NOTHING else names a patch — no vendor-ledger entry, no lockfile reference (hosted or vendored) — and the message says so (exit 2 standalone; `apply`/`vendor --vex` treat it as their calm no-op). v3.5: `repair` proceeds anyway (vendored phase only) when a vendor ledger or vendor-path lockfile references exist, and exits 0 with a `redirect_only_project` skip (not this error) when the project's only patch state is hosted pins in its lockfiles (v5.0; or a pre-v5 `redirect-state.json`). `list` likewise no longer fires this on a hosted-only project: v5.0 lists every hosted pin the lockfiles wire (exit 0, labeled `details.mode: "hosted"` + `details.lockfiles: [<files wiring it>]` — no `details.ledger`, since hosted mode keeps none; when the manifest exists too, both are shown, purl-sorted with the manifest entry first on a tie). A pin carries its uuid and empty details unless a pre-v5 redirect ledger records the same purl and uuid (read for migration only: its record supplies the vulnerabilities / tier / description); a pre-v5 ledger record whose pin is in no lockfile is not listed. v5.0: `list` reads the vendor ledger the same way — a vendored-only project (every `scan`/`get --mode vendored` project) lists its ledger entries' embedded records labeled `Mode: vendored (recorded in .socket/vendor/state.json)` in human mode — the twin of the hosted `Mode: hosted (wired in <lockfiles>)` line — (`details.mode: "vendored"` + `details.ledger: ".socket/vendor/state.json"` in JSON), exit 0. A standalone-`vendor` entry's fallback `record` lists the same way once no manifest entry covers it (by ledger key or base purl) — the copy manifest-less `vex` attests from, so `list` never reports `manifest_not_found` for a tree whose VEX document attests a patch; while the manifest covers it, only the manifest entry is listed. All sources always come from the SAME project (see the `--manifest-path` flag row), so `--manifest-path` into another project reads that project's state, never the local one. The error still fires when NONE of the three sources has a patch, and a present-but-broken manifest still reports `manifest_invalid`/`manifest_unreadable` regardless (corruption is never masked). A malformed pre-v5 redirect ledger degrades to "nothing to consult" with a stderr warning, muted by `--silent` (the pins still list); `list --json` carries it in the run-level `warnings[]` as `redirect_ledger_corrupt` instead of on stderr. v5.0: `rollback` likewise proceeds manifest-less when the vendor ledger or the lockfiles' hosted pins hold work (its error is the legacy `{status: "error", error: {code: "manifest_not_found", message: "Manifest not found"}, path}` shape); only the truly-empty project — no manifest, no vendor ledger, no hosted pin (a lone pre-v5 redirect ledger is deleted, exit 0) — keeps the exit-1 error, and a project whose lockfiles still reference `.socket/vendor/` artifacts with NO vendor ledger gets a distinct error naming `socket-patch repair`. `remove` (v5.0) proceeds manifest-less whenever a vendor ledger file exists or the lockfiles pin a hosted patch (an existence probe and the read-only hosted-pin discovery before the lock; the vendor ledger loads under it): ANY vendor-ledger entry matching the identifier — detached or not — is removed through the ledger path (`--preserve-state` and drift-keeps behave exactly as on the manifest path), a hosted-only match restores its upstream registry entry, and when that state exists but holds nothing for the identifier the error is `not_found` (exit 1), not this code — `manifest_not_found` fires from `remove` only when all three sources are empty. Manifest entries are removed in sorted purl order. |
 | `ambiguous_target`    | remove, get, rollback            | The identifier is a package name whose last-segment rule selects several packages across the manifest, the vendor ledger and the hosted pins (`core` → `@angular/core` and `@babel/core`). Nothing is changed (exit 1); the message names each package. Use the full name or a purl. See **Target grammar**. |
 | `manifest_invalid`    | list, remove, apply (incl. `--check`), repair, vendor (incl. `--check`), vex, rollback, get, scan (agent mode) | Manifest exists but is unparseable (malformed JSON or a schema violation). **v5.0, MAJOR (#931)**: one mapping on every command — `apply` used to say `apply_failed`, `repair` `repair_failed`, `vendor` the undocumented `invalid_manifest`, and `apply --check` / `vendor --check` / `vex` / `get` / `rollback` `manifest_unreadable`. Exit codes unchanged (`vex`: 2, every other command 1). Agent-mode `scan` fails with it (exit 1); the other scan modes, which don't read the manifest, add a warning with the same code and go on. |
-| `manifest_unreadable` | list, remove, apply (incl. `--check`), repair, vendor (incl. `--check`), vex, rollback, get, scan (agent mode) | Manifest exists but cannot be read (an I/O error: permission denied, a directory at the path, …). `vex`: exit 2. Other scan modes warn, as for `manifest_invalid`. |
+| `manifest_unreadable` | list, remove, apply (incl. `--check`), repair, vendor (incl. `--check`), vex, rollback, get, scan (agent mode) | Manifest exists but cannot be read (an I/O error: permission denied, a directory at the path, …). Only a missing file is absence: a path that cannot even be stated (an unsearchable `.socket/`, `.socket` as a plain file, a symlink loop) is this code too, never `noManifest` or `manifest_not_found` (#998), and the verdict is the same whether the path is the default, spelled with `--manifest-path` or set through `SOCKET_MANIFEST_PATH` (#1123). `vex`: exit 2. Other scan modes warn, as for `manifest_invalid`, and so does a `scan --prune` GC pass that finds the manifest unloadable under its lock (#1063). |
 | `manifest_write_failed` | get                            | The manifest write after a download failed; the blobs that run wrote are removed again. |
 | `patch_fetch_failed`  | get                              | The patch search or view request failed. |
 | `patch_no_applicable_files` | get                        | The fetched patch has no file it could record; nothing written. |
@@ -2071,7 +2071,7 @@ Exit `1` when `status` is `partialFailure` (any `events[*].action == "failed"`)
 |---|---|
 | `0` | Success |
 | `1` | Error (missing/invalid manifest, fetch failed, apply failed, selection cancelled in non-JSON mode, an invalid or ambiguous socket.yml on `scan` (v5.0), etc.) |
-| `2` | Usage error: clap parse failures (unknown flag/value, missing required arg, an unknown subcommand such as the removed `setup`) and the conflicts the commands enforce themselves — `scan`'s cross-mode conflict (`--sync` combined with a `--mode` other than `agent`, rejected in `resolve_mode_flags`) and `--mode hosted\|vendored` with `--global`/`--global-prefix` (same enforcement point; `get` refuses the same combination) or with a `--manifest-path` in another project (v5.0, `manifest_path_foreign_project`; `get` and `vendor` other than `--check` likewise, as do `apply --vex` and any other `scan --vex`); a `scan`/`get` with no `--mode` on a project holding both agent and vendored state (`mode_ambiguous`, v5.0); in hosted/vendored `scan` (bare `scan` included), a PATH that is not a directory, a PATH glob matching no directory, and `--json` with more than one project directory (`run_project_dirs`); on every project command (all but `--update`), a `--cwd` / `SOCKET_CWD` or `--global-prefix` / `SOCKET_GLOBAL_PREFIX` that is not an existing directory, and a non-default `--manifest-path` / `SOCKET_MANIFEST_PATH` that names a directory or sits in a project directory that does not exist (v5.0; checked once before the command runs, whether or not the command uses the flag — previously these read as an empty project and exited 0, and `remove` / `rollback` exited 1; a missing manifest FILE in an existing project stays legal, and `get` / `scan`, which create the manifest, still create a missing manifest directory); `remove --preserve-state --skip-rollback` (the no-op quadrant; flag- or env-sourced alike), an unparseable path glob on `scan`/`rollback`, a `scan` PATH outside the repository root and a malformed `SOCKET_MIN_SEVERITY` or `SOCKET_MAX_NEW_PATCHES` (v5.0), `repair --offline --download-only`. Under `--json`, every usage error a command enforces itself prints the coded error on stdout (v5.0, MAJOR: `scan`, `remove` and `rollback` printed nothing there): a full envelope for the envelope commands, `{status: "error", error: {code, message}}` for `scan`, `get` and `rollback`; clap's own parse errors, and the path-flag check that runs before every command (`--cwd` / `--global-prefix` / `--manifest-path` naming nothing), still print nothing on stdout. `vex` also exits `2` on hard errors before document generation (see its tri-state table below). v5.0: `get`'s self-enforced conflicts exit `2` too (`--id`/`--cve`/`--ghsa`/`--package` multi-select, `--mode hosted\|vendored --save-only`, a malformed identifier for a forced `--id`/`--cve`/`--ghsa`) — previously `1` (MAJOR). The never-implemented `get --one-off` / `rollback --one-off` (and `SOCKET_ONE_OFF`) are removed in v5.0; `--one-off` is now an ordinary unknown-flag clap error. |
+| `2` | Usage error: clap parse failures (unknown flag/value, missing required arg, an unknown subcommand such as the removed `setup`) and the conflicts the commands enforce themselves — `scan`'s cross-mode conflict (`--sync` combined with a `--mode` other than `agent`, rejected in `resolve_mode_flags`) and `--mode hosted\|vendored` with `--global`/`--global-prefix` (same enforcement point; `get` refuses the same combination) or with a `--manifest-path` in another project (v5.0, `manifest_path_foreign_project`; `get` and `vendor` other than `--check` likewise, as do `apply --vex` and any other `scan --vex`); a `scan`/`get` with no `--mode` on a project holding both agent and vendored state (`mode_ambiguous`, v5.0); in hosted/vendored `scan` (bare `scan` included), a PATH that is not a directory, a PATH glob matching no directory, and `--json` with more than one project directory (`run_project_dirs`); on every project command (all but `--update`), a `--cwd` / `SOCKET_CWD` or `--global-prefix` / `SOCKET_GLOBAL_PREFIX` that is not an existing directory, and a non-default `--manifest-path` / `SOCKET_MANIFEST_PATH` that sits in a project directory that does not exist (v5.0; checked once before the command runs, whether or not the command uses the flag — previously these read as an empty project and exited 0, and `remove` / `rollback` exited 1; a missing manifest FILE in an existing project stays legal, and `get` / `scan`, which create the manifest, still create a missing manifest directory; one that names a directory is an unreadable manifest, `manifest_unreadable` exit 1 like the default path, not a usage error, #1123); `remove --preserve-state --skip-rollback` (the no-op quadrant; flag- or env-sourced alike), an unparseable path glob on `scan`/`rollback`, a `scan` PATH outside the repository root and a malformed `SOCKET_MIN_SEVERITY` or `SOCKET_MAX_NEW_PATCHES` (v5.0), `repair --offline --download-only`. Under `--json`, every usage error a command enforces itself prints the coded error on stdout (v5.0, MAJOR: `scan`, `remove` and `rollback` printed nothing there): a full envelope for the envelope commands, `{status: "error", error: {code, message}}` for `scan`, `get` and `rollback`; clap's own parse errors, and the path-flag check that runs before every command (`--cwd` / `--global-prefix` / `--manifest-path` naming nothing), still print nothing on stdout. `vex` also exits `2` on hard errors before document generation (see its tri-state table below). v5.0: `get`'s self-enforced conflicts exit `2` too (`--id`/`--cve`/`--ghsa`/`--package` multi-select, `--mode hosted\|vendored --save-only`, a malformed identifier for a forced `--id`/`--cve`/`--ghsa`) — previously `1` (MAJOR). The never-implemented `get --one-off` / `rollback --one-off` (and `SOCKET_ONE_OFF`) are removed in v5.0; `--one-off` is now an ordinary unknown-flag clap error. |
 
 `list` returns **`0`** for every project it can read, empty or not (**v5.0, BREAKING**: a project with no manifest and no ledger record — normal for hosted mode, which writes no manifest — used to exit `1` with `manifest_not_found`; it is now an empty list: `No patches in this project. Run \`socket-patch scan\`.` on stdout, and under `--json` the success envelope with `events: []`). Only an unreadable or invalid manifest (`manifest_unreadable` / `manifest_invalid`) exits `1`. Every lock-taking subcommand — including `scan`/`get --mode hosted` as of v5.0 — returns **`1`** with `error.code: lock_held` when another live socket-patch process holds `<.socket>/apply.lock`.
 

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

@@ -357,8 +357,8 @@ pub struct GlobalArgs {
 impl GlobalArgs {
     /// Reject path flags that name nothing: `--cwd` and `--global-prefix`
     /// (flag or env) must be existing directories, and a `--manifest-path`
-    /// other than the default must sit in an existing project directory and
-    /// must not itself be a directory. `main` maps `Err` to the usage exit
+    /// other than the default must sit in an existing project directory.
+    /// `main` maps `Err` to the usage exit
     /// (2), the same exit a hosted/vendored `scan` PATH that is not a
     /// directory gets.
     ///
@@ -399,16 +399,15 @@ impl GlobalArgs {
                 ));
             }
         }
+        // A manifest path that names a directory is not refused here: that
+        // is a corrupt project, not a bad flag, and `read_manifest` refuses
+        // it as `manifest_unreadable` under every spelling of the path. A
+        // check keyed on the flag's text gave the explicit spellings exit 2
+        // with no `--json` document instead (#1123).
         if self.manifest_path != DEFAULT_PATCH_MANIFEST_PATH {
-            let manifest = self.resolved_manifest_path();
-            if manifest.is_dir() {
-                return Err(not_dir(
-                    "--manifest-path",
-                    "SOCKET_MANIFEST_PATH",
-                    &manifest,
-                    "is a directory, not a manifest file",
-                ));
-            }
+            // `.`-free for the message: `--cwd .` joined onto
+            // `./.socket/manifest.json` would print `././.socket/…`.
+            let manifest: PathBuf = self.resolved_manifest_path().components().collect();
             // The directory that must exist: the project of a `.socket/`
             // manifest (`.socket/` itself is created on demand), else the
             // manifest file's own directory.

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

@@ -6,7 +6,7 @@ use socket_patch_core::crawlers::ruby_crawler::{
 use socket_patch_core::crawlers::{
     bun_uses_global_store, detect_npm_pkg_manager, Ecosystem, NpmPkgManager, RubyCrawler,
 };
-use socket_patch_core::manifest::operations::read_manifest;
+use socket_patch_core::manifest::operations::{manifest_exists, read_manifest};
 use socket_patch_core::manifest::schema::{PatchFileInfo, PatchManifest, PatchRecord};
 use socket_patch_core::patch::apply::{
     apply_package_patch, verify_file_patch, ApplyResult, MismatchPolicy, PatchSources, VerifyStatus,
@@ -318,25 +318,7 @@ async fn run_check(args: &ApplyArgs, manifest_path: &Path) -> i32 {
             }
             return 0;
         }
-        Err(e) => {
-            let msg = format!(
-                "Patch check could not read the manifest ({e}); \
-                 treating it as drift (fail-closed)."
-            );
-            if args.common.json {
-                let mut env = Envelope::new(Command::Apply);
-                env.dry_run = args.common.dry_run;
-                let mut err = crate::json_envelope::manifest_load_error(manifest_path, &e);
-                err.message = msg;
-                env.mark_error(err);
-                println!("{}", env.to_pretty_json());
-            } else {
-                // Errors print even under --silent ("errors only", never
-                // "nothing"): exit 1 with no message would be undiagnosable.
-                eprintln!("Error: {msg}");
-            }
-            return 1;
-        }
+        Err(e) => return refuse_unreadable_manifest(args, manifest_path, &e),
     };
 
     // (purl_or_name, reason_code, detail) for each drift.
@@ -868,6 +850,32 @@ fn manifest_targets_npm(manifest: &PatchManifest) -> bool {
 /// The yarn-berry PnP refusal's envelope error text.
 const YARN_PNP_UNSUPPORTED: &str = "yarn-berry Plug'n'Play layout is not supported by socket-patch (packages live inside .yarn/cache zips). Use `yarn patch <pkg>` instead.";
 
+/// A manifest that exists but cannot be stated, refused before the API
+/// client, the lock or telemetry are touched: the same envelope and exit
+/// as the load failure further down (#931, #998).
+/// `--check` says so as the drift it is treated as, whether the manifest
+/// failed to stat here or to load in [`run_check`].
+fn refuse_unreadable_manifest(args: &ApplyArgs, manifest_path: &Path, e: &std::io::Error) -> i32 {
+    let mut err = crate::json_envelope::manifest_load_error(manifest_path, e);
+    if args.check {
+        err.message = format!(
+            "Patch check could not read the manifest ({e}); \
+             treating it as drift (fail-closed)."
+        );
+    }
+    if args.common.json {
+        let mut env = Envelope::new(Command::Apply);
+        env.dry_run = args.common.dry_run;
+        env.mark_error(err);
+        println!("{}", env.to_pretty_json());
+    } else {
+        // Errors print even under --silent ("errors only", never
+        // "nothing"): exit 1 with no message would be undiagnosable.
+        eprintln!("Error: {}", crate::ui::sentence_case(&err.message));
+    }
+    1
+}
+
 fn refuse_yarn_pnp(args: &ApplyArgs) -> i32 {
     refuse_yarn_pnp_with_skips(args, &[])
 }
@@ -921,7 +929,14 @@ pub async fn run(args: ApplyArgs) -> i32 {
     // every install).
     // Nothing below this gate is touched — no API client (its config read,
     // stderr advisory and org-slug round-trip), no lock, no `.socket/`.
-    if tokio::fs::metadata(&manifest_path).await.is_err() {
+    // Only a missing file is "no manifest": one apply cannot even stat
+    // (an unsearchable `.socket/`, `.socket` as a file, a symlink loop)
+    // fails closed, or every patch would be skipped with exit 0 (#998).
+    let manifest_present = match manifest_exists(&manifest_path).await {
+        Ok(present) => present,
+        Err(e) => return refuse_unreadable_manifest(&args, &manifest_path, &e),
+    };
+    if !manifest_present {
         // A yarn-PnP layout refuses loudly even with no manifest: scan
         // cannot discover PnP packages (they live inside .yarn/cache zips),
         // so it never writes one — without this hoisted check the layout