Skip to content

Stage prebuilt service archives through one shared helper instead of four per-backend service-copy pipelines #906

Description

[agent] Filed by the scheduled architecture audit routine (ecosystems and formats). Register: discussion #560 register.

Kind: refactor. Source: review Part 5.4 / 5.8 (per-backend copies), register E25.

Problem

On main @ 9c43dfc, four vendored backends each implement the same "materialise the copy from the patch service" pipeline:

  1. Refuse when the service is required but has no client.
  2. Fetch, then policy.settle(fetched, noun, subject, warnings).
  3. Stage a sibling dir; take a prestage tree, or extract_on_blocking_pool(bytes, &stage, <extractor>).
  4. Check that every recorded patched file landed at its path ("… extracted to an unexpected layout (patched files absent at their recorded paths)", spelled four times).
  5. Run the ecosystem's post-step.
  6. swap_stage_into_place.
  7. On failure, clean up.

The four copies:

diff of the cargo and composer copies differs only in:

  • type names and nouns ("crate" / "dist zip");
  • the extractor (extract_tgz / extract_dist_zip);
  • the post-step (cargo deletes .cargo-checksum.json and tags the version);
  • comment wording.

The failure cleanup is copied too:

No drift is proven yet. Any fix to the stage, verify or swap order (for example a new layout check) has to be made four times.

The review also listed service_preflight_names_exactly_*, which is still copied seven times (cargo, composer, gem, golang, maven, nuget, pypi). Its body already shares test_support::plan_matches_grants, so only the fixture and the closure remain per backend. That part is no longer worth a refactor and is out of scope here.

Symptoms

None filed. Impact: low risk, moderate size. About 500 production lines carry one policy.

Proposed change

  • Add vendor::service_fetch::stage_prebuilt(spec: PrebuiltSpec, …) -> ServiceAttempt<T>:
    • PrebuiltSpec carries noun, subject, extract: fn(&[u8], &Path) -> io::Result<()>, an optional post_stage: FnOnce(&Path) -> Result<T, String> hook, and the CleanupShape (unwind uuid dir; keep wired copy).
    • The helper owns steps 1–7 above, including the single layout-mismatch message.
  • Each backend keeps only its spec and its post-stage hook:
    • cargo: checksum drop and version tag;
    • gem: stub-gemspec artifact;
    • go: replace wiring.
  • Delete: both cleanup_failed_stage copies, composer's prune_empty_vendor_dirs / cleanup_failed_stage, go's cleanup_failed_service_stage, and the four inline stage/extract/verify/swap blocks.

Size and scope

  • PR 1: cargo + composer (mechanical, about −120 net). PR 2: gem + go (the hooks).
  • Each PR stays under ~400 changed production lines, in vendor/{service_fetch,cargo,composer_lock,gem,golang}.rs.
  • Out of scope: npm-family and PyPI service legs (different staging), the Maven/NuGet vend_installed! path, and the preflight test copies.

Acceptance criteria

  • One stage → verify → swap implementation; grep -rn "patched files absent at their recorded paths" crates/socket-patch-core/src hits one production site.
  • No cleanup_failed_stage left in a backend file.
  • Green, unchanged:
    • the existing service tests of each backend (service_*, failed_service_rebuild_of_*, first_run_service_extract_failure_*, offline_service_mode_*);
    • the four service_preflight_names_exactly_* tests;
    • prestage claim tests.
  • Add one table-driven test of the shared helper: a layout mismatch, an extract failure with and without a pre-existing wired copy, and the prestage move.

Dependencies

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    agent:triagedarch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)priority:p3refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions