[Mikola Lysenko (mikolalysenko)]

LLM Description written by Claude Code:claude-opus-5-5

Fixes #998
Fixes #1063
Fixes #1123

Summary

A manifest that exists but can't be read is now refused as manifest_unreadable (exit 1, with the --json envelope) on every command and under every spelling of its path. Before, several commands decided "no manifest" with their own looser probes:

• apply, apply --check and vendor report noManifest (exit 0) when .socket/manifest.json exists but can't be stat'd #998 (fail-open): apply, apply --check and vendor reported noManifest and exited 0 when stating .socket/manifest.json failed for any reason (.socket checked in as a file, a symlink loop, an unsearchable .socket/), so install hooks and CI read "every patch skipped" as success. repair, remove and rollback said manifest_not_found.
• scan reads a corrupt manifest as empty, so --prune skips GC and reports success #1063: scan's GC passes (wet and --dry-run preview) read an unloadable manifest as missing and returned a clean summary.
• A directory at .socket/manifest.json exits 2 with no --json output when --manifest-path spells the path, but exits 1 with manifest_unreadable under the default path #1123: a --manifest-path naming a directory exited 2 with no --json document when the path was spelled out (./.socket/manifest.json, an absolute path, SOCKET_MANIFEST_PATH), but 1 with manifest_unreadable under the default spelling.

Root cause (shared)

