Skip to content

Decide: keep the --update binary swap, or replace it with the installer and keep only the update notifier #983

Description

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

Kind: decision. Source: review §5 ("Self-update binary swap"), §6 Q2, 7.5 and recommendation 15; register C36 (the self-update half; the agent-mode half is a separate row).

Question

Should socket-patch --update keep replacing its own binary, or should it hand the upgrade to the installer and keep only the passive notifier?

Options:

  1. Keep the swap (recommended), with no product change. It is the only update path for the Windows standalone zip, and it already uses the installer's trust model. Engineering follow-ups that don't change behavior go through the existing rows: its two private HTTP clients and 300 s whole-download budget fold into the shared retry and timeout primitive (Tracking: one retry primitive for the patch API client (JSON, vendor service, blob and diff) #676), and the SOCKET_FORCE sharing stays Decide: give SOCKET_FORCE per-command names so forcing a self-update doesn't also force apply and vendor #615.
  2. Replace the swap with an installer hint. --update would print (or, with --yes, run) curl -fsSL https://install.socket.dev/patch | sh, honoring the SOCKET_PATCH_VERSION pin. That deletes update/download.rs, update/swap.rs, the update lock and most of commands/update.rs: about −1.1K production lines and −1.5K test lines. It's a contract MAJOR (the self-update section, its error codes, the --update --dry-run probe). Windows standalone users lose their only updater, because there is no PowerShell installer: the README tells them to extract the zip by hand.
  3. Hybrid: delegate to install.sh on Unix and keep the swap only for Windows. This keeps every piece of the swap machinery for one platform and adds a second path, so it saves the least.

Whatever is chosen, the notifier stays: the review and this issue agree it's cheap and channel-aware.

Problem (main @ 9c43dfc)

Self-update is 2,350 production lines (unchanged since the review's 2,351), plus 2,369 inline and 2,634 external test lines:

What it does, against the installer:

  • Only InstallChannel::Standalone may swap; npm, Cargo and Homebrew get their own upgrade command, and the pre-v5 PyPI and gem locations get a migration hint.
  • Release resolution has two strategies: a releases/latest redirect probe, then a GitHub API JSON fallback (fetch_latest_version). install.sh needs neither, because it downloads from latest/download/.
  • Download, SHA256SUMS check, stage, sanity-exec and atomic rename run under a separate lock (perform_update). install.sh does the same download and checksum steps, then install -m 755. The trust model is the same (HTTPS + GitHub, unsigned checksums), as docs/installer-hosting.md says.
  • The asset name comes from the compiled target triple (asset_name_for_target), so a musl binary updates to musl. install.sh re-detects libc with ldd (L55-L70), and its platform table has no Windows rows.
  • Windows: the README says to extract the zip by hand and then use socket-patch --update. swap.rs has its own #[cfg(windows)] path.
  • Two private reqwest clients (download_client, metadata_client) with whole-request budgets (30 s metadata, 300 s download) and no retry.

Symptoms

No open bugs. #128, #140 and #171 were the swap's own flake and hardening fixes.

Impact

This is a product and maintenance trade-off, not a defect. Option 2 removes the code that's the most expensive to test (exec sanity checks, ETXTBSY retries, Windows rename), but it costs Windows users and changes a documented contract.

Proposed change (after the decision)

Size and scope

Option 2: about −1.1K production and −1.5K test lines in update/, commands/update.rs, the self_update_* and tests/update/ suites, CLI_CONTRACT.md and the README. The notifier and channel detection are out of scope.

Acceptance criteria

  • An owner picks an option.
  • If 2 or 3: CLI_CONTRACT.md documents the new --update behavior with a MAJOR note, the README's install section matches, and update_notifier_e2e stays green.
  • If 1: the living document's §5 and 7.5 rows record the decision.

Dependencies

Activity

  1. added
    arch-auditFiled by a scheduled architecture audit routine (see the architecture review discussion)
    refactorStructural change: duplicated code or logic, missing abstraction, layering, dead code
    on Oct 7, 2026
  2. mikolalysenko commented on Oct 7, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Triaged as priority:p3. This is a product decision (keep the self-update swap, replace it, or go hybrid), so it stays agent:needs-human and isn't eligible for an agent claim until a maintainer chooses an option. Related follow-ups: #676 and #615.


    Generated by Claude Code

  3. mikolalysenko commented on Oct 7, 2026

    @mikolalysenko
    CollaboratorAuthor

    Self update is a great feature and arguably more essential than the install.socket.dev pathway for users who already running socket-patch. We must keep this working and continue to improve it.

  4. mikolalysenko commented on Oct 7, 2026

    @mikolalysenko
    CollaboratorAuthor

    [agent] Decision recorded: keep the --update binary swap and the update notifier (option 1). Options 2 and 3 are rejected. Self-update is a supported, first-class update path. It is the only updater Windows standalone users have, and per the maintainer it matters more than install.socket.dev for people who already run socket-patch.

    What this means

    • No behavior or contract change. The CLI_CONTRACT.md "Self-update contract" section, its error codes and the --update --dry-run probe stay as they are. The repo docs (README, docs/installer-hosting.md, docs/releasing.md) already present --update as the standalone update path and have no "may be removed" wording, so no docs PR is needed.
    • The architecture audit's suggestion to "replace the swap with re-run install.sh" is withdrawn. The living document (§3 recommendation 15, §5 support tiers, §7.5 and §7.6 Use patches-api.socket.dev for public patch data #10) will be updated to say the swap stays and is being improved, and register row C36 is closed as decided.

    Improvements to file as follow-ups (main @ db83f01)

    1. Retry transient failures. The two private clients (download_client, crates/socket-patch-core/src/update/download.rs:59; metadata_client, release.rs:262) make one attempt each. A single 502 or connection reset from GitHub or the CDN fails the update. Add retry with Retry-After and jitter to the redirect probe, the API fallback, SHA256SUMS and the archive download, reusing the api::retry primitive from Tracking: one retry primitive for the patch API client (JSON, vendor service, blob and diff) #676. Tracking: one retry primitive for the patch API client (JSON, vendor service, blob and diff) #676 currently lists self-update as out of scope; this will be a sibling child, or Tracking: one retry primitive for the patch API client (JSON, vendor service, blob and diff) #676's scope will be widened.
    2. Stall timeout instead of a whole-download budget. The archive download has a 300 s whole-request budget (release.rs:114), so an update over a slow link fails while bytes are still arriving. This is the same class of bug as Registry downloads give up after 60 s even while the body is still arriving #872. Switch to a connect timeout plus an idle (no bytes received) timeout, keeping SOCKET_UPDATE_TIMEOUT_MS.
    3. Stream the archive to disk. fetch_archive buffers up to 256 MiB in memory (download.rs:98, read_capped) before verifying it. Instead, stream it to the stage directory, hashing as it goes, the way blob downloads already do (C37). Verify-before-extract ordering is unchanged.
    4. Live post-release test. The only test against real GitHub releases is an ignored --dry-run smoke (crates/socket-patch-cli/tests/self_update_e2e.rs:339). PR coverage uses a wiremock fixture. Add a post-release job: install the previous release on Linux, macOS and Windows, run a real --update to the new tag, and check --version. That catches asset-naming, redirect and SHA256SUMS drift in the real published pipeline.
    5. SOCKET_FORCE split: still Decide: give SOCKET_FORCE per-command names so forcing a self-update doesn't also force apply and vendor #615, so forcing a self-update stops also forcing apply and vendor.
    6. Shared spawn deadline: the hand-rolled 10 s timeout in sanity_exec (download.rs:253) moves onto the shared child-process deadline when C48 lands. No behavior change.

    Closing this decision issue. No PR is needed for the decision itself, since nothing in the repo changes. The improvements above will be filed as their own issues.


    Generated by Claude Code

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