Skip to content

Implementation Plan: Fix xdist Isolation — Hardcoded /dev/shm and /tmp Paths (groupB) #563

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Trecek merged 1 commit into from
Mar 28, 2026
Merged

Implementation Plan: Fix xdist Isolation — Hardcoded /dev/shm and /tmp Paths (groupB) #563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 deletions tests/execution/test_headless.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2104,7 +2104,9 @@ class TestCrashSessionLog:
"""flush_session_log is called with success=False when runner raises."""

@pytest.mark.anyio
async def test_crash_session_log_written_when_runner_raises(self, monkeypatch, tool_ctx):
async def test_crash_session_log_written_when_runner_raises(
self, monkeypatch, tool_ctx, tmp_path
):
"""flush_session_log is called with CRASHED termination_reason when runner raises."""
from autoskillit.execution.headless import run_headless_core

Expand All @@ -2121,7 +2123,7 @@ async def raising_runner(*args: object, **kwargs: object) -> None:
tool_ctx.runner = raising_runner # type: ignore[assignment]

with pytest.raises(RuntimeError, match="simulated crash"):
await run_headless_core("/investigate test", cwd="/tmp", ctx=tool_ctx)
await run_headless_core("/investigate test", cwd=str(tmp_path), ctx=tool_ctx)
Copy link
Copy Markdown
Collaborator Author

@Trecek Trecek Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[info] cohesion: Passes str(tmp_path) directly as cwd without creating a subdirectory (e.g. tmp_path / 'proj'), while tests in test_headless_add_dirs.py create an explicit proj subdirectory. The cwd construction pattern differs across the diff.


crash_calls = [f for f in flushed if f.get("termination_reason") == "CRASHED"]
assert len(crash_calls) >= 1
Expand Down
10 changes: 7 additions & 3 deletions tests/execution/test_headless_add_dirs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def factory(runner=None):


@pytest.mark.anyio
async def test_run_headless_core_no_add_dir_when_empty(make_ctx):
async def test_run_headless_core_no_add_dir_when_empty(make_ctx, tmp_path):
"""T-OVR-012: run_headless_core with empty add_dirs emits no --add-dir flags."""
from autoskillit.execution.headless import run_headless_core
from tests.conftest import _make_result
Expand All @@ -38,7 +38,9 @@ async def mock_runner(cmd, **kwargs):
return _make_result()

ctx = make_ctx(runner=mock_runner)
await run_headless_core("/autoskillit:investigate foo", "/tmp/proj", ctx, add_dirs=())
proj = tmp_path / "proj"
Copy link
Copy Markdown
Collaborator Author

@Trecek Trecek Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[info] cohesion: Creates proj = tmp_path / 'proj' with explicit mkdir(). Pattern is consistent within the file; note that test_headless.py passes str(tmp_path) directly as cwd without a subdirectory — minor inconsistency across the diff.

proj.mkdir()
await run_headless_core("/autoskillit:investigate foo", str(proj), ctx, add_dirs=())
assert "--add-dir" not in captured_cmd


Expand All @@ -64,9 +66,11 @@ async def mock_runner(cmd, **kwargs):
return _make_result()

ctx = make_ctx(runner=mock_runner)
proj = tmp_path / "proj"
proj.mkdir()
await run_headless_core(
"/autoskillit:investigate foo",
"/tmp/proj",
str(proj),
ctx,
add_dirs=[dir_a, dir_b],
)
Expand Down
3 changes: 3 additions & 0 deletions tests/workspace/test_session_skills.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
def test_resolve_ephemeral_root_returns_writable_dir(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
import autoskillit.workspace.session_skills as ss

monkeypatch.setattr(ss, "_CANDIDATE_ROOTS", [tmp_path])
Copy link
Copy Markdown
Collaborator Author

@Trecek Trecek Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[warning] cohesion: monkeypatch.setattr(ss, '_CANDIDATE_ROOTS', [tmp_path]) injects tmp_path directly as a candidate root rather than a subdirectory. Both tests in test_headless_add_dirs.py construct tmp_path / 'proj' via mkdir(). The convention for how tmp_path is used (raw vs. subdirectory) is inconsistent across the three files in this diff. Consider using tmp_path / 'root' with mkdir() for consistency.

Copy link
Copy Markdown
Collaborator Author

@Trecek Trecek Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigated — this is intentional. resolve_ephemeral_root() at session_skills.py:54 appends 'autoskillit-sessions' to each candidate root, so tmp_path as a candidate root means all I/O lands in tmp_path/autoskillit-sessions/ — correctly isolated. test_resolve_ephemeral_root_fallback (line 37) also passes tmp_path directly without issue. The _CANDIDATE_ROOTS pattern (base dirs) and the cwd argument (project dirs) are different abstractions; surface consistency across them would be misleading.

root = resolve_ephemeral_root()
assert root.exists()
assert root.is_dir()
Expand Down
Loading