Core's read_manifest has one rule: only NotFound means absent. The callers above each decided presence another way: metadata().is_err() (#998), _ => no manifest after read_manifest (#1063), and a directory check keyed on the flag's text (#1123).

Fix

• socket_patch_core::manifest::operations::manifest_exists keeps read_manifest's rule (a thin wrapper over tokio::fs::try_exists).
• apply (incl. --check), vendor, repair, remove and rollback use it and refuse a stat error with manifest_unreadable before building a client or taking a lock. A real NotFound keeps every existing no-manifest path (noManifest, hosted/vendored fallbacks).
• scan GC: an unloadable manifest under the lock now yields a manifest_invalid/manifest_unreadable warning next to the vendored half, and the pre-lock gate no longer treats an unstatable manifest as absent. (Agent-mode scan already failed closed at its top-level load since An unparseable .socket/manifest.json is reported under five different --json error codes depending on the command #931; this closes the GC passes themselves.)
• GlobalArgs::validate_paths no longer refuses a directory manifest (the issue's option A). read_manifest refuses it the same way for every spelling. The remaining "project directory does not exist" message prints a .-free path (no more ././.socket/…).
• CLI_CONTRACT.md: the manifest_unreadable row and the exit-2 row describe the new rule.
• On Windows a path under a plain file reports as not found, so there .socket as a file still reads as "no manifest" for both read_manifest and manifest_exists (consistent). Those test cases are #[cfg(unix)].

Per-issue tests (red on main → green here)

Issue	Test	On main
#998	manifest_load_error_matrix::socket_dir_as_file_is_manifest_unreadable_on_every_command (ENOTDIR, runs as root)	FAILED (apply: noManifest, exit 0)
#998	manifest_load_error_matrix::manifest_symlink_loop_is_manifest_unreadable_on_every_command (ELOOP)	FAILED
#998	apply_invariants::apply_with_unreadable_socket_dir_fails_closed un-ignored (chmod 000; self-skips as root)	was #[ignore = "RED: …"]
#998	core test_manifest_exists_*	new API
#1063	scan::gc::tests::apply_gc_reports_an_unloadable_manifest (invalid JSON + directory, wet + preview)	FAILED (warnings: []), verified by reverting only the GC arms
#1123	manifest_load_error_matrix::directory_manifest_verdict_does_not_depend_on_spelling (flag relative, flag absolute, env)	FAILED (exit 2, no JSON)

The matrix now covers rollback too. cli_parse_list and cli_path_flags_validated pinned the old exit-2 directory verdict and are updated to the new one.

Local verification

• cargo fmt --all -- --check: clean
• cargo clippy --workspace --all-features -- -D warnings: clean
• cargo test --workspace --all-features --no-fail-fast: everything this PR touches passes (core lib, CLI lib 936, apply, repair, rollback, remove, matrix, cli_parse_list, cli_path_flags_validated). Remaining local failures are environmental and also fail on unmodified main in this container: chmod-based write/delete-failure tests that can't fail as root (repair_cleanup_failure_*, repair_*_lock_file_unremovable, three in_process_redirect write-failure tests, four core lib permission tests) and e2e_bun_lockb (Nightly e2e_bun_lockb red: offline takeover test predates #1648 #1770, fix in CI: update bun.lockb offline takeover test (#1770) #1771).
• No wrapper changes needed: npm/, pypi/, gem/ only dispatch the binary.

/code-review high

Fixed: Windows ENOTDIR→NotFound (tests now Unix-only), stale exit-2 contract row, GC comment accuracy, try_exists reuse, apply --check message parity, repair telemetry, GC message cleanup.
Not fixed, by choice:

• Share one "probe and emit" helper across the five commands. Each command already has its own error emitter (emit_error_envelope, emit_rollback_error, emit_eject_refusal, error_envelope, apply's refusal), and each call is the existing load-failure mapping, manifest_load_error, plus that emitter. A helper would save a line per site.
• scan GC's has_ledger probes --cwd instead of the manifest's project, and vendor-ledger presence probes use the same metadata().is_ok() pattern. Both predate this change and are about the vendor ledger, not the manifest. Left as follow-ups so this PR stays one root cause.

https://claude.ai/code/session_011f9aeR3v4Q36DgE4agLMEK

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 1f3e0ba. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review at 1f3e0ba (burn-down agent).

• CI: 82/82 check runs on this head green or skipped (32 success, 50 skipped); mergeable into main.
• Bugbot: reviewed 1f3e0ba, no findings. No open review threads.
• CHANGELOG.md untouched.
• Reviewer focus: the directory-manifest verdict moved from GlobalArgs::validate_paths (exit 2) to read_manifest (manifest_unreadable, exit 1), and cli_parse_list / cli_path_flags_validated were updated to match. On Windows, .socket as a plain file still reads as "no manifest".

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. Several commands decided "no manifest" with their own loose probe (metadata().is_err(), _ => after read_manifest, or a directory check on the flag's text), so a manifest that exists but can't be stated or loaded was treated as absent: apply/apply --check/vendor exited 0 with noManifest (#998), scan's GC returned a clean summary (#1063), and a directory --manifest-path exited 2 with no JSON only when spelled out (#1123). A new core manifest_exists (only NotFound is absence) is used everywhere, and stat/load errors now surface as manifest_unreadable/manifest_invalid.

Risk: low. Every change moves a fail-open path to fail-closed; a genuine NotFound keeps all existing no-manifest behavior (hosted/vendored fallbacks, noManifest). The one visible contract change is #1123: a directory --manifest-path now exits 1 with manifest_unreadable (and a JSON envelope) instead of exit 2, documented in CLI_CONTRACT.md.

Look here

• operations.rs:80: manifest_exists, a wrapper over tokio::fs::try_exists with read_manifest's rule
• apply.rs:858 and apply.rs:935: the shared refusal, now hit before the API client, lock or telemetry
• gc.rs:252 and gc.rs:83: GC gate (unwrap_or(true)) and the new load-error warning; nothing is pruned or swept without a readable manifest
• args.rs:402: the dropped flag-text directory check (A directory at .socket/manifest.json exits 2 with no --json output when --manifest-path spells the path, but exits 1 with manifest_unreadable under the default path #1123)
• vendor.rs:960: one up-front probe replaces two; --revert still never asks

Verified

• Read the full diff (13 files, +396/−85). Traced remove, repair, rollback and vendor (incl. the contested-hosted-wiring arm, which only runs without --revert, so the hoisted probe keeps its old meaning). Dangling symlink still reads as absent for both manifest_exists and read_manifest, so they agree. CHANGELOG.md untouched; no debug leftovers.
• Tests exercise each issue and were red on main per the description (ENOTDIR, ELOOP, chmod-000 un-ignored, GC wet+preview, three path spellings). Windows: .socket-as-file cases are #[cfg(unix)], documented as reading as absent there for both functions.
• CI on 1f3e0ba: 32 success, 50 path-skipped, 0 failed. Bugbot: no issues. No unresolved threads.

Changes I made: none.

Open questions: none that block merging.

Auto-merge is armed, so approving sends it straight to the merge queue.

---

Generated by Claude Code