[Mikola Lysenko (mikolalysenko)]

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

Fixes #1766

Summary

In a go.work workspace, rollback and remove in a member other than
the root (use ./a) now warn go_vendor_modules_txt_out_of_sync, and
apply --check exits 1, when vendor/modules.txt still records the
removed socket replace. Before this change both commands exited 0 with
no warning, and every workspace build then failed with
"inconsistent vendoring".

Root cause

vendor_modules_txt_issues only judged a recorded line stale when
ReplaceEntry::socket_owned() held. detect_owner anchors that test at
the project root, by design. go work vendor records a member's
directory replace relative to the go.work directory
(./a/.socket/go-patches/…), so the line read as user-authored and was
never checked. The forward ("not recorded") check got by on a loose
path-suffix match, and that match also let a stale ./a/… line pass as
member b's ./.socket/… replace.

Fix (crates/socket-patch-core/src/vendor/go_consumer_sync.rs)

• Recorded lines resolve against the directory that holds vendor/
  (the go.work or go.mod directory). Each go.mod/go.work replace resolves
  against its own file's directory, and the two are compared as go joins
  them (lexically). This replaces the suffix rule.
• A recorded line also counts as socket-owned when its resolved target
  lies under any readable use member's .socket/go-patches/ or
  .socket/vendor/golang/ directory. detect_owner stays root-anchored,
  so a user's ../sibling/.socket/… replace stays user-owned.
• A member whose go.mod can't be read is skipped. Its replaces are
  unknown, so its lines are not reported stale from another member.

No docs change: docs/ecosystems.md already documents this warning for
go work vendor. The npm, PyPI and gem wrappers don't touch this code.

Test evidence

Issue	Regression test	Without fix	With fix
#1766 (rollback, remove; go-patches + vendored copies; nested sub/a and out-of-tree ../a members)	unit vendor::go_consumer_sync::tests::workspace_member_rollback_leaves_modules_txt_stale	FAILED (a .socket/go-patches/…: [], no issue reported)	ok
#1766 real go 1.24.7 repro (use ./consumer, go work vendor, rollback then remove, apply --check, rebuild)	e2e e2e_golang_build::go_work_vendor_member_needs_go_work_vendor_after_rollback_and_remove	FAILED ("rollback must name the vendor/modules.txt regeneration")	ok

The same unit test also covers the review follow-ups: a cross-member
same-spelling collision (a stale ./a/… line next to b's ./.socket/…
replace) and a member with an unreadable go.mod.

Local gates:

• cargo fmt --all -- --check: clean
• cargo clippy --workspace --all-features -- -D warnings: clean
• SOCKET_PATCH_GO_E2E_REQUIRED=1 cargo test -p socket-patch-cli --all-features --test e2e_golang_build: 28 passed
• cargo test --workspace --all-features: 15,076 passed, 12 failed. All 12 failures are environmental and outside Go code: 3 in e2e_bun_lockb fail identically on main with the local bun 1.4.4, and 9 write-failure, unremovable-lock or symlink-root tests (partial_lockfile_write_failure…, repair_cleanup_failure…, copy_tree::relax_loop…, pypi_*::wire_*failure…, vlt_heal and similar) can't simulate permission denial because this sandbox runs as root. CI runs them unprivileged.

/code-review high was run on the diff. Findings fixed: the suffix-match
false negative, the wrong-level ownership heuristic, the absolute-path
exclusion (cross-volume Windows members), false stale reports for
unreadable members, and the per-line clones and normalization. Not
fixed: the claim that the e2e comment is wrong ("rollback dropped the
manifest entry"). Rollback does remove the entry (see rollback.rs, "remove … from the local
manifest"), and the test showed re-apply discovering 0 patches until the
patch was re-staged.

https://claude.ai/code/session_011f9aeSV7eaqPG4mX6fgBKB

---

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 9376571. Configure here.}

[Mikola Lysenko (mikolalysenko)]

Ready for review at 9376571b (burn-down agent label sweep).

• CI: 82/82 check runs green or skipped on this head (32 success, 50 skipped); mergeable, no conflicts.
• Bugbot: reviewed 9376571 (check success), no review threads.
• Look at: vendor/go_consumer_sync.rs — recorded vendor/modules.txt lines now resolve against the vendor directory and count as socket-owned under any use member's .socket/ dirs, replacing the old path-suffix match.
• Slack announcement not sent this run (Slack send unavailable); next run retries.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Final review brief

What it does. In a go.work workspace, rollback/remove in a non-root member (use ./a) now warn that vendor/modules.txt is stale. Before, the leftover ./a/.socket/… line was treated as user-authored and the warning was missed, so the next workspace build failed with "inconsistent vendoring". The path-suffix check is replaced with a comparison of resolved paths, done the way go records them.

Risk: low. The change only reads files and only decides whether an audit warning is emitted. Outside a workspace, work_dir falls back to the project root, so behaviour there is unchanged.

Look here

• go_consumer_sync.rs:553-594:`` each replace is resolved against its own file's dir, and each recorded line against the go.work dir.
• go_consumer_sync.rs:601-625:``` the missing-line and stale-line loops.
• go_consumer_sync.rs:633-645:```` member_owned. It matches whole path components, so member `a` never matches `ab/`, and `../sibling/.socket` stays user-owned.
• go_consumer_sync.rs:866 unit tests and e2e_golang_build.rs:507, which runs real go work vendor.

Verified

• I read the full diff. The tests would fail on main: detect_owner("./a/.socket/…") is None there, so the stale line was never checked.
• The cross-member a/b case, an unreadable member, members outside the go.work dir (../a), and Windows separators and CRLF are all handled.
• CHANGELOG.md is untouched and there is no leftover debug code.
• CI is green on the head (32 passed, 50 skipped, 0 failed), and Bugbot is clean.

Changes I made: none.

Open questions (non-blocking):

• lexical_target calls current_dir() on every comparison. That is cheap at this scale.
• If the root member has an unreadable go.mod, it can still report a ./.socket line as stale. This existed before the PR.

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

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Bugbot is paused — your team has an unpaid invoice

Bugbot is paused because your team has an unpaid invoice.

Pay it from the billing page in the Cursor dashboard and Bugbot resumes automatically.

(requestId: 1a13b573-aab2-4fb1-9eab-55a41a9bbe20)