Skip to content

Commit c65b4cf

Browse files
committed
refactor: A27 in PR #1870
Assisted-by: Claude Code:claude-opus-5-5
1 parent 90acc15 commit c65b4cf

3 files changed

Lines changed: 46 additions & 9 deletions

File tree

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
[agent] 2026-10-30: architecture refactor (structural)
2+
3+
`main` @ `5ca5623`. The previous run was a sweep, so this run was structural. A27 [#1869](https://github.com/SocketDev/socket-patch/issues/1869) scored about 7.5 (D2.75 C2, risk M), and no open PR touches `api/client.rs`. The other candidates were blocked or scored lower:
4+
- T09 [#1858](https://github.com/SocketDev/socket-patch/issues/1858), about 7, is blocked by #1864 and #1698.
5+
- A02 is now the fixer's #1866.
6+
- T01 is blocked by #1698.
7+
8+
There was no maintainer steering, and no PR of mine merged or closed.
9+
10+
**PR:** [#1870](https://github.com/SocketDev/socket-patch/pull/1870), ready for review. CI and Bugbot are pending.
11+
12+
### What changed (`api/client.rs`)
13+
- `serve_get_once(url, max_bytes, noun)` replaces the two serve GETs (`download_vendor_archive_once`, `download_artifact_capped_once`).
14+
- `retry_vendor` replaces the three hand-written vendor retry loops. It also takes a split download's deferred first attempt, so a split still spends one budget.
15+
- **Deleted:** `download_vendor_archive_retrying`, `download_vendor_archive_from`, `vendor_download_retry`, `download_artifact_capped`, `download_artifact_capped_once`, `CappedAttempt` and `ServeDownload::Failed`.
16+
- **Error text unified, as the issue asked:**
17+
- A cap breach is now `Other("<noun> exceeds the N-byte limit")`.
18+
- Status errors go through `status_error`.
19+
- The error body is read under the caller's cap. /code-review caught that the first version read it uncapped on the `PatchApi` path.
20+
- **Policy values unchanged.** The `Retry-After`-over-cap disagreement is left for a follow-up.
21+
- **Tests:**
22+
- 1 added (`trait_download_error_body_is_capped`, shown red→green).
23+
- 1 tightened (`cap_breach_is_not_retried` now asserts the exact error).
24+
- 1 renamed and re-pointed (`every_vendor_retry_path_…`).
25+
- The merged loop was shown red by ignoring `deferred`: 2 tests failed.
26+
- **Net lines:** production +119 / −238 (−119), tests +36 / −6.
27+
28+
### Merges recorded
29+
None.
30+
31+
### Lessons
32+
- Grep a new helper's name first: `with_vendor_retry` is already the public builder.
33+
- When two code paths are merged, keep each caller's resource bounds.
34+
- The core lib suite has 4 root-only failures in the sandbox (`copy_tree`, `vlt_heal`, `pypi_poetry`, `pypi_requirements`).

‎register/10-audit-core.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ _Last updated 2026-10-30T00:51Z · main @ 5ca5623 · owner: audit-core_
2929
| A24 | 3 | "Is this our earlier hosted pin" ×2: pdm/poetry `python_lock::is_prior_hosted_url` (origin + last segment) vs uv/pylock `python_script::same_hosted_artifact` (uuid segment + `replaceable_hosted_pin`). | | to verify |
3030
| A25 | 3 | PEP 508 leading-name scanner ×3 (`vendor/common.rs` L602, `requirements.rs` L438, inline `python_lock.rs` L665, which skips `trim_start`); `canonicalize_pypi_name` still in `crawlers::python_crawler` (#883 closed). | | to verify |
3131
| A26 | 3 | Civil-date math lives in three modules: `days_from_civil` twice (`api/date.rs` L48, `formats/cargo/floor.rs` L26) and its inverse in `vex/time.rs`, whose doc claims to be the only copy. Target: one `utils::time`. | #1867 | filed #1867 |
32-
| A27 | 2 | `api/client.rs` GETs a serve URL through two near-copies (`download_vendor_archive_once`, `download_artifact_capped_once`; drifted cap-breach variant and status text), hand-writes 4 retry loops, and runs two retry policies that disagree (executed: `Retry-After: 60` → JSON gives up, vendor retries after 4 s). Related: #1650. | #1869 | filed #1869 |
32+
| A27 | 2 | `api/client.rs` GETs a serve URL through two near-copies (`download_vendor_archive_once`, `download_artifact_capped_once`; drifted cap-breach variant and status text), hand-writes 4 retry loops, and runs two retry policies that disagree (executed: `Retry-After: 60` → JSON gives up, vendor retries after 4 s). Related: #1650. | #1869 | in PR #1870 |
3333
| A28 | 3 | `api/` is not a leaf: `vendor_prefetch` (the vendor-service plan engine) imports `vendor::prestage`/`registry_fetch`/`lock_inventory` and belongs in `vendor/`; `client.rs` carries the pypi wheel gate (`PYPI_NOT_A_WHEEL`, `wheel_filename_from_url`); `api::types` imports `patch::redirect::{Integrity, RegistryOverride}`. Overlaps #833. | #833 | to verify |
3434
| A29 | 3 | Four xorshift64* test RNGs (`test_rng` claims to be the only one; `oracle_support`, `copy_tree`, npm oracle). Eight `crawlers/**/oracle.rs` (2,978 lines with `oracle_support`) keep pre-migration crawlers as oracles; cargo's already reads differently from production. | #1868 | filed #1868; oracle deletion still to decide |
3535

‎register/90-refactor.md‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
### Refactor routine
2-
_Last updated 2026-10-29T23:25Z · main @ 5ca5623_
2+
_Last updated 2026-10-30T01:50Z · main @ 5ca5623_
33

44
#### In flight
55
| PR | Kind | Scope | State |
@@ -8,21 +8,22 @@ _Last updated 2026-10-29T23:25Z · main @ 5ca5623_
88
| [#1855](https://github.com/SocketDev/socket-patch/pull/1855) | sweep | `cli/src/commands/{bun_preflight,composer_hints,context,fetch_stage}.rs` | ready; Bugbot clean; CI lost Depot runners twice (0 steps), needs a fresh run |
99
| [#1849](https://github.com/SocketDev/socket-patch/pull/1849) | structural | A01 [#1832](https://github.com/SocketDev/socket-patch/issues/1832): `utils/cargo_build_cache.rs` | ready; CI and Bugbot green |
1010
| [#1864](https://github.com/SocketDev/socket-patch/pull/1864) | sweep | `cli/src/commands/{hosted_bundle,hosted_unwind,list,lock_cli,mod,pypi_reinstall}.rs` (+ dead branch in `rollback.rs`) | ready; CI (82 checks) and Bugbot green |
11+
| [#1870](https://github.com/SocketDev/socket-patch/pull/1870) | structural | A27 [#1869](https://github.com/SocketDev/socket-patch/issues/1869): `api/client.rs`, one serve GET and one vendor retry loop | ready; CI and Bugbot pending |
1112

1213
#### Merged
1314
_None._
1415

1516
#### Queue
1617
Structural candidates, scored `3·B + 2·U + 2·D + 2·C − risk`:
17-
1. A02 [#1833](https://github.com/SocketDev/socket-patch/issues/1833), three segment glob matchers (bug): B1 D1 C1, risk L. **7** (blocked while #1829 touches `workspace_globs.rs` / `npm_crawler.rs`)
18-
2. T01 [#1835](https://github.com/SocketDev/socket-patch/issues/1835), `get <uuid>` re-implements `download_and_apply_patches_into`: D2.2 C2, risk M. **~6.4** (blocked while #1698 touches `get.rs` / `agent_download.rs`)
19-
3. A03 [#1834](https://github.com/SocketDev/socket-patch/issues/1834), GOMODCACHE rule twice: D1 C1, risk L. **4**
20-
4. T03 [#1837](https://github.com/SocketDev/socket-patch/issues/1837), hand-written env var lists: D1 C1, risk L. **4**
21-
5. T16 [#1861](https://github.com/SocketDev/socket-patch/issues/1861), remove/rollback `VendorRevertStep` loops: D1.75 C2, risk M. **~5.5** (files free)
18+
1. T09 [#1858](https://github.com/SocketDev/socket-patch/issues/1858), lock acquire open-coded 9×: D2.6 C2, risk M. **~7** (blocked while #1864 touches `lock_cli.rs` and #1698 touches `vendor.rs` / `get.rs` / `agent_download.rs`)
19+
2. T01 [#1835](https://github.com/SocketDev/socket-patch/issues/1835), `get <uuid>` re-implements `download_and_apply_patches_into`: D2.2 C2, risk M. **~6.4** (blocked by #1698)
20+
3. T16 [#1861](https://github.com/SocketDev/socket-patch/issues/1861), remove/rollback `VendorRevertStep` loops: D1.75 C2, risk M. **~5.5** (files free)
21+
4. A03 [#1834](https://github.com/SocketDev/socket-patch/issues/1834), GOMODCACHE rule twice: D1 C1, risk L. **4**
22+
5. A26 [#1867](https://github.com/SocketDev/socket-patch/issues/1867), civil-date math in three modules: D1 C1, risk L. **4**
2223

23-
A03/T03 stay at 4, and A05 [#1844](https://github.com/SocketDev/socket-patch/issues/1844) at ~2.
24+
A02 [#1833](https://github.com/SocketDev/socket-patch/issues/1833) moved to the fixer's #1866. T03 [#1837](https://github.com/SocketDev/socket-patch/issues/1837) stays at 4, A05 [#1844](https://github.com/SocketDev/socket-patch/issues/1844) at ~2.
2425

25-
The next run is structural if a candidate reaches 6; otherwise it sweeps. Next sweep files, in order: `cli/commands/remove.rs`, `repair.rs`, `rollback.rs`, `scan/discovery.rs` (check against #1856), then `core/constants.rs`, `core/lib.rs`, `core/utils/cargo_home.rs`, `core/utils/cargo_workspace.rs` (skip `cargo_build_cache.rs` while #1849 is open). Skip `agent_download.rs`, `apply.rs`, `get.rs` and `vendor.rs` while #1698 is open.
26+
The next run is a sweep (this one was structural). Next sweep files, in order: `cli/commands/remove.rs`, `repair.rs`, `rollback.rs`, `scan/discovery.rs` (check against #1856), then `core/constants.rs`, `core/lib.rs`, `core/utils/cargo_home.rs`, `core/utils/cargo_workspace.rs` (skip `cargo_build_cache.rs` while #1849 is open, and `api/client.rs` while #1870 is open). Skip `agent_download.rs`, `apply.rs`, `get.rs` and `vendor.rs` while #1698 is open.
2627

2728
#### Notes
2829
- The PRs of the first thread (`arch-refactor/<issue>-*`) are listed in `review/2026-10-v1/register/90-refactor.md`.
@@ -32,3 +33,5 @@ The next run is structural if a candidate reaches 6; otherwise it sweeps. Next s
3233
- `vendor/cargo_config.rs` (~L165-L190) keeps a third, async copy of the `.cargo/config` pick, with canonical dedup. It agrees on precedence; fold it into `config_layer_paths` when that file is swept.
3334
- #1698 also touches `vendor.rs`, so the three `bun_vendor_preflight*` entry points (callers in `get.rs`, `agent_download.rs`, `vendor.rs`) and `StageOutcome::Unavailable`'s payload (caller `apply.rs`) wait until it merges. Check every caller file against open PRs, not only the swept file.
3435
- Build with `CARGO_PROFILE_DEV_DEBUG=0 CARGO_INCREMENTAL=0`: a default debug build of the CLI tests exhausts the session disk allowance. CI's clippy is `--workspace --all-features` without `--tests`; the integration tests carry pre-existing lints.
36+
- `ApiClient::with_vendor_retry` is the policy builder; a new private helper with that name collides (E0592), and `cargo build` of the lib alone doesn't show it once the test build is the first to compile the clash. Grep a name before you introduce it.
37+
- A shared GET must keep every caller's bounds: folding an uncapped `resp.text()` error read into the capped `PatchApi` path is what /code-review caught on #1870.

0 commit comments

Comments
 (0)