test(aw-client-rust): read wait_for_start mock connections concurrently - #783

Confidence Score: 5/5

[Low risk] Test code updates for concurrent mock connections.

The PR appears safe to merge; no outstanding blocking issue was identified.

Summary

The PR makes the wait-for-start test mock read accepted connections concurrently and bounds its wait for a request.

• Expands the silent-connection regression test and its retry margin.
• Updates test-only Chrono and iterator calls and removes duplicate parser lint allowances.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Mock listener] --> B[Accept connection]
  B --> C[Spawn request reader]
  C --> D{Complete request?}
  D -- No --> E[Keep reading or close]
  D -- Yes --> F[Send mock response]
  A --> G[10 s reply deadline]
  G --> H[Fail test if no request arrives]

_{Reviews (2) · Last reviewed commit: "test(aw-client-rust): bound the concurre..."}

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.01%. Comparing base (656f3c9) to head (5eacbb7).
⚠️ Report is 185 commits behind head on master.

@@             Coverage Diff             @@
##           master     #783       +/-   ##
===========================================
+ Coverage   70.81%   84.01%   +13.19%     
===========================================
  Files          51       82       +31     
  Lines        2916    10798     +7882     
===========================================
+ Hits         2065     9072     +7007     
- Misses        851     1726      +875     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

@greptileai review

CI-green and mergeable (Greptile 5/5) — waiting only on a maintainer click.

This PR is ready to merge, but the bot has pull-only access to this repo and can't self-merge — surfacing it here so it isn't lost. The monitoring loop will stop re-flagging it now that this note is posted.

🤖 AI code review

This PR reworks the mock HTTP server in aw-client-rust's unit tests so that accepted connections are read concurrently via a tokio JoinSet, answering the first connection that sends a complete request instead of serially skipping silent ones. It also updates the silent-connection test to queue 16 silent connections, adjusts test deadlines, replaces deprecated chrono DateTime::from_utc calls with from_naive_utc_and_offset, and switches std::iter::repeat to repeat_n in aw-transform's classify test.

Safe to merge — no P0/P1 findings

Confidence 5/5

✅ No findings. The diff looks correct to me on this pass.

_{Reviewed 5eacbb7956c2 · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 173s · about this reviewer}

Rebased onto master (fcc1ede) to clear the conflict.

Dropped fix(aw-query): remove duplicated clippy allow attributes. Master already fixed the same duplicated_attributes lint in #771 and a follow-up: it removed block_scrutinee from the outer #[allow] in lib.rs and kept it inside parser.rs. This PR's version did the opposite, so I kept master's. The remaining 4 commits applied cleanly.

Checked locally with stable 1.99: cargo clippy --workspace -- -D warnings passes (same command as CI), cargo fmt --check passes, and the aw-client-rust and aw-transform tests pass.

Correction to my earlier "CI-green, waiting on a maintainer click": don't merge this yet.

windows-latest on the rebased head d61b2c8d fails the exact test this PR targets (job):

---- tests::test_wait_for_start_retries_without_blocking_executor stdout ----
panicked at aw-client-rust\src\lib.rs:638:18:
"Server at http://127.0.0.1:49658 not responding after 3 seconds of retrying"
test result: FAILED. 29 passed; 1 failed; ... finished in 3.28s

The 4 earlier Windows runs on this branch passed, so the concurrent mock lowers the flake rate but doesn't remove it. The cause isn't only stale connections queued ahead of the live one: with concurrent reads, a live attempt that reached the listener would have been answered. So, after the listener is up, every attempt in the 3 s window must be failing before the mock sees a request. That points at the client/connect side on Windows (connect to a bound-but-not-listening socket, SYN retry after RST), not at the mock's read order.

(I can't re-run jobs here, and a green rerun wouldn't validate the fix anyway.) Next step: instrument the attempts (per-attempt error kind and elapsed time) on Windows CI to find which phase times out, before claiming a root cause.

Diagnostic PR #796 opened to get DIAG attempt logs from Windows CI. It adds an eprintln! per attempt (start offset, elapsed, error kind) on top of this branch's current head.

Once those logs land I'll post the relevant output here and revise the fix.

Windows attempt logs from the diagnostic in #796 are in. The 20-iteration windows-latest loop ran on two heads: f612b06 and 8dd5e1c. Both use the 3 s deadline, i.e. before 5eacbb79 widened it.

• 0/40 failures. The 3.28 s failure on d61b2c8d did not reproduce.
• Every run has the same two attempts:

  DIAG attempt start=100ns     took=502.391ms result=Err(reqwest::Error { kind: Request, source: TimedOut })
  DIAG attempt start=608.8108ms took=1.7567ms result=Ok(200)

  The first attempt fires before the listener is up and hangs on connect instead of being refused, which is the Windows behaviour this PR's doc comment describes. The 500 ms per-attempt bound cuts it off, and the retry is answered in ~2 ms. The mock never delayed a live attempt.

The first attempt costs ~0.6 s of a 3 s budget, so a single failure would take ~5 consecutive hung attempts. The logs don't show what caused the one CI failure; my best guess is runner stall, and that is unconfirmed. Widening the deadline to 10 s in 5eacbb79 gives that case margin without hiding a real regression, since a healthy run still finishes in ~0.6 s. windows-latest is green on 5eacbb79.

So I'm lifting my earlier hold: this is mergeable once CI finishes, and macOS is still queued. Closing #796, since it has served its purpose.