From 00dc5a6e50f17dd25db39d467794d44a60d623cc Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 13 Jun 2026 04:47:54 -0700 Subject: [PATCH 01/13] feat: add canary state machine and probe result cache modules Adds two new IL-0 modules that provide reusable building blocks for live probe classes: - src/autoskillit/_probe_canary.py: CanaryState (JSON-persisted state machine with NETWORK/SCHEMA failure streak tracking, flake-guard gating via should_report), and CanaryIssueUpdater (GitHub issue ensure/update via run_gh from autoskillit.core). Loaded from __init__.pyi:5 export. - src/autoskillit/execution/backends/_probe_cache.py: ProbeResult (frozen+slots dataclass) with 24-hour TTL keyed by cli_version, using versioned-JSON read/write from autoskillit.core. Plus their test files under tests/execution/backends/ and AGENTS.md file table updates. Co-Authored-By: Claude Fable 5 --- src/autoskillit/AGENTS.md | 1 + src/autoskillit/_probe_canary.py | 142 +++++++++++++ src/autoskillit/execution/backends/AGENTS.md | 1 + .../execution/backends/_probe_cache.py | 63 ++++++ tests/execution/backends/test_probe_cache.py | 136 ++++++++++++ tests/execution/backends/test_probe_canary.py | 194 ++++++++++++++++++ 6 files changed, 537 insertions(+) create mode 100644 src/autoskillit/_probe_canary.py create mode 100644 src/autoskillit/execution/backends/_probe_cache.py create mode 100644 tests/execution/backends/test_probe_cache.py create mode 100644 tests/execution/backends/test_probe_canary.py diff --git a/src/autoskillit/AGENTS.md b/src/autoskillit/AGENTS.md index d1a8a1f918..6baebfdd8b 100644 --- a/src/autoskillit/AGENTS.md +++ b/src/autoskillit/AGENTS.md @@ -11,6 +11,7 @@ Package root — entry points, hook registry, and cross-cutting utilities. | `_llm_triage.py` | Contract staleness triage (Haiku subprocess) | | `smoke_utils/` | Callables for smoke-test pipeline `run_python` steps (package) | | `hook_registry.py` | `HookDef`, `HOOK_REGISTRY`, `generate_hooks_json` | +| `_probe_canary.py` | Canary state machine and GitHub issue updater for live probes (IL-0) | | `_test_filter.py` | Test filter manifest: glob-to-test-directory mapping | | `version.py` | Version health utilities (IL-0) | diff --git a/src/autoskillit/_probe_canary.py b/src/autoskillit/_probe_canary.py new file mode 100644 index 0000000000..ce4dbb6f62 --- /dev/null +++ b/src/autoskillit/_probe_canary.py @@ -0,0 +1,142 @@ +"""Reusable canary state machine and GitHub issue updater for live probes. + +IL-0 module: imports only stdlib and `autoskillit.core`. Provides the +persistence + flake-guard primitives that live probe classes build on. +""" + +from __future__ import annotations + +import json +from dataclasses import asdict, dataclass +from enum import StrEnum, unique +from pathlib import Path + +from autoskillit.core import atomic_write, get_logger, run_gh + +logger = get_logger(__name__) + +N_CONSECUTIVE_FLAKE_GUARD: int = 3 + + +@unique +class ErrorKind(StrEnum): + NETWORK = "network" + SCHEMA = "schema" + + +@dataclass +class CanaryState: + network_streak: int = 0 + schema_streak: int = 0 + last_issue_number: int | None = None + + @classmethod + def load(cls, path: Path) -> CanaryState: + try: + raw = json.loads(path.read_text(encoding="utf-8")) + except (FileNotFoundError, json.JSONDecodeError, OSError): + return cls() + if not isinstance(raw, dict): + return cls() + return cls( + network_streak=raw.get("network_streak", 0), + schema_streak=raw.get("schema_streak", 0), + last_issue_number=raw.get("last_issue_number"), + ) + + def save(self, path: Path) -> None: + atomic_write(path, json.dumps(asdict(self), indent=2)) + + def record_failure(self, kind: ErrorKind) -> None: + if kind is ErrorKind.NETWORK: + self.network_streak += 1 + elif kind is ErrorKind.SCHEMA: + self.schema_streak += 1 + else: + raise ValueError(f"Unhandled ErrorKind: {kind!r}") + + def record_success(self) -> None: + self.network_streak = 0 + self.schema_streak = 0 + + def should_report(self, flake_guard: int = N_CONSECUTIVE_FLAKE_GUARD) -> bool: + return self.network_streak >= flake_guard or self.schema_streak >= flake_guard + + +class CanaryIssueUpdater: + def __init__(self, *, owner: str, repo: str) -> None: + self._owner = owner + self._repo = repo + + def ensure_issue(self, state: CanaryState, title: str, body: str) -> int: + existing = self._find_existing(title) + if existing is not None: + result = run_gh( + [ + "issue", + "edit", + str(existing), + "--repo", + f"{self._owner}/{self._repo}", + "--body", + body, + ], + ) + if result.returncode != 0: + logger.warning( + "canary_issue_edit_failed", + issue=existing, + stderr=result.stderr, + ) + state.last_issue_number = existing + return existing + result = run_gh( + [ + "issue", + "create", + "--repo", + f"{self._owner}/{self._repo}", + "--title", + title, + "--body", + body, + "--json", + "number", + ], + ) + if result.returncode != 0: + msg = f"gh issue create failed: {result.stderr}" + raise RuntimeError(msg) + issue_number = json.loads(result.stdout)["number"] + state.last_issue_number = issue_number + return issue_number + + def _find_existing(self, title: str) -> int | None: + result = run_gh( + [ + "issue", + "list", + "--repo", + f"{self._owner}/{self._repo}", + "--search", + title, + "--state", + "open", + "--json", + "number,title", + "--limit", + "10", + ], + ) + if result.returncode != 0: + return None + try: + issues = json.loads(result.stdout) + except (json.JSONDecodeError, KeyError): + return None + if not isinstance(issues, list): + return None + for issue in issues: + if issue.get("title") == title: + return issue["number"] + return None diff --git a/src/autoskillit/execution/backends/AGENTS.md b/src/autoskillit/execution/backends/AGENTS.md index 7aa087b397..2337b5f716 100644 --- a/src/autoskillit/execution/backends/AGENTS.md +++ b/src/autoskillit/execution/backends/AGENTS.md @@ -9,6 +9,7 @@ IL-1 backend abstraction layer — concrete `CodingAgentBackend` implementations | `__init__.py` | `BACKEND_REGISTRY`, `get_backend()` factory, re-exports | | `_backend_cmd_builder_base.py` | `BackendCmdBuilderBase` ABC (frozen dataclass), `FlagVocabulary` NamedTuple, `SHARED_BASELINE_ENV` — canonical location for shared env-assembly keys | | `_composite_locator.py` | `CompositeSessionLocator` — single dispatch point iterating BACKEND_REGISTRY for session location | +| `_probe_cache.py` | `ProbeResult`, `PROBE_CACHE_TTL`, `read_probe_cache`, `write_probe_cache` — versioned probe result cache | | `claude.py` | `ClaudeCodeBackend` (incl. `validate_session_layout`), `ClaudeEnvPolicy`, `ClaudeSessionLocator`, `ClaudeStreamParser`, `ClaudeResultParser` (prompt utilities moved to `_claude_prompt.py`) | | `_claude_prompt.py` | Prompt injection utilities, session constants (`_ensure_skill_prefix`, `_inject_completion_directive`, `_compose_resume_prompt`, etc.), shared by claude + codex + commands | | `codex.py` | `CodexFlags`, `CodexBackend` (incl. `validate_session_layout`, `setup_session_dir` agent TOML generation via `_generate_agent_tomls`), `CodexEnvPolicy`, `CodexSessionLocator` (parse/config moved to `_codex_parse.py` / `_codex_config.py`) | diff --git a/src/autoskillit/execution/backends/_probe_cache.py b/src/autoskillit/execution/backends/_probe_cache.py new file mode 100644 index 0000000000..37893b46ee --- /dev/null +++ b/src/autoskillit/execution/backends/_probe_cache.py @@ -0,0 +1,63 @@ +"""Versioned probe result cache with 24-hour TTL. + +IL-0 module: imports only stdlib and `autoskillit.core`. Stores a per-cli-version +`ProbeResult` keyed by `cli_version`, surviving across runs while staying +under `PROBE_CACHE_TTL`. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from datetime import UTC, datetime, timedelta +from pathlib import Path + +from autoskillit.core import get_logger, read_versioned_json, write_versioned_json + +logger = get_logger(__name__) + +PROBE_CACHE_TTL: timedelta = timedelta(hours=24) +_SCHEMA_VERSION: int = 1 + + +@dataclass(frozen=True, slots=True) +class ProbeResult: + cli_version: str + passed: bool + failure_detail: str | None + probe_timestamp: str + + +def read_probe_cache(cache_path: Path, cli_version: str) -> ProbeResult | None: + raw = read_versioned_json(cache_path, _SCHEMA_VERSION, logger=logger) + if raw is None: + return None + entries: dict[str, dict] = raw.get("entries", {}) + entry = entries.get(cli_version) + if entry is None: + return None + try: + ts = datetime.fromisoformat(entry["probe_timestamp"]) + if (datetime.now(UTC) - ts) > PROBE_CACHE_TTL: + return None + except (KeyError, ValueError, TypeError): + return None + return ProbeResult( + cli_version=cli_version, + passed=entry.get("passed", False), + failure_detail=entry.get("failure_detail"), + probe_timestamp=entry["probe_timestamp"], + ) + + +def write_probe_cache(cache_path: Path, result: ProbeResult) -> None: + try: + existing = read_versioned_json(cache_path, _SCHEMA_VERSION, logger=logger) + entries: dict[str, dict] = (existing or {}).get("entries", {}) + entries[result.cli_version] = { + "passed": result.passed, + "failure_detail": result.failure_detail, + "probe_timestamp": result.probe_timestamp, + } + write_versioned_json(cache_path, {"entries": entries}, _SCHEMA_VERSION) + except OSError: + pass diff --git a/tests/execution/backends/test_probe_cache.py b/tests/execution/backends/test_probe_cache.py new file mode 100644 index 0000000000..dc9fb4eee6 --- /dev/null +++ b/tests/execution/backends/test_probe_cache.py @@ -0,0 +1,136 @@ +from __future__ import annotations + +import json +from datetime import UTC, datetime, timedelta +from pathlib import Path + +import pytest + +from autoskillit.execution.backends._probe_cache import ( + _SCHEMA_VERSION, + PROBE_CACHE_TTL, + ProbeResult, + read_probe_cache, + write_probe_cache, +) + +pytestmark = [pytest.mark.layer("execution"), pytest.mark.small] + + +def _make_result( + cli_version: str = "1.0.0", + passed: bool = True, + failure_detail: str | None = None, + ts: datetime | None = None, +) -> ProbeResult: + if ts is None: + ts = datetime.now(UTC) + return ProbeResult( + cli_version=cli_version, + passed=passed, + failure_detail=failure_detail, + probe_timestamp=ts.isoformat(), + ) + + +def _write_raw_cache(path: Path, entries: dict, *, schema_version: int = _SCHEMA_VERSION) -> None: + payload = {"entries": entries, "schema_version": schema_version} + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(payload)) + + +class TestReadProbeCache: + def test_returns_none_on_missing_file(self, tmp_path: Path) -> None: + assert read_probe_cache(tmp_path / "nope.json", "1.0.0") is None + + def test_returns_none_on_corrupt_json(self, tmp_path: Path) -> None: + p = tmp_path / "cache.json" + p.write_text("NOT VALID JSON") + assert read_probe_cache(p, "1.0.0") is None + + def test_returns_none_on_stale_entry(self, tmp_path: Path) -> None: + stale_ts = (datetime.now(UTC) - PROBE_CACHE_TTL - timedelta(hours=1)).isoformat() + _write_raw_cache( + tmp_path / "cache.json", + { + "1.0.0": {"passed": True, "failure_detail": None, "probe_timestamp": stale_ts}, + }, + ) + assert read_probe_cache(tmp_path / "cache.json", "1.0.0") is None + + def test_returns_none_on_version_mismatch(self, tmp_path: Path) -> None: + fresh_ts = datetime.now(UTC).isoformat() + _write_raw_cache( + tmp_path / "cache.json", + { + "2.0.0": {"passed": True, "failure_detail": None, "probe_timestamp": fresh_ts}, + }, + ) + assert read_probe_cache(tmp_path / "cache.json", "1.0.0") is None + + def test_returns_probe_result_on_fresh_hit(self, tmp_path: Path) -> None: + fresh_ts = datetime.now(UTC).isoformat() + _write_raw_cache( + tmp_path / "cache.json", + { + "1.0.0": {"passed": True, "failure_detail": None, "probe_timestamp": fresh_ts}, + }, + ) + result = read_probe_cache(tmp_path / "cache.json", "1.0.0") + assert result is not None + assert result.passed is True + assert result.cli_version == "1.0.0" + + def test_returns_none_on_schema_version_mismatch(self, tmp_path: Path) -> None: + fresh_ts = datetime.now(UTC).isoformat() + _write_raw_cache( + tmp_path / "cache.json", + {"1.0.0": {"passed": True, "failure_detail": None, "probe_timestamp": fresh_ts}}, + schema_version=999, + ) + assert read_probe_cache(tmp_path / "cache.json", "1.0.0") is None + + def test_returns_none_on_naive_timestamp(self, tmp_path: Path) -> None: + _write_raw_cache( + tmp_path / "cache.json", + { + "1.0.0": { + "passed": True, + "failure_detail": None, + "probe_timestamp": "2026-06-13T10:00:00", + }, + }, + ) + assert read_probe_cache(tmp_path / "cache.json", "1.0.0") is None + + +class TestWriteProbeCache: + def test_creates_file(self, tmp_path: Path) -> None: + p = tmp_path / "cache.json" + write_probe_cache(p, _make_result()) + assert p.exists() + raw = json.loads(p.read_text()) + assert "entries" in raw + assert "1.0.0" in raw["entries"] + + def test_preserves_other_entries(self, tmp_path: Path) -> None: + p = tmp_path / "cache.json" + write_probe_cache(p, _make_result(cli_version="1.0.0")) + write_probe_cache(p, _make_result(cli_version="2.0.0")) + raw = json.loads(p.read_text()) + assert "1.0.0" in raw["entries"] + assert "2.0.0" in raw["entries"] + + def test_swallows_oserror(self, tmp_path: Path) -> None: + p = tmp_path / "readonly-dir" / "cache.json" + (tmp_path / "readonly-dir").mkdir() + (tmp_path / "readonly-dir").chmod(0o444) + write_probe_cache(p, _make_result()) + (tmp_path / "readonly-dir").chmod(0o755) + + def test_overwrites_same_version(self, tmp_path: Path) -> None: + p = tmp_path / "cache.json" + write_probe_cache(p, _make_result(cli_version="1.0.0", passed=True)) + write_probe_cache(p, _make_result(cli_version="1.0.0", passed=False)) + raw = json.loads(p.read_text()) + assert raw["entries"]["1.0.0"]["passed"] is False diff --git a/tests/execution/backends/test_probe_canary.py b/tests/execution/backends/test_probe_canary.py new file mode 100644 index 0000000000..9657d62d6a --- /dev/null +++ b/tests/execution/backends/test_probe_canary.py @@ -0,0 +1,194 @@ +from __future__ import annotations + +import json +from pathlib import Path +from subprocess import CompletedProcess + +import pytest + +from autoskillit._probe_canary import ( + N_CONSECUTIVE_FLAKE_GUARD, + CanaryIssueUpdater, + CanaryState, + ErrorKind, +) + +pytestmark = [pytest.mark.layer("execution"), pytest.mark.small] + + +class TestCanaryStateLoad: + def test_load_absent_file_returns_zero_state(self, tmp_path: Path) -> None: + state = CanaryState.load(tmp_path / "nonexistent.json") + assert state.network_streak == 0 + assert state.schema_streak == 0 + assert state.last_issue_number is None + + def test_load_existing_file(self, tmp_path: Path) -> None: + p = tmp_path / "state.json" + p.write_text( + json.dumps( + { + "network_streak": 2, + "schema_streak": 1, + "last_issue_number": 42, + } + ) + ) + state = CanaryState.load(p) + assert state.network_streak == 2 + assert state.schema_streak == 1 + assert state.last_issue_number == 42 + + def test_load_corrupt_file_returns_zero_state(self, tmp_path: Path) -> None: + p = tmp_path / "state.json" + p.write_text("NOT JSON{{{") + state = CanaryState.load(p) + assert state.network_streak == 0 + + +class TestCanaryStateSave: + def test_save_creates_file(self, tmp_path: Path) -> None: + p = tmp_path / "state.json" + state = CanaryState(network_streak=3, schema_streak=1) + state.save(p) + assert p.exists() + raw = json.loads(p.read_text()) + assert raw["network_streak"] == 3 + + def test_save_roundtrip(self, tmp_path: Path) -> None: + p = tmp_path / "state.json" + original = CanaryState(network_streak=5, schema_streak=2, last_issue_number=99) + original.save(p) + loaded = CanaryState.load(p) + assert loaded.network_streak == original.network_streak + assert loaded.schema_streak == original.schema_streak + assert loaded.last_issue_number == original.last_issue_number + + +class TestCanaryStateTransitions: + def test_record_failure_network(self) -> None: + state = CanaryState() + state.record_failure(ErrorKind.NETWORK) + assert state.network_streak == 1 + assert state.schema_streak == 0 + + def test_record_failure_schema(self) -> None: + state = CanaryState() + state.record_failure(ErrorKind.SCHEMA) + assert state.schema_streak == 1 + assert state.network_streak == 0 + + def test_record_failure_accumulates(self) -> None: + state = CanaryState() + state.record_failure(ErrorKind.NETWORK) + state.record_failure(ErrorKind.NETWORK) + state.record_failure(ErrorKind.NETWORK) + assert state.network_streak == 3 + + def test_record_success_resets_all(self) -> None: + state = CanaryState(network_streak=5, schema_streak=3) + state.record_success() + assert state.network_streak == 0 + assert state.schema_streak == 0 + + +class TestShouldReport: + def test_below_guard_returns_false(self) -> None: + state = CanaryState(network_streak=N_CONSECUTIVE_FLAKE_GUARD - 1) + assert state.should_report() is False + + def test_at_guard_returns_true(self) -> None: + state = CanaryState(network_streak=N_CONSECUTIVE_FLAKE_GUARD) + assert state.should_report() is True + + def test_above_guard_returns_true(self) -> None: + state = CanaryState(schema_streak=N_CONSECUTIVE_FLAKE_GUARD + 1) + assert state.should_report() is True + + def test_custom_guard(self) -> None: + state = CanaryState(network_streak=5) + assert state.should_report(flake_guard=5) is True + assert state.should_report(flake_guard=6) is False + + def test_either_kind_triggers(self) -> None: + state = CanaryState(network_streak=0, schema_streak=N_CONSECUTIVE_FLAKE_GUARD) + assert state.should_report() is True + + +class TestErrorKind: + def test_values(self) -> None: + assert set(ErrorKind) == {ErrorKind.NETWORK, ErrorKind.SCHEMA} + + +class TestCanaryIssueUpdater: + def test_ensure_issue_creates_new(self, monkeypatch: pytest.MonkeyPatch) -> None: + def mock_run_gh(args, **kwargs): + if args[0:2] == ["issue", "list"]: + return CompletedProcess(args=args, returncode=0, stdout="[]", stderr="") + if args[0:2] == ["issue", "create"]: + return CompletedProcess( + args=args, + returncode=0, + stdout=json.dumps({"number": 123}), + stderr="", + ) + return CompletedProcess(args=args, returncode=1, stdout="", stderr="") + + monkeypatch.setattr("autoskillit._probe_canary.run_gh", mock_run_gh) + updater = CanaryIssueUpdater(owner="test-org", repo="test-repo") + state = CanaryState() + num = updater.ensure_issue(state, "Probe failure", "Details here") + assert num == 123 + assert state.last_issue_number == 123 + + def test_ensure_issue_updates_existing(self, monkeypatch: pytest.MonkeyPatch) -> None: + def mock_run_gh(args, **kwargs): + if args[0:2] == ["issue", "list"]: + return CompletedProcess( + args=args, + returncode=0, + stdout=json.dumps([{"number": 42, "title": "Probe failure"}]), + stderr="", + ) + if args[0:2] == ["issue", "edit"]: + return CompletedProcess(args=args, returncode=0, stdout="", stderr="") + return CompletedProcess(args=args, returncode=1, stdout="", stderr="") + + monkeypatch.setattr("autoskillit._probe_canary.run_gh", mock_run_gh) + updater = CanaryIssueUpdater(owner="test-org", repo="test-repo") + state = CanaryState() + num = updater.ensure_issue(state, "Probe failure", "Updated body") + assert num == 42 + assert state.last_issue_number == 42 + + def test_ensure_issue_raises_on_create_failure(self, monkeypatch: pytest.MonkeyPatch) -> None: + def mock_run_gh(args, **kwargs): + if args[0:2] == ["issue", "list"]: + return CompletedProcess(args=args, returncode=0, stdout="[]", stderr="") + return CompletedProcess(args=args, returncode=1, stdout="", stderr="auth error") + + monkeypatch.setattr("autoskillit._probe_canary.run_gh", mock_run_gh) + updater = CanaryIssueUpdater(owner="test-org", repo="test-repo") + state = CanaryState() + with pytest.raises(RuntimeError, match="gh issue create failed"): + updater.ensure_issue(state, "Probe failure", "Details") + + def test_ensure_issue_logs_on_edit_failure(self, monkeypatch: pytest.MonkeyPatch) -> None: + def mock_run_gh(args, **kwargs): + if args[0:2] == ["issue", "list"]: + return CompletedProcess( + args=args, + returncode=0, + stdout=json.dumps([{"number": 42, "title": "Probe failure"}]), + stderr="", + ) + if args[0:2] == ["issue", "edit"]: + return CompletedProcess(args=args, returncode=1, stdout="", stderr="locked") + return CompletedProcess(args=args, returncode=1, stdout="", stderr="") + + monkeypatch.setattr("autoskillit._probe_canary.run_gh", mock_run_gh) + updater = CanaryIssueUpdater(owner="test-org", repo="test-repo") + state = CanaryState() + num = updater.ensure_issue(state, "Probe failure", "Updated body") + assert num == 42 + assert state.last_issue_number == 42 From 2e2682caaf34571d63e7adc71ed05dbe31e61aff Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 13 Jun 2026 04:48:08 -0700 Subject: [PATCH 02/13] test: register new canary/probe modules in architectural guard allowlists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Bump EXEMPTIONS['execution/backends'] from 11 to 12 to account for the new _probe_cache.py module. - Add _probe_canary.py to the root module allowlist in test_root_module_allowlist. - Add _probe_canary to _cmd_runner MODULE_CASCADE_CORE consumers (run_gh import). - Add _probe_canary to LAYER_CASCADE_CONSERVATIVE['core'] and as a new top-level entry in both conservative and aggressive cascade maps. - Add _probe_cache to SINGLETON_ALLOWED_MODULES (module-level timedelta() call for PROBE_CACHE_TTL constant). - Add _probe_canary.py atomic_write site to _LEGACY_JSON_WRITES allowlist (CanaryState save — 3-field state file, intentionally unversioned). - Add the two new source files to .autoskillit/test-source-map.json so the test filter cascade maps changes to their tests. Co-Authored-By: Claude Fable 5 --- .autoskillit/test-source-map.json | 6 ++++++ tests/_test_filter.py | 5 ++++- tests/arch/test_subpackage_isolation.py | 3 ++- tests/contracts/test_package_gateways.py | 1 + tests/infra/test_schema_version_convention.py | 2 ++ 5 files changed, 15 insertions(+), 2 deletions(-) diff --git a/.autoskillit/test-source-map.json b/.autoskillit/test-source-map.json index 2e0e0ecf32..b1005f1ef1 100644 --- a/.autoskillit/test-source-map.json +++ b/.autoskillit/test-source-map.json @@ -15071,5 +15071,11 @@ "tests/server/test_tools_git.py", "tests/server/test_tools_git_merge_cleanup.py", "tests/workspace/test_worktree.py" + ], + "src/autoskillit/_probe_canary.py": [ + "tests/execution/backends/test_probe_canary.py" + ], + "src/autoskillit/execution/backends/_probe_cache.py": [ + "tests/execution/backends/test_probe_cache.py" ] } diff --git a/tests/_test_filter.py b/tests/_test_filter.py index 861e242ae6..c484dd3e1f 100644 --- a/tests/_test_filter.py +++ b/tests/_test_filter.py @@ -168,7 +168,7 @@ class ImportContext(enum.StrEnum): } MODULE_CASCADE_CORE: dict[str, frozenset[str]] = { - "_cmd_runner": frozenset({"cli", "core", "recipe"}), + "_cmd_runner": frozenset({"cli", "core", "recipe", "_probe_canary"}), "_json": frozenset({"core", "execution", "pipeline", "recipe", "server"}), "feature_flags": frozenset( {"core", "cli", "config", "execution", "recipe", "server", "workspace"} @@ -605,6 +605,7 @@ class ImportContext(enum.StrEnum): "hook_registry", "planner", "smoke_utils", + "_probe_canary", } ), # L1 @@ -896,6 +897,7 @@ class ImportContext(enum.StrEnum): "_test_filter": frozenset({"arch", "infra", "contracts"}), "smoke_utils": frozenset({"test_smoke_utils.py", "recipe", "smoke_utils"}), "version": frozenset({"test_version.py", "server", "cli"}), + "_probe_canary": frozenset({"core", "execution/backends/test_probe_canary.py"}), } # --------------------------------------------------------------------------- @@ -979,6 +981,7 @@ class ImportContext(enum.StrEnum): "version": frozenset({"test_version.py"}), "_test_filter": frozenset({"arch", "contracts"}), "report": frozenset({"report"}), + "_probe_canary": frozenset({"core", "execution/backends/test_probe_canary.py"}), } # --------------------------------------------------------------------------- diff --git a/tests/arch/test_subpackage_isolation.py b/tests/arch/test_subpackage_isolation.py index 436b216fe2..f728387559 100644 --- a/tests/arch/test_subpackage_isolation.py +++ b/tests/arch/test_subpackage_isolation.py @@ -77,6 +77,7 @@ def _get_call_func_name(node: ast.Call) -> str | None: "validator", # recipe/validator.py: defensive exemption for decorator-based rule registry "settings", # config/settings.py: _CONFIG_SCHEMA = _build_config_schema() "_headless_path_tokens", # execution/_headless_path_tokens.py: _OUTPUT_PATH_TOKENS + "_probe_cache", # execution/backends/_probe_cache.py: PROBE_CACHE_TTL = timedelta(...) # _STABLE_DISMISS_WINDOW = timedelta(days=7), _DEV_DISMISS_WINDOW = timedelta(hours=12) "_install_info", # cli/_install_info.py: window constants (see comment above) # KITCHEN_GUARDED_COMMANDS: frozenset[str] @@ -881,7 +882,7 @@ def test_no_subpackage_exceeds_10_files() -> None: "recipe/rules": 51, # +commit_guard_regression_route +rules_model +rules_gitignored_deliverable # noqa: E501 "server/tools": 27, # +_preflight.py (dispatch-feasibility preflight) "hooks/guards": 32, # +fleet_claim_guard, +reset_resume_gate, +recipe_read_guard - "execution/backends": 11, # +_composite_locator.py (CompositeSessionLocator) + "execution/backends": 12, # +_composite_locator.py, +_probe_cache.py } violations: list[str] = [] dirs_to_check: list[Path] = [] diff --git a/tests/contracts/test_package_gateways.py b/tests/contracts/test_package_gateways.py index 87ecd63624..013c534825 100644 --- a/tests/contracts/test_package_gateways.py +++ b/tests/contracts/test_package_gateways.py @@ -397,6 +397,7 @@ def test_root_module_allowlist() -> None: "__init__.py", "__main__.py", "_llm_triage.py", + "_probe_canary.py", "_test_filter.py", "hook_registry.py", "version.py", diff --git a/tests/infra/test_schema_version_convention.py b/tests/infra/test_schema_version_convention.py index d15f7eece5..897734272b 100644 --- a/tests/infra/test_schema_version_convention.py +++ b/tests/infra/test_schema_version_convention.py @@ -175,6 +175,8 @@ def _is_yaml_dump(node: ast.expr) -> bool: ("src/autoskillit/planner/manifests.py", 303), # _cmd_rpc_issues.py — emit_fallback_map: BEM fallback execution map (recipe-internal) ("src/autoskillit/recipe/_cmd_rpc_issues.py", 77), + # _probe_canary.py — CanaryState save() 3-field state file (intentionally unversioned) + ("src/autoskillit/_probe_canary.py", 48), } From a9d4113a79f7c0ed2bf2359facbdea9d5cc796dc Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 13 Jun 2026 05:13:20 -0700 Subject: [PATCH 03/13] fix(review): use --body-file for gh issue calls and guard json.loads output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses two reviewer findings in _probe_canary.py: - Replace --body CLI arg with --body-file tempfile per AGENTS.md §3.4 mandate (issue create and issue edit both now write body to a NamedTemporaryFile) - Wrap json.loads(result.stdout)["number"] in try/except (JSONDecodeError, KeyError) and re-raise as RuntimeError with stdout content for diagnostics Co-Authored-By: Claude Sonnet 4.6 --- src/autoskillit/_probe_canary.py | 70 ++++++++++++++++++++------------ 1 file changed, 44 insertions(+), 26 deletions(-) diff --git a/src/autoskillit/_probe_canary.py b/src/autoskillit/_probe_canary.py index ce4dbb6f62..84c9215e22 100644 --- a/src/autoskillit/_probe_canary.py +++ b/src/autoskillit/_probe_canary.py @@ -7,6 +7,8 @@ from __future__ import annotations import json +import os +import tempfile from dataclasses import asdict, dataclass from enum import StrEnum, unique from pathlib import Path @@ -71,17 +73,23 @@ def __init__(self, *, owner: str, repo: str) -> None: def ensure_issue(self, state: CanaryState, title: str, body: str) -> int: existing = self._find_existing(title) if existing is not None: - result = run_gh( - [ - "issue", - "edit", - str(existing), - "--repo", - f"{self._owner}/{self._repo}", - "--body", - body, - ], - ) + with tempfile.NamedTemporaryFile(mode="w", suffix=".md", delete=False) as f: + f.write(body) + body_path = f.name + try: + result = run_gh( + [ + "issue", + "edit", + str(existing), + "--repo", + f"{self._owner}/{self._repo}", + "--body-file", + body_path, + ], + ) + finally: + os.unlink(body_path) if result.returncode != 0: logger.warning( "canary_issue_edit_failed", @@ -90,24 +98,34 @@ def ensure_issue(self, state: CanaryState, title: str, body: str) -> int: ) state.last_issue_number = existing return existing - result = run_gh( - [ - "issue", - "create", - "--repo", - f"{self._owner}/{self._repo}", - "--title", - title, - "--body", - body, - "--json", - "number", - ], - ) + with tempfile.NamedTemporaryFile(mode="w", suffix=".md", delete=False) as f: + f.write(body) + body_path = f.name + try: + result = run_gh( + [ + "issue", + "create", + "--repo", + f"{self._owner}/{self._repo}", + "--title", + title, + "--body-file", + body_path, + "--json", + "number", + ], + ) + finally: + os.unlink(body_path) if result.returncode != 0: msg = f"gh issue create failed: {result.stderr}" raise RuntimeError(msg) - issue_number = json.loads(result.stdout)["number"] + try: + issue_number = json.loads(result.stdout)["number"] + except (json.JSONDecodeError, KeyError) as exc: + msg = f"gh issue create returned unexpected output: {result.stdout!r}" + raise RuntimeError(msg) from exc state.last_issue_number = issue_number return issue_number From e157289ba7eea2198118d034a7ad405b4c2576b3 Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 13 Jun 2026 05:13:24 -0700 Subject: [PATCH 04/13] fix(review): correct IL-1 docstring and log OSError on cache write failure - Correct module docstring from IL-0 to IL-1 (file lives in execution/backends/) - Add logger.debug call on OSError in write_probe_cache instead of silent pass Co-Authored-By: Claude Sonnet 4.6 --- src/autoskillit/execution/backends/_probe_cache.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/autoskillit/execution/backends/_probe_cache.py b/src/autoskillit/execution/backends/_probe_cache.py index 37893b46ee..43ad4fbd94 100644 --- a/src/autoskillit/execution/backends/_probe_cache.py +++ b/src/autoskillit/execution/backends/_probe_cache.py @@ -1,6 +1,6 @@ """Versioned probe result cache with 24-hour TTL. -IL-0 module: imports only stdlib and `autoskillit.core`. Stores a per-cli-version +IL-1 module: imports only stdlib and `autoskillit.core`. Stores a per-cli-version `ProbeResult` keyed by `cli_version`, surviving across runs while staying under `PROBE_CACHE_TTL`. """ @@ -60,4 +60,4 @@ def write_probe_cache(cache_path: Path, result: ProbeResult) -> None: } write_versioned_json(cache_path, {"entries": entries}, _SCHEMA_VERSION) except OSError: - pass + logger.debug("probe_cache_write_failed", path=str(cache_path), exc_info=True) From 4a4aae73875df2c566265c01100b7153a77729e9 Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 13 Jun 2026 05:13:31 -0700 Subject: [PATCH 05/13] fix(review): strengthen assertions in probe cache and canary tests - test_swallows_oserror: add assert not p.exists() to catch regressions where write_probe_cache writes to an unexpected path - test_load_corrupt_file_returns_zero_state: add schema_streak == 0 and last_issue_number is None assertions to match the full zero-state contract Co-Authored-By: Claude Sonnet 4.6 --- tests/execution/backends/test_probe_cache.py | 1 + tests/execution/backends/test_probe_canary.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/tests/execution/backends/test_probe_cache.py b/tests/execution/backends/test_probe_cache.py index dc9fb4eee6..04536c3cb6 100644 --- a/tests/execution/backends/test_probe_cache.py +++ b/tests/execution/backends/test_probe_cache.py @@ -127,6 +127,7 @@ def test_swallows_oserror(self, tmp_path: Path) -> None: (tmp_path / "readonly-dir").chmod(0o444) write_probe_cache(p, _make_result()) (tmp_path / "readonly-dir").chmod(0o755) + assert not p.exists() def test_overwrites_same_version(self, tmp_path: Path) -> None: p = tmp_path / "cache.json" diff --git a/tests/execution/backends/test_probe_canary.py b/tests/execution/backends/test_probe_canary.py index 9657d62d6a..f9e4ed409d 100644 --- a/tests/execution/backends/test_probe_canary.py +++ b/tests/execution/backends/test_probe_canary.py @@ -44,6 +44,8 @@ def test_load_corrupt_file_returns_zero_state(self, tmp_path: Path) -> None: p.write_text("NOT JSON{{{") state = CanaryState.load(p) assert state.network_streak == 0 + assert state.schema_streak == 0 + assert state.last_issue_number is None class TestCanaryStateSave: From 74ced692c513650ff40bfeade3c609650785f26c Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 13 Jun 2026 05:16:48 -0700 Subject: [PATCH 06/13] fix(review): update schema version allowlist line number for _probe_canary.py The json.dumps call in CanaryState.save() shifted from line 48 to 50 after adding os and tempfile imports. Update the _LEGACY_JSON_WRITES allowlist entry. Co-Authored-By: Claude Sonnet 4.6 --- tests/infra/test_schema_version_convention.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/infra/test_schema_version_convention.py b/tests/infra/test_schema_version_convention.py index 897734272b..d3adbaf893 100644 --- a/tests/infra/test_schema_version_convention.py +++ b/tests/infra/test_schema_version_convention.py @@ -176,7 +176,7 @@ def _is_yaml_dump(node: ast.expr) -> bool: # _cmd_rpc_issues.py — emit_fallback_map: BEM fallback execution map (recipe-internal) ("src/autoskillit/recipe/_cmd_rpc_issues.py", 77), # _probe_canary.py — CanaryState save() 3-field state file (intentionally unversioned) - ("src/autoskillit/_probe_canary.py", 48), + ("src/autoskillit/_probe_canary.py", 50), } From 5bb2142d83c76ea6948cb85e31669e16b1e803ec Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 13 Jun 2026 05:37:26 -0700 Subject: [PATCH 07/13] fix(review): correct IL-1 docstring and extract _run_gh_with_body_file helper MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Change "IL-0 module" to "IL-1 module" in _probe_canary.py docstring to match _probe_cache.py convention (module imports from autoskillit.core → IL-1) - Extract duplicated NamedTemporaryFile/write/run_gh/finally:os.unlink pattern into _run_gh_with_body_file(args, body) helper, eliminating the duplication between the issue edit and issue create branches Co-Authored-By: Claude Sonnet 4.6 --- src/autoskillit/_probe_canary.py | 73 +++++++++++++++----------------- 1 file changed, 35 insertions(+), 38 deletions(-) diff --git a/src/autoskillit/_probe_canary.py b/src/autoskillit/_probe_canary.py index 84c9215e22..57b23d2798 100644 --- a/src/autoskillit/_probe_canary.py +++ b/src/autoskillit/_probe_canary.py @@ -1,6 +1,6 @@ """Reusable canary state machine and GitHub issue updater for live probes. -IL-0 module: imports only stdlib and `autoskillit.core`. Provides the +IL-1 module: imports only stdlib and `autoskillit.core`. Provides the persistence + flake-guard primitives that live probe classes build on. """ @@ -8,6 +8,7 @@ import json import os +import subprocess import tempfile from dataclasses import asdict, dataclass from enum import StrEnum, unique @@ -65,6 +66,16 @@ def should_report(self, flake_guard: int = N_CONSECUTIVE_FLAKE_GUARD) -> bool: return self.network_streak >= flake_guard or self.schema_streak >= flake_guard +def _run_gh_with_body_file(args: list[str], body: str) -> subprocess.CompletedProcess[str]: + with tempfile.NamedTemporaryFile(mode="w", suffix=".md", delete=False) as f: + f.write(body) + body_path = f.name + try: + return run_gh([*args, "--body-file", body_path]) + finally: + os.unlink(body_path) + + class CanaryIssueUpdater: def __init__(self, *, owner: str, repo: str) -> None: self._owner = owner @@ -73,23 +84,16 @@ def __init__(self, *, owner: str, repo: str) -> None: def ensure_issue(self, state: CanaryState, title: str, body: str) -> int: existing = self._find_existing(title) if existing is not None: - with tempfile.NamedTemporaryFile(mode="w", suffix=".md", delete=False) as f: - f.write(body) - body_path = f.name - try: - result = run_gh( - [ - "issue", - "edit", - str(existing), - "--repo", - f"{self._owner}/{self._repo}", - "--body-file", - body_path, - ], - ) - finally: - os.unlink(body_path) + result = _run_gh_with_body_file( + [ + "issue", + "edit", + str(existing), + "--repo", + f"{self._owner}/{self._repo}", + ], + body, + ) if result.returncode != 0: logger.warning( "canary_issue_edit_failed", @@ -98,26 +102,19 @@ def ensure_issue(self, state: CanaryState, title: str, body: str) -> int: ) state.last_issue_number = existing return existing - with tempfile.NamedTemporaryFile(mode="w", suffix=".md", delete=False) as f: - f.write(body) - body_path = f.name - try: - result = run_gh( - [ - "issue", - "create", - "--repo", - f"{self._owner}/{self._repo}", - "--title", - title, - "--body-file", - body_path, - "--json", - "number", - ], - ) - finally: - os.unlink(body_path) + result = _run_gh_with_body_file( + [ + "issue", + "create", + "--repo", + f"{self._owner}/{self._repo}", + "--title", + title, + "--json", + "number", + ], + body, + ) if result.returncode != 0: msg = f"gh issue create failed: {result.stderr}" raise RuntimeError(msg) From 50cdfdd8333ff3ced0cca13fd55a52ce97bcc95d Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 13 Jun 2026 05:37:53 -0700 Subject: [PATCH 08/13] fix(review): use time-invariant naive timestamp and guard chmod restore - test_returns_none_on_naive_timestamp: change hardcoded "2026-06-13T10:00:00" to "2000-01-01T00:00:00" so test is not tied to the current date - test_swallows_oserror: wrap write + assert in try/finally so readonly-dir chmod is restored even if the test assertion fails Co-Authored-By: Claude Sonnet 4.6 --- tests/execution/backends/test_probe_cache.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/execution/backends/test_probe_cache.py b/tests/execution/backends/test_probe_cache.py index 04536c3cb6..e25726e556 100644 --- a/tests/execution/backends/test_probe_cache.py +++ b/tests/execution/backends/test_probe_cache.py @@ -97,7 +97,7 @@ def test_returns_none_on_naive_timestamp(self, tmp_path: Path) -> None: "1.0.0": { "passed": True, "failure_detail": None, - "probe_timestamp": "2026-06-13T10:00:00", + "probe_timestamp": "2000-01-01T00:00:00", }, }, ) @@ -125,9 +125,11 @@ def test_swallows_oserror(self, tmp_path: Path) -> None: p = tmp_path / "readonly-dir" / "cache.json" (tmp_path / "readonly-dir").mkdir() (tmp_path / "readonly-dir").chmod(0o444) - write_probe_cache(p, _make_result()) - (tmp_path / "readonly-dir").chmod(0o755) - assert not p.exists() + try: + write_probe_cache(p, _make_result()) + assert not p.exists() + finally: + (tmp_path / "readonly-dir").chmod(0o755) def test_overwrites_same_version(self, tmp_path: Path) -> None: p = tmp_path / "cache.json" From 4aa41cab69e24cf4540ab242b629e3049056d32c Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 13 Jun 2026 05:39:51 -0700 Subject: [PATCH 09/13] fix(review): assert --body-file in mocks and verify warning log on edit failure - test_ensure_issue_creates_new: assert '--body-file' in args inside the create branch of mock_run_gh to catch regression to inline --body - test_ensure_issue_updates_existing: same assertion in the edit branch - test_ensure_issue_logs_on_edit_failure: add structlog.testing.capture_logs() assertion to verify logger.warning('canary_issue_edit_failed') is emitted Co-Authored-By: Claude Sonnet 4.6 --- tests/execution/backends/test_probe_canary.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/execution/backends/test_probe_canary.py b/tests/execution/backends/test_probe_canary.py index f9e4ed409d..0f14bd6d1a 100644 --- a/tests/execution/backends/test_probe_canary.py +++ b/tests/execution/backends/test_probe_canary.py @@ -5,6 +5,7 @@ from subprocess import CompletedProcess import pytest +import structlog.testing from autoskillit._probe_canary import ( N_CONSECUTIVE_FLAKE_GUARD, @@ -128,6 +129,7 @@ def mock_run_gh(args, **kwargs): if args[0:2] == ["issue", "list"]: return CompletedProcess(args=args, returncode=0, stdout="[]", stderr="") if args[0:2] == ["issue", "create"]: + assert "--body-file" in args return CompletedProcess( args=args, returncode=0, @@ -153,6 +155,7 @@ def mock_run_gh(args, **kwargs): stderr="", ) if args[0:2] == ["issue", "edit"]: + assert "--body-file" in args return CompletedProcess(args=args, returncode=0, stdout="", stderr="") return CompletedProcess(args=args, returncode=1, stdout="", stderr="") @@ -191,6 +194,8 @@ def mock_run_gh(args, **kwargs): monkeypatch.setattr("autoskillit._probe_canary.run_gh", mock_run_gh) updater = CanaryIssueUpdater(owner="test-org", repo="test-repo") state = CanaryState() - num = updater.ensure_issue(state, "Probe failure", "Updated body") + with structlog.testing.capture_logs() as cap_logs: + num = updater.ensure_issue(state, "Probe failure", "Updated body") assert num == 42 assert state.last_issue_number == 42 + assert any(e.get("event") == "canary_issue_edit_failed" for e in cap_logs) From 69612a757c465a44f6e1f5d466abd5ad4a2a761d Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 13 Jun 2026 05:43:32 -0700 Subject: [PATCH 10/13] fix(review): move assert after chmod restore and update allowlist line number - test_swallows_oserror: move assert not p.exists() after the finally block so it runs after chmod(0o755) restores directory access (chmod 0o444 removes execute bit, causing PermissionError on os.stat inside the try block) - test_schema_version_convention: update _LEGACY_JSON_WRITES entry for _probe_canary.py from line 50 to 51 (shifted by added subprocess import) Co-Authored-By: Claude Sonnet 4.6 --- tests/execution/backends/test_probe_cache.py | 2 +- tests/infra/test_schema_version_convention.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/execution/backends/test_probe_cache.py b/tests/execution/backends/test_probe_cache.py index e25726e556..d384124e51 100644 --- a/tests/execution/backends/test_probe_cache.py +++ b/tests/execution/backends/test_probe_cache.py @@ -127,9 +127,9 @@ def test_swallows_oserror(self, tmp_path: Path) -> None: (tmp_path / "readonly-dir").chmod(0o444) try: write_probe_cache(p, _make_result()) - assert not p.exists() finally: (tmp_path / "readonly-dir").chmod(0o755) + assert not p.exists() def test_overwrites_same_version(self, tmp_path: Path) -> None: p = tmp_path / "cache.json" diff --git a/tests/infra/test_schema_version_convention.py b/tests/infra/test_schema_version_convention.py index d3adbaf893..998e6ffb5f 100644 --- a/tests/infra/test_schema_version_convention.py +++ b/tests/infra/test_schema_version_convention.py @@ -176,7 +176,7 @@ def _is_yaml_dump(node: ast.expr) -> bool: # _cmd_rpc_issues.py — emit_fallback_map: BEM fallback execution map (recipe-internal) ("src/autoskillit/recipe/_cmd_rpc_issues.py", 77), # _probe_canary.py — CanaryState save() 3-field state file (intentionally unversioned) - ("src/autoskillit/_probe_canary.py", 50), + ("src/autoskillit/_probe_canary.py", 51), } From bb509560f3523f958d826877227f0d2da74af70b Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 13 Jun 2026 06:07:11 -0700 Subject: [PATCH 11/13] fix(review): correct _probe_canary.py IL label from IL-0 to IL-1 in AGENTS.md --- src/autoskillit/AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/autoskillit/AGENTS.md b/src/autoskillit/AGENTS.md index 6baebfdd8b..e886396afc 100644 --- a/src/autoskillit/AGENTS.md +++ b/src/autoskillit/AGENTS.md @@ -11,7 +11,7 @@ Package root — entry points, hook registry, and cross-cutting utilities. | `_llm_triage.py` | Contract staleness triage (Haiku subprocess) | | `smoke_utils/` | Callables for smoke-test pipeline `run_python` steps (package) | | `hook_registry.py` | `HookDef`, `HOOK_REGISTRY`, `generate_hooks_json` | -| `_probe_canary.py` | Canary state machine and GitHub issue updater for live probes (IL-0) | +| `_probe_canary.py` | Canary state machine and GitHub issue updater for live probes (IL-1) | | `_test_filter.py` | Test filter manifest: glob-to-test-directory mapping | | `version.py` | Version health utilities (IL-0) | From 1ccdf658a4ad515232097d95a172b71adda172e8 Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 13 Jun 2026 06:07:49 -0700 Subject: [PATCH 12/13] fix(review): guard os.unlink, add warning log on find failure, remove misleading KeyError in _find_existing --- src/autoskillit/_probe_canary.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/autoskillit/_probe_canary.py b/src/autoskillit/_probe_canary.py index 57b23d2798..e7c0b4500b 100644 --- a/src/autoskillit/_probe_canary.py +++ b/src/autoskillit/_probe_canary.py @@ -73,7 +73,10 @@ def _run_gh_with_body_file(args: list[str], body: str) -> subprocess.CompletedPr try: return run_gh([*args, "--body-file", body_path]) finally: - os.unlink(body_path) + try: + os.unlink(body_path) + except OSError: + logger.debug("canary_body_file_unlink_failed", path=body_path) class CanaryIssueUpdater: @@ -144,14 +147,21 @@ def _find_existing(self, title: str) -> int | None: ], ) if result.returncode != 0: + logger.warning( + "canary_find_existing_failed", + returncode=result.returncode, + stderr=result.stderr, + ) return None try: issues = json.loads(result.stdout) - except (json.JSONDecodeError, KeyError): + except json.JSONDecodeError: return None if not isinstance(issues, list): return None for issue in issues: if issue.get("title") == title: - return issue["number"] + number = issue.get("number") + if isinstance(number, int): + return number return None From 46fed7602770a2eccf66565d88ca55f16d0d00a2 Mon Sep 17 00:00:00 2001 From: Trecek Date: Sat, 13 Jun 2026 06:08:31 -0700 Subject: [PATCH 13/13] =?UTF-8?q?fix(review):=20harden=20mocks=20in=20test?= =?UTF-8?q?=5Fprobe=5Fcanary=20=E2=80=94=20explicit=20create=20match=20and?= =?UTF-8?q?=20--body-file=20on=20edit=20failure=20path?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/execution/backends/test_probe_canary.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/execution/backends/test_probe_canary.py b/tests/execution/backends/test_probe_canary.py index 0f14bd6d1a..6aa89b0ce8 100644 --- a/tests/execution/backends/test_probe_canary.py +++ b/tests/execution/backends/test_probe_canary.py @@ -170,7 +170,9 @@ def test_ensure_issue_raises_on_create_failure(self, monkeypatch: pytest.MonkeyP def mock_run_gh(args, **kwargs): if args[0:2] == ["issue", "list"]: return CompletedProcess(args=args, returncode=0, stdout="[]", stderr="") - return CompletedProcess(args=args, returncode=1, stdout="", stderr="auth error") + if args[0:2] == ["issue", "create"]: + return CompletedProcess(args=args, returncode=1, stdout="", stderr="auth error") + raise AssertionError(f"Unexpected gh call: {args}") monkeypatch.setattr("autoskillit._probe_canary.run_gh", mock_run_gh) updater = CanaryIssueUpdater(owner="test-org", repo="test-repo") @@ -188,8 +190,9 @@ def mock_run_gh(args, **kwargs): stderr="", ) if args[0:2] == ["issue", "edit"]: + assert "--body-file" in args return CompletedProcess(args=args, returncode=1, stdout="", stderr="locked") - return CompletedProcess(args=args, returncode=1, stdout="", stderr="") + raise AssertionError(f"Unexpected gh call: {args}") monkeypatch.setattr("autoskillit._probe_canary.run_gh", mock_run_gh) updater = CanaryIssueUpdater(owner="test-org", repo="test-repo")