Repository navigation
Fix gitignored ledger/manifest silently dropped (#1638) - #1640
Mikola Lysenko (mikolalysenko) wants to merge 7 commits into
Conversation
Assisted-by: Claude Code:claude-opus-5-5
A .gitignore rule such as state.json or *.json kept .socket/vendor/state.json out of the commit a vendored run asks for. The scan exited 0, but every other checkout then had the lock wiring and no ledger, so rollback failed with vendor_ledger_missing and list showed no patches. Vendored runs (vendor, scan/get --mode vendored, every backend) now ask git before writing anything and refuse with vendor_ledger_gitignored, naming the rule, dry run included. Refs #1638 Assisted-by: Claude Code:claude-opus-5-5
A .gitignore rule such as manifest.json kept .socket/manifest.json out of the commit while its blobs went in. In CI or a fresh clone, socket-patch apply then found no manifest, applied nothing and exited 0, so the package stayed unpatched. Agent-mode scan and get now warn with manifest_gitignored, naming the rule, whenever the run leaves patches recorded in an ignored manifest. It stays a warning: a pipeline that scans and applies in one job never commits .socket/. Refs #1638 Assisted-by: Claude Code:claude-opus-5-5
The standalone vendor eject restores every hosted pin upstream before the vendored engine runs. With a .gitignore rule matching .socket/vendor/state.json it now refuses with vendor_ledger_gitignored before fetching or restoring anything, so the project stays hosted. Refs #1638 Assisted-by: Claude Code:claude-opus-5-5
Review follow-ups for the #1638 checks: - Ask git with the index, so a ledger or manifest already tracked (force-added, or committed before the rule) is not reported as ignored: git keeps committing changes to it. - A .socket/ or .socket/vendor/ directory rule now refuses the run as vendor_artifact_gitignored when it includes a backend that does not check that rule per package (gem, PyPI, Cargo, Go, Composer), so those projects no longer lose the ledger and artifacts silently. - One git check-ignore call per run instead of two. - get <uuid> warns about an ignored manifest like the search path. - The manifest warning no longer suggests a !rule that cannot work under an ignored parent directory. - Move the helpers next to the ledger in vendor/state.rs. Refs #1638 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
uv 0.13.2 refuses the pylock.toml Pipenv writes (environments = []), so the Pipenv e2e legs fail on every PR. Same change as #1639; it no-ops once that lands on main. Assisted-by: Claude Code:claude-opus-5-5
|
[agent] Generated by Claude Code |
|
BugBot review Generated by Claude Code |
|
bugbot run Generated by Claude Code |
|
Labeled Ready for review (burn-down agent).
Generated by Claude Code |
artifacts_gitignore_checked was inserted between emit_eject_refusal and its doc comment, so the refusal helper lost its doc and the new helper's doc opened with an unrelated sentence. The PR description already claims this was fixed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Pu6ZWFiGyCEMEvfpnZrnVa
|
[final reviewer] I reviewed What I checked on
Non-blocking notes and questions for the approver
Generated by Claude Code |
|
bugbot run Generated by Claude Code |
There was a problem hiding this comment.
✅ 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 6094254. Configure here.
|
Labeled Ready for review again (burn-down agent) after the final reviewer's push.
Generated by Claude Code |
Final review briefWhat it does. Vendored runs ( Risk: low to medium. The checks fail open: no git, not a work tree, or a git error mean no refusal. They cost one Look here
Verified
Changes I made: none. Open questions (non-blocking, follow-up material)
Auto-merge (squash) is armed, so approving sends this straight to the merge queue. Generated by Claude Code |
LLM Description written by Claude Code:claude-opus-5-5
Fixes #1638
Root cause
The CLI tells users to commit two state files,
.socket/vendor/state.json(the vendor ledger) and.socket/manifest.json(the agent-mode manifest). It never asked git whether it would actually commit them. The only gitignore checks (npm_dir::gitignored) looked at the vendored artifact directories, and only in some backends. So a.gitignorerule likestate.json,*.jsonormanifest.jsonsilently dropped the file from the commit. The scan still exited 0, and every other checkout then broke:rollbackhitsvendor_ledger_missingandlistshows no patches.socket-patch applyreportsnoManifest, exits 0, and the package installs unpatched.Changes
vendor::ledger_gitignore_refusallives invendor/state.rs, next to the ledger. It makes onegit check-ignorecall over.socket/vendor/and the ledger and honours the index, so a ledger that is already tracked is never refused. It runs at the shared boundaryvendor_records_reusing, which coversvendorandscan/get --mode vendoredfor every backend. It also runs at the start of the hosted→vendored eject, before any pin is restored.vendor_ledger_gitignored, and the message names the rule..socket/or.socket/vendor/directory rule is left to the per-packagevendor_artifact_gitignoredrefusals when every ecosystem in the run checks that itself (npm family, NuGet, JVM), so that behaviour is unchanged. Otherwise (gem, PyPI, Cargo, Go, Composer) it is now a run-levelvendor_artifact_gitignored. Before this PR, those projects lost the ledger and the artifacts silently.manifest_gitignored, exit code unchanged). Both the search/scan path (download_and_apply_patches_into) andget <uuid>(save_and_apply_patch) warn whenever the run leaves patches recorded in a manifest git would not commit. This is a warning and not a refusal, because running scan and apply in the same CI job without ever committing.socket/is a legitimate workflow.CLI_CONTRACT.mddocuments both codes.Tests (red → green)
Each new test fails on
mainand passes here. The ones I checked by reverting just the fix are marked ✔.in_process_vendor::a_gitignored_ledger_refuses_before_any_write✔ coversstate.json,*.json+!package*.jsonand**/state.json, each with and without--dry-run: exit 1, error code, nothing written, lock unchanged.in_process_vendor::scan_vendor_gem_refuses_a_gitignored_ledger✔ runsscan --mode vendoredon gem (a non-npm backend) with astate.jsonrule, which givesvendor_ledger_gitignored, and with a.socket/rule, which givesvendor_artifact_gitignored.in_process_vendor::hosted_to_vendor_conversion::hosted_then_vendor_with_a_gitignored_ledger_keeps_the_hosted_wiring✔ covers the eject (no manifest) and the manifest-driven takeover. The hosted lock and.npmrcstay untouched.get::get_failure_paths::agent_record_warns_when_git_ignores_the_manifest✔ coversmanifest.json,*.json+!package.jsonand.socket/, which warn, and a re-included manifest, which doesn't.get::get_failure_paths::get_uuid_warns_when_git_ignores_the_manifest✔ covers theget <uuid>save path.a_re_included_ledger_vendors,a_tracked_ledger_is_not_refused,agent_record_does_not_warn_for_a_tracked_manifest. The existinghosted_then_vendor_under_a_gitignored_socket_dir_keeps_the_hosted_wiringandvlt::vendor_vlt_refuses_gitignored_payloadstill pass.Local:
cargo fmt --all -- --checkandcargo clippy --workspace --all-features -- -D warningsare clean. Tests:socket-patch-corelib has 6542 passing and 4 failing. All 4 are read-only-directory tests (copy_tree::relax_loop_must_not_traverse_symlinked_root,vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry,pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouched,pypi_requirements::wire_failure_rolls_back_already_written_files). They fail only because this container runs as root, which ignores chmod, and none of their files are in this diff.socket-patch-clilib,in_process_vendorandgetall pass (924, 139 and 170 tests). A fullcargo test --workspacerun ran out of disk while linking, so CI runs the full matrix. No wrapper changes are needed: thenpm/,pypi/andgem/wrappers only dispatch to the binary./code-review high
The review found 10 issues. I fixed 1–5, 7, 8 and 10 in 8caf8d6: tracked files were falsely reported as ignored (
--no-index), the directory-rule gap for backends that don't check it,get <uuid>not warning, remedy text that couldn't work under an ignored.socket/, four git spawns per check, a misplaced doc comment, and the helpers living in the npm module. Not fixed:npm_dir::gitignored's documented fail-open. The npm-family artifact check already warnsvendor_artifact_gitignore_uncheckedfor that same broken-git case, so a second warning would only duplicate it.git_project_ignoringtest helper is copied in two test binaries. The two binaries include different shared-helper modules (common/hermetic.rsvscommon/mod.rs), so sharing it would mean wiring a new module into one of them just for a 10-line helper.CI
1a25bb3 ports #1639's setup-uv 0.13.1 pin. uv 0.13.2 broke the Pipenv pylock e2e legs on every PR, and the pin no-ops once #1639 merges. Bugbot found no issues on 8caf8d6.
https://claude.ai/code/session_01QvLvEUfidP565pLJTJUJLK
Note
Medium Risk
Changes vendor exit behavior and adds git subprocess checks on common code paths; low security impact but can block vendoring in repos with broad
*.jsonorstate.jsonignore rules until fixed.Overview
Fixes #1638 by detecting
.gitignorerules that would keep critical Socket state out of git before users commit broken trees.Vendored workflows now fail early (
vendor_ledger_gitignored, exit 1, no writes, including--dry-run) when.socket/vendor/state.jsonwould not be committed. The check runs on normalvendor/scan --mode vendoredpaths and on hosted→vendored eject before any lock changes. For ecosystems that do not already enforce per-artifact ignores (e.g. gem), a run-level refusal also covers directory rules like.socket/that would drop the whole vendor tree.Agent mode (
get, batch download/apply) emits amanifest_gitignoredwarning (exit unchanged) when.socket/manifest.jsonwould be ignored—so CI that never commits.socket/still works, but teams that do commit get a clear signal. Already-tracked ledger/manifest files are not flagged.Git probing was extended so
check-ignorecan honor the index (tracked files) vs--no-indexfor new paths. CI pins uv 0.13.1 in Pipenv/pylock legs because 0.13.2 rejects Pipenv’s emptyenvironmentspylock.Reviewed by Cursor Bugbot for commit 6094254. Configure here.
Generated by Claude Code