From 14549de496304f42f877d713b3978c38535c58e0 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Tue, 10 Nov 2026 22:41:57 -0500 Subject: [PATCH 1/2] Fix waiting-line configuration for Process and Review runs (#1983) --- src/command.rs | 1 - src/evals/experiment.rs | 1 - src/logs.rs | 4 ++- tests/ci.rs | 79 +++++++++++++++++++++++++++++++++++++++++ tests/command_logs.rs | 57 +++++++++++++++++++++++++++++ 5 files changed, 139 insertions(+), 3 deletions(-) diff --git a/src/command.rs b/src/command.rs index f8d34368f..7df3767af 100644 --- a/src/command.rs +++ b/src/command.rs @@ -113,7 +113,6 @@ pub fn start( .and_then(|()| UserConfig::load()) .map_err(|error| failure(&error))?; logs::configured(&config); - crate::poll::configure_waiting_lines(config.effective_waiting_line_interval()); // First, so no interrupt can end the command once its notification is // checked. interrupt::install().map_err(|error| failure(&error))?; diff --git a/src/evals/experiment.rs b/src/evals/experiment.rs index bbef480dc..3fb9ee3dc 100644 --- a/src/evals/experiment.rs +++ b/src/evals/experiment.rs @@ -406,7 +406,6 @@ pub fn run(options: Options) -> Result<()> { ); logs::begin(logs::Begin::Experiment); logs::configured(&config); - crate::poll::configure_waiting_lines(config.effective_waiting_line_interval()); let repo = Repo { owner: "JacobStephens2".into(), name: "thirdshift".into(), diff --git a/src/logs.rs b/src/logs.rs index 55b1a00f3..e4f0fddc8 100644 --- a/src/logs.rs +++ b/src/logs.rs @@ -309,8 +309,10 @@ pub(crate) fn tickets(tickets: Vec) { /// `activity.quiet_skips` is set, show the lines a pass held from the /// terminal and every line from here on. With it set, a pass's lines stay /// held until it starts work, ends or fails, so a skipped pass prints -/// nothing. +/// nothing. Capture the Command's effective waiting-line interval before +/// any waiting stages begin. pub fn configured(config: &UserConfig) { + crate::poll::configure_waiting_lines(config.effective_waiting_line_interval()); record().configured(config); } diff --git a/tests/ci.rs b/tests/ci.rs index b7cd02372..80bc4cb98 100644 --- a/tests/ci.rs +++ b/tests/ci.rs @@ -266,6 +266,85 @@ fn unchanged_running_checks_keep_writing_waiting_lines_to_the_command_log() { } } +#[test] +fn custom_process_ci_waiting_lines_use_the_configured_interval() { + for config in [ + "[logs]\nwaiting_line_minutes = 1\n", + "[logs]\nwaiting_line_minutes = 20\n[inbox]\nsilent_minutes = 4\n", + ] { + custom_process_ci_waiting_lines(config); + } +} + +fn custom_process_ci_waiting_lines(config: &str) { + let scenario = Scenario::new(); + scenario.user_config_is(config); + let processes = scenario.path("home/.thirdshift/processes"); + std::fs::create_dir_all(&processes).unwrap(); + std::fs::write( + processes.join("ship.toml"), + include_str!("../processes/ticket-run.toml"), + ) + .unwrap(); + scenario.agent_does(&format!( + "{AGENT_OPENS_PR}{}", + checks_on_head(r#"[{"name":"test","conclusion":"success","pending_polls":14}]"#) + )); + + assert_ci_waiting_lines(&scenario, &["process", "ship", "issue", "7"], "process"); +} + +#[test] +fn review_ci_waiting_lines_use_the_configured_interval() { + for author in [false, true] { + for config in [ + "[logs]\nwaiting_line_minutes = 1\n", + "[logs]\nwaiting_line_minutes = 20\n[inbox]\nsilent_minutes = 4\n", + ] { + let scenario = Scenario::new(); + scenario.user_config_is(config); + scenario.origin_has_branch("colleague", "main", &["Colleague feature"]); + let url = scenario.github_has_pr("colleague", "main", "OPEN"); + scenario.agent_does(&format!( + "{}printf 'No findings.' > \"$FAKE_CLAUDE_FINAL_MESSAGE\"\n", + checks_on_head(r#"[{"name":"test","conclusion":"success","pending_polls":14}]"#) + )); + let args = if author { + vec!["review", "--author", "colleague"] + } else { + vec!["review", &url] + }; + + assert_ci_waiting_lines(&scenario, &args, "review"); + } + } +} + +fn assert_ci_waiting_lines(scenario: &Scenario, args: &[&str], command: &str) { + let result = scenario.run_with_env(args, &[("THIRDSHIFT_WAITING_MINUTE_MS", "150")]); + + assert_eq!(result.code, Some(0), "{}", result.stderr); + let waiting = "1 still running out of 1 checks listed so far"; + assert!( + result.stderr.matches(waiting).count() >= 3, + "expected repeated CI waiting lines, got {}:\n{}", + result.stderr.matches(waiting).count(), + result.stderr + ); + let log = std::fs::read_dir(scenario.path(&format!( + "home/.thirdshift/logs/acme/widgets/commands/{command}" + ))) + .unwrap() + .map(|entry| entry.unwrap().path()) + .find(|path| path.extension().is_some_and(|ext| ext == "log")) + .unwrap(); + let log = std::fs::read_to_string(log).unwrap(); + assert_eq!( + log.matches(waiting).count(), + result.stderr.matches(waiting).count() + ); +} + #[test] fn ci_watch_uses_half_the_silent_time_when_the_waiting_interval_is_too_long() { let scenario = Scenario::new(); diff --git a/tests/command_logs.rs b/tests/command_logs.rs index 3e6dacd45..68a5bdb48 100644 --- a/tests/command_logs.rs +++ b/tests/command_logs.rs @@ -91,6 +91,63 @@ fn the_one_command_log(scenario: &Scenario, folder: &str, prefix: &str) -> (Path (path, format!("{stamp}~{token}")) } +#[test] +fn process_pass_and_issue_waiting_lines_reach_the_command_log() { + for issue in [false, true] { + for config in [ + "[logs]\nwaiting_line_minutes = 1\n", + "[logs]\nwaiting_line_minutes = 20\n[inbox]\nsilent_minutes = 4\n", + ] { + let scenario = Scenario::new(); + scenario.user_config_is(&format!( + "{config}[resources]\nmin_available_memory_mb = 9007199254740991\n" + )); + let processes = scenario.path("home/.thirdshift/processes"); + fs::create_dir_all(&processes).unwrap(); + fs::write( + processes.join("notes.toml"), + "version = 1\ndescription = 'Take notes'\n[[steps]]\nname = 'notes'\nskill = 'code-review'\n", + ) + .unwrap(); + scenario.agent_does("printf 'Done.' > \"$FAKE_CLAUDE_FINAL_MESSAGE\""); + let blocked = |_: u32| assert!(scenario.claude_calls().is_empty()); + let release = |pid: u32| { + blocked(pid); + scenario.user_config_is(""); + }; + let waiting = "waiting for host resources:"; + let mut args = vec!["process", "notes"]; + if issue { + args.extend(["issue", "7"]); + } + + let result = scenario.run_terminal( + &args, + &[ZONE, ("THIRDSHIFT_WAITING_MINUTE_MS", "150")], + &[ + support::TerminalStep::inspect(waiting, &blocked), + support::TerminalStep::inspect(waiting, &blocked), + support::TerminalStep::inspect(waiting, &release), + ], + ); + + assert_eq!(result.code, Some(0), "{}", result.stderr); + assert_eq!(scenario.claude_calls().len(), 1); + assert!( + result.stderr.matches(waiting).count() >= 3, + "{}", + result.stderr + ); + let (path, _) = the_one_command_log(&scenario, "process", ""); + let log = fs::read_to_string(path).unwrap(); + assert_eq!( + log.matches(waiting).count(), + result.stderr.matches(waiting).count() + ); + } + } +} + #[test] fn self_merge_keeps_writing_waiting_lines_while_github_calculates_mergeability() { let scenario = Scenario::new(); From c9fa1a38f9e45e79ee730738879c40240d7f5f43 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Tue, 10 Nov 2026 22:44:39 -0500 Subject: [PATCH 2/2] Share CI heartbeat assertions across Ticket and Process tests --- tests/ci.rs | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/tests/ci.rs b/tests/ci.rs index 80bc4cb98..bcdbf8593 100644 --- a/tests/ci.rs +++ b/tests/ci.rs @@ -237,28 +237,10 @@ fn unchanged_running_checks_keep_writing_waiting_lines_to_the_command_log() { args.push("merge".to_string()); } - let result = scenario.run_with_env( + assert_ci_waiting_lines( + &scenario, &args.iter().map(String::as_str).collect::>(), - &[("THIRDSHIFT_WAITING_MINUTE_MS", "150")], - ); - - assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); - let waiting = "1 still running out of 1 checks listed so far"; - assert!( - result.stderr.matches(waiting).count() >= 3, - "{}", - result.stderr - ); - let logs = scenario.path("home/.thirdshift/logs/acme/widgets/commands/issue"); - let log = std::fs::read_dir(logs) - .unwrap() - .map(|entry| entry.unwrap().path()) - .find(|path| path.extension().is_some_and(|ext| ext == "log")) - .unwrap(); - let log = std::fs::read_to_string(log).unwrap(); - assert_eq!( - log.matches(waiting).count(), - result.stderr.matches(waiting).count() + "issue", ); if merge { assert_eq!(scenario.gh_state()["prs"][0]["state"], "MERGED");