From 56de51e69bc6c08938bc87cdd871eae6e7b3d624 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 15:52:50 +0000 Subject: [PATCH 1/3] Start update slice of #1089 Assisted-by: Claude Code:claude-opus-5-5 From 01ead209a5e22912d4ed73c3f62e3d4d92295f47 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 16:04:21 +0000 Subject: [PATCH 2/3] Update tests assert codes, not sentences The self-update covgap tests pinned English sentences, so a copy edit to an update message broke tests that were really about behavior. - update_json_contracts.rs: the already-latest and pin-to-current no-ops and the unlocked swap now read the --json envelope (event actions, already_latest code, from/to details) plus disk state. - update_render.rs: the human wording (pinned already-there, both dry-run reports, the PTY reinstall decline), one check per message. - The download_failed 500 test joins its 404 sibling in self_update_failures_e2e.rs. No covgap files remain in the update suite. Refs #1089. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/self_update_failures_e2e.rs | 50 ++++ .../tests/update/covgap_update_download.rs | 71 ------ .../tests/update/covgap_update_swap.rs | 140 ---------- crates/socket-patch-cli/tests/update/main.rs | 5 +- .../tests/update/update_json_contracts.rs | 240 ++++++++++++++++++ ...ap_commands_update.rs => update_render.rs} | 114 +-------- 6 files changed, 305 insertions(+), 315 deletions(-) delete mode 100644 crates/socket-patch-cli/tests/update/covgap_update_download.rs delete mode 100644 crates/socket-patch-cli/tests/update/covgap_update_swap.rs create mode 100644 crates/socket-patch-cli/tests/update/update_json_contracts.rs rename crates/socket-patch-cli/tests/update/{covgap_commands_update.rs => update_render.rs} (66%) diff --git a/crates/socket-patch-cli/tests/self_update_failures_e2e.rs b/crates/socket-patch-cli/tests/self_update_failures_e2e.rs index 63ebf37cc..831d662b6 100644 --- a/crates/socket-patch-cli/tests/self_update_failures_e2e.rs +++ b/crates/socket-patch-cli/tests/self_update_failures_e2e.rs @@ -157,6 +157,56 @@ async fn asset_404_names_the_target() { release.verify_request_hygiene().await; } +/// A non-404 HTTP failure on the asset download (a 500 from an outage, a +/// half-published release, a WAF) aborts as `download_failed`, not +/// `asset_not_found` (reserved for 404: "no prebuilt binary for your +/// target"), names the HTTP status, and leaves the install pristine. +#[tokio::test] +async fn asset_server_error_is_download_failed() { + use wiremock::matchers::{method, path as urlpath}; + use wiremock::{Mock, ResponseTemplate}; + + let install = staged_install(); + let (served, _) = make_served_binary(); + let asset = asset_name_for_current_target(); + + // SHA256SUMS vouches for the asset (so the pipeline gets past the + // sums fetch and reaches the archive GET), but the download route + // answers 500 instead of the bytes: `omit_asset` keeps the sums entry + // while skipping the fixture's 200 mock, and the explicit mock below + // owns the asset path. + let release = FakeReleaseBuilder::new("9.9.9") + .asset_for_current_target(&served) + .omit_asset(&asset) + .mount() + .await; + Mock::given(method("GET")) + .and(urlpath(format!( + "/SocketDev/socket-patch/releases/download/v9.9.9/{asset}" + ))) + .respond_with(ResponseTemplate::new(500)) + .mount(&release.server) + .await; + + let (code, stdout, stderr) = run_installed( + &install, + &["--update", "--json"], + &[("SOCKET_UPDATE_BASE_URL", &release.base_url)], + ); + assert_eq!(code, 1, "stdout:\n{stdout}\nstderr:\n{stderr}"); + let env = common::parse_json_envelope(&stdout); + assert_eq!(common::envelope_error_code(&env), Some("download_failed")); + let message = common::envelope_error_message(&env).unwrap_or_default(); + assert!( + message.contains("500"), + "download_failed must carry the HTTP status: {message}" + ); + + install.assert_binary_intact(); + install.assert_only_binary_present(); + release.verify_request_hygiene().await; +} + /// SOCKET_UPDATE_TIMEOUT_MS must actually bound the run: a hung release /// host may not turn `--update` into an indefinite stall (a hung /// self-update is strictly worse than a hung scan). diff --git a/crates/socket-patch-cli/tests/update/covgap_update_download.rs b/crates/socket-patch-cli/tests/update/covgap_update_download.rs deleted file mode 100644 index 18cf2e018..000000000 --- a/crates/socket-patch-cli/tests/update/covgap_update_download.rs +++ /dev/null @@ -1,71 +0,0 @@ -//! Coverage-gap e2e for `update::download::fetch_archive`'s non-404 HTTP -//! failure branch: a server error on the asset -//! download must surface as `download_failed` — not `asset_not_found`, -//! which is reserved for 404 ("no prebuilt binary for your target"). -//! -//! Same discipline as `self_update_failures_e2e.rs`: every test runs a -//! staged COPY of the built binary (`update_fixture::staged_install`) — -//! `CARGO_BIN_EXE_socket-patch` itself must never be a swap target — and -//! a failed update must leave the install byte-identical with no stage -//! droppings. - -use crate::common; -use crate::update_fixture; - -use wiremock::matchers::{method, path as urlpath}; -use wiremock::{Mock, ResponseTemplate}; - -use update_fixture::{ - asset_name_for_current_target, make_served_binary, run_installed, staged_install, - FakeReleaseBuilder, -}; - -/// A CDN that answers the archive GET with a 500 (outage, half-published -/// release, WAF tantrum) aborts the update as `download_failed`, names the -/// HTTP status, and leaves the install pristine. -#[tokio::test] -async fn asset_server_error_is_download_failed() { - let install = staged_install(); - let (served, _) = make_served_binary(); - let asset = asset_name_for_current_target(); - - // SHA256SUMS vouches for the asset (so the pipeline gets past the - // sums fetch and reaches the archive GET), but the download route - // answers 500 instead of the bytes: `omit_asset` keeps the sums entry - // while skipping the fixture's 200 mock, and the explicit mock below - // owns the asset path. - let release = FakeReleaseBuilder::new("9.9.9") - .asset_for_current_target(&served) - .omit_asset(&asset) - .mount() - .await; - Mock::given(method("GET")) - .and(urlpath(format!( - "/SocketDev/socket-patch/releases/download/v9.9.9/{asset}" - ))) - .respond_with(ResponseTemplate::new(500)) - .mount(&release.server) - .await; - - let (code, stdout, stderr) = run_installed( - &install, - &["--update", "--json"], - &[("SOCKET_UPDATE_BASE_URL", &release.base_url)], - ); - assert_eq!(code, 1, "stdout:\n{stdout}\nstderr:\n{stderr}"); - let env = common::parse_json_envelope(&stdout); - assert_eq!( - common::envelope_error_code(&env), - Some("download_failed"), - "a non-404 asset error is download_failed, not asset_not_found" - ); - let message = common::envelope_error_message(&env).unwrap_or_default(); - assert!( - message.contains("returned 500"), - "download_failed must carry the HTTP status: {message}" - ); - - install.assert_binary_intact(); - install.assert_only_binary_present(); - release.verify_request_hygiene().await; -} diff --git a/crates/socket-patch-cli/tests/update/covgap_update_swap.rs b/crates/socket-patch-cli/tests/update/covgap_update_swap.rs deleted file mode 100644 index cefbedf94..000000000 --- a/crates/socket-patch-cli/tests/update/covgap_update_swap.rs +++ /dev/null @@ -1,140 +0,0 @@ -//! Coverage-gap e2e for `update::swap::acquire_update_lock`'s unlocked -//! degrade: when no per-user state dir resolves -//! at all — the `SOCKET_UPDATE_STATE_DIR` override, `XDG_CACHE_HOME`, and -//! `HOME` all unset/empty — the updater proceeds WITHOUT the single-flight -//! lock rather than refusing on exotic environments. A documented -//! contract (swap.rs's `return Ok(None)` degrade) no test had exercised. -//! -//! Binary-level on purpose: an inline test would have to unset HOME and -//! XDG_CACHE_HOME process-wide, and the suites mutating those use a -//! different serial key than the swap/state tests (`update_state_dir_env`) -//! — the two groups do not serialize against each other, so an inline -//! version is flake-prone. Child-process env sidesteps all of that. -//! -//! Same discipline as `self_update_e2e.rs`: the test runs a staged COPY of -//! the built binary (`update_fixture::staged_install`) — -//! `CARGO_BIN_EXE_socket-patch` itself must never be a swap target — and -//! re-verifies the real artifact at the end. - -use crate::update_fixture; - -use std::path::Path; - -use sha2::{Digest, Sha256}; -use update_fixture::{ - make_served_binary, run_installed, sha256_file, staged_install, FakeReleaseBuilder, -}; - -/// Recursively collect file names under `dir` matching `name`. -fn find_files_named(dir: &Path, name: &str) -> Vec { - let mut hits = Vec::new(); - let Ok(entries) = std::fs::read_dir(dir) else { - return hits; - }; - for entry in entries.filter_map(|e| e.ok()) { - let path = entry.path(); - if path.is_dir() { - hits.extend(find_files_named(&path, name)); - } else if entry.file_name() == name { - hits.push(path); - } - } - hits -} - -/// The degrade contract end to end: with no resolvable state dir the -/// update is unlocked but COMPLETE — a real download→verify→swap pass -/// that leaves the served payload installed, writes no lock file and no -/// notifier state anywhere, and never touches the build artifact. -#[tokio::test] -async fn update_without_resolvable_state_dir_proceeds_unlocked() { - let real_hash = update_fixture::real_binary_hash(); - let install = staged_install(); - let (served, byte_distinct) = make_served_binary(); - let served_hash = hex::encode(Sha256::digest(&served)); - - let release = FakeReleaseBuilder::new("9.9.9") - .asset_for_current_target(&served) - .mount() - .await; - - let (code, stdout, stderr) = run_installed( - &install, - &["--update", "--yes"], - &[ - ("SOCKET_UPDATE_BASE_URL", &release.base_url), - // Caller env lands last and wins: the empty values read as - // unset (state.rs's env_dir filters empty), overriding the - // fixture-injected SOCKET_UPDATE_STATE_DIR — so state_dir() - // resolves to None inside the child and acquire_update_lock - // takes the Ok(None) unlocked-degrade branch. The Windows - // pair keeps the shape correct should this ever run there. - ("SOCKET_UPDATE_STATE_DIR", ""), - ("XDG_CACHE_HOME", ""), - ("HOME", ""), - ("LOCALAPPDATA", ""), - ("USERPROFILE", ""), - ], - ); - assert_eq!( - code, 0, - "no resolvable state dir must degrade to unlocked, not refuse.\n\ - stdout:\n{stdout}\nstderr:\n{stderr}" - ); - assert!( - stdout.contains("Updated socket-patch"), - "the unlocked update must still report success: {stdout}" - ); - - // The swap genuinely completed: the installed file IS the served - // payload… - assert_eq!( - sha256_file(&install.bin), - served_hash, - "installed binary must be exactly the served payload" - ); - if byte_distinct { - assert_ne!(sha256_file(&install.bin), install.pre_hash); - } - // …via a real rename (new inode), not an in-place overwrite — the - // only swap proof on macOS, where the served bytes are pristine. - #[cfg(unix)] - { - use std::os::unix::fs::MetadataExt; - assert_ne!( - std::fs::metadata(&install.bin).unwrap().ino(), - install.pre_ino, - "unlocked swap must still be a rename, not an overwrite" - ); - } - let out = crate::common::hermetic_command(&install.bin) - .arg("--version") - .output() - .expect("spawn updated binary"); - assert!(out.status.success(), "updated binary must execute"); - - // Unlocked means UNLOCKED: no update.lock materialized anywhere in - // the install tree… - assert_eq!( - find_files_named(install.root.path(), "update.lock"), - Vec::::new(), - "no lock file may be created when no state dir resolves" - ); - // …and the state dir the fixture provisioned (whose env pointer this - // test blanked) stayed completely untouched — proof the child really - // resolved None rather than locking somewhere else. - let state_leftovers: Vec<_> = std::fs::read_dir(&install.state_dir) - .expect("read state dir") - .filter_map(|e| e.ok()) - .map(|e| e.file_name()) - .collect(); - assert!( - state_leftovers.is_empty(), - "with no resolvable state dir nothing may write update state: {state_leftovers:?}" - ); - - install.assert_only_binary_present(); - install.assert_workdir_untouched(); - release.verify_request_hygiene().await; - update_fixture::StagedInstall::assert_build_artifact_untouched(&real_hash); -} diff --git a/crates/socket-patch-cli/tests/update/main.rs b/crates/socket-patch-cli/tests/update/main.rs index 69da0e5f2..f5265d0fa 100644 --- a/crates/socket-patch-cli/tests/update/main.rs +++ b/crates/socket-patch-cli/tests/update/main.rs @@ -9,9 +9,8 @@ mod pty_io; #[path = "../common/update_fixture.rs"] mod update_fixture; -mod covgap_commands_update; -mod covgap_update_download; -mod covgap_update_swap; mod self_update_channels_e2e; mod self_update_telemetry_e2e; +mod update_json_contracts; mod update_notifier_e2e; +mod update_render; diff --git a/crates/socket-patch-cli/tests/update/update_json_contracts.rs b/crates/socket-patch-cli/tests/update/update_json_contracts.rs new file mode 100644 index 000000000..8d1709223 --- /dev/null +++ b/crates/socket-patch-cli/tests/update/update_json_contracts.rs @@ -0,0 +1,240 @@ +//! `socket-patch --update --json` contracts that scripts branch on: the +//! already-there no-ops (latest and pinned) and the unlocked-swap degrade. +//! Each test reads the envelope's codes, events and details through +//! `common::envelope` plus the disk state, never the human sentences; +//! the human wording lives in `update_render.rs`. +//! +//! Every run drives a COPY of the built binary staged into a tempdir +//! (`update_fixture::staged_install`): `CARGO_BIN_EXE_socket-patch` +//! itself must never be a swap target. + +use crate::common; +use crate::common::envelope::{assert_envelope_invariants, events, find_event}; +use crate::update_fixture; + +use std::path::Path; + +use sha2::{Digest, Sha256}; +use update_fixture::{ + make_served_binary, run_installed, sha256_file, staged_install, FakeReleaseBuilder, +}; + +const CURRENT: &str = env!("CARGO_PKG_VERSION"); + +/// A TCP endpoint nothing listens on (port 9 = discard, never bound on a +/// dev host): any request the child makes fails fast and loudly, so a +/// "zero network" contract regressing into a fetch flips the exit code. +pub(crate) const DEAD_ENDPOINT: &str = "http://127.0.0.1:9"; + +/// The already-latest envelope: exit 0, `status: success`, `dryRun: +/// false`, exactly one skipped/already_latest event carrying +/// `details.current`/`details.latest`, summary agreeing, and the empty +/// `warnings` omitted (additive-only envelope contract). One resolve, +/// zero sums, zero downloads. +#[tokio::test] +async fn update_already_latest_json_envelope_shape() { + let install = staged_install(); + let (served, _) = make_served_binary(); + + let release = FakeReleaseBuilder::new(CURRENT) + .asset_for_current_target(&served) + .expect_resolves(1) + .expect_sums_fetches(0) + .expect_asset_downloads(0) + .mount() + .await; + + let (code, stdout, stderr) = run_installed( + &install, + &["--update", "--json"], + &[("SOCKET_UPDATE_BASE_URL", &release.base_url)], + ); + assert_eq!(code, 0, "stdout:\n{stdout}\nstderr:\n{stderr}"); + + let env = common::parse_json_envelope(&stdout); + assert_envelope_invariants(&env, "update"); + assert_eq!(common::json_string(&env, "status"), Some("success")); + assert_eq!( + env["dryRun"], false, + "a wet already-latest run must not read as a dry run: {stdout}" + ); + assert_eq!(events(&env).len(), 1, "exactly one skip event: {stdout}"); + let event = find_event(&env, "skipped", Some("already_latest")); + assert_eq!(event["details"]["current"], CURRENT); + assert_eq!(event["details"]["latest"], CURRENT); + assert_eq!(env["summary"]["skipped"], 1); + + // No advisories were raised, so `warnings` must be omitted (the + // skip_serializing_if contract keeps existing consumers byte-stable). + assert!( + env.get("warnings").is_none(), + "empty warnings must be omitted from the envelope: {stdout}" + ); + + install.assert_binary_intact(); + release.verify_request_hygiene().await; +} + +/// `--update `: an explicit pin equal to the compiled version is +/// a no-op decided offline (no latest-resolution, no sums, no download; +/// the dead endpoint fails any request). It reports the same +/// skipped/already_latest event with the pin as `latest`, and it must NOT +/// refresh the passive notifier cache (the `if !pinned` state-write gate +/// in `run()`: a pin says nothing about the latest release, so caching it +/// as `latestSeen` would poison the nag). +#[test] +fn update_pin_to_current_is_noop_zero_network() { + let real_hash = update_fixture::real_binary_hash(); + let install = staged_install(); + + let (code, stdout, stderr) = run_installed( + &install, + &["--update", CURRENT, "--json"], + &[("SOCKET_UPDATE_BASE_URL", DEAD_ENDPOINT)], + ); + assert_eq!( + code, 0, + "pin-to-current must be a clean no-op.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let env = common::parse_json_envelope(&stdout); + assert_envelope_invariants(&env, "update"); + assert_eq!(common::json_string(&env, "status"), Some("success")); + assert_eq!(events(&env).len(), 1, "exactly one skip event: {stdout}"); + let event = find_event(&env, "skipped", Some("already_latest")); + assert_eq!(event["details"]["current"], CURRENT); + assert_eq!(event["details"]["latest"], CURRENT); + + install.assert_binary_intact(); + install.assert_only_binary_present(); + install.assert_workdir_untouched(); + assert!( + !install.state_dir.join("update-check.json").exists(), + "a pinned run must not write the passive notifier cache" + ); + update_fixture::StagedInstall::assert_build_artifact_untouched(&real_hash); +} + +/// Recursively collect file names under `dir` matching `name`. +fn find_files_named(dir: &Path, name: &str) -> Vec { + let mut hits = Vec::new(); + let Ok(entries) = std::fs::read_dir(dir) else { + return hits; + }; + for entry in entries.filter_map(|e| e.ok()) { + let path = entry.path(); + if path.is_dir() { + hits.extend(find_files_named(&path, name)); + } else if entry.file_name() == name { + hits.push(path); + } + } + hits +} + +/// `update::swap::acquire_update_lock`'s unlocked degrade: when no +/// per-user state dir resolves at all (the `SOCKET_UPDATE_STATE_DIR` +/// override, `XDG_CACHE_HOME` and `HOME` all empty), the updater proceeds +/// WITHOUT the single-flight lock rather than refusing on exotic +/// environments. The update is unlocked but COMPLETE: downloaded and +/// updated events, the served payload installed by a real rename, and no +/// lock file or notifier state written anywhere. +/// +/// Binary-level on purpose: an inline test would have to unset HOME and +/// XDG_CACHE_HOME process-wide, and the suites mutating those use a +/// different serial key than the swap/state tests, so an inline version +/// is flake-prone. Child-process env sidesteps that. +#[tokio::test] +async fn update_without_resolvable_state_dir_proceeds_unlocked() { + let real_hash = update_fixture::real_binary_hash(); + let install = staged_install(); + let (served, byte_distinct) = make_served_binary(); + let served_hash = hex::encode(Sha256::digest(&served)); + + let release = FakeReleaseBuilder::new("9.9.9") + .asset_for_current_target(&served) + .mount() + .await; + + let (code, stdout, stderr) = run_installed( + &install, + &["--update", "--yes", "--json"], + &[ + ("SOCKET_UPDATE_BASE_URL", &release.base_url), + // Caller env lands last and wins: the empty values read as + // unset (state.rs's env_dir filters empty), overriding the + // fixture-injected SOCKET_UPDATE_STATE_DIR, so state_dir() + // resolves to None inside the child and acquire_update_lock + // takes the Ok(None) unlocked-degrade branch. The Windows + // pair keeps the shape correct should this ever run there. + ("SOCKET_UPDATE_STATE_DIR", ""), + ("XDG_CACHE_HOME", ""), + ("HOME", ""), + ("LOCALAPPDATA", ""), + ("USERPROFILE", ""), + ], + ); + assert_eq!( + code, 0, + "no resolvable state dir must degrade to unlocked, not refuse.\n\ + stdout:\n{stdout}\nstderr:\n{stderr}" + ); + let env = common::parse_json_envelope(&stdout); + assert_envelope_invariants(&env, "update"); + assert_eq!(common::json_string(&env, "status"), Some("success")); + find_event(&env, "downloaded", None); + let updated = find_event(&env, "updated", None); + assert_eq!(updated["details"]["from"], CURRENT); + assert_eq!(updated["details"]["to"], "9.9.9"); + + // The swap genuinely completed: the installed file IS the served + // payload… + assert_eq!( + sha256_file(&install.bin), + served_hash, + "installed binary must be exactly the served payload" + ); + if byte_distinct { + assert_ne!(sha256_file(&install.bin), install.pre_hash); + } + // …via a real rename (new inode), not an in-place overwrite: the + // only swap proof on macOS, where the served bytes are pristine. + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + assert_ne!( + std::fs::metadata(&install.bin).unwrap().ino(), + install.pre_ino, + "unlocked swap must still be a rename, not an overwrite" + ); + } + let out = common::hermetic_command(&install.bin) + .arg("--version") + .output() + .expect("spawn updated binary"); + assert!(out.status.success(), "updated binary must execute"); + + // Unlocked means UNLOCKED: no update.lock materialized anywhere in + // the install tree… + assert_eq!( + find_files_named(install.root.path(), "update.lock"), + Vec::::new(), + "no lock file may be created when no state dir resolves" + ); + // …and the state dir the fixture provisioned (whose env pointer this + // test blanked) stayed completely untouched: proof the child really + // resolved None rather than locking somewhere else. + let state_leftovers: Vec<_> = std::fs::read_dir(&install.state_dir) + .expect("read state dir") + .filter_map(|e| e.ok()) + .map(|e| e.file_name()) + .collect(); + assert!( + state_leftovers.is_empty(), + "with no resolvable state dir nothing may write update state: {state_leftovers:?}" + ); + + install.assert_only_binary_present(); + install.assert_workdir_untouched(); + release.verify_request_hygiene().await; + update_fixture::StagedInstall::assert_build_artifact_untouched(&real_hash); +} diff --git a/crates/socket-patch-cli/tests/update/covgap_commands_update.rs b/crates/socket-patch-cli/tests/update/update_render.rs similarity index 66% rename from crates/socket-patch-cli/tests/update/covgap_commands_update.rs rename to crates/socket-patch-cli/tests/update/update_render.rs index 4f5846990..64c0ae5ff 100644 --- a/crates/socket-patch-cli/tests/update/covgap_commands_update.rs +++ b/crates/socket-patch-cli/tests/update/update_render.rs @@ -1,50 +1,32 @@ -//! Coverage-gap tests for `commands/update.rs`: the pinned already-there -//! message (a zero-network path), the -//! already-latest `--json` envelope shape, the human-mode dry-run prints -//! (both the `--force` "Would reinstall" wording and the plain -//! "Update available" wording), and the PTY-driven interactive decline. +//! Human output of `socket-patch --update`, one check per message +//! family: the pinned already-there wording, the two dry-run reports and +//! the interactive reinstall confirm (PTY). The behavior behind each is +//! asserted through `--json` in `update_json_contracts.rs`; these tests +//! only pin what a person reads. //! -//! Every wet run drives a COPY of the built binary staged into a tempdir -//! (`update_fixture::staged_install`) — `CARGO_BIN_EXE_socket-patch` -//! itself must never be a swap target. Fixture shapes are copied from -//! self_update_e2e.rs / interactive_prompts_e2e.rs. +//! Every run drives a COPY of the built binary staged into a tempdir +//! (`update_fixture::staged_install`): `CARGO_BIN_EXE_socket-patch` +//! itself must never be a swap target. -use crate::common; use crate::update_fixture; +use crate::update_json_contracts::DEAD_ENDPOINT; use update_fixture::{make_served_binary, run_installed, staged_install, FakeReleaseBuilder}; const CURRENT: &str = env!("CARGO_PKG_VERSION"); -/// A TCP endpoint nothing listens on (port 9 = discard, never bound on a -/// dev host): any request the child makes fails fast and loudly, so a -/// "zero network" contract regressing into a fetch flips the exit code. -const DEAD_ENDPOINT: &str = "http://127.0.0.1:9"; - -/// `--update `: an explicit pin equal to the compiled version is -/// an informational no-op that never creates a network client — no -/// latest-resolution, no sums, no download. It must use -/// the PINNED wording ("already version X", not "already the latest": the -/// pin path cannot know what the latest is), and it must NOT refresh the -/// passive notifier cache (the `if !pinned` state-write gate in `run()` — -/// a pin says nothing about the latest release, so caching it -/// as `latestSeen` would poison the nag). +/// The pinned already-there message names the pin, not "the latest": the +/// pin path never resolves the latest release, so it can't claim it. #[test] -fn update_pin_to_current_is_noop_zero_network() { - let real_hash = update_fixture::real_binary_hash(); +fn update_pin_to_current_human_says_already_version() { let install = staged_install(); - // No fake release is mounted at all: the dead endpoint proves the - // pinned already-there path is decided entirely offline. let (code, stdout, stderr) = run_installed( &install, &["--update", CURRENT], &[("SOCKET_UPDATE_BASE_URL", DEAD_ENDPOINT)], ); - assert_eq!( - code, 0, - "pin-to-current must be a clean no-op.\nstdout:\n{stdout}\nstderr:\n{stderr}" - ); + assert_eq!(code, 0, "stdout:\n{stdout}\nstderr:\n{stderr}"); assert!( stdout.contains(&format!("socket-patch is already version {CURRENT}.")), "must print the pinned already-there message: {stdout}" @@ -53,77 +35,7 @@ fn update_pin_to_current_is_noop_zero_network() { !stdout.contains("already the latest"), "the pin path must not claim latest-knowledge it doesn't have: {stdout}" ); - - install.assert_binary_intact(); - install.assert_only_binary_present(); - install.assert_workdir_untouched(); - // Pinned runs skip the notifier state refresh entirely. - assert!( - !install.state_dir.join("update-check.json").exists(), - "a pinned run must not write the passive notifier cache" - ); - update_fixture::StagedInstall::assert_build_artifact_untouched(&real_hash); -} - -/// The already-latest `--json` envelope is a machine contract scripts -/// branch on: exit 0, `status: success`, `dryRun: false`, exactly one -/// skipped/already_latest event carrying `details.current`/`details.latest`, -/// summary agreeing, and the empty `warnings` omitted (additive-only -/// envelope contract). One resolve, zero sums, zero downloads. -#[tokio::test] -async fn update_already_latest_json_envelope_shape() { - let install = staged_install(); - let (served, _) = make_served_binary(); - - let release = FakeReleaseBuilder::new(CURRENT) - .asset_for_current_target(&served) - .expect_resolves(1) - .expect_sums_fetches(0) - .expect_asset_downloads(0) - .mount() - .await; - - let (code, stdout, stderr) = run_installed( - &install, - &["--update", "--json"], - &[("SOCKET_UPDATE_BASE_URL", &release.base_url)], - ); - assert_eq!(code, 0, "stdout:\n{stdout}\nstderr:\n{stderr}"); - - let env = common::parse_json_envelope(&stdout); - assert_eq!(common::json_string(&env, "command"), Some("update")); - assert_eq!(common::json_string(&env, "status"), Some("success")); - assert_eq!( - env["dryRun"], false, - "a wet already-latest run must not read as a dry run: {stdout}" - ); - - let events = env["events"].as_array().expect("events array"); - assert_eq!(events.len(), 1, "exactly one skip event: {stdout}"); - let event = &events[0]; - assert_eq!(event["action"], "skipped"); - assert_eq!( - event["errorCode"], "already_latest", - "the routing tag scripts branch on: {stdout}" - ); - let reason = event["reason"].as_str().unwrap_or_default(); - assert!( - reason.contains("already the latest"), - "the human message rides the event as its reason: {reason}" - ); - assert_eq!(event["details"]["current"], CURRENT); - assert_eq!(event["details"]["latest"], CURRENT); - assert_eq!(env["summary"]["skipped"], 1); - - // No advisories were raised, so `warnings` must be omitted (the - // skip_serializing_if contract keeps existing consumers byte-stable). - assert!( - env.get("warnings").is_none(), - "empty warnings must be omitted from the envelope: {stdout}" - ); - install.assert_binary_intact(); - release.verify_request_hygiene().await; } /// `--dry-run --force` when already up to date: the probe still reports From 9dd5b35421d288d98b8bca5f59ebeb5f15bccdb5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 16:09:45 +0000 Subject: [PATCH 3/3] Tighten update JSON checks from review - The 500 test matches "returned 500" again: a bare "500" could hit the mock server's random port. - The already-latest event must still carry a non-empty reason. - The pinned no-op checks dryRun and summary like its sibling, and no longer pins already_latest/latest for a pin (a pin says nothing about the latest release). - DEAD_ENDPOINT lives in update_fixture for both update files. Refs #1089. Assisted-by: Claude Code:claude-opus-5-5 --- .../tests/common/update_fixture.rs | 5 ++++ .../tests/self_update_failures_e2e.rs | 2 +- .../tests/update/update_json_contracts.rs | 24 ++++++++++++------- .../tests/update/update_render.rs | 9 ++++--- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/crates/socket-patch-cli/tests/common/update_fixture.rs b/crates/socket-patch-cli/tests/common/update_fixture.rs index 4c567c124..4e8884359 100644 --- a/crates/socket-patch-cli/tests/common/update_fixture.rs +++ b/crates/socket-patch-cli/tests/common/update_fixture.rs @@ -22,6 +22,11 @@ use wiremock::{Mock, MockServer, ResponseTemplate}; use socket_patch_cli::commands::update::UPDATE_TARGET; use socket_patch_core::update::asset_name_for_target; +/// A TCP endpoint nothing listens on (port 9 = discard, never bound on a +/// dev host): any request the child makes fails fast and loudly, so a +/// "zero network" contract regressing into a fetch flips the exit code. +pub const DEAD_ENDPOINT: &str = "http://127.0.0.1:9"; + /// Release asset filename for the target this test binary was built for. pub fn asset_name_for_current_target() -> String { asset_name_for_target(UPDATE_TARGET) diff --git a/crates/socket-patch-cli/tests/self_update_failures_e2e.rs b/crates/socket-patch-cli/tests/self_update_failures_e2e.rs index 831d662b6..5c37f1bb6 100644 --- a/crates/socket-patch-cli/tests/self_update_failures_e2e.rs +++ b/crates/socket-patch-cli/tests/self_update_failures_e2e.rs @@ -198,7 +198,7 @@ async fn asset_server_error_is_download_failed() { assert_eq!(common::envelope_error_code(&env), Some("download_failed")); let message = common::envelope_error_message(&env).unwrap_or_default(); assert!( - message.contains("500"), + message.contains("returned 500"), "download_failed must carry the HTTP status: {message}" ); diff --git a/crates/socket-patch-cli/tests/update/update_json_contracts.rs b/crates/socket-patch-cli/tests/update/update_json_contracts.rs index 8d1709223..2f6adb73a 100644 --- a/crates/socket-patch-cli/tests/update/update_json_contracts.rs +++ b/crates/socket-patch-cli/tests/update/update_json_contracts.rs @@ -17,15 +17,11 @@ use std::path::Path; use sha2::{Digest, Sha256}; use update_fixture::{ make_served_binary, run_installed, sha256_file, staged_install, FakeReleaseBuilder, + DEAD_ENDPOINT, }; const CURRENT: &str = env!("CARGO_PKG_VERSION"); -/// A TCP endpoint nothing listens on (port 9 = discard, never bound on a -/// dev host): any request the child makes fails fast and loudly, so a -/// "zero network" contract regressing into a fetch flips the exit code. -pub(crate) const DEAD_ENDPOINT: &str = "http://127.0.0.1:9"; - /// The already-latest envelope: exit 0, `status: success`, `dryRun: /// false`, exactly one skipped/already_latest event carrying /// `details.current`/`details.latest`, summary agreeing, and the empty @@ -62,6 +58,12 @@ async fn update_already_latest_json_envelope_shape() { let event = find_event(&env, "skipped", Some("already_latest")); assert_eq!(event["details"]["current"], CURRENT); assert_eq!(event["details"]["latest"], CURRENT); + // The human explanation rides the event (its wording is checked in + // update_render.rs / self_update_e2e.rs, not here). + assert!( + event["reason"].as_str().is_some_and(|r| !r.is_empty()), + "the skip event must carry a reason: {stdout}" + ); assert_eq!(env["summary"]["skipped"], 1); // No advisories were raised, so `warnings` must be omitted (the @@ -77,8 +79,8 @@ async fn update_already_latest_json_envelope_shape() { /// `--update `: an explicit pin equal to the compiled version is /// a no-op decided offline (no latest-resolution, no sums, no download; -/// the dead endpoint fails any request). It reports the same -/// skipped/already_latest event with the pin as `latest`, and it must NOT +/// the dead endpoint fails any request). It reports one skipped event for +/// the running version, and it must NOT /// refresh the passive notifier cache (the `if !pinned` state-write gate /// in `run()`: a pin says nothing about the latest release, so caching it /// as `latestSeen` would poison the nag). @@ -99,10 +101,14 @@ fn update_pin_to_current_is_noop_zero_network() { let env = common::parse_json_envelope(&stdout); assert_envelope_invariants(&env, "update"); assert_eq!(common::json_string(&env, "status"), Some("success")); + assert_eq!( + env["dryRun"], false, + "a wet pinned no-op must not read as a dry run: {stdout}" + ); assert_eq!(events(&env).len(), 1, "exactly one skip event: {stdout}"); - let event = find_event(&env, "skipped", Some("already_latest")); + let event = find_event(&env, "skipped", None); assert_eq!(event["details"]["current"], CURRENT); - assert_eq!(event["details"]["latest"], CURRENT); + assert_eq!(env["summary"]["skipped"], 1); install.assert_binary_intact(); install.assert_only_binary_present(); diff --git a/crates/socket-patch-cli/tests/update/update_render.rs b/crates/socket-patch-cli/tests/update/update_render.rs index 64c0ae5ff..ba7d9e646 100644 --- a/crates/socket-patch-cli/tests/update/update_render.rs +++ b/crates/socket-patch-cli/tests/update/update_render.rs @@ -2,16 +2,19 @@ //! family: the pinned already-there wording, the two dry-run reports and //! the interactive reinstall confirm (PTY). The behavior behind each is //! asserted through `--json` in `update_json_contracts.rs`; these tests -//! only pin what a person reads. +//! only pin what a person reads. The unpinned already-latest and the +//! `Updated socket-patch X → Y` lines are still checked by the happy-path +//! tests in `self_update_e2e.rs`. //! //! Every run drives a COPY of the built binary staged into a tempdir //! (`update_fixture::staged_install`): `CARGO_BIN_EXE_socket-patch` //! itself must never be a swap target. use crate::update_fixture; -use crate::update_json_contracts::DEAD_ENDPOINT; -use update_fixture::{make_served_binary, run_installed, staged_install, FakeReleaseBuilder}; +use update_fixture::{ + make_served_binary, run_installed, staged_install, FakeReleaseBuilder, DEAD_ENDPOINT, +}; const CURRENT: &str = env!("CARGO_PKG_VERSION");