From 4447179448b82c4c3537b3788de33d93acb33ce5 Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 5 Oct 2026 09:12:28 -0700 Subject: [PATCH 01/25] feat(e2e): add sandbox headless smoke lifecycle and verification --- scripts/e2e/catalog.json | 20 + scripts/e2e/e2e_catalog.py | 17 +- scripts/e2e/e2e_harness.py | 574 +++++++++++++++- scripts/e2e/recipes/sandbox-smoke.yaml | 168 +++++ tests/infra/test_e2e_harness.py | 878 ++++++++++++++++++++++++- tests/infra/test_e2e_select.py | 92 ++- tests/infra/test_e2e_select_cli.py | 31 +- 7 files changed, 1741 insertions(+), 39 deletions(-) create mode 100644 scripts/e2e/recipes/sandbox-smoke.yaml diff --git a/scripts/e2e/catalog.json b/scripts/e2e/catalog.json index 8059cae320..00bf957ce1 100644 --- a/scripts/e2e/catalog.json +++ b/scripts/e2e/catalog.json @@ -50,6 +50,26 @@ "issue": "https://github.com/TalonT-Org/AutoSkillit/issues/5244" } ] + }, + { + "name": "headless-smoke", + "kind": "recipe", + "peak_sessions": 2, + "timeout_sec": 900, + "trigger_paths": [ + "src/autoskillit/execution/headless/*", + "src/autoskillit/execution/backends/*", + "src/autoskillit/cli/fleet/*", + "src/autoskillit/fleet/dispatch/*", + "src/autoskillit/server/*", + "src/autoskillit/hooks/*", + "scripts/e2e/*", + ".github/workflows/e2e.yml" + ], + "recipe": "sandbox-smoke", + "recipe_fixture": "sandbox-smoke.yaml", + "ingredients": {}, + "expected_pull_request_state": "closed" } ] } diff --git a/scripts/e2e/e2e_catalog.py b/scripts/e2e/e2e_catalog.py index e37d106b1d..46354d43f4 100644 --- a/scripts/e2e/e2e_catalog.py +++ b/scripts/e2e/e2e_catalog.py @@ -24,6 +24,7 @@ PULL_REQUEST_STATES = ("open", "merged", "closed") _NAME_PATTERN = re.compile(r"^[a-z0-9][a-z0-9-]*$") +_RECIPE_FIXTURE_PATTERN = re.compile(r"^[a-z0-9][a-z0-9-]*\.yaml$") _CATALOG_KEYS = frozenset({"sandbox_repository", "tests"}) _COMMON_KEYS = frozenset({"name", "kind", "peak_sessions", "timeout_sec", "trigger_paths"}) _RECIPE_KEYS = frozenset({"recipe", "ingredients", "expected_pull_request_state"}) @@ -48,6 +49,7 @@ class CatalogTest: ingredients: tuple[tuple[str, str], ...] expected_pull_request_state: str | None expected_failures: tuple[Mapping[str, str], ...] = () + recipe_fixture: str | None = None @dataclass(frozen=True, slots=True) @@ -156,7 +158,9 @@ def _parse_test(raw: object, index: int) -> CatalogTest: if kind not in KINDS: raise CatalogError(f"{where}.kind must be one of {KINDS}") keys = _COMMON_KEYS | _RECIPE_KEYS if kind == "recipe" else _COMMON_KEYS - if kind == "clean-install" and "expected_failures" in raw: + if kind == "recipe" and "recipe_fixture" in raw: + keys |= {"recipe_fixture"} + if kind in ("recipe", "clean-install") and "expected_failures" in raw: keys |= {"expected_failures"} _require_keys(raw, keys, where) name = _require_str(raw["name"], f"{where}.name") @@ -165,6 +169,14 @@ def _parse_test(raw: object, index: int) -> CatalogTest: recipe, ingredients, state = ( _parse_recipe_fields(raw, where) if kind == "recipe" else (None, (), None) ) + recipe_fixture = None + if kind == "recipe" and "recipe_fixture" in raw: + recipe_fixture = _require_str(raw["recipe_fixture"], f"{where}.recipe_fixture") + if not _RECIPE_FIXTURE_PATTERN.fullmatch(recipe_fixture): + raise CatalogError(f"{where}.recipe_fixture must be a .yaml basename") + expected_failures = _parse_expected_failures(raw.get("expected_failures", []), where) + if kind == "recipe" and any(row["severity"] != "error" for row in expected_failures): + raise CatalogError(f"{where}.expected_failures severity must be error for recipe tests") test = CatalogTest( name=name, kind=kind, @@ -179,7 +191,8 @@ def _parse_test(raw: object, index: int) -> CatalogTest: recipe=recipe, ingredients=ingredients, expected_pull_request_state=state, - expected_failures=_parse_expected_failures(raw.get("expected_failures", []), where), + expected_failures=expected_failures, + recipe_fixture=recipe_fixture, ) if job_timeout_minutes(test) > MAX_JOB_MINUTES: raise CatalogError(f"{where}.timeout_sec exceeds the {MAX_JOB_MINUTES}-minute job limit") diff --git a/scripts/e2e/e2e_harness.py b/scripts/e2e/e2e_harness.py index 558be58abd..215dbe268a 100644 --- a/scripts/e2e/e2e_harness.py +++ b/scripts/e2e/e2e_harness.py @@ -4,13 +4,16 @@ from __future__ import annotations import argparse +import ast import json import os +import re import shutil import subprocess import sys import tempfile import time +import uuid from collections.abc import Callable, Mapping, Sequence from pathlib import Path from typing import Any @@ -217,10 +220,18 @@ def clone_sandbox(repository: str, env: Mapping[str, str], runner: Runner) -> st def _list_pull_requests( - repository: str, limit: int, fields: str, env: Mapping[str, str], runner: Runner + repository: str, + limit: int, + fields: str, + env: Mapping[str, str], + runner: Runner, + *, + head: str | None = None, ) -> tuple[list[dict[str, Any]] | None, str | None]: argv = ["gh", "pr", "list", "--repo", repository, "--state", "all"] argv += ["--limit", str(limit), "--json", fields] + if head is not None: + argv += ["--head", head] result, failure = _setup_call(runner, argv, env) if failure is not None or result is None: return None, failure @@ -255,8 +266,12 @@ def new_pull_requests( return [row for row in rows if int(row["number"]) > baseline], None -def fleet_run_argv(test: e2e_catalog.CatalogTest) -> list[str]: - ingredients = [arg for key, value in test.ingredients for arg in ("-i", f"{key}={value}")] +def fleet_run_argv( + test: e2e_catalog.CatalogTest, runtime_ingredients: Mapping[str, str] | None = None +) -> list[str]: + effective = dict(test.ingredients) + effective.update(runtime_ingredients or {}) + ingredients = [arg for key, value in effective.items() for arg in ("-i", f"{key}={value}")] return [ "autoskillit", "fleet", @@ -274,39 +289,540 @@ def last_line(stdout: str) -> str: return lines[-1] if lines else "" -def check_envelope(returncode: int, stdout: str) -> list[str]: +def check_envelope(returncode: int, stdout: str) -> tuple[list[str], dict[str, Any] | None]: failures = [] if returncode == 0 else [f"fleet run: exit {returncode}"] line = last_line(stdout) try: envelope = json.loads(line) except json.JSONDecodeError: - return [*failures, "fleet run: the last stdout line is not a JSON envelope"] + return [*failures, "fleet run: the last stdout line is not a JSON envelope"], None if not isinstance(envelope, dict): failures.append("fleet run: the envelope is not a JSON object") + return failures, None elif envelope.get("success") is not True: failures.append("fleet run: the envelope does not report success") - return failures + return failures, envelope def run_fleet( - test: e2e_catalog.CatalogTest, out: Path, env: Mapping[str, str], runner: Runner -) -> list[str]: + test: e2e_catalog.CatalogTest, + out: Path, + env: Mapping[str, str], + runner: Runner, + runtime_ingredients: Mapping[str, str] | None = None, +) -> tuple[list[str], dict[str, Any] | None]: result, failure = _call( runner, - fleet_run_argv(test), + fleet_run_argv(test, runtime_ingredients), what="fleet run", cwd=SANDBOX_CLONE, env=env, timeout=test.timeout_sec + e2e_catalog.HARNESS_GRACE_SEC, + evidence=out / "fleet-run", ) if result is None: - return [str(failure)] + return [str(failure)], None (out / "fleet-run.stdout.log").write_text(result.stdout, encoding="utf-8") (out / "fleet-run.stderr.log").write_text(result.stderr, encoding="utf-8") (out / "envelope.json").write_text(last_line(result.stdout) + "\n", encoding="utf-8") return check_envelope(result.returncode, result.stdout) +def match_recipe_failure( + test: e2e_catalog.CatalogTest, + failures: list[str], + envelope: Mapping[str, Any] | None, +) -> tuple[list[str], list[dict[str, str]]]: + if envelope is None: + return failures, [] + if envelope.get("success") is True: + stale = [ + f"fleet run: remove stale expected failure for fixed bug {row['issue']}" + for row in test.expected_failures + ] + return [*failures, *stale], [] + valid = ( + envelope.get("success") is False + and envelope.get("kind") in ("completed", "rejected") + and ( + envelope.get("kind") == "rejected" + or isinstance(envelope.get("dispatch_status"), str) + and bool(envelope["dispatch_status"].strip()) + ) + and all( + isinstance(envelope.get(k), str) and envelope[k].strip() + for k in ("error", "user_visible_message") + ) + ) + if not valid: + return failures, [] + matched = [ + dict(row) + for row in test.expected_failures + if row["check"] == envelope["error"] and row["message"] == envelope["user_visible_message"] + ] + if not matched: + return failures, [] + consumed = { + "fleet run: the envelope does not report success", + *(f"fleet run: exit {code}" for code in (1, 2, 3)), + } + return [failure for failure in failures if failure not in consumed], matched + + +def _atomic_json(path: Path, payload: Mapping[str, Any]) -> None: + with tempfile.NamedTemporaryFile(mode="w", dir=path.parent, delete=False) as stream: + json.dump(payload, stream, indent=2) + stream.write("\n") + staged = Path(stream.name) + staged.replace(path) + + +def _validate_smoke_descriptor( + descriptor: Any, test: e2e_catalog.CatalogTest, repository: str +) -> dict[str, str]: + if not isinstance(descriptor, dict) or descriptor.get("test") != test.name: + raise ValueError("invalid smoke lifecycle test") + if descriptor.get("repository") != repository: + raise ValueError("invalid smoke lifecycle repository") + if not re.fullmatch(r"e2e-smoke-[0-9a-f]{32}", str(descriptor.get("branch_name", ""))): + raise ValueError("invalid smoke lifecycle branch") + base = descriptor.get("base_branch") + if ( + not isinstance(base, str) + or not re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9._/-]*", base) + or ".." in base + ): + raise ValueError("invalid smoke lifecycle base branch") + if base == descriptor["branch_name"]: + raise ValueError("smoke lifecycle branch equals base") + return descriptor + + +def prepare_smoke( + test: e2e_catalog.CatalogTest, + repository: str, + out: Path, + env: Mapping[str, str], + runner: Runner, +) -> tuple[list[str], dict[str, str] | None]: + source = Path(__file__).with_name("recipes") / str(test.recipe_fixture) + target = SANDBOX_CLONE / ".autoskillit" / "recipes" / f"{test.recipe}.yaml" + target.parent.mkdir(parents=True, exist_ok=True) + shutil.copyfile(source, target) + runtime = { + "source_dir": str(SANDBOX_CLONE), + "repository": repository, + "branch_name": f"e2e-smoke-{uuid.uuid4().hex}", + } + for key, argv in ( + ("remote_url", ["git", "remote", "get-url", "origin"]), + ("base_branch", ["git", "branch", "--show-current"]), + ): + result, failure = _setup_call(runner, argv, env, cwd=SANDBOX_CLONE) + if failure or result is None or not result.stdout.strip(): + return [failure or f"smoke setup: empty {key}"], None + runtime[key] = result.stdout.strip() + result, failure = _setup_call( + runner, + ["git", "ls-remote", "--exit-code", "origin", f"refs/heads/{runtime['branch_name']}"], + env, + cwd=SANDBOX_CLONE, + ) + if result is None or result.returncode != 2 or result.stdout.strip(): + return [failure or "smoke setup: assigned remote branch already exists"], None + descriptor = {key: runtime[key] for key in ("repository", "base_branch", "branch_name")} + descriptor["test"] = test.name + _validate_smoke_descriptor(descriptor, test, repository) + _atomic_json(out / "smoke-lifecycle.json", descriptor) + return [], runtime + + +def run_smoke_recipe( + test: e2e_catalog.CatalogTest, + repository: str, + out: Path, + env: Mapping[str, str], + runner: Runner, +) -> tuple[list[str], list[dict[str, str]]]: + failures, runtime = prepare_smoke(test, repository, out, env, runner) + if runtime is None: + return failures, [] + (out / "smoke-launched").write_text(test.name + "\n", encoding="utf-8") + fleet_failures, envelope = run_fleet(test, out, env, runner, runtime) + fleet_failures, matched = match_recipe_failure(test, fleet_failures, envelope) + failures += fleet_failures + if ( + envelope is not None + and envelope.get("success") is True + and (envelope.get("kind") != "completed" or envelope.get("dispatch_status") != "success") + ): + failures.append("smoke: invalid successful fleet envelope") + if not matched and envelope is not None and envelope.get("success") is True: + failures += verify_smoke(test, runtime, out, env, runner) + return failures, matched + + +def _smoke_pull_requests( + descriptor: Mapping[str, str], env: Mapping[str, str], runner: Runner +) -> tuple[list[dict[str, Any]] | None, str | None]: + return _list_pull_requests( + descriptor["repository"], + 100, + "number,state,headRefName,headRefOid,baseRefOid,additions,deletions,headRepository,headRepositoryOwner", + env, + runner, + head=descriptor["branch_name"], + ) + + +def _smoke_pr_matches(pr: Mapping[str, Any], descriptor: Mapping[str, str]) -> bool: + owner, repository = pr.get("headRepositoryOwner"), pr.get("headRepository") + identity = ( + f"{owner.get('login')}/{repository.get('name')}" + if isinstance(owner, dict) and isinstance(repository, dict) + else "" + ) + return ( + pr.get("headRefName") == descriptor["branch_name"] + and identity.lower() == descriptor["repository"].lower() + ) + + +def _ref_request( + descriptor: Mapping[str, str], + branch: str, + env: Mapping[str, str], + runner: Runner, + *, + method: str = "GET", +) -> tuple[int | None, dict[str, Any] | None, str | None]: + plural = "refs" if method == "DELETE" else "ref" + endpoint = f"/repos/{descriptor['repository']}/git/{plural}/heads/{branch}" + result, failure = _setup_call( + runner, ["gh", "api", "--include", "--method", method, endpoint], env + ) + if result is None: + return None, None, failure + header, _, body = result.stdout.replace("\r\n", "\n").partition("\n\n") + match = re.search(r"^HTTP/\S+ (\d{3})\b", header) + status = int(match[1]) if match else None + if method == "DELETE" and status == 204 and not failure: + return status, None, None + if method == "GET" and status == 404: + return status, None, None + if status != 200 or failure: + return status, None, failure or f"gh api: unexpected HTTP status {status}" + try: + payload = json.loads(body) + except json.JSONDecodeError: + return status, None, "gh api: invalid ref JSON" + if ( + not isinstance(payload, dict) + or payload.get("ref") != f"refs/heads/{branch}" + or not isinstance(payload.get("object"), dict) + or not re.fullmatch( + r"(?:[0-9a-f]{40}|[0-9a-f]{64})", str(payload["object"].get("sha", "")) + ) + ): + return status, None, "gh api: ref or commit OID does not match the request" + return status, payload, None + + +def _canary_sources_valid(code: str, test_code: str) -> bool: + try: + tree = ast.parse(test_code) + ast.parse(code) + except SyntaxError: + return False + imports = any( + isinstance(node, ast.ImportFrom) + and node.module == "sandbox.text" + and any(alias.name == "smoke_canary" for alias in node.names) + for node in ast.walk(tree) + ) + for node in ast.walk(tree): + if not isinstance(node, ast.ClassDef): + continue + is_case = any( + isinstance(base, ast.Attribute) + and base.attr == "TestCase" + or isinstance(base, ast.Name) + and base.id == "TestCase" + for base in node.bases + ) + for method in node.body: + if ( + not is_case + or not isinstance(method, ast.FunctionDef) + or not method.name.startswith("test") + ): + continue + for call in ast.walk(method): + if ( + isinstance(call, ast.Call) + and isinstance(call.func, ast.Attribute) + and call.func.attr.startswith("assert") + and any( + isinstance(inner, ast.Call) + and isinstance(inner.func, ast.Name) + and inner.func.id == "smoke_canary" + for inner in ast.walk(call) + ) + ): + return imports + return False + + +def _verify_smoke_commit( + head: str, base: str, out: Path, env: Mapping[str, str], runner: Runner +) -> tuple[list[str], str | None]: + results: dict[str, str] = {} + for name, args in ( + ("fetch", ["fetch", "--no-tags", "origin", head, base]), + ("type", ["cat-file", "-t", head]), + ("base", ["merge-base", base, head]), + ): + result, failure = _setup_call(runner, ["git", *args], env, cwd=SANDBOX_CLONE) + if failure or result is None: + return [failure or f"smoke: missing {name} result"], None + results[name] = result.stdout.strip() + if results["type"] != "commit" or not re.fullmatch( + r"[0-9a-f]{40}|[0-9a-f]{64}", results["base"] + ): + return ["smoke: fetched immutable commit or comparison base is invalid"], None + result, failure = _setup_call( + runner, + ["git", "diff", "--name-only", results["base"], head, "--"], + env, + cwd=SANDBOX_CLONE, + ) + paths = ("sandbox/text.py", "tests/test_smoke_canary.py") + if failure or result is None: + return [failure or "smoke: missing complete diff"], None + if set(result.stdout.splitlines()) != set(paths): + return [ + "smoke: complete PR diff must change only " + "sandbox/text.py and tests/test_smoke_canary.py" + ], None + sources = [] + for path in paths: + result, failure = _setup_call( + runner, ["git", "show", f"{head}:{path}"], env, cwd=SANDBOX_CLONE + ) + if failure or result is None: + return [failure or f"smoke: missing {path}"], None + sources.append(result.stdout) + if not _canary_sources_valid(*sources): + return ["smoke: missing executable unittest canary assertion"], None + with tempfile.TemporaryDirectory(dir=out) as directory: + root = Path(directory) + for path, source in zip(paths, sources, strict=True): + target = root / path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(source, encoding="utf-8") + script = ( + "import unittest; from sandbox.text import smoke_canary; " + "assert smoke_canary() is True; " + "suite=unittest.defaultTestLoader.discover('tests'); " + "assert suite.countTestCases()>0; " + "result=unittest.TextTestRunner().run(suite); " + "raise SystemExit(0 if result.wasSuccessful() else 1)" + ) + _, failure = _setup_call( + runner, [sys.executable, "-c", script], env, cwd=root, evidence=out / "smoke-canary" + ) + return ([failure] if failure else []), results["base"] + + +def verify_smoke( + test: e2e_catalog.CatalogTest, + runtime: Mapping[str, str], + out: Path, + env: Mapping[str, str], + runner: Runner, +) -> list[str]: + rows, failure = _smoke_pull_requests(runtime, env, runner) + if rows is None: + return [str(failure)] + failures = check_pull_requests(rows, test.expected_pull_request_state) + if failures: + return failures + pr = rows[0] + if not _smoke_pr_matches(pr, runtime): + return ["smoke: PR head branch or repository does not match the sandbox"] + head, base = pr.get("headRefOid"), pr.get("baseRefOid") + if not isinstance(head, str) or not isinstance(base, str): + return ["smoke: invalid PR commit OIDs"] + if not all(re.fullmatch(r"[0-9a-f]{40}|[0-9a-f]{64}", oid) for oid in (head, base)): + return ["smoke: invalid PR commit OIDs"] + status, ref, failure = _ref_request(runtime, runtime["branch_name"], env, runner) + if failure or status != 200 or ref is None or ref["object"]["sha"] != head: + return [failure or "smoke: remote branch missing or does not match the PR head"] + failures, comparison_base = _verify_smoke_commit(head, base, out, env, runner) + latest, failure = _smoke_pull_requests(runtime, env, runner) + if failure or latest is None or len(latest) != 1 or latest[0].get("headRefOid") != head: + failures.append(failure or "smoke: PR head changed during verification") + _atomic_json( + out / "smoke-evidence.json", + { + "number": pr["number"], + "branch_name": runtime["branch_name"], + "head_commit": head, + "comparison_base": comparison_base, + "pull_request": pr, + "pre_cleanup_ref": ref, + "canary_verified": not failures, + }, + ) + return failures + + +def _close_smoke_pull_requests( + descriptor: Mapping[str, str], env: Mapping[str, str], runner: Runner +) -> list[str]: + failures: list[str] = [] + rows, failure = _smoke_pull_requests(descriptor, env, runner) + if failure: + failures.append(failure) + for pr in rows or []: + if not _smoke_pr_matches(pr, descriptor): + failures.append("cleanup: PR does not match assigned branch") + continue + if str(pr.get("state", "")).lower() == "open": + time.sleep(1) + _, failure = _setup_call( + runner, + ["gh", "pr", "close", str(pr["number"]), "--repo", descriptor["repository"]], + env, + ) + if failure: + failures.append(failure) + return failures + + +def _cleanup_smoke_resources( + descriptor: Mapping[str, str], env: Mapping[str, str], runner: Runner +) -> tuple[list[str], dict[str, Any]]: + failures: list[str] = [] + base_status, _, failure = _ref_request(descriptor, descriptor["base_branch"], env, runner) + access_verified = base_status == 200 and failure is None + if not access_verified: + failures.append(failure or "cleanup: base ref read access could not be verified") + failures += _close_smoke_pull_requests(descriptor, env, runner) + status, ref, failure = _ref_request(descriptor, descriptor["branch_name"], env, runner) + if failure: + failures.append(failure) + elif status == 200: + time.sleep(1) + _, _, failure = _ref_request( + descriptor, descriptor["branch_name"], env, runner, method="DELETE" + ) + if failure: + failures.append(failure) + elif status != 404 or not access_verified: + failures.append("cleanup: assigned ref absence could not be verified") + final_rows, failure = _smoke_pull_requests(descriptor, env, runner) + if failure: + failures.append(failure) + elif final_rows is None or any( + not _smoke_pr_matches(pr, descriptor) + or str(pr.get("state", "")).lower() not in ("closed", "merged") + for pr in final_rows + ): + failures.append("cleanup: assigned branch still has an open or invalid PR") + final_status, _, failure = _ref_request(descriptor, descriptor["branch_name"], env, runner) + if failure: + failures.append(failure) + elif final_status != 404 or not access_verified: + failures.append("cleanup: assigned remote ref is not verified absent") + return failures, { + "branch_name": descriptor["branch_name"], + "pre_cleanup_ref": ref, + "final_pull_requests": final_rows, + "final_ref_status": final_status, + "access_verified": access_verified, + "failures": failures, + } + + +def _model_result(out: Path, test_name: str) -> tuple[dict[str, Any], list[str]]: + path = out / "result.json" + try: + raw = path.read_bytes() + payload = json.loads(raw) + if ( + not isinstance(payload, dict) + or payload.get("test") != test_name + or not isinstance(payload.get("passed"), bool) + or not isinstance(payload.get("failures"), list) + or not all(isinstance(failure, str) for failure in payload["failures"]) + or payload.get("outcome") not in ("passed", "expected_failure", "failed") + or not isinstance(payload.get("expected_findings"), list) + ): + raise ValueError("invalid model result fields") + if payload["passed"] != (payload["outcome"] == "passed"): + raise ValueError("model result passed/outcome disagree") + if bool(payload["failures"]) != (payload["outcome"] == "failed"): + raise ValueError("model result failures/outcome disagree") + if payload["outcome"] == "expected_failure" and not payload["expected_findings"]: + raise ValueError("model result has no matched expected finding") + return payload, [] + except (OSError, ValueError) as exc: + if path.is_file(): + shutil.copyfile(path, out / "result-before-cleanup.json") + result = { + "test": test_name, + "passed": False, + "outcome": "failed", + "failures": [], + "expected_findings": [], + } + return result, [f"cleanup: missing or malformed model result: {exc}"] + + +def cleanup_test( + catalog: e2e_catalog.Catalog, + test_name: str, + runner: Runner, + out: Path, + env: Mapping[str, str], +) -> int: + test = catalog.get(test_name) + if test.recipe_fixture is None or not (out / "smoke-launched").exists(): + return 0 + out.mkdir(parents=True, exist_ok=True) + result, failures = _model_result(out, test_name) + evidence: dict[str, Any] = {} + try: + descriptor = _validate_smoke_descriptor( + json.loads((out / "smoke-lifecycle.json").read_text(encoding="utf-8")), + test, + catalog.sandbox_repository, + ) + token = env.get("E2E_SANDBOX_TOKEN", "") + if not token: + failures.append("cleanup: E2E_SANDBOX_TOKEN is not set") + else: + scrubbed = child_env(env) + failure = authenticate_gh(token, scrubbed, runner) + if failure: + failures.append(failure) + resource_failures, evidence = _cleanup_smoke_resources(descriptor, scrubbed, runner) + failures += resource_failures + except Exception as exc: + failures.append(f"cleanup error: {exc}") + evidence["failures"] = list(failures) + _atomic_json(out / "smoke-cleanup.json", evidence) + result["failures"] += failures + if result["failures"]: + result.update(passed=False, outcome="failed") + _atomic_json(out / "result.json", result) + for failure in failures: + print(f"e2e_harness: FAIL {failure}", file=sys.stderr) + return 1 if result["outcome"] == "failed" else 0 + + def check_pull_requests(prs: Sequence[Mapping[str, Any]], expected_state: str | None) -> list[str]: if len(prs) != 1: return [f"expected exactly one new sandbox pull request, found {len(prs)}"] @@ -349,29 +865,34 @@ def run_recipe( out: Path, env: Mapping[str, str], runner: Runner, -) -> list[str]: +) -> tuple[list[str], list[dict[str, str]]]: failure = ( configure_git(env, runner) or authenticate_gh(token, env, runner) or clone_sandbox(repository, env, runner) ) if failure is not None: - return [failure] + return [failure], [] + if test.recipe_fixture is not None: + return run_smoke_recipe(test, repository, out, env, runner) baseline, failure = latest_pull_request_number(repository, env, runner) if baseline is None: - return [str(failure)] + return [str(failure)], [] failures: list[str] = [] listed: list[dict[str, Any]] | None = None try: - failures += run_fleet(test, out, env, runner) - listed, failure = new_pull_requests(repository, baseline, env, runner) - if listed is None: - failures.append(str(failure)) - else: - failures += check_pull_requests(listed, test.expected_pull_request_state) + fleet_failures, envelope = run_fleet(test, out, env, runner) + fleet_failures, matched = match_recipe_failure(test, fleet_failures, envelope) + failures += fleet_failures + if not matched: + listed, failure = new_pull_requests(repository, baseline, env, runner) + if listed is None: + failures.append(str(failure)) + else: + failures += check_pull_requests(listed, test.expected_pull_request_state) finally: failures += close_open_pull_requests(repository, baseline, listed, env, runner) - return failures + return failures, matched def check_clean_install_doctor( @@ -485,7 +1006,7 @@ def _run_test( if test.kind == "canary": return run_canary(test, out, scrubbed, runner), [] token = env["E2E_SANDBOX_TOKEN"] - return run_recipe(test, catalog.sandbox_repository, token, out, scrubbed, runner), [] + return run_recipe(test, catalog.sandbox_repository, token, out, scrubbed, runner) def run_test( @@ -507,7 +1028,7 @@ def run_test( except Exception as exc: failures = [f"harness error: {exc}"] result = {"test": test.name, "passed": not failures, "failures": failures} - if test.kind == "clean-install": + if test.kind in ("clean-install", "recipe"): outcome = "failed" if failures else "expected_failure" if matched_findings else "passed" result.update( outcome=outcome, passed=outcome == "passed", expected_findings=matched_findings @@ -583,7 +1104,7 @@ def _run(test_name: str, out: Path, catalog_path: Path) -> int: for failure in failures: print(f"e2e_harness: FAIL {failure}", file=sys.stderr) outcome = "failed" if failures else "passed" - if test.kind == "clean-install": + if test.kind in ("clean-install", "recipe"): outcome = json.loads((out / "result.json").read_text(encoding="utf-8"))["outcome"] print(f"e2e_harness: {test.name} {outcome}") return 1 if failures else 0 @@ -607,6 +1128,10 @@ def main(argv: Sequence[str]) -> int: run.add_argument("--test", required=True) run.add_argument("--out", required=True) run.add_argument("--catalog", default=str(e2e_catalog.CATALOG_PATH)) + cleanup = commands.add_parser("cleanup", help="Verify exact smoke resource cleanup.") + cleanup.add_argument("--test", required=True) + cleanup.add_argument("--out", required=True) + cleanup.add_argument("--catalog", default=str(e2e_catalog.CATALOG_PATH)) redact = commands.add_parser("redact", help="Copy artifacts with every secret redacted.") redact.add_argument("--dest", required=True) redact.add_argument("--secret-env", action="append", required=True) @@ -614,6 +1139,9 @@ def main(argv: Sequence[str]) -> int: args = parser.parse_args(argv) if args.command == "run": return _run(args.test, Path(args.out), Path(args.catalog)) + if args.command == "cleanup": + catalog = e2e_catalog.load_catalog(Path(args.catalog)) + return cleanup_test(catalog, args.test, run_command, Path(args.out), os.environ) return _redact_command(Path(args.dest), args.secret_env, args.sources) diff --git a/scripts/e2e/recipes/sandbox-smoke.yaml b/scripts/e2e/recipes/sandbox-smoke.yaml new file mode 100644 index 0000000000..56643fb637 --- /dev/null +++ b/scripts/e2e/recipes/sandbox-smoke.yaml @@ -0,0 +1,168 @@ +name: sandbox-smoke +recipe_version: "1.0.0" +description: >- + Run one headless smoke worker in a sandbox worktree, verify its canary, + publish a pull request, then close it without merging or deleting the branch. +summary: worktree > setup > implement > commit > test > push > open PR > close PR +requires_packs: [kitchen-core, github] + +ingredients: + source_dir: + description: Path to the sandbox clone used as the worktree source. + required: true + hidden: true + repository: + description: Owner and repository where the smoke pull request is opened. + required: true + hidden: true + branch_name: + description: Exact UUID branch name assigned by the E2E harness. + required: true + hidden: true + remote_url: + description: Sandbox repository URL used to push the smoke branch. + required: true + hidden: true + base_branch: + description: Sandbox branch targeted by the smoke pull request. + required: true + hidden: true + +kitchen_rules: + - >- + Run exactly one smoke-task worker. Do not spawn child agents, replay the + worker call, or dispatch tools to additional workers. Do not resume or + replay the task, even if continuation data is returned. The worker makes + the only new commit, containing only the two requested files. + - >- + Preserve the exact branch_name supplied by the harness. Create its genuine + git worktree from source_dir with the bundled create_impl_worktree.sh script. + - >- + Create and close one pull request in repository. Never merge it, post a + comment, or delete the remote branch. + - >- + The orchestrator must not use native Claude Code tools (Read, Grep, Glob, + Edit, Write, Bash, Agent, WebFetch, WebSearch, NotebookEdit). + +steps: + create_worktree: + tool: run_cmd + with: + cmd: >- + bash {{AUTOSKILLIT_SCRIPTS}}/create_impl_worktree.sh + "${{ inputs.branch_name }}" "{{AUTOSKILLIT_TEMP}}" + cwd: "${{ inputs.source_dir }}" + step_name: create_worktree + capture: + worktree_path: "${{ result.worktree_path }}" + branch_name: "${{ result.branch_name }}" + on_success: setup + on_failure: failed + retries: 0 + on_exhausted: failed + + setup: + tool: run_cmd + with: + cmd: task install-worktree + cwd: "${{ context.worktree_path }}" + step_name: setup + on_success: implement + on_failure: failed + retries: 0 + on_exhausted: failed + + implement: + tool: run_skill + with: + skill_command: >- + /autoskillit:smoke-task In ${{ context.worktree_path }}, append this + function to sandbox/text.py: `def smoke_canary() -> bool:` followed by + ` return True`. Create tests/test_smoke_canary.py as a unittest + module importing unittest and smoke_canary from sandbox.text, with a + unittest.TestCase method using `self.assertIs(smoke_canary(), True)`. Change + only those two files, then create one new commit containing only + sandbox/text.py and tests/test_smoke_canary.py with message + `smoke: add sandbox canary`. Do not run a test command or call tools + that dispatch work to another worker. + cwd: "${{ context.worktree_path }}" + step_name: implement + on_success: test + on_failure: failed + on_context_limit: failed + on_rate_limit: failed + retries: 0 + on_exhausted: failed + + test: + tool: test_check + with: + worktree_path: "${{ context.worktree_path }}" + on_success: push + on_failure: failed + retries: 0 + on_exhausted: failed + + push: + tool: push_to_remote + with: + clone_path: "${{ context.worktree_path }}" + branch: "${{ context.branch_name }}" + remote_url: "${{ inputs.remote_url }}" + step_name: push + on_success: create_pull_request + on_failure: failed + retries: 0 + on_exhausted: failed + + create_pull_request: + tool: run_cmd + with: + cmd: >- + body_file="{{AUTOSKILLIT_TEMP}}/sandbox-smoke-pr-body.md" && + mkdir -p "{{AUTOSKILLIT_TEMP}}" && + printf '%s\n' 'Headless sandbox smoke canary.' > "$body_file" && + gh pr create --repo "${{ inputs.repository }}" + --head "${{ context.branch_name }}" + --base "${{ inputs.base_branch }}" + --title 'Automated smoke test (DO NOT MERGE)' + --body-file "$body_file" + cwd: "${{ context.worktree_path }}" + step_name: create_pull_request + on_success: close_pull_request + on_failure: failed + retries: 0 + on_exhausted: failed + + close_pull_request: + tool: run_cmd + with: + cmd: >- + pull_request_number="$(gh pr view "${{ context.branch_name }}" + --repo "${{ inputs.repository }}" --json number --jq .number)" && + sleep 1 && + gh pr close "$pull_request_number" + --repo "${{ inputs.repository }}" + cwd: "${{ context.worktree_path }}" + step_name: close_pull_request + on_success: done + on_failure: failed + retries: 0 + on_exhausted: failed + + done: + action: stop + message: >- + The headless sandbox smoke completed and its pull request was closed. + Emit the L3 result sentinel JSON block with + {"success": true, "reason": "sandbox smoke completed"}. + + failed: + action: stop + message: >- + The headless sandbox smoke failed. Emit the L3 result sentinel JSON block + with success=false, preserving the observed failed step, its exact reason, + and the relevant evidence already in context. Do not invent details. For + example: {"success": false, "failed_step": "", + "reason": "", "evidence": ""}. + Do not call any tools, retry, recover, resume, or replay this task. diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index 1e800b9d3e..fa94fd8715 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -9,7 +9,7 @@ import subprocess import tempfile from collections.abc import Callable -from dataclasses import dataclass +from dataclasses import dataclass, replace from pathlib import Path import pytest @@ -43,6 +43,12 @@ ) _SKILL_NOTICE = "2 skills unavailable on this backend (codex): a, b" _CLEAN_INSTALL_ISSUE = "https://github.com/TalonT-Org/AutoSkillit/issues/5231" +_SMOKE_FAILURE = { + "severity": "error", + "check": "workflow_failed", + "message": "model reported failure", + "issue": "https://github.com/TalonT-Org/AutoSkillit/issues/5232", +} @dataclass @@ -166,6 +172,834 @@ def _run_canary(tmp_path: Path, runner: FakeRunner, monkeypatch, env=None): return failures, json.loads((out / "result.json").read_text(encoding="utf-8")) +def _smoke_case(expected_failures=None): + catalog = e2e_catalog.load_catalog() + test = catalog.get("headless-smoke") + if expected_failures is not None: + test = replace(test, expected_failures=tuple(expected_failures)) + catalog = replace( + catalog, + tests=tuple(test if entry.name == test.name else entry for entry in catalog.tests), + ) + return catalog, test + + +def _run_git(cwd: Path, *argv: str, env: dict[str, str]) -> str: + result = subprocess.run( + ["git", *argv], cwd=cwd, env=env, capture_output=True, text=True, check=True + ) + return result.stdout.strip() + + +def _smoke_sources(*, valid_test: bool = True) -> tuple[str, str]: + code = '"""Sandbox smoke target."""\n\ndef smoke_canary() -> bool:\n return True\n' + if valid_test: + test_code = ( + "import unittest\n" + "from sandbox.text import smoke_canary\n\n" + "class SmokeCanaryTest(unittest.TestCase):\n" + " def test_smoke_canary(self):\n" + " self.assertTrue(smoke_canary())\n" + ) + else: + test_code = "import unittest\n\nclass SmokeCanaryTest:\n pass\n" + return code, test_code + + +def _smoke_metadata( + argv: list[str], mode: str, state: dict[str, str], remote_url: str +) -> subprocess.CompletedProcess[str]: + if argv[:2] == ["git", "remote"]: + assert argv == ["git", "remote", "get-url", "origin"] + return _completed(argv, remote_url + "\n") + if argv[:2] == ["git", "branch"]: + assert argv == ["git", "branch", "--show-current"] + return _completed(argv, "main\n") + assert argv[:2] == ["git", "ls-remote"] + state["branch_name"] = argv[-1].removeprefix("refs/heads/") + assert argv == [ + "git", + "ls-remote", + "--exit-code", + "origin", + f"refs/heads/{state['branch_name']}", + ] + assert argv == [ + "git", + "ls-remote", + "--exit-code", + "origin", + f"refs/heads/{state['branch_name']}", + ] + collision = mode == "branch-collision" + record = f"{'a' * 40}\trefs/heads/{state['branch_name']}\n" if collision else "" + return _completed(argv, record, returncode=0 if collision else 2) + + +def _smoke_fleet( + argv: list[str], + call: RecordedCall, + clone: Path, + out: Path, + fleet: subprocess.CompletedProcess[str] | BaseException | None, + state: dict[str, str], +) -> subprocess.CompletedProcess[str] | BaseException: + assert argv[:3] == ["autoskillit", "fleet", "run"] and call.cwd == clone + assert (clone / ".autoskillit" / "recipes" / "sandbox-smoke.yaml").is_file() + assert (out / "smoke-launched").is_file() + descriptor = json.loads((out / "smoke-lifecycle.json").read_text(encoding="utf-8")) + state.update(branch_name=descriptor["branch_name"], base_oid="b" * 40, head_oid="a" * 40) + fields = [ + argv[index + 1].split("=", 1) for index, item in enumerate(argv[:-1]) if item == "-i" + ] + ingredients = dict(fields) + assert len(fields) == len(ingredients) == 5 + assert ingredients == { + "source_dir": str(clone), + "repository": state["repository"], + "remote_url": state["remote_url"], + "base_branch": "main", + "branch_name": descriptor["branch_name"], + } + if isinstance(fleet, BaseException): + return fleet + if fleet is not None: + return _completed( + argv, + fleet.stdout or "", + returncode=fleet.returncode, + stderr=fleet.stderr or "", + ) + return _completed( + argv, + json.dumps({"success": True, "kind": "completed", "dispatch_status": "success"}), + ) + + +def _smoke_pr_list( + argv: list[str], mode: str, state: dict[str, str] +) -> subprocess.CompletedProcess[str]: + branch = argv[argv.index("--head") + 1] + assert branch == state["branch_name"] + assert argv[argv.index("--repo") + 1] == state["repository"] + fields = ( + "number,state,headRefName,headRefOid,baseRefOid,additions,deletions," + "headRepository,headRepositoryOwner" + ) + assert argv == [ + "gh", + "pr", + "list", + "--repo", + state["repository"], + "--state", + "all", + "--limit", + "100", + "--json", + fields, + "--head", + branch, + ] + if mode == "no-pr": + rows = [] + else: + rows = [ + { + "number": 8, + "state": "OPEN" if mode == "open-pr" else "CLOSED", + "headRefName": "other-branch" if mode == "wrong-pr-branch" else branch, + "headRefOid": "d" * 40 if mode == "wrong-pr-head" else state["head_oid"], + "baseRefOid": state["base_oid"], + "additions": 3, + "deletions": 0, + "headRepository": {"name": state["repository"].split("/", 1)[1]}, + "headRepositoryOwner": {"login": state["repository"].split("/", 1)[0]}, + } + ] + return _completed(argv, json.dumps(rows)) + + +def _smoke_ref( + argv: list[str], mode: str, state: dict[str, str] +) -> subprocess.CompletedProcess[str]: + assert argv[argv.index("--method") + 1] == "GET" + branch = argv[-1].rsplit("/", 1)[-1] + assert branch == state["branch_name"] + assert argv == [ + "gh", + "api", + "--include", + "--method", + "GET", + f"/repos/{state['repository']}/git/ref/heads/{branch}", + ] + if mode == "missing-ref": + return _completed(argv, "HTTP/2.0 404 Not Found\r\n\r\n", returncode=1) + ref_oid = "d" * 40 if mode == "wrong-ref" else state["head_oid"] + body = json.dumps({"ref": f"refs/heads/{branch}", "object": {"sha": ref_oid}}) + return _completed(argv, f"HTTP/2.0 200 OK\r\n\r\n{body}") + + +def _smoke_git_verification( + argv: list[str], mode: str, state: dict[str, str] +) -> subprocess.CompletedProcess[str]: + if argv[1] == "fetch": + assert argv == [ + "git", + "fetch", + "--no-tags", + "origin", + state["head_oid"], + state["base_oid"], + ] + return _completed(argv) + if argv[1] == "cat-file": + assert argv == ["git", "cat-file", "-t", state["head_oid"]] + return _completed(argv, "commit\n") + if argv[1] == "merge-base": + assert argv == ["git", "merge-base", state["base_oid"], state["head_oid"]] + return _completed(argv, state["base_oid"] + "\n") + if argv[1] == "diff": + assert argv == ["git", "diff", "--name-only", state["base_oid"], state["head_oid"], "--"] + paths = ["sandbox/text.py", "tests/test_smoke_canary.py"] + if mode == "earlier-unrelated-commit": + paths.append("sandbox/unrelated.py") + return _completed(argv, "\n".join(paths) + "\n") + prefix = state["head_oid"] + ":" + assert argv[1] == "show" and argv[2].startswith(prefix) + path = argv[2][len(prefix) :] + assert path in ("sandbox/text.py", "tests/test_smoke_canary.py") + code, test_code = _smoke_sources(valid_test=mode != "invalid-canary") + return _completed(argv, code if path == "sandbox/text.py" else test_code) + + +def _smoke_unittest(argv: list[str], call: RecordedCall) -> subprocess.CompletedProcess[str]: + assert argv[0] == harness.sys.executable and argv[1:2] == ["-c"] + assert "unittest.defaultTestLoader.discover" in argv[2] + return subprocess.run( + argv, + cwd=call.cwd, + env=call.env, + capture_output=True, + text=True, + timeout=call.timeout, + check=False, + ) + + +def _smoke_setup( + argv: list[str], clone: Path, repository: str +) -> subprocess.CompletedProcess[str]: + if argv[:3] == ["git", "config", "--global"]: + assert argv in ( + ["git", "config", "--global", "user.name", "AutoSkillit E2E"], + [ + "git", + "config", + "--global", + "user.email", + "autoskillit-e2e@users.noreply.github.com", + ], + ) + return _completed(argv) + if argv[:2] == ["gh", "auth"]: + assert argv[2:] in (["login", "--hostname", "github.com", "--with-token"], ["setup-git"]) + return _completed(argv) + assert argv == ["gh", "repo", "clone", repository, str(clone)] + return _completed(argv) + + +def _smoke_runner( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + *, + mode: str = "success", + fleet: subprocess.CompletedProcess[str] | BaseException | None = None, +) -> tuple[FakeRunner, Path]: + clone = tmp_path / "sandbox" + clone.mkdir() + monkeypatch.setattr(harness, "SANDBOX_CLONE", clone) + out = tmp_path / "out" + catalog, _test = _smoke_case() + repository = catalog.sandbox_repository + remote_url = f"https://github.com/{repository}.git" + state = {"repository": repository, "remote_url": remote_url} + routes = { + ("git", "config"): lambda argv, _call: _smoke_setup(argv, clone, repository), + ("gh", "auth"): lambda argv, _call: _smoke_setup(argv, clone, repository), + ("gh", "repo"): lambda argv, _call: _smoke_setup(argv, clone, repository), + ("git", "remote"): lambda argv, _call: _smoke_metadata(argv, mode, state, remote_url), + ("git", "branch"): lambda argv, _call: _smoke_metadata(argv, mode, state, remote_url), + ("git", "ls-remote"): lambda argv, _call: _smoke_metadata(argv, mode, state, remote_url), + ("autoskillit", "fleet"): lambda argv, call: _smoke_fleet( + argv, call, clone, out, fleet, state + ), + ("gh", "pr"): lambda argv, _call: _smoke_pr_list(argv, mode, state), + ("gh", "api"): lambda argv, _call: _smoke_ref(argv, mode, state), + ("git", "fetch"): lambda argv, _call: _smoke_git_verification(argv, mode, state), + ("git", "cat-file"): lambda argv, _call: _smoke_git_verification(argv, mode, state), + ("git", "merge-base"): lambda argv, _call: _smoke_git_verification(argv, mode, state), + ("git", "diff"): lambda argv, _call: _smoke_git_verification(argv, mode, state), + ("git", "show"): lambda argv, _call: _smoke_git_verification(argv, mode, state), + (harness.sys.executable, "-c"): _smoke_unittest, + } + runner: FakeRunner + + def handle(argv: list[str]): + if len(argv) < 2: + raise AssertionError(f"unexpected smoke command: {argv!r}") + route = routes.get((argv[0], argv[1])) + if route is None: + raise AssertionError(f"unexpected smoke command: {argv!r}") + return route(argv, runner.calls[-1]) + + runner = FakeRunner(handle) + return runner, clone + + +def _run_smoke(tmp_path: Path, runner: FakeRunner, monkeypatch: pytest.MonkeyPatch, env=None): + monkeypatch.setattr(tempfile, "tempdir", str(tmp_path)) + catalog, test = _smoke_case() + out = tmp_path / "out" + failures = harness.run_test( + test, + catalog, + out=out, + home=tmp_path / "home", + env=env if env is not None else _env(HOME=str(tmp_path / "home")), + runner=runner, + ) + return failures, json.loads((out / "result.json").read_text(encoding="utf-8")), out, catalog + + +class TestSandboxSmokeFixture: + def test_fixture_stages_discovers_and_passes_the_production_validator(self, tmp_path: Path): + from autoskillit.recipe.io import find_recipe_by_name, load_recipe + from autoskillit.recipe.validator import validate_recipe_structure + + _catalog, test = _smoke_case() + assert test.recipe is not None and test.recipe_fixture is not None + project = tmp_path / "project" + target = project / ".autoskillit" / "recipes" / f"{test.recipe}.yaml" + target.parent.mkdir(parents=True) + source = Path(harness.__file__).with_name("recipes") / test.recipe_fixture + target.write_bytes(source.read_bytes()) + + discovered = find_recipe_by_name(str(test.recipe), project) + assert discovered is not None + assert discovered.path == target + recipe = load_recipe(discovered.path) + assert validate_recipe_structure(recipe) == [] + + skill_steps = [step for step in recipe.steps.values() if step.tool == "run_skill"] + assert len(skill_steps) == 1 + worker = skill_steps[0] + assert worker.skill_name == "smoke-task" + prompt = worker.with_args["skill_command"].lower() + assert "one new commit" in prompt and "only sandbox/text.py" in prompt + discipline = " ".join(recipe.kitchen_rules).lower() + assert "child agents" in discipline and "replay" in discipline + assert recipe.steps["setup"].with_args["cwd"] == "${{ context.worktree_path }}" + assert worker.with_args["cwd"] == "${{ context.worktree_path }}" + assert recipe.steps["test"].tool == "test_check" + assert recipe.steps["test"].with_args["worktree_path"] == "${{ context.worktree_path }}" + assert recipe.steps["push"].with_args["clone_path"] == "${{ context.worktree_path }}" + assert recipe.steps["push"].on_success == "create_pull_request" + assert recipe.steps["create_pull_request"].on_success == "close_pull_request" + assert recipe.steps["close_pull_request"].on_success == "done" + assert "DO NOT MERGE" in recipe.steps["create_pull_request"].with_args["cmd"] + executable = [step for step in recipe.steps.values() if step.tool or step.python] + assert all( + step.retries == 0 and step.on_failure == "failed" and step.on_exhausted == "failed" + for step in executable + ) + done = recipe.steps["done"] + failed = recipe.steps["failed"] + assert done.action == "stop" and '"success": true' in done.message + assert failed.action == "stop" + assert all( + word in failed.message.lower() for word in ("success", "false", "reason", "evidence") + ) + assert "do not call any tools" in failed.message.lower() + + +class TestSandboxSmokeFlow: + def test_success_verifies_exact_closed_pr_ref_complete_diff_and_canary( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + runner, clone = _smoke_runner(tmp_path, monkeypatch) + external_env = _env(HOME=str(tmp_path / "home")) + failures, result, out, _catalog = _run_smoke( + tmp_path, runner, monkeypatch, env=external_env + ) + + descriptor = json.loads((out / "smoke-lifecycle.json").read_text(encoding="utf-8")) + evidence = json.loads((out / "smoke-evidence.json").read_text(encoding="utf-8")) + settings = json.loads( + (tmp_path / "home" / ".claude" / "settings.json").read_text(encoding="utf-8") + ) + assert failures == [] + assert result["outcome"] == "passed" + assert result["expected_findings"] == [] + assert descriptor == { + "repository": _catalog.sandbox_repository, + "base_branch": "main", + "branch_name": descriptor["branch_name"], + "test": "headless-smoke", + } + assert descriptor["branch_name"].startswith("e2e-smoke-") + assert len(descriptor["branch_name"]) == len("e2e-smoke-") + 32 + assert (out / "smoke-launched").read_text(encoding="utf-8") == "headless-smoke\n" + assert (clone / ".autoskillit" / "recipes" / "sandbox-smoke.yaml").is_file() + assert evidence["branch_name"] == descriptor["branch_name"] + assert evidence["pre_cleanup_ref"]["object"]["sha"] == evidence["head_commit"] + assert evidence["canary_verified"] is True + assert settings["env"] == _EXPECTED_SETTINGS_ENV + assert not (clone / ".claude" / "settings.json").exists() + assert not (clone / ".autoskillit" / "config.yaml").exists() + assert all( + not any(name.startswith("ANTHROPIC_") or "OAUTH" in name for name in call.env) + for call in runner.calls + if call.env is not None + ) + assert [call.input for call in runner.calls if call.input] == [SANDBOX_TOKEN] + assert MINIMAX_KEY not in (out / "smoke-lifecycle.json").read_text(encoding="utf-8") + assert SANDBOX_TOKEN not in (out / "smoke-evidence.json").read_text(encoding="utf-8") + assert sum(call.argv[:2] == ["git", "show"] for call in runner.calls) == 2 + assert sum(call.argv[:3] == ["gh", "pr", "list"] for call in runner.calls) == 2 + + @pytest.mark.parametrize( + "fault", + [ + "nonzero", + "timeout", + "exception", + "no-pr", + "open-pr", + "wrong-pr-branch", + "wrong-pr-head", + "wrong-ref", + "missing-ref", + "earlier-unrelated-commit", + "invalid-canary", + ], + ) + def test_failed_launch_or_verification_keeps_lifecycle_evidence( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, fault: str + ) -> None: + mode = fault if fault not in {"nonzero", "timeout", "exception"} else "success" + fleet = None + if fault == "nonzero": + fleet = _completed( + [], + json.dumps( + { + "success": False, + "kind": "completed", + "dispatch_status": "failed", + "error": "workflow_failed", + "user_visible_message": "model reported failure", + } + ), + returncode=1, + ) + elif fault == "timeout": + fleet = subprocess.TimeoutExpired(["autoskillit", "fleet", "run"], 600) + elif fault == "exception": + fleet = RuntimeError("fleet runner failed") + runner, _clone = _smoke_runner(tmp_path, monkeypatch, mode=mode, fleet=fleet) + failures, result, out, _catalog = _run_smoke(tmp_path, runner, monkeypatch) + + assert failures + assert result["outcome"] == "failed" + assert (out / "smoke-lifecycle.json").is_file() + assert (out / "smoke-launched").is_file() + assert not any(call.argv[:2] == ["gh", "pr", "close"] for call in runner.calls) + + def test_branch_collision_stops_before_launch_marker(self, tmp_path, monkeypatch): + runner, _clone = _smoke_runner(tmp_path, monkeypatch, mode="branch-collision") + failures, result, out, _catalog = _run_smoke(tmp_path, runner, monkeypatch) + + assert failures + assert result["outcome"] == "failed" + assert (out / "smoke-lifecycle.json").exists() is False + assert (out / "smoke-launched").exists() is False + assert not any(call.argv[:3] == ["autoskillit", "fleet", "run"] for call in runner.calls) + + def test_expected_failure_matches_only_the_exact_structured_envelope( + self, tmp_path, monkeypatch + ): + expected = _SMOKE_FAILURE + runner, _clone = _smoke_runner( + tmp_path, + monkeypatch, + fleet=_completed( + [], + json.dumps( + { + "success": False, + "kind": "completed", + "dispatch_status": "failed", + "error": "workflow_failed", + "user_visible_message": "model reported failure", + } + ), + returncode=1, + ), + ) + catalog, test = _smoke_case([expected]) + out = tmp_path / "out" + failures = harness.run_test( + test, + catalog, + out=out, + home=tmp_path / "home", + env=_env(HOME=str(tmp_path / "home")), + runner=runner, + ) + result = json.loads((out / "result.json").read_text(encoding="utf-8")) + + assert failures == [] + assert result["passed"] is False + assert result["outcome"] == "expected_failure" + assert result["expected_findings"] == [expected] + + def test_recipe_matching_preserves_extra_errors_and_rejects_malformed_or_stale_rows( + self, + ) -> None: + expected = _SMOKE_FAILURE + _catalog, test = _smoke_case([expected]) + matching = { + "success": False, + "kind": "completed", + "dispatch_status": "failed", + "error": "workflow_failed", + "user_visible_message": "model reported failure", + } + failures, findings = harness.match_recipe_failure( + test, ["cleanup: delete failed"], matching + ) + assert failures == ["cleanup: delete failed"] + assert findings == [expected] + + malformed = {**matching, "user_visible_message": " "} + failures, findings = harness.match_recipe_failure(test, ["fleet run failed"], malformed) + assert failures == ["fleet run failed"] + assert findings == [] + + failures, findings = harness.match_recipe_failure(test, [], {"success": True}) + assert len(failures) == 1 and "remove stale expected failure" in failures[0] + assert findings == [] + + def test_expected_failure_console_outcome_comes_from_result_file( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] + ) -> None: + catalog, test = _smoke_case([_SMOKE_FAILURE]) + + def record_expected_failure(test_arg, _catalog, *, out, **_kwargs): + out.mkdir(parents=True, exist_ok=True) + (out / "result.json").write_text( + json.dumps( + { + "test": test_arg.name, + "passed": False, + "failures": [], + "outcome": "expected_failure", + "expected_findings": [_SMOKE_FAILURE], + } + ), + encoding="utf-8", + ) + return [] + + monkeypatch.setattr(harness.e2e_catalog, "load_catalog", lambda _path: catalog) + monkeypatch.setattr(harness, "run_test", record_expected_failure) + status = harness._run(test.name, tmp_path / "out", tmp_path / "catalog.json") + + assert status == 0 + assert capsys.readouterr().out.strip() == f"e2e_harness: {test.name} expected_failure" + + +def _make_bare_remote(tmp_path: Path, branch: str) -> tuple[Path, dict[str, str], str]: + git_home = tmp_path / "git-home" + git_home.mkdir() + env = { + "HOME": str(git_home), + "PATH": os.environ.get("PATH", "/usr/bin"), + "GIT_CONFIG_GLOBAL": str(git_home / "config"), + "GIT_CONFIG_NOSYSTEM": "1", + } + (git_home / "config").write_text("", encoding="utf-8") + remote = tmp_path / "sandbox.git" + builder = tmp_path / "builder" + builder.mkdir() + _run_git(builder, "init", "--bare", "--initial-branch=main", str(remote), env=env) + clone = builder / "clone" + _run_git(builder, "init", "--initial-branch=main", str(clone), env=env) + _run_git(clone, "config", "user.name", "Harness test", env=env) + _run_git(clone, "config", "user.email", "harness-test@example.invalid", env=env) + (clone / "README.md").write_text("base\n", encoding="utf-8") + _run_git(clone, "add", "README.md", env=env) + _run_git(clone, "commit", "-m", "base", env=env) + _run_git(clone, "remote", "add", "origin", str(remote), env=env) + _run_git(clone, "push", "-u", "origin", "main", env=env) + + _run_git(clone, "checkout", "-b", branch, env=env) + (clone / "smoke.txt").write_text("smoke\n", encoding="utf-8") + _run_git(clone, "add", "smoke.txt", env=env) + _run_git(clone, "commit", "-m", "smoke branch", env=env) + _run_git(clone, "push", "-u", "origin", branch, env=env) + + _run_git(clone, "checkout", "main", env=env) + _run_git(clone, "checkout", "-b", "feature/unrelated", env=env) + (clone / "unrelated.txt").write_text("preserve\n", encoding="utf-8") + _run_git(clone, "add", "unrelated.txt", env=env) + _run_git(clone, "commit", "-m", "unrelated branch", env=env) + _run_git(clone, "push", "-u", "origin", "feature/unrelated", env=env) + unrelated_oid = _bare_ref(remote, "feature/unrelated") + assert unrelated_oid is not None + return remote, env, unrelated_oid + + +def _bare_ref(remote: Path, branch: str) -> str | None: + result = subprocess.run( + ["git", "--git-dir", str(remote), "rev-parse", "--verify", f"refs/heads/{branch}"], + capture_output=True, + text=True, + check=False, + ) + return result.stdout.strip() if result.returncode == 0 else None + + +def _smoke_cleanup_runner( + remote: Path, + branch: str, + repository: str, + *, + fault: str | None = None, + with_pr: bool = True, +) -> tuple[FakeRunner, dict[str, object]]: + state: dict[str, object] = {"pr_open": True, "unrelated_pr_open": True} + runner: FakeRunner + + def handle(argv: list[str]): + if argv[:2] == ["gh", "auth"]: + return _completed(argv) + if argv[:2] == ["gh", "api"]: + method = argv[argv.index("--method") + 1] + endpoint = argv[-1] + requested = endpoint.rsplit("/heads/", 1)[-1] + assert endpoint.startswith(f"/repos/{repository}/git/") + if method == "GET": + oid = _bare_ref(remote, requested) + if oid is None: + return _completed(argv, "HTTP/2.0 404 Not Found\r\n\r\n", returncode=1) + body = json.dumps( + { + "ref": f"refs/heads/{requested}", + "object": {"sha": oid}, + } + ) + return _completed(argv, f"HTTP/2.0 200 OK\r\n\r\n{body}") + assert method == "DELETE" + assert requested == branch + if fault == "delete": + return _completed( + argv, "HTTP/2.0 500 Server Error\r\n\r\n", returncode=1, stderr="delete failed" + ) + result = subprocess.run( + ["git", "--git-dir", str(remote), "update-ref", "-d", f"refs/heads/{branch}"], + capture_output=True, + text=True, + check=False, + ) + return _completed( + argv, + "HTTP/2.0 204 No Content\r\n\r\n", + returncode=result.returncode, + stderr=result.stderr, + ) + if argv[:3] == ["gh", "pr", "list"]: + assert argv[argv.index("--repo") + 1] == repository + assert argv[argv.index("--head") + 1] == branch + rows = ( + [] + if not with_pr + else [ + { + "number": 8, + "state": "OPEN" if state["pr_open"] else "CLOSED", + "headRefName": branch, + "headRepository": {"name": repository.split("/", 1)[1]}, + "headRepositoryOwner": {"login": repository.split("/", 1)[0]}, + } + ] + ) + return _completed(argv, json.dumps(rows)) + if argv[:3] == ["gh", "pr", "close"]: + assert argv[3:] == ["8", "--repo", repository] + state["close_attempts"] = int(state.get("close_attempts", 0)) + 1 + if fault == "close": + return _completed(argv, returncode=1, stderr="close failed") + state["pr_open"] = False + return _completed(argv) + raise AssertionError(f"unexpected cleanup command: {argv!r}") + + runner = FakeRunner(handle) + return runner, state + + +def _write_cleanup_inputs( + out: Path, test: e2e_catalog.CatalogTest, repository: str, *, malformed_result: bool = False +) -> str: + branch = "e2e-smoke-" + "a" * 32 + out.mkdir(parents=True) + (out / "smoke-launched").write_text(test.name + "\n", encoding="utf-8") + (out / "smoke-lifecycle.json").write_text( + json.dumps( + { + "test": test.name, + "repository": repository, + "base_branch": "main", + "branch_name": branch, + } + ), + encoding="utf-8", + ) + if malformed_result: + (out / "result.json").write_text("{broken", encoding="utf-8") + else: + (out / "result.json").write_text( + json.dumps( + { + "test": test.name, + "passed": False, + "failures": [], + "outcome": "expected_failure", + "expected_findings": [_SMOKE_FAILURE], + } + ), + encoding="utf-8", + ) + return branch + + +class TestSmokeCleanup: + def test_real_bare_remote_cleanup_preserves_unrelated_branch_and_expected_failure( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + catalog, test = _smoke_case() + branch = "e2e-smoke-" + "a" * 32 + remote, git_env, unrelated_oid = _make_bare_remote(tmp_path, branch) + out = tmp_path / "out" + _write_cleanup_inputs(out, test, catalog.sandbox_repository) + runner, state = _smoke_cleanup_runner(remote, branch, catalog.sandbox_repository) + env = _env(HOME=git_env["HOME"], PATH=git_env["PATH"], E2E_SANDBOX_TOKEN=SANDBOX_TOKEN) + sleeps: list[float] = [] + monkeypatch.setattr(harness.time, "sleep", sleeps.append) + + status = harness.cleanup_test(catalog, test.name, runner, out, env) + result = json.loads((out / "result.json").read_text(encoding="utf-8")) + evidence = json.loads((out / "smoke-cleanup.json").read_text(encoding="utf-8")) + + assert status == 0 + assert result["outcome"] == "expected_failure" + assert result["expected_findings"] == [_SMOKE_FAILURE] + assert result["failures"] == [] + assert _bare_ref(remote, branch) is None + assert _bare_ref(remote, "feature/unrelated") == unrelated_oid + assert state["pr_open"] is False and state["unrelated_pr_open"] is True + assert sleeps == [1, 1] + assert evidence["final_ref_status"] == 404 + assert evidence["final_pull_requests"][0]["state"] == "CLOSED" + assert [call.input for call in runner.calls if call.input] == [SANDBOX_TOKEN] + assert all( + "E2E_SANDBOX_TOKEN" not in call.env for call in runner.calls if call.env is not None + ) + close_calls = [ + call.argv for call in runner.calls if call.argv[:3] == ["gh", "pr", "close"] + ] + assert close_calls == [["gh", "pr", "close", "8", "--repo", catalog.sandbox_repository]] + + def test_cleanup_deletes_a_pushed_branch_even_without_a_pull_request( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + catalog, test = _smoke_case() + branch = "e2e-smoke-" + "a" * 32 + remote, git_env, unrelated_oid = _make_bare_remote(tmp_path, branch) + out = tmp_path / "out" + _write_cleanup_inputs(out, test, catalog.sandbox_repository) + runner, _state = _smoke_cleanup_runner( + remote, branch, catalog.sandbox_repository, with_pr=False + ) + env = _env(HOME=git_env["HOME"], PATH=git_env["PATH"], E2E_SANDBOX_TOKEN=SANDBOX_TOKEN) + sleeps: list[float] = [] + monkeypatch.setattr(harness.time, "sleep", sleeps.append) + + assert harness.cleanup_test(catalog, test.name, runner, out, env) == 0 + + evidence = json.loads((out / "smoke-cleanup.json").read_text(encoding="utf-8")) + assert _bare_ref(remote, branch) is None + assert _bare_ref(remote, "feature/unrelated") == unrelated_oid + assert evidence["final_pull_requests"] == [] + assert evidence["final_ref_status"] == 404 + assert sleeps == [1] + assert not any(call.argv[:3] == ["gh", "pr", "close"] for call in runner.calls) + + @pytest.mark.parametrize("fault", ["close", "delete"]) + def test_cleanup_errors_fail_and_attempt_the_other_resource_operation( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, fault: str + ) -> None: + catalog, test = _smoke_case() + branch = "e2e-smoke-" + "a" * 32 + remote, git_env, _unrelated_oid = _make_bare_remote(tmp_path, branch) + out = tmp_path / "out" + _write_cleanup_inputs(out, test, catalog.sandbox_repository) + runner, state = _smoke_cleanup_runner( + remote, branch, catalog.sandbox_repository, fault=fault + ) + env = _env(HOME=git_env["HOME"], PATH=git_env["PATH"], E2E_SANDBOX_TOKEN=SANDBOX_TOKEN) + monkeypatch.setattr(harness.time, "sleep", lambda _duration: None) + + status = harness.cleanup_test(catalog, test.name, runner, out, env) + result = json.loads((out / "result.json").read_text(encoding="utf-8")) + close_called = any(call.argv[:3] == ["gh", "pr", "close"] for call in runner.calls) + delete_called = any( + call.argv[:2] == ["gh", "api"] and "DELETE" in call.argv for call in runner.calls + ) + + assert status == 1 + assert result["outcome"] == "failed" + assert result["expected_findings"] == [_SMOKE_FAILURE] + assert close_called and delete_called + assert state["pr_open"] is (fault == "close") + assert (_bare_ref(remote, branch) is not None) is (fault == "delete") + + def test_missing_marker_is_a_noop_but_bad_descriptor_and_result_are_saved( + self, tmp_path: Path + ) -> None: + catalog, test = _smoke_case() + runner = FakeRunner(lambda argv: _completed(argv)) + out = tmp_path / "not-launched" + assert harness.cleanup_test(catalog, test.name, runner, out, _env()) == 0 + assert runner.calls == [] + assert not out.exists() + + bad = tmp_path / "bad-evidence" + bad.mkdir() + (bad / "smoke-launched").write_text(test.name + "\n", encoding="utf-8") + (bad / "smoke-lifecycle.json").write_text("{broken", encoding="utf-8") + (bad / "result.json").write_text("{also broken", encoding="utf-8") + assert harness.cleanup_test(catalog, test.name, runner, bad, _env()) == 1 + assert (bad / "result-before-cleanup.json").read_text(encoding="utf-8") == "{also broken" + result = json.loads((bad / "result.json").read_text(encoding="utf-8")) + evidence = json.loads((bad / "smoke-cleanup.json").read_text(encoding="utf-8")) + assert result["outcome"] == "failed" + assert len(evidence["failures"]) == 2 + assert runner.calls == [] + + def _clean_install_catalog(expected_failures: list[dict[str, str]] | None = None): test = { "name": "clean-install", @@ -311,12 +1145,12 @@ def test_bad_reply_fails_once(self, stdout: str) -> None: class TestEnvelope: def test_success_envelope_passes(self) -> None: - assert harness.check_envelope(0, '{"success": true}\n') == [] + assert harness.check_envelope(0, '{"success": true}\n') == ([], {"success": True}) def test_envelope_is_the_last_line_after_plain_text(self) -> None: stdout = f'{_SKILL_NOTICE}\n{{"success": true}}\n\n' assert harness.last_line(stdout) == '{"success": true}' - assert harness.check_envelope(0, stdout) == [] + assert harness.check_envelope(0, stdout) == ([], {"success": True}) @pytest.mark.parametrize( ("returncode", "stdout"), @@ -330,7 +1164,12 @@ def test_envelope_is_the_last_line_after_plain_text(self) -> None: ids=["nonzero-exit", "unsuccessful", "list", "non-json-last-line", "empty"], ) def test_bad_envelope_fails(self, returncode: int, stdout: str) -> None: - assert harness.check_envelope(returncode, stdout) + failures, envelope = harness.check_envelope(returncode, stdout) + assert failures + if stdout in ('{"success": true}', '{"success": false}'): + assert envelope is not None + else: + assert envelope is None class TestPullRequestAssertions: @@ -352,7 +1191,13 @@ def test_steps_run_in_order_with_the_token_only_on_stdin(self, tmp_path: Path) - failures, result = _run_recipe(tmp_path, runner) assert failures == [] - assert result == {"test": "impl", "passed": True, "failures": []} + assert result == { + "test": "impl", + "passed": True, + "failures": [], + "outcome": "passed", + "expected_findings": [], + } assert [call.argv for call in runner.calls] == [ ["git", "config", "--global", "user.name", "AutoSkillit E2E"], [ @@ -431,7 +1276,13 @@ def test_unexpected_fleet_error_records_failure_and_still_cleans_up( runner = FakeRunner(_recipe_handler(fleet=RuntimeError("unexpected fleet error"))) failures, result = _run_recipe(tmp_path, runner) assert failures == ["harness error: unexpected fleet error"] - assert result == {"test": "impl", "passed": False, "failures": failures} + assert result == { + "test": "impl", + "passed": False, + "failures": failures, + "outcome": "failed", + "expected_findings": [], + } assert runner.calls[-1].argv[:4] == ["gh", "pr", "close", "8"] def test_missing_gh_is_a_failure_string(self, tmp_path: Path) -> None: @@ -471,7 +1322,13 @@ def malformed_listing(argv: list[str]): runner = FakeRunner(_recipe_handler(extra=malformed_listing)) failures, result = _run_recipe(tmp_path, runner) assert "gh pr list: pull request number is missing or invalid" in failures - assert result == {"test": "impl", "passed": False, "failures": failures} + assert result == { + "test": "impl", + "passed": False, + "failures": failures, + "outcome": "failed", + "expected_findings": [], + } assert ("gh", "pr", "close") not in runner.argv_prefixes() def test_missing_sandbox_token_fails_before_any_launch(self, tmp_path: Path) -> None: @@ -819,11 +1676,18 @@ def test_secrets_are_replaced_in_nested_text_and_binary_files(self, tmp_path: Pa (source / "result.json").write_text(f"key={MINIMAX_KEY}\n", encoding="utf-8") (source / "nested" / "blob.bin").write_bytes(b"\x00" + SANDBOX_TOKEN.encode() + b"\xff") (source / "nested" / "blob.bin").chmod(0o600) + (source / "smoke-lifecycle.json").write_text( + f'{{"credential":"{MINIMAX_KEY}","token":"{SANDBOX_TOKEN}"}}\n', + encoding="utf-8", + ) dest = tmp_path / "upload" harness.redact_tree([source, tmp_path / "missing"], dest, [MINIMAX_KEY, SANDBOX_TOKEN, ""]) assert (dest / "out" / "result.json").read_text(encoding="utf-8") == "key=[REDACTED]\n" + assert (dest / "out" / "smoke-lifecycle.json").read_text(encoding="utf-8") == ( + '{"credential":"[REDACTED]","token":"[REDACTED]"}\n' + ) assert (dest / "out" / "nested" / "blob.bin").read_bytes() == b"\x00[REDACTED]\xff" assert stat.S_IMODE((dest / "out" / "nested" / "blob.bin").stat().st_mode) == 0o644 assert stat.S_IMODE((dest / "out" / "nested").stat().st_mode) == 0o755 diff --git a/tests/infra/test_e2e_select.py b/tests/infra/test_e2e_select.py index 72ef31376d..7295c85da3 100644 --- a/tests/infra/test_e2e_select.py +++ b/tests/infra/test_e2e_select.py @@ -65,6 +65,16 @@ def _entry(name: str, trigger_paths: tuple[str, ...] = ("src/*",), **overrides) "scripts/e2e/*", ".github/workflows/e2e.yml", ) +_HEADLESS_SMOKE_TRIGGER_PATHS = ( + "src/autoskillit/execution/headless/session.py", + "src/autoskillit/execution/backends/claude.py", + "src/autoskillit/cli/fleet/run.py", + "src/autoskillit/fleet/dispatch/worker.py", + "src/autoskillit/server/recipe.py", + "src/autoskillit/hooks/runner.py", + "scripts/e2e/e2e_harness.py", + ".github/workflows/e2e.yml", +) def _clean_install_entry(**overrides) -> dict: @@ -318,17 +328,27 @@ def _recipe_entry_without(field: str) -> dict: "recipe-unknown-state": _raw_catalog( _entry("a", kind="recipe", **{**_RECIPE_FIELDS, "expected_pull_request_state": "draft"}) ), + "recipe-fixture-path": _raw_catalog( + _entry("a", kind="recipe", recipe_fixture="../sandbox-smoke.yaml", **_RECIPE_FIELDS) + ), + "recipe-fixture-extension": _raw_catalog( + _entry("a", kind="recipe", recipe_fixture="sandbox-smoke.yml", **_RECIPE_FIELDS) + ), "canary-with-recipe-fields": _raw_catalog(_entry("a", **_RECIPE_FIELDS)), + "canary-with-recipe-fixture": _raw_catalog(_entry("a", recipe_fixture="sandbox-smoke.yaml")), "clean-install-with-recipe-fields": _raw_catalog(_clean_install_entry(**_RECIPE_FIELDS)), + "clean-install-with-recipe-fixture": _raw_catalog( + _clean_install_entry(recipe_fixture="sandbox-smoke.yaml") + ), "clean-install-unknown-key": _raw_catalog(_clean_install_entry(extra=1)), "expected-failures-on-canary": _raw_catalog( _entry("a", expected_failures=[_EXPECTED_FAILURE]) ), - "expected-failures-on-recipe": _raw_catalog( + "expected-failures-warning-on-recipe": _raw_catalog( _entry( "impl", kind="recipe", - expected_failures=[_EXPECTED_FAILURE], + expected_failures=[{**_EXPECTED_FAILURE, "severity": "warning"}], **_RECIPE_FIELDS, ) ), @@ -399,6 +419,26 @@ def test_shipped_catalog_holds_the_canary(self) -> None: assert clean_install.trigger_paths == _CLEAN_INSTALL_TRIGGER_PATHS assert clean_install.recipe is None + smoke = catalog.get("headless-smoke") + assert smoke.kind == "recipe" + assert smoke.peak_sessions == 2 + assert smoke.timeout_sec == 900 + assert smoke.recipe == "sandbox-smoke" + assert smoke.recipe_fixture == "sandbox-smoke.yaml" + assert smoke.ingredients == () + assert smoke.expected_pull_request_state == "closed" + assert smoke.expected_failures == () + assert smoke.trigger_paths == ( + "src/autoskillit/execution/headless/*", + "src/autoskillit/execution/backends/*", + "src/autoskillit/cli/fleet/*", + "src/autoskillit/fleet/dispatch/*", + "src/autoskillit/server/*", + "src/autoskillit/hooks/*", + "scripts/e2e/*", + ".github/workflows/e2e.yml", + ) + def test_clean_install_expected_failures_are_immutable_mapping_rows(self) -> None: rows = [{**_EXPECTED_FAILURE}, {**_EXPECTED_FAILURE, "severity": "warning"}] test = _catalog(_clean_install_entry(expected_failures=rows)).get("clean-install") @@ -421,11 +461,57 @@ def test_clean_install_expected_failures_allow_distinct_identities(self) -> None assert len(test.expected_failures) == 3 def test_recipe_entry_parses(self) -> None: - catalog = _catalog(_entry("impl", kind="recipe", **_RECIPE_FIELDS)) + catalog = _catalog( + _entry( + "impl", + kind="recipe", + recipe_fixture="sandbox-smoke.yaml", + **_RECIPE_FIELDS, + ) + ) test = catalog.get("impl") assert test.recipe == "implementation" assert test.ingredients == (("task", "Add a greeting"),) assert test.expected_pull_request_state == "merged" + assert test.recipe_fixture == "sandbox-smoke.yaml" + + def test_recipe_without_fixture_defaults_to_none(self) -> None: + test = _catalog(_entry("impl", kind="recipe", **_RECIPE_FIELDS)).get("impl") + assert test.recipe_fixture is None + + def test_recipe_expected_failure_keeps_exact_error_diagnostic(self) -> None: + row = { + **_EXPECTED_FAILURE, + "check": " exact-check ", + "message": " exact message ", + } + test = _catalog( + _entry( + "impl", + kind="recipe", + expected_failures=[row], + **_RECIPE_FIELDS, + ) + ).get("impl") + assert dict(test.expected_failures[0]) == row + + @pytest.mark.parametrize("changed_path", _HEADLESS_SMOKE_TRIGGER_PATHS) + def test_headless_smoke_trigger_paths_match(self, changed_path: str) -> None: + catalog = e2e_catalog.load_catalog() + smoke = catalog.get("headless-smoke") + assert select._touches(smoke, (changed_path,)) + + def test_headless_smoke_path_selects_shipped_recipe(self) -> None: + catalog = e2e_catalog.load_catalog() + assert select.pick_tests( + catalog, + ["src/autoskillit/execution/headless/session.py"], + f"5230:{SHA_ZERO}", + ) == ("headless-smoke",) + + def test_dispatch_selects_headless_smoke(self) -> None: + catalog = e2e_catalog.load_catalog() + assert select.select_for_dispatch("headless-smoke", catalog).tests == ("headless-smoke",) def test_longest_timeout_within_the_job_limit_parses(self) -> None: catalog = _catalog(_entry("slow", timeout_sec=_MAX_TIMEOUT_SEC)) diff --git a/tests/infra/test_e2e_select_cli.py b/tests/infra/test_e2e_select_cli.py index 0049a64170..25a7f2c1e2 100644 --- a/tests/infra/test_e2e_select_cli.py +++ b/tests/infra/test_e2e_select_cli.py @@ -17,9 +17,12 @@ _SCRIPT = REPO_ROOT / "scripts" / "e2e" / "e2e_select.py" REPOSITORY = "TalonT-Org/AutoSkillit" _CANARY_MATRIX = '{"include":[{"test":"canary","kind":"canary","timeout_minutes":37}]}' -_BOTH_MATRIX = ( +_SHARED_HARNESS_MATRIX = ( '{"include":[{"test":"canary","kind":"canary","timeout_minutes":37},' - '{"test":"clean-install","kind":"clean-install","timeout_minutes":37}]}' + '{"test":"headless-smoke","kind":"recipe","timeout_minutes":47}]}' +) +_HEADLESS_SMOKE_MATRIX = ( + '{"include":[{"test":"headless-smoke","kind":"recipe","timeout_minutes":47}]}' ) @@ -69,7 +72,7 @@ def _select(tmp_path: Path, changed_paths: Path, event_name: str, payload: objec ) -def test_shared_harness_pull_request_selects_both_shipped_tests_deterministically( +def test_shared_harness_pull_request_selection_is_deterministic( tmp_path: Path, ) -> None: changed = tmp_path / "changed-paths.txt" @@ -79,13 +82,24 @@ def test_shared_harness_pull_request_selects_both_shipped_tests_deterministicall assert result.returncode == 0, result.stderr outputs = _outputs(result.stdout) assert outputs["selected"] == "true" - assert outputs["matrix"] == _BOTH_MATRIX + assert outputs["matrix"] == _SHARED_HARNESS_MATRIX repeated = _select(tmp_path, changed, "pull_request", payload) assert repeated.returncode == 0, repeated.stderr assert _outputs(repeated.stdout)["matrix"] == outputs["matrix"] +def test_headless_source_pull_request_selects_smoke_recipe(tmp_path: Path) -> None: + changed = tmp_path / "changed-paths.txt" + changed.write_text("src/autoskillit/execution/headless/session.py\n", encoding="utf-8") + payload = _pull_request(changed_files=1, additions=5, labels=("e2e",)) + result = _select(tmp_path, changed, "pull_request", payload) + assert result.returncode == 0, result.stderr + outputs = _outputs(result.stdout) + assert outputs["selected"] == "true" + assert outputs["matrix"] == _HEADLESS_SMOKE_MATRIX + + def test_small_unlabelled_pull_request_selects_nothing(tmp_path: Path) -> None: changed = tmp_path / "changed-paths.txt" changed.write_text("README.md\n", encoding="utf-8") @@ -115,6 +129,15 @@ def test_dispatch_selects_the_named_test(tmp_path: Path) -> None: assert outputs["matrix"] == _CANARY_MATRIX +def test_dispatch_selects_headless_smoke(tmp_path: Path) -> None: + payload = {"inputs": {"tests": "headless-smoke"}} + result = _select(tmp_path, tmp_path / "missing.txt", "workflow_dispatch", payload) + assert result.returncode == 0, result.stderr + outputs = _outputs(result.stdout) + assert outputs["selected"] == "true" + assert outputs["matrix"] == _HEADLESS_SMOKE_MATRIX + + def test_dispatch_selects_both_named_tests_in_request_order(tmp_path: Path) -> None: payload = {"inputs": {"tests": "clean-install, canary"}} result = _select(tmp_path, tmp_path / "missing.txt", "workflow_dispatch", payload) From 602b4753f2bb1e213780766bbe962f6ee849aa5c Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 5 Oct 2026 09:14:22 -0700 Subject: [PATCH 02/25] ci(e2e): stop the smoke container before exact remote cleanup --- .github/workflows/e2e.yml | 85 ++++++++++- scripts/e2e/README.md | 144 +++++++++++++++++-- tests/infra/test_e2e_workflow.py | 233 ++++++++++++++++++++++++++++++- 3 files changed, 445 insertions(+), 17 deletions(-) diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 6ee1930e9c..a492fcb441 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -106,16 +106,91 @@ jobs: E2E_SANDBOX_TOKEN: ${{ matrix.kind != 'clean-install' && secrets.E2E_SANDBOX_TOKEN || '' }} TEST: ${{ matrix.test }} run: | + set -euo pipefail docker_args=() if [[ '${{ matrix.kind }}' != 'clean-install' ]]; then docker_args+=(--env MINIMAX_API_KEY --env E2E_SANDBOX_TOKEN) docker_args+=(--volume "$RUNNER_TEMP/e2e/data:/home/autoskillit/.local/share/autoskillit") fi - docker run --rm "${docker_args[@]}" \ - --volume "$GITHUB_WORKSPACE/scripts/e2e:/opt/e2e:ro" \ - --volume "$RUNNER_TEMP/e2e/out:/artifacts" \ - "$IMAGE" \ - python3 /opt/e2e/e2e_harness.py run --test "$TEST" --out /artifacts + if [[ "$TEST" == 'headless-smoke' ]]; then + cid_file="$RUNNER_TEMP/e2e/out/${TEST}-${GITHUB_RUN_ID}-${GITHUB_RUN_ATTEMPT}.cid" + if [[ -e "$cid_file" ]]; then + echo "refusing to reuse existing container ID file: $cid_file" >&2 + exit 1 + fi + + cleanup_smoke() { + model_status=$? + set +e + removal_status=0 + cleanup_status=0 + rm_status=1 + verify_status=1 + lifecycle_file="$RUNNER_TEMP/e2e/out/${TEST}-lifecycle.txt" + + if [[ ! -s "$cid_file" ]]; then + echo "missing container ID evidence: $cid_file" >&2 + removal_status=1 + else + container_id="$(<"$cid_file")" + if [[ ! "$container_id" =~ ^[0-9a-f]{64}$ ]]; then + echo "invalid container ID evidence retained: $cid_file" >&2 + removal_status=1 + else + timeout --signal=TERM --kill-after=10s 60s \ + docker rm -f "$container_id" + rm_status=$? + remaining="$(timeout --signal=TERM --kill-after=10s 60s \ + docker container ls --all --quiet --no-trunc \ + --filter "id=$container_id")" + verify_status=$? + if (( verify_status != 0 )) || [[ -n "$remaining" ]]; then + echo "container removal could not be verified; retaining $cid_file" >&2 + removal_status=1 + elif ! rm -f -- "$cid_file"; then + echo "could not remove verified container ID file: $cid_file" >&2 + removal_status=1 + elif (( rm_status != 0 )); then + echo "container was already absent; exact-ID absence verified" + fi + fi + fi + + timeout --signal=TERM --kill-after=20s 300s docker run --rm \ + --env E2E_SANDBOX_TOKEN \ + --volume "$GITHUB_WORKSPACE/scripts/e2e:/opt/e2e:ro" \ + --volume "$RUNNER_TEMP/e2e/out:/artifacts" \ + "$IMAGE" \ + python3 /opt/e2e/e2e_harness.py cleanup \ + --test "$TEST" --catalog /opt/e2e/catalog.json --out /artifacts + cleanup_status=$? + + printf 'model_exit=%s\ndocker_rm=%s\ncontainer_verify=%s\ncontainer_removal=%s\nremote_cleanup=%s\n' \ + "$model_status" "$rm_status" "$verify_status" "$removal_status" "$cleanup_status" \ + > "$lifecycle_file" + trap - EXIT + if (( model_status != 0 || removal_status != 0 || cleanup_status != 0 )); then + exit 1 + fi + exit 0 + } + trap cleanup_smoke EXIT + + # The harness budget is 900 seconds plus 120 seconds of grace. The job + # timeout reserves the catalog's remaining overhead for teardown and upload. + timeout --signal=TERM --kill-after=30s 1020s docker run --rm \ + --cidfile "$cid_file" "${docker_args[@]}" \ + --volume "$GITHUB_WORKSPACE/scripts/e2e:/opt/e2e:ro" \ + --volume "$RUNNER_TEMP/e2e/out:/artifacts" \ + "$IMAGE" \ + python3 /opt/e2e/e2e_harness.py run --test "$TEST" --out /artifacts + else + docker run --rm "${docker_args[@]}" \ + --volume "$GITHUB_WORKSPACE/scripts/e2e:/opt/e2e:ro" \ + --volume "$RUNNER_TEMP/e2e/out:/artifacts" \ + "$IMAGE" \ + python3 /opt/e2e/e2e_harness.py run --test "$TEST" --out /artifacts + fi - name: Redact secrets from the artifacts id: redact diff --git a/scripts/e2e/README.md b/scripts/e2e/README.md index 4f2a458031..e95d3d5684 100644 --- a/scripts/e2e/README.md +++ b/scripts/e2e/README.md @@ -10,7 +10,7 @@ This directory holds everything the workflow runs: | `catalog.json` | Registered E2E tests (the only source of test names, budgets and trigger paths) | | `e2e_catalog.py` | Stdlib loader and validator for the catalog | | `e2e_select.py` | Picks the tests an event runs (`select`) and judges the required check (`gate`) | -| `e2e_harness.py` | Runs one test inside the image (`run`) and redacts its artifacts (`redact`) | +| `e2e_harness.py` | Runs one test (`run`), performs exact sandbox teardown (`cleanup`) and redacts artifacts (`redact`) | | `autoskillit-config.yaml` | AutoSkillit user-layer config the harness installs | | `post-e2e-failure.sh` | Appends a failure to the test's tracking issue | @@ -35,7 +35,9 @@ model tests: | Actions: read | `wait_for_ci` | | Commit statuses: read | `wait_for_ci` | - The canary needs only `MINIMAX_API_KEY`; no Anthropic credential is used anywhere. + The canary needs only `MINIMAX_API_KEY`; no Anthropic API key or Claude OAuth credential + is supplied. The harness writes MiniMax's key into the Claude-compatible settings file + because MiniMax serves the Anthropic Messages API. 3. Create the `e2e` label in this repository. 4. In branch protection, mark only `e2e-gate` as required — never `select` or `run (…)`. @@ -216,12 +218,40 @@ requests. The test container never sees the workflow token. ## Recipe test assertions -A recipe test passes when `fleet run` exits 0, its envelope — the last non-empty stdout +Regular recipe tests pass when `fleet run` exits 0, its envelope — the last non-empty stdout line, since earlier lines can be plain-text notices — reports `"success": true`, and exactly one sandbox pull request numbered above the pre-run baseline exists in the catalog's `expected_pull_request_state` with a non-empty diff. `gh` reports states in upper case; the -harness lower-cases them. Afterwards the harness closes, with `--delete-branch`, every new -pull request still open — but only once it captured the baseline. +harness lower-cases them. Its existing cleanup closes newly opened PRs with their branches. + +`headless-smoke` exercises one orchestrator and one recipe worker (`peak_sessions: 2`). The +harness stages `sandbox-smoke.yaml` from the read-only `/opt/e2e` mount into the disposable +sandbox clone's `.autoskillit/recipes/` directory before invoking fleet by recipe name. This +lets the clone's project name resolve the staged recipe; the host scripts remain read-only. +The worker changes only `sandbox/text.py` and `tests/test_smoke_canary.py`, pushes its assigned +per-run branch, opens a PR and closes it. Before cleanup, the harness verifies the successful +envelope, that exactly one PR for the assigned branch is closed, the complete PR diff contains +only those prescribed paths, and the PR head, remote ref and fetched commit agree. The source +and test must contain the requested sentinel and an executable assertion. It saves the PR, +commit, diff and branch evidence for redaction and upload. + +The harness writes a lifecycle descriptor before fleet starts, so cleanup does not depend on +the model process or a fleet result. The workflow wraps this test's model container in a +per-run Docker CID file. Its exit trap captures the model result, removes the recorded +container by its exact ID, verifies that ID is absent, then launches a fresh user-image +container for `e2e_harness.py cleanup`. That command uses only `E2E_SANDBOX_TOKEN`, closes any +remaining PR for the assigned branch, deletes that exact remote branch and verifies both are +gone. Cleanup runs after model success, failure or timeout, and its result is part of the step +status. The normal artifact redaction step then runs even when the test step failed. Recipes +without this fixture keep their existing in-container cleanup path. + +Recipe `expected_failures` rows identify an individual, verified product defect by exact +`severity: error`, `check` equal to the failed envelope's `error`, `message` equal to its +`user_visible_message`, and an open bug issue URL. Only that exact failed envelope can be +accepted; process timeouts, malformed results, other failures and cleanup failures remain +fatal. An accepted defect is provisional until mandatory branch/PR cleanup succeeds. A clean +run that observes a configured row makes it stale and fails with instructions to remove the +row and close the bug. The smoke catalog starts without expected defects. ## Catalog schema @@ -251,10 +281,12 @@ pull request still open — but only once it captured the baseline. ``` `recipe`, `ingredients` and `expected_pull_request_state` are required for `recipe` tests -and forbidden for `canary` and `clean-install` tests. `expected_failures` is optional only -for `clean-install`: each row contains nonempty `severity` (`error` or `warning`), `check`, -`message`, and `issue` (`https://github.com/TalonT-Org/AutoSkillit/issues/`). -Duplicate diagnostic identities and unknown keys are rejected. `peak_sessions` must be +and forbidden for `canary` and `clean-install` tests. `expected_failures` is optional for +`clean-install` and recipe tests. Clean-install rows use the exact doctor `severity`, `check` +and `message`; recipe rows require `severity: error`, `check` and `message` matching the failed +envelope. Every row includes an open bug issue URL +(`https://github.com/TalonT-Org/AutoSkillit/issues/`). Duplicate diagnostic identities +and unknown keys are rejected. `peak_sessions` must be 1–8 for model tests and exactly zero for clean-install. `timeout_sec` must keep the derived job timeout within 360 minutes. @@ -292,3 +324,97 @@ docker run --rm \ autoskillit-e2e:local \ python3 /opt/e2e/e2e_harness.py run --test clean-install --out /artifacts ``` + +### Sandbox smoke with teardown + +Set `MINIMAX_API_KEY` and `E2E_SANDBOX_TOKEN` in the local environment first. Build the same +user image and run this wrapper from the repository root. It creates the `out`, `data` and +`upload` directories used by CI. The model and cleanup containers see E2E scripts read-only; +the cleanup container receives the sandbox token without the MiniMax key. The EXIT trap +keeps the model's status, removes and verifies the exact container, runs remote cleanup, then +runs redaction even if an earlier stage failed. It retains the CID file if container absence +cannot be verified. + +```bash +git archive HEAD | docker build --file scripts/docker/Dockerfile --target user \ + --tag autoskillit-e2e:local - + +set -euo pipefail +IMAGE=autoskillit-e2e:local +TEST=headless-smoke +E2E_DIR="$PWD/.autoskillit/temp/e2e" +mkdir -p "$E2E_DIR"/{out,data,upload} +chmod 0777 "$E2E_DIR"/{out,data,upload} +RUN_ID="$(date +%s)-$$" +CID_FILE="$E2E_DIR/out/${TEST}-${RUN_ID}.cid" +[[ ! -e "$CID_FILE" ]] + +cleanup_smoke() { + model_status=$? + set +e + rm_status=1 + verify_status=1 + removal_status=0 + cleanup_status=0 + redact_status=0 + lifecycle_file="$E2E_DIR/out/${TEST}-lifecycle.txt" + + if [[ ! -s "$CID_FILE" ]]; then + echo "missing container ID evidence: $CID_FILE" >&2 + removal_status=1 + else + container_id="$(<"$CID_FILE")" + if [[ ! "$container_id" =~ ^[0-9a-f]{64}$ ]]; then + echo "invalid container ID evidence retained: $CID_FILE" >&2 + removal_status=1 + else + timeout --signal=TERM --kill-after=10s 60s docker rm -f "$container_id" + rm_status=$? + remaining="$(timeout --signal=TERM --kill-after=10s 60s \ + docker container ls --all --quiet --no-trunc --filter "id=$container_id")" + verify_status=$? + if (( verify_status != 0 )) || [[ -n "$remaining" ]]; then + echo "container removal could not be verified; retaining $CID_FILE" >&2 + removal_status=1 + elif ! rm -f -- "$CID_FILE"; then + removal_status=1 + fi + fi + fi + + timeout --signal=TERM --kill-after=20s 300s docker run --rm \ + --env E2E_SANDBOX_TOKEN \ + --volume "$PWD/scripts/e2e:/opt/e2e:ro" \ + --volume "$E2E_DIR/out:/artifacts" \ + "$IMAGE" python3 /opt/e2e/e2e_harness.py cleanup \ + --test "$TEST" --catalog /opt/e2e/catalog.json --out /artifacts + cleanup_status=$? + + docker run --rm --env MINIMAX_API_KEY --env E2E_SANDBOX_TOKEN \ + --volume "$PWD/scripts/e2e:/opt/e2e:ro" \ + --volume "$E2E_DIR:/e2e" \ + "$IMAGE" python3 /opt/e2e/e2e_harness.py redact --dest /e2e/upload \ + --secret-env MINIMAX_API_KEY --secret-env E2E_SANDBOX_TOKEN \ + /e2e/out /e2e/data/logs + redact_status=$? + + printf 'model_exit=%s\ndocker_rm=%s\ncontainer_verify=%s\ncontainer_removal=%s\nremote_cleanup=%s\nredaction=%s\n' \ + "$model_status" "$rm_status" "$verify_status" "$removal_status" \ + "$cleanup_status" "$redact_status" > "$lifecycle_file" + trap - EXIT + if (( model_status != 0 || removal_status != 0 || cleanup_status != 0 || redact_status != 0 )); then + exit 1 + fi + exit 0 +} +trap cleanup_smoke EXIT + +timeout --signal=TERM --kill-after=30s 1020s docker run --rm --cidfile "$CID_FILE" \ + --env MINIMAX_API_KEY --env E2E_SANDBOX_TOKEN \ + --volume "$PWD/scripts/e2e:/opt/e2e:ro" \ + --volume "$E2E_DIR/out:/artifacts" \ + --volume "$E2E_DIR/data:/home/autoskillit/.local/share/autoskillit" \ + "$IMAGE" python3 /opt/e2e/e2e_harness.py run --test "$TEST" --out /artifacts +``` + +Select it directly with workflow dispatch input `tests=headless-smoke`. diff --git a/tests/infra/test_e2e_workflow.py b/tests/infra/test_e2e_workflow.py index e719cfd8a9..05bfed1736 100644 --- a/tests/infra/test_e2e_workflow.py +++ b/tests/infra/test_e2e_workflow.py @@ -2,14 +2,17 @@ from __future__ import annotations +import json +import os import re +import subprocess from pathlib import Path import pytest from autoskillit.core.io import load_yaml -pytestmark = [pytest.mark.layer("infra"), pytest.mark.small] +pytestmark = [pytest.mark.layer("infra"), pytest.mark.medium] REPO_ROOT = Path(__file__).resolve().parents[2] _WORKFLOWS = REPO_ROOT / ".github" / "workflows" @@ -58,6 +61,223 @@ def _model_branch(run: str) -> tuple[str, str]: return "\n".join(lines[start + 1 : end]), "\n".join(lines[:start] + lines[end + 1 :]) +_DOCKER_STUB = r"""#!/usr/bin/env python3 +import json +import os +from pathlib import Path +import sys + +args = sys.argv[1:] +log = Path(os.environ["DOCKER_LOG"]) +state = Path(os.environ["DOCKER_STATE"]) +container_id = os.environ["CONTAINER_ID"] + +def record(operation): + with log.open("a", encoding="utf-8") as stream: + stream.write(json.dumps({"operation": operation, "args": args}) + "\n") + +if args[0] == "run": + operation = "cleanup" if "cleanup" in args else "model" + record(operation) + if operation == "model": + cidfile = Path(args[args.index("--cidfile") + 1]) + with cidfile.open("x", encoding="utf-8") as stream: + stream.write(container_id + "\n") + state.write_text(os.environ["MODEL_CONTAINER_STATE"], encoding="utf-8") + raise SystemExit(int(os.environ["MODEL_EXIT"])) + raise SystemExit(int(os.environ["CLEANUP_EXIT"])) + +if args[0] == "rm": + record("rm") + status = int(os.environ["RM_EXIT"]) + if status == 0: + state.write_text("absent", encoding="utf-8") + raise SystemExit(status) + +if args[:2] == ["container", "ls"]: + record("list") + status = int(os.environ["LIST_EXIT"]) + if status == 0 and state.read_text(encoding="utf-8") == "present": + print(container_id) + raise SystemExit(status) + +record("unexpected") +raise SystemExit(99) +""" + + +def _run_smoke_workflow_shell(workflow: dict, tmp_path: Path, **scenario: str): + run = _step(workflow, "run", step_id="e2e")["run"].replace("${{ matrix.kind }}", "recipe") + runner_temp = tmp_path / "runner" + (runner_temp / "e2e" / "out").mkdir(parents=True) + (runner_temp / "e2e" / "data").mkdir() + workspace = tmp_path / "workspace" + (workspace / "scripts" / "e2e").mkdir(parents=True) + docker_bin = tmp_path / "bin" + docker_bin.mkdir() + docker = docker_bin / "docker" + docker.write_text(_DOCKER_STUB, encoding="utf-8") + docker.chmod(0o755) + + log = tmp_path / "docker.jsonl" + state = tmp_path / "container-state" + state.write_text("absent", encoding="utf-8") + env = os.environ.copy() + env.update( + { + "PATH": f"{docker_bin}{os.pathsep}{env['PATH']}", + "RUNNER_TEMP": str(runner_temp), + "GITHUB_WORKSPACE": str(workspace), + "GITHUB_RUN_ID": "1234", + "GITHUB_RUN_ATTEMPT": "2", + "TEST": "headless-smoke", + "IMAGE": "autoskillit-e2e:test", + "MINIMAX_API_KEY": "model-secret", + "E2E_SANDBOX_TOKEN": "sandbox-secret", + "DOCKER_LOG": str(log), + "DOCKER_STATE": str(state), + "CONTAINER_ID": "a" * 64, + "MODEL_EXIT": "0", + "MODEL_CONTAINER_STATE": "present", + "RM_EXIT": "0", + "LIST_EXIT": "0", + "CLEANUP_EXIT": "0", + **scenario, + } + ) + result = subprocess.run( + ["bash", "--noprofile", "--norc", "-c", run], + cwd=tmp_path, + env=env, + check=False, + capture_output=True, + text=True, + timeout=15, + ) + events = [json.loads(line) for line in log.read_text(encoding="utf-8").splitlines()] + cidfile = runner_temp / "e2e" / "out" / "headless-smoke-1234-2.cid" + lifecycle = runner_temp / "e2e" / "out" / "headless-smoke-lifecycle.txt" + statuses = dict( + line.split("=", maxsplit=1) for line in lifecycle.read_text(encoding="utf-8").splitlines() + ) + return result, events, cidfile, statuses + + +@pytest.mark.parametrize( + ( + "scenario", + "expected_status", + "expected_events", + "expected_cidfile", + "expected_lifecycle", + ), + [ + pytest.param( + {"MODEL_EXIT": "19", "MODEL_CONTAINER_STATE": "absent", "RM_EXIT": "1"}, + 1, + ["model", "rm", "list", "cleanup"], + False, + { + "model_exit": "19", + "docker_rm": "1", + "container_verify": "0", + "container_removal": "0", + "remote_cleanup": "0", + }, + id="model-failure-still-cleans", + ), + pytest.param( + {"RM_EXIT": "23", "MODEL_CONTAINER_STATE": "present"}, + 1, + ["model", "rm", "list", "cleanup"], + True, + { + "model_exit": "0", + "docker_rm": "23", + "container_verify": "0", + "container_removal": "1", + "remote_cleanup": "0", + }, + id="removal-failure-still-cleans", + ), + pytest.param( + {"CLEANUP_EXIT": "17"}, + 1, + ["model", "rm", "list", "cleanup"], + False, + { + "model_exit": "0", + "docker_rm": "0", + "container_verify": "0", + "container_removal": "0", + "remote_cleanup": "17", + }, + id="cleanup-failure", + ), + pytest.param( + {"MODEL_CONTAINER_STATE": "absent", "RM_EXIT": "1"}, + 0, + ["model", "rm", "list", "cleanup"], + False, + { + "model_exit": "0", + "docker_rm": "1", + "container_verify": "0", + "container_removal": "0", + "remote_cleanup": "0", + }, + id="already-absent-container", + ), + ], +) +def test_headless_smoke_exit_trap_removes_exact_container_then_runs_cleanup( + workflow: dict, + tmp_path: Path, + scenario: dict[str, str], + expected_status: int, + expected_events: list[str], + expected_cidfile: bool, + expected_lifecycle: dict[str, str], +) -> None: + result, events, cidfile, statuses = _run_smoke_workflow_shell(workflow, tmp_path, **scenario) + + assert result.returncode == expected_status, result.stderr + assert [event["operation"] for event in events] == expected_events + container_id = "a" * 64 + model = events[0]["args"] + model_cidfile = Path(model[model.index("--cidfile") + 1]) + assert model_cidfile == cidfile + model_mounts = [model[index + 1] for index, value in enumerate(model) if value == "--volume"] + assert any(mount.endswith("/scripts/e2e:/opt/e2e:ro") for mount in model_mounts) + assert any(mount.endswith(":/artifacts") for mount in model_mounts) + assert events[1]["args"] == ["rm", "-f", container_id] + assert events[2]["args"][2:5] == ["--all", "--quiet", "--no-trunc"] + assert events[2]["args"][-2:] == ["--filter", f"id={container_id}"] + cleanup = events[-1]["args"] + assert "--env" in cleanup + assert cleanup[cleanup.index("--env") + 1] == "E2E_SANDBOX_TOKEN" + assert "MINIMAX_API_KEY" not in cleanup + cleanup_mounts = [ + cleanup[index + 1] for index, value in enumerate(cleanup) if value == "--volume" + ] + assert any(mount.endswith("/scripts/e2e:/opt/e2e:ro") for mount in cleanup_mounts) + assert any(mount.endswith(":/artifacts") for mount in cleanup_mounts) + assert cleanup[-7:] == [ + "cleanup", + "--test", + "headless-smoke", + "--catalog", + "/opt/e2e/catalog.json", + "--out", + "/artifacts", + ] + assert cidfile.exists() is expected_cidfile + if expected_cidfile: + assert cidfile.read_text(encoding="utf-8") == container_id + "\n" + for key, value in expected_lifecycle.items(): + assert statuses[key] == value + + class TestTriggers: def test_pull_request_types_include_labeled(self, workflow: dict) -> None: types = _on(workflow)["pull_request"]["types"] @@ -108,7 +328,10 @@ def test_report_step_uses_only_the_workflow_token(self, workflow: dict) -> None: def test_docker_run_forwards_secrets_by_name_only(self, workflow: dict, step_id: str) -> None: run = _step(workflow, "run", step_id=step_id)["run"] forwarded = re.findall(r"--env ([A-Z0-9_]+)", run) - assert forwarded == ["MINIMAX_API_KEY", "E2E_SANDBOX_TOKEN"] + expected = ["MINIMAX_API_KEY", "E2E_SANDBOX_TOKEN"] + if step_id == "e2e": + expected.append("E2E_SANDBOX_TOKEN") + assert forwarded == expected def test_redaction_keeps_required_secret_names(self, workflow: dict) -> None: run = _step(workflow, "run", step_id="redact")["run"] @@ -162,7 +385,11 @@ def test_clean_install_uses_the_common_user_image_and_artifacts( "MINIMAX_API_KEY", "E2E_SANDBOX_TOKEN", ] - assert not re.search(r"--env (?:MINIMAX_API_KEY|E2E_SANDBOX_TOKEN)\b", outside) + if step_id == "e2e": + assert not re.search(r"--env MINIMAX_API_KEY\b", outside) + assert re.findall(r"--env ([A-Z0-9_]+)", outside) == ["E2E_SANDBOX_TOKEN"] + else: + assert not re.search(r"--env (?:MINIMAX_API_KEY|E2E_SANDBOX_TOKEN)\b", outside) data_mounts = re.findall(r'--volume "\$RUNNER_TEMP/e2e/data:[^"]+"', run) if step_id == "e2e": From 509a5208bd81c34c505fda178a2c23542bf96093 Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 5 Oct 2026 09:15:32 -0700 Subject: [PATCH 03/25] test(e2e): include smoke validation in the recipe test cascade --- tests/_test_filter.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/_test_filter.py b/tests/_test_filter.py index 32bbe61997..0fcd757d07 100644 --- a/tests/_test_filter.py +++ b/tests/_test_filter.py @@ -1394,6 +1394,7 @@ class SourceMapRejection(enum.StrEnum): "fleet/test_gate_state_persistence.py", "fleet/test_dispatch_outcome_codex_backend.py", # Other file-level entries: + "infra/test_e2e_harness.py", "infra/test_pretty_output_recipe.py", "infra/test_generated_files.py", "skills/test_graphql_invocation_completeness.py", From 6e826284dc4b5cbb98a1632f31f649e89498bee4 Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 5 Oct 2026 09:53:51 -0700 Subject: [PATCH 04/25] fix(e2e): resolve smoke PR base through fleet config authority --- scripts/e2e/README.md | 11 +++-- scripts/e2e/recipes/sandbox-smoke.yaml | 3 +- tests/_test_filter.py | 8 ++-- tests/infra/test_e2e_harness.py | 65 ++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 8 deletions(-) diff --git a/scripts/e2e/README.md b/scripts/e2e/README.md index e95d3d5684..ac651bab51 100644 --- a/scripts/e2e/README.md +++ b/scripts/e2e/README.md @@ -228,8 +228,11 @@ harness lower-cases them. Its existing cleanup closes newly opened PRs with thei harness stages `sandbox-smoke.yaml` from the read-only `/opt/e2e` mount into the disposable sandbox clone's `.autoskillit/recipes/` directory before invoking fleet by recipe name. This lets the clone's project name resolve the staged recipe; the host scripts remain read-only. -The worker changes only `sandbox/text.py` and `tests/test_smoke_canary.py`, pushes its assigned -per-run branch, opens a PR and closes it. Before cleanup, the harness verifies the successful +Fleet resolves `base_branch` from the sandbox project's existing +`branching.default_base_branch` configuration, matching the clone's base branch. +The worker changes and commits only `sandbox/text.py` and `tests/test_smoke_canary.py`. +The orchestrator runs the sandbox test gate, pushes the assigned per-run branch, opens a PR +and closes it. Before cleanup, the harness verifies the successful envelope, that exactly one PR for the assigned branch is closed, the complete PR diff contains only those prescribed paths, and the PR head, remote ref and fetched commit agree. The source and test must contain the requested sentinel and an executable assertion. It saves the PR, @@ -240,8 +243,8 @@ the model process or a fleet result. The workflow wraps this test's model contai per-run Docker CID file. Its exit trap captures the model result, removes the recorded container by its exact ID, verifies that ID is absent, then launches a fresh user-image container for `e2e_harness.py cleanup`. That command uses only `E2E_SANDBOX_TOKEN`, closes any -remaining PR for the assigned branch, deletes that exact remote branch and verifies both are -gone. Cleanup runs after model success, failure or timeout, and its result is part of the step +remaining PR for the assigned branch, deletes that exact remote branch and verifies no open +PR remains and the ref is absent. Cleanup runs after model success, failure or timeout, and its result is part of the step status. The normal artifact redaction step then runs even when the test step failed. Recipes without this fixture keep their existing in-container cleanup path. diff --git a/scripts/e2e/recipes/sandbox-smoke.yaml b/scripts/e2e/recipes/sandbox-smoke.yaml index 56643fb637..eeaa735c73 100644 --- a/scripts/e2e/recipes/sandbox-smoke.yaml +++ b/scripts/e2e/recipes/sandbox-smoke.yaml @@ -25,7 +25,8 @@ ingredients: hidden: true base_branch: description: Sandbox branch targeted by the smoke pull request. - required: true + default: "" + authority: config hidden: true kitchen_rules: diff --git a/tests/_test_filter.py b/tests/_test_filter.py index 0fcd757d07..aba11ca839 100644 --- a/tests/_test_filter.py +++ b/tests/_test_filter.py @@ -870,7 +870,7 @@ class SourceMapRejection(enum.StrEnum): MODULE_CASCADE_CONFIG: dict[str, frozenset[str]] = { "_config_loader": frozenset({"config", "cli"}), - "ingredient_defaults": frozenset({"config", "recipe", "server"}), + "ingredient_defaults": frozenset({"config", "recipe", "server", "infra/test_e2e_harness.py"}), } _API_ORCHESTRATION_SHARD_CASCADE: frozenset[str] = frozenset( @@ -975,9 +975,9 @@ class SourceMapRejection(enum.StrEnum): } ), "rules_skill_write_path_alignment": frozenset({"recipe"}), - # --- Internal utility modules (no external src importers) --- + # --- Internal utility modules with narrow server/test consumers --- "_analysis": frozenset({"recipe"}), - "_binding": frozenset({"recipe", "server"}), + "_binding": frozenset({"recipe", "server", "infra/test_e2e_harness.py"}), "_binding_input": frozenset({"recipe", "server"}), "_analysis_bfs": frozenset({"recipe"}), "_analysis_blocks": frozenset({"recipe"}), @@ -1431,6 +1431,8 @@ class SourceMapRejection(enum.StrEnum): # file-level: fleet tests that import server tool handlers directly "fleet/test_resume_precondition.py", "fleet/test_headless_resume_max_attempts.py", + # file-level: sandbox smoke E2E covers fleet ingredient materialization + "infra/test_e2e_harness.py", } ), # L3 diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index fa94fd8715..9c714d893f 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -523,6 +523,71 @@ def test_fixture_stages_discovers_and_passes_the_production_validator(self, tmp_ ) assert "do not call any tools" in failed.message.lower() + def test_config_branch_survives_fleet_validation_and_binds_to_staged_recipe( + self, tmp_path: Path + ) -> None: + from typing import Any, cast + + from autoskillit.config import ( + build_config_authoritative_layer, + resolve_ingredient_defaults, + strip_server_authoritative_overrides, + ) + from autoskillit.fleet.dispatch._lineage import _validate_and_materialize_ingredients + from autoskillit.recipe import bind_recipe + from autoskillit.recipe.io import load_recipe + + catalog, test = _smoke_case() + assert test.recipe is not None and test.recipe_fixture is not None + project = tmp_path / "sandbox" + recipes = project / ".autoskillit" / "recipes" + recipes.mkdir(parents=True) + (project / ".autoskillit" / "config.yaml").write_text( + "branching:\n default_base_branch: main\n", encoding="utf-8" + ) + source = Path(harness.__file__).with_name("recipes") / test.recipe_fixture + staged = recipes / f"{test.recipe}.yaml" + staged.write_bytes(source.read_bytes()) + recipe = load_recipe(staged) + base_branch = recipe.ingredients["base_branch"] + assert base_branch.default == "" and base_branch.authority == "config" + assert base_branch.required is False + + caller_ingredients = { + "source_dir": str(project), + "repository": catalog.sandbox_repository, + "remote_url": f"https://github.com/{catalog.sandbox_repository}.git", + "branch_name": "e2e-smoke-" + "a" * 32, + "base_branch": "main", + } + stripped, stripped_names = strip_server_authoritative_overrides(caller_ingredients) + assert stripped_names == frozenset({"base_branch"}) + defaults = resolve_ingredient_defaults(project) + config_layer = build_config_authoritative_layer(defaults) + assert config_layer["base_branch"] == "main" + + materialized = _validate_and_materialize_ingredients( + effective_ingredients=stripped, + full_recipe=recipe, + dispatches_dir=tmp_path / "dispatches", + campaign_id="e2e-smoke", + dispatch_id="smoke-dispatch", + provenance=cast(Any, None), + state_path=project / ".autoskillit" / "state.json", + effective_name=test.recipe, + tool_ctx=cast(Any, None), + ) + assert isinstance(materialized, dict) + assert "base_branch" not in materialized + + projection = bind_recipe(recipe, ingredient_values={**materialized, **config_layer}) + invocation = projection.invocations["create_pull_request"] + command = next( + value.effective_value for value in invocation.mcp_kwargs if value.name == "cmd" + ) + assert invocation.is_valid + assert isinstance(command, str) and '--base "main"' in command + class TestSandboxSmokeFlow: def test_success_verifies_exact_closed_pr_ref_complete_diff_and_canary( From 4146eeb09ba85ce6f343984dcb8bb6cab69bd9b9 Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 5 Oct 2026 10:05:57 -0700 Subject: [PATCH 05/25] test(e2e): complete fleet infra dependency coverage --- tests/_test_filter.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/_test_filter.py b/tests/_test_filter.py index aba11ca839..fdf286b8b1 100644 --- a/tests/_test_filter.py +++ b/tests/_test_filter.py @@ -1431,8 +1431,9 @@ class SourceMapRejection(enum.StrEnum): # file-level: fleet tests that import server tool handlers directly "fleet/test_resume_precondition.py", "fleet/test_headless_resume_max_attempts.py", - # file-level: sandbox smoke E2E covers fleet ingredient materialization + # file-level: infra tests that import fleet directly "infra/test_e2e_harness.py", + "infra/test_pretty_output_hook_infra.py", } ), # L3 From d7df705b9bd0a3b88cd8ce96d16702767753104e Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 5 Oct 2026 10:43:08 -0700 Subject: [PATCH 06/25] test(e2e): track retired kitchen bootstrap as an expected failure --- scripts/e2e/catalog.json | 10 +++++++++- tests/infra/test_e2e_harness.py | 11 +++++------ tests/infra/test_e2e_select.py | 1 - 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/scripts/e2e/catalog.json b/scripts/e2e/catalog.json index 00bf957ce1..c568fddafb 100644 --- a/scripts/e2e/catalog.json +++ b/scripts/e2e/catalog.json @@ -69,7 +69,15 @@ "recipe": "sandbox-smoke", "recipe_fixture": "sandbox-smoke.yaml", "ingredients": {}, - "expected_pull_request_state": "closed" + "expected_pull_request_state": "closed", + "expected_failures": [ + { + "severity": "error", + "check": "open_kitchen_failed", + "message": "open_kitchen_failed", + "issue": "https://github.com/TalonT-Org/AutoSkillit/issues/5250" + } + ] } ] } diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index 9c714d893f..62772e141b 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -175,12 +175,11 @@ def _run_canary(tmp_path: Path, runner: FakeRunner, monkeypatch, env=None): def _smoke_case(expected_failures=None): catalog = e2e_catalog.load_catalog() test = catalog.get("headless-smoke") - if expected_failures is not None: - test = replace(test, expected_failures=tuple(expected_failures)) - catalog = replace( - catalog, - tests=tuple(test if entry.name == test.name else entry for entry in catalog.tests), - ) + test = replace(test, expected_failures=tuple(expected_failures or ())) + catalog = replace( + catalog, + tests=tuple(test if entry.name == test.name else entry for entry in catalog.tests), + ) return catalog, test diff --git a/tests/infra/test_e2e_select.py b/tests/infra/test_e2e_select.py index 7295c85da3..ecec41bf95 100644 --- a/tests/infra/test_e2e_select.py +++ b/tests/infra/test_e2e_select.py @@ -427,7 +427,6 @@ def test_shipped_catalog_holds_the_canary(self) -> None: assert smoke.recipe_fixture == "sandbox-smoke.yaml" assert smoke.ingredients == () assert smoke.expected_pull_request_state == "closed" - assert smoke.expected_failures == () assert smoke.trigger_paths == ( "src/autoskillit/execution/headless/*", "src/autoskillit/execution/backends/*", From 1beaac212c3473dbb314b5d25c2ec6a728b8ec08 Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 5 Oct 2026 18:10:01 -0700 Subject: [PATCH 07/25] fix(review): remove duplicate smoke metadata assertion --- tests/infra/test_e2e_harness.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index 62772e141b..b871b1a32a 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -223,13 +223,6 @@ def _smoke_metadata( "origin", f"refs/heads/{state['branch_name']}", ] - assert argv == [ - "git", - "ls-remote", - "--exit-code", - "origin", - f"refs/heads/{state['branch_name']}", - ] collision = mode == "branch-collision" record = f"{'a' * 40}\trefs/heads/{state['branch_name']}\n" if collision else "" return _completed(argv, record, returncode=0 if collision else 2) From ba596c62cd393592f0ba4618cb64da997e178d75 Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 5 Oct 2026 18:10:51 -0700 Subject: [PATCH 08/25] fix(review): preserve smoke cleanup exception diagnostics --- scripts/e2e/e2e_harness.py | 4 +++- tests/infra/test_e2e_harness.py | 24 ++++++++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/scripts/e2e/e2e_harness.py b/scripts/e2e/e2e_harness.py index 215dbe268a..2fe506b7f2 100644 --- a/scripts/e2e/e2e_harness.py +++ b/scripts/e2e/e2e_harness.py @@ -13,6 +13,7 @@ import sys import tempfile import time +import traceback import uuid from collections.abc import Callable, Mapping, Sequence from pathlib import Path @@ -811,7 +812,8 @@ def cleanup_test( resource_failures, evidence = _cleanup_smoke_resources(descriptor, scrubbed, runner) failures += resource_failures except Exception as exc: - failures.append(f"cleanup error: {exc}") + failures.append(f"cleanup error: {type(exc).__name__}: {exc}") + evidence["exception"] = traceback.format_exc() evidence["failures"] = list(failures) _atomic_json(out / "smoke-cleanup.json", evidence) result["failures"] += failures diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index b871b1a32a..f8c09300cd 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -1033,6 +1033,30 @@ def test_cleanup_errors_fail_and_attempt_the_other_resource_operation( assert state["pr_open"] is (fault == "close") assert (_bare_ref(remote, branch) is not None) is (fault == "delete") + def test_unexpected_cleanup_exception_preserves_type_and_traceback( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + catalog, test = _smoke_case() + out = tmp_path / "out" + _write_cleanup_inputs(out, test, catalog.sandbox_repository) + runner = FakeRunner(lambda argv: _completed(argv)) + + def fail_cleanup(*_args: object) -> None: + raise RuntimeError("unexpected cleanup fault") + + monkeypatch.setattr(harness, "_cleanup_smoke_resources", fail_cleanup) + + assert harness.cleanup_test(catalog, test.name, runner, out, _env()) == 1 + evidence = json.loads((out / "smoke-cleanup.json").read_text(encoding="utf-8")) + result = json.loads((out / "result.json").read_text(encoding="utf-8")) + failure = "cleanup error: RuntimeError: unexpected cleanup fault" + assert evidence["failures"] == [failure] + assert "Traceback (most recent call last)" in evidence["exception"] + assert "fail_cleanup" in evidence["exception"] + assert "RuntimeError: unexpected cleanup fault" in evidence["exception"] + assert result["failures"] == [failure] + assert result["outcome"] == "failed" + def test_missing_marker_is_a_noop_but_bad_descriptor_and_result_are_saved( self, tmp_path: Path ) -> None: From 41963376bd76de505efff79c2b38930fdf61c861 Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 5 Oct 2026 18:11:33 -0700 Subject: [PATCH 09/25] fix(review): name the smoke cleanup entry point explicitly --- scripts/e2e/e2e_harness.py | 4 ++-- tests/infra/test_e2e_harness.py | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/scripts/e2e/e2e_harness.py b/scripts/e2e/e2e_harness.py index 2fe506b7f2..5625e30cfc 100644 --- a/scripts/e2e/e2e_harness.py +++ b/scripts/e2e/e2e_harness.py @@ -782,7 +782,7 @@ def _model_result(out: Path, test_name: str) -> tuple[dict[str, Any], list[str]] return result, [f"cleanup: missing or malformed model result: {exc}"] -def cleanup_test( +def cleanup_smoke_test( catalog: e2e_catalog.Catalog, test_name: str, runner: Runner, @@ -1143,7 +1143,7 @@ def main(argv: Sequence[str]) -> int: return _run(args.test, Path(args.out), Path(args.catalog)) if args.command == "cleanup": catalog = e2e_catalog.load_catalog(Path(args.catalog)) - return cleanup_test(catalog, args.test, run_command, Path(args.out), os.environ) + return cleanup_smoke_test(catalog, args.test, run_command, Path(args.out), os.environ) return _redact_command(Path(args.dest), args.secret_env, args.sources) diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index f8c09300cd..59a0dc27eb 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -956,7 +956,7 @@ def test_real_bare_remote_cleanup_preserves_unrelated_branch_and_expected_failur sleeps: list[float] = [] monkeypatch.setattr(harness.time, "sleep", sleeps.append) - status = harness.cleanup_test(catalog, test.name, runner, out, env) + status = harness.cleanup_smoke_test(catalog, test.name, runner, out, env) result = json.loads((out / "result.json").read_text(encoding="utf-8")) evidence = json.loads((out / "smoke-cleanup.json").read_text(encoding="utf-8")) @@ -994,7 +994,7 @@ def test_cleanup_deletes_a_pushed_branch_even_without_a_pull_request( sleeps: list[float] = [] monkeypatch.setattr(harness.time, "sleep", sleeps.append) - assert harness.cleanup_test(catalog, test.name, runner, out, env) == 0 + assert harness.cleanup_smoke_test(catalog, test.name, runner, out, env) == 0 evidence = json.loads((out / "smoke-cleanup.json").read_text(encoding="utf-8")) assert _bare_ref(remote, branch) is None @@ -1019,7 +1019,7 @@ def test_cleanup_errors_fail_and_attempt_the_other_resource_operation( env = _env(HOME=git_env["HOME"], PATH=git_env["PATH"], E2E_SANDBOX_TOKEN=SANDBOX_TOKEN) monkeypatch.setattr(harness.time, "sleep", lambda _duration: None) - status = harness.cleanup_test(catalog, test.name, runner, out, env) + status = harness.cleanup_smoke_test(catalog, test.name, runner, out, env) result = json.loads((out / "result.json").read_text(encoding="utf-8")) close_called = any(call.argv[:3] == ["gh", "pr", "close"] for call in runner.calls) delete_called = any( @@ -1046,7 +1046,7 @@ def fail_cleanup(*_args: object) -> None: monkeypatch.setattr(harness, "_cleanup_smoke_resources", fail_cleanup) - assert harness.cleanup_test(catalog, test.name, runner, out, _env()) == 1 + assert harness.cleanup_smoke_test(catalog, test.name, runner, out, _env()) == 1 evidence = json.loads((out / "smoke-cleanup.json").read_text(encoding="utf-8")) result = json.loads((out / "result.json").read_text(encoding="utf-8")) failure = "cleanup error: RuntimeError: unexpected cleanup fault" @@ -1063,7 +1063,7 @@ def test_missing_marker_is_a_noop_but_bad_descriptor_and_result_are_saved( catalog, test = _smoke_case() runner = FakeRunner(lambda argv: _completed(argv)) out = tmp_path / "not-launched" - assert harness.cleanup_test(catalog, test.name, runner, out, _env()) == 0 + assert harness.cleanup_smoke_test(catalog, test.name, runner, out, _env()) == 0 assert runner.calls == [] assert not out.exists() @@ -1072,7 +1072,7 @@ def test_missing_marker_is_a_noop_but_bad_descriptor_and_result_are_saved( (bad / "smoke-launched").write_text(test.name + "\n", encoding="utf-8") (bad / "smoke-lifecycle.json").write_text("{broken", encoding="utf-8") (bad / "result.json").write_text("{also broken", encoding="utf-8") - assert harness.cleanup_test(catalog, test.name, runner, bad, _env()) == 1 + assert harness.cleanup_smoke_test(catalog, test.name, runner, bad, _env()) == 1 assert (bad / "result-before-cleanup.json").read_text(encoding="utf-8") == "{also broken" result = json.loads((bad / "result.json").read_text(encoding="utf-8")) evidence = json.loads((bad / "smoke-cleanup.json").read_text(encoding="utf-8")) From fb416ac08881e570dea673fd7da2ad2be367c7ba Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 5 Oct 2026 18:12:02 -0700 Subject: [PATCH 10/25] fix(review): use the ls-remote exit code for branch absence --- scripts/e2e/e2e_harness.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/e2e/e2e_harness.py b/scripts/e2e/e2e_harness.py index 5625e30cfc..e417ac6ab4 100644 --- a/scripts/e2e/e2e_harness.py +++ b/scripts/e2e/e2e_harness.py @@ -430,7 +430,7 @@ def prepare_smoke( env, cwd=SANDBOX_CLONE, ) - if result is None or result.returncode != 2 or result.stdout.strip(): + if result is None or result.returncode != 2: return [failure or "smoke setup: assigned remote branch already exists"], None descriptor = {key: runtime[key] for key in ("repository", "base_branch", "branch_name")} descriptor["test"] = test.name From 785a3465b79fa428298972f369f7fb0314841f7a Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 5 Oct 2026 18:12:44 -0700 Subject: [PATCH 11/25] fix(review): validate smoke recipe basenames before staging --- scripts/e2e/e2e_harness.py | 2 ++ tests/infra/test_e2e_harness.py | 16 ++++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/scripts/e2e/e2e_harness.py b/scripts/e2e/e2e_harness.py index e417ac6ab4..c6f2ef5dcc 100644 --- a/scripts/e2e/e2e_harness.py +++ b/scripts/e2e/e2e_harness.py @@ -407,6 +407,8 @@ def prepare_smoke( env: Mapping[str, str], runner: Runner, ) -> tuple[list[str], dict[str, str] | None]: + if test.recipe is None or not re.fullmatch(r"[a-z0-9][a-z0-9-]*", test.recipe): + return ["smoke setup: recipe must be a basename"], None source = Path(__file__).with_name("recipes") / str(test.recipe_fixture) target = SANDBOX_CLONE / ".autoskillit" / "recipes" / f"{test.recipe}.yaml" target.parent.mkdir(parents=True, exist_ok=True) diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index 59a0dc27eb..8c7f8af3aa 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -582,6 +582,22 @@ def test_config_branch_survives_fleet_validation_and_binds_to_staged_recipe( class TestSandboxSmokeFlow: + def test_recipe_path_traversal_is_rejected_before_staging( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + catalog, test = _smoke_case() + runner, clone = _smoke_runner(tmp_path, monkeypatch) + test = replace(test, recipe="../escaped") + + failures, runtime = harness.prepare_smoke( + test, catalog.sandbox_repository, tmp_path / "out", _env(), runner + ) + + assert failures == ["smoke setup: recipe must be a basename"] + assert runtime is None + assert not (clone / ".autoskillit").exists() + assert runner.calls == [] + def test_success_verifies_exact_closed_pr_ref_complete_diff_and_canary( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: From 0ff33253248e439ef4566b7774b9ef3d43fef553 Mon Sep 17 00:00:00 2001 From: Trecek Date: Tue, 6 Oct 2026 08:22:13 -0700 Subject: [PATCH 12/25] fix(review): share sandbox smoke resource cleanup --- .github/workflows/e2e.yml | 44 +++----------------------------- scripts/e2e/README.md | 37 ++------------------------- scripts/e2e/smoke-cleanup.sh | 43 +++++++++++++++++++++++++++++++ tests/infra/test_e2e_workflow.py | 2 ++ 4 files changed, 50 insertions(+), 76 deletions(-) create mode 100644 scripts/e2e/smoke-cleanup.sh diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index a492fcb441..e721b7eb0f 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -119,51 +119,13 @@ jobs: exit 1 fi + source "$GITHUB_WORKSPACE/scripts/e2e/smoke-cleanup.sh" cleanup_smoke() { model_status=$? set +e - removal_status=0 - cleanup_status=0 - rm_status=1 - verify_status=1 lifecycle_file="$RUNNER_TEMP/e2e/out/${TEST}-lifecycle.txt" - - if [[ ! -s "$cid_file" ]]; then - echo "missing container ID evidence: $cid_file" >&2 - removal_status=1 - else - container_id="$(<"$cid_file")" - if [[ ! "$container_id" =~ ^[0-9a-f]{64}$ ]]; then - echo "invalid container ID evidence retained: $cid_file" >&2 - removal_status=1 - else - timeout --signal=TERM --kill-after=10s 60s \ - docker rm -f "$container_id" - rm_status=$? - remaining="$(timeout --signal=TERM --kill-after=10s 60s \ - docker container ls --all --quiet --no-trunc \ - --filter "id=$container_id")" - verify_status=$? - if (( verify_status != 0 )) || [[ -n "$remaining" ]]; then - echo "container removal could not be verified; retaining $cid_file" >&2 - removal_status=1 - elif ! rm -f -- "$cid_file"; then - echo "could not remove verified container ID file: $cid_file" >&2 - removal_status=1 - elif (( rm_status != 0 )); then - echo "container was already absent; exact-ID absence verified" - fi - fi - fi - - timeout --signal=TERM --kill-after=20s 300s docker run --rm \ - --env E2E_SANDBOX_TOKEN \ - --volume "$GITHUB_WORKSPACE/scripts/e2e:/opt/e2e:ro" \ - --volume "$RUNNER_TEMP/e2e/out:/artifacts" \ - "$IMAGE" \ - python3 /opt/e2e/e2e_harness.py cleanup \ - --test "$TEST" --catalog /opt/e2e/catalog.json --out /artifacts - cleanup_status=$? + cleanup_smoke_resources "$cid_file" "$GITHUB_WORKSPACE/scripts/e2e" \ + "$RUNNER_TEMP/e2e/out" "$IMAGE" "$TEST" printf 'model_exit=%s\ndocker_rm=%s\ncontainer_verify=%s\ncontainer_removal=%s\nremote_cleanup=%s\n' \ "$model_status" "$rm_status" "$verify_status" "$removal_status" "$cleanup_status" \ diff --git a/scripts/e2e/README.md b/scripts/e2e/README.md index ac651bab51..0d469a99cb 100644 --- a/scripts/e2e/README.md +++ b/scripts/e2e/README.md @@ -352,46 +352,13 @@ RUN_ID="$(date +%s)-$$" CID_FILE="$E2E_DIR/out/${TEST}-${RUN_ID}.cid" [[ ! -e "$CID_FILE" ]] +source "$PWD/scripts/e2e/smoke-cleanup.sh" cleanup_smoke() { model_status=$? set +e - rm_status=1 - verify_status=1 - removal_status=0 - cleanup_status=0 redact_status=0 lifecycle_file="$E2E_DIR/out/${TEST}-lifecycle.txt" - - if [[ ! -s "$CID_FILE" ]]; then - echo "missing container ID evidence: $CID_FILE" >&2 - removal_status=1 - else - container_id="$(<"$CID_FILE")" - if [[ ! "$container_id" =~ ^[0-9a-f]{64}$ ]]; then - echo "invalid container ID evidence retained: $CID_FILE" >&2 - removal_status=1 - else - timeout --signal=TERM --kill-after=10s 60s docker rm -f "$container_id" - rm_status=$? - remaining="$(timeout --signal=TERM --kill-after=10s 60s \ - docker container ls --all --quiet --no-trunc --filter "id=$container_id")" - verify_status=$? - if (( verify_status != 0 )) || [[ -n "$remaining" ]]; then - echo "container removal could not be verified; retaining $CID_FILE" >&2 - removal_status=1 - elif ! rm -f -- "$CID_FILE"; then - removal_status=1 - fi - fi - fi - - timeout --signal=TERM --kill-after=20s 300s docker run --rm \ - --env E2E_SANDBOX_TOKEN \ - --volume "$PWD/scripts/e2e:/opt/e2e:ro" \ - --volume "$E2E_DIR/out:/artifacts" \ - "$IMAGE" python3 /opt/e2e/e2e_harness.py cleanup \ - --test "$TEST" --catalog /opt/e2e/catalog.json --out /artifacts - cleanup_status=$? + cleanup_smoke_resources "$CID_FILE" "$PWD/scripts/e2e" "$E2E_DIR/out" "$IMAGE" "$TEST" docker run --rm --env MINIMAX_API_KEY --env E2E_SANDBOX_TOKEN \ --volume "$PWD/scripts/e2e:/opt/e2e:ro" \ diff --git a/scripts/e2e/smoke-cleanup.sh b/scripts/e2e/smoke-cleanup.sh new file mode 100644 index 0000000000..e74584c4ff --- /dev/null +++ b/scripts/e2e/smoke-cleanup.sh @@ -0,0 +1,43 @@ +# Sourced by CI and the local smoke wrapper with errexit disabled. +# Sets rm_status, verify_status, removal_status and cleanup_status for lifecycle reporting. +cleanup_smoke_resources() { + local cid_file=$1 scripts_dir=$2 out_dir=$3 image=$4 test=$5 + local container_id remaining + rm_status=1 + verify_status=1 + removal_status=0 + + if [[ ! -s "$cid_file" ]]; then + echo "missing container ID evidence: $cid_file" >&2 + removal_status=1 + else + container_id="$(<"$cid_file")" + if [[ ! "$container_id" =~ ^[0-9a-f]{64}$ ]]; then + echo "invalid container ID evidence retained: $cid_file" >&2 + removal_status=1 + else + timeout --signal=TERM --kill-after=10s 60s docker rm -f "$container_id" + rm_status=$? + remaining="$(timeout --signal=TERM --kill-after=10s 60s \ + docker container ls --all --quiet --no-trunc --filter "id=$container_id")" + verify_status=$? + if (( verify_status != 0 )) || [[ -n "$remaining" ]]; then + echo "container removal could not be verified; retaining $cid_file" >&2 + removal_status=1 + elif ! rm -f -- "$cid_file"; then + echo "could not remove verified container ID file: $cid_file" >&2 + removal_status=1 + elif (( rm_status != 0 )); then + echo "container was already absent; exact-ID absence verified" + fi + fi + fi + + timeout --signal=TERM --kill-after=20s 300s docker run --rm \ + --env E2E_SANDBOX_TOKEN \ + --volume "$scripts_dir:/opt/e2e:ro" \ + --volume "$out_dir:/artifacts" \ + "$image" python3 /opt/e2e/e2e_harness.py cleanup \ + --test "$test" --catalog /opt/e2e/catalog.json --out /artifacts + cleanup_status=$? +} diff --git a/tests/infra/test_e2e_workflow.py b/tests/infra/test_e2e_workflow.py index 05bfed1736..a6fa7bdf4b 100644 --- a/tests/infra/test_e2e_workflow.py +++ b/tests/infra/test_e2e_workflow.py @@ -113,6 +113,8 @@ def _run_smoke_workflow_shell(workflow: dict, tmp_path: Path, **scenario: str): (runner_temp / "e2e" / "data").mkdir() workspace = tmp_path / "workspace" (workspace / "scripts" / "e2e").mkdir(parents=True) + helper = REPO_ROOT / "scripts" / "e2e" / "smoke-cleanup.sh" + (workspace / "scripts" / "e2e" / helper.name).write_bytes(helper.read_bytes()) docker_bin = tmp_path / "bin" docker_bin.mkdir() docker = docker_bin / "docker" From 89d777b2f5060995e6735e87411ac8916bf00db9 Mon Sep 17 00:00:00 2001 From: Trecek Date: Tue, 6 Oct 2026 08:24:32 -0700 Subject: [PATCH 13/25] fix(review): accept already absent smoke refs during cleanup --- scripts/e2e/e2e_harness.py | 2 +- tests/infra/test_e2e_harness.py | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/scripts/e2e/e2e_harness.py b/scripts/e2e/e2e_harness.py index c6f2ef5dcc..bfff103587 100644 --- a/scripts/e2e/e2e_harness.py +++ b/scripts/e2e/e2e_harness.py @@ -512,7 +512,7 @@ def _ref_request( status = int(match[1]) if match else None if method == "DELETE" and status == 204 and not failure: return status, None, None - if method == "GET" and status == 404: + if method in ("GET", "DELETE") and status == 404: return status, None, None if status != 200 or failure: return status, None, failure or f"gh api: unexpected HTTP status {status}" diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index 8c7f8af3aa..f4e1995f79 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -887,10 +887,13 @@ def handle(argv: list[str]): text=True, check=False, ) + http_status, returncode = { + "already-deleted": ("HTTP/2.0 404 Not Found", 1), + }.get(fault, ("HTTP/2.0 204 No Content", result.returncode)) return _completed( argv, - "HTTP/2.0 204 No Content\r\n\r\n", - returncode=result.returncode, + f"{http_status}\r\n\r\n", + returncode=returncode, stderr=result.stderr, ) if argv[:3] == ["gh", "pr", "list"]: @@ -995,8 +998,9 @@ def test_real_bare_remote_cleanup_preserves_unrelated_branch_and_expected_failur ] assert close_calls == [["gh", "pr", "close", "8", "--repo", catalog.sandbox_repository]] + @pytest.mark.parametrize("fault", [None, "already-deleted"]) def test_cleanup_deletes_a_pushed_branch_even_without_a_pull_request( - self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, fault: str | None ) -> None: catalog, test = _smoke_case() branch = "e2e-smoke-" + "a" * 32 @@ -1004,7 +1008,7 @@ def test_cleanup_deletes_a_pushed_branch_even_without_a_pull_request( out = tmp_path / "out" _write_cleanup_inputs(out, test, catalog.sandbox_repository) runner, _state = _smoke_cleanup_runner( - remote, branch, catalog.sandbox_repository, with_pr=False + remote, branch, catalog.sandbox_repository, with_pr=False, fault=fault ) env = _env(HOME=git_env["HOME"], PATH=git_env["PATH"], E2E_SANDBOX_TOKEN=SANDBOX_TOKEN) sleeps: list[float] = [] From 6460d251bf05e5e8a90a68c2406db06066dacec1 Mon Sep 17 00:00:00 2001 From: Trecek Date: Tue, 6 Oct 2026 08:25:40 -0700 Subject: [PATCH 14/25] fix(review): project validated smoke lifecycle fields --- scripts/e2e/e2e_harness.py | 2 +- tests/infra/test_e2e_harness.py | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/scripts/e2e/e2e_harness.py b/scripts/e2e/e2e_harness.py index bfff103587..dd1f0e4437 100644 --- a/scripts/e2e/e2e_harness.py +++ b/scripts/e2e/e2e_harness.py @@ -397,7 +397,7 @@ def _validate_smoke_descriptor( raise ValueError("invalid smoke lifecycle base branch") if base == descriptor["branch_name"]: raise ValueError("smoke lifecycle branch equals base") - return descriptor + return {key: descriptor[key] for key in ("test", "repository", "branch_name", "base_branch")} def prepare_smoke( diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index f4e1995f79..c3abab7246 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -466,6 +466,21 @@ def _run_smoke(tmp_path: Path, runner: FakeRunner, monkeypatch: pytest.MonkeyPat class TestSandboxSmokeFixture: + def test_lifecycle_descriptor_returns_only_validated_fields(self): + catalog, test = _smoke_case() + expected = { + "test": test.name, + "repository": catalog.sandbox_repository, + "branch_name": "e2e-smoke-" + "a" * 32, + "base_branch": "main", + } + descriptor = {**expected, "extra": {"unvalidated": True}} + + assert ( + harness._validate_smoke_descriptor(descriptor, test, catalog.sandbox_repository) + == expected + ) + def test_fixture_stages_discovers_and_passes_the_production_validator(self, tmp_path: Path): from autoskillit.recipe.io import find_recipe_by_name, load_recipe from autoskillit.recipe.validator import validate_recipe_structure From 07e455c730f834c1d6eef42eac6aea84eee9c165 Mon Sep 17 00:00:00 2001 From: Trecek Date: Tue, 6 Oct 2026 08:26:42 -0700 Subject: [PATCH 15/25] test(review): isolate recipe failure matching scenarios --- tests/infra/test_e2e_harness.py | 54 +++++++++++++++++++++++---------- 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index c3abab7246..570b1c927a 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -753,32 +753,54 @@ def test_expected_failure_matches_only_the_exact_structured_envelope( assert result["outcome"] == "expected_failure" assert result["expected_findings"] == [expected] + @pytest.mark.parametrize( + ("envelope_overrides", "initial_failures", "expected_failures", "expected_findings"), + [ + pytest.param( + {}, + ["cleanup: delete failed"], + ["cleanup: delete failed"], + [_SMOKE_FAILURE], + id="matched-with-extra-errors", + ), + pytest.param( + {"user_visible_message": " "}, + ["fleet run failed"], + ["fleet run failed"], + [], + id="malformed-envelope", + ), + pytest.param( + {"success": True}, + [], + [ + "fleet run: remove stale expected failure for fixed bug " + f"{_SMOKE_FAILURE['issue']}" + ], + [], + id="stale-expectation", + ), + ], + ) def test_recipe_matching_preserves_extra_errors_and_rejects_malformed_or_stale_rows( self, + envelope_overrides, + initial_failures, + expected_failures, + expected_findings, ) -> None: - expected = _SMOKE_FAILURE - _catalog, test = _smoke_case([expected]) + _catalog, test = _smoke_case([_SMOKE_FAILURE]) matching = { "success": False, "kind": "completed", "dispatch_status": "failed", "error": "workflow_failed", "user_visible_message": "model reported failure", + **envelope_overrides, } - failures, findings = harness.match_recipe_failure( - test, ["cleanup: delete failed"], matching - ) - assert failures == ["cleanup: delete failed"] - assert findings == [expected] - - malformed = {**matching, "user_visible_message": " "} - failures, findings = harness.match_recipe_failure(test, ["fleet run failed"], malformed) - assert failures == ["fleet run failed"] - assert findings == [] - - failures, findings = harness.match_recipe_failure(test, [], {"success": True}) - assert len(failures) == 1 and "remove stale expected failure" in failures[0] - assert findings == [] + failures, findings = harness.match_recipe_failure(test, initial_failures, matching) + assert failures == expected_failures + assert findings == expected_findings def test_expected_failure_console_outcome_comes_from_result_file( self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] From 6e7f114be8ab7d4cf24955dda35bfce4263ded95 Mon Sep 17 00:00:00 2001 From: Trecek Date: Tue, 6 Oct 2026 08:27:47 -0700 Subject: [PATCH 16/25] test(review): cover individual smoke canary validation guards --- tests/infra/test_e2e_harness.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index 570b1c927a..6414944b8c 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -466,6 +466,26 @@ def _run_smoke(tmp_path: Path, runner: FakeRunner, monkeypatch: pytest.MonkeyPat class TestSandboxSmokeFixture: + @pytest.mark.parametrize( + ("original", "replacement"), + [ + pytest.param("from sandbox.text import smoke_canary\n", "", id="missing-import"), + pytest.param("unittest.TestCase", "object", id="wrong-base-class"), + pytest.param("def test_smoke_canary", "def helper_smoke_canary", id="no-test-method"), + pytest.param("self.assertTrue", "print", id="no-assert-call"), + pytest.param( + "self.assertTrue(smoke_canary())", + "self.assertIs(True, True)", + id="constant-assertion", + ), + pytest.param("self.assertTrue(smoke_canary())", "(", id="invalid-syntax"), + ], + ) + def test_canary_sources_reject_each_missing_requirement(self, original, replacement): + code, test_code = _smoke_sources() + assert harness._canary_sources_valid(code, test_code) + assert not harness._canary_sources_valid(code, test_code.replace(original, replacement)) + def test_lifecycle_descriptor_returns_only_validated_fields(self): catalog, test = _smoke_case() expected = { From 5f1e2129669dd091b4fa21aa277a3ba22b6367a1 Mon Sep 17 00:00:00 2001 From: Trecek Date: Tue, 6 Oct 2026 08:44:09 -0700 Subject: [PATCH 17/25] docs(review): clarify smoke kitchen expected failure boundary --- scripts/e2e/README.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/scripts/e2e/README.md b/scripts/e2e/README.md index 0d469a99cb..f6d7a1b5dd 100644 --- a/scripts/e2e/README.md +++ b/scripts/e2e/README.md @@ -254,7 +254,12 @@ Recipe `expected_failures` rows identify an individual, verified product defect accepted; process timeouts, malformed results, other failures and cleanup failures remain fatal. An accepted defect is provisional until mandatory branch/PR cleanup succeeds. A clean run that observes a configured row makes it stale and fails with instructions to remove the -row and close the bug. The smoke catalog starts without expected defects. +row and close the bug. The smoke catalog currently records +[#5250](https://github.com/TalonT-Org/AutoSkillit/issues/5250) for `open_kitchen_failed` +during kitchen initialization, before the recipe executes. This row does not accept +arbitrary failures from the recipe's `failed` stop step. Fleet translates the L3 result's +`reason` into the failed command envelope's `error` and `user_visible_message` fields; +the harness matches that envelope rather than the recipe payload's `failed_step`/`reason`. ## Catalog schema From 29bf3228fdc90be40df83809099acf0713cd04a0 Mon Sep 17 00:00:00 2001 From: Trecek Date: Tue, 6 Oct 2026 08:49:53 -0700 Subject: [PATCH 18/25] test(review): inspect sourced smoke cleanup in workflow assertions --- tests/infra/test_e2e_workflow.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/infra/test_e2e_workflow.py b/tests/infra/test_e2e_workflow.py index a6fa7bdf4b..8c2203d973 100644 --- a/tests/infra/test_e2e_workflow.py +++ b/tests/infra/test_e2e_workflow.py @@ -61,6 +61,14 @@ def _model_branch(run: str) -> tuple[str, str]: return "\n".join(lines[start + 1 : end]), "\n".join(lines[:start] + lines[end + 1 :]) +def _inline_smoke_cleanup(run: str) -> str: + helper = REPO_ROOT / "scripts" / "e2e" / "smoke-cleanup.sh" + return run.replace( + 'source "$GITHUB_WORKSPACE/scripts/e2e/smoke-cleanup.sh"', + helper.read_text(encoding="utf-8"), + ) + + _DOCKER_STUB = r"""#!/usr/bin/env python3 import json import os @@ -328,7 +336,7 @@ def test_report_step_uses_only_the_workflow_token(self, workflow: dict) -> None: @pytest.mark.parametrize("step_id", ["e2e", "redact"]) def test_docker_run_forwards_secrets_by_name_only(self, workflow: dict, step_id: str) -> None: - run = _step(workflow, "run", step_id=step_id)["run"] + run = _inline_smoke_cleanup(_step(workflow, "run", step_id=step_id)["run"]) forwarded = re.findall(r"--env ([A-Z0-9_]+)", run) expected = ["MINIMAX_API_KEY", "E2E_SANDBOX_TOKEN"] if step_id == "e2e": @@ -367,7 +375,7 @@ def test_redaction_keeps_required_secret_names(self, workflow: dict) -> None: def test_clean_install_uses_the_common_user_image_and_artifacts( workflow: dict, step_id: str, common_mounts: tuple[str, ...] ) -> None: - run = _step(workflow, "run", step_id=step_id)["run"] + run = _inline_smoke_cleanup(_step(workflow, "run", step_id=step_id)["run"]) branch, outside = _model_branch(run) assert run.count("docker_args=()") == 1 From 078d52156f81b68db149d45dd2ee8db1209e319b Mon Sep 17 00:00:00 2001 From: Trecek Date: Tue, 6 Oct 2026 13:53:41 -0700 Subject: [PATCH 19/25] fix(review): share smoke lifecycle finalization --- .github/workflows/e2e.yml | 9 ++------- scripts/e2e/README.md | 9 ++------- scripts/e2e/smoke-cleanup.sh | 12 ++++++++++++ tests/infra/test_e2e_workflow.py | 29 +++++++++++++++++++++++++++++ 4 files changed, 45 insertions(+), 14 deletions(-) diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index e721b7eb0f..ce9c788876 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -127,14 +127,9 @@ jobs: cleanup_smoke_resources "$cid_file" "$GITHUB_WORKSPACE/scripts/e2e" \ "$RUNNER_TEMP/e2e/out" "$IMAGE" "$TEST" - printf 'model_exit=%s\ndocker_rm=%s\ncontainer_verify=%s\ncontainer_removal=%s\nremote_cleanup=%s\n' \ - "$model_status" "$rm_status" "$verify_status" "$removal_status" "$cleanup_status" \ - > "$lifecycle_file" trap - EXIT - if (( model_status != 0 || removal_status != 0 || cleanup_status != 0 )); then - exit 1 - fi - exit 0 + finish_smoke_lifecycle "$lifecycle_file" "$model_status" + exit $? } trap cleanup_smoke EXIT diff --git a/scripts/e2e/README.md b/scripts/e2e/README.md index f6d7a1b5dd..c4371c613f 100644 --- a/scripts/e2e/README.md +++ b/scripts/e2e/README.md @@ -373,14 +373,9 @@ cleanup_smoke() { /e2e/out /e2e/data/logs redact_status=$? - printf 'model_exit=%s\ndocker_rm=%s\ncontainer_verify=%s\ncontainer_removal=%s\nremote_cleanup=%s\nredaction=%s\n' \ - "$model_status" "$rm_status" "$verify_status" "$removal_status" \ - "$cleanup_status" "$redact_status" > "$lifecycle_file" trap - EXIT - if (( model_status != 0 || removal_status != 0 || cleanup_status != 0 || redact_status != 0 )); then - exit 1 - fi - exit 0 + finish_smoke_lifecycle "$lifecycle_file" "$model_status" "$redact_status" + exit $? } trap cleanup_smoke EXIT diff --git a/scripts/e2e/smoke-cleanup.sh b/scripts/e2e/smoke-cleanup.sh index e74584c4ff..9144c2bd8e 100644 --- a/scripts/e2e/smoke-cleanup.sh +++ b/scripts/e2e/smoke-cleanup.sh @@ -41,3 +41,15 @@ cleanup_smoke_resources() { --test "$test" --catalog /opt/e2e/catalog.json --out /artifacts cleanup_status=$? } + +finish_smoke_lifecycle() { + local lifecycle_file=$1 model_status=$2 redact_status=${3:-0} + { + printf 'model_exit=%s\ndocker_rm=%s\ncontainer_verify=%s\ncontainer_removal=%s\nremote_cleanup=%s\n' \ + "$model_status" "$rm_status" "$verify_status" "$removal_status" "$cleanup_status" + if (( $# == 3 )); then + printf 'redaction=%s\n' "$redact_status" + fi + } > "$lifecycle_file" || return 1 + (( model_status == 0 && removal_status == 0 && cleanup_status == 0 && redact_status == 0 )) +} diff --git a/tests/infra/test_e2e_workflow.py b/tests/infra/test_e2e_workflow.py index 8c2203d973..bb3f7bb193 100644 --- a/tests/infra/test_e2e_workflow.py +++ b/tests/infra/test_e2e_workflow.py @@ -288,6 +288,35 @@ def test_headless_smoke_exit_trap_removes_exact_container_then_runs_cleanup( assert statuses[key] == value +@pytest.mark.parametrize("redact_status", [0, 1]) +def test_local_lifecycle_records_redaction_and_fails_when_redaction_fails( + tmp_path: Path, redact_status: int +) -> None: + lifecycle = tmp_path / "lifecycle.txt" + result = subprocess.run( + [ + "bash", + "--noprofile", + "--norc", + "-c", + 'source "$1"; rm_status=0; verify_status=0; removal_status=0; ' + 'cleanup_status=0; finish_smoke_lifecycle "$2" 0 "$3"', + "bash", + str(REPO_ROOT / "scripts/e2e/smoke-cleanup.sh"), + str(lifecycle), + str(redact_status), + ], + check=False, + capture_output=True, + text=True, + timeout=15, + ) + assert result.returncode == redact_status, result.stderr + statuses = dict(line.split("=", 1) for line in lifecycle.read_text().splitlines()) + assert statuses["redaction"] == str(redact_status) + assert statuses["model_exit"] == "0" + + class TestTriggers: def test_pull_request_types_include_labeled(self, workflow: dict) -> None: types = _on(workflow)["pull_request"]["types"] From 4f85134440673a7c436e99fbb359f93ec598d5ce Mon Sep 17 00:00:00 2001 From: Trecek Date: Tue, 6 Oct 2026 13:54:26 -0700 Subject: [PATCH 20/25] fix(review): remove abandoned JSON staging files --- scripts/e2e/e2e_harness.py | 14 +++++++++----- tests/infra/test_e2e_harness.py | 11 +++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/scripts/e2e/e2e_harness.py b/scripts/e2e/e2e_harness.py index dd1f0e4437..818b11d819 100644 --- a/scripts/e2e/e2e_harness.py +++ b/scripts/e2e/e2e_harness.py @@ -372,11 +372,15 @@ def match_recipe_failure( def _atomic_json(path: Path, payload: Mapping[str, Any]) -> None: - with tempfile.NamedTemporaryFile(mode="w", dir=path.parent, delete=False) as stream: - json.dump(payload, stream, indent=2) - stream.write("\n") - staged = Path(stream.name) - staged.replace(path) + stream = tempfile.NamedTemporaryFile(mode="w", dir=path.parent, delete=False) + staged = Path(stream.name) + try: + with stream: + json.dump(payload, stream, indent=2) + stream.write("\n") + staged.replace(path) + finally: + staged.unlink(missing_ok=True) def _validate_smoke_descriptor( diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index 6414944b8c..c4470b9603 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -465,6 +465,17 @@ def _run_smoke(tmp_path: Path, runner: FakeRunner, monkeypatch: pytest.MonkeyPat return failures, json.loads((out / "result.json").read_text(encoding="utf-8")), out, catalog +def test_atomic_json_cleans_failed_write_without_replacing_target(tmp_path: Path) -> None: + target = tmp_path / "evidence.json" + target.write_text('{"existing": true}\n', encoding="utf-8") + + with pytest.raises(TypeError): + harness._atomic_json(target, {"invalid": object()}) + + assert target.read_text(encoding="utf-8") == '{"existing": true}\n' + assert list(tmp_path.iterdir()) == [target] + + class TestSandboxSmokeFixture: @pytest.mark.parametrize( ("original", "replacement"), From a381c8543b29a3f505a3a5fd74c96ac08a458bd6 Mon Sep 17 00:00:00 2001 From: Trecek Date: Tue, 6 Oct 2026 13:54:56 -0700 Subject: [PATCH 21/25] fix(review): compare forwarded secrets without ordering --- tests/infra/test_e2e_workflow.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/infra/test_e2e_workflow.py b/tests/infra/test_e2e_workflow.py index bb3f7bb193..c1fa814d2e 100644 --- a/tests/infra/test_e2e_workflow.py +++ b/tests/infra/test_e2e_workflow.py @@ -6,6 +6,7 @@ import os import re import subprocess +from collections import Counter from pathlib import Path import pytest @@ -370,7 +371,7 @@ def test_docker_run_forwards_secrets_by_name_only(self, workflow: dict, step_id: expected = ["MINIMAX_API_KEY", "E2E_SANDBOX_TOKEN"] if step_id == "e2e": expected.append("E2E_SANDBOX_TOKEN") - assert forwarded == expected + assert Counter(forwarded) == Counter(expected) def test_redaction_keeps_required_secret_names(self, workflow: dict) -> None: run = _step(workflow, "run", step_id="redact")["run"] From e8d5ec2be66887891350582ebded4c5b7e6b1707 Mon Sep 17 00:00:00 2001 From: Trecek Date: Tue, 6 Oct 2026 13:55:32 -0700 Subject: [PATCH 22/25] fix(review): bound bare-ref test subprocess --- tests/infra/test_e2e_harness.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index c4470b9603..623ad6c905 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -909,6 +909,7 @@ def _bare_ref(remote: Path, branch: str) -> str | None: capture_output=True, text=True, check=False, + timeout=harness.SETUP_COMMAND_TIMEOUT_SEC, ) return result.stdout.strip() if result.returncode == 0 else None From a21b377200e0af21d82121e5a06f04eb71c04a9a Mon Sep 17 00:00:00 2001 From: Trecek Date: Tue, 6 Oct 2026 13:56:14 -0700 Subject: [PATCH 23/25] fix(review): verify smoke behavior instead of call totals --- tests/infra/test_e2e_harness.py | 34 +++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index 623ad6c905..06d6792e25 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -685,8 +685,38 @@ def test_success_verifies_exact_closed_pr_ref_complete_diff_and_canary( assert [call.input for call in runner.calls if call.input] == [SANDBOX_TOKEN] assert MINIMAX_KEY not in (out / "smoke-lifecycle.json").read_text(encoding="utf-8") assert SANDBOX_TOKEN not in (out / "smoke-evidence.json").read_text(encoding="utf-8") - assert sum(call.argv[:2] == ["git", "show"] for call in runner.calls) == 2 - assert sum(call.argv[:3] == ["gh", "pr", "list"] for call in runner.calls) == 2 + verified_sources = { + call.argv[2] for call in runner.calls if call.argv[:2] == ["git", "show"] + } + assert verified_sources >= { + f"{evidence['head_commit']}:sandbox/text.py", + f"{evidence['head_commit']}:tests/test_smoke_canary.py", + } + + def test_pr_head_changed_during_verification_is_rejected( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + runner, _clone = _smoke_runner(tmp_path, monkeypatch) + original_handler = runner.handler + listed = False + + def changed_head(argv: list[str]): + nonlocal listed + response = original_handler(argv) + if argv[:3] == ["gh", "pr", "list"]: + assert isinstance(response, subprocess.CompletedProcess) + if listed: + rows = json.loads(response.stdout) + rows[0]["headRefOid"] = "d" * 40 + response = _completed(argv, json.dumps(rows)) + listed = True + return response + + monkeypatch.setattr(runner, "handler", changed_head) + failures, result, _out, _catalog = _run_smoke(tmp_path, runner, monkeypatch) + + assert "smoke: PR head changed during verification" in failures + assert result["outcome"] == "failed" @pytest.mark.parametrize( "fault", From 426ca820355438e7ff0764901891810277b603aa Mon Sep 17 00:00:00 2001 From: Trecek Date: Tue, 6 Oct 2026 13:56:45 -0700 Subject: [PATCH 24/25] fix(review): bound git fixture setup subprocess --- tests/infra/test_e2e_harness.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index 06d6792e25..3b5b5e9f9e 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -185,7 +185,13 @@ def _smoke_case(expected_failures=None): def _run_git(cwd: Path, *argv: str, env: dict[str, str]) -> str: result = subprocess.run( - ["git", *argv], cwd=cwd, env=env, capture_output=True, text=True, check=True + ["git", *argv], + cwd=cwd, + env=env, + capture_output=True, + text=True, + check=True, + timeout=harness.SETUP_COMMAND_TIMEOUT_SEC, ) return result.stdout.strip() From 7fd9a554531f2914ed7f14702236095005826f27 Mon Sep 17 00:00:00 2001 From: Trecek Date: Tue, 6 Oct 2026 14:32:35 -0700 Subject: [PATCH 25/25] fix(ci): isolate smoke cleanup sleep mocks from subprocess --- tests/infra/test_e2e_harness.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/tests/infra/test_e2e_harness.py b/tests/infra/test_e2e_harness.py index 3b5b5e9f9e..1670cc60c2 100644 --- a/tests/infra/test_e2e_harness.py +++ b/tests/infra/test_e2e_harness.py @@ -11,6 +11,7 @@ from collections.abc import Callable from dataclasses import dataclass, replace from pathlib import Path +from types import SimpleNamespace import pytest @@ -1078,7 +1079,19 @@ def test_real_bare_remote_cleanup_preserves_unrelated_branch_and_expected_failur runner, state = _smoke_cleanup_runner(remote, branch, catalog.sandbox_repository) env = _env(HOME=git_env["HOME"], PATH=git_env["PATH"], E2E_SANDBOX_TOKEN=SANDBOX_TOKEN) sleeps: list[float] = [] - monkeypatch.setattr(harness.time, "sleep", sleeps.append) + monkeypatch.setattr(harness, "time", SimpleNamespace(sleep=sleeps.append)) + + with subprocess.Popen( + [harness.sys.executable, "-c", "import sys; sys.stdin.read()"], + stdin=subprocess.PIPE, + ) as process: + try: + with pytest.raises(subprocess.TimeoutExpired): + process.wait(timeout=0.05) + finally: + process.kill() + process.wait(timeout=5) + assert len(sleeps) == 0 status = harness.cleanup_smoke_test(catalog, test.name, runner, out, env) result = json.loads((out / "result.json").read_text(encoding="utf-8")) @@ -1117,7 +1130,7 @@ def test_cleanup_deletes_a_pushed_branch_even_without_a_pull_request( ) env = _env(HOME=git_env["HOME"], PATH=git_env["PATH"], E2E_SANDBOX_TOKEN=SANDBOX_TOKEN) sleeps: list[float] = [] - monkeypatch.setattr(harness.time, "sleep", sleeps.append) + monkeypatch.setattr(harness, "time", SimpleNamespace(sleep=sleeps.append)) assert harness.cleanup_smoke_test(catalog, test.name, runner, out, env) == 0 @@ -1142,7 +1155,7 @@ def test_cleanup_errors_fail_and_attempt_the_other_resource_operation( remote, branch, catalog.sandbox_repository, fault=fault ) env = _env(HOME=git_env["HOME"], PATH=git_env["PATH"], E2E_SANDBOX_TOKEN=SANDBOX_TOKEN) - monkeypatch.setattr(harness.time, "sleep", lambda _duration: None) + monkeypatch.setattr(harness, "time", SimpleNamespace(sleep=lambda _duration: None)) status = harness.cleanup_smoke_test(catalog, test.name, runner, out, env) result = json.loads((out / "result.json").read_text(encoding="utf-8"))