Fix Composer reinstall hints ignoring vendor-dir (#658)

crates/socket-patch-cli/CLI_CONTRACT.md

@@ -747,7 +747,7 @@ to **six flavors**.
 | npm / vlt (`vlt-lock.json`, lockfileVersion 0 or 1 — A0 locks without a version and every other version refuse `vendor_lockfile_version_unsupported`; flavor `vlt`) | patched package **directory** `.socket/vendor/npm/<uuid>/[@scope/]<name>-<version>/node_modules/<name>/` (the extra `node_modules/<name>` level lets a package `require()` its own name), its `package.json` without `devDependencies`, plus `<uuid>/.gitignore` (re-includes the payload against the project's ignores, ignores vlt's links inside it) and `<uuid>/.gitattributes` (`-text`) | direct dependencies of the root or a workspace member only: the lock node becomes a `file` node for the directory, its importer edges and outgoing edges are re-keyed, and each importer's `package.json` spec becomes `file:<path>`; every moved entry lands where vlt's serializer puts it. A node whose only extra is one peer context (`ṗ:N`, `peer.N`, `peer.<16 hex>`: from vlt 1.0.8 a root dependency with resolved peers, from rc.15 a workspace member's) becomes a `file` node without the extra, as vlt writes `file:` dependencies, keeping its peer edges; revert restores the extra-bearing DepID. Refused before any write: transitive targets (`vendor_vlt_transitive_unsupported`), two or more instances of one `name@version` or a modifier extra, importer `peer` edges, foreign registries, a git, remote-tarball or local-directory node of the same package name (vlt records no version for it), a package `vlt build` would build in place (`vendor_vlt_build_scripts_unsupported`), a name declared in several dependency fields (`vendor_lock_entry_unsupported`), a spec that disagrees with the lock (`vendor_vlt_lock_out_of_sync`), a payload git would ignore (`vendor_artifact_gitignored`), a purl vendored under another flavor (`vendor_flavor_changed`); era-A locks warn `vendor_vlt_legacy_lockfile`; an optional dependency (or any dependency node_modules still links to its installed upstream copy) gets `vendor_vlt_reinstall_required` | fresh checkout, `vlt ci` with cold caches: the patched bytes load and `vlt-lock.json` stays byte-identical, also through a warm and a cold `vlt install --frozen-lockfile` (checked on 1.2.0, 1.0.10, 1.0.4, 1.0.0-rc.32 and 1.0.0-rc.14, and on every release by `docs/testing/vlt-compatibility.md`); no-op installs, `vlt install <new>`, `uninstall` and `vlt update` keep the direct dependency vendored. `vendor --revert` restores the registry node, edges and specs, keeping what vlt re-laid since, and refuses on drift |
 | cargo | crate dir `<name>-<version>/` (no `.cargo-checksum.json`) | (v5.0) `[patch.crates-io]` path entry in the **workspace-root `Cargo.toml`** (the manifest beside the `Cargo.lock` it detaches — never `.cargo/config*`) **+** Cargo.lock surgery (the `[[package]]` entry's `source`/`checksum` removed and its `version` set to the copy's TAGGED version `<version>+socket.<uuid>` — `<core>+<meta>.socket.<uuid>` when the version already has build metadata — with every lock reference that spells the old version rewritten, formats v1–v4; the copy's own `Cargo.toml` version carries the same tag, so the patched crate sees it in `CARGO_PKG_VERSION`; revert restores the lock byte for byte). Key: always the Socket-owned `<name>-socket-<first 8 hex of the uuid>` with `package = "<name>"` (the full uuid hex when that key is taken), never the bare crate name — cargo lets a config-file `[patch]` item (project, ancestor directory or `$CARGO_HOME`) replace the manifest item with the same key whatever its version, so keys any of those configs use are avoided and a re-run moves an entry off a now-shadowed key; two versions of one crate are wired side by side. Pre-v5 wiring in `.cargo/config.toml` / `.cargo/config` is moved into `Cargo.toml` by a re-run (`vendor`, `scan`/`get --mode vendored`) or `repair` (`cargo_wiring_migrated` note; the ledger's `cargo_patch_entry` record then names `Cargo.toml`); a detached lock entry left unwired by the pre-v5 multi-version overwrite is re-wired the same way (`cargo_wiring_restored`); every revert removes both spellings | `cargo build --locked --offline` on a fresh checkout — single-version manifest `[patch]` also builds with no network on cargo older than 1.56 (the old config-file wiring's floor); two vendored versions of ONE crate need cargo 1.45 or newer (`--offline` from an empty CARGO_HOME is enough there); older cargo fails closed whatever the index state, and a project that does not pin cargo ≥ 1.45 (`rust-version` or toolchain file) gets the `cargo_multi_version_old_cargo` warning. Note: path deps build **without** `--cap-lints allow` |
 | golang | module dir `<module>@<version>/` | `go.mod` `replace <module> <ver> => ./.socket/vendor/golang/<uuid>/<module>@<ver>` | `go build` with `GOPROXY=off` + empty `GOMODCACHE` (directory replaces bypass go.sum entirely; survives `go mod tidy`) |
-| composer | package dir `<vendor>/<name>@<version>/`; the copy's `.gitignore` / `.hgignore` are emptied and its `.gitattributes` `export-ignore` rules dropped, because Composer's path mirror skips the files they match (`vendor_composer_mirror_filters_neutralized`; a patch that rewrites one of them is refused `vendor_composer_mirror_filter_conflict`). Re-runs heal copies vendored before this | `composer.lock` only: entry's `dist` → `{type: "path", url, reference: "<patch-uuid>"}`, `source` removed, `transport-options: {symlink: false}` added. `content-hash` unaffected; `composer.json` untouched | `composer install` (from the lock alone, real copy not symlink, works under `--network none`). Composer 1 does not reinstall an already-installed package whose dist changed: remove `vendor/<vendor>/<name>` first. `composer update <pkg>` reverts it. See `docs/testing/composer-compatibility.md` |
+| composer | package dir `<vendor>/<name>@<version>/`; the copy's `.gitignore` / `.hgignore` are emptied and its `.gitattributes` `export-ignore` rules dropped, because Composer's path mirror skips the files they match (`vendor_composer_mirror_filters_neutralized`; a patch that rewrites one of them is refused `vendor_composer_mirror_filter_conflict`). Re-runs heal copies vendored before this | `composer.lock` only: entry's `dist` → `{type: "path", url, reference: "<patch-uuid>"}`, `source` removed, `transport-options: {symlink: false}` added. `content-hash` unaffected; `composer.json` untouched | `composer install` (from the lock alone, real copy not symlink, works under `--network none`). Composer 1 does not reinstall an already-installed package whose dist changed: remove `<vendor-dir>/<vendor>/<name>` first (the hint names the project's resolved vendor dir: `COMPOSER_VENDOR_DIR`, else `config.vendor-dir`, else `vendor`). `composer update <pkg>` reverts it. See `docs/testing/composer-compatibility.md` |
 | gem | gem dir `<name>-<version>/` + gemspec materialized from `specifications/` | **Gemfile + Gemfile.lock pair**: the `gem` line gains `path:` (or a managed block for transitive deps); the lock's spec block moves GEM→PATH and the DEPENDENCIES entry becomes `<name> (= <ver>)!`, in bundler's exact canonical form | `bundle install` (normal **and** `BUNDLE_FROZEN=true`), byte-stable lock. Lock-only edits are a silent unpatch — hence the mandatory pair |
 | pypi / uv (uv.lock) | rebuilt wheel (canonical PEP 427 filename; RECORD regenerated) | `[tool.uv.sources] <name> = {path}` in pyproject + surgical uv.lock rewrite; transitive deps via `[tool.uv] override-dependencies` | `uv sync --locked` / `--frozen --offline` (hash-verified, byte-stable lock) |
 | pypi / poetry (poetry.lock: legacy `[metadata.hashes]`, lock 1.0/1.1 `[metadata.files]`, 2.x `files`) | (rebuilt wheel) | lock-only: the target `[[package]]` gets `[package.source] type="file"` (+ `reference = ""` on the 0.12/1.0 layouts, which read it unconditionally) and the single `{file, hash: sha256-of-our-wheel}` entry in whichever table the generation keeps it. pyproject + `metadata.content-hash` untouched; CRLF locks keep their line endings. A lock written by Poetry < 1.4 emits `pypi_poetry_integrity_unverified` (that installer verifies no local hashes and skips an already-installed version) | `poetry check --lock && poetry sync`, cold cache (hash fail-closed from Poetry 1.4; byte-stable lock) — see `docs/testing/poetry-compatibility.md` |

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

@@ -7,11 +7,40 @@
 //! dist reference to the patch uuid; a hosted redirect keeps it and drops
 //! the entry's `source`, so an entry that had no `source` changes nothing
 //! Composer compares. Composer 1 never reinstalls a changed package. In both
-//! cases `vendor/<vendor>/<name>` must be removed first.
+//! cases `<vendor-dir>/<vendor>/<name>` must be removed first, where
+//! `<vendor-dir>` is the directory Composer installs into
+//! (`COMPOSER_VENDOR_DIR`, else `config.vendor-dir`, else `vendor`): a hint
+//! naming `vendor/` in a project that relocated it sends the user to delete
+//! a path that does not exist, and `composer install` then leaves the
+//! package unpatched (#658).
+
+use std::path::Path;
 
 use serde_json::{Map, Value};
 use socket_patch_core::patch::redirect::FileEdit;
 
+/// The project's Composer vendor directory as the hints print it: relative
+/// to `cwd` with `/` separators when it lies inside the project (`vendor`,
+/// `lib`, `deps/php`), else the absolute path `COMPOSER_VENDOR_DIR` named.
+/// A `config.vendor-dir` the crawler refuses (absolute, or climbing out of
+/// the project) prints as the `<vendor-dir>` placeholder rather than a
+/// directory that would be wrong.
+pub(crate) fn vendor_dir_label(cwd: &Path) -> String {
+    let Some(dir) =
+        socket_patch_core::crawlers::composer_crawler::resolve_local_vendor_dir_sync(cwd)
+    else {
+        return "<vendor-dir>".to_string();
+    };
+    match dir.strip_prefix(cwd) {
+        Ok(rel) if !rel.as_os_str().is_empty() => rel
+            .components()
+            .map(|c| c.as_os_str().to_string_lossy())
+            .collect::<Vec<_>>()
+            .join("/"),
+        _ => dir.display().to_string(),
+    }
+}
+
 const VENDOR_PREFIX: &str = ".socket/vendor/composer/";
 
 /// The `<vendor>/<name>` of every composer.lock package (`packages` and
@@ -45,15 +74,21 @@ pub(crate) fn vendored_composer_packages(lock_text: &str) -> Vec<String> {
 
 /// The lines to print after a vendored run that left `packages` wired, or
 /// nothing when none are.
-pub(crate) fn vendored_reinstall_hints(packages: &[String]) -> Vec<String> {
+/// `vendor_dir` is the [`vendor_dir_label`] the package dirs are under.
+pub(crate) fn vendored_reinstall_hints(packages: &[String], vendor_dir: &str) -> Vec<String> {
     if packages.is_empty() {
         return Vec::new();
     }
-    let dirs: Vec<String> = packages.iter().map(|p| format!("vendor/{p}")).collect();
+    let dirs: Vec<String> = packages
+        .iter()
+        .map(|p| format!("{vendor_dir}/{p}"))
+        .collect();
     vec![
-        "Run `composer install` to update vendor/ — vendoring rewires composer.lock only, so \
-         the installed vendor/ tree keeps the unpatched bytes until Composer reinstalls it."
-            .to_string(),
+        format!(
+            "Run `composer install` to update {vendor_dir}/ — vendoring rewires composer.lock \
+             only, so the installed {vendor_dir}/ tree keeps the unpatched bytes until Composer \
+             reinstalls it."
+        ),
         format!(
             "Composer 1 does not reinstall a locked package whose dist changed: remove {} \
              first, then run `composer install`.",
@@ -65,30 +100,34 @@ pub(crate) fn vendored_reinstall_hints(packages: &[String]) -> Vec<String> {
 /// The reinstall example for a hosted run's next steps when it rewrote
 /// `composer.lock`. `edits` are the run's rewrite edits: a redirected entry
 /// whose edit removed no `source` must have its vendor dir removed on every
-/// Composer version.
-pub(crate) fn hosted_reinstall_hint(files: &[String], edits: &[FileEdit]) -> Option<String> {
+/// Composer version. `vendor_dir` is the [`vendor_dir_label`] the package
+/// dirs are under.
+pub(crate) fn hosted_reinstall_hint(
+    files: &[String],
+    edits: &[FileEdit],
+    vendor_dir: &str,
+) -> Option<String> {
     if !files.iter().any(|f| f == "composer.lock") {
         return None;
     }
     let mut unchanged: Vec<String> = edits
         .iter()
         .filter(|e| e.path == "composer.lock" && e.kind == "redirect_composer_dist")
         .filter(|e| !removes_source(e))
-        .filter_map(|e| Some(format!("vendor/{}", e.key.as_deref()?.to_lowercase())))
+        .filter_map(|e| Some(format!("{vendor_dir}/{}", e.key.as_deref()?.to_lowercase())))
         .collect();
     unchanged.sort_unstable();
     unchanged.dedup();
     if unchanged.is_empty() {
-        return Some(
+        return Some(format!(
             " (e.g. `composer install`; on Composer 1 first remove the patched packages' \
-             vendor/<vendor>/<name> directories)"
-                .to_string(),
-        );
+             {vendor_dir}/<vendor>/<name> directories)"
+        ));
     }
     Some(format!(
         " (e.g. `composer install`; first remove {} — Composer does not reinstall a package \
          whose lock entry has no `source` when only its dist url changes — and on Composer 1 \
-         every other patched package's vendor/<vendor>/<name> directory)",
+         every other patched package's {vendor_dir}/<vendor>/<name> directory)",
         unchanged.join(", ")
     ))
 }
@@ -146,8 +185,8 @@ mod tests {
 
     #[test]
     fn vendored_hints_name_every_package_dir() {
-        assert!(vendored_reinstall_hints(&[]).is_empty());
-        let hints = vendored_reinstall_hints(&["a/b".to_string(), "c/d".to_string()]);
+        assert!(vendored_reinstall_hints(&[], "vendor").is_empty());
+        let hints = vendored_reinstall_hints(&["a/b".to_string(), "c/d".to_string()], "vendor");
         assert_eq!(hints.len(), 2);
         assert!(
             hints[0].starts_with("Run `composer install`"),
@@ -163,8 +202,8 @@ mod tests {
 
     #[test]
     fn hosted_hint_needs_a_rewritten_composer_lock() {
-        assert!(hosted_reinstall_hint(&["package-lock.json".to_string()], &[]).is_none());
-        let hint = hosted_reinstall_hint(&["composer.lock".to_string()], &[]).unwrap();
+        assert!(hosted_reinstall_hint(&["package-lock.json".to_string()], &[], "vendor").is_none());
+        let hint = hosted_reinstall_hint(&["composer.lock".to_string()], &[], "vendor").unwrap();
         assert!(hint.contains("composer install") && hint.contains("Composer 1"));
     }
 
@@ -200,14 +239,70 @@ mod tests {
             &format!(r#"{NEW_DIST}, "extra": {{"source": {{}}}}"#),
         );
         for edits in [vec![with_source.clone()], vec![source_after_extra]] {
-            let hint = hosted_reinstall_hint(&files, &edits).unwrap();
+            let hint = hosted_reinstall_hint(&files, &edits, "vendor").unwrap();
             assert!(hint.contains("on Composer 1 first remove"), "{hint}");
         }
 
         let dist_only = dist_edit("Acme/Lib", OLD_DIST, NEW_DIST);
-        let hint = hosted_reinstall_hint(&files, &[with_source, dist_only]).unwrap();
+        let hint =
+            hosted_reinstall_hint(&files, &[with_source.clone(), dist_only.clone()], "vendor")
+                .unwrap();
         assert!(hint.contains("first remove vendor/acme/lib —"), "{hint}");
         assert!(!hint.contains("vendor/psr/log"), "{hint}");
         assert!(hint.contains("on Composer 1"), "{hint}");
+
+        // A relocated vendor dir is the one named (#658).
+        let hint = hosted_reinstall_hint(&files, &[with_source, dist_only], "lib").unwrap();
+        assert!(hint.contains("first remove lib/acme/lib —"), "{hint}");
+        assert!(
+            hint.contains("patched package's lib/<vendor>/<name>"),
+            "{hint}"
+        );
+        assert!(!hint.contains("vendor/"), "{hint}");
+        let hint = hosted_reinstall_hint(&files, &[], "lib").unwrap();
+        assert!(
+            hint.contains("remove the patched packages' lib/<vendor>/<name>"),
+            "{hint}"
+        );
+    }
+
+    /// #658: with `config.vendor-dir: lib` the vendored hints name
+    /// `lib/<vendor>/<name>`, the directory Composer installed into.
+    #[test]
+    fn vendored_hints_name_a_relocated_vendor_dir() {
+        let hints = vendored_reinstall_hints(&["acme/tool".to_string()], "lib");
+        assert!(hints[0].contains("update lib/"), "{}", hints[0]);
+        assert!(
+            hints[1].contains("remove lib/acme/tool first"),
+            "{}",
+            hints[1]
+        );
+        assert!(!hints.join("\n").contains("vendor/"), "{hints:?}");
+    }
+
+    /// The label follows Composer's own resolution: `config.vendor-dir`
+    /// (normalized, `/`-separated) else `vendor`; a refused value prints a
+    /// placeholder instead of a wrong path. `COMPOSER_VENDOR_DIR` is left
+    /// unset by the test environment, as everywhere in this crate.
+    #[test]
+    fn vendor_dir_label_follows_config_vendor_dir() {
+        let label = |json: Option<&str>| {
+            let dir = tempfile::tempdir().unwrap();
+            if let Some(json) = json {
+                std::fs::write(dir.path().join("composer.json"), json).unwrap();
+            }
+            vendor_dir_label(dir.path())
+        };
+        assert_eq!(label(None), "vendor");
+        assert_eq!(label(Some("{}")), "vendor");
+        assert_eq!(label(Some(r#"{"config":{"vendor-dir":"lib"}}"#)), "lib");
+        assert_eq!(
+            label(Some(r#"{"config":{"vendor-dir":"./deps\\php/"}}"#)),
+            "deps/php"
+        );
+        assert_eq!(
+            label(Some(r#"{"config":{"vendor-dir":"../outside"}}"#)),
+            "<vendor-dir>"
+        );
     }
 }