From c943f12abec8505b608a79ad6d3a67da54305091 Mon Sep 17 00:00:00 2001
From: "Andrei G." <k05h31@gmail.com>
Date: Tue, 28 Jul 2026 14:19:37 +0200
Subject: [PATCH] fix: remove wall-clock-timing races from
 performance_agent_integration tests

agent_integration_with_safe_bash_blocks asserted elapsed < 1000ms around a
fully-mocked Agent::run() call and intermittently failed under CI load.
agent_integration_no_bash_blocks had the same anti-pattern (already widened
once for this exact class, 500ms -> 2000ms, issue #3649) with insufficient
headroom against observed failure times. Drop both timing assertions and
keep the assertions that carry real signal: tool dispatch count and that
the tool result reached the output channel.
---
 CHANGELOG.md                           | 15 ++++++++++++
 tests/performance_agent_integration.rs | 34 ++++++++++++--------------
 2 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 91058d082..49b94a0ea 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -210,6 +210,21 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
   `TestWriteGate` channel handshake (with a 30s bound so a regression to an inline write path
   fails with a clear panic instead of hanging the CI shard) to prove `dump_request` returns
   before its `spawn_blocking` write completes, with no timing race window at all.
+- top-level integration tests: removed flaky wall-clock budgets from
+  `agent_integration_with_safe_bash_blocks` and `agent_integration_no_bash_blocks`
+  (`tests/performance_agent_integration.rs`, issue #6687), the same defect class as issue
+  #6679/#6685 but in a different crate/file that PR didn't touch. Both assertions raced a
+  fully-mocked `Agent::run()` call against a fixed threshold and intermittently failed under
+  CI load (observed 1.247s and 1.610s on PR #6678 against `agent_integration_with_safe_bash_blocks`'s
+  1000ms budget); `agent_integration_no_bash_blocks`'s 2000ms budget had already been widened
+  once for this exact class (500ms -> 2000ms, issue #3649) and was left with only ~24% headroom
+  against those same observed elapsed times, so it was dropped rather than widened again.
+  Dropped the timing checks entirely: the tests' real assertions of value —
+  `executor.get_call_count()` proving whether the native `tool_use` path was taken, plus
+  asserting the response/tool-result actually reached the output channel — are unaffected.
+  A genuine hang is not caught by nextest's `slow-timeout` (warn-only, no `terminate-after`
+  configured); it surfaces only coarsely, as CI's job-level `timeout-minutes: 10` killing the
+  whole shard.
 - `zeph-acp`: fixed a session-turn race where a closed/deleted session's agent-loop task could
   keep running and emit events/notifications under a `SessionId` reused by a later
   `session/load`/`session/resume` (issue #6674). `do_close_session`/`do_delete_session` previously
diff --git a/tests/performance_agent_integration.rs b/tests/performance_agent_integration.rs
index 739bd26ac..a8adcce85 100644
--- a/tests/performance_agent_integration.rs
+++ b/tests/performance_agent_integration.rs
@@ -186,17 +186,13 @@ async fn agent_integration_no_bash_blocks() {
         executor.clone(),
     );
 
-    let start = Instant::now();
     let _ = agent.run().await;
-    let elapsed = start.elapsed();
-
-    // Should complete within a generous bound; the agent uses mocks with no real I/O.
-    // 2 s accounts for slow CI runners while still catching genuine regressions.
-    assert!(
-        elapsed.as_millis() < 2000,
-        "Agent run should be fast for non-bash response: {elapsed:?}",
-    );
 
+    // No wall-clock budget here (see #6687): this class of assertion already flaked once at
+    // 500ms and again at the widened 2000ms (observed 1.247s/1.610s on PR #6678, ~24% headroom
+    // under CI load) — widening is a proven non-fix, so it's dropped rather than widened again.
+    // A genuine hang is caught only coarsely, at the shard level, by CI's job-level
+    // `timeout-minutes: 10` (.github/workflows/ci.yml).
     // Plain text response doesn't trigger tool execution (native tool_use path)
     assert_eq!(executor.get_call_count(), 0);
 
@@ -223,18 +219,20 @@ async fn agent_integration_with_safe_bash_blocks() {
         executor.clone(),
     );
 
-    let start = Instant::now();
     let _ = agent.run().await;
-    let elapsed = start.elapsed();
 
-    // Should complete reasonably
-    assert!(
-        elapsed.as_millis() < 1000,
-        "Agent run should complete: {elapsed:?}",
-    );
+    // No wall-clock budget here (see #6687): the test's real assertion of value is that the
+    // native tool_use path was taken. A genuine hang is caught only coarsely, at the shard
+    // level, by CI's job-level `timeout-minutes: 10` (.github/workflows/ci.yml) — nextest's
+    // `slow-timeout` is warn-only with no `terminate-after` configured, so it never kills a
+    // hung test.
+    // Native tool_use path calls execute_tool_call exactly once (one scripted ToolUse response
+    // followed by Text, one channel input).
+    assert_eq!(executor.get_call_count(), 1);
 
-    // Native tool_use path calls execute_tool_call at least once
-    assert!(executor.get_call_count() >= 1);
+    // The tool result was fed back and the turn completed.
+    let outputs = output_sent.lock().unwrap();
+    assert!(outputs.iter().any(|m| m.contains("Done.")));
 }
 
 #[tokio::test]