fix(standards): Windows CI test fixes + architecture-checker template-scaffold exemption
Follow-up to 4105a7e. CI Windows Test surfaced 3 Windows-only test failures where
the sweep cross-platformed the code but tests still asserted POSIX behavior, plus
a fleet-wide architecture ripple from the template scaffold test:
- ai_mail: 3 Popen detach sites now use creationflags=CREATE_NEW_PROCESS_GROUP on
win32 / start_new_session on POSIX (real win32 detachment); test asserts the
platform-correct kwargs.
- drone: test_rm.py /tmp assertions guarded to POSIX-only (win32 uses
tempfile.gettempdir()); the swept code already dropped hardcoded /tmp on win32.
- seedgo: architecture checker exempts template scaffold test files
(test_scaffold.py via TEMPLATE_IGNORE_PATTERNS) from branch conformance -- a
template example test is not a structural requirement in every branch. Restores
all 17 branches to 100%.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013uzDhtcZ6wT1T9e2AHPQig
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
4105a7e8a7
commit
12e54e45f6
@@ -408,14 +408,19 @@ def spawn_agent(
|
||||
logger.info(f"Lock acquisition failed for {branch_email}: {lock_msg}")
|
||||
return False
|
||||
|
||||
_detach_kwargs: dict = {}
|
||||
if sys.platform == "win32":
|
||||
_detach_kwargs["creationflags"] = subprocess.CREATE_NEW_PROCESS_GROUP
|
||||
else:
|
||||
_detach_kwargs["start_new_session"] = True
|
||||
try:
|
||||
process = subprocess.Popen(
|
||||
monitor_cmd,
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
start_new_session=(sys.platform != "win32"),
|
||||
cwd=str(branch_path),
|
||||
env=spawn_env,
|
||||
**_detach_kwargs,
|
||||
)
|
||||
|
||||
monitor_pid = process.pid
|
||||
|
||||
@@ -578,14 +578,19 @@ def wake_branch(
|
||||
|
||||
if not spawned_via_scope:
|
||||
try:
|
||||
_detach_kwargs: dict = {}
|
||||
if sys.platform == "win32":
|
||||
_detach_kwargs["creationflags"] = subprocess.CREATE_NEW_PROCESS_GROUP
|
||||
else:
|
||||
_detach_kwargs["start_new_session"] = True
|
||||
process = subprocess.Popen(
|
||||
monitor_cmd,
|
||||
stdin=subprocess.DEVNULL,
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
start_new_session=(sys.platform != "win32"),
|
||||
cwd=str(branch_path),
|
||||
env=spawn_env,
|
||||
**_detach_kwargs,
|
||||
)
|
||||
|
||||
monitor_pid = process.pid
|
||||
|
||||
@@ -386,15 +386,19 @@ def _spawn_watchdog(target: str) -> None:
|
||||
if local_bin not in spawn_env.get("PATH", ""):
|
||||
spawn_env["PATH"] = local_bin + ":" + spawn_env.get("PATH", "")
|
||||
|
||||
_new_session = sys.platform != "win32"
|
||||
_detach_kwargs: dict = {}
|
||||
if sys.platform == "win32":
|
||||
_detach_kwargs["creationflags"] = subprocess.CREATE_NEW_PROCESS_GROUP
|
||||
else:
|
||||
_detach_kwargs["start_new_session"] = True
|
||||
try:
|
||||
subprocess.Popen(
|
||||
cmd,
|
||||
stdout=subprocess.DEVNULL,
|
||||
stderr=subprocess.DEVNULL,
|
||||
start_new_session=_new_session,
|
||||
cwd=str(devpulse_dir),
|
||||
env=spawn_env,
|
||||
**_detach_kwargs,
|
||||
)
|
||||
console.print(f"[green]Watchdog armed for {target}[/green]")
|
||||
except Exception as e:
|
||||
|
||||
@@ -16,6 +16,8 @@ All handler dependencies are mocked -- these tests verify orchestration
|
||||
logic, not business logic.
|
||||
"""
|
||||
|
||||
import subprocess
|
||||
import sys
|
||||
from contextlib import ExitStack
|
||||
|
||||
import pytest
|
||||
@@ -1005,7 +1007,11 @@ class TestSpawnWatchdog:
|
||||
|
||||
assert len(popen_calls) == 1
|
||||
assert popen_calls[0]["cmd"] == ["drone", "@devpulse", "watchdog", "agent", "@flow"]
|
||||
assert popen_calls[0]["start_new_session"] is True
|
||||
if sys.platform == "win32":
|
||||
assert popen_calls[0].get("creationflags") == subprocess.CREATE_NEW_PROCESS_GROUP
|
||||
assert "start_new_session" not in popen_calls[0]
|
||||
else:
|
||||
assert popen_calls[0]["start_new_session"] is True
|
||||
assert popen_calls[0]["cwd"] == str(devpulse_dir)
|
||||
combined = " ".join(printed)
|
||||
assert "Watchdog armed for @flow" in combined
|
||||
|
||||
@@ -18,7 +18,7 @@ Not suggestions. Violating = bug.
|
||||
|
||||
## What I Do
|
||||
|
||||
- Guide new users through `aipass init` (11 stages: welcome, system detect, profile, style questions, tool choice, docker offer, first agent, ping sweep, smoke test, handoff, done)
|
||||
- Guide new users through `aipass init` (10 stages: welcome, system detect, profile, style questions, tool choice, first agent, ping sweep, smoke test, handoff, done)
|
||||
- Answer "how does X work?" via `aipass help` — live README reads, offer depth, route branch experts
|
||||
- Run `aipass doctor` — aggregate seedgo, pytest, registry, hooks, git state, AIPASS_HOME
|
||||
- Remember user — name, OS, preferred CLI, setup progress `.trinity/local.json`
|
||||
@@ -30,7 +30,7 @@ Not suggestions. Violating = bug.
|
||||
aipass # Help banner with all commands
|
||||
aipass help [q] # Chatbot Q&A over branch READMEs
|
||||
aipass doctor # System health aggregation
|
||||
aipass init # 11-stage guided setup for new users, resumable
|
||||
aipass init # 10-stage guided setup for new users, resumable
|
||||
aipass profile # Show/edit what I know about the user
|
||||
aipass --version
|
||||
```
|
||||
@@ -53,7 +53,7 @@ apps/
|
||||
├── modules/
|
||||
│ ├── doctor.py # System health aggregation
|
||||
│ ├── help_chat.py # README-backed Q&A
|
||||
│ ├── init_flow.py # 11-stage guided setup, resumable
|
||||
│ ├── init_flow.py # 10-stage guided setup, resumable
|
||||
│ ├── handoff.py # CLI handoff (tmux / wt.exe)
|
||||
│ └── profile.py # User profile read/write
|
||||
└── handlers/
|
||||
@@ -79,6 +79,6 @@ apps/
|
||||
|
||||
## Known Gotchas
|
||||
|
||||
- **Status: under construction.** Whole branch gitignored. Do not PR anything this directory until Phase 8 reveal (DPLAN-0136).
|
||||
- **Status: under construction (DPLAN-0136).** Don't PR / reveal this branch until Phase 8 — that's a *policy*, NOT a gitignore. Only the usual runtime/memory layer is ignored (`.trinity/`, plan files, `*.local`, logs) same as every branch; my code (init_flow, cross_os, tests, README) IS trackable. Committed-or-not = git's call, devpulse's lane.
|
||||
- **`aipass` binary currently `cli` branch's `aipass init`** — project bootstrap, not citizen creation. Eventually this CLI entry moves here. Until then, use `drone @spawn create` citizen creation.
|
||||
- **Test-convention tokens need buy-in.** Core agents don't yet recognize `[AIPASS-TEST — ...]`. Coordinating @ai_mail before pinging anyone.
|
||||
|
||||
@@ -8,6 +8,7 @@ and sibling branches are protected even inside allowed roots.
|
||||
|
||||
import os
|
||||
import shutil
|
||||
import sys
|
||||
import tempfile
|
||||
from pathlib import Path
|
||||
from unittest.mock import patch
|
||||
@@ -49,13 +50,16 @@ def project_with_branches(project_dir):
|
||||
def _patch_roots(project_dir):
|
||||
"""Patch get_allowed_roots to use deterministic test roots."""
|
||||
tmpdir = Path(tempfile.gettempdir()).resolve()
|
||||
slash_tmp = Path("/tmp").resolve()
|
||||
roots = [project_dir.resolve()]
|
||||
seen = set(roots)
|
||||
for r in (slash_tmp, tmpdir):
|
||||
if r not in seen:
|
||||
seen.add(r)
|
||||
roots.append(r)
|
||||
if sys.platform != "win32":
|
||||
slash_tmp = Path("/tmp").resolve()
|
||||
if slash_tmp not in seen:
|
||||
seen.add(slash_tmp)
|
||||
roots.append(slash_tmp)
|
||||
if tmpdir not in seen:
|
||||
seen.add(tmpdir)
|
||||
roots.append(tmpdir)
|
||||
with patch(
|
||||
"aipass.drone.apps.handlers.rm_handler.get_allowed_roots",
|
||||
return_value=roots,
|
||||
@@ -76,7 +80,8 @@ class TestGetAllowedRoots:
|
||||
|
||||
def test_includes_slash_tmp(self):
|
||||
roots = get_allowed_roots()
|
||||
assert Path("/tmp").resolve() in roots
|
||||
if sys.platform != "win32":
|
||||
assert Path("/tmp").resolve() in roots
|
||||
|
||||
def test_includes_project_root_when_in_project(self, project_dir, monkeypatch):
|
||||
monkeypatch.chdir(project_dir)
|
||||
@@ -91,17 +96,18 @@ class TestGetAllowedRoots:
|
||||
|
||||
def test_tmpdir_and_slash_tmp_both_present_when_different(self, monkeypatch):
|
||||
"""When $TMPDIR != /tmp, both must appear in roots."""
|
||||
fake_tmpdir = "/tmp/claude-9999"
|
||||
os.makedirs(fake_tmpdir, exist_ok=True)
|
||||
try:
|
||||
monkeypatch.setenv("TMPDIR", fake_tmpdir)
|
||||
tempfile.tempdir = None
|
||||
roots = get_allowed_roots()
|
||||
resolved_roots = {r for r in roots}
|
||||
assert Path("/tmp").resolve() in resolved_roots
|
||||
assert Path(fake_tmpdir).resolve() in resolved_roots
|
||||
finally:
|
||||
tempfile.tempdir = None
|
||||
if sys.platform != "win32":
|
||||
fake_tmpdir = "/tmp/claude-9999"
|
||||
os.makedirs(fake_tmpdir, exist_ok=True)
|
||||
try:
|
||||
monkeypatch.setenv("TMPDIR", fake_tmpdir)
|
||||
tempfile.tempdir = None
|
||||
roots = get_allowed_roots()
|
||||
resolved_roots = {r for r in roots}
|
||||
assert Path("/tmp").resolve() in resolved_roots
|
||||
assert Path(fake_tmpdir).resolve() in resolved_roots
|
||||
finally:
|
||||
tempfile.tempdir = None
|
||||
|
||||
def test_roots_are_deduplicated(self):
|
||||
roots = get_allowed_roots()
|
||||
@@ -167,7 +173,7 @@ class TestAllowDeletion:
|
||||
|
||||
@pytest.mark.usefixtures("_patch_roots")
|
||||
def test_delete_nested_tmp_dir(self):
|
||||
"""e.g. /tmp/claude-1000/<x>."""
|
||||
"""e.g. tempdir/claude-1000/<x>."""
|
||||
parent = Path(tempfile.mkdtemp())
|
||||
target = parent / "nested"
|
||||
target.mkdir()
|
||||
@@ -243,16 +249,17 @@ class TestAllowDeletion:
|
||||
|
||||
@pytest.mark.usefixtures("_patch_roots")
|
||||
def test_slash_tmp_literal_allowed(self):
|
||||
"""/tmp/<x> must succeed even if $TMPDIR differs."""
|
||||
target = Path("/tmp") / f"rm_test_{os.getpid()}"
|
||||
target.mkdir(exist_ok=True)
|
||||
try:
|
||||
results = safe_delete([str(target)])
|
||||
assert results[0][1] is True
|
||||
assert not target.exists()
|
||||
finally:
|
||||
if target.exists():
|
||||
shutil.rmtree(target)
|
||||
"""Literal POSIX tmp path must succeed even if $TMPDIR differs."""
|
||||
if sys.platform != "win32":
|
||||
target = Path("/tmp") / f"rm_test_{os.getpid()}"
|
||||
target.mkdir(exist_ok=True)
|
||||
try:
|
||||
results = safe_delete([str(target)])
|
||||
assert results[0][1] is True
|
||||
assert not target.exists()
|
||||
finally:
|
||||
if target.exists():
|
||||
shutil.rmtree(target)
|
||||
|
||||
@pytest.mark.usefixtures("_patch_roots")
|
||||
def test_tmpdir_env_allowed(self):
|
||||
@@ -333,7 +340,8 @@ class TestRefuseDeletion:
|
||||
|
||||
@pytest.mark.usefixtures("_patch_roots")
|
||||
def test_nonexistent_path_clean_error(self):
|
||||
results = safe_delete(["/tmp/this_path_does_not_exist_abc123xyz"])
|
||||
nonexistent = os.path.join(tempfile.gettempdir(), "this_path_does_not_exist_abc123xyz")
|
||||
results = safe_delete([nonexistent])
|
||||
assert results[0][1] is False
|
||||
assert "does not exist" in results[0][2]
|
||||
|
||||
|
||||
@@ -31,6 +31,7 @@ TEMPLATE_IGNORE_PATTERNS = [
|
||||
".gitkeep", # Git placeholder files - not actual requirements
|
||||
"notepad.md", # Optional scratch file
|
||||
".gitignore", # Optional - branches inherit from root
|
||||
"test_scaffold.py", # Scaffold example — branches have their own tests
|
||||
]
|
||||
|
||||
# =============================================
|
||||
|
||||
@@ -202,6 +202,14 @@ def test_get_template_ignore_patterns_returns_copy():
|
||||
assert a != get_template_ignore_patterns()
|
||||
|
||||
|
||||
def test_template_ignore_excludes_test_scaffold():
|
||||
"""test_scaffold.py is in TEMPLATE_IGNORE_PATTERNS so branches don't require it."""
|
||||
from aipass.seedgo.apps.handlers.bypass.ignore_handler import get_template_ignore_patterns
|
||||
|
||||
patterns = get_template_ignore_patterns()
|
||||
assert "test_scaffold.py" in patterns
|
||||
|
||||
|
||||
def test_get_deprecated_patterns_returns_dict():
|
||||
"""get_deprecated_patterns returns a dict of string keys and string values."""
|
||||
from aipass.seedgo.apps.handlers.bypass.ignore_handler import get_deprecated_patterns
|
||||
|
||||
@@ -766,6 +766,27 @@ class TestScanTemplate:
|
||||
assert not any("README.md" in f for f in result["files"])
|
||||
assert any("entry.py" in f for f in result["files"])
|
||||
|
||||
def test_scan_template_excludes_test_scaffold(self, tmp_path, monkeypatch):
|
||||
import sys
|
||||
|
||||
_arch = "aipass.seedgo.apps.handlers.aipass_standards.architecture_check"
|
||||
monkeypatch.delitem(sys.modules, _arch, raising=False)
|
||||
|
||||
_ignore = sys.modules["aipass.seedgo.apps.handlers.bypass.ignore_handler"]
|
||||
monkeypatch.setattr(_ignore, "get_template_ignore_patterns", MagicMock(return_value=["test_scaffold.py"]))
|
||||
|
||||
from aipass.seedgo.apps.handlers.aipass_standards.architecture_check import (
|
||||
_scan_template,
|
||||
)
|
||||
|
||||
(tmp_path / "tests").mkdir()
|
||||
(tmp_path / "tests" / "conftest.py").write_text("# conf\n", encoding="utf-8")
|
||||
(tmp_path / "tests" / "test_scaffold.py").write_text("# scaffold\n", encoding="utf-8")
|
||||
|
||||
result = _scan_template(tmp_path)
|
||||
assert not any("test_scaffold.py" in f for f in result["files"])
|
||||
assert any("conftest.py" in f for f in result["files"])
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# 7. architecture_check — check_template_baseline with mocked templates
|
||||
|
||||
Reference in New Issue
Block a user