From 198a0535420abf3a978fd7ffc918e31c12b3f52e Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Tue, 10 Nov 2026 16:42:09 -0500 Subject: [PATCH] Keep Experiment scope wrappers alive across updates (#1498) --- docs/guide.md | 2 +- site/architecture/index.html | 2 +- site/docs/index.html | 2 +- src/scope.rs | 10 +++-- src/update.rs | 4 +- tests/experiments.rs | 3 ++ tests/experiments/update.rs | 72 ++++++++++++++++++++++++++++++++++++ tests/support/mod.rs | 14 ++++++- 8 files changed, 101 insertions(+), 8 deletions(-) create mode 100644 tests/experiments/update.rs diff --git a/docs/guide.md b/docs/guide.md index 432cb13b4..76de6d4ce 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -984,7 +984,7 @@ Run `sudo nginx -t`, then `sudo systemctl restart nginx` on a systemd host. The thirdshift update ``` -It replaces the installed binary with the latest stable GitHub Release, or says it is already on it. Messages go to stderr and stdout stays empty. It exits `0` when it updated or was already up to date, and `1` on any failure, such as no network. Updating is safe while a Ticket run or a Spec run is using the old binary: it keeps running it. So do the child Ticket runs it starts afterwards, a Ticket run or a Base fix, which it starts through `/proc/self/exe` from the binary it is running rather than from the install path, so they run its version even once that path holds the new one or nothing at all. A Ticket run never checks for updates or updates itself. +It replaces the installed binary with the latest stable GitHub Release, or says it is already on it. Messages go to stderr and stdout stays empty. It exits `0` when it updated or was already up to date, and `1` on any failure, such as no network. Updating is safe while a Ticket run, Spec run or Experiment is using the old binary: it keeps running it. So do the child Ticket runs it starts afterwards, a Ticket run or a Base fix, which it starts through `/proc/self/exe` from the binary it is running rather than from the install path, so they run its version even once that path holds the new one or nothing at all. An Experiment's later Trials and Graders keep its original OOM wrapper through the owning process's `/proc//exe` link, including scopes prepared before the update. The requested Arm programs, arguments, environment and memory caps stay the same. A Ticket run never checks for updates or updates itself. `thirdshift update` only replaces a copy put in place by the shell installer (the `curl` command above), which leaves an install receipt in `~/.config/thirdshift/`. It refuses to touch any other copy and lists the command that updates each other kind of install: `cargo install thirdshift` for a crates.io install, pulling and reinstalling for a build from source (see [Building from source](#building-from-source)), and the `curl` command for a copy placed by hand. diff --git a/site/architecture/index.html b/site/architecture/index.html index d4e2bf306..caa568c1c 100644 --- a/site/architecture/index.html +++ b/site/architecture/index.html @@ -273,7 +273,7 @@

Architecture

The Bar also saves min_discriminating_tasks, defaulting to 20, before the first Trial. --min-discriminating-tasks sets it for one Experiment; a changed minimum refuses resume. A shorter fixed discriminating list makes an Advisory suite, still shown with its Verdict and intervals. Overall aggregation, the Void dropout guard and required rechecks use only suites that are not advisory. All suites advisory gives inconclusive. CLI text, JSON and the Console mark Advisory suites and show judged and measurement counts. The release summary lists their Verdicts and keeps asking on fail or inconclusive. Saved Experiments without the minimum keep their original Bar and Verdict.

Trial integrity protects the issue text, declared inputs and commits under review. Review commits, local-origin pushes and report files are allowed. Review code Graders receive a private copy of the prepared repository captured before the session, so session edits cannot change the code its checks grade. Process code Graders export the final commit and replace the declared test files and shared fakes from the private reference fix before running the task’s selectors. After the Trials, the same export and overlay run each Arm's new test files on the base and on the other Arm's final commit; the results are kept per pair under the Experiment, apart from the Scores.

Trial completion removes Harness caches from each attempt's Session home, retaining native transcripts and Session evidence. Once grading and all rechecks finish, the Experiment removes every attempt's repository copy. Paused and interrupted Experiments retain repositories for resume. Requests, results, traces, reports, Grader packets and Scores stay readable; pruning failures only warn.

-

Each Trial, code Grader and model Grader gets its own systemd-run --user --scope with MemoryHigh, MemoryMax, MemorySwapMax and OOMPolicy=kill. Its process raises oom_score_adj before starting work. The User config defaults are 1 GiB, 2 GiB and 1 GiB; scopes require a reachable user manager with lingering enabled for logged-out work.

+

Each Trial, code Grader and model Grader gets its own systemd-run --user --scope with MemoryHigh, MemoryMax, MemorySwapMax and OOMPolicy=kill. Its process raises oom_score_adj before starting work. The wrapper uses the owning Experiment process's /proc/<pid>/exe link, so Trials and Graders keep its original wrapper after an update replaces or removes the install path, including scopes prepared before the update. The requested Arm programs, arguments, environment and caps stay the same. The User config defaults are 1 GiB, 2 GiB and 1 GiB; scopes require a reachable user manager with lingering enabled for logged-out work.

Ordinary Harness sessions use uncapped scopes with OOMPolicy=continue: a child tool's kernel OOM kill leaves the Harness running so it can observe the tool's failure and recover. If the Harness itself fails, its cause names the Unix signal when present and adds (scope OOM-killed) when scope evidence confirms an OOM kill.

The Experiment owns recheck history beside its immutable Grader Scores. thirdshift experiments <id> --json exposes pending borderline and decisive Scores, naming each decisive flip and its resulting provisional Verdict. thirdshift recheck <id> <trial> pass|fail --reason '…' takes the Experiment's exclusive lock and atomically saves the history, recalculated Verdict and pending list. thirdshift recheck <id> --pending lists the pending Scores and their fingerprint; thirdshift recheck <id> --batch <fingerprint> pass|fail --reason '…' checks that list under the same lock and records every listed Score in one atomic save. A stale fingerprint changes nothing. Rechecks stay in the CLI. They also update the latest invocation's Command outcome and Inbox group, preserving its completion time. The Verdict waits until the list is empty; flipping either pass/fail or a wrong-finding flag can make a Score decisive. Waiting rechecks place that invocation in Needs you; a confirmed pass moves it to Done.

scripts/release.sh runs on Linux. It refuses other hosts before Git or GitHub work. On macOS, use an Ubuntu 24.04 Linux VM such as Lima; on Windows, use WSL2.

diff --git a/site/docs/index.html b/site/docs/index.html index 639503d32..64a839162 100644 --- a/site/docs/index.html +++ b/site/docs/index.html @@ -1119,7 +1119,7 @@ 
thirdshift update
-

It replaces the installed binary with the latest stable GitHub Release, or says it is already on it. Messages go to stderr and stdout stays empty. It exits 0 when it updated or was already up to date, and 1 on any failure, such as no network. Updating is safe while a Ticket run or a Spec run is using the old binary: it keeps running it. So do the child Ticket runs it starts afterwards, a Ticket run or a Base fix, which it starts through /proc/self/exe from the binary it is running rather than from the install path, so they run its version even once that path holds the new one or nothing at all. A Ticket run never checks for updates or updates itself.

+

It replaces the installed binary with the latest stable GitHub Release, or says it is already on it. Messages go to stderr and stdout stays empty. It exits 0 when it updated or was already up to date, and 1 on any failure, such as no network. Updating is safe while a Ticket run, Spec run or Experiment is using the old binary: it keeps running it. So do the child Ticket runs it starts afterwards, a Ticket run or a Base fix, which it starts through /proc/self/exe from the binary it is running rather than from the install path, so they run its version even once that path holds the new one or nothing at all. An Experiment's later Trials and Graders keep its original OOM wrapper through the owning process's /proc/<pid>/exe link, including scopes prepared before the update. The requested Arm programs, arguments, environment and memory caps stay the same. A Ticket run never checks for updates or updates itself.

thirdshift update only replaces a copy put in place by the shell installer (the curl command above), which leaves an install receipt in ~/.config/thirdshift/. It refuses to touch any other copy and lists the command that updates each other kind of install: cargo install thirdshift for a crates.io install, pulling and reinstalling for a build from source (see Building from source), and the curl command for a copy placed by hand.

Keep a single copy: with copies in more than one directory on your PATH (say ~/.local/bin and ~/.cargo/bin), you can end up running a stale one without noticing. type -a thirdshift lists every copy on your PATH.

diff --git a/src/scope.rs b/src/scope.rs index 19d3b48b3..8c02849a1 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -726,7 +726,7 @@ fn launch>( } let mut command = scope_base(&unit, Some(caps)); command - .arg(current_exe()?) + .arg(own_executable()) .arg("--oom-exec") .arg(program) .args(args); @@ -741,8 +741,12 @@ fn launch>( }) } -fn current_exe() -> Result { - std::env::current_exe().context("could not find the running thirdshift binary") +/// Keep the running wrapper version after an update replaces or removes its +/// install path (#1498). Another process executes this path, so `self` would +/// name that process instead. Preserve the owning PID's proc link without +/// resolving it to the possibly deleted install path. +fn own_executable() -> PathBuf { + PathBuf::from(format!("/proc/{}/exe", std::process::id())) } /// Raise this process's own `oom_score_adj` to [`OOM_SCORE_ADJ`], best diff --git a/src/update.rs b/src/update.rs index 502d44936..3338b276d 100644 --- a/src/update.rs +++ b/src/update.rs @@ -5,7 +5,9 @@ //! release's installer moves the new binary into place with a rename, so a Run //! or a Spec run still using the old binary keeps running it. So do the child //! Runs it starts afterwards, which the child Run module starts from the -//! running binary rather than from its install path. +//! running binary rather than from its install path. An Experiment's later +//! Trials and Graders also keep its original OOM wrapper through the owning +//! process's `/proc//exe` link, even for scopes prepared before the update. use std::fmt::Display; diff --git a/tests/experiments.rs b/tests/experiments.rs index 6ab37f1fa..3c3ebfbba 100644 --- a/tests/experiments.rs +++ b/tests/experiments.rs @@ -66,6 +66,9 @@ mod pairs; #[path = "experiments/panel.rs"] mod panel; +#[path = "experiments/update.rs"] +mod update; + #[test] fn saved_single_level_experiments_keep_their_bar_and_verdict_when_read_and_resumed() { let fixture = Fixture::new(); diff --git a/tests/experiments/update.rs b/tests/experiments/update.rs new file mode 100644 index 000000000..2a5fae49f --- /dev/null +++ b/tests/experiments/update.rs @@ -0,0 +1,72 @@ +use super::*; + +#[test] +fn trials_and_graders_keep_running_after_the_install_path_is_replaced() { + survives_install_change(false, false); +} + +#[test] +fn trials_and_graders_keep_running_after_the_install_path_is_removed() { + survives_install_change(true, false); +} + +#[test] +fn a_scope_prepared_before_replacement_executes_the_original_wrapper() { + survives_install_change(false, true); +} + +fn survives_install_change(remove: bool, prepared: bool) { + let fixture = Fixture::with_session("spec-review", 3); + let scenario = &fixture.scenario; + let wait = scenario.waits_to_be_replaced(); + if prepared { + // Hold the first launch at the external systemd seam, after the + // Experiment has prepared its wrapper path and before it is exec'd. + support::write_executable( + &scenario.path("bin/systemd-run"), + &format!( + "#!/bin/sh\n{wait}\nwhile [ \"$#\" -gt 0 ] && [ \"$1\" != -- ]; do shift; done\nshift\nexec \"$@\"\n" + ), + ); + } + scenario.agent_does(&format!("{}{wait}", review_reports_prelude())); + let replacement_ran = scenario.path("replacement-ran"); + let result = scenario.run_copy_replaced_midway_with_env( + &Fixture::experiment_args(&[]), + &NO_SANDBOX, + |installed| { + if remove { + fs::remove_file(installed).unwrap(); + } else { + let replacement = installed.with_extension("new"); + support::write_executable( + &replacement, + &format!("#!/bin/sh\ntouch {}\nexit 1\n", replacement_ran.display()), + ); + fs::rename(replacement, installed).unwrap(); + } + }, + ); + + assert_eq!(result.code, Some(0), "{}", result.stderr); + assert!(!replacement_ran.exists(), "the replacement wrapper ran"); + let experiment: Value = serde_json::from_str(&result.stdout).unwrap(); + let results = experiment["results"].as_array().unwrap(); + assert_eq!(results.len(), 6, "later Trials must still dispatch"); + for result in results { + assert_eq!(result["outcome"], "succeeded", "{result}"); + assert_eq!(result["score"]["passed"], true, "{result}"); + } + assert_eq!(fixture.trial_calls().len(), 6); + assert!(!fixture.grader_calls().is_empty(), "Graders must run too"); + for call in fixture.grader_calls() { + let directory = PathBuf::from(call["cwd"].as_str().unwrap()); + assert_eq!( + fs::read_to_string(directory.join("scores.json.adj")) + .unwrap() + .trim(), + "1000", + "the original OOM wrapper must run" + ); + } +} diff --git a/tests/support/mod.rs b/tests/support/mod.rs index e0b4c929f..59c8c60a6 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -655,11 +655,23 @@ impl Scenario { &self, args: &[&str], replace: impl FnOnce(&Path), + ) -> RunResult { + self.run_copy_replaced_midway_with_env(args, &[], replace) + } + + /// [`Scenario::run_copy_replaced_midway`] with child-only environment overrides. + pub fn run_copy_replaced_midway_with_env( + &self, + args: &[&str], + env: &[(&str, &str)], + replace: impl FnOnce(&Path), ) -> RunResult { let copy = self.path("installed/thirdshift"); fs::create_dir_all(copy.parent().unwrap()).unwrap(); copy_executable(Path::new(env!("CARGO_BIN_EXE_thirdshift")), ©); - let held = self.spawn_until_started(self.command_running(©, args), COPY_IN_USE); + let mut command = self.command_running(©, args); + command.envs(env.iter().copied()); + let held = self.spawn_until_started(command, COPY_IN_USE); replace(©); fs::write(self.path(COPY_REPLACED), "").unwrap(); held.finish()