fix: harden fake-gemini test double against stdin broken-pipe race - #111
Conversation
ai_assess_writes_llm_invocation_audit_row_via_runner intermittently failed under heavy parallel cargo test --lib load with "failed to write Gemini stdin: Broken pipe (os error 32)". The fake-gemini.sh test double never read stdin before exiting 0, racing against run_gemini_assessment's concurrent stdin-writer task under scheduler pressure. Draining stdin (cat >/dev/null) before responding forces the child to block until the parent's write+shutdown completes, removing the race deterministically. Closes syslog-mcp-sxx56.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR hardens a unit-test fake Gemini CLI script to eliminate an intermittent broken-pipe race when run_gemini_assessment writes to child stdin concurrently under heavy parallel test scheduling. It also applies the repo’s patch-version bump workflow for the fix.
Changes:
- Update the test double
fake-gemini.shto drain stdin (cat >/dev/null) before emitting stream-json output, preventing the child from exiting before the parent finishes writing+shutdown. - Bump version
3.2.2 → 3.2.3across the tracked version carriers. - Add a
3.2.3changelog entry documenting the flaky-test fix.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/app/service_tests.rs |
Makes the fake gemini test script block on stdin EOF to remove a deterministic EPIPE race during concurrent stdin writing. |
server.json |
Updates MCP registry metadata version and OCI identifier tag to v3.2.3. |
mcpb/manifest.json |
Updates MCP bundle manifest version to 3.2.3. |
docker-compose.prod.yml |
Updates default image tag fallback to 3.2.3. |
CHANGELOG.md |
Adds the 3.2.3 entry describing the flaky-test fix. |
Cargo.toml |
Bumps crate version to 3.2.3. |
Cargo.lock |
Updates the locked cortex package version to 3.2.3. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
ai_assess_writes_llm_invocation_audit_row_via_runnerintermittently failed under heavy parallelcargo test --libload withfailed to write Gemini stdin: Broken pipe (os error 32).geminiscript (fake-gemini.sh) insrc/app/service_tests.rsexited without ever reading stdin, racing againstrun_gemini_assessment's concurrent stdin-writer task — under scheduler pressure the child could exit and close its stdin read end before the parent finished writing, surfacing a spuriousBroken pipeeven though stdout already produced valid, complete output.cat >/dev/null) before responding, forcing it to block until the parent's write+shutdown completes. Test-only change — production code (src/assessment.rs) is untouched.Closes syslog-mcp-sxx56.
Test plan
Broken pipeerror under current system load)cargo test --lib app::services::tests::ai_assess_writes_llm_invocation_audit_row_via_runner— passed 6/6 consecutive runs after the fixcargo test --lib assessment::tests::— all 12 tests pass, including the intentionally-racygemini_assessment_reports_child_stderr_before_stdin_pipe_error(confirms the fix doesn't mask the exit-status-priority behavior that test depends on)cargo fmt -- --checkcargo clippy --all-targets -- -D warningscargo xtask check-release-versionsSummary by cubic
Fixes a flaky unit test by making the fake
geminitest double read stdin to EOF (cat >/dev/null) before printing, removing the broken-pipe race under parallel runs. Test-only change; bumpscortexto 3.2.3 and updates Docker/manifest versions to match (addresses syslog-mcp-sxx56).Written for commit e7d3572. Summary will update on new commits.