Skip to content

test: prevent executable-copy races in namespace tests - #38

Merged
marcinpsk merged 1 commit into
developfrom
fix/namespace-test-executable
Oct 17, 2026
Merged

marcinpsk merged 1 commit into
developfrom
fix/namespace-test-executable

Conversation

@marcinpsk

@marcinpsk marcinpsk commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Parallel namespace tests intermittently fail to start the subagent with ExecutableFileBusy (Text file busy). This is the current failure on Dependabot PRs #33 and #34. The test process copies an executable while another thread can fork and inherit its writable descriptor. Closing the parent descriptor does not close the child's inherited descriptor before execution.

Run the executable copy in a short-lived cp process and require successful completion before launch. The test process no longer owns a writable executable descriptor. Keep the existing file permissions and unprivileged identity. Add a real-process regression that starts 64 subagents while another thread forks children that briefly retain inherited descriptors. The tests remain parallel.

Validation:

  • The regression reproduced the exact original error in 0.12 seconds before the fix, then passed after the fix.
  • cargo fmt --check, strict Clippy, and the complete Cargo suite pass locally with Rust 1.99.0.
  • All seven privileged namespace tests pass with the CI container restrictions and the default parallel test pool.
  • Read-only adversarial review found no defects.

All four open Dependabot PRs were inspected. After requesting rebases on #32, #33, and #35, all four pass the workflow audit. The code checks pass on #32 and #35; #33 and #34 hit this separate executable-copy race. This fix targets develop. The dependency PRs target main and need the fix on their base before rebasing can pick it up.

Summary by CodeRabbit

  • Tests
    • Expanded coverage for starting and connecting test subagents while other processes are active.
    • Added checks for handling startup failures and ensuring background test activity is stopped and joined afterward.
  • Reliability
    • The test setup now verifies that copying the test executable succeeds before continuing, while preserving its executable permissions.

@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: fd8fc0cb-35a3-4862-898f-31734123d53a
📥 Commits

Reviewing files that changed from the base of the PR and between 9fe5f7e and b3000d9.

📒 Files selected for processing: 1
  • tests/real_namespace.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The test binary copy now uses cp -- and checks for success. A new test starts and connects 64 test-identity subagents while concurrent /bin/true children run.

Changes

Subagent startup test

Layer / File(s) Summary
Startup setup and concurrency test
tests/real_namespace.rs
The test setup checks the result of copying the test binary. A new test runs concurrent child processes during 64 subagent startup and connection attempts, then stops and joins the child-process loop. If an assertion panics, the test resumes the captured panic after joining the loop.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to b3000

The executable-copy change and new startup stress test have no identified issue requiring a fix before merge.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: preventing executable-copy races in namespace tests.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
✨ Simplify code
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

A rabbit checks the copy with care,
Then starts subagents in the air.
While child processes briefly run,
It joins the loop when tests are done.
The burrow hums; the checks all pass.

Comment @coderabbitai help to get the list of available commands.

@marcinpsk
marcinpsk merged commit e6c0268 into develop Oct 17, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant