Repository navigation
socket-patch implementation review (living document, file by file): abstraction, duplication, mechanisms, tests #1830
Replies: 39 comments
Defect register[agent] This comment is the living register for this thread. It is regenerated from the Progress: 213 problems tracked · 8 partly fixed · 27 already fixed · 15 in PR · 55 filed · 4 decision pending · 30 carried · 72 to verify · 2 rejected. On GitHub: 94 open and 114 closed Coverage: 162/868 files audited (177,754 of 908,273 lines) · 48/868 swept.
Core audit (A rows)Last updated 2026-11-01T12:54Z · main @ cbbbdb9 · main @ cbbbdb9 · owner: audit-core
Handed offNone. RejectedNone. CLI and tests audit (T rows)Last updated 2026-11-01T09:51Z · main @ cbbbdb9 · owner: audit-cli-tests
Handed offNone. RejectedNone. Carried over: ecosystems and formats (E rows)Carried over from #560 on 2026-10-29 · statuses as of its last update 2026-10-09 · owner: audit-core Re-verify each row on current
Carried over: CLI, core infra and tests (C rows)Carried over from #560 on 2026-10-29 · statuses as of its last update 2026-10-09 · owner: audit-cli-tests Re-verify each row on current
Refactor routineLast updated 2026-11-01T11:15Z · main @ cbbbdb9 In flight
[truncated: the full text is on the |
Part 2Last checked against main @ cbbbdb9 on 2026-11-01 by audit-cli-tests. 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 Findings
|
Part 3Last checked against main @ cbbbdb9 on 2026-11-01 by audit-cli-tests. 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. Findings
|
Part 4Last checked against main @ cbbbdb9 on 2026-11-01 by audit-cli-tests. 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. Findings
|
Part 5Last checked against main @ cbbbdb9 on 2026-11-01 by audit-cli-tests. 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. Findings
|
Part 6: History — what carried over from #560The first thread, #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 Carried over: every row that wasn't fixed, already fixed, rejected or handed off. These are 54 ecosystem and format rows (
What #560 concluded (its TL;DR as of 2026-10-09; the numbers are from then):
|
|
[agent] 2026-10-29: architecture audit (audit-core)
Per fileCounts are single-caller functions / wrappers / deletable comment lines / fake tests.
The detailed cleanup lists are in each file's Structural findingsBoth bugs were shown by scratch tests in a throwaway worktree. No product code was changed.
Carried rows reconciled (5)
Ruled out
Next files
Generated by Claude Code |
|
[agent] 2026-10-29: architecture audit (audit-cli-tests)
Per fileCounts are single-caller functions / wrappers / deletable comment lines / fake tests.
Each file's detailed cleanup list is in its
Structural findings
Carried rows reconciled (5)Each was checked against the code on
Already covered, not re-filed
Ruled out
Next files
Generated by Claude Code |
|
[agent] 2026-10-29: architecture refactor (sweep)
PR: #1842, ready for review, with CI and Bugbot pending. args.rs
Merges recordedNone. Lessons
NextStructural work, per the alternation rule. The top candidates are A01 (#1832) and A02 (#1833), both scoring 7. |
|
[agent] 2026-10-29: handover from audit-core to audit-cli-tests Found while auditing
Generated by Claude Code |
|
[agent] 2026-10-29: architecture audit (audit-core)
Per fileCounts are single-caller functions / wrappers / deletable comment lines / fake or weak tests.
The detailed cleanup lists are in each file's Structural findingsFiled (5):
Added to an open tracker: A14, purl New rows left
Existing rows extended:
Handover: two CLI test duplicates went to audit-cli-tests. Carried rows reconciled (5)
Ruled out
Next filesAll of
That is about 12K lines. After that come Generated by Claude Code |
|
[agent] 2026-10-29: architecture refactor (structural)
PR: #1849, ready for review, with CI and Bugbot pending. What changed
Merges recordedNone. #1842 is still open. Lessons
NextA sweep: |
|
[agent] 2026-10-29: architecture refactor (sweep)
PR: #1855, ready for review, with CI and Bugbot pending. Per file
Not doneThe audit notes' collapse of the three Merges recordedNone. Lessons
|
|
[agent] 2026-10-29: architecture audit (audit-cli-tests)
Per fileCounts are single-caller functions / wrappers / deletable comment lines / fake or weak tests. Each file's cleanup list is in its
Structural findings
Carried rows reconciled (5)
Next to reconcile, from the reading:
HandoverTo
Ruled out
Next files
Generated by Claude Code |
|
[agent] 2026-10-29: handover from audit-cli-tests to audit-core Found while auditing
Generated by Claude Code |
|
[agent] 2026-10-30: architecture refactor (sweep)
So I ran a sweep, per the rule. The next audited, unswept row with free files is PR: #1892, ready for review. Per file
BehaviorNone. Merges recordedNone. Lessons
|
|
[agent] 2026-10-30: architecture refactor (sweep)
The free candidates, A20 and A26, score about 4. So I ran a sweep, per the rule. The next audited, unswept row with free files is PR: #1897, ready for review. Per file
BehaviorNone. Merges recordedNone. Lessons
|
|
[agent] 2026-11-01: architecture audit (audit-core)
Per fileCounts are single-caller fns / wrappers / deletable comment lines / fake or duplicate tests.
The detailed cleanup lists are in each row's
Structural findingsFiled (4):
New rows left
Register statuses refreshed:
Carried rows reconciled (5)
Ruled out
Next files
Generated by Claude Code |
|
[agent] 2026-11-01: architecture refactor (sweep)
So I ran a sweep, per the rule. The next audited, unswept rows with free files are PR: #1903, ready for review. Per file
BehaviorNone. Merges recordedNone. Lessons
|
|
[agent] 2026-11-01: architecture refactor (sweep)
So I ran a sweep, per the rule. PR: #1907, ready for review. Per file
BehaviorNone. Merges recordedNone. Lessons
|
|
[agent] 2026-11-01: architecture audit (audit-cli-tests)
Per fileCounts are single-caller functions / wrappers / deletable comment lines / fake or weak tests. Each file's cleanup list is in its
Structural findings
Carried rows reconciled (4)
HandoverTo Ruled out
Next files
Generated by Claude Code |
|
[agent] 2026-11-01: handover from audit-cli-tests to audit-core Found while auditing
Generated by Claude Code |
|
[agent] 2026-11-01: architecture refactor (sweep)
So I ran a sweep, per the rule. The next audited, unswept rows with free files are core PR: #1914, ready for review. Per file
BehaviorNone. Merges recordedNone. Lessons
|
|
[agent] 2026-11-01: architecture audit (audit-core)
Per fileCounts are single-caller fns / wrappers / deletable comment lines / fake or duplicate tests.
Further detail for
Structural findingsFiled (4):
New rows left Register statuses refreshed: A42 Handover from audit-cli-tests: the Carried rows reconciled (5)
Ruled out
Next files
Generated by Claude Code |
|
[agent] 2026-11-01: architecture refactor (sweep)
PR: #1920, ready for review. Per file
BehaviorNone. Merges recordedNone. Lessons
|
|
[agent] 2026-11-01: architecture refactor (sweep)
PR: #1923, ready for review. Per file
BehaviorNone, except that the npm Merges recordedNone. Lessons
|
|
[agent] 2026-11-01: architecture audit (audit-cli-tests)
The manifest needed no sync, and there was no maintainer steering. Four parallel readers did the reading. I re-checked every filed claim against Per fileCounts are single-caller functions / wrappers / deletable comment lines / fake or weak tests. Each file's cleanup list is in its
Structural findings
Carried rows reconciled (4)
Register refresh
HandoverTo Ruled out
Next files
Generated by Claude Code |
|
[agent] 2026-11-01: handover from audit-cli-tests to audit-core Found while auditing the CLI
Generated by Claude Code |
|
[agent] 2026-11-01: architecture refactor (sweep)
PR: #1929, ready for review. Per file
BehaviorNone. Merges recordedNone. Lessons
|
|
[agent] 2026-11-01: architecture audit (audit-core)
Per fileCounts are single-caller fns / wrappers / deletable comment lines / fake or duplicate tests.
Structural findingsFiled (5):
Also: commented on #1919 with the ruby New rows left Rows extended: A01 ( Register statuses: no change from GitHub; Carried rows reconciled (5)
Ruled out
Next files
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 the second architecture thread for socket-patch, and it is a living document. It replaces #560, which reviewed the code module by module and is now closed. Its open items are carried over in the register below and summarized in Part 6. Scheduled routines keep this post and its Parts true of
main, and every run posts a log entry as a comment at the end of the thread.Progress: 202 problems tracked · 8 partly fixed · 27 already fixed · 15 in PR · 46 filed · 3 decision pending · 35 carried · 66 to verify · 2 rejected. On GitHub: 89 open and 114 closed
arch-auditissues · 15 open and 79 mergedarch-refactorPRs.Coverage: 141/868 files audited (160,540 of 908,273 lines) · 48/868 swept.
arch-audit· refactor PRs labelledarch-refactor· decisions neededRendered 2026-11-01 11:26 UTC (ledger
bb72ce2a) fromdoc/on thearch-audit/ledgerbranch.socket-patch implementation review: file by file
Started 2026-10-29 against
main@b291caa.Charter
The first thread asked what to cut and combine at the level of products and modules: modes × ecosystems × lockfile formats. This thread looks at the implementation: is each file's code as small, direct and honest as it can be? Nothing in the codebase is precious. The routines look for:
How it works
coverage/files.tsvon the ledger branch lists every.rsfile undercrates/: 868 files, ~908K lines at the start.Two auditors each read about 15K lines per run, in file order, and never change code:
audit-corecoverscrates/socket-patch-core/src;audit-cli-testscovers the CLI crate and every test file.For each file they record its local cleanups (what to inline, delete or fix) in the manifest. Problems that span files go into the register and become
arch-auditissues.The
refactorroutine alternates between two kinds of work:When every file has been read once, the next pass starts with the files that changed most since they were last read.
Top problems
Filled in as the audit progresses. Each bullet carries its register token.
.cargo/configfile winsA01· in PR Read the cargo config chain once (#1832) #1849.A02· in PR Fix Cargo member globs with [..] classes (#1833) #1866.Retry-After: 60ends a JSON call but is retried after 4 s by the vendor pathA27· in PR One serve GET and one vendor retry loop in api/client.rs (#1869) #1870.--cwd .the npm crawler never reads a parent.yarnrcmodules folder: the crawlers getcwdunabsolutized and walk lexical ancestors of.(executed)A45· in PR Fix crawlers missing parent dirs under --cwd . (#1902) #1906.utils/is a mix of ecosystem-specific code and true utilities. Some of it importsvendor::*A08· to verify.A14· filed Tracking: build and classify purls through one validated utils::purl API #748; copies listed in a comment, uuid shape 4×A15· filed Validate patch UUIDs through one utils::uuid grammar instead of five #705; Check uuid shape through one helper; python_script's Uuid::parse_str also accepts braced, urn and unhyphenated forms #1845 closed as its duplicate, and the shared poetry-writer and credential-strip helpers have no caller while copies serve productionA18A20: 2 in PR.get <uuid>re-implements the agent download engineT01· filed Routeget <uuid>agent mode through download_and_apply_patches_into instead of a second copy of the engine #1835, five callers glue the Bun and vlt preflights by handT05· filed Run the Bun and vlt vendored preflights as one vendor_preflight with one per-purl refusal map #1839, remove and rollback render vendored reverts in two drifted loopsT16· filed remove and rollback each render VendorRevertStep in their own loop, and the loops drifted (remove --preserve-state --dry-run previews a full revert) #1861, andscan --prunereverts vendored entries by hand, dropping each failure's errorT37· filed scan --prune's vendored GC reverts entries by hand instead of through VendoredBackend::revert, and drops each revert's error and still-wired orphans #1910. The copies have drifted in user-visible text and JSONT02T04: 2 filed.T24· to verify, and the two emit differentredirectJSON blocks while core's doc claims one shapeT29· decision Decide: one hostedredirectJSON block for disk scan and the in-memory engine (today {mode, rewrittenFiles} vs a seven-key block) #1881.--vexembedding is written ~8× and only scan reports a skipped dry runT25· filed Embed --vex through one helper: the step is hand-written 8 times, and only scan reports vex_skipped_dry_run under --json #1877.The numbers
b291caapatch/redirect/mod.rs, 30,223 linesAll reactions