Repository navigation
Fix stale docs and comments, and the npm wrapper's schema and exit code - #1022
Open
Mikola Lysenko (mikolalysenko) wants to merge 8 commits into
Open
Mikola Lysenko (mikolalysenko) wants to merge 8 commits into
Mikola Lysenko (mikolalysenko) wants to merge 8 commits into
Conversation
The rollback prompt's decline line is the shared "Cancelled; no changes made." (ui::CANCELLED), not "Rollback cancelled.". The JSON envelope shape is pinned by json_envelope.rs's unit tests and the commands' e2e tests; tests/cli_parse_*.rs pin the parsed clap arguments, not the envelope. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
v5 hosted mode keeps no ledger, and patch/redirect has no ledger, replay or takeover module any more. The hosted engine, the CLI hosted flow and the staged I/O layer still described a ledger load, a merge-then-persist, a ledger-record fallback for the gem stale-install probe and edit ordering for a whole-ledger replay. Say what the code does now: the vendored ledger is the only one the hosted flow loads, the gem probe judges only this run's fetched records, and staged.rs serves the hosted -> upstream restore. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- update/release.rs: the API client is no longer timeout-free; contrast self-update's whole-request deadlines with ApiTimeouts' connect and per-read bounds instead. - vex/discover/pypi_other.rs: is_socket_hosted_reference was deleted by #572; name hosted_pypi_reference, which shares the path parser and the origin allowlist. - scan/policy.rs: dir_markers falls back to manifests when a directory has no lock marker, which the in-memory engine does not do. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
spawnSync gives status null when the binary dies by a signal, and the wrapper turned that into exit 1, so Ctrl-C read as an ordinary failure. Return 128 + the signal number, as a shell does (SIGINT 130, SIGTERM 143). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
- The zod PatchManifestSchema lacked the legacy setup block the Rust manifest still parses and keeps, so validating a pre-v5 manifest stripped it. Add SetupConfigSchema (optional exclude/manual lists). - package.json had no files list, so the tarball shipped src/, the tsconfig and the compiled tests. Publish only the wrapper, bundled binaries and the compiled schema; a pack test guards the list. - The test script called pnpm in an npm-locked package and no workflow ran it. Use npm, and run it in the dispatch-tests job. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Filtered Rust tests for the touched modules pass on macOS:
|
The test helper and one test still described the merged ledger map the deleted hosted ledger used to feed. Production now hands over only this run's fetched records; the renamed test pins what it really checks: a record keyed under another purl spelling still matches by uuid. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The pack-contents test only allowlisted dist/schema files, so a files list that dropped them would still pass. When dist/schema is built, both manifest-schema.js and .d.ts must be in the pack list. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Mikola Lysenko (mikolalysenko)
marked this pull request as ready for review
October 7, 2026 15:37
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The architecture audit (report §6.3, §7 item 17, and B71) found docs and code comments that no longer match the code, plus three defects in the npm wrapper package:
Rollback cancelled.. The code prints the sharedui::CANCELLEDtext,Cancelled; no changes made.(rollback.rs:1430).tests/cli_parse_*.rsare "snapshot tests that lock the shape" of the JSON envelope. Those tests pin the parsed clap*Argsstructs. The envelope shape is pinned byjson_envelope.rs's unit tests and by each command's e2e tests.scan/hosted.rs, corehosted/engine.rsandpatch/redirect/staged.rsstill describe a redirect-ledger load, a merge-then-persist, a ledger-record fallback,replay.rs,super::ledger,super::takeover,super::replay, and edit ordering for a "whole-ledger replay". None of these exist in v5.vex/discover/pypi_other.rs:20namesvendor::pypi_pipenv::is_socket_hosted_referenceas an any-host twin. Recognize hosted Pipenv references through one shared grammar (#563) #572 deleted that function.update/release.rs:15-17says the API client has a "no-timeout posture". That has been false since Bound patch API connects and stalled reads (#570) #581 addedApiTimeouts.scan/policy.rs:70-72says "manifests are not markers".dir_markersfalls back to manifests whenever a directory has no lock marker.PatchManifestSchemalacks the legacysetupblock that the Rust manifest still parses and keeps, so validating a pre-v5 manifest strips it.testscript callspnpmin an npm-locked package.fileslist, the tarball shipssrc/, the tsconfig and the compiled tests.Change
staged.rsserves the hosted → upstream restore.ApiTimeoutsuses connect and per-read bounds.hosted_pypi_reference.dir_markersfalls back to manifests, and the in-memory engine does not.SetupConfigSchema(optionalexclude/manualstring lists, mirroringSetupConfigin coremanifest/schema.rs) toPatchManifestSchema.fileslist: the wrapper, any bundledbin/socket-patch-*binaries, and the compiled schema.js/.d.ts.testscript now uses npm, and thedispatch-testsCI job runs it.Every change is to docs, comments or the
npm/socket-patchpackage, plus one added step in the CIdispatch-testsjob. No Rust behavior changes. Duplicate copies deleted: none (this workstream consolidates no code).Testing
node --test npm/socket-patch/bin/socket-patch.test.mjs: 23 pass, 1 skipped (the Linux-only musl fallback). Four of these are new and failed first: three signal-exit tests, and thenpm pack --dry-runcontents test. Oncedist/schemais built, the contents test also requires bothdist/schema/manifest-schema.jsand.d.tsin the pack list. CI re-runs it with--test-name-pattern="npm package contents"right afternpm testbuilds dist/.npm testinnpm/socket-patch(tsc + zod tests): 10 pass. The three newsetuptests failed first, at compile time.cargo build -p socket-patch-cli: OK.cargo test -p socket-patch-core --libfiltered to hosted::engine, update::release, vex::discover::pypi_other and patch::redirect: 166 pass.cargo test -p socket-patch-cli --lib -- scan::hosted scan::policy: 61 pass, re-run after the review fixes. That includes the renamedgem_stale_probe_matches_records_by_uuid_not_purl_key.cargo clippy -p socket-patch-cli --all-targets -- -D warningson macOS:unix_defaultat corecrawlers/python_crawler.rs:2734, a cfg-dependent macOS-only warning in a file this PR does not touch.-A unused_variables), every remaining lint is incrates/socket-patch-cli/tests/**integration-test files this PR does not touch. There are none insrc/.--flag, subcommand and--mode/--vendor-sourcevalue in README.md and docs/*.md against the built binary's--help. I found no drift: every unmatched flag belongs to another tool (cargo, git, gh, gradle, dotnet).heavy-job.shlimiter with-j4andCARGO_INCREMENTAL=0.Deferred
dispatch-testsin a needs aggregate. This PR only adds steps inside thedispatch-testsjob, a different hunk, but whichever PR lands second needs a rebase check.fileslist (333acc4 failsnode --testby itself). It is left as is because the PR will be squash-merged.CHANGELOG.md:12-21is not touched, per AGENTS.md.RedirectStateused as an in-memory carrier for hosted records is a code change (proposed typedHostedRecords), not a comment fix.vex/discoverand the vendor backends describe the pre-v5 migration read, which still exists, so they are kept.--jsontop-levelerror(scan and get emit both a string and a {code, message} object) #704 exit-policy decision. This PR does not decide it.🤖 Generated with Claude Code
Note
Low Risk
Documentation and npm wrapper packaging/exit-code behavior only; Rust CLI logic is unchanged aside from comment edits.
Overview
Aligns CLI_CONTRACT and scattered Rust comments with v5 behavior (no hosted redirect ledger, shared cancel text, where JSON envelope tests live,
dir_markersmanifest fallback, self-update timeouts, PyPI hosted URL parsing). No Rust runtime changes.The
@socketsecurity/socket-patchnpm package gets substantive fixes: optional legacysetupblock on the Zod manifest schema (parity with Rust), shell-style exit codes on signal death, an explicitfileswhitelist so publishes ship only the wrapper and compiled schema,npm testinstead of pnpm, and new tests (signals,npm packcontents, setup round-trip). CIdispatch-testsnow runs schema build/tests and re-runs the pack-contents check afterdist/exists.Reviewed by Cursor Bugbot for commit 6af73d3. Configure here.