fix(orchestrator): prevent work directory collisions (#29)#35
fix(orchestrator): prevent work directory collisions (#29)#35
Conversation
Add Integration Constraints section to limitations.md: - One first::test() per #[test] - Async tests not supported - Not thread-safe - No nested workspaces Add limitations summary to lib.rs crate docs. Closes #18
- Rewrite README for conciseness and professional formatting - Switch license from MIT to Apache 2.0 - Update project logo - Rewrite all architecture docs (~60% reduction) - Rewrite limitations.md and proof/reference_wal.md - Remove empty invariants.md
- Generate unique base directory per orchestrator run using UUID + PID - Pass base directory to child processes via FIRST_WORK_DIR - Derive leaf run_N directory in child process from base + target - Update extract_test_name to use thread name for better isolation - Add reproduction test case (tests/repro_issue_29.rs) - Closes #29
There was a problem hiding this comment.
3 issues found across 15 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/orchestrator.rs">
<violation number="1" location="src/orchestrator.rs:13">
P2: This import is incorrectly placed after a doc comment that was meant for the removed `FIRST_BASE_DIR` constant. Move the import to the top of the file with other imports, and either remove the orphaned doc comment or add a new constant/comment explaining the base directory pattern (now dynamically generated in the `run` function).</violation>
</file>
<file name="docs/architecture/004_orchestrator.md">
<violation number="1" location="docs/architecture/004_orchestrator.md:10">
P3: The core-loop doc still describes the old shared `/tmp/first/run_{target}` path, but the orchestrator now creates a unique base directory per run. Update the documentation to reflect the base directory + run_N layout.</violation>
</file>
<file name="docs/architecture/002_crash_point.md">
<violation number="1" location="docs/architecture/002_crash_point.md:35">
P3: Doc example still shows `/tmp/first/run_5`, but the runtime now uses a unique base dir (`/tmp/first/first-<pid>-<uuid>/run_5`). Update the example to match the new base directory scheme so users copy the correct paths.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| loop { | ||
| 1. Create work_dir: /tmp/first/run_{target} | ||
| 2. Spawn EXECUTION child with target | ||
| 1. Create /tmp/first/run_{target} |
There was a problem hiding this comment.
P3: The core-loop doc still describes the old shared /tmp/first/run_{target} path, but the orchestrator now creates a unique base directory per run. Update the documentation to reflect the base directory + run_N layout.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/architecture/004_orchestrator.md, line 10:
<comment>The core-loop doc still describes the old shared `/tmp/first/run_{target}` path, but the orchestrator now creates a unique base directory per run. Update the documentation to reflect the base directory + run_N layout.</comment>
<file context>
@@ -1,136 +1,64 @@
loop {
- 1. Create work_dir: /tmp/first/run_{target}
- 2. Spawn EXECUTION child with target
+ 1. Create /tmp/first/run_{target}
+ 2. Spawn EXECUTION child
3. Wait for exit:
</file context>
| 1. Create /tmp/first/run_{target} | |
| 1. Create /tmp/first/first-<pid>-<uuid>/run_{target} |
| **Format (JSON to stderr before kill):** | ||
| ```json | ||
| {"event":"crash","point_id":42,"label":"after_wal_write","seed":12345,"work_dir":"/tmp/first/run_42"} | ||
| {"event":"crash","point_id":5,"label":"after_commit","seed":null,"work_dir":"/tmp/first/run_5"} |
There was a problem hiding this comment.
P3: Doc example still shows /tmp/first/run_5, but the runtime now uses a unique base dir (/tmp/first/first-<pid>-<uuid>/run_5). Update the example to match the new base directory scheme so users copy the correct paths.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/architecture/002_crash_point.md, line 35:
<comment>Doc example still shows `/tmp/first/run_5`, but the runtime now uses a unique base dir (`/tmp/first/first-<pid>-<uuid>/run_5`). Update the example to match the new base directory scheme so users copy the correct paths.</comment>
<file context>
@@ -1,128 +1,51 @@
-**Format (JSON to stderr before kill):**
```json
-{"event":"crash","point_id":42,"label":"after_wal_write","seed":12345,"work_dir":"/tmp/first/run_42"}
+{"event":"crash","point_id":5,"label":"after_commit","seed":null,"work_dir":"/tmp/first/run_5"}
</file context>
</details>
```suggestion
{"event":"crash","point_id":5,"label":"after_commit","seed":null,"work_dir":"/tmp/first/first-<pid>-<uuid>/run_5"}
Closes #29.
This PR resolves issue #29 by introducing unique base directories for each test run, ensuring that parallel execution of tests via
cargo testis safe and free from filesystem collisions.Changes:
/tmp/first/first-<pid>-<uuid>) for each run.FIRST_WORK_DIR.run_Ndirectory from the base directory, ensuring strict ownership by the orchestrator.extract_test_nameto use thread name fallback.uuidcrate.tests/repro_issue_29.rs.Summary by cubic
Prevents work directory collisions during parallel test runs by generating a unique base directory per orchestrator run and deriving per-target run_N paths from it. Improves test isolation and reliability; also updates documentation and switches the project to Apache 2.0.
Bug Fixes
Refactors
Written for commit 6d959d2. Summary will update on new commits.