Skip to content

Commit f290bee

Browse files
committed
Reboot the ledger for the file-by-file review
Archive the first thread (#560) under review/2026-10-v1/, carry its 101 open register rows over with live issue states, and start a new discussion (#1830) organized by concern: mechanisms, duplication, structure, tests and history. Add coverage/files.tsv (868 .rs files) for the file-by-file pass, a {{COVERAGE}} token and a `carried` status in the renderer, and new AUDIT.md and REFACTOR.md for the audit-core, audit-cli-tests and refactor routines. Assisted-by: Claude Code:claude-opus-5-5
1 parent 8761f60 commit f290bee

175 files changed

Lines changed: 2346 additions & 770 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎AUDIT.md‎

Lines changed: 189 additions & 162 deletions
Large diffs are not rendered by default.

‎README.md‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,20 @@
11
# Architecture audit ledger
22

3-
This orphan branch holds the working state of the scheduled socket-patch architecture routines. Cloud sessions can't write to GitHub Discussions, so the routines commit here, and `.github/workflows/arch-audit-ledger.yml` runs `tools/mirror.py` on every push to mirror the branch into discussion #560. The node IDs are in `ledger.json`.
3+
This orphan branch holds the working state of the scheduled socket-patch architecture routines. Cloud sessions can't write to GitHub Discussions, so the routines commit here, and `.github/workflows/arch-audit-ledger.yml` runs `tools/mirror.py` on every push to mirror the branch into the discussion named in `ledger.json`.
44

55
| Path | What it is | Mirrored to |
66
|---|---|---|
7-
| `doc/01-summary.md` … `doc/09-appendix.md` | The **living architecture document**: the October 2026 review, kept true of current `main` by the routines | The discussion body (part 1) and the Part 2–9 comments, re-rendered on every push |
8-
| `register/*.md` | The defect register. Each routine owns one file; `00-header.md` is maintainer-owned. | The register comment (the first comment), regenerated on every push |
9-
| `entries/<slug>/<UTC timestamp>.md` | One file per routine run | A new discussion comment for each file added |
10-
| `review/2026-10/` | The original review snapshot (read-only) | — |
7+
| `doc/01-summary.md` … `doc/06-history.md` | The living document, organized by concern: mechanisms, duplication, structure, tests, history | The discussion body and its Part 2–6 comments, re-rendered on every push |
8+
| `register/*.md` | The defect register. `00-header.md` is maintainer-owned; each routine owns one file; `50-`/`60-carried-*` hold the open items carried over from #560 | The register comment (the first comment) |
9+
| `coverage/files.tsv` | The file-by-file manifest: every `.rs` file, who audits it, when it was audited and swept, and its local cleanup notes | The `{{COVERAGE}}` line |
10+
| `entries/<slug>/<UTC timestamp>.md` | One file per routine run: the log | A new discussion comment for each file added |
1111
| `AUDIT.md`, `REFACTOR.md` | The procedures the routines read at the start of every run | — |
12+
| `review/2026-10/` | The original October review snapshot (read-only) | — |
13+
| `review/2026-10-v1/` | The first thread (#560): living document, register, procedures and entries, archived 2026-10-29 (read-only) | — |
1214
| `tools/mirror.py` | The renderer and mirror, which the workflow runs | — |
1315

14-
In `doc/` and `register/`, status tokens such as `{{E01}}`, `{{E07-E20}}` and `{{PROGRESS}}` render the live status of register rows. That way the document never carries a stale status. To preview locally, run `python3 tools/mirror.py --worktree --dry-run --out /tmp/render` from the branch root; it needs `gh` auth to read the current posts.
15-
1616
The routines:
17-
- `audit-ecosystems` and `audit-core` run every 6 hours. They verify problems, file `arch-audit` issues and keep their parts of the living document current.
18-
- `refactor` runs every hour. It implements the highest-leverage refactoring as a PR against `main`.
17+
- `audit-core` (core crate sources) and `audit-cli-tests` (the CLI crate and all tests) run every 6 hours, staggered. They read the code file by file, never change it, and file `arch-audit` issues.
18+
- `refactor` runs every 2 hours. It alternates between sweep PRs (applying the auditors' per-file cleanups) and structural PRs (the highest-leverage register item).
1919

20-
Edit `AUDIT.md` or `REFACTOR.md` to retune them. Only add or change the files you own, and never rewrite or force-push this branch.
20+
Edit `AUDIT.md` or `REFACTOR.md` to retune them. Only add or change the files you own, and never rewrite or force-push this branch. To preview the rendering locally, run `python3 tools/mirror.py --worktree --dry-run --out /tmp/render` from the branch root.

‎REFACTOR.md‎

Lines changed: 127 additions & 148 deletions
Large diffs are not rendered by default.

‎coverage/files.tsv‎

Lines changed: 869 additions & 0 deletions
Large diffs are not rendered by default.

‎doc/01-summary.md‎

Lines changed: 38 additions & 348 deletions
Large diffs are not rendered by default.

‎doc/02-mechanisms.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
## Part 2
2+
3+
_Not yet checked._
4+
5+
Common mechanisms:For each cross-cutting concern: the **canonical mechanism** (the one way the code should do it, with a permalink to its home), the **deviations** found (file, what it does instead, its register token), and a count. Concerns: error types and context; error codes and exit codes; swallowed errors and panics in non-test code; stdout/stderr and the `ui` layer; the JSON envelope; human text, color and progress; logging; env and config reading; paths and containment; file I/O (atomic writes, BOM/CRLF); HTTP clients, timeouts and retries; process spawning and deadlines; concurrency.
6+
7+
### Findings
8+
_None yet._

‎doc/03-duplication.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
## Part 3
2+
3+
_Not yet checked._
4+
5+
Duplicated business logic:One bullet per rule that has more than one implementation: the copies (permalinks), whether they have drifted, the register token, and the target home. Rules carried over from the first thread stay in the carried registers until reconciled.
6+
7+
### Findings
8+
_None yet._

‎doc/04-structure.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
## Part 4
2+
3+
_Not yet checked._
4+
5+
Structure, abstraction and placement:Needless layers (one-impl traits, forwarding modules, invariant-free newtypes), tiny-function clusters, unclear control flow (mode flags, giant functions, accumulate-then-decide), and misplaced code (with its destination). Per-directory tallies of single-caller functions, wrappers and deletable comment lines go here as the audit covers each directory.
6+
7+
### Findings
8+
_None yet._

‎doc/05-tests.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
## Part 5
2+
3+
_Not yet checked._
4+
5+
Tests:Fake tests (asserting on their own mocks, recomputing expected values with the code under test, unable to fail, claiming more than they check), mock overuse where a tempdir or a real install is cheap, sentence-matching assertions, duplicate tests, and real coverage gaps. Per-directory tallies and the fixes that landed.
6+
7+
### Findings
8+
_None yet._

‎doc/06-history.md‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
## Part 6: History — what carried over from #560
2+
3+
The first thread, [#560](https://github.com/SocketDev/socket-patch/discussions/560) (2026-10-01 to 2026-10-09, 151 comments), reviewed the code module by module. Its living document, register, procedures and run entries are archived read-only under `review/2026-10-v1/` on the ledger branch, and the original snapshot under `review/2026-10/`.
4+
5+
**Carried over:** every row that wasn't fixed, already fixed, rejected or handed off. These are 54 ecosystem and format rows (`E`) and 47 CLI, core and test rows (`C`), with their issue links. Each one is `carried` while one of its issues or PRs is still open, and `to verify` when none is. The auditors reconcile a few each run against current `main`, and an item that no longer fits this charter is rejected with a reason.
6+
- Ecosystems and formats: {{E04,E05,E10,E12,E14,E15,E16,E19,E20,E21,E22,E24,E26,E28,E29,E30,E31,E32,E33,E36,E37,E40,E41,E42,E43,E44,E45,E46,E47,E50,E52,E58,E63,E64,E65,E68,E69,E71,E72,E75,E79,E80,E81,E82,E83,E84,E86,E87,E90,E91,E93,E94,E95,E96}}
7+
- CLI, core and tests: {{C05,C09,C10,C11,C12,C13,C15,C16,C17,C18,C19,C20,C23,C24,C25,C26,C29,C30,C31,C32,C33,C34,C38,C41,C43,C45,C47,C48,C50,C51,C54,C56,C59,C60,C61,C67,C68,C69,C70,C71,C72,C73,C74,C75,C76,C78,C79}}
8+
9+
**What #560 concluded** (its TL;DR as of 2026-10-09; the numbers are from then):
10+
11+
12+
1. **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:
13+
- no lockfile codec;
14+
- no mode backend;
15+
- no vendor-backend trait;
16+
- no hosted-rewriter trait;
17+
- no single inventory.
18+
19+
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.
20+
21+
2. **Most of the open bug backlog has one shape: "reported success, but the build consumes unpatched code".** Of the 88 issues open at the snapshot, roughly 29 were a success or a `not_affected` VEX attestation that the installed bytes don't back up (about 46 such false attestations are open on 2026-10-07; #1033 and #940 address the largest groups). Another 15 were discovery missing what the package manager actually installed. The root cause is architectural:
22+
- the tool **re-implements package-manager behavior** (install layouts, venv naming, config layering, resolution precedence);
23+
- it then **reports success based on its own model of what the package manager will do**;
24+
- every package-manager release or config knob it doesn't model becomes a silent false negative.
25+
26+
3. **Support breadth has outrun the architecture.** The long tail is disproportionately expensive:
27+
28+
| Long-tail target | Cost |
29+
|---|---|
30+
| vlt | ~6.5K production lines, ~22K total, ~123 CI jobs per push |
31+
| JVM (Maven/Gradle) | ~9K production lines, 22 of the 88 open bugs |
32+
| `bun.lockb` | ~2.8K production lines for a format Bun itself replaced |
33+
| vendored pnpm 7/8 | 1.85K lines, ~96% a copy of the v9 backend |
34+
35+
**A support-tier policy would let the core get simpler.**
36+
37+
4. **The code is large for what it does, and much of the size is duplication and scaffolding.**
38+
- ~134K lines of production code on `9c43dfc` (~118K at the snapshot, plus 35K comment lines then).
39+
- ~450K lines of tests.
40+
- Nine functions over 500 lines; `run_scan` alone is 1,540 (on `045d7ec`).
41+
- A 21.9K-line `redirect/mod.rs` on `db83f01` (17.5K at the snapshot).
42+
- Two hosted orchestrators kept equal by parity tests.
43+
- Four discovery systems.
44+
- Nine different revert mechanisms.
45+
- Two JSON envelope shapes, 27 global flags silently accepted by every command, and 156 documented error codes.
46+
47+
**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.
48+
49+
5. **The user model is harder than it needs to be.** It has:
50+
- 9 verbs, 2 hidden subcommands, 2 aliases and 3 hidden flag spellings;
51+
- defaults that change with unrelated flags;
52+
- `scan` writing lockfiles by default;
53+
- mode that is not project state: a plain `scan` performs the documented "mode takeover" and switches vendored npm, Cargo, Go, PyPI and Gradle-built Maven packages back to hosted (made atomic in #1039, E70);
54+
- `remove`, `rollback` and `vendor --revert` as three ways to undo.
55+
56+
A seven-verb model (`scan` read-only, `fix`, `undo`, `sync`, `check`, `list`, `vex`) with mode inferred from the project would cover everything (§4).
57+
58+
6. **There are a few real defects to fix now** (§1), regardless of any refactor:
59+
- no HTTP timeouts on the main API client (fixed, #581);
60+
- a ledger-loss bug in the vendored→hosted takeover (fixed, #708);
61+
- a planted-binary spawn (fixed, #617);
62+
- a comment-blind NuGet config reader (fixed, #597);
63+
- a stale digest ratchet that turned `main` red for every PR (fixed, #1016; a merge queue follows in #1018).
64+
65+
The October 7 audit added credential leaks (C59), `.socket` containment (C60) and the non-atomic takeover (E70); each has a PR. `SOCKET_FORCE` (#615, PR #1021) and the `bun.lockb` registry override (E02, fixed by #574) are no longer open defects.
66+
67+
---
68+

0 commit comments

Comments
 (0)