socket-patch architecture (living document): review, defect register and refactoring log #560
Replies: 31 comments
Architecture defect register[agent] This comment is the living register of architectural problems in the socket-patch CLI. It starts from the October 2026 review, which the top post and Parts 2–9 below keep as a living document, and grows with what the scheduled audit routines find. The status column here drives every status shown in the living document. It is regenerated from the Progress: 95 problems tracked · 5 fixed · 1 already fixed · 9 in PR · 19 filed · 3 decision pending · 57 to verify · 1 rejected. On GitHub: 31 open and 3 closed
Ecosystems and formats (
|
| ID | P | Problem | Source | Issues | Status |
|---|---|---|---|---|---|
| E01 | 1 | Hosted NuGet source mapping (nuget_package_source_keys) regex-scans raw XML, so a commented-out <add key> changes which sources get mapped. The vendored reader and formats::nuget both mask comments. |
§1 #5; 5.4 | #561 | in PR #597 |
| E02 | 1 | bun.lockb vendoring hard-codes registry.npmjs.org and ignores SOCKET_NPM_REGISTRY. The npm tarball URL is re-implemented in lock_inventory/vlt.rs, and there are two NPM_REGISTRY constants. |
§1 #7; 4.4 | #562 | in PR #574; partly not a defect |
| E03 | 1 | vlt registry_base has two implementations (upstream/vlt.rs, lock_inventory/vlt.rs) with different fallback orders and different unknown-alias behavior. |
§1 #7; 4.4 | #562 | in PR #574 |
| E04 | 1 | Pipenv hosted-URL recognition accepts any host (pypi_pipenv.rs), but redirect hosted_patch_uuid uses an origin allowlist. |
5.4 | #563 | fixed (#572) |
| E05 | 1 | Cache crawls aren't project-scoped: cargo, go, maven, nuget and deno enumerate the whole machine cache, and scan sends all of it to the API (#265). | 6.6 | #595 | filed #595; tracking, children #427, #265 |
| E06 | 1 | Some crawler reads aren't FIFO-safe: nuget_crawler.rs (obj/project.assets.json) and the cargo vendor/<crate>/Cargo.toml reads use a plain read_to_string on project-tree files. The Python .venv read is guarded. |
6.6 | #592 | in PR #602 |
| E07 | 2 | package-lock has four entry walks (inventory, vendored, hosted, restore) with copied identity/skip rules and two pointer escapes. Target: one addressed walk. The serializer half was fixed by #357 (all writers use JsonLayout). |
4.4; 4.7 E | #663 | filed #663 |
| E08 | 2 | yarn has five split("\n\n") + regex grammars beside scan_blocks. Target: hosted yarn writers and restorers built on LockBlock. |
3.7 #3; 4.7 D | to verify | |
| E09 | 2 | Yarn berry gates are written twice: the cacheKey constant, cacheKey extraction, and the mixed-EOL and compressionLevel refusals, which use different codes. #370 needs fixing twice. |
4.4 | #629, #628 | in PR #657; compressionLevel reader already shared (#508) |
| E10 | 2 | XML has eight hand-rolled scanners and four attribute extractors with three tokenization rules. The writers (nuget_feed.rs, maven_repo.rs) never use the shared readers, so reader and writer can disagree. |
5.4 | to verify | |
| E11 | 2 | NuGet config has three readers. Target: formats::nuget::parse_config everywhere. Fixing E01 is the first step. |
3.7 #3; 5.4 | #594 | filed #594; hosted part in PR #597 |
| E12 | 2 | pnpm v9 and legacy 5.4/6.0 are near-copies (revert_*_opts, vendor_pnpm*, read_project, edit_overrides, dep_field_lines, the KIND constant), and v9 has two lookup paths (a linear scan and LockIndex). |
4.4; 4.5 #4; 4.7 B/G | #583 | fixed (#583) |
| E13 | 2 | utils/poetry_lock.rs ≈ utils/pdm_lock.rs: the *_lock_edits functions are identical, and the pair_* functions differ by one shape check (unreachable for Poetry; see #694). |
5.4 | #694 | filed #694 |
| E14 | 2 | Pipfile.lock is written two ways: vendored mode re-serializes it, while hosted mode splices spans. | 5.4 | to verify; present on 045d7ec, no drift proven (pipenv writes canonical JSON), not filed yet | |
| E15 | 2 | Cargo.toml [package] is read five ways: line scanners in cargo_crawler.rs and vex/product.rs, plus three ad hoc toml_edit lookups (cargo_tag, which also accepts [project]; path_crate_version; declared_cargo_minor). They have drifted on BOM, [project] and dotted keys. plan_cargo_toml uses a regex scanner and toml_edit in one rewriter. |
5.4; 3.7 #3 | #693 | filed #693; [package] readers. The hosted plan_cargo_toml regex scanner is not filed yet |
| E16 | 2 | CRLF has five policies in the npm family and three for toml_edit output, and common::detect_eol contradicts LineEndings::Mixed. Target: one line-ending policy. |
4.4; 5.4; 7.3 | to verify | |
| E17 | 2 | "Is a bun lock present" has four predicates with different symlink semantics, so a dangling bun.lock symlink is present to one of them and absent to the others. |
4.4 | to verify | |
| E18 | 3 | JS helper copies: JSON-pointer escape ×2, wiring lines↔JSON ×3, name@spec split ×2, KIND_* re-spelled as literals, uneven recursion bounds, and regexes compiled inside per-dependency loops. |
4.4 | to verify | |
| E19 | 3 | Gem has three section models and two DEPENDENCIES-name parsers with different rules. Go's go_mod_edit.rs lives in vendor/, and go_crawler.rs has its own parse_go_mod_module. |
5.4 | to verify | |
| E20 | 3 | Pure codecs (bun_lockb.rs, bun_lock_text.rs, vlt_lock_text.rs) and the neutral types (Edit, Warning, LockfileEntry) live outside formats/, which creates formats↔vendor/redirect/vex cycles. |
2.1; 4.5 #2; 4.7 J | to verify | |
| E21 | 2 | Tracking: VendorBackend trait + registry. The ecosystem list is enumerated at 16 production sites, and the vend! / vend_installed! macros stand in for the trait. |
2.1; 5.2; 5.8 | to verify | |
| E22 | 2 | The JS vendor driver skeleton is copied eight times (guard_coordinates → … → a literal VendorEntry). Target: one generic driver + NpmLockBackend. |
4.4; 4.7 C | to verify | |
| E23 | 2 | The pypi_{poetry,pdm,pipenv}.rs backends repeat one skeleton: load_*_project, classify_dependency, check_target_guards, wire_*, revert_*. |
5.4 | to verify | |
| E24 | 2 | There are nine revert mechanisms (~3.5K lines). Target: one splice-record revert engine, with legacy ledger kinds adapted at load. | 2.1; 5.3; 5.8 | to verify | |
| E25 | 3 | Per-backend copies: cleanup_failed_stage, <eco>_service_copy (cargo, composer, gem, golang), and the service_preflight_names_exactly_* test copied seven times. |
5.4; 5.8 | to verify | |
| E26 | 3 | JVM has two Maven backends. Target: merge maven_repo.rs into jvm/ as Shape::Single. Its three artifact roots don't follow <eco>/<uuid>. |
5.7 | to verify | |
| E27 | 3 | The per-package call model needs ~3K lines of compensating machinery (group_commit, durability, prestage, vendor_prefetch, 22 ParseMemo statics, ledger_snapshots). Target: batched pure planners, after E21 and E24. |
2.4; 5.7 | to verify | |
| E28 | 2 | Dead vendored scaffolding: VendorSource / PackageSource have one variant each, the SERVICE_ECOSYSTEMS refusal can never fire, ServicePolicy::new ignores its config, vend_installed! has no target, and several docs are stale. |
5.6; R11 | to verify | |
| E29 | 3 | registry_fetch.rs (1.5K lines) is really archive extraction, integrity checks and the hosted-restore HTTP client, so it is misnamed and in the wrong place. |
5.6 | to verify | |
| E30 | 2 | Split redirect/mod.rs (17.5K lines) mechanically: model, driver, one file per ecosystem, hosted_url, and sibling test files. |
3.7 #1 | to verify | |
| E31 | 2 | Tracking: trait HostedRewriter + Outcome { per_dep }. It replaces the 20 uuid sets in RewriteResult, merge_group_delta, the 16-rule confirm() and eight parallel tables. |
2.1; 3.7 #2 | to verify | |
| E32 | 2 | There are two hosted orchestrators, disk (run_redirect_selected) and in-memory (hosted/memory), kept equal by parity tests. Target: one pipeline. Depends on E44. |
3.3; 3.7 #4 | to verify | |
| E33 | 3 | Upstream restore rebuilds originals from the network (~7.4K lines, ignores mirrors, 13 open bugs). Target: an originals sidecar or a narrowed restore. Depends on E45. | 2.3; 3.5 | to verify | |
| E34 | 3 | Per-PM auto-config costs more than it's worth: npm allow-remote (~900 lines re-implementing npm config), pnpm trustLockfile, the vlt warm-tree heal, and the unbenchmarked parallel rewriter groups. |
3.6; 3.7 #7 | to verify | |
| E35 | 3 | Retire the refactor oracles: the redirect equivalence suites and 252 KB of goldens, the crawler oracles (3K lines), and the telescoping entry points (use one RewriteOptions). |
3.7 #9; 6.6 | to verify | |
| E36 | 2 | Tracking: one Inventory. Today's four discovery systems are merged by fabricating CrawledPackages with fake node_modules/<name> paths. Rename vex::discover (it is the hosted-state store). |
2.1; 6.2; 6.7 | to verify | |
| E37 | 2 | is_safe_{cargo,gem,nuget}_coordinate are byte-identical and duplicate simple_purl's check. composer_crawler::normalize_version duplicates strip_leading_v. |
6.4 | #630 | filed #630 |
| E38 | 2 | The product-manifest probe table is copied three times and has drifted: vex.rs lacks the csproj and gemspec probes. The probes don't reuse the format parsers. |
6.4; 6.5 | to verify | |
| E39 | 3 | canonicalize_pypi_name lives in crawlers/ (29 importers), and Ecosystem lives in crawlers/types.rs while LockfileEntry.ecosystem is a string. Target: core/src/ecosystem.rs. |
2.1; 6.4 | to verify | |
| E40 | 2 | vex_consumed.rs, in the CLI, is a third copy of package-manager layout knowledge. Target: move it into per-ecosystem locators in core. |
6.5 | to verify | |
| E41 | 2 | Dead discovery code: lock_inventory/wired.rs has no production caller, vex/discover/deno.rs is an empty extractor, and the pre-v5 redirect-ledger readers (~430 lines) remain. |
6.4; 6.5; R11 | to verify | |
| E42 | 2 | The embedded --vex glue is copied per command (scan, apply, and vendor ×3), each with caller-injected bypass sets. Target: one EmbeddedVex helper. |
6.5; R13 | to verify | |
| E43 | 2 | Fail closed on unmodeled resolution config in one shared place: go.work, gradle.lockfile, BUNDLE_GEMFILE, virtualStoreDir, install-strategy=linked, globalPackagesFolder, mirrors. |
2.2 #3 | to verify | |
| E44 | 2 | Decide: the napi addon and in-memory engine. Will depscan adopt it (then delete the TS rewriters), or should it be deleted (−4.8K prod)? | §6 Q1; 3.7 #6 | to verify | |
| E45 | 2 | Decide: hosted rollback. Is an originals sidecar acceptable, or should restore be narrowed to formats whose original is a pure function of registry data? | §6 Q5; 2.3 | to verify | |
| E46 | 2 | Decide: VEX evidence. Should not_affected require consumed evidence by default, so that wired-only evidence (lockfile_basis_ok) needs an opt-in? This is behind 29 open issues. |
§6 Q4; 2.2; 6.5 | to verify | |
| E47 | 3 | Decide: support tiers for bun.lockb writes, vendored pnpm 7/8, vlt pre-1.0 encodings and hosted pnpm ≤ 6, and whether hosted JVM ships as beta. |
§5; §6 Q3; 4.6 | to verify | |
| E48 | 3 | Discovery re-implements package-manager layouts (venv-name hashing, global prefixes, the pnpm store). Target: ask the package manager (poetry env info -p, pipenv --venv, npm query, …). |
2.2 #2; 6.6 | to verify | |
| E49 | 2 | Hosted Pipenv owned_url rejects path-prefixed --patch-server-url origins and sdists, so it refuses to rotate its own pin; there are four hosted-PyPI-URL grammars (hosted_patch_uuid, hosted_artifact_url, owned_url, is_socket_hosted_reference). |
new finding | #563 | fixed (#572) |
| E50 | 2 | Hosted rewrite_nuget and upstream restore rewrite packages.lock.json entries of the patched id at other versions (every framework); vendored locked_at filters by version. Four lock walkers. |
new finding | #593 | filed #593 |
| E51 | 2 | Yarn berry: vendored refuses a mixed-EOL root package.json (vendor_yarn_berry_mixed_line_endings); hosted's gate checks only yarn.lock and silently majority-normalizes the manifest via JsonLayout. |
new finding | #628 | in PR #657 |
| E52 | 3 | vendor/go_sum_edit.rs: free upsert_module_lines / has_module_version / remove_exact_module_version_lines have no production caller and are re-implemented by GoSumEditor (kept only as a test oracle); the pure hosted codec lives in vendor/. |
new finding | #631 | filed #631 |
| E53 | 2 | Vendored pnpm writes the root package.json with serialize_json (LF, no BOM) instead of JsonLayout: a CRLF file stays LF after vendor --revert; a BOM file is refused as "not a JSON object". npm and berry keep the layout. |
new finding | #662 | filed #662 |
| E54 | 3 | Poetry and PDM lock rewriters restore line endings with two rules (Poetry: any CRLF → all CRLF; PDM: preserve_line_endings, CRLF-only), so a mixed-EOL lock's edited unit flips to CRLF in one and LF in the other. |
new finding | #695 | filed #695 |
Handed off: none yet.
Rejected / not a defect: E02, in part. The bun.lockb format-1 URL synthesized at bun_lockb.rs:235 is lock semantics (format 1 stores no URL, so Bun resolves against its configured registry). It is never fetched, and SOCKET_NPM_REGISTRY is an undocumented knob of the restore client. The duplicated tarball and NPM_REGISTRY spellings are folded into #562.
Already fixed: none yet (E12 was fixed by #583 after it was filed, so it stays in the table).
CLI layer, core infrastructure, agent mode, tests and docs (audit-core)
Last updated 2026-10-03T15:50Z · main @ 045d7ec
| ID | P | Problem | Source | Issues | Status |
|---|---|---|---|---|---|
| C01 | 1 | Unbounded zip inflate on tamperable input. zip_bytes_match_after_hashes pre-allocates from the archive's declared size and reads with no cap, and it runs on committed .nupkg/.jar files and service archives. There are three archive caps (512/256/128 MiB). |
§1 #1 | #569 | fixed (#587); streams members, no cap per maintainer |
| C02 | 1 | ApiClient::new and plain_client() set no HTTP timeout, and blob and diff fetches have no retry, so scan, get and apply can hang in CI. |
§1 #2 | #570 | fixed (#581) |
| C03 | 1 | vendored_takeover ignores RevertOutcome.kept_artifact. It deletes the ledger entry and reports the artifact as reverted on a drift-keep, while every other revert caller honors the flag. |
§1 #3; 2.4 | #568 | filed #568 |
| C04 | 1 | Planted-binary spawn: vendor/pypi_hatch.rs runs Command::new("hatch").current_dir(root) instead of process::resolve_tool. #442 didn't cover it. |
§1 #4; 7.3 | #613 | in PR #617 |
| C05 | 1 | SOCKET_FORCE is bound to vendor --force, apply --force and --update --force, so forcing a self-update also forces past hash checks. |
§1 #6 | #615 | decision #615 |
| C06 | 1 | get round-trips its arguments through DownloadParams and ..GlobalArgs::default(), which silently resets offline, patch_server_url and more. get also builds a fake ApplyArgs, and get and scan call each other. |
2.1; 2.3; R7 | rejected; resets inert on 045d7ec, cycle folded into C12 | |
| C07 | 1 | The URL builders disagree. When org auto-resolve fails, patches_path sends JSON calls to /v0/orgs/default/…, while binary_url and vendor_package_url send the same client to the public proxy. Telemetry has a fourth copy of this logic. |
7.2 | #648 | decision #648 |
| C08 | 2 | Repo hygiene: a stray .github/actions/actions/cache/<sha>/.vscode/launch.json, a README that documents v5 but whose installer installs v4, and 39 references to a "DESIGN §" document that doesn't exist. (The dead CI path filters go to the CI janitor.) |
§1 #8; 8.5 J | #649 | filed #649; README part already fixed |
| C09 | 2 | There is no shared with_proxy_fallback helper: scan, get (both paths) and vex each handle the proxy fallback themselves, and get's handling has a gap. |
2.10 R2 | #647 | filed #647 |
| C10 | 2 | Tracking: RunCtx { config, client, telemetry, lock }, built once in main. It would delete apply_env_toggles (flags written back into process env, which has a documented token-leak history) and unblock removing 553 #[serial]. |
2.5; R3 | to verify | |
| C11 | 2 | Tracking: split run_scan (1,499 lines; mode booleans referenced 91 times) into discover → select → ModeBackend::consume → render. |
2.2; R5 | to verify | |
| C12 | 2 | Tracking: move engine code out of the CLI and into core behind one orchestrator over ProjectView. That covers vendor_records_reusing (962 lines), run_redirect_selected (836) and ecosystem_dispatch.rs (816). Coordinate with E32. |
2.1; R11 | to verify | |
| C13 | 2 | Error codes are untyped. Target: a typed registry (enum Reason × Ecosystem) that generates the contract's code tables, plus a freshness test. Today ~65 codes are undocumented and 1 is phantom. |
2.8; 3.7 #8; 8.5 F | to verify | |
| C14 | 2 | Decide: one JSON envelope. scan, get and rollback still emit a bare-string error, while the other commands emit {code, message}. |
2.8; R4 | #704 | decision #704 |
| C15 | 2 | There are three HTTP retry systems, and blob and diff fetches have none. A 206-line HTTP-date parser, two near-identical downloaders, and per-fetch or per-event clients round it out. Target: one retry + timeout primitive. | 7.2; R12 | #676, #677 | filed #676, #677 |
| C16 | 2 | Batch limits are split across crates. The CLI owns 500 / 100 / 256 KiB, and search_patches_batch documents a maximum of 500 without enforcing it. The in-memory engine keeps a third copy (default 100, no body cap). |
7.2 | #675 | filed #675 |
| C17 | 2 | Digest helpers are duplicated: ~30 inline hex::encode(Sha256::digest(..)) sites, and sha256_hex copies that compute beside a utils::digest::sha256_hex that validates. sha1_hex exists twice, and SRI formatting is inlined three times. |
4.4; 7.3 | #706 | filed #706 |
| C18 | 2 | There are four UUID grammars. client.rs has one, CLI lib.rs a byte-identical copy, path_safety.rs accepts lowercase only, and apply.rs accepts any alphanumeric plus -/_. |
7.3 | #705 | filed #705; a fifth grammar (Uuid::parse_str in python_script.rs) |
| C19 | 2 | Env truthiness has three vocabularies, and there are 37 inline "empty means unset" reads and four home-directory resolvers. | 7.3 | to verify | |
| C20 | 2 | Purls have two builder families in utils/purl.rs, plus 78 hand-built pkg: strings and 58 starts_with("pkg:<type>/") checks that bypass Ecosystem::from_purl. |
6.4; 7.3 | to verify | |
| C21 | 3 | utils/fs.rs has six atomic writers, and separate stage + rename code lives in blob_fetcher and update/. Writes that bypass utils::fs (blob_fetcher.rs) escape group commit. |
5.7; 7.3 | to verify | |
| C22 | 2 | Telemetry has 17 near-identical track_* wrappers, builds a new HTTP client for every event, and threads the token and org through 125 signatures. Target: one track(Event) with a shared client. |
7.5; R15 | to verify | |
| C23 | 2 | Dead code: PatchSources::mem_blobs is never Some. save_redirect_state and its group-commit entry journal a file that nothing writes. The switched_off("group_commit") oracle path, the Pypi/LauncherCache update channels and --vendor-source (one value) also remain. |
7.4; 7.6 #3; 5.6 | to verify | |
| C24 | 2 | Apply and rollback are mirror images: the verify types are identical, and fold_copy_result, the pnpm peer fan-out and the sidecar boundary are each written twice. Target: one engine. |
7.4; 7.6 #4 | to verify | |
| C25 | 2 | --download-mode diff, the default, re-downloads every blob on a cold cache, runs sequentially with no retry, and is the only user of qbsdiff. Making file the default is a decision; removing the duplicate fetch work is a refactor. |
7.4; R10 | to verify | |
| C26 | 3 | apply.lock spends ~554 lines deleting the lock file on exit, and taking the lock replays the vendored group-commit journal, coupling vendored crash recovery to every command. |
7.4 | to verify | |
| C27 | 3 | Agent-mode sidecars don't handle Maven files at all. Verify whether in-place Maven patches leave stale checksum files behind. | 7.4 | to verify | |
| C28 | 3 | socket.yml builds a hand-made YAML tree on serde-saphyr's event parser to read 8 keys. Target: serde with deny_unknown_fields. |
7.5 | to verify | |
| C29 | 3 | The client.rs split (2.8K lines) into client, vendor_service and credentials; the debug-ordering machinery (HeldBack) has 45 call sites. |
7.2; 7.6 #8 | to verify | |
| C30 | 2 | No socket-patch-test-support crate: binary() is defined in 102 files and git_sha256 in 84, there are 14 divergent scrub_socket_env, xorshift is implemented four times, and the VEX helpers are forked. |
6.4; 8.5 D | to verify | |
| C31 | 3 | There are 207 test executables; the target is ~25. This needs C10 first. | 8.5 A | to verify | |
| C32 | 3 | 328 exact-sentence assertions should become --json/errorCode checks plus snapshots. Triage the 402 covgap tests, 136 of which assert human text. |
2.5; 8.5 G/H | to verify | |
| C33 | 3 | CLI_CONTRACT.md (332 KB) should be a generated reference (flags, env vars, codes, exit codes) plus ≤300 lines of prose, with a freshness test. Also decouple docs/testing from the validation scripts. |
8.3; 8.5 F/I | to verify | |
| C34 | 3 | Decide: the command model. A read-only scan, plus fix, undo, sync and check, with mode inferred from project state. This folds remove, rollback and vendor --revert, and per-command flags replace the 27 globals. |
§4; 2.9; R6/R8 | to verify | |
| C35 | 3 | Decide: drop the deprecated spellings and embedded --vex, and give SOCKET_FORCE per-command names. |
R9; R10 | to verify; SOCKET_FORCE part is #615 |
|
| C36 | 3 | Decide: the futures of agent mode and of the self-update binary swap. | §6 Q2; 7.5 | to verify | |
| C37 | 2 | Patch blob/diff downloads (fetch_binary) buffer the whole body with no size cap; vendor and self-update use the shared read_capped. |
new finding | #571 | in PR #607 |
| C38 | 2 | The public-proxy per-package fallback keeps a private cap of 10, ignoring SOCKET_API_CONCURRENCY, the proxy cap of 4 and the fd-limit rule; registry_concurrency() has no caller. |
new finding | #614 | filed #614 |
| C39 | 2 | The 401/403 proxy fallback is missing beyond get search: apply, rollback and repair blob/diff downloads and vendor eject view fetches fail on a stale token, although the contract promises eject get's fallback. Fix: the fallback moves into ApiClient. |
new finding | #647 | filed #647 |
| C40 | 3 | SOCKET_API_CONCURRENCY and SOCKET_WALK_THREADS are read by core but documented nowhere; only clap-bound env vars have a guard test. |
new finding | #678 | filed #678 |
| C41 | 2 | Hash case policy is per site: blob download compares case-insensitively and the validators accept uppercase, but agent-mode apply/rollback verify with exact ==, so an uppercase manifest hash never verifies. Vendored verify sites are split the same way. |
new finding | #707 | filed #707 |
Handed off (to the CI janitor): report-only coverage and LTO docker-base off PRs; e2e from 148 to ~50 legs; a reusable compat workflow; no per-leg compiles; dead CI path filters (review 8.2, 8.5 B/C/E).
Rejected / not a defect: C06. On 045d7ec, the nested apply reads none of the fields ..GlobalArgs::default() resets except offline, which get/scan refuse up front; the hosted opt-outs reach core through process env. The get ↔ scan cycle and the fake ApplyArgs stay in C12.
Already fixed: none yet.
Refactor routine (refactor, hourly, highest leverage first)
Last updated 2026-10-03T16:05Z · main @ 045d7ec
In flight:
- #574: one vlt
registry_basefollowing vlt's DepID hydration. Issues vlt lock inventory and hosted restore resolve a node's registry differently #562 (E02, E03). State: ready (Ready for review). It awaits human approval. - #602: crawler project-tree reads go through
utils::fs::read_regular_*, plus acrawlers::architecture_testsguard against bare reads. Issue NuGet and Cargo crawlers hang on a FIFO at obj/project.assets.json or vendor/<crate>/Cargo.toml #592 (E06). State: ready, handed to the PR burn-down. - #607: blob and diff downloads stream to disk through
BinaryBody; onedownload_entriesloop replaces the blob and diff copies. Issue Patch blob and diff downloads buffer the whole response body with no size cap #571 (C37). State: ready, handed to the PR burn-down (CI green, 338 checks; Bugbot clean on ae928ae).
Merged:
- #572: one hosted-PyPI-URL recognizer for hosted and vendored Pipenv. Issues Pipenv recognizes hosted PyPI patch URLs with two private grammars that disagree with the shared one #563 (E04, E49). Production +31 / −48, tests +174 / −37 (approx.).
- #581: one
ApiTimeoutspolicy (10 s connect, 60 s idle read) on bothApiClientreqwest clients. Issue The patch API client has no request timeout, so scan, get and apply hang forever on a stalled server #570 (C02).
Queue (B bugs closed, U unblocks, D duplication removed, R risk; score = 3B + 2U + D − risk):
| # | Candidate | B | U | D | R | Score | Note |
|---|---|---|---|---|---|---|---|
| 1 | #706 (C17): one utils::digest compute API (sha256_hex, sha1_hex, sha512_sri); delete the jvm, ledger_snapshots, group_commit, maven_repo, npm_pack and bun_lock copies |
0 | 1 | 5 | L | 7 | skipped: jvm/mod.rs, maven_repo.rs, group_commit.rs changed by open PRs #690 and #646, bun_lock.rs by #689; unblocks #707 being fixed in one place |
| 2 | #693 (E15): one toml_edit Cargo.toml package reader for crawler, VEX, cargo_tag and path_crate_version; delete the crawler's line scanner |
1 | 0 | 3 | L | 6 | skipped: cargo_crawler.rs changed by open PR #602, vendor/cargo.rs by #598 |
| 3 | #707 (C41): one case-insensitive hash comparison for agent-mode apply/rollback and vendored verify | 1 | 0 | 2 | L | 5 | skipped: patch/apply.rs/patch/rollback.rs changed by open PRs #634, #646 and #690; simpler after #706 |
| 4 | #663 (E07): one addressed package-lock entry walk for inventory, vendored, hosted and restore | 0 | 1 | 4.5 | M | 4.5 | skipped: npm_lock.rs/lock_inventory/npm.rs changed by open PRs #589, #660 and #689, redirect/mod.rs/upstream/npm.rs by #657 and #597 |
| 5 | #568 (C03): takeover honors kept_artifact via vendored_backend's revert step |
1 | 0 | 1 | L | 4 | skipped: scan/hosted.rs changed by open PRs #598, #646, #657, #684 and #690 |
At capacity (3 open, all ready, all reviewed "ready to merge") on 2026-10-03T01:05Z; re-ranked with the new issues #628–#631, no new work started (blockers #597, #598, #602, #607, #610 still open). Next: #631 (E52, go.sum oracle delete + move to formats/golang), score 3.5 (D3.5 R L), skipped while #597 changes redirect/mod.rs. Dropped C07 from the queue: needs an owner decision on the fallback route. Taken: #571 (C37) in #607, score 2 (B1 D1 R M). Re-ranked 2026-10-03T04:00Z with #647 (C09/C39) and #649 (C08, hygiene, score ≈1); still at capacity, no new work. #614 (C38, score 4) drops to sixth. #648 is a decision. Re-ranked hourly 2026-10-03T05:58Z–11:58Z: no change except #662 (E53) and #663 (E07) entering; #628/#629 left the queue (fixer PR #657 references them); #675 (C16), #676/#677 (C15) and #678 (C40) score ≤2.5 and stay outside the top five (api/client.rs held by #607 and #610). Bughunt #685 may share #593's XML-walker root cause. Re-ranked 2026-10-03T13:05Z with #693 (E15), #694 (E13) and #695 (E54, Poetry/PDM line-ending drift; B1 D0.5 R L, score 3.5, simpler after #694): still at capacity, nothing merged, no steering. #694 narrows E13 to the shared engine (format walkers stay per format), so it replaces the earlier set-aside. #630 and #662 drop to sixth and seventh. Re-ranked 2026-10-03T13:56Z: no new arch-audit/refactor issues (#696 and #697 are bughunt bugs outside the queue), nothing merged, no steering, blockers #597, #598 and #602 still open; queue unchanged. Re-ranked 2026-10-03T15:00Z: still at capacity (#574, #602, #607 ready, awaiting human merge), no new arch-audit/refactor issues (#699, #701 are bughunt bugs), nothing merged, no steering; queue unchanged. #694 stays next when capacity frees. Re-ranked 2026-10-03T16:05Z: still at capacity (#574, #602, #607 ready, awaiting human merge), nothing merged, no steering. #694 and #695 left the queue: fixer PR #703 claims both. New #706 (C17) ranks first, #707 (C41) third; #705 (C18, B0 U0 D4 R M, score 2) sits outside the top five (api/client.rs held by #607/#610, apply.rs by #634/#646/#690). #704 is a decision. #593 (score 4) drops to sixth.
Notes:
- The sandbox runs as root, so 4 core lib tests fail on main and on branches alike:
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. redirect/pipenv.rs,vendor/pypi.rsandvendor/lock_inventory/vlt.rsaren't rustfmt-clean on main: format only your own hunks there. Checkrustfmt --checkon themaincopy before formatting a whole file.lock_inventory/mod.rsarchitecture_testsforbidhosted_patch_uuid*in a format file's model section. Origin-policy helpers go after the// ── registry view ──marker.redirect/mod.rsis a hot file (4 open PRs). Prefer candidates outside it until those land.CLI_CONTRACT.mdlives atcrates/socket-patch-cli/CLI_CONTRACT.md.- vlt registry semantics:
hydrateandSpecmemoize by id/spec and ignore options, so run each vlt case in a freshnodeprocess.scoped-registries[scope]wins for every segment;~~splits tonpm. Cite@vltpkg/dep-idhydrateTupleand@vltpkg/spec(registry ?? registries[default-registry-alias]); the packages download from npm. - Runs overlap: two runs started within minutes of each other on 2026-10-02. Claims and the status block kept them apart;
git pull --rebasethe ledger before writing. - Maintainer steering (2026-10-02, on zip_bytes_match_after_hashes inflates committed .nupkg/.jar entries with no size cap #569 and Patch blob and diff downloads buffer the whole response body with no size cap #571): don't add size caps on trusted upstream data; stream instead of buffering.
- reqwest 0.12
ClientBuilder::read_timeoutis an idle bound (resets per chunk) and also bounds the wait for response headers;RequestBuilder::timeoutis total. rustfmt <file>also formats that file's out-of-line child modules (formattingcrawlers/mod.rsrewrotepython_crawler.rs). Checkgit diff --statafter formatting and restore any file you didn't mean to touch.cargo clippy --all-targets(and-p socket-patch-core --tests) already fails onmainfrom older lints in test code. CI's gate iscargo clippy --workspace --all-features -- -D warnings.- Windows CI checks out with CRLF. A test that scans source text for
\n-joined markers must normalize\r\nfirst: Read NuGet and Cargo crawler project files through the FIFO-safe reader (#592) #602 failedtest (windows-latest)this way. - reqwest 0.12
Response::chunk()streams without thestreamfeature.BinaryBody::chunkreturnsimpl AsRef<[u8]>, so core needs no directbytesdependency. cargo test -p socket-patch-cli --test repairhas 2 root-only failures (repair_exits_zero_and_stays_quiet_when_lock_file_unremovable,repair_cleanup_failure_is_reported_in_json_and_silent_modes). They chmod a directory read-only.- Windows:
DirEntry::metadata().len()reports a stale (cached) size for a file another handle is still writing. Size live files withstd::fs::metadata(path)in tests.
Part 2: CLI command layer and user experienceLast checked against main @ 045d7ec on 2026-10-03 by audit-core. Owner: audit-core.
2.1 Headline numbers
2.2 God functions
Mode is three booleans ( The root cause is that output mode leaks into the engine. For example, 2.3 No service layer: commands call each otherCommand modules double as libraries and form a dense web:
2.4 Shared abstractions exist but are bypassed
2.5 Configuration flows through process environment
2.6 Structural duplicationA sliding-window copy-paste detector finds little literal duplication. The duplication is structural: helpers were extracted, but the pipelines around them were forked.
2.7 Flag and surface sprawl
2.8 Inconsistent JSON (verified against the binary)
2.9 UX: the command model is the real problem
A simpler command model:
That is 7 verbs instead of 9 visible + 2 hidden + 2 aliases + 3 hidden flag spellings, with one rule for mode ("whatever the project is, unless you say otherwise"). 2.10 Recommendations
New findings since the review
(The Generated by Claude Code |
Part 3: Hosted mode (redirect, hosted engine, upstream restore, Node addon)Last checked against main @ 203e092 on 2026-10-02 by audit-ecosystems. Owner:
3.1 Size
That is about 27K production lines for hosted mode, plus about 23.6K lines of inline tests and about 42K lines of hosted integration tests ( 3.2 How the redirect logic is organizedThere is no trait. Each rewriter is a free function with the signature
Adding an ecosystem to hosted mode means editing at least eight parallel tables:
Long functions.
In total there are 647 functions, 44 of them over 100 lines. Other symptoms:
Misplaced and dead code.
3.3 Two orchestrators: disk and in-memoryThe shared stages are good:
The warning-order row is a real divergence, and nothing catches it:
The parity suites exist only because there are two orchestrators: The memory engine also cannot reach several rewriters: vlt is always offline-withheld, maven and nuget raise Recommendation. Both paths converge on 3.4 Duplication with vendored, VEX and formatsA "one model per format" layer (
What already works, and is the template to copy: Module cycles (production
3.5 Upstream restore: rebuilding what was thrown awayCost. 7,446 production lines: Python 2,474 ( Why it exists. v5 dropped the redirect ledger. Yet every rewriter still computes Network dependencies: the npm registry, the crates.io sparse index, the Go proxy and Failure modes:
The open backlog shows the cost:
Assessment. Upstream restore is justified only where the original entry is a pure function of registry data: npm/pnpm/bun-text For uv, pylock, poetry, pdm, hatch, vlt, maven and bun.lockb, the module's own fallback (
There are two cheaper alternatives:
3.6 Per-package-manager features with poor complexity-to-value
3.7 Recommendations
New findings since the review
Generated by Claude Code |
Part 4: JavaScript lockfiles (npm, pnpm, yarn, bun, vlt)Last checked against main @ 045d7ec on 2026-10-03 by audit-ecosystems (§4.4 pnpm, berry gates, package-lock walks and JSON writers re-checked; the rest is as of
4.1 Summary
4.2 Code per format
Shared npm-family infrastructure adds
4.3 Parser and splicer matrix
The question "which lockfile drives installs?" is also answered in five places with different rules:
The modes disagree on policy. Vendored mode picks one flavor and warns about the rest. Hosted mode runs all five npm-family rewriters over every lock present. A repo with both 4.4 Verified duplicationpnpm v9 vs pnpm legacy. The drivers are now shared (#583): The vendor driver skeleton is copied eight times. npm_lock, pnpm, pnpm-legacy, yarn-berry, yarn-classic, bun_lock, bun_binary and vlt all repeat the same sequence: Yarn berry project gates are written twice.
Small helpers that have already drifted apart:
CRLF policy is inconsistent for the same file family:
Five different answers to one question, and every one of them is a bug class (see the open-issue appendix). 4.5 Architecture defects
4.6 Complexity vs value
4.7 Target structureEach format exposes
Combined (A, B-drop, C-G, I): about 6-7K production lines and 8-10K test lines, before any vlt decision. New findings since the review
Generated by Claude Code |
Part 5: Vendored mode and the non-JS backendsLast checked against main @ 045d7ec on 2026-10-03 by audit-ecosystems (5.4 Python and Cargo only). Owner: audit-ecosystems.
5.1 Size
Production lines per backend:
Framework files: 5.2 No backend traitThe only traits under
The signatures are close but not identical:
The CLI papers over the differences with two macros, 16 production sites enumerate the ecosystems, and each is a place a new ecosystem must be added:
Inside backends there is a second dispatch layer: Ecosystem identity is inconsistent. The legacy single-POM Maven entry is The CLI reaches into backend internals: 54 distinct
These are hooks a trait should expose. The orchestrator
5.3 The ledger, and nine ways to undo a changeEach
"Record the original and restore it" is the right idea, but it is implemented about nine different ways:
Revert/restore/unwind code in the non-npm backends totals about 3,540 lines: gem 422, nuget 305, pypi 291, pypi_lock 254, uv 231, cargo 220, composer 205, and more. Back-compat costs:
5.4 DuplicationFormat parsers live in three homes ( XML: eight hand-rolled scanners and no XML crate.
Python:
Cargo:
Gem: Go: Small helpers:
5.5 Security: unbounded zip inflate (verified)
It runs on:
Its twin A malicious PR that commits a crafted 5.6 Scaffolding left over from the removed local-build pathv5 removed local artifact building, but the scaffolding remains:
Value: negative. Delete it (about 250 lines, near-zero risk). 5.7 Complexity vs value
5.8 Target designtrait VendorBackend { // one impl per ecosystem, listed in a static REGISTRY
const ECO: &str;
fn artifact_shape(&self) -> Shape;
fn leaf_to_purl(..);
fn wiring_files(&self, v: &ProjectView) -> Vec<String>;
fn preflight(&self, v, pkg) -> Result<Option<PlannedDownload>, Refusal>; // = lock_text_refusal + service_preflight
fn plan(&self, v: &ProjectView, pkgs: &[Pkg]) -> Result<Plan, Refusal>; // pure, BATCHED
fn materialize(&self, archive, stage) -> Result<(), String>; // extract / tag / afterHash check
fn in_use(&self, v, e) -> Option<bool>;
fn recover(&self, e) -> Option<LockfileEntry>;
}
struct Plan { writes: Vec<FileWrite>, records: Vec<SpliceRecord { file, anchor, original: String, new: String }> }The engine owns:
Old Estimated saving: about 6–8K production lines of the ~44K in this slice (15–18%), and more in tests. Per-backend conformance tests collapse into one suite; for example, Risks:
New findings since the review
Generated by Claude Code |
Part 6: Discovery, inventory and VEXLast checked against main @ 045d7ec on 2026-10-03 by audit-ecosystems. Owner:
6.1 Size
The VEX stack is about 13K production lines tested by about 59K test lines: ~11K of core crawler tests and ~40K of CLI 6.2 Four discovery systems
How
Overlaps:
This could be one pass, because the per-format parsers are already mostly shared. What is duplicated is the classification and selection layer. One model would replace all of them: struct Instance {
purl: CanonicalPurl,
declared_in: Option<Rel>,
resolution: Registry { url, integrity, source_kind }
| Hosted { uuid, url, integrity, required }
| Vendored { uuid, artifact_rel, integrity }
| Other,
installed_at: Vec<PathBuf>, // filled in by locators (ex-crawlers)
}From that model:
6.3 Format × subsystem matrix (abridged)
Across the repo that is eight hand-rolled XML scanners, 4–5 independent walks of package-lock 6.4 Recurring helpers (confirmed)
Dead code:
6.5 VEX design
6.6 Crawlers
Are crawlers needed in hosted and vendored modes? Only as locators, not enumerators.
6.7 Target layoutRisks. Discovery is fail-closed, security-sensitive code. The golden snapshots and the ~40K lines of end-to-end tests are the safety net, so migrate one format at a time behind them. The New findings since the reviewNone yet. Generated by Claude Code |
Part 7: Core infrastructure and agent (in-place) modeLast checked against main @ 045d7ec on 2026-10-03 by audit-core. Owner: audit-core. Only the timeout, blob/diff body, zip-read, process-spawning, API-pacing, URL-builder, retry, batching, hashing and UUID passages have been re-checked; the rest is as of
7.1 Size
That is about 19.6K production lines. Comments are a large share of them: 25% of 7.2 API clientWhy
Three retry systems in one module:
The vendor policy also has three separate hand-written retry loops, plus a first-attempt/resume split that exists only to keep the request sequence identical under prefetch. Two near-identical downloaders ( Three URL builders with two different policies. Timeouts on the main paths (#581).
Batching is defined three times. The CLI owns the batch sizes (500 authenticated, 100 proxy, the 256 KiB body cap) and accepts any Other HTTP stacks keep TLS and proxy settings consistent but diverge on timeouts, retry and error formatting:
Target. One retry primitive (a classifier, a Retry-After parser, a timeout) to replace the four loops. That gives blob and diff downloads retry and timeouts for the first time. Then merge the downloaders and URL builders, and move the vendor service and credentials into their own files. 7.3 Duplicated utilities (verified)
7.4 Agent modeFootprint:
Counting the agent arms in The safety model is sound and worth keeping:
The default Apply and rollback are mirror images.
So diff only saves bytes when a user commits
Sidecars (
Maven sidecars are not handled at all. This code exists only for in-place mode.
Dead path (verified): 7.5 Features with questionable value
7.6 Recommendations
New findings since the review
Generated by Claude Code |
Part 8: Tests, CI, docs and distributionLast checked against main @ 045d7ec on 2026-10-03 by audit-core. Owner: audit-core. Only the repository-hygiene passages (stray
PR #277 has already started cleaning up: it deleted 237,608 lines, including 136,809 lines of 8.1 Test suite architecture
Consolidation was started but not finished.
Duplicated helpers.
Coverage-chasing tests.
Exact human-text assertions. 328 Process-global env forces serialization. 8.2 CI costFor
About 516 jobs per push across all workflows:
8.3 Docs
User docs are lean, with rough spots.
8.4 Scripts and distribution
8.5 Recommendations
New findings since the review
Generated by Claude Code |
Appendix A: The open-issue backlog, mapped to architectureLast checked against The repository had 88 open issues when this review was written. Most were filed between 2026-09-26 and 2026-10-01 by a bug hunt ( By ecosystem
By architectural cause
What the backlog says about the design:
Appendix B: Methodology and measurement notesSizing.
Function lengths come from brace matching over that lexed text: start line to closing brace. They can be off by a few lines but are not affected by braces inside string literals. Duplicates were found three ways:
CLI surface was measured on a debug build: CI numbers are from the GitHub Actions run for Verified by hand (not just reported by an area reviewer):
Not verified by execution: the per-area reviewers' "probable bug" readings beyond those listed (they are labelled as such in each part), and every LOC savings estimate. Those are engineering estimates, not measurements. Read-only: no code was changed. The review text lives under Generated by Claude Code |
|
[agent] 2026-10-02: the architecture routines are set up This discussion holds the October 2026 architecture review: the top post, plus Parts 2–9 above. It is also the running log of the refactoring work.
Generated by Claude Code |
|
[agent] 2026-10-02: architecture audit (ecosystems and formats) main @ Verified on main, each reproduced with a unit test run twice. The probe tests were not committed.
New finding: E49. There are four hosted-PyPI-URL grammars. Hosted Pipenv's Searched without filing:
Next backlog rows: E05 (cache crawls not project-scoped), E06 (non-FIFO-safe crawler reads), E07 (package-lock walks), E08 (yarn grammars), E09 (berry gates). Generated by Claude Code |
|
[agent] 2026-10-02: this discussion is now a living document
Generated by Claude Code |
|
[agent] 2026-10-02: architecture audit (CLI and core) main @ Verified on main. Every bug below was reproduced twice by execution. The probes were not committed.
New finding: C37. Searched without filing:
Living document: check lines set on Parts 2 and 7, tokens added for C02/C03, a C37 bullet under Part 7's new findings, and the cheap §0 numbers refreshed (209 test executables, Next backlog rows: C04 (hatch spawn, vs #442), C05 ( Generated by Claude Code |
|
[agent] 2026-10-02: architecture refactor run main @ Ranking.
The full top five is in the register's Queue. PR. #572 adds
Claimed: #563 ( Living document: added the Lessons: the four root-only test failures, and the rustfmt state of the files touched (see Notes). |
|
[agent] 2026-10-02T16:27Z: architecture refactor run
|
|
[agent] 2026-10-02: architecture refactor run
|
|
[agent] 2026-10-02: architecture audit (ecosystems and formats) main @ Reconciled.
Verified on main and filed. Every bug below was reproduced with a unit test run twice. The probes were not committed.
New finding: E50. Hosted Searched without filing:
Living document: check lines set on Parts 3 and 6. Tokens added for E05 and E06 (Part 6.6), E11 (Parts 3.4 and 5.4) and E12 (Part 4.4). Moved line numbers were corrected, and an E50 bullet was added under Part 3's new findings. Next backlog rows: E37 (byte-identical Generated by Claude Code |
|
[agent] 2026-10-02: architecture refactor run Merged: #572 (merge Register: E04 and E49 are now Living document: Part 5.4 ( CI notes from the PR: it got 3 transient failures, and each passed on its one re-run:
CodeQL flagged a test assert message that printed the refusal detail; that was fixed in ff3ef3e. |
|
[agent] 2026-10-02T20:05Z: architecture refactor run Merged: #581 (merge Register: C02 is now Living document: rewrote the C02 row in Lessons: |
|
[agent] 2026-10-02T21:25Z: architecture refactor run main: Ranking: 2 of 3 slots were in use (#574, #602). The top three candidates (#568, #593, E37) change files that open PRs #598, #597 and #602 change. C07 was verified on PR: #607, "Stream patch blob and diff downloads to disk (#571)". It is ready for review, with CI and Bugbot pending.
Claimed: #571 (Claim-ID Merged / living document: #581 was recorded by the previous run. This run removed it from In flight in the register. C37 is now Lessons: The skip rule holds even when another PR's hunks are far from the code you'd change (#598 edits |
|
[agent] 2026-10-02: architecture audit (CLI and core) main @ Reconciled:
Backlog verified and decomposed:
New finding: C38 → #614. Every CLI patch-API window paces through Searched without filing:
Living document:
Next backlog rows: C07 (URL builders on org fallback; needs a decision, see the refactor run's verification), C08 (repo hygiene), C09 ( Generated by Claude Code |
|
[agent] 2026-10-03: architecture audit (ecosystems and formats) main @ Reconciled.
Verified and filed:
New finding: E51 (proved by execution, a unit probe run twice, not committed). Take a yarn berry project whose root
Filed #628 (bug, pm:yarn-berry). The recommended fix is that hosted mode refuses this too, from the shared gate. New finding: E52. In Searched without filing:
Living document:
Next backlog rows: E07 (package-lock walks), E08 (yarn grammars, now pairs with #629), E10 (XML scanners, Maven/Gradle half), E14 (Pipfile.lock two writers), E15 (Cargo.toml parsers). Generated by Claude Code |
|
[agent] 2026-10-03: architecture audit (CLI and core) main @ Reconciled:
Backlog verified and decomposed:
New finding: C39, filed together with C09 as #647. The command-level fallback misses more than Searched without filing:
Living document:
Next backlog rows: C10 ( Generated by Claude Code |
|
[agent] 2026-10-03: architecture audit (ecosystems and formats) main @ Reconciled.
Verified and filed:
New finding: E53 (proved by execution, a unit probe run twice, not committed). Vendored pnpm still writes the root
npm and yarn berry keep the layout through Searched without filing:
Living document: Part 4's check line is refreshed. The package-lock row of the walk table has new line numbers and #357 notes, plus {{E07}}. The JSON-serializer bullet is rewritten for current main (#357, {{E53}}), the stale "npm_lock normalizes to LF" claim in the CRLF list is corrected, the §4.7 E row is updated, and an E53 bullet is added under new findings. Next backlog rows: E08 (yarn grammars; after #657 lands), E10 (XML scanners, Maven/Gradle half; #646 is adding Gradle), E15 (Cargo.toml parsers), E13 (poetry/pdm; the refactor routine notes the walkers really differ), E16 (CRLF policies). Generated by Claude Code |
|
[agent] 2026-10-03: architecture audit (CLI and core) main @ Reconciled: all register rows hold their statuses: C03 #568, C04 in PR #617, C05 #615, C07 #648, C08 #649, C09/C39 #647, C37 in PR #607 and C38 #614. Backlog verified and decomposed:
New finding: C40 → #678. I checked every Searched without filing:
Living document:
Next backlog rows: C10 ( Generated by Claude Code |
|
[agent] 2026-10-03: architecture audit (ecosystems and formats) main @ Reconciled. No status changed in this area: E01 (#597), E02/E03 (#574), E06 (#602) and E09/E51 (#657) are still in their PRs, and nothing in scope merged. Because main did not move, no open issue needed re-checking. Verified and filed:
New finding: E54 (proved by execution, run twice). The finish steps of #694 differ in their line-ending rule. Poetry turns the whole rendering CRLF if the input has any CRLF; PDM uses Searched without filing:
Living document: Part 5's check line is refreshed (5.4 Python and Cargo). The poetry/pdm bullet is rewritten with {{E13}}/{{E54}}, the Cargo bullets are rewritten for current main with {{E15}}, the CRLF list now names four policies, and an E54 bullet is added under new findings. Next backlog rows: E15b (hosted Generated by Claude Code |
|
[agent] 2026-10-03: architecture audit (CLI and core) main @ Reconciled: all register rows hold their statuses. That covers C03 #568, C04 in PR #617, C05 #615, C07 #648, C08 #649, C09/C39 #647, C15 #676/#677, C16 #675, C37 in PR #607, C38 #614 and C40 #678. Backlog verified and decomposed:
New finding: C41 → #707 (bug). The hash case policy is decided per site:
A temporary core integration test (run twice, not committed) uppercased a manifest's hashes. Searched without filing:
Living document:
Next backlog rows: C10 ( Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Note
[agent] This is a living document. It is the October 2026 architecture review of the socket-patch CLI, kept current as we work through the code. The scheduled routines rewrite a section when a refactor lands or they find a new problem. Every
E…/C…reference shows that problem's live status, taken from the register.Progress: 95 problems tracked · 5 fixed · 1 already fixed · 9 in PR · 19 filed · 3 decision pending · 57 to verify · 1 rejected. On GitHub: 31 open and 3 closed
arch-auditissues · 4 open and 4 mergedarch-refactorPRs.arch-audit· refactoring PRs labelledarch-refactorRendered 2026-10-03 15:51 UTC (ledger
84e4dbd2) fromdoc/on thearch-audit/ledgerbranch. The original snapshot is inreview/2026-10/on the same branch.Architecture review of socket-patch v5: what to cut, combine, refactor and simplify
TL;DR
socket-patch is a product matrix implemented cell by cell. The matrix is 3 modes × 9 ecosystems × ~25 package-manager and lockfile generations × 6 operations (discover, apply/wire, verify, attest, revert, mode-takeover). There is no shared abstraction on any axis:
Each cell is hand-written text surgery. As a result, the same file format is parsed and spliced two to five times with different rules, and those rules have already drifted. For example, there are five different CRLF policies for the npm lockfile family alone.
Most of the open bug backlog has one shape: "reported success, but the build consumes unpatched code". Of 88 open issues, roughly 29 are a success or a
not_affectedVEX attestation that the installed bytes don't back up. Another 15 are discovery missing what the package manager actually installed. The root cause is architectural:Support breadth has outrun the architecture. The long tail is disproportionately expensive:
bun.lockbA support-tier policy would let the core get simpler.
The code is large for what it does, and much of the size is duplication and scaffolding.
run_scanalone is 1,499.redirect/mod.rs.We estimate 25–35K production lines (20–30%) and 50K+ test lines could go. Roughly 20K of that comes from consolidation that keeps every capability; the rest comes from the support-tier and product decisions in §5.
The user model is harder than it needs to be. It has:
scanwriting lockfiles by default;scanperforms the documented "mode takeover" and switches vendored npm, Cargo and Go packages back to hosted;remove,rollbackandvendor --revertas three ways to undo.A seven-verb model (
scanread-only,fix,undo,sync,check,list,vex) with mode inferred from the project would cover everything (§4).There are a few real defects to fix now (§1), regardless of any refactor:
SOCKET_FORCEbound to three unrelated--forceflags.0. The numbers
#[cfg(test)]code insrc/crates/*/tests)045d7ec, 2026-10-03)patch/redirect/mod.rs: 19,571 lines at045d7ec(2026-10-03; 17,517 at the snapshot, 6.2K production then)run_scan1,499,rollback::run984,vendor_records_reusing962,run_redirect_selected836,remove::run797,get::run635, memoryengine604, …)SOCKET_*names in source); 156 documentederrorCodes; ~570 code-like strings in source--helplist --helplists 27 options, most of which do nothing forlisttestis the 28-minute critical pathCLI_CONTRACT.md045d7ec, 2026-10-03; 332 KB / 9,320 at the snapshot)bug). At the snapshot: 88, filed mostly in the last 5 days by a bug hunt; JS 26, JVM 22, Python 18, Go 6, Cargo 5, NuGet 5, Ruby 3, Composer 31. Fix now (small, independent of any refactor)
zip_bytes_match_after_hashesnow streams each member through the Git SHA-256 reader with an 8 KiB buffer and checks the declared length against the bytes read, instead of inflating it into aVec(#587). The maintainer ruled that this data is trusted not to be too big, so the goal was streaming, not a cap.C01· fixed (#587); streams members, no cap per maintainerApiClientreqwest clients now takeapi::retry::ApiTimeouts(10 s connect, 60 s idle read), and a stalled JSON body reportsApiError::Network(#581). Blob/diff downloads still have no retry.C02· fixed (#581)RevertOutcome.kept_artifactsays callers "must ALSO keep the state.json entry" (core/vendor/mod.rs:664-673).vendored_takeover(cli/scan/hosted.rs:1732-1790) never reads it, deletes the entry, and tells the user that the "committed artifact" was reverted. Every other revert caller honors the flag.VendoredBackend, and add a regression test.C03· filed #568vendor/pypi_hatch.rs:118runsCommand::new("hatch").current_dir(root). That is exactly the patternutils/process.rs:23-33documents as unsafe: a relativePATHentry executes ahatchplanted in the scanned repo. It is the only production bare-name spawn. Reproduced on045d7ec: withPATH=.:…, a plantedhatchruns and its fake version passes the>=1.2gate.process::resolve_tool.C04· in PR #617redirect/mod.rs:4302 nuget_package_source_keysregex-scans raw XML without masking<!-- -->. A commented-out<add key>changes which sources get mapped. The vendored andformats::nugetreaders both mask comments.formats::nuget::parse_config.E01· in PR #597SOCKET_FORCEis bound to three unrelated flagsvendor --force,apply --forceand--update --force(vendor.rs:80,apply.rs:339,update.rs:61at045d7ec). Exporting it to force a self-update also forcesapply/vendorpast hash checks.C05· decision #615bun.lockbignores the registry overridevendor/bun_lockb.rs:235hard-codesregistry.npmjs.orginstead ofregistry_fetch::npm_tarball_url, ignoringSOCKET_NPM_REGISTRY. vlt has two divergentregistry_baseimplementations.E02E03: 2 in PR.github/actions/actions/cache/<sha>/.vscode/launch.json(accidentally committed in #358); 2 dead CI path filters (CI janitor); 39 references in 20 files to a "DESIGN §x.y" document that isn't in this repository. The README now says plainly that its installer selects the latest release (verified on045d7ec).C08· filed #649; README part already fixed2. The big picture: why the code is the size and shape it is
2.1 No abstraction on any axis of the matrix
formats/layer. Its module doc promises "entry grammar, key rules, version sniff and planners" per format; only pnpm, cargo, gem, composer and bun are partly there.parse → model (with byte spans) → entries() / wired_refs() / splice(edits), used by every mode. Today package-lock has 4 entry walks, yarn has 5 copies of asplit("\n\n")+regex grammar beside the shared block scanner, there are 8 hand-rolled XML scanners (no XML crate), Cargo.toml has a regex scanner andtoml_editinside one rewriter, and poetry/pdm lock code are near-twins.run_scan, referenced 91 times; JSON and human arms that each re-dispatch all three modes.trait ModeBackend { plan, consume, revert, verify }, with rendering only at the end.vend!,vend_installed!). The ecosystem list is enumerated at 16 production sites. Nine different revert mechanisms (~3.5K lines).trait VendorBackend+ a registry + one generic splice-record revert engine. The JVM planner (jvm/mod.rs) already is this design; copy it.Vec<Box<dyn Fn>>. Results flow through aRewriteResultwith 20 per-ecosystem uuid sets and a 16-ruleconfirm()if-chain. Eight parallel tables must be edited to add an ecosystem.trait HostedRewriter { drives(), rewrite() -> Outcome { per_dep: Map<Uuid, DepStatus> } }vex::discover) and a ledger supplement. They are merged by fabricatingCrawledPackages with a fakenode_modules/<name>path for every ecosystem.Inventory { instances: purl × declared_in × resolution (Registry/Hosted/Vendored) × installed_at }. Crawlers become locators.args.rs:559 apply_env_toggles) so core can read them. Its doc comment records a bug where telemetry sent a Bearer token to the wrong host. This also forces 553#[serial]test attributes.RunCtx { config, client, telemetry, lock }built once inmain.Layering is inverted and cyclic (production
crate::X::reference counts):Other examples:
crate::vendor::*, sovendor/has become the codec library.Ecosystemandcanonicalize_pypi_namelive incrawlers/(the latter is imported by 29 files).formats/pnpm/hosted.rsimports the hosted engine'sRewriteResult.The CLI holds engine code.
vendor_records_reusing(962 lines) is the vendored orchestrator.run_redirect_selected(836) is the disk hosted orchestrator, written a second time in core'shosted/memory(~1.3K lines of orchestration).ecosystem_dispatch.rsis 816 lines of crawler fan-out.Commands call each other as libraries.
get↔scancycle.getbuilds a fakeApplyArgsand callsapply::run_locked.DownloadParams→..GlobalArgs::default(). On045d7ecthe reset fields are inert: the nested apply reads none of them exceptoffline, whichgetandscanrefuse up front.2.2 The correctness model: "wired" is treated as "consumed"
The tool decides that a patch is applied, and VEX marks it
not_affected, mostly from what it wrote. It does not check what the package manager will install:PatchedRef::lockfile_basis_oklets an attestation stand with no installed bytes;Every package-manager behavior outside that model becomes a silent false negative. The open backlog, by title, includes:
go.workreplaces overriding go.mod (A user replace in go.work silently overrides the Socket go.mod replace: Go apply and vendor report success and VEX attests not_affected while the build links the user's target #393);vexattests a hosted patch as not_affected (verified) while the installed copy under node_modules/.bun is still unpatched (v5 regression) #405);deno.locktaking precedence over package-lock (Hosted npm pin in package-lock.json is attested by VEX in a Deno project, but deno.lock keeps installing the unpatched registry copy #406);--system-site-packages(In a--system-site-packagesvenv, pip keeps the base interpreter's unpatched copy after a hosted rewrite, no stale-install warning fires, andvexattests it as patched #409);<repository>or amirrorOf *mirror serves the same GAV, and VEX still attests #263);BUNDLE_GEMFILE(Gem hosted redirect andsetupignoreBUNDLE_GEMFILEfrom.bundle/config, so they wireGemfilewhile bundler loads the configured manifest unpatched (VEX andsetup --checkstill pass) #390);gems.rbbesideGemfile(Vendored gem mode wires Gemfile when gems.rb is also present, so bundler installs the unpatched gem while vex attests it #341);inBundlecopies (npm VEX attests not_affected while a bundled (inBundle) copy of the same package@version stays unpatched #325);globalPackagesFolder(Agent-mode NuGet apply patches ~/.nuget/packages instead of the project's configured globalPackagesFolder / RestorePackagesPath, reports success, and VEX attests not_affected #397).Patching each case individually makes the model ever larger. Structural options, which can be combined:
Verify what the package manager consumes, not what we wrote.
--allow-wired-basisflag.not_affectedis worse than no statement, because the whole point of VEX is that scanners trust it.Ask the package manager instead of re-implementing it wherever possible:
poetry env info -p,pipenv --venv,uv python find;npm query/npm ls --json,pnpm list --json;go list -m -json all,cargo metadata;mvn dependency:list,dotnet list package --include-transitive.Today the crawlers re-implement Poetry's and Pipenv's venv-name hashing, npm/pnpm/yarn/bun global-prefix discovery, and the pnpm store layout, and those re-implementations are the source of the agent-mode discovery bugs (Agent-mode scan misses Poetry's venv for nameless non-package-mode projects, [project].name overrides, and in-project = false with a stray .venv, and still exits 0 #327, Windows agent-mode scan never finds Poetry's default out-of-tree virtualenv, so patches are skipped and the scan exits 0 #329, Agent-mode scan skips Pipenv's out-of-tree venv when the project has a stray venv/ directory or a .venv with PIPENV_VENV_IN_PROJECT=0, and still exits 0 #334, Agent mode ignores pnpm's virtualStoreDir: transitive dependencies are reported package_not_installed with a custom virtualStoreDir or the global virtual store #362, Bun isolated linker: transitive packages under node_modules/.bun are "not installed" in agent mode, and scan --mode agent exits 0 with them unpatched #366, Deno nodeModulesDir: transitive npm packages under node_modules/.deno are "not installed", and apply/scan exit 0 leaving them unpatched #373, Agent-mode scan patches the activated VIRTUAL_ENV even when PIPENV_IGNORE_VIRTUALENVS or PIPENV_ACTIVE tells Pipenv to ignore it, leaving the Pipenv venv unpatched with exit 0 #384).
Fail closed on unmodeled configuration. Detect the knobs that change resolution (
go.work,gradle.lockfile,BUNDLE_GEMFILE,virtualStoreDir/enableGlobalVirtualStore,install-strategy=linked,repositoryPath/globalPackagesFolder, mirrors) and refuse or warn instead of reporting success.Make "verify after install" a first-class step (
socket-patch check) that CI runs after the package manager, with a non-zero exit when what was installed doesn't match what was wired.2.3 Hosted rollback rebuilds data it threw away
v5 dropped the hosted ledger, so
rollback/removereconstruct the original lock entries from the network. That is ~7.4K production lines across about nine upstream sources, including a Socket endpoint that may download the whole upstream tarball. Meanwhile, every rewriter already computesFileEdit { original, new }, and production discardsoriginal, except in a Composer hint.Failure modes:
SOCKET_NPM_REGISTRY);bun.lockbalways refused.That is 13 open rollback/takeover bugs (#271, #331, #382, #385, #407, #408, #410, #411, …).
Options:
resolved+integrity, cargocksum, go.sum, gem/composer/nuget hashes; ~2.2K lines), and refuse the rest with an exactgit checkout -- <file>or relock command.Either removes ~3.3K production lines.
2.4 Machinery that compensates for the per-package call model
Vendored backends are invoked once per package, and each call re-reads, re-parses and durably re-writes the same lockfile and ledger. Several mechanisms exist to make that fast and crash-safe again:
group_commit.rs(1,059 lines), a process-wide virtual filesystem that intercepts everyutils::fsread and write;durability.rs;prestage.rs;api/vendor_prefetch.rs;ParseMemostatics;ledger_snapshots.rs, the schema-v2 delta encoding added because whole-file snapshots bloated ledgers by tens of MB.Together that is ~3K production lines. Backends written as pure batched planners (
plan(view, pkgs) -> {writes, records}, asjvm/already does) would retire most of it.2.5 Process smell: nothing gets deleted
Several patterns show code that outlived its purpose:
--vendor-source(one valid value),VendorSource/PackageSource(one variant each),PatchSources::mem_blobs(neverSome),lock_inventory/wired.rs(no production caller), pre-v5 redirect-ledger readers.v5.0annotations in the contract.Suggested norms:
3. Ranked recommendations
C = cut, M = combine/merge, R = refactor, S = simplify. LOC are production lines unless noted. Risk: L/M/H.
formats/(package-lock → yarn → XML for NuGet/Maven/Gradle → requirements → Cargo.toml → Pipfile → pnpm single grammar), shared by hosted, vendored, upstream, inventory and VEX. Neutral types (Edit,Warning,LockfileEntry) move intoformats, which breaks the cycles.E07–E20: 1 fixed · 2 in PR · 3 filed · 8 to verifyVendorBackendtrait + registry + one generic splice-record revert engine, with backends as batched pure planners (the JVM pattern). Legacy ledger kinds are adapted at load time.E21E22E23E24E25E27: 6 to verifyInventoryfusing lock inventory, wiring discovery and the ledger supplement. Crawlers become project-scoped locators, with no whole-machine cache enumeration (~/.m2,~/.nuget/packages,$CARGO_HOME,GOMODCACHE). Renamevex::discovertoinventory::wiring.E05E36E37E38E39E40E41: 2 filed · 5 to verifyE33E45: 2 to verifyDiskSnapshot→MemoryProject→redirect_root(view, selected, api, hooks)); parity suites become ordinary tests. Decide the napi addon's fate: if depscan adopts it, delete the TS rewriters; if not, delete the addon,hosted-bundleand the memory-only branches.E32E44: 2 to verifyrun_scaninto discover → select → consume → render; aRunCtxbuilt once; a service layer in core so commands stop calling each other;HostedRewriter+Outcome; mechanical split ofredirect/mod.rs.C10C11C12E30E31: 5 to verifyscan,fix,undo(foldsremove+rollback+vendor --revert),sync,check; mode inferred from project state; per-command flags; delete deprecated spellings.C34· to verifyscan/get/rollbackstill emit an untypederror: a bare string on some paths, a{code, message}object on others;C14· decision #704); a typed code registry (enum Reason × Ecosystem) that generates the contract's code tables, with a freshness test.C13C14: 1 decision pending · 1 to verifybun.lockbwrite support → refuse with remedy; vendored pnpm 7/8 → refuse (or a dialect of v9); vlt pre-1.0 encodings; Maven single-POM backend merged intojvm/.E26E47: 2 to verify--download-modedefaultfile:diffre-downloads every blob anyway on a cold cache (fetch_stage.rs:377); delete the diff machinery andqbsdiff.C25· to verify--vendor-source,VendorSource,PackageSource,vend_installed!,mem_blobs,lock_inventory/wired.rs, dead vlt ledger helpers,save_redirect_state+ its group-commit entry,Pypi/LauncherCacheupdate channels, the empty Deno extractor, theswitched_off("group_commit")oracle path.E28E41C23: 3 to verifyformat!("pkg:…"), 58 prefix checks); one digest/SRI helper set (fixing thesha256_hexname collision: one copy validates, three compute); one line-ending policy; one env-truthiness vocabulary (there are three); one UUID grammar (there are four).C15C17C18C19C20E16: 3 filed · 3 to verify--vex(15 flag instances on 3 commands, ~600 lines of glue, plus bypass sets that couple VEX correctness to each caller) →fix && vex -O.E42C35: 2 to verifyallow-remote(re-implements npm'siniand config layering, ~900 lines), pnpmtrustLockfile(~450; three open corruption bugs), the vlt warm-tree heal (installed-tree surgery in a lockfile-only mode), parallel rewriter groups (benchmark them or drop them).E34· to verifytrack(Event)+ a shared client instead of 17 wrappers and 125 token/org plumbing sites.C22C36: 2 to verifyRunCtxfirst, to drop the env-mutating#[serial]); asocket-patch-test-supportcrate (binary()is defined in 102 files,git_sha256in 84, and 14 divergentscrub_socket_env); retire the oracles; triage covgap; snapshots instead of 328 sentence assertions.C30C31C32E35: 4 to verifydocker-baseoff PRs (≈74 runner-min/run); PR e2e 148 legs → ~50 boundary versions; reusable compat workflow; no per-leg compiles.ecosystems.mdand history into the CHANGELOG; decoupledocs/testingfrom validation scripts.C33· to verifyEstimated total: ~25–35K production lines (20–30%) and 50K+ test lines. About 20K is pure consolidation (recommendations 1–3, 6, 8, 10–12, 14); the rest depends on the tier and product decisions (recommendations 4, 5, 9, 13, 15). That is before any decision to deprecate agent mode, which would remove another ~10K. The per-recommendation numbers overlap: for example, recommendation 1 shares work with 2 and 9.
4. User experience: a simpler model
Today a new user has to learn:
scan --pruneandscan --globalbecome report-only;get --save-onlyandget --globalbecome agent mode;get -g xpatches files in place whilescan -gonly reports;scanmutating lockfiles by default;remove,rollback,vendor --revert;repair(aliasgc), filed under "Agent mode" in help but also repairing vendored artifacts, andscan --prune;--help, most of them no-ops for that command;Proposed command model:
socket-patch scanscan --dry-run,listpartiallysocket-patch fix [TARGET…] [--mode hosted|vendored|agent]--modechooses it for a fresh project, and switching an existing project's mode requires--modeexplicitly.scan(write),get,vendor(eject)socket-patch undo [TARGET…] [--keep-state|--forget]rollback,remove,vendor --revertsocket-patch syncapply,repair/gc,scan --prunesocket-patch checkvendor --check,apply --checksocket-patch list,socket-patch vexOn top of that:
errorCode;This is a MAJOR change. Because v5 is still a prerelease, now is the cheapest time to make it.
5. Support tiers (product decisions needed)
bun.lockbwriting (hosted + vendored)bun.lock. The codec writes a privatesktpnrmmarker into an unused slot of the user's lockfile.bun install --save-text-lockfile --frozen-lockfile --lockfile-only; keep a ~300-line read-only parser if inventory needs it.--frozen-lockfileonly passes at the original checkout path, which undercuts the point of vendoring.PnpmDialect.maven_repo.rsintojvm/(Shape::Single); one XML scanner; scope the crawler to project dependencies, not all of~/.m2. Consider labelling hosted Maven/Gradle "beta" until the backlog is under control.replace), or keep it and ask the package manager for layouts (§2.2)."private": true, never released, built and smoke-tested on every PR.--updateprint or run the installer one-liner.6. Suggested sequencing
setupcommand (Composer setup rewrites a CRLF composer.json as LF (and un-escapes \/ and \uXXXX), so setup --remove does not restore it byte-for-byte #351, Gem hosted redirect andsetupignoreBUNDLE_GEMFILEfrom.bundle/config, so they wireGemfilewhile bundler loads the configured manifest unpatched (VEX andsetup --checkstill pass) #390, npm apply exits 1 when every patch targets a platform-skipped optional dependency (fsevents, @esbuild/*), so the setup hook fails npm ci and npm install on other OSes #403).formats/codecs, one format per PR, each PR deleting the duplicate walks it replaces.RunCtx, which also unblocks the test-binary merge.VendorBackend+ revert engine.HostedRewriter+Outcome+ theredirect/mod.rssplit.Inventory.--vex.fileas the default download mode.Questions for owners:
bun.lockb, pnpm ≤ 8 and Gradle usage, to set tiers?.socket/in hosted mode" a hard requirement?Generated by Claude Code
All reactions