Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
34 changes: 16 additions & 18 deletions tests/performance_agent_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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]
Expand Down
Loading