From eb545b4e5b93f2e39a5924aebb41b0e7c2bde6b0 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 19 Sep 2026 06:08:28 +0000 Subject: [PATCH 1/4] fix(cli): align update identity with the installed binary The GitHub upgrade path stored the release tag as the maintenance window's expected version. The daemon advertises build_version(), which is that release plus +sha, and readiness compares the two exactly, so update refused the binary it had just installed. Ask the published binary for --version, the same identity the package-manager path already reports. An unreadable binary stays unversioned instead of substituting the tag. Co-authored-by: Zack Jackson --- crates/tracedecay-cli/src/upgrade.rs | 84 ++++++++++++++++--- .../src/service/update_restore_tests.rs | 46 ++++++++++ 2 files changed, 120 insertions(+), 10 deletions(-) diff --git a/crates/tracedecay-cli/src/upgrade.rs b/crates/tracedecay-cli/src/upgrade.rs index 032cf81c0c..5f21190786 100644 --- a/crates/tracedecay-cli/src/upgrade.rs +++ b/crates/tracedecay-cli/src/upgrade.rs @@ -554,15 +554,15 @@ pub enum UpgradeOutcome { /// binary: `which_tracedecay()`'s current-exe-first order can point /// at the OLD binary (e.g. a stale Homebrew keg) after an upgrade. binary: Option, - /// Version of the freshly installed binary: the release-manifest - /// version for GitHub-release installs, the linked binary's - /// self-reported version for package-manager installs. Daemon restore - /// validates this version, the binary it actually restarts, instead - /// of the one that was running before the upgrade. `None` only when - /// the manager's install could not be interrogated; restore - /// verification then validates the pre-upgrade version and, if a new - /// daemon really was installed, fails with a typed identity mismatch - /// rather than silently passing. + /// Protocol identity of the freshly installed binary, the string + /// `tracedecay --version` prints and the daemon advertises as + /// `build_version()`. That is `{release}+{sha}[.dirty]`, not the + /// GitHub release tag. Daemon restore compares it to the answering + /// process with exact equality. `None` when the binary could not be + /// read: restore then keeps the pre-upgrade identity and fails with + /// a typed mismatch if a different daemon starts, instead of + /// substituting the bare release tag and refusing the binary that + /// was just installed. version: Option, }, /// Already on the latest version. The binary was not replaced. @@ -796,8 +796,12 @@ fn run_versioned_upgrade(current: &str, is_beta: bool) -> Result record_previous_version(); eprintln!("\x1b[32m✔\x1b[0m Successfully upgraded to v{latest}!"); Ok(UpgradeOutcome::Installed { + // The release tag is the bare semver the operator downloaded. The + // daemon advertises `{release}+{sha}`, and readiness compares those + // strings exactly, so the window must expect the binary's own + // identity rather than `latest`. + version: installed_protocol_identity(binary.as_deref()), binary, - version: Some(latest.to_owned()), }) } @@ -949,6 +953,35 @@ fn installed_binary_version(path: &Path) -> std::result::Result`. Reporting the tag made `tracedecay update` +/// refuse the daemon it had just started. An unreadable binary is `None`, +/// never the tag: inventing a less specific identity is what produced the +/// mismatch. +fn installed_protocol_identity(binary: Option<&Path>) -> Option { + match binary.map(installed_binary_version) { + Some(Ok(version)) => Some(version), + Some(Err(reason)) => { + eprintln!( + " \x1b[33mwarning:\x1b[0m could not read the installed binary's protocol identity \ + ({reason}); daemon restore will not substitute the release tag" + ); + None + } + None => { + eprintln!( + " \x1b[33mwarning:\x1b[0m installed binary path is unknown; daemon restore will \ + not substitute the release tag" + ); + None + } + } +} + fn installed_binary_version_within( path: &Path, deadline: Duration, @@ -1235,6 +1268,7 @@ mod tests { use super::super::{ VersionProbeError, installed_binary_version, installed_binary_version_within, + installed_protocol_identity, }; fn script(dir: &Path, body: &str) -> PathBuf { @@ -1252,6 +1286,36 @@ mod tests { assert_eq!(installed_binary_version(&binary).unwrap(), "1.2.3+abcdef"); } + /// The `tracedecay update` failure: the release tag is `0.1.0-beta.47` + /// and the binary that tag installs names itself with `+`. + /// Readiness compares those strings exactly, so the maintenance + /// window must be handed the binary's identity. + #[test] + fn a_direct_install_reports_the_binary_identity_not_the_release_tag() { + let dir = tempfile::tempdir().unwrap(); + let build = "0.1.0-beta.47+84598a0b9c841b914565f46b20bb6c765706e8e5"; + let binary = script(dir.path(), &format!("printf 'tracedecay {build}\\n'")); + + assert_eq!( + installed_protocol_identity(Some(&binary)).as_deref(), + Some(build) + ); + + let bare = script(dir.path(), "printf 'tracedecay 0.1.0-beta.47\\n'"); + assert_eq!( + installed_protocol_identity(Some(&bare)).as_deref(), + Some("0.1.0-beta.47") + ); + + let missing = dir.path().join("absent"); + assert_eq!( + installed_protocol_identity(Some(&missing)), + None, + "an unreadable binary must not fall back to the release tag" + ); + assert_eq!(installed_protocol_identity(None), None); + } + #[test] fn failed_exits_and_malformed_output_are_not_version_evidence() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/tracedecay-daemon-control/src/service/update_restore_tests.rs b/crates/tracedecay-daemon-control/src/service/update_restore_tests.rs index 5a84ee931f..765255c79d 100644 --- a/crates/tracedecay-daemon-control/src/service/update_restore_tests.rs +++ b/crates/tracedecay-daemon-control/src/service/update_restore_tests.rs @@ -133,3 +133,49 @@ fn restore_readiness_rejects_a_stale_daemon_after_an_upgrade() { ); server.join().expect("join stale daemon"); } + +/// Protocol identity is the string the daemon advertises, build metadata +/// included. A bare release tag is not that identity: `tracedecay update` +/// refused the daemon it had just installed when expected_version was +/// `0.1.0-beta.47` and the process answered `0.1.0-beta.47+`. +#[cfg(unix)] +#[test] +fn restore_readiness_accepts_the_advertised_build_and_rejects_its_release_tag() { + let _env_lock = lock_user_data_dir_test_env(); + let profile = TempDir::new().expect("profile temp dir"); + let _data_dir_guard = EnvVarGuard::set(USER_DATA_DIR_ENV, profile.path()); + + const BUILD: &str = "0.1.0-beta.47+84598a0b9c841b914565f46b20bb6c765706e8e5"; + const RELEASE: &str = "0.1.0-beta.47"; + + let ready_socket = profile.path().join("ready.sock"); + let ready_listener = UnixListener::bind(&ready_socket).expect("bind ready daemon socket"); + let ready_server = serve_initialize_identity(ready_listener, "tracedecay", BUILD); + assert_eq!( + super::probe::daemon_protocol_state_with_timeout( + &ready_socket, + BUILD, + std::time::Duration::from_secs(5), + ), + super::probe::DaemonProtocolState::Ready, + ); + ready_server.join().expect("join ready daemon"); + + let mismatch_socket = profile.path().join("mismatch.sock"); + let mismatch_listener = + UnixListener::bind(&mismatch_socket).expect("bind mismatched daemon socket"); + let mismatch_server = serve_initialize_identity(mismatch_listener, "tracedecay", BUILD); + assert_eq!( + super::probe::daemon_protocol_state_with_timeout( + &mismatch_socket, + RELEASE, + std::time::Duration::from_secs(5), + ), + super::probe::DaemonProtocolState::IdentityMismatch { + name: Some("tracedecay".to_string()), + version: Some(BUILD.to_string()), + expected_version: RELEASE.to_string(), + } + ); + mismatch_server.join().expect("join mismatched daemon"); +} From 10f90f172c60ba65106a79e9aa34817e450c9f5d Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 08:43:43 +0000 Subject: [PATCH 2/4] test(daemon-control): drop readiness test contradicting master This PR's restore_readiness_accepts_the_advertised_build_and_rejects_its_release_tag asserted DaemonProtocolState::IdentityMismatch when a daemon advertising 0.1.0-beta.47+ is probed against the bare release 0.1.0-beta.47. Master 5e03dfa9c5 landed versions_name_same_build and makes exactly that pair Ready, with probe.rs a_daemon_naming_its_commit_is_ready_against_its_bare_release pinning it. After merging master the test failed (left: Ready, right: IdentityMismatch). Its other half, Ready against an exactly equal build, is already covered by that probe.rs module, so the file returns to master's version rather than being rewritten. Co-Authored-By: Claude Fable 5.1 --- .../src/service/update_restore_tests.rs | 46 ------------------- 1 file changed, 46 deletions(-) diff --git a/crates/tracedecay-daemon-control/src/service/update_restore_tests.rs b/crates/tracedecay-daemon-control/src/service/update_restore_tests.rs index 765255c79d..5a84ee931f 100644 --- a/crates/tracedecay-daemon-control/src/service/update_restore_tests.rs +++ b/crates/tracedecay-daemon-control/src/service/update_restore_tests.rs @@ -133,49 +133,3 @@ fn restore_readiness_rejects_a_stale_daemon_after_an_upgrade() { ); server.join().expect("join stale daemon"); } - -/// Protocol identity is the string the daemon advertises, build metadata -/// included. A bare release tag is not that identity: `tracedecay update` -/// refused the daemon it had just installed when expected_version was -/// `0.1.0-beta.47` and the process answered `0.1.0-beta.47+`. -#[cfg(unix)] -#[test] -fn restore_readiness_accepts_the_advertised_build_and_rejects_its_release_tag() { - let _env_lock = lock_user_data_dir_test_env(); - let profile = TempDir::new().expect("profile temp dir"); - let _data_dir_guard = EnvVarGuard::set(USER_DATA_DIR_ENV, profile.path()); - - const BUILD: &str = "0.1.0-beta.47+84598a0b9c841b914565f46b20bb6c765706e8e5"; - const RELEASE: &str = "0.1.0-beta.47"; - - let ready_socket = profile.path().join("ready.sock"); - let ready_listener = UnixListener::bind(&ready_socket).expect("bind ready daemon socket"); - let ready_server = serve_initialize_identity(ready_listener, "tracedecay", BUILD); - assert_eq!( - super::probe::daemon_protocol_state_with_timeout( - &ready_socket, - BUILD, - std::time::Duration::from_secs(5), - ), - super::probe::DaemonProtocolState::Ready, - ); - ready_server.join().expect("join ready daemon"); - - let mismatch_socket = profile.path().join("mismatch.sock"); - let mismatch_listener = - UnixListener::bind(&mismatch_socket).expect("bind mismatched daemon socket"); - let mismatch_server = serve_initialize_identity(mismatch_listener, "tracedecay", BUILD); - assert_eq!( - super::probe::daemon_protocol_state_with_timeout( - &mismatch_socket, - RELEASE, - std::time::Duration::from_secs(5), - ), - super::probe::DaemonProtocolState::IdentityMismatch { - name: Some("tracedecay".to_string()), - version: Some(BUILD.to_string()), - expected_version: RELEASE.to_string(), - } - ); - mismatch_server.join().expect("join mismatched daemon"); -} From efaabc7f3d211d2d04f270e01da37d9890dd3790 Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 08:43:51 +0000 Subject: [PATCH 3/4] docs(cli): correct update identity rationale after 5e03dfa9c5 The new installed_protocol_identity and the UpgradeOutcome::Installed version field justified themselves with "readiness compares those strings exactly". Master 5e03dfa9c5 replaced that raw == with versions_name_same_build, so the stated reason no longer describes the code. The change itself is still right, for the opposite reason: because build metadata a side omits is ignored, a bare release tag as expected_version matches EVERY build of that release and silently disables the same-release different-commit skew detection 367a44ad00 added. Reporting the installed binary's own identity keeps the window pinned to one build. Comments only; no behavior change. Co-Authored-By: Claude Fable 5.1 --- crates/tracedecay-cli/src/upgrade.rs | 36 +++++++++++++++------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/crates/tracedecay-cli/src/upgrade.rs b/crates/tracedecay-cli/src/upgrade.rs index 5f21190786..04da93665c 100644 --- a/crates/tracedecay-cli/src/upgrade.rs +++ b/crates/tracedecay-cli/src/upgrade.rs @@ -558,11 +558,12 @@ pub enum UpgradeOutcome { /// `tracedecay --version` prints and the daemon advertises as /// `build_version()`. That is `{release}+{sha}[.dirty]`, not the /// GitHub release tag. Daemon restore compares it to the answering - /// process with exact equality. `None` when the binary could not be - /// read: restore then keeps the pre-upgrade identity and fails with - /// a typed mismatch if a different daemon starts, instead of - /// substituting the bare release tag and refusing the binary that - /// was just installed. + /// process with `versions_name_same_build`, which ignores build + /// metadata a side omits, so a bare release tag would accept ANY + /// build of that release. `None` when the binary could not be read: + /// restore then keeps the pre-upgrade identity and fails with a + /// typed mismatch if a different daemon starts, rather than + /// widening the window to a whole release. version: Option, }, /// Already on the latest version. The binary was not replaced. @@ -797,8 +798,8 @@ fn run_versioned_upgrade(current: &str, is_beta: bool) -> Result eprintln!("\x1b[32m✔\x1b[0m Successfully upgraded to v{latest}!"); Ok(UpgradeOutcome::Installed { // The release tag is the bare semver the operator downloaded. The - // daemon advertises `{release}+{sha}`, and readiness compares those - // strings exactly, so the window must expect the binary's own + // daemon advertises `{release}+{sha}`, and a bare tag matches every + // build of that release, so the window must expect the binary's own // identity rather than `latest`. version: installed_protocol_identity(binary.as_deref()), binary, @@ -956,12 +957,15 @@ fn installed_binary_version(path: &Path) -> std::result::Result`. Reporting the tag made `tracedecay update` -/// refuse the daemon it had just started. An unreadable binary is `None`, -/// never the tag: inventing a less specific identity is what produced the -/// mismatch. +/// `build_version()` with `versions_name_same_build` (5e03dfa9c5), which +/// treats a side that names no commit as less specific rather than +/// different. A GitHub release tag is only the release +/// (`0.1.0-beta.47`); the binary the tag installs names itself +/// `0.1.0-beta.47+`. Reporting the tag therefore made the maintenance +/// window accept any build of that release, disabling the same-release +/// different-commit skew detection 367a44ad00 added. An unreadable binary +/// is `None`, never the tag: a less specific identity is what widened the +/// window in the first place. fn installed_protocol_identity(binary: Option<&Path>) -> Option { match binary.map(installed_binary_version) { Some(Ok(version)) => Some(version), @@ -1287,9 +1291,9 @@ mod tests { } /// The `tracedecay update` failure: the release tag is `0.1.0-beta.47` - /// and the binary that tag installs names itself with `+`. - /// Readiness compares those strings exactly, so the maintenance - /// window must be handed the binary's identity. + /// and the binary that tag installs names itself with `+`. A + /// bare tag names every build of that release, so the maintenance + /// window must be handed the binary's own identity. #[test] fn a_direct_install_reports_the_binary_identity_not_the_release_tag() { let dir = tempfile::tempdir().unwrap(); From cf89ba4bd75135c3e5e681bf5b011ea6ba9e0ced Mon Sep 17 00:00:00 2001 From: ScriptedAlchemy Date: Sat, 19 Sep 2026 08:22:00 +0000 Subject: [PATCH 4/4] style(session-temporal): keep the test module last and rustfmt master Master run 35431539771 failed Check formatting (projector.rs, query.rs) and Clippy (items_after_test_module in query.rs) after #1844/#1845 merged without CI. Co-Authored-By: Claude Fable 5.1 --- .../projector.rs | 8 ++--- .../src/query.rs | 29 +++++++++---------- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/projector.rs b/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/projector.rs index 10818733c1..632898ca24 100644 --- a/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/projector.rs +++ b/crates/tracedecay-session-runtime/src/session_temporal_refresh_scheduler/projector.rs @@ -119,11 +119,11 @@ impl SessionTemporalRefreshProjector for CanonicalSessionTemporalProjector { // Empty remaining range is a durable no-op: terminalize with an // empty complete progress batch instead of deferring forever. Ok(None) => canonical_noop_complete_effect(&recovery), - Err(error) if error.is_storage() => Err( - SessionTemporalRefreshProjectorError::retryable(format!( + Err(error) if error.is_storage() => { + Err(SessionTemporalRefreshProjectorError::retryable(format!( "source_busy: {error}" - )), - ), + ))) + } Err(_) => Err(SessionTemporalRefreshProjectorError::terminal( "projector_failed", )), diff --git a/crates/tracedecay-session-temporal-store/src/query.rs b/crates/tracedecay-session-temporal-store/src/query.rs index 367b14bb79..ae0ab6b4de 100644 --- a/crates/tracedecay-session-temporal-store/src/query.rs +++ b/crates/tracedecay-session-temporal-store/src/query.rs @@ -246,8 +246,7 @@ pub(super) async fn read_observations( // ceiling. Split until a single observation remains; that // observation is then a typed storage failure, not a retry // that looks like a busy source. - if observation_prefetch_exceeded_materialization_limit(&error) && chunk.len() > 1 - { + if observation_prefetch_exceeded_materialization_limit(&error) && chunk.len() > 1 { let mid = start + chunk.len() / 2; pending.push((mid, end)); pending.push((start, mid)); @@ -289,11 +288,18 @@ fn observation_prefetch_exceeded_materialization_limit(error: &SessionStoreError } } +/// The error `read_observation` raises for an id the store does not hold, reused +/// by callers that resolve prefetched observations out of a batch map. +pub(super) fn missing_observation(observation_id: &CanonicalObservationIdV1) -> SessionStoreError { + storage_message( + PERSIST_OPERATION, + format!("source observation {} is missing", observation_id.as_str()), + ) +} + #[cfg(test)] mod tests { - use super::{ - PERSIST_OPERATION, observation_prefetch_exceeded_materialization_limit, storage, - }; + use super::{PERSIST_OPERATION, observation_prefetch_exceeded_materialization_limit, storage}; #[test] fn materialization_limit_is_the_prefetch_split_signal() { @@ -308,15 +314,8 @@ mod tests { assert!(observation_prefetch_exceeded_materialization_limit( &exceeded )); - assert!(!observation_prefetch_exceeded_materialization_limit(&locked)); + assert!(!observation_prefetch_exceeded_materialization_limit( + &locked + )); } } - -/// The error `read_observation` raises for an id the store does not hold, reused -/// by callers that resolve prefetched observations out of a batch map. -pub(super) fn missing_observation(observation_id: &CanonicalObservationIdV1) -> SessionStoreError { - storage_message( - PERSIST_OPERATION, - format!("source observation {} is missing", observation_id.as_str()), - ) -}