audit-core: C25 decision #792, C10 tracking #793/#794, new C45 (#791)

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

Author: claude

doc/01-summary.md

@@ -131,7 +131,7 @@
 | **Vendored backend** (per ecosystem) | Naming conventions plus two macros (`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. |
 | **Hosted rewriter** | Free functions in a hand-wired `Vec<Box<dyn Fn>>`. Results flow through a `RewriteResult` with **20 per-ecosystem uuid sets** and a 16-rule `confirm()` if-chain. **Eight parallel tables** must be edited to add an ecosystem. | `trait HostedRewriter { drives(), rewrite() -> Outcome { per_dep: Map<Uuid, DepStatus> } }` |
 | **Inventory** | **Four discovery systems:** crawlers, lock inventory, wiring discovery (`vex::discover`) and a ledger supplement. They are merged by fabricating `CrawledPackage`s with a fake `node_modules/<name>` path *for every ecosystem*. | One `Inventory { instances: purl × declared_in × resolution (Registry/Hosted/Vendored) × installed_at }`. Crawlers become *locators*. |
-| **Configuration** | Parsed flags are written back into process env (`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. | An explicit `RunCtx { config, client, telemetry, lock }` built once in `main`. |
+| **Configuration** | Parsed flags are written back into process env (`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 **993 `#[serial]`** test attributes (plus 185 in `src`). {{C10}} | An explicit `RunCtx { config, client, telemetry, lock }` built once in `main`. |
 
 **Layering is inverted and cyclic** (production `crate::X::` reference counts):
 

doc/02-cli.md

@@ -69,8 +69,9 @@ Command modules double as libraries and form a dense web:
 `apply_env_toggles` (`args.rs:559-575`) writes parsed flags back into `SOCKET_OFFLINE`, `SOCKET_DEBUG`, `SOCKET_TELEMETRY_DISABLED`, `SOCKET_API_URL` and `SOCKET_PROXY_URL`, so that core (51 production env reads) can see them.
 - Its own doc comment records the bug this caused: an on-prem `--api-url` run "POSTed the event — Bearer token included — to the default `api.socket.dev`".
 - It is called separately in 10 command entry points rather than once in `main`, and `vendor --check` returns before calling it.
-- It is why the test suites carry **553 `#[serial]` attributes** (plus 182 in `src`).
-- **Fix:** an explicit `RunCtx { config, client, telemetry, lock }` built once in `main` and passed down. Core should not read ambient env except at the edge.
+- It is why the test suites carry **993 `#[serial]` attributes** in `tests/` (plus 185 in `src`), counted on `045d7ec`; the review counted 553 + 182.
+- The lock timeout is converted by hand at 12 sites, and the API client is built at 13 production sites.
+- **Fix:** an explicit `RunCtx { config, client, telemetry, lock }` built once in `main` and passed down. Core should not read ambient env except at the edge. {{C10}}
 
 ### 2.6 Structural duplication
 
@@ -96,7 +97,7 @@ A sliding-window copy-paste detector finds little *literal* duplication. **The d
 - **Every global flag is silently accepted by every command.** `list --download-mode bogus --strict --yes --lock-timeout 9 --maven-config none --no-vlt-install-cleanup --dry-run` exits 0. `--update --ecosystems npm --manifest-path x.json --strict --global` parses.
 - **Dead or vestigial flags:**
   - `--vendor-source`: core's `VendorSource` has **one variant**, and `build` is an error.
-  - `--download-mode` is an unvalidated `String`, checked only where it is used. That breaks the "fail loud on typo" posture the same file applies to `--ecosystems`.
+  - `--download-mode` is an unvalidated `String`, checked only where it is used. That breaks the "fail loud on typo" posture the same file applies to `--ecosystems`. On `045d7ec` a bad value fails `apply` and `repair` with exit 1 and `apply_failed`/`repair_failed`, while `apply --check`, `rollback`, `list` and `vendor` exit 0; `--vendor-source bogus` is a clap usage error (exit 2). {{C45}}
 - **Deprecated spellings and aliases:** `scan --apply` (hidden), `scan --vendor` (hidden), `--sync`, `get --no-apply`, and the `download` and `gc` aliases. `resolve_mode_flags` (`scan/mod.rs:205–255`) exists mainly to reconcile these.
 - **Name collisions:**
   - `--package` is a value list on `scan` but a boolean type-forcer (`-p`) on `get`.
@@ -181,6 +182,8 @@ That is **7 verbs instead of 9 visible + 2 hidden + 2 aliases + 3 hidden flag sp
 
 - {{C44}} The ecosystem-name parser is written three times. `--ecosystems`/`SOCKET_ECOSYSTEMS` require an exact, case-sensitive `cli_name()` with no trim, socket.yml `patches.ecosystems` trims and lowercases, and `vendor::ecosystem_in_scope` has its own exact lookup. On `045d7ec`, `-e NPM`, `-e "npm, pypi"` and `SOCKET_ECOSYSTEMS=PyPI` exit 2, while `ecosystems: [NPM, pypi]` parses. `--min-severity` and `minSeverity` already share one parser.
 
+- {{C45}} `--download-mode` typos are runtime failures in two commands only (see 2.7): `apply`/`repair` exit 1 with a generic command code, and the rest accept them. This narrows the review's R8 note to a non-breaking fix (a typed clap parser).
+
 (The `C38` pacing finding is in Part 7.)
 
 ---

doc/07-infra-agent.md

@@ -2,7 +2,7 @@
 
 ## Part 7: Core infrastructure and agent (in-place) mode
 
-_Last checked against main @ 045d7ec on 2026-10-04 by audit-core. Owner: audit-core._ Only the timeout, blob/diff body, zip-read, process-spawning, API-pacing, URL-builder, retry, batching, hashing, UUID, env/home-dir, atomic-write, purl, dead-code, telemetry and apply/rollback-engine passages have been re-checked; the rest is as of `2463257`.
+_Last checked against main @ 045d7ec on 2026-10-04 by audit-core. Owner: audit-core._ Only the timeout, blob/diff body, zip-read, process-spawning, API-pacing, URL-builder, retry, batching, hashing, UUID, env/home-dir, atomic-write, purl, dead-code, telemetry, apply/rollback-engine, diff-download and `apply.lock` passages have been re-checked; the rest is as of `2463257`.
 
 > Scope: `api/*`, `manifest/*`, `ledgers.rs`, `constants.rs`, `patch/` (excluding `redirect/`), `policy/*`, `rollout*`, `update/*`, the CLI `update_notifier.rs`/`update.rs`, `telemetry.rs`, and the generic `utils/*` and `hash/*`.
 
@@ -104,7 +104,7 @@ The default `MismatchPolicy::Warn` silently overwrites locally modified dependen
 
 So diff only saves bytes when a user commits `.socket/diffs` but not `.socket/blobs`. The diff fetch is also sequential with no retry, and it is the only user of the `qbsdiff` dependency. Blob downloads are sequential with no retry too, while the JSON calls run 32 at a time.
 - **Recommendation:** make `file` the default (keep `diff` as an alias for one major), then delete `patch/diff.rs`, the diff branches in `blob_fetcher`/`fetch_stage`/`repair`, and `qbsdiff`.
-- **Saving:** about 600 production and 1,000 test lines, plus one dependency.
+- **Saving:** about 600 production and 1,000 test lines, plus one dependency. Re-verified on `045d7ec` (top-up at `fetch_stage.rs:377-398`; diff-only code includes `patch/diff.rs` (99 production lines) and `patch/package.rs` (332)). The default change is a contract MAJOR, so it is filed as a decision. {{C25}}
 
 **Sidecars** (`patch/sidecars/`, 708 production / 1,312 test lines) are post-apply fixes for package-manager checksum files:
 - cargo: rewrite `.cargo-checksum.json`;
@@ -113,7 +113,7 @@ So diff only saves bytes when a user commits `.socket/diffs` but not `.socket/bl
 
 Maven sidecars are not handled at all. This code exists only for in-place mode.
 
-**`apply.lock`** is 554 production lines. Most of that complexity comes from *deleting* the lock file on exit: unlinking while it is held, identity checks, Windows delete-pending handling. Lock acquisition also replays the vendored group-commit journal, which couples vendored crash recovery into every command's lock. Leaving a gitignored lock file on disk (the convention every package manager uses) would cut about 150 lines.
+**`apply.lock`** is 553 production lines (re-checked on `045d7ec`). Most of that complexity comes from *deleting* the lock file on exit: unlinking while it is held, identity checks, Windows delete-pending handling. Lock acquisition also replays the vendored group-commit journal, which couples vendored crash recovery into every command's lock. Leaving a gitignored lock file on disk (the convention every package manager uses) would cut about 150 lines.
 
 **Dead path (verified):** `PatchSources::mem_blobs` is never `Some` in production, but its doc still says vendor flows stage content there. {{C23}}
 

doc/08-tests-ci-docs.md

@@ -2,7 +2,7 @@
 
 ## Part 8: Tests, CI, docs and distribution
 
-_Last checked against main @ 045d7ec on 2026-10-03 by audit-core. Owner: audit-core._ Only the repository-hygiene passages (stray `launch.json`, "DESIGN §" references) and the contract's env-var tables have been re-checked; the rest is as of `2463257`.
+_Last checked against main @ 045d7ec on 2026-10-04 by audit-core. Owner: audit-core._ Only the repository-hygiene passages (stray `launch.json`, "DESIGN §" references) the contract's env-var tables and the `#[serial]` count have been re-checked; the rest is as of `2463257`.
 
 > Scope: `crates/*/tests/**`, `tests/` (docker fixtures), `.github/workflows/*`, `.github/actions/*`, `scripts/`, `docs/`, `CLI_CONTRACT.md`, `CHANGELOG.md`, `npm/`, `crates/socket-patch-node/npm/`, and the Cargo profiles. CI timings come from the GitHub Actions run for `2463257` on `main`.
 
@@ -46,7 +46,7 @@ PR #277 has already started cleaning up: it deleted 237,608 lines, including 136
 
 **Exact human-text assertions.** 328 `.contains("…")` assertions pin sentences of four or more words, for example `"[dry-run] Would download and vendor 0 of 1 patch (1 would be refused). No changes made."`. The repo has no snapshot tooling. The output-polish PR (#248) had to touch **65 test files (4,346 lines)**.
 
-**Process-global env forces serialization.** `args.rs` mirrors flags into process env (see Part 2.5), so in-process tests carry **553 `#[serial]`** attributes and 29 test files call `set_var`. That is the main obstacle to merging binaries. CI uses no nextest, mold/lld or sccache.
+**Process-global env forces serialization.** `args.rs` mirrors flags into process env (see Part 2.5), so in-process tests carry **993 `#[serial]`** attributes (185 more in `src`, on `045d7ec`; the review counted 553) and 29 test files call `set_var`. That is the main obstacle to merging binaries. CI uses no nextest, mold/lld or sccache.
 
 ### 8.2 CI cost
 

entries/audit-core/20261004T155140Z.md

@@ -0,0 +1,42 @@
+[agent] 2026-10-04: architecture audit (CLI and core)
+
+**main @ `045d7ec`**, unchanged since the previous run. There were no new maintainer comments in the discussion, and no handovers addressed to `audit-core`.
+
+**Reconciled:** C24's first child #772 is now in [PR #774](https://github.com/SocketDev/socket-patch/pull/774), and tracking issue #771 stays open. All other rows hold their statuses: C03 #568, C04 in PR #617, C05 #615, C07 #648, C08 #649, C09/C39 #647, C14 #704, C15 #676/#677, C16 #675, C17 #706, C18 #705, C19 #727, C20 #748/#747, C21 #728, C22 #770, C23 #746, C37 in PR #607, C38 #614, C40 #678, C41 #707, C42 #726, C43 #745 and C44 #773. `main` hasn't moved, so I didn't re-check open issues against it.
+
+**Backlog verified and decomposed:**
+- **C25 → decision [#792](https://github.com/SocketDev/socket-patch/issues/792)**: make `--download-mode file` the default and retire the diff path. Re-verified on `045d7ec`: on a cold cache, the top-up at `fetch_stage.rs:377-398` sets `blob_scope = manifest` whenever any archive was missing, so diff mode fetches every archive *and* every blob. The diff-only code is `patch/diff.rs` (99 production lines), `patch/package.rs` (332), the diff branches of `blob_fetcher`/`fetch_stage`/`repair`, `AppliedVia::Diff` and `qbsdiff`. Changing the default is a contract MAJOR, so it is a decision. The options are A (make `file` the default, then delete), B (keep diff, but scope the blob top-up) and C (no change).
+- **C10 → tracking [#793](https://github.com/SocketDev/socket-patch/issues/793), first child [#794](https://github.com/SocketDev/socket-patch/issues/794)** (linked as a sub-issue). Verified:
+  - `apply_env_toggles` is still called separately in 10 command entry points; `vendor --check` and `hosted-bundle` skip it, and the update notifier is spawned before it;
+  - the lock timeout is converted by hand 12 times;
+  - 13 production sites build the API client;
+  - `#[serial]` now appears 993 times in `tests/` plus 185 in `src` (the review counted 553 + 182).
+
+  #794 mirrors the flags once in `main` and adds `GlobalArgs::lock_timeout()`. Children 2–5 are listed in #793: a typed core `RunConfig`, a shared client, project paths (#745), and dropping `#[serial]`.
+- **C26 (`apply.lock`) checked, not filed:** 553 production lines (the review said 554). The deletion-on-exit machinery and the journal replay inside `acquire` are confirmed. Leaving the lock file on disk is a user-visible change, and this run's one decision went to C25, so C26's decision is due next run.
+
+**New finding: C45 → [#791](https://github.com/SocketDev/socket-patch/issues/791)** (bug). `--download-mode` is an unvalidated `String`, parsed at runtime in two places.
+- **Proof (debug CLI, run twice, empty manifest, `--offline --json`):**
+  - `--download-mode bogus`, `--download-mode package` and `SOCKET_DOWNLOAD_MODE=Bogus` fail `apply` and `repair` with exit 1 and `apply_failed`/`repair_failed`;
+  - `apply --check`, `rollback`, `list`, `vendor` and `vendor --check` exit 0;
+  - the `--vendor-source bogus` control is a clap usage error (exit 2).
+- **Honest scope:** the review already noted the unvalidated `String` (Part 2.7) inside the MAJOR R8 bundle. This finding adds the execution proof and the exit-code and error-code split, and carves out the non-breaking fix, a typed clap parser. A comment on #791 corrects its Source line to say so.
+
+**Searched without filing:**
+- **Unlocked readers of the vendored state:** `list`, `vex` and `vendor --check` take no `apply.lock`, so they never replay an interrupted group-commit journal and can read a torn state (lockfiles new, ledger old). `vex` cross-checks the lock wiring (`vex_sources`), which limits the risk. I didn't prove a wrong output by execution, so it isn't filed; it is a candidate for the C26 decision.
+- **Production `ApiClient::new` bypasses:** only `hosted-bundle`, which is internal; the `scan/hosted/vlt.rs` and `discovery.rs` hits are test-only.
+- **Client rebuilds inside one command:** `rollback` reuses its telemetry client for blob downloads, and `repair` caches its client. No duplicate org auto-resolve was found.
+- **Lock-timeout handling:** uniform across sites (a C10 duplication, not a bug).
+
+**False positives ruled out:** `vendor --check` skipping `apply_env_toggles` has no proven effect, because the path does no network, telemetry or debug-gated work. It is folded into #794 as a structural fix.
+
+**Living document:**
+- Part 2: 2.5 has the corrected `#[serial]` counts and a {{C10}} token; 2.7 has the `--download-mode` proof and {{C45}}; {{C45}} is under new findings.
+- Part 7: the diff-download passage is re-verified with {{C25}}, the `apply.lock` count is corrected, and the check line now covers diff download and `apply.lock`.
+- Part 8: the `#[serial]` count is corrected and the check line refreshed.
+- Summary: the Configuration row's `#[serial]` count is corrected, with {{C10}}.
+
+**Next backlog rows:** C26 (decision: keep `.socket/apply.lock` on disk; move journal replay?), C13 (typed error codes; after #704), C27 (Maven sidecars), C28 (socket.yml serde), C30 (test-support crate).
+
+---
+_Generated by [Claude Code](https://claude.ai/code)_