Fix stray files breaking vendored Gradle (#656) by mikolalysenko · Pull Request #1819 · SocketDev/socket-patch

Mikola Lysenko (mikolalysenko) · 2026-10-29T08:23:59Z

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

Fixes #656

Summary

A file in a vendored JVM tree that its index doesn't list (a Finder .DS_Store, Thumbs.db, an editor backup, a stray directory) fails every Gradle build with "unindexed files … re-run socket-patch vendor". Before this PR, that remedy did nothing: vendor reported already_vendored, repair changed nothing, and only vendor --check saw the problem.

Now:

vendor treats unindexed entries in its tree directories as drift. --dry-run previews a re-vendor (status ready, not already_vendored). A real run deletes the strays before it writes any wiring, then re-vendors. A stray symlink is deleted, not its target. A stray directory is deleted with its contents, since only socket-patch's files belong in the tree.
repair deletes them offline for a healthy JVM entry and records rebuilt with unindexedRemoved: [paths]. repair --dry-run records verified with wouldRemoveUnindexed: [paths] and writes nothing.
vendor --check (check_entry) and the sweep share one lister (jvm::apply::unindexed_in), so they agree on what counts as unindexed.

Covers the Gradle tree (the reported case), the Maven reactor tree, and a mixed root's two trees.

Root cause

vendor_maven_jvm decides "in sync" from plan.writes.is_empty(), which compares only the files the plan would write. It never listed the tree directories, so an extra file was invisible to vendor/repair. check_entry and the Gradle settings script both list them and reject extras.

Safety notes

The sweep runs outside the run's group commit: nothing names these entries, so there's nothing to journal. A captured removal is read back through a stray symlink at commit time, which would have failed the whole commit (found by /code-review high).
repair reads tree records from the committed ledger, which can be tampered with. It only uses records that record_allowed accepts for the entry's own coordinates, so a forged marker can't point the sweep at another artifact's directory (unit test).
A tree directory reached through an in-checkout symlink is never swept. Its strays are reported with an error naming the path, because they might be another artifact's files (Bugbot finding; `sweep_unindexed_refuses_a_linked_tree_dir`).
A file that the committed ledger still records is never treated as a stray. Files a patch update drops are removed only after the commit, through sweep_replaced_tree, as on main (jvm_rerun_keeps_tree_files_the_ledger_records, from a human review).
On Windows, a stray directory symlink or junction is removed with remove_dir, so its target is left alone (from the final review brief).
The Gradle settings script is unchanged. Changing it would make every existing vendored project fail vendor --check until re-vendored.

Tests (red → green)

Issue facet	Test	Without fix	With fix
#656 `vendor` / `--dry-run` say `already_vendored` over a stray (reactor + Gradle)	core `vendor::maven_repo::tests::jvm_rerun_sweeps_unindexed_tree_files`	FAIL: dry run reports `AlreadyPatched`	pass
#656 `vendor`, `vendor --check`, `repair`, `repair --dry-run`, stray symlink under the group commit (reactor + Gradle)	cli `vendor_jvm_cli::stray_tree_files_are_swept_by_vendor_and_repair`	FAIL: `"errorCode":"already_vendored"`	pass
#656 real Gradle build fails, then builds the patched jar after `vendor` and after `repair`	`e2e_vendor_gradle_build::gradle_vendor_656_stray_tree_file` (`--ignored`, Gradle 8.14.3 locally; runs in the `gradle_vendor_` CI legs)	(the build-fail precondition is the issue's repro)	pass
Sweep confinement: own tree only, symlink not followed, forged marker ignored	core `vendor::jvm::apply::tests::sweep_unindexed_removes_only_strays_in_the_own_tree`	n/a (new API)	pass
No sweep through a linked tree dir	core `vendor::jvm::apply::tests::sweep_unindexed_refuses_a_linked_tree_dir`	n/a (new API)	pass

Commands run locally:

cargo fmt --all -- --check: clean
cargo clippy --workspace --all-features -- -D warnings: clean
cargo test -p socket-patch-core --all-features --lib: 6925 pass, 4 fail. All 4 (copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_…, pypi_requirements::wire_failure_rolls_back_…) are permission-denial tests that can't fail a write when run as root in this sandbox. None touch JVM code.
cargo test -p socket-patch-cli --all-features --test vendor_jvm_cli: 53/53 pass
--test repair: 127 pass, 2 fail (repair_cleanup_failure_…, …_lock_file_unremovable), same root-only cause
The full cargo test --workspace ran out of sandbox disk while linking test binaries, so CI is the full-suite gate.
No npm/pypi/gem wrapper changes needed: this is Rust-only behavior.

/code-review high findings not changed

(The sweep-before-write_plan ordering concern is fixed: ledger-recorded files are excluded from the stray sweep.)
"repair skips the sweep when the artifact is Missing/Corrupt": that path is redownload::restore, whose swap_stage_into_place replaces the whole tree directory (and a mixed root's other tree), so strays don't survive it.
vex still attests not_affected while a stray file makes the build fail. That failure is fail-closed (no unpatched jar is built), and the stray is now cleared by the remedy the error names, so this PR leaves it.

https://claude.ai/code/session_011f9aePVZMMe2T6XR46RuNb

Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5

A file the index does not list in a vendored Gradle or Maven tree, such as a Finder .DS_Store, fails the Gradle build and vendor --check. The build error says to re-run `socket-patch vendor`, but vendor compared only the files it plans to write, reported already_vendored and left the stray file in place, and repair did nothing either. vendor now treats anything unindexed in its tree directories as drift: a dry run previews a re-vendor and a real run deletes the strays. repair deletes them offline for a healthy JVM entry and reports the package as rebuilt. Fixes #656 Assisted-by: Claude Code:claude-opus-5-5

Adds a real Gradle e2e for #656: a .DS_Store in the vendored tree fails the build, and both `vendor` and `repair` restore a build that consumes the patched jar. repair now only trusts tree records inside the entry's own version directory, so a forged ledger marker cannot point the sweep at another artifact's files. Assisted-by: Claude Code:claude-opus-5-5

Review fixes for the #656 sweep: - vendor deletes stray tree entries before writing any wiring, and deletes them directly instead of through the run's group commit, which would read a stray symlink back through its target and fail the whole commit. - A directory that cannot be deleted is now an error instead of a silent success; an entry already gone counts as removed. - vendor --check and the sweep share one lister, so they agree on what is unindexed. - repair --dry-run previews the files it would remove (wouldRemoveUnindexed), and a real repair lists them. Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-29T09:09:12Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

Mikola Lysenko (mikolalysenko) · 2026-10-29T09:19:48Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

If a vendored version directory, or a parent of it, is a symlink to another place in the checkout, its "stray" entries may be another artifact's files. vendor and repair now refuse to delete them and name the path, while vendor --check still reports them as unindexed. Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-29T09:40:28Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

vendor --dry-run and repair --dry-run now fail the same way as a real run when a vendored tree directory is reached through a symlink, instead of previewing a sweep that the real run then refuses. Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-29T10:01:42Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

Mikola Lysenko (mikolalysenko) · 2026-10-29T11:13:37Z

Burn-down agent: labeled Ready for review at 454fe4a6807c12612928aade6f5dcc58d9d5c392.

CI: 31/31 non-skipped checks green on this head (19 skipped), PR is mergeable.
Bugbot: reviewed this head, no new issues. Its two earlier findings (sweep following tree-dir symlinks; dry run ignoring the linked-tree refusal) were real, fixed in dae9b90 and 454fe4a, and their threads are resolved.
Reviewer focus: the stray-file sweep in jvm::apply::unindexed_in / refuse_linked and the repair path that reads tree records from the committed ledger.
Slack announcement could not be sent this run (the Slack send tool is not available); the next run will retry.

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-29T12:13:33Z

Final review brief

What it does. vendor used to check only the files it planned to write. A stray file in a vendored Maven/Gradle tree, such as a .DS_Store or a backup folder, was never treated as drift, but Gradle and vendor --check reject it. Now every planned version directory is listed, and entries the plan doesn't index count as drift. --dry-run previews the re-vendor, a real run deletes them before wiring, and repair cleans them on healthy JVM entries. All three refuse a tree reached through a symlink, dry runs included.

Risk: low-medium. It deletes files, but only inside .socket/vendor/{maven2,gradle,coursier}/…/<version> directories derived from indexed files. repair uses only record_allowed own-tree records. tree_files comes from the full write list, so legitimate files are always indexed. Removal never follows symlinks, and a stray link is removed, not its target.

Look here

apply.rs:832: unindexed_in, and the linked-tree refusal at :841
apply.rs:912: sweep_unindexed, the deletion
maven_repo.rs:1081: strays make the tree out of sync; swept at :1108 before write_plan
repair.rs:557:`` the repair branch

Verified.

Full diff reviewed by a subagent.
Ran cargo test -p socket-patch-core vendor::jvm (244 passed) and cargo test -p socket-patch-cli --test vendor_jvm_cli (53 passed). The new tests exercise vendor, dry-run, check and repair on both Maven and Gradle shapes, and fail without the sweep.
Ran cargo clippy -p socket-patch-core -p socket-patch-cli -- -D warnings: clean.
CI on 454fe4a: 31 passed, 19 skipped, 0 failed (ci-ok and clippy green). Bugbot passed. CHANGELOG.md untouched. The design doc matches the code.

Changes I made. None.

Open questions (non-blocking; follow-ups if you care):

Windows: a stray directory symlink or junction goes through remove_file at apply.rs:917. That fails on Windows, so vendor/repair errors out instead of removing it. It's a loud failure, not data loss.
Ordering: the sweep runs before write_plan. If the new plan drops a file the old ledger indexed and write_plan then fails, that file is already gone while the old ledger still lists it. This needs a changed plan plus a write failure.
Stale comment: vendor_jvm_cli.rs:382 still says the sweep is "inside vendor's group commit". A later commit moved it out.

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

Generated by Claude Code

On Windows a directory symlink or junction can't be removed with remove_file, so a stray one in a vendored tree made vendor and repair fail. It is now unlinked with remove_dir, which leaves its target alone. Also fixes a stale test comment about the group commit. Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-29T12:22:09Z

[agent] On the final review brief's open questions:

Windows directory symlink / junction: fixed in 86ee192. It's now unlinked with remove_dir, which doesn't touch the target, instead of remove_file. Type-checked against x86_64-pc-windows-gnu. The Windows test legs only run nightly.
Stale comment at vendor_jvm_cli.rs:382: fixed in 86ee192.
Sweep before write_plan with a changed plan: left as is. It needs a Gradle patch update that drops a tree file the old plan had, and a write_plan failure afterwards. The file lost is a vendored copy of the old patch, not user data. A re-run or repair rebuilds it from the patch service. Sweeping after write_plan would bring back the group-commit/symlink problem fixed in 5a0419d.

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-29T12:22:10Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

vendor removed stray files from the vendored tree before writing anything else, and judged them against the new plan only. A file the committed ledger still recorded, such as a classifier jar the build no longer declares, was deleted there. If the write was then refused or rolled back, the restored ledger and gradle-index.tsv named a file that was gone. Recorded files are no longer strays; a patch update's dropped files are still removed after the commit, as on main. Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-29T13:40:48Z

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

Mikola Lysenko (mikolalysenko) · 2026-10-29T14:58:55Z

Final review brief

What it does. A file a vendored JVM tree's index doesn't list (.DS_Store, Thumbs.db, an editor backup, a stray directory) breaks every Gradle build, and the error says to re-run socket-patch vendor, which until now did nothing. With this PR, vendor treats such files as drift and deletes them before it writes any wiring, and repair deletes them offline. Both use the same lister as vendor --check, so all three agree on what counts as a stray (#656).

Risk: medium. The change deletes files inside .socket/vendor/. The deletion is limited to directories the entry's own ledger records name and that pass record_allowed. It refuses any tree dir reached through a symlink, removes a stray symlink without touching its target, and never removes a file the committed ledger still records. The Gradle settings script is unchanged.

Look here

unindexed_in and sweep_unindexed: what counts as a stray, the linked-dir check, and the deletion (including Windows directory symlinks)
recorded_tree_files: only ledger records for the entry's own coordinates name a directory to sweep, so a forged marker can't redirect it
vendor_maven_jvm stray detection and the sweep before write_plan: runs after the dry-run return and outside the group commit
repair sweep:`` rebuilt with `unindexedRemoved`, or `verified` with `wouldRemoveUnindexed` on a dry run
check_entry now uses the shared lister. A missing tree dir no longer errors here, but a missing indexed file still shows up as plan drift a few lines earlier.

Verified. Read the full diff and the PR description, which matches the code. CI is green on 6292d2a (31 checks passed, 19 skipped, ci-ok and clippy green). Bugbot found no new issues on 6292d2a. All 3 review threads are resolved. CHANGELOG.md is untouched. Tests cover vendor, --dry-run, --check, repair and repair --dry-run on both the reactor and Gradle shapes. Sweep confinement has unit tests, and the real-Gradle e2e (gradle_vendor_656_stray_tree_file) runs in the gradle_vendor_ CI legs. Per the description, the red→green core test fails without the fix (AlreadyPatched).

Changes I made. None.

Open questions. None blocking. Design choice for the approver: a stray directory in the tree is deleted recursively, on the basis that only socket-patch's files belong there.

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

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-29T16:19:20Z

Burn-down agent: Ready for review confirmed at 6292d2af4cc84accf72a417dce5340fb5a54b1c9 (the label was first applied at 454fe4a).

CI: 31/31 non-skipped checks green on this head (19 skipped). PR is mergeable and only waiting on human approval.
Bugbot: reviewed 6292d2a and found no new issues. All 3 earlier review threads are resolved.
No pushes from this run. Auto-merge is still armed.

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-29T20:30:39Z

[agent] Dequeued by a CI infrastructure failure, not by this PR's code. In merge-group CI run 40947334054, dispatch-tests, coverage (1) and coverage (2) each failed at exactly 600 s on three different Depot runners (depot-lmhsfjk2pb, depot-bc143crx00, depot-hnr8783rw5) with no steps recorded. The runners were lost before checkout, so no test ran. ci-ok then failed only because those three were red. The other 22 jobs passed. This is the same pattern as #1660 (Depot runners lost mid-job evicting queue entries).

The PR's own CI is green on 6292d2a, it's approved, and GitHub reports it clear to merge. The dequeue turned auto-merge off, and this agent doesn't merge or re-queue PRs, so it needs a re-queue by a maintainer (re-enable auto-merge or add it to the queue). No code change is needed.

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-29T21:04:31Z

[agent] Dequeued again by the same Depot runner loss. In merge-group CI run 40950997135, 9 jobs failed with zero steps recorded after 600–1100 s, each on a different Depot runner: hosted-e2e, and the e2e legs for npm, pnpm, vlt, composer/uv, maven, the two gradle legs, and the poetry/pdm/hatch/pipenv VEX leg. Fail-fast then cancelled 4 more. No test body ran in any failed job, so nothing points at this PR's code. Pull-request CI runs in the same window passed, but they run fewer e2e legs on Depot than a merge-group run.

It needs another re-queue once the Depot runners recover (see #1660). No code change is needed.

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-29T21:07:59Z

CI janitor: both merge-queue evictions of this PR today (20:29 and 21:03 UTC) came from Depot runners losing contact mid-job ("self-hosted runner lost communication"), not from this PR's code. Evidence is on #1660. This PR was evicted twice in 24h, so I am not re-adding it. Re-queue it once Depot is stable again.

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-29T21:20:46Z

CI janitor: this PR was evicted from the merge queue twice today. Both times the cause was Depot runner loss, not this PR's code.

20:29 UTC: merge_group CI 40947334054. dispatch-tests and coverage (1)/(2) failed with "self-hosted runner lost communication". I re-queued it at 20:39.
21:03 UTC: merge_group CI 40950997135. About 10 e2e and hosted-e2e jobs failed the same way.

The PR is still approved, green on its head and clean. I'm not re-queuing it a third time while the Depot incident (#1660) continues. The final-review routine can re-add it once runners are healthy again.

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-29T21:40:31Z

Both merge-queue evictions today (merge_group runs 40947334054 and 40950997135) were caused by Depot runners losing connection, not by this PR: every failed job has the self-hosted runner lost communication annotation. Details are in #1857. This PR is safe to re-queue once runners are stable again.

Generated by Claude Code

Start fix for #656

369ae27

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 29, 2026

Vendored Gradle build fails on a stray file such as .DS_Store in the vendored tree, and the fix the error prescribes ("re-run socket-patch vendor") does nothing: vendor says already_vendored and repair is a no-op #656

Closed

Claude (claude) added 2 commits October 29, 2026 08:37

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 29, 2026

Swarm health manager log #1382

Open

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 29, 2026 09:09