From 3051acdb5f000f2720ff6340f18d064564a67b9a Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 17 Aug 2026 12:41:32 -0700 Subject: [PATCH 01/11] refactor(backends): decompose codex.py into 4 modules --- .../execution/backends/_codex_cmd_builders.py | 413 ++++++ .../backends/_codex_explorer_projection.py | 599 ++++++++ .../execution/backends/_codex_probes.py | 376 +++++ src/autoskillit/execution/backends/codex.py | 1315 +---------------- tests/arch/test_subpackage_isolation.py | 67 +- 5 files changed, 1445 insertions(+), 1325 deletions(-) create mode 100644 src/autoskillit/execution/backends/_codex_cmd_builders.py create mode 100644 src/autoskillit/execution/backends/_codex_explorer_projection.py create mode 100644 src/autoskillit/execution/backends/_codex_probes.py diff --git a/src/autoskillit/execution/backends/_codex_cmd_builders.py b/src/autoskillit/execution/backends/_codex_cmd_builders.py new file mode 100644 index 000000000..09acc5c3a --- /dev/null +++ b/src/autoskillit/execution/backends/_codex_cmd_builders.py @@ -0,0 +1,413 @@ +"""Codex command builders, flag vocabulary, env policy, state probe, and session locator. + +These are the supporting types for the Codex backend (`codex.py`). The backend +itself owns the cmd/cmd-spec grammar; this module holds the cross-cutting +vocabulary, the env-builder dataclass, the state-readiness probe, and the +session locator so the backend file can stay focused on command construction. +""" + +from __future__ import annotations + +import json +import math +import sqlite3 +import stat +import time +from collections.abc import Callable, Mapping, Sequence +from dataclasses import dataclass, field +from enum import StrEnum, unique +from pathlib import Path + +import zstandard + +from autoskillit.core import ( + AGENT_BACKEND_CODEX, + AGENT_BACKEND_DYNACONF_ENV_VAR, + AGENT_BACKEND_ENV_VAR, + AUDIT_ADMISSION_AUTHORITY_PATH_ENV_VAR, + AUTOSKILLIT_APPLICABLE_GUARDS, + AUTOSKILLIT_PRIVATE_ENV_VARS, + AUTOSKILLIT_STATE_ROOT_ENV_VAR, + AUTOSKILLIT_WRITE_GUARD_TOOL_NAMES, + CODEX_SESSIONS_SUBDIR, + CODEX_STARTUP_TRACE_ENV_VAR, + FLEET_INSPECTOR_MODEL_ENV_VAR, + FOOD_TRUCK_TOOL_TAGS_ENV_VAR, + LAUNCH_ID_ENV_VAR, + MCP_CLIENT_BACKEND_ENV_VAR, + HookTrustPolicy, + ObserverStatus, + SessionLocator, + SessionSummary, + default_log_dir, + get_logger, +) +from autoskillit.execution.backends._backend_cmd_builder_base import ( + SHARED_BASELINE_ENV, + _filter_protected_native_shell_env, +) +from autoskillit.execution.backends._codex_session_storage import CodexSessionStore + +logger = get_logger(__name__) + + +@unique +class CodexFlags(StrEnum): + JSON = "--json" + SANDBOX = "--sandbox" + MODEL = "--model" + MODEL_SHORT = "-m" + ADD_DIR = "--add-dir" + RESUME_SUBCOMMAND = "resume" + CONFIG_OVERRIDE = "-c" + PROFILE = "--profile" + DANGEROUSLY_BYPASS = "--dangerously-bypass-approvals-and-sandbox" + DANGEROUSLY_BYPASS_HOOK_TRUST = "--dangerously-bypass-hook-trust" + + +CODEX_EXEC_FLAGS: frozenset[str] = frozenset( + { + CodexFlags.JSON, + CodexFlags.SANDBOX, + CodexFlags.MODEL, + CodexFlags.CONFIG_OVERRIDE, + CodexFlags.ADD_DIR, + CodexFlags.DANGEROUSLY_BYPASS_HOOK_TRUST, + } +) + +CODEX_TOP_LEVEL_ONLY_FLAGS: frozenset[str] = frozenset( + { + CodexFlags.DANGEROUSLY_BYPASS, + CodexFlags.MODEL_SHORT, + CodexFlags.PROFILE, + } +) + +VARIADIC_CODEX_FLAGS: frozenset[str] = frozenset({CodexFlags.ADD_DIR, CodexFlags.CONFIG_OVERRIDE}) + +NON_VARIADIC_CODEX_FLAGS: frozenset[str] = frozenset( + { + CodexFlags.JSON, + CodexFlags.SANDBOX, + CodexFlags.MODEL, + CodexFlags.MODEL_SHORT, + CodexFlags.PROFILE, + CodexFlags.RESUME_SUBCOMMAND, + CodexFlags.DANGEROUSLY_BYPASS, + CodexFlags.DANGEROUSLY_BYPASS_HOOK_TRUST, + } +) + + +CODEX_ENV_DENYLIST: frozenset[str] = frozenset( + { + "ANTHROPIC_API_KEY", + "ANTHROPIC_AUTH_TOKEN", + "ANTHROPIC_BASE_URL", + "CLAUDE_STREAM_IDLE_TIMEOUT_MS", + } +) + +CODEX_ENV_PREFIX_DENYLIST: tuple[str, ...] = ("CLAUDE_CODE_",) + +_IMAGE_GENERATION_DISABLED = "features.image_generation=false" + + +def _codex_exec_base( + *, + sandbox: str | None, + json: bool = True, + extra_overrides: Sequence[str] = (), + bypass_hook_trust: bool = False, +) -> list[str]: + cmd: list[str] = ["codex", "exec"] + if json: + cmd.append(CodexFlags.JSON) + if sandbox is not None: + cmd.extend([CodexFlags.SANDBOX, sandbox]) + for override in extra_overrides: + cmd.extend([CodexFlags.CONFIG_OVERRIDE, override]) + cmd.extend([CodexFlags.CONFIG_OVERRIDE, _IMAGE_GENERATION_DISABLED]) + if bypass_hook_trust: + # Hook trust is independent from the sandbox selected by config/CLI. + cmd.append(CodexFlags.DANGEROUSLY_BYPASS_HOOK_TRUST) + return cmd + + +def _should_bypass_hook_trust( + policy: HookTrustPolicy, + *, + automated_session: bool, +) -> bool: + """Translate backend hook policy at the command-construction boundary.""" + if automated_session: + return True + match policy: + case HookTrustPolicy.AUTOMATED: + return True + case HookTrustPolicy.REVIEW_EACH_SESSION: + return False + raise AssertionError(f"Unhandled hook trust policy: {policy!r}") + + +_CODEX_STATE_READINESS_COMMIT = "ad65f016ed0c91992fb175fa881a373cc460dd2a" + + +@dataclass(frozen=True, slots=True) +class _StateReadinessDef: + database_name: str + upstream_commit: str + + +_SUPPORTED_STATE_CONTRACTS = { + "codex-cli 0.145.0": _StateReadinessDef( + database_name="state_5.sqlite", + upstream_commit=_CODEX_STATE_READINESS_COMMIT, + ) +} + + +@dataclass(frozen=True, slots=True) +class CodexStateReadinessProbe: + """Read the version-mapped disposable Codex state database without mutation.""" + + codex_version: str + sqlite_home: Path + poll_interval_seconds: float = 0.05 + _clock: Callable[[], float] = field(default=time.monotonic, repr=False) + _sleep: Callable[[float], None] = field(default=time.sleep, repr=False) + + def __post_init__(self) -> None: + if not math.isfinite(self.poll_interval_seconds) or self.poll_interval_seconds <= 0: + raise ValueError("poll_interval_seconds must be finite and positive") + object.__setattr__(self, "sqlite_home", Path(self.sqlite_home)) + + @property + def database_path(self) -> Path | None: + """Return the exact database path for a supported Codex version.""" + compatibility = _SUPPORTED_STATE_CONTRACTS.get(self.codex_version) + return None if compatibility is None else self.sqlite_home / compatibility.database_name + + @property + def upstream_commit(self) -> str | None: + """Return the source revision defining the probed schema contract.""" + compatibility = _SUPPORTED_STATE_CONTRACTS.get(self.codex_version) + return None if compatibility is None else compatibility.upstream_commit + + def check(self) -> ObserverStatus: + """Perform one zero-wait, read-only readiness observation.""" + database_path = self.database_path + if database_path is None: + return ObserverStatus.UNSUPPORTED_VERSION + try: + path_stat = database_path.lstat() + except FileNotFoundError: + return ObserverStatus.ABSENT + except OSError: + return ObserverStatus.CORRUPT + if not stat.S_ISREG(path_stat.st_mode): + return ObserverStatus.CORRUPT + + connection: sqlite3.Connection | None = None + try: + uri = f"{database_path.resolve(strict=True).as_uri()}?mode=ro" + connection = sqlite3.connect( + uri, + uri=True, + timeout=0.0, + isolation_level=None, + ) + connection.execute("PRAGMA query_only = ON") + connection.execute("PRAGMA busy_timeout = 0") + columns = { + row[1] + for row in connection.execute("PRAGMA table_info(backfill_state)") + if len(row) > 1 and isinstance(row[1], str) + } + if not {"id", "status"}.issubset(columns): + return ObserverStatus.SCHEMA_CHANGED + row = connection.execute("SELECT status FROM backfill_state WHERE id = 1").fetchone() + if row is None or len(row) != 1 or not isinstance(row[0], str): + return ObserverStatus.INCOMPLETE + return ObserverStatus.READY if row[0] == "complete" else ObserverStatus.INCOMPLETE + except sqlite3.OperationalError as exc: + message = str(exc).lower() + if "locked" in message or "busy" in message: + return ObserverStatus.LOCKED + if "no such table" in message or "no such column" in message: + return ObserverStatus.SCHEMA_CHANGED + return ObserverStatus.CORRUPT + except (OSError, sqlite3.DatabaseError, ValueError): + return ObserverStatus.CORRUPT + finally: + if connection is not None: + connection.close() + + def wait( + self, + *, + timeout_seconds: float, + cancelled: Callable[[], bool] | None = None, + ) -> ObserverStatus: + """Poll until ready, a terminal adapter failure, timeout, or cancellation.""" + if not math.isfinite(timeout_seconds) or timeout_seconds < 0: + raise ValueError("timeout_seconds must be finite and non-negative") + is_cancelled = cancelled or (lambda: False) + deadline = self._clock() + timeout_seconds + while True: + if is_cancelled(): + return ObserverStatus.CANCELLED + if self._clock() >= deadline: + return ObserverStatus.TIMEOUT + status = self.check() + if status is ObserverStatus.READY: + return status + if status in { + ObserverStatus.CORRUPT, + ObserverStatus.SCHEMA_CHANGED, + ObserverStatus.UNSUPPORTED_VERSION, + }: + return status + remaining = deadline - self._clock() + if remaining <= 0: + return ObserverStatus.TIMEOUT + self._sleep(min(self.poll_interval_seconds, remaining)) + + +def _codex_exec_extras( + *, + session_type: str, + include_session_baseline: bool = False, + include_agent_backend_flat: bool = False, + applicable_guards: frozenset[str] | None = None, + write_guard_tool_names: frozenset[str] | None = None, +) -> dict[str, str]: + extras: dict[str, str] = {} + if include_session_baseline: + extras.update(SHARED_BASELINE_ENV) + extras.update( + { + "AUTOSKILLIT_HEADLESS": "1", + "AUTOSKILLIT_HEADLESS_AUTO_GATE": "1", + "AUTOSKILLIT_SESSION_TYPE": session_type, + AGENT_BACKEND_DYNACONF_ENV_VAR: AGENT_BACKEND_CODEX, + MCP_CLIENT_BACKEND_ENV_VAR: AGENT_BACKEND_CODEX, + FLEET_INSPECTOR_MODEL_ENV_VAR: "", + FOOD_TRUCK_TOOL_TAGS_ENV_VAR: "", + } + ) + extras.setdefault(LAUNCH_ID_ENV_VAR, "") + extras.setdefault(AUTOSKILLIT_STATE_ROOT_ENV_VAR, "") + if include_agent_backend_flat: + extras[AGENT_BACKEND_ENV_VAR] = AGENT_BACKEND_CODEX + if applicable_guards is not None: + extras[AUTOSKILLIT_APPLICABLE_GUARDS] = ",".join(sorted(applicable_guards)) + if write_guard_tool_names is not None: + extras[AUTOSKILLIT_WRITE_GUARD_TOOL_NAMES] = ",".join(sorted(write_guard_tool_names)) + return extras + + +@dataclass(frozen=True, slots=True) +class CodexEnvPolicy: + denylist_prefixes: tuple[str, ...] = CODEX_ENV_PREFIX_DENYLIST + + def build_env( + self, + base_env: Mapping[str, str], + *, + extras: Mapping[str, str] | None = None, + required: frozenset[str] | None = None, + ) -> dict[str, str]: + out: dict[str, str] = { + k: v + for k, v in base_env.items() + if k not in CODEX_ENV_DENYLIST + and k not in AUTOSKILLIT_PRIVATE_ENV_VARS + and not any(k.startswith(p) for p in self.denylist_prefixes) + } + if extras is not None: + filtered_extras = _filter_protected_native_shell_env(extras) + filtered_extras.setdefault("AUTOSKILLIT_SKILL_NAME", "") + out.update( + (key, value) + for key, value in filtered_extras.items() + if key != CODEX_STARTUP_TRACE_ENV_VAR + ) + out.setdefault(AUDIT_ADMISSION_AUTHORITY_PATH_ENV_VAR, "") # Outer-cook control only. + out.pop(CODEX_STARTUP_TRACE_ENV_VAR, None) + if required is not None: + missing = required - frozenset(out) + if missing: + raise ValueError(f"Required env vars missing from session env: {sorted(missing)}") + return out + + +@dataclass(frozen=True, slots=True) +class CodexSessionLocator(SessionLocator): + store_root: Path | None = None + index_path: Path | None = None + + def _store(self) -> CodexSessionStore: + return CodexSessionStore( + log_dir=self.store_root or default_log_dir(), + index_path=self.index_path, + ) + + def locate_session(self, session_id: str) -> Path | None: + if not session_id or session_id.startswith(("no_session_", "crashed_")): + return None + return self._store().locate_session(session_id) + + def read_session(self, path: Path) -> list[dict]: + """Read and parse a Codex session log file. + + Handles both plain .jsonl (current Codex v0.133.0+) and + .jsonl.zst (legacy) formats based on file extension. + """ + try: + if path.name.endswith(".zst"): + raw = path.read_bytes() + decompressed = zstandard.ZstdDecompressor().decompress(raw) + text = decompressed.decode("utf-8") + else: + text = path.read_text(encoding="utf-8") + except Exception: + logger.warning("read_session: failed to read", path=str(path), exc_info=True) + return [] + result: list[dict] = [] + for line in text.splitlines(): + if not line: + continue + try: + obj = json.loads(line) + except json.JSONDecodeError: + continue + if isinstance(obj, dict): + result.append(obj) + return result + + def project_log_dir(self, cwd: str) -> Path: # cwd unused; Codex uses a global session store + return (self.store_root or default_log_dir()) / CODEX_SESSIONS_SUBDIR + + def session_log_path(self, cwd: str, session_id: str) -> Path | None: + if not session_id or session_id.startswith(("no_session_", "crashed_")): + return None + return self.locate_session(session_id) + + def list_sessions(self, cwd: str) -> tuple[SessionSummary, ...]: + return self._store().read_index(cwd) + + +__all__ = [ + "CODEX_EXEC_FLAGS", + "CODEX_ENV_DENYLIST", + "CODEX_ENV_PREFIX_DENYLIST", + "CODEX_TOP_LEVEL_ONLY_FLAGS", + "CodexEnvPolicy", + "CodexFlags", + "CodexSessionLocator", + "CodexStateReadinessProbe", + "NON_VARIADIC_CODEX_FLAGS", + "VARIADIC_CODEX_FLAGS", +] diff --git a/src/autoskillit/execution/backends/_codex_explorer_projection.py b/src/autoskillit/execution/backends/_codex_explorer_projection.py new file mode 100644 index 000000000..ca13b8ee2 --- /dev/null +++ b/src/autoskillit/execution/backends/_codex_explorer_projection.py @@ -0,0 +1,599 @@ +"""Codex explorer role projection — agent toml rendering, registration, refresh, clear. + +Extracted from `codex.py`. The backend file is focused on cmd/cmd-spec grammar; +this module owns the projection, registration, and atomic-replacement mechanics +for the bundled explorer role set (BUNDLED_EXPLORER_ROLES) and the +parent config that fronts them. + +This file is unrelated to `execution/backends/_codex/explorer_projection.py` +which holds the role-transport primitives (e.g. `_canonical_explorer_mcp_transport`). +That module is imported here. +""" + +from __future__ import annotations + +import os +import shutil +import tempfile +import tomllib +from collections.abc import Mapping +from pathlib import Path + +from autoskillit.core import ( + BUNDLED_EXPLORER_ROLES, + CODEX_EFFORT_MAPPING, + CODEX_MODEL_ALIASES, + WEB_EVIDENCE_RESEARCHER_ROLE, + AgentDef, + SkillExecutionRole, + agent_definition_digest, + atomic_write, + get_logger, + load_bundled_agent_definitions, +) +from autoskillit.execution.backends import _codex_config as _codex_cfg +from autoskillit.execution.backends._claude_prompt import codex_discipline_suffix +from autoskillit.execution.backends._codex.explorer_projection import ( + _EXPLORER_ROLE_NAMES, + _canonical_explorer_mcp_transport, + _direct_agent_mcp_tools, + _explorer_mcp_projection, + _render_direct_role_mcp_lines, + _render_parent_explorer_config, + _render_role_mcp_lines, + _resolve_role_mcp_transport, + _validated_explorer_binding_env, + _validated_explorer_binding_envs, +) +from autoskillit.execution.backends._codex_config import ( + _CODEX_AGENT_NAME_COLLISIONS, + _format_toml_value, +) + +logger = get_logger(__name__) + + +def _bundled_agent_definitions() -> tuple[AgentDef, ...]: + return load_bundled_agent_definitions() + + +def _canonical_codex_model_effort( + model_class: str | None, + reasoning_effort: str | None = None, +) -> tuple[str, str | None]: + if model_class is None: + return "", reasoning_effort + model = CODEX_MODEL_ALIASES[model_class] + return model, reasoning_effort or CODEX_EFFORT_MAPPING.get(model_class) + + +CODEX_SPAWNABLE_BUILT_IN_AGENT_NAMES = _codex_cfg.CODEX_SPAWNABLE_BUILT_IN_AGENT_NAMES + + +def _preflight_agent_projection( + session_dir: Path, + definitions: tuple[AgentDef, ...], + *, + exact_definitions: bool, +) -> tuple[AgentDef, ...]: + """Validate the complete role set and select roles safe to project.""" + names = tuple(definition.name for definition in definitions) + duplicates = sorted({name for name in names if names.count(name) > 1}) + if duplicates: + raise ValueError(f"duplicate Codex agent definitions: {duplicates}") + built_in_collisions = sorted(set(names) & _CODEX_AGENT_NAME_COLLISIONS) + if built_in_collisions: + raise ValueError(f"Codex built-in agent name collision: {built_in_collisions}") + config_path = session_dir / "config.toml" + config = tomllib.loads(config_path.read_text(encoding="utf-8")) + if exact_definitions and any(map(_direct_agent_mcp_tools, definitions)): + _canonical_explorer_mcp_transport(config_path) + configured_agents = config.get("agents", {}) + if not isinstance(configured_agents, dict): + raise ValueError("Codex config agents table must be a mapping") + protected_names = ( + set(names) + if exact_definitions + else {*BUNDLED_EXPLORER_ROLES, WEB_EVIDENCE_RESEARCHER_ROLE} + ) + ambient_collisions = sorted(set(names) & set(configured_agents) & protected_names) + if ambient_collisions: + raise ValueError(f"ambient Codex agent name collision: {ambient_collisions}") + + agents_dir = session_dir / "agents" + if agents_dir.exists() and not agents_dir.is_dir(): + raise ValueError(f"Codex agents path is not a directory: {agents_dir}") + artifact_collisions = sorted( + definition.name + for definition in definitions + if (agents_dir / f"{definition.name}.toml").exists() + ) + if artifact_collisions: + raise ValueError(f"ambient Codex agent artifact collision: {artifact_collisions}") + return tuple( + definition for definition in definitions if definition.name not in configured_agents + ) + + +def _render_agent_toml( + definition: AgentDef, + *, + explorer_binding_env: Mapping[str, str] | None = None, + explorer_mcp_transport: Mapping[str, object] | None = None, + project_explorer_mcp: bool = False, +) -> str: + """Render and parse one role before its output directory is touched.""" + direct_mcp_tools = _direct_agent_mcp_tools(definition) + digest = agent_definition_digest(definition) + lines = [ + f"name = {_format_toml_value(definition.name)}", + f"description = {_format_toml_value(definition.description)}", + f"sandbox_mode = {_format_toml_value(definition.codex.sandbox_mode)}", + ] + if definition.codex.model is not None: + lines.append(f"model = {_format_toml_value(definition.codex.model)}") + if definition.codex.reasoning_effort is not None: + lines.append( + f"model_reasoning_effort = {_format_toml_value(definition.codex.reasoning_effort)}" + ) + if definition.codex.web_search is not None: + lines.append(f"web_search = {_format_toml_value(definition.codex.web_search)}") + body = ( + f"{definition.body}\n\n" + f"AutoSkillit agent definition digest: {digest}\n\n" + f"{codex_discipline_suffix()}" + ) + lines.append(f"instructions = '''\n{body}\n'''") + lines.append(f"developer_instructions = '''\n{body}\n'''") + if definition.codex.disabled_features: + lines.append("[features]") + lines.extend(f"{feature} = false" for feature in definition.codex.disabled_features) + if not definition.codex.agents_enabled: + lines.extend(("[agents]", "enabled = false")) + if explorer_binding_env is not None and not project_explorer_mcp: + raise ValueError("an explorer binding requires an explorer MCP projection") + if explorer_mcp_transport is not None and not project_explorer_mcp and not direct_mcp_tools: + raise ValueError("an explorer MCP transport requires an explorer MCP projection") + if project_explorer_mcp: + if explorer_mcp_transport is None: + raise ValueError("an explorer MCP projection requires a canonical transport") + projection = _explorer_mcp_projection( + explorer_mcp_transport, + explorer_binding_env, + ) + lines.extend(_render_role_mcp_lines(projection, explorer_binding_env)) + elif direct_mcp_tools: + lines.extend(_render_direct_role_mcp_lines(explorer_mcp_transport, direct_mcp_tools)) + rendered = "\n".join(lines) + "\n" + tomllib.loads(rendered) + return rendered + + +def _eligible_agent_definitions( + definitions: tuple[AgentDef, ...], + bindings: Mapping[str, Mapping[str, str]], + *, + exact: bool, +) -> tuple[AgentDef, ...]: + definitions = tuple(d for d in definitions if not d.reader_tools) + if exact: + return definitions + return tuple( + definition + for definition in definitions + if definition.name not in BUNDLED_EXPLORER_ROLES or definition.name in bindings + ) + + +def _generate_agent_tomls( + session_dir: Path, + agent_defs: tuple[AgentDef, ...] | None = None, + *, + explorer_binding_envs: Mapping[str, Mapping[str, str]] | None = None, + explorer_mcp_transport: Mapping[str, object] | None = None, +) -> int: + definitions = _bundled_agent_definitions() if agent_defs is None else agent_defs + bindings = explorer_binding_envs or {} + eligible = _eligible_agent_definitions( + definitions, + bindings, + exact=agent_defs is not None, + ) + direct_mcp_transport = _resolve_role_mcp_transport( + session_dir, eligible, bindings, explorer_mcp_transport + ) + rendered = { + definition.name: _render_agent_toml( + definition, + explorer_binding_env=bindings.get(definition.name), + explorer_mcp_transport=( + direct_mcp_transport + if definition.name in bindings or _direct_agent_mcp_tools(definition) + else None + ), + project_explorer_mcp=definition.name in bindings, + ) + for definition in eligible + } + out_dir = session_dir / "agents" + out_dir.mkdir(exist_ok=True) + for definition in eligible: + toml_path = out_dir / f"{definition.name}.toml" + atomic_write(toml_path, rendered[definition.name]) + logger.debug("codex_agents_generated", count=len(eligible), dest=str(out_dir)) + return len(eligible) + + +def _register_agent_tomls( + session_dir: Path, + agent_defs: tuple[AgentDef, ...] | None = None, + *, + explorer_binding_envs: Mapping[str, Mapping[str, str]] | None = None, +) -> int: + config_path = session_dir / "config.toml" + config_text = config_path.read_text(encoding="utf-8") + tomllib.loads(config_text) + registrations: list[str] = [] + definitions = _bundled_agent_definitions() if agent_defs is None else agent_defs + bindings = explorer_binding_envs or {} + eligible = _eligible_agent_definitions( + definitions, + bindings, + exact=agent_defs is not None, + ) + for definition in eligible: + agent_path = session_dir / "agents" / f"{definition.name}.toml" + agent = tomllib.loads(agent_path.read_text(encoding="utf-8")) + if agent.get("name") != definition.name: + raise ValueError(f"generated agent identity mismatch: {agent_path}") + registrations.extend( + [ + f"[agents.{_format_toml_value(definition.name)}]", + f"description = {_format_toml_value(definition.description)}", + f"config_file = {_format_toml_value(f'agents/{agent_path.name}')}", + "", + ] + ) + if not registrations: + return 0 + separator = "\n" if config_text.endswith("\n") else "\n\n" + registration_text = "\n".join(registrations) + updated = f"{config_text}{separator}{registration_text}" + tomllib.loads(updated) + atomic_write(config_path, updated) + return len(registrations) // 4 + + +def _validate_existing_explorer_role_toml( + toml_path: Path, + definition: AgentDef, + *, + require_binding_env: bool, + explorer_mcp_transport: Mapping[str, object], +) -> dict[str, str] | None: + """Validate persisted role identity and recover its binding environment.""" + try: + current = tomllib.loads(toml_path.read_text(encoding="utf-8")) + except (OSError, tomllib.TOMLDecodeError) as exc: + raise ValueError(f"invalid materialized explorer role {toml_path}: {exc}") from exc + servers = current.get("mcp_servers") + if not isinstance(servers, dict) or set(servers) != {"autoskillit"}: + raise ValueError(f"materialized explorer role missing MCP projection: {toml_path}") + current_server = servers.get("autoskillit") + if not isinstance(current_server, dict): + raise ValueError(f"materialized explorer role missing MCP projection: {toml_path}") + current_server = dict(current_server) + current_env = current_server.pop("env", None) + expected_server = _explorer_mcp_projection(explorer_mcp_transport, None) + if current_server != expected_server: + raise ValueError(f"materialized explorer role has a divergent MCP projection: {toml_path}") + if current.get("name") != definition.name: + raise ValueError(f"materialized explorer role identity mismatch: {toml_path}") + if current_env is None and not require_binding_env: + return None + if not isinstance(current_env, dict): + raise ValueError( + f"materialized explorer role has an invalid binding environment: {toml_path}" + ) + return _validated_explorer_binding_env(definition.name, current_env) + + +def _validate_existing_parent_explorer_projection( + session_config: Mapping[str, object], + *, + require_binding_env: bool, + explorer_mcp_transport: Mapping[str, object], +) -> dict[str, str] | None: + """Validate the parent half of the shared-principal MCP projection.""" + if session_config.get("sandbox_mode") != "read-only": + raise ValueError("materialized explorer parent must be read-only") + servers = session_config.get("mcp_servers") + if not isinstance(servers, dict) or set(servers) != {"autoskillit"}: + raise ValueError("materialized explorer parent must configure exactly one MCP server") + current_server = servers.get("autoskillit") + if not isinstance(current_server, dict): + raise ValueError("materialized explorer parent is missing its MCP projection") + current_server = dict(current_server) + current_env = current_server.pop("env", None) + expected_server = _explorer_mcp_projection(explorer_mcp_transport, None) + if current_server != expected_server: + raise ValueError("materialized explorer parent has a divergent MCP projection") + if current_env is None and not require_binding_env: + return None + if not isinstance(current_env, dict): + raise ValueError("materialized explorer parent has an invalid binding environment") + return _validated_explorer_binding_env("parent", current_env) + + +def _validate_materialized_explorer_roles( + session_dir: Path, + definitions: tuple[AgentDef, ...], + roles: frozenset[str], + *, + require_binding_env: bool, +) -> tuple[dict[str, AgentDef], dict[str, object], str]: + """Validate registered persisted explorer artifacts before a grouped rewrite.""" + if any(type(role) is not str for role in roles): + raise ValueError("explorer role cleanup set must contain only text names") + definitions_by_name = {definition.name: definition for definition in definitions} + if roles != _EXPLORER_ROLE_NAMES or not roles <= set(definitions_by_name): + raise ValueError(f"unknown explorer roles: {sorted(roles - set(definitions_by_name))}") + agents_dir = session_dir / "agents" + if not agents_dir.is_dir(): + raise ValueError(f"materialized Codex agents directory is missing: {agents_dir}") + config_path = session_dir / "config.toml" + try: + config_text = config_path.read_text(encoding="utf-8") + session_config = tomllib.loads(config_text) + except (OSError, tomllib.TOMLDecodeError) as exc: + raise ValueError(f"invalid materialized Codex config: {exc}") from exc + registered_agents = session_config.get("agents") + if not isinstance(registered_agents, dict): + raise ValueError("materialized Codex config has no agent registrations") + explorer_mcp_transport = _canonical_explorer_mcp_transport(config_path) + projected_bindings = [ + _validate_existing_parent_explorer_projection( + session_config, + require_binding_env=require_binding_env, + explorer_mcp_transport=explorer_mcp_transport, + ) + ] + + selected: dict[str, AgentDef] = {} + for role in sorted(roles): + definition = definitions_by_name[role] + registration = registered_agents.get(role) + expected_path = f"agents/{role}.toml" + if not isinstance(registration, dict) or registration.get("config_file") != expected_path: + raise ValueError( + f"materialized Codex config has no canonical registration for {role!r}" + ) + projected_bindings.append( + _validate_existing_explorer_role_toml( + agents_dir / f"{role}.toml", + definition, + require_binding_env=require_binding_env, + explorer_mcp_transport=explorer_mcp_transport, + ) + ) + selected[role] = definition + if any(binding != projected_bindings[0] for binding in projected_bindings[1:]): + raise ValueError("materialized explorer bindings diverge from the shared principal") + return selected, explorer_mcp_transport, config_text + + +def _atomically_replace_explorer_projection( + session_dir: Path, + rendered_config: str, + rendered_roles: Mapping[str, str], +) -> None: + """Swap the parent and both roles as one staged session-root transaction.""" + stage_root = Path( + tempfile.mkdtemp(prefix=".autoskillit-explorer-refresh-", dir=session_dir.parent) + ) + staged_session = stage_root / "session" + backup_session = stage_root / "previous-session" + moved_original = False + try: + shutil.copytree(session_dir, staged_session, symlinks=True) + atomic_write(staged_session / "config.toml", rendered_config) + for role, content in rendered_roles.items(): + atomic_write(staged_session / "agents" / f"{role}.toml", content) + os.replace(session_dir, backup_session) + moved_original = True + try: + os.replace(staged_session, session_dir) + except OSError as install_error: + try: + os.replace(backup_session, session_dir) + except OSError as restore_error: + raise restore_error from install_error + moved_original = False + raise + moved_original = False + finally: + if not moved_original: + shutil.rmtree(stage_root, ignore_errors=True) + + +def refresh_explorer_binding_env( + session_dir: Path, + explorer_binding_env: Mapping[str, Mapping[str, str]], +) -> None: + """Atomically replace only server-issued explorer binding values on resume. + + The helper validates the persisted parent and both definition-derived role + layers before staging a replacement session root, so a failed refresh + cannot leave any of the three configs on a different principal. + """ + definitions = _bundled_agent_definitions() + binding_envs = _validated_explorer_binding_envs(definitions, explorer_binding_env) + if not binding_envs: + return + + definitions_by_name, explorer_mcp_transport, config_text = ( + _validate_materialized_explorer_roles( + session_dir, + definitions, + frozenset(binding_envs), + require_binding_env=True, + ) + ) + shared_binding = next(iter(binding_envs.values())) + rendered_config = _render_parent_explorer_config( + config_text, + explorer_mcp_transport=explorer_mcp_transport, + explorer_binding_env=shared_binding, + ) + rendered_roles: dict[str, str] = {} + for role, definition in definitions_by_name.items(): + rendered_roles[role] = _render_agent_toml( + definition, + explorer_binding_env=shared_binding, + explorer_mcp_transport=explorer_mcp_transport, + project_explorer_mcp=True, + ) + _atomically_replace_explorer_projection( + session_dir, + rendered_config, + rendered_roles, + ) + + +def clear_explorer_binding_env(session_dir: Path, roles: frozenset[str]) -> None: + """Atomically scrub persisted explorer secrets while retaining the broker allowlist.""" + if not isinstance(roles, frozenset): + raise ValueError("explorer role cleanup set must be a frozenset") + if not roles: + return + definitions = _bundled_agent_definitions() + definitions_by_name, explorer_mcp_transport, config_text = ( + _validate_materialized_explorer_roles( + session_dir, + definitions, + roles, + require_binding_env=False, + ) + ) + rendered_config = _render_parent_explorer_config( + config_text, + explorer_mcp_transport=explorer_mcp_transport, + explorer_binding_env=None, + ) + rendered_roles = { + role: _render_agent_toml( + definition, + explorer_mcp_transport=explorer_mcp_transport, + project_explorer_mcp=True, + ) + for role, definition in definitions_by_name.items() + } + _atomically_replace_explorer_projection( + session_dir, + rendered_config, + rendered_roles, + ) + + +def _render_parent_sandbox_config(config_text: str, sandbox_mode: str) -> str: + """Render the generated-home config with the normalized parent sandbox.""" + if sandbox_mode not in {"read-only", "workspace-write"}: + raise ValueError(f"unsupported parent sandbox mode: {sandbox_mode!r}") + lines = config_text.splitlines() + table_start = next( + (i for i, line in enumerate(lines) if line.lstrip().startswith("[")), len(lines) + ) + key_indexes = [ + i + for i, line in enumerate(lines[:table_start]) + if line.split("=", 1)[0].strip() == "sandbox_mode" + ] + if len(key_indexes) > 1: + raise ValueError("generated Codex config has duplicate top-level sandbox_mode keys") + if key_indexes: + del lines[key_indexes[0]] + if sandbox_mode == "read-only": + table_start = next( + (i for i, line in enumerate(lines) if line.lstrip().startswith("[")), len(lines) + ) + replacement = f"sandbox_mode = {_format_toml_value(sandbox_mode)}" + lines.insert(table_start, replacement) + updated = "\n".join(lines) + "\n" + parsed = tomllib.loads(updated) + if sandbox_mode == "read-only" and parsed.get("sandbox_mode") != sandbox_mode: + raise ValueError("generated Codex config did not retain the parent sandbox mode") + if sandbox_mode == "workspace-write" and "sandbox_mode" in parsed: + raise ValueError("generated Codex config retained a workspace-write sandbox pin") + return updated + + +def _render_cli_auth_store(config_text: str, execution_role: SkillExecutionRole) -> str: + """Pin ORCHESTRATOR homes to the durable file credential store.""" + if execution_role is not SkillExecutionRole.ORCHESTRATOR: + return config_text + lines = config_text.splitlines() + table_start = next( + (i for i, line in enumerate(lines) if line.lstrip().startswith("[")), len(lines) + ) + key_indexes = [ + i + for i, line in enumerate(lines[:table_start]) + if line.split("=", 1)[0].strip() == "cli_auth_credentials_store" + ] + if len(key_indexes) > 1: + raise ValueError( + "generated Codex config has duplicate top-level cli_auth_credentials_store keys" + ) + if key_indexes: + del lines[key_indexes[0]] + table_start -= 1 + lines.insert(table_start, 'cli_auth_credentials_store = "file"') + updated = "\n".join(lines) + "\n" + if tomllib.loads(updated).get("cli_auth_credentials_store") != "file": + raise ValueError("generated Codex config did not retain the file credential store") + return updated + + +def _materialize_profile_skills( + session_dir: Path, + *, + source_codex_home: Path | None = None, +) -> int: + """Symlink source-home profile skills into a generated Codex home. + + Scans the selected Codex home's ``skills`` for subdirectories containing + SKILL.md. Each is symlinked into session_dir/skills/. Falls back + to shutil.copytree if symlink creation fails. Subdirectories without + SKILL.md are skipped. Returns the number of skills materialized. + """ + source_home = Path.home() / ".codex" if source_codex_home is None else Path(source_codex_home) + profile_skills_root = source_home / "skills" + if not profile_skills_root.is_dir(): + return 0 + count = 0 + skills_base = session_dir / "skills" + skills_base.mkdir(parents=True, exist_ok=True) + entries = list(profile_skills_root.iterdir()) + for entry in entries: + if not entry.is_dir() or not (entry / "SKILL.md").is_file(): + continue + target = skills_base / entry.name + if target.exists() or target.is_symlink(): + continue + try: + target.symlink_to(entry.resolve()) + except OSError: + logger.debug( + "codex_profile_skill_symlink_failed_using_copytree", + skill=entry.name, + exc_info=True, + ) + shutil.copytree(entry, target) + count += 1 + return count + + +__all__ = [ + "clear_explorer_binding_env", + "refresh_explorer_binding_env", +] diff --git a/src/autoskillit/execution/backends/_codex_probes.py b/src/autoskillit/execution/backends/_codex_probes.py new file mode 100644 index 000000000..1bccc3c69 --- /dev/null +++ b/src/autoskillit/execution/backends/_codex_probes.py @@ -0,0 +1,376 @@ +"""Codex startup / validation probes — bounded subprocess + inventory + cache. + +Extracted from `codex.py` so the backend file can stay focused on the cmd/cmd-spec +grammar. These helpers run a small Codex subprocess to verify MCP server inventory +and confirm the generated home is wired correctly. Every helper is either +stateless or guarded by a module-level lock; the lock is the only module-level +state. +""" + +from __future__ import annotations + +import hashlib +import json +import os +import selectors +import subprocess +import threading +import time +import tomllib +from collections.abc import Mapping +from dataclasses import dataclass +from pathlib import Path +from typing import Any + +from autoskillit.core import CODEX_COOK_RESERVED_ENV_VARS, get_logger +from autoskillit.execution.backends._codex_cmd_builders import CodexFlags +from autoskillit.execution.backends._codex_config import _format_toml_value + +logger = get_logger(__name__) + + +_CODEX_PROBE_TIMEOUT_SECONDS = 15.0 +_CODEX_PROBE_STREAM_LIMIT = 64 * 1024 +_CODEX_VALIDATION_CACHE_LIMIT = 128 +_CODEX_VALIDATION_CACHE: dict[str, None] = {} +_CODEX_VALIDATION_CACHE_GUARD = threading.Lock() + + +@dataclass(frozen=True, slots=True) +class _BoundedProbeResult: + returncode: int | None + stdout: bytes + stderr: bytes + failure: str | None = None + + +def _terminate_probe(owner: object) -> None: + from autoskillit.execution.process._process_kill import OwnedProcessGroup + + if not isinstance(owner, OwnedProcessGroup): + raise TypeError("Codex probe cleanup requires its spawn-bound owner") + try: + owner.settle(timeout=2) + finally: + for stream in (owner.process.stdout, owner.process.stderr): + if stream is not None: + stream.close() + + +def _run_bounded_codex_probe( + command: tuple[str, ...], + *, + env: Mapping[str, str], + cwd: str, +) -> _BoundedProbeResult: + try: + from autoskillit.execution.process._process_kill import spawn_owned_process + + owner = spawn_owned_process( + command, + cwd=cwd, + env=dict(env), + stdin=subprocess.DEVNULL, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + start_new_session=True, + ) + process = owner.process + except OSError as exc: + return _BoundedProbeResult( + returncode=None, + stdout=b"", + stderr=b"", + failure=f"binary unavailable ({type(exc).__name__})", + ) + + selector: selectors.BaseSelector | None = None + output = {"stdout": bytearray(), "stderr": bytearray()} + deadline = time.monotonic() + _CODEX_PROBE_TIMEOUT_SECONDS + try: + assert process.stdout is not None + assert process.stderr is not None + selector_factory = selectors.DefaultSelector + selector = selector_factory() + selector.register(process.stdout, selectors.EVENT_READ, "stdout") + selector.register(process.stderr, selectors.EVENT_READ, "stderr") + while selector.get_map() or owner.observe_exit() is None: + remaining = deadline - time.monotonic() + if remaining <= 0: + _terminate_probe(owner) + return _BoundedProbeResult( + returncode=None, + stdout=bytes(output["stdout"]), + stderr=bytes(output["stderr"]), + failure="timed out", + ) + if not selector.get_map(): + time.sleep(min(0.01, remaining)) + continue + events = selector.select(timeout=min(0.1, remaining)) + for key, _ in events: + stream_name = key.data + try: + file_descriptor = ( + key.fileobj if isinstance(key.fileobj, int) else key.fileobj.fileno() + ) + chunk = os.read(file_descriptor, 8192) + except OSError: + chunk = b"" + if not chunk: + selector.unregister(key.fileobj) + continue + target = output[stream_name] + target.extend(chunk) + if len(target) > _CODEX_PROBE_STREAM_LIMIT: + del target[_CODEX_PROBE_STREAM_LIMIT:] + _terminate_probe(owner) + return _BoundedProbeResult( + returncode=None, + stdout=bytes(output["stdout"]), + stderr=bytes(output["stderr"]), + failure=f"{stream_name} exceeded {_CODEX_PROBE_STREAM_LIMIT} bytes", + ) + returncode, _cleanup = owner.settle(timeout=max(0.0, deadline - time.monotonic())) + except subprocess.TimeoutExpired: + _terminate_probe(owner) + return _BoundedProbeResult( + returncode=None, + stdout=bytes(output["stdout"]), + stderr=bytes(output["stderr"]), + failure="timed out while reaping", + ) + except BaseException as exc: + if process.returncode is None: + try: + _terminate_probe(owner) + except BaseException as cleanup_exc: + logger.error("codex_probe_cleanup_failed", exc_info=True) + exc.add_note(f"Codex probe cleanup failed: {type(cleanup_exc).__name__}") + raise + finally: + if selector is not None: + selector.close() + for stream in (process.stdout, process.stderr): + if stream is not None: + stream.close() + return _BoundedProbeResult( + returncode=returncode, + stdout=bytes(output["stdout"]), + stderr=bytes(output["stderr"]), + ) + + +def _probe_diagnostic(result: _BoundedProbeResult) -> str: + """Return bounded, non-content diagnostics safe for configs containing secrets.""" + stdout_digest = hashlib.sha256(result.stdout).hexdigest()[:16] + stderr_digest = hashlib.sha256(result.stderr).hexdigest()[:16] + return ( + f"stdout_bytes={len(result.stdout)} stdout_sha256={stdout_digest} " + f"stderr_bytes={len(result.stderr)} stderr_sha256={stderr_digest}" + ) + + +def _mcp_inventory_entries(document: Any) -> list[dict[str, Any]] | None: + if isinstance(document, list): + return [entry for entry in document if isinstance(entry, dict)] + if not isinstance(document, dict): + return None + for key in ("servers", "mcp_servers"): + value = document.get(key) + if isinstance(value, list): + return [entry for entry in value if isinstance(entry, dict)] + if isinstance(value, dict): + return [ + {"name": name, **entry} + for name, entry in value.items() + if isinstance(name, str) and isinstance(entry, dict) + ] + return None + + +def _string_array(value: Any) -> list[str] | None: + if not isinstance(value, list) or not all(isinstance(item, str) for item in value): + return None + return value + + +def _validate_codex_mcp_inventory(stdout: bytes, config_bytes: bytes) -> list[str]: + try: + document = json.loads(stdout.decode("utf-8")) + except (UnicodeDecodeError, json.JSONDecodeError): + return ["Codex MCP validation returned malformed JSON"] + try: + config = tomllib.loads(config_bytes.decode("utf-8")) + except (UnicodeDecodeError, tomllib.TOMLDecodeError): + return ["Final Codex config bytes are not valid UTF-8 TOML"] + + expected = config.get("mcp_servers", {}).get("autoskillit") + if not isinstance(expected, dict): + return ["Final Codex config is missing mcp_servers.autoskillit"] + entries = _mcp_inventory_entries(document) + if entries is None: + return ["Codex MCP validation JSON has no server inventory"] + matches = [entry for entry in entries if entry.get("name") == "autoskillit"] + if len(matches) != 1: + return [ + "Codex MCP validation expected exactly one enabled autoskillit server; " + f"found {len(matches)}" + ] + actual = matches[0] + if actual.get("enabled") is False: + return ["Codex MCP validation reports autoskillit as disabled"] + transport = actual.get("transport") + if not isinstance(transport, dict): + transport = actual + errors: list[str] = [] + if transport.get("type", "stdio") != "stdio": + errors.append("Codex MCP autoskillit transport is not stdio") + if transport.get("command") != expected.get("command"): + errors.append("Codex MCP autoskillit command does not match final config") + expected_args = _string_array(expected.get("args", [])) + actual_args = _string_array(transport.get("args", [])) + if expected_args is None: + errors.append("Final Codex config autoskillit args are not an array of strings") + if actual_args is None: + errors.append("Codex MCP autoskillit args are not an array of strings") + elif expected_args is not None and actual_args != expected_args: + errors.append("Codex MCP autoskillit args do not match final config") + expected_env_vars = _string_array(expected.get("env_vars", [])) + actual_env_vars = _string_array(transport.get("env_vars", [])) + if expected_env_vars is None: + errors.append("Final Codex config autoskillit env_vars are not an array of strings") + if actual_env_vars is None: + errors.append("Codex MCP autoskillit env_vars are not an array of strings") + elif expected_env_vars is not None and set(actual_env_vars) != set(expected_env_vars): + errors.append("Codex MCP autoskillit env_vars do not match final config") + for key in ("startup_timeout_sec", "tool_timeout_sec"): + if key in expected and actual.get(key) != expected[key]: + errors.append(f"Codex MCP autoskillit {key} does not match final config") + return errors + + +def _validation_digest( + command: tuple[str, ...], + *, + env: Mapping[str, str], + cwd: str, + config_bytes: bytes, +) -> str: + digest = hashlib.sha256() + for value in command: + digest.update(value.encode("utf-8")) + digest.update(b"\0") + for key, value in sorted(env.items()): + digest.update(key.encode("utf-8")) + digest.update(b"=") + digest.update(value.encode("utf-8")) + digest.update(b"\0") + digest.update(cwd.encode("utf-8")) + digest.update(b"\0") + digest.update(config_bytes) + return digest.hexdigest() + + +def _is_cached_validation(digest: str) -> bool: + with _CODEX_VALIDATION_CACHE_GUARD: + if digest not in _CODEX_VALIDATION_CACHE: + return False + _CODEX_VALIDATION_CACHE[digest] = _CODEX_VALIDATION_CACHE.pop(digest) + return True + + +def _cache_validation(digest: str) -> None: + with _CODEX_VALIDATION_CACHE_GUARD: + _CODEX_VALIDATION_CACHE.pop(digest, None) + _CODEX_VALIDATION_CACHE[digest] = None + while len(_CODEX_VALIDATION_CACHE) > _CODEX_VALIDATION_CACHE_LIMIT: + del _CODEX_VALIDATION_CACHE[next(iter(_CODEX_VALIDATION_CACHE))] + + +def _validate_mcp_probe( + command: tuple[str, ...], + *, + env: Mapping[str, str], + cwd: str, + config_bytes: bytes, +) -> list[str]: + digest = _validation_digest(command, env=env, cwd=cwd, config_bytes=config_bytes) + if _is_cached_validation(digest): + return [] + result = _run_bounded_codex_probe(command, env=env, cwd=cwd) + if result.failure is not None: + return [f"Codex MCP validation {result.failure}; {_probe_diagnostic(result)}"] + if result.returncode != 0: + return [ + f"Codex MCP validation exited with status {result.returncode}; " + f"{_probe_diagnostic(result)}" + ] + errors = _validate_codex_mcp_inventory(result.stdout, config_bytes) + if errors: + diagnostic = _probe_diagnostic(result) + return [f"{error}; {diagnostic}" for error in errors] + _cache_validation(digest) + return [] + + +def _validate_global_codex_home( + source_codex_home: Path, + *, + config_path: Path, +) -> list[str]: + try: + config_bytes = config_path.read_bytes() + except OSError as exc: + return [f"Failed to read final Codex config: {type(exc).__name__}: {exc}"] + sqlite_override = f"sqlite_home={_format_toml_value(str(source_codex_home))}" + command = ( + "codex", + CodexFlags.CONFIG_OVERRIDE, + sqlite_override, + "mcp", + "list", + CodexFlags.JSON, + ) + env = dict(os.environ) + for key in CODEX_COOK_RESERVED_ENV_VARS: + env[key] = str(source_codex_home) + return _validate_mcp_probe( + command, + env=env, + cwd=str(source_codex_home), + config_bytes=config_bytes, + ) + + +def _validate_inert_rollout_paths( + generated_home: Path, +) -> tuple[list[str], tuple[tuple[str, str, int, int], ...]]: + errors: list[str] = [] + fingerprint: list[tuple[str, str, int, int]] = [] + for name in ("sessions", "archived_sessions"): + public_path = generated_home / name + if not public_path.is_symlink(): + errors.append(f"{public_path} must be an inert pre-view symlink") + continue + try: + target = public_path.resolve(strict=True) + stat = target.stat() + except OSError as exc: + errors.append(f"{public_path} has an invalid target: {type(exc).__name__}: {exc}") + continue + if not target.is_relative_to(generated_home): + errors.append(f"{public_path} escapes the generated home") + continue + if not target.is_dir(): + errors.append(f"{public_path} target is not a directory") + continue + try: + entries = list(target.iterdir()) + except OSError as exc: + errors.append(f"{public_path} target is unreadable: {type(exc).__name__}: {exc}") + continue + if entries: + errors.append(f"{public_path} inert target is not empty") + fingerprint.append((name, os.readlink(public_path), stat.st_dev, stat.st_ino)) + return errors, tuple(fingerprint) diff --git a/src/autoskillit/execution/backends/codex.py b/src/autoskillit/execution/backends/codex.py index e7b791516..d1cf5a031 100644 --- a/src/autoskillit/execution/backends/codex.py +++ b/src/autoskillit/execution/backends/codex.py @@ -2,37 +2,21 @@ from __future__ import annotations -import hashlib -import json -import math import os -import selectors import shutil -import sqlite3 -import stat import subprocess -import tempfile -import threading -import time import tomllib -from collections.abc import Callable, Mapping, Sequence +from collections.abc import Mapping, Sequence from contextlib import AbstractContextManager -from dataclasses import dataclass, field -from enum import StrEnum, unique +from dataclasses import dataclass from pathlib import Path from typing import Any -import zstandard - from autoskillit.core import ( AGENT_BACKEND_CODEX, AGENT_BACKEND_DYNACONF_ENV_VAR, AGENT_BACKEND_ENV_VAR, - AUDIT_ADMISSION_AUTHORITY_PATH_ENV_VAR, - AUTOSKILLIT_APPLICABLE_GUARDS, - AUTOSKILLIT_PRIVATE_ENV_VARS, AUTOSKILLIT_STATE_ROOT_ENV_VAR, - AUTOSKILLIT_WRITE_GUARD_TOOL_NAMES, BUNDLED_EXPLORER_ROLES, CODEX_COOK_RESERVED_ENV_VARS, CODEX_EFFORT_MAPPING, @@ -40,7 +24,6 @@ CODEX_MCP_ENV_FORWARD_VARS, CODEX_MODEL_ALIASES, CODEX_SESSIONS_SUBDIR, - CODEX_STARTUP_TRACE_ENV_VAR, FLEET_INSPECTOR_MODEL_ENV_VAR, FOOD_TRUCK_TOOL_TAGS_ENV_VAR, LAUNCH_ID_ENV_VAR, @@ -53,7 +36,6 @@ SESSION_TYPE_ORCHESTRATOR, SESSION_TYPE_SKILL, SKILL_SESSION_REQUIRED_ENV, - WEB_EVIDENCE_RESEARCHER_ROLE, AgentDef, BackendCapabilities, BackendConventions, @@ -71,32 +53,26 @@ NativeShellCaptureDecision, NativeShellCaptureMode, NoResume, - ObserverStatus, OutputFormat, PluginLaunchBinding, PreLaunchReadiness, ResumeSpec, SessionCheckpoint, - SessionLocator, - SessionSummary, SkillExecutionRole, SkillSemanticAdaptationResult, SkillSemanticPlan, SkillSessionConfig, ValidatedAddDir, - agent_definition_digest, atomic_write, default_log_dir, extract_skill_name, get_logger, - load_bundled_agent_definitions, ) from autoskillit.execution.backends import _codex_config as _codex_cfg from autoskillit.execution.backends._backend_cmd_builder_base import ( SHARED_BASELINE_ENV, BackendCmdBuilderBase, FlagVocabulary, - _filter_protected_native_shell_env, _managed_native_shell_env, _merge_caller_env_extras, ) @@ -112,20 +88,27 @@ ) from autoskillit.execution.backends._cmd_builder import CmdBuilder from autoskillit.execution.backends._codex.explorer_projection import ( - _EXPLORER_ROLE_NAMES, _canonical_explorer_mcp_transport, - _direct_agent_mcp_tools, - _explorer_mcp_projection, - _render_direct_role_mcp_lines, _render_parent_explorer_config, - _render_role_mcp_lines, - _resolve_role_mcp_transport, _validate_injected_explorer_parent_policy, - _validated_explorer_binding_env, _validated_explorer_binding_envs, ) +from autoskillit.execution.backends._codex_cmd_builders import ( + _IMAGE_GENERATION_DISABLED, + CODEX_ENV_PREFIX_DENYLIST, + CODEX_EXEC_FLAGS, + CODEX_TOP_LEVEL_ONLY_FLAGS, + NON_VARIADIC_CODEX_FLAGS, + VARIADIC_CODEX_FLAGS, + CodexEnvPolicy, + CodexFlags, + CodexSessionLocator, + CodexStateReadinessProbe, + _codex_exec_base, + _codex_exec_extras, + _should_bypass_hook_trust, +) from autoskillit.execution.backends._codex_config import ( - _CODEX_AGENT_NAME_COLLISIONS, CODEX_RECIPE_DELIVERY_BUDGET, _format_toml_value, ensure_codex_mcp_registered, @@ -133,8 +116,29 @@ from autoskillit.execution.backends._codex_execution_identity import ( extract_codex_execution_identity, ) +from autoskillit.execution.backends._codex_explorer_projection import ( + _bundled_agent_definitions, + _canonical_codex_model_effort, + _generate_agent_tomls, + _materialize_profile_skills, + _preflight_agent_projection, + _register_agent_tomls, + _render_cli_auth_store, + _render_parent_sandbox_config, + clear_explorer_binding_env, + refresh_explorer_binding_env, +) from autoskillit.execution.backends._codex_parse import CodexResultParser, CodexStreamParser from autoskillit.execution.backends._codex_prelaunch import codex_prelaunch_transaction + +# Re-export probe helpers so existing consumers (e.g. evidence_reader, tests) +# can keep importing them from the canonical codex module path. +from autoskillit.execution.backends._codex_probes import ( + _BoundedProbeResult, # noqa: F401 + _validate_global_codex_home, + _validate_inert_rollout_paths, + _validate_mcp_probe, +) from autoskillit.execution.backends._codex_session_storage import CodexSessionStore from autoskillit.execution.backends._explorer_dispatch import ( CODEX_EXPLORATION_DISPATCH_RENDERER, @@ -149,6 +153,10 @@ def _codex_home_from_plugin_binding( return str(plugin_binding.plugin_dir) +_CODEX_HOME_ENV_VAR = "CODEX_HOME" +_CODEX_SQLITE_HOME_ENV_VAR = "CODEX_SQLITE_HOME" + + __all__ = [ "CODEX_EXEC_FLAGS", "CODEX_TOP_LEVEL_ONLY_FLAGS", @@ -167,1243 +175,6 @@ def _codex_home_from_plugin_binding( logger = get_logger(__name__) -@unique -class CodexFlags(StrEnum): - JSON = "--json" - SANDBOX = "--sandbox" - MODEL = "--model" - MODEL_SHORT = "-m" - ADD_DIR = "--add-dir" - RESUME_SUBCOMMAND = "resume" - CONFIG_OVERRIDE = "-c" - PROFILE = "--profile" - DANGEROUSLY_BYPASS = "--dangerously-bypass-approvals-and-sandbox" - DANGEROUSLY_BYPASS_HOOK_TRUST = "--dangerously-bypass-hook-trust" - - -CODEX_EXEC_FLAGS: frozenset[str] = frozenset( - { - CodexFlags.JSON, - CodexFlags.SANDBOX, - CodexFlags.MODEL, - CodexFlags.CONFIG_OVERRIDE, - CodexFlags.ADD_DIR, - CodexFlags.DANGEROUSLY_BYPASS_HOOK_TRUST, - } -) - -CODEX_TOP_LEVEL_ONLY_FLAGS: frozenset[str] = frozenset( - { - CodexFlags.DANGEROUSLY_BYPASS, - CodexFlags.MODEL_SHORT, - CodexFlags.PROFILE, - } -) - -VARIADIC_CODEX_FLAGS: frozenset[str] = frozenset({CodexFlags.ADD_DIR, CodexFlags.CONFIG_OVERRIDE}) - -NON_VARIADIC_CODEX_FLAGS: frozenset[str] = frozenset( - { - CodexFlags.JSON, - CodexFlags.SANDBOX, - CodexFlags.MODEL, - CodexFlags.MODEL_SHORT, - CodexFlags.PROFILE, - CodexFlags.RESUME_SUBCOMMAND, - CodexFlags.DANGEROUSLY_BYPASS, - CodexFlags.DANGEROUSLY_BYPASS_HOOK_TRUST, - } -) - - -CODEX_ENV_DENYLIST: frozenset[str] = frozenset( - { - "ANTHROPIC_API_KEY", - "ANTHROPIC_AUTH_TOKEN", - "ANTHROPIC_BASE_URL", - "CLAUDE_STREAM_IDLE_TIMEOUT_MS", - } -) - -CODEX_ENV_PREFIX_DENYLIST: tuple[str, ...] = ("CLAUDE_CODE_",) - -_IMAGE_GENERATION_DISABLED = "features.image_generation=false" -_CODEX_HOME_ENV_VAR = "CODEX_HOME" -_CODEX_SQLITE_HOME_ENV_VAR = "CODEX_SQLITE_HOME" - - -def _codex_exec_base( - *, - sandbox: str | None, - json: bool = True, - extra_overrides: Sequence[str] = (), - bypass_hook_trust: bool = False, -) -> list[str]: - cmd: list[str] = ["codex", "exec"] - if json: - cmd.append(CodexFlags.JSON) - if sandbox is not None: - cmd.extend([CodexFlags.SANDBOX, sandbox]) - for override in extra_overrides: - cmd.extend([CodexFlags.CONFIG_OVERRIDE, override]) - cmd.extend([CodexFlags.CONFIG_OVERRIDE, _IMAGE_GENERATION_DISABLED]) - if bypass_hook_trust: - # Hook trust is independent from the sandbox selected by config/CLI. - cmd.append(CodexFlags.DANGEROUSLY_BYPASS_HOOK_TRUST) - return cmd - - -def _should_bypass_hook_trust( - policy: HookTrustPolicy, - *, - automated_session: bool, -) -> bool: - """Translate backend hook policy at the command-construction boundary.""" - if automated_session: - return True - match policy: - case HookTrustPolicy.AUTOMATED: - return True - case HookTrustPolicy.REVIEW_EACH_SESSION: - return False - raise AssertionError(f"Unhandled hook trust policy: {policy!r}") - - -_CODEX_STATE_READINESS_COMMIT = "ad65f016ed0c91992fb175fa881a373cc460dd2a" - - -@dataclass(frozen=True, slots=True) -class _StateReadinessDef: - database_name: str - upstream_commit: str - - -_SUPPORTED_STATE_CONTRACTS = { - "codex-cli 0.145.0": _StateReadinessDef( - database_name="state_5.sqlite", - upstream_commit=_CODEX_STATE_READINESS_COMMIT, - ) -} - - -@dataclass(frozen=True, slots=True) -class CodexStateReadinessProbe: - """Read the version-mapped disposable Codex state database without mutation.""" - - codex_version: str - sqlite_home: Path - poll_interval_seconds: float = 0.05 - _clock: Callable[[], float] = field(default=time.monotonic, repr=False) - _sleep: Callable[[float], None] = field(default=time.sleep, repr=False) - - def __post_init__(self) -> None: - if not math.isfinite(self.poll_interval_seconds) or self.poll_interval_seconds <= 0: - raise ValueError("poll_interval_seconds must be finite and positive") - object.__setattr__(self, "sqlite_home", Path(self.sqlite_home)) - - @property - def database_path(self) -> Path | None: - """Return the exact database path for a supported Codex version.""" - compatibility = _SUPPORTED_STATE_CONTRACTS.get(self.codex_version) - return None if compatibility is None else self.sqlite_home / compatibility.database_name - - @property - def upstream_commit(self) -> str | None: - """Return the source revision defining the probed schema contract.""" - compatibility = _SUPPORTED_STATE_CONTRACTS.get(self.codex_version) - return None if compatibility is None else compatibility.upstream_commit - - def check(self) -> ObserverStatus: - """Perform one zero-wait, read-only readiness observation.""" - database_path = self.database_path - if database_path is None: - return ObserverStatus.UNSUPPORTED_VERSION - try: - path_stat = database_path.lstat() - except FileNotFoundError: - return ObserverStatus.ABSENT - except OSError: - return ObserverStatus.CORRUPT - if not stat.S_ISREG(path_stat.st_mode): - return ObserverStatus.CORRUPT - - connection: sqlite3.Connection | None = None - try: - uri = f"{database_path.resolve(strict=True).as_uri()}?mode=ro" - connection = sqlite3.connect( - uri, - uri=True, - timeout=0.0, - isolation_level=None, - ) - connection.execute("PRAGMA query_only = ON") - connection.execute("PRAGMA busy_timeout = 0") - columns = { - row[1] - for row in connection.execute("PRAGMA table_info(backfill_state)") - if len(row) > 1 and isinstance(row[1], str) - } - if not {"id", "status"}.issubset(columns): - return ObserverStatus.SCHEMA_CHANGED - row = connection.execute("SELECT status FROM backfill_state WHERE id = 1").fetchone() - if row is None or len(row) != 1 or not isinstance(row[0], str): - return ObserverStatus.INCOMPLETE - return ObserverStatus.READY if row[0] == "complete" else ObserverStatus.INCOMPLETE - except sqlite3.OperationalError as exc: - message = str(exc).lower() - if "locked" in message or "busy" in message: - return ObserverStatus.LOCKED - if "no such table" in message or "no such column" in message: - return ObserverStatus.SCHEMA_CHANGED - return ObserverStatus.CORRUPT - except (OSError, sqlite3.DatabaseError, ValueError): - return ObserverStatus.CORRUPT - finally: - if connection is not None: - connection.close() - - def wait( - self, - *, - timeout_seconds: float, - cancelled: Callable[[], bool] | None = None, - ) -> ObserverStatus: - """Poll until ready, a terminal adapter failure, timeout, or cancellation.""" - if not math.isfinite(timeout_seconds) or timeout_seconds < 0: - raise ValueError("timeout_seconds must be finite and non-negative") - is_cancelled = cancelled or (lambda: False) - deadline = self._clock() + timeout_seconds - while True: - if is_cancelled(): - return ObserverStatus.CANCELLED - if self._clock() >= deadline: - return ObserverStatus.TIMEOUT - status = self.check() - if status is ObserverStatus.READY: - return status - if status in { - ObserverStatus.CORRUPT, - ObserverStatus.SCHEMA_CHANGED, - ObserverStatus.UNSUPPORTED_VERSION, - }: - return status - remaining = deadline - self._clock() - if remaining <= 0: - return ObserverStatus.TIMEOUT - self._sleep(min(self.poll_interval_seconds, remaining)) - - -def _codex_exec_extras( - *, - session_type: str, - include_session_baseline: bool = False, - include_agent_backend_flat: bool = False, - applicable_guards: frozenset[str] | None = None, - write_guard_tool_names: frozenset[str] | None = None, -) -> dict[str, str]: - extras: dict[str, str] = {} - if include_session_baseline: - extras.update(SHARED_BASELINE_ENV) - extras.update( - { - "AUTOSKILLIT_HEADLESS": "1", - "AUTOSKILLIT_HEADLESS_AUTO_GATE": "1", - "AUTOSKILLIT_SESSION_TYPE": session_type, - AGENT_BACKEND_DYNACONF_ENV_VAR: AGENT_BACKEND_CODEX, - MCP_CLIENT_BACKEND_ENV_VAR: AGENT_BACKEND_CODEX, - FLEET_INSPECTOR_MODEL_ENV_VAR: "", - FOOD_TRUCK_TOOL_TAGS_ENV_VAR: "", - } - ) - extras.setdefault(LAUNCH_ID_ENV_VAR, "") - extras.setdefault(AUTOSKILLIT_STATE_ROOT_ENV_VAR, "") - if include_agent_backend_flat: - extras[AGENT_BACKEND_ENV_VAR] = AGENT_BACKEND_CODEX - if applicable_guards is not None: - extras[AUTOSKILLIT_APPLICABLE_GUARDS] = ",".join(sorted(applicable_guards)) - if write_guard_tool_names is not None: - extras[AUTOSKILLIT_WRITE_GUARD_TOOL_NAMES] = ",".join(sorted(write_guard_tool_names)) - return extras - - -@dataclass(frozen=True, slots=True) -class CodexEnvPolicy: - denylist_prefixes: tuple[str, ...] = CODEX_ENV_PREFIX_DENYLIST - - def build_env( - self, - base_env: Mapping[str, str], - *, - extras: Mapping[str, str] | None = None, - required: frozenset[str] | None = None, - ) -> dict[str, str]: - out: dict[str, str] = { - k: v - for k, v in base_env.items() - if k not in CODEX_ENV_DENYLIST - and k not in AUTOSKILLIT_PRIVATE_ENV_VARS - and not any(k.startswith(p) for p in self.denylist_prefixes) - } - if extras is not None: - filtered_extras = _filter_protected_native_shell_env(extras) - filtered_extras.setdefault("AUTOSKILLIT_SKILL_NAME", "") - out.update( - (key, value) - for key, value in filtered_extras.items() - if key != CODEX_STARTUP_TRACE_ENV_VAR - ) - out.setdefault(AUDIT_ADMISSION_AUTHORITY_PATH_ENV_VAR, "") # Outer-cook control only. - out.pop(CODEX_STARTUP_TRACE_ENV_VAR, None) - if required is not None: - missing = required - frozenset(out) - if missing: - raise ValueError(f"Required env vars missing from session env: {sorted(missing)}") - return out - - -@dataclass(frozen=True, slots=True) -class CodexSessionLocator(SessionLocator): - store_root: Path | None = None - index_path: Path | None = None - - def _store(self) -> CodexSessionStore: - return CodexSessionStore( - log_dir=self.store_root or default_log_dir(), - index_path=self.index_path, - ) - - def locate_session(self, session_id: str) -> Path | None: - if not session_id or session_id.startswith(("no_session_", "crashed_")): - return None - return self._store().locate_session(session_id) - - def read_session(self, path: Path) -> list[dict]: - """Read and parse a Codex session log file. - - Handles both plain .jsonl (current Codex v0.133.0+) and - .jsonl.zst (legacy) formats based on file extension. - """ - try: - if path.name.endswith(".zst"): - raw = path.read_bytes() - decompressed = zstandard.ZstdDecompressor().decompress(raw) - text = decompressed.decode("utf-8") - else: - text = path.read_text(encoding="utf-8") - except Exception: - logger.warning("read_session: failed to read", path=str(path), exc_info=True) - return [] - result: list[dict] = [] - for line in text.splitlines(): - if not line: - continue - try: - obj = json.loads(line) - except json.JSONDecodeError: - continue - if isinstance(obj, dict): - result.append(obj) - return result - - def project_log_dir(self, cwd: str) -> Path: # cwd unused; Codex uses a global session store - return (self.store_root or default_log_dir()) / CODEX_SESSIONS_SUBDIR - - def session_log_path(self, cwd: str, session_id: str) -> Path | None: - if not session_id or session_id.startswith(("no_session_", "crashed_")): - return None - return self.locate_session(session_id) - - def list_sessions(self, cwd: str) -> tuple[SessionSummary, ...]: - return self._store().read_index(cwd) - - -_CODEX_PROBE_TIMEOUT_SECONDS = 15.0 -_CODEX_PROBE_STREAM_LIMIT = 64 * 1024 -_CODEX_VALIDATION_CACHE_LIMIT = 128 -_CODEX_VALIDATION_CACHE: dict[str, None] = {} -_CODEX_VALIDATION_CACHE_GUARD = threading.Lock() - - -@dataclass(frozen=True, slots=True) -class _BoundedProbeResult: - returncode: int | None - stdout: bytes - stderr: bytes - failure: str | None = None - - -def _terminate_probe(owner: object) -> None: - from autoskillit.execution.process._process_kill import OwnedProcessGroup - - if not isinstance(owner, OwnedProcessGroup): - raise TypeError("Codex probe cleanup requires its spawn-bound owner") - try: - owner.settle(timeout=2) - finally: - for stream in (owner.process.stdout, owner.process.stderr): - if stream is not None: - stream.close() - - -def _run_bounded_codex_probe( - command: tuple[str, ...], - *, - env: Mapping[str, str], - cwd: str, -) -> _BoundedProbeResult: - try: - from autoskillit.execution.process._process_kill import spawn_owned_process - - owner = spawn_owned_process( - command, - cwd=cwd, - env=dict(env), - stdin=subprocess.DEVNULL, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - start_new_session=True, - ) - process = owner.process - except OSError as exc: - return _BoundedProbeResult( - returncode=None, - stdout=b"", - stderr=b"", - failure=f"binary unavailable ({type(exc).__name__})", - ) - - selector: selectors.BaseSelector | None = None - output = {"stdout": bytearray(), "stderr": bytearray()} - deadline = time.monotonic() + _CODEX_PROBE_TIMEOUT_SECONDS - try: - assert process.stdout is not None - assert process.stderr is not None - selector_factory = selectors.DefaultSelector - selector = selector_factory() - selector.register(process.stdout, selectors.EVENT_READ, "stdout") - selector.register(process.stderr, selectors.EVENT_READ, "stderr") - while selector.get_map() or owner.observe_exit() is None: - remaining = deadline - time.monotonic() - if remaining <= 0: - _terminate_probe(owner) - return _BoundedProbeResult( - returncode=None, - stdout=bytes(output["stdout"]), - stderr=bytes(output["stderr"]), - failure="timed out", - ) - if not selector.get_map(): - time.sleep(min(0.01, remaining)) - continue - events = selector.select(timeout=min(0.1, remaining)) - for key, _ in events: - stream_name = key.data - try: - file_descriptor = ( - key.fileobj if isinstance(key.fileobj, int) else key.fileobj.fileno() - ) - chunk = os.read(file_descriptor, 8192) - except OSError: - chunk = b"" - if not chunk: - selector.unregister(key.fileobj) - continue - target = output[stream_name] - target.extend(chunk) - if len(target) > _CODEX_PROBE_STREAM_LIMIT: - del target[_CODEX_PROBE_STREAM_LIMIT:] - _terminate_probe(owner) - return _BoundedProbeResult( - returncode=None, - stdout=bytes(output["stdout"]), - stderr=bytes(output["stderr"]), - failure=f"{stream_name} exceeded {_CODEX_PROBE_STREAM_LIMIT} bytes", - ) - returncode, _cleanup = owner.settle(timeout=max(0.0, deadline - time.monotonic())) - except subprocess.TimeoutExpired: - _terminate_probe(owner) - return _BoundedProbeResult( - returncode=None, - stdout=bytes(output["stdout"]), - stderr=bytes(output["stderr"]), - failure="timed out while reaping", - ) - except BaseException as exc: - if process.returncode is None: - try: - _terminate_probe(owner) - except BaseException as cleanup_exc: - logger.error("codex_probe_cleanup_failed", exc_info=True) - exc.add_note(f"Codex probe cleanup failed: {type(cleanup_exc).__name__}") - raise - finally: - if selector is not None: - selector.close() - for stream in (process.stdout, process.stderr): - if stream is not None: - stream.close() - return _BoundedProbeResult( - returncode=returncode, - stdout=bytes(output["stdout"]), - stderr=bytes(output["stderr"]), - ) - - -def _probe_diagnostic(result: _BoundedProbeResult) -> str: - """Return bounded, non-content diagnostics safe for configs containing secrets.""" - stdout_digest = hashlib.sha256(result.stdout).hexdigest()[:16] - stderr_digest = hashlib.sha256(result.stderr).hexdigest()[:16] - return ( - f"stdout_bytes={len(result.stdout)} stdout_sha256={stdout_digest} " - f"stderr_bytes={len(result.stderr)} stderr_sha256={stderr_digest}" - ) - - -def _mcp_inventory_entries(document: Any) -> list[dict[str, Any]] | None: - if isinstance(document, list): - return [entry for entry in document if isinstance(entry, dict)] - if not isinstance(document, dict): - return None - for key in ("servers", "mcp_servers"): - value = document.get(key) - if isinstance(value, list): - return [entry for entry in value if isinstance(entry, dict)] - if isinstance(value, dict): - return [ - {"name": name, **entry} - for name, entry in value.items() - if isinstance(name, str) and isinstance(entry, dict) - ] - return None - - -def _string_array(value: Any) -> list[str] | None: - if not isinstance(value, list) or not all(isinstance(item, str) for item in value): - return None - return value - - -def _validate_codex_mcp_inventory(stdout: bytes, config_bytes: bytes) -> list[str]: - try: - document = json.loads(stdout.decode("utf-8")) - except (UnicodeDecodeError, json.JSONDecodeError): - return ["Codex MCP validation returned malformed JSON"] - try: - config = tomllib.loads(config_bytes.decode("utf-8")) - except (UnicodeDecodeError, tomllib.TOMLDecodeError): - return ["Final Codex config bytes are not valid UTF-8 TOML"] - - expected = config.get("mcp_servers", {}).get("autoskillit") - if not isinstance(expected, dict): - return ["Final Codex config is missing mcp_servers.autoskillit"] - entries = _mcp_inventory_entries(document) - if entries is None: - return ["Codex MCP validation JSON has no server inventory"] - matches = [entry for entry in entries if entry.get("name") == "autoskillit"] - if len(matches) != 1: - return [ - "Codex MCP validation expected exactly one enabled autoskillit server; " - f"found {len(matches)}" - ] - actual = matches[0] - if actual.get("enabled") is False: - return ["Codex MCP validation reports autoskillit as disabled"] - transport = actual.get("transport") - if not isinstance(transport, dict): - transport = actual - errors: list[str] = [] - if transport.get("type", "stdio") != "stdio": - errors.append("Codex MCP autoskillit transport is not stdio") - if transport.get("command") != expected.get("command"): - errors.append("Codex MCP autoskillit command does not match final config") - expected_args = _string_array(expected.get("args", [])) - actual_args = _string_array(transport.get("args", [])) - if expected_args is None: - errors.append("Final Codex config autoskillit args are not an array of strings") - if actual_args is None: - errors.append("Codex MCP autoskillit args are not an array of strings") - elif expected_args is not None and actual_args != expected_args: - errors.append("Codex MCP autoskillit args do not match final config") - expected_env_vars = _string_array(expected.get("env_vars", [])) - actual_env_vars = _string_array(transport.get("env_vars", [])) - if expected_env_vars is None: - errors.append("Final Codex config autoskillit env_vars are not an array of strings") - if actual_env_vars is None: - errors.append("Codex MCP autoskillit env_vars are not an array of strings") - elif expected_env_vars is not None and set(actual_env_vars) != set(expected_env_vars): - errors.append("Codex MCP autoskillit env_vars do not match final config") - for key in ("startup_timeout_sec", "tool_timeout_sec"): - if key in expected and actual.get(key) != expected[key]: - errors.append(f"Codex MCP autoskillit {key} does not match final config") - return errors - - -def _validation_digest( - command: tuple[str, ...], - *, - env: Mapping[str, str], - cwd: str, - config_bytes: bytes, -) -> str: - digest = hashlib.sha256() - for value in command: - digest.update(value.encode("utf-8")) - digest.update(b"\0") - for key, value in sorted(env.items()): - digest.update(key.encode("utf-8")) - digest.update(b"=") - digest.update(value.encode("utf-8")) - digest.update(b"\0") - digest.update(cwd.encode("utf-8")) - digest.update(b"\0") - digest.update(config_bytes) - return digest.hexdigest() - - -def _is_cached_validation(digest: str) -> bool: - with _CODEX_VALIDATION_CACHE_GUARD: - if digest not in _CODEX_VALIDATION_CACHE: - return False - _CODEX_VALIDATION_CACHE[digest] = _CODEX_VALIDATION_CACHE.pop(digest) - return True - - -def _cache_validation(digest: str) -> None: - with _CODEX_VALIDATION_CACHE_GUARD: - _CODEX_VALIDATION_CACHE.pop(digest, None) - _CODEX_VALIDATION_CACHE[digest] = None - while len(_CODEX_VALIDATION_CACHE) > _CODEX_VALIDATION_CACHE_LIMIT: - del _CODEX_VALIDATION_CACHE[next(iter(_CODEX_VALIDATION_CACHE))] - - -def _validate_mcp_probe( - command: tuple[str, ...], - *, - env: Mapping[str, str], - cwd: str, - config_bytes: bytes, -) -> list[str]: - digest = _validation_digest(command, env=env, cwd=cwd, config_bytes=config_bytes) - if _is_cached_validation(digest): - return [] - result = _run_bounded_codex_probe(command, env=env, cwd=cwd) - if result.failure is not None: - return [f"Codex MCP validation {result.failure}; {_probe_diagnostic(result)}"] - if result.returncode != 0: - return [ - f"Codex MCP validation exited with status {result.returncode}; " - f"{_probe_diagnostic(result)}" - ] - errors = _validate_codex_mcp_inventory(result.stdout, config_bytes) - if errors: - diagnostic = _probe_diagnostic(result) - return [f"{error}; {diagnostic}" for error in errors] - _cache_validation(digest) - return [] - - -def _validate_global_codex_home( - source_codex_home: Path, - *, - config_path: Path, -) -> list[str]: - try: - config_bytes = config_path.read_bytes() - except OSError as exc: - return [f"Failed to read final Codex config: {type(exc).__name__}: {exc}"] - sqlite_override = f"sqlite_home={_format_toml_value(str(source_codex_home))}" - command = ( - "codex", - CodexFlags.CONFIG_OVERRIDE, - sqlite_override, - "mcp", - "list", - CodexFlags.JSON, - ) - env = dict(os.environ) - for key in CODEX_COOK_RESERVED_ENV_VARS: - env[key] = str(source_codex_home) - return _validate_mcp_probe( - command, - env=env, - cwd=str(source_codex_home), - config_bytes=config_bytes, - ) - - -def _validate_inert_rollout_paths( - generated_home: Path, -) -> tuple[list[str], tuple[tuple[str, str, int, int], ...]]: - errors: list[str] = [] - fingerprint: list[tuple[str, str, int, int]] = [] - for name in ("sessions", "archived_sessions"): - public_path = generated_home / name - if not public_path.is_symlink(): - errors.append(f"{public_path} must be an inert pre-view symlink") - continue - try: - target = public_path.resolve(strict=True) - stat = target.stat() - except OSError as exc: - errors.append(f"{public_path} has an invalid target: {type(exc).__name__}: {exc}") - continue - if not target.is_relative_to(generated_home): - errors.append(f"{public_path} escapes the generated home") - continue - if not target.is_dir(): - errors.append(f"{public_path} target is not a directory") - continue - try: - entries = list(target.iterdir()) - except OSError as exc: - errors.append(f"{public_path} target is unreadable: {type(exc).__name__}: {exc}") - continue - if entries: - errors.append(f"{public_path} inert target is not empty") - fingerprint.append((name, os.readlink(public_path), stat.st_dev, stat.st_ino)) - return errors, tuple(fingerprint) - - -def _bundled_agent_definitions() -> tuple[AgentDef, ...]: - return load_bundled_agent_definitions() - - -def _canonical_codex_model_effort( - model_class: str | None, - reasoning_effort: str | None = None, -) -> tuple[str, str | None]: - if model_class is None: - return "", reasoning_effort - model = CODEX_MODEL_ALIASES[model_class] - return model, reasoning_effort or CODEX_EFFORT_MAPPING.get(model_class) - - -CODEX_SPAWNABLE_BUILT_IN_AGENT_NAMES = _codex_cfg.CODEX_SPAWNABLE_BUILT_IN_AGENT_NAMES - - -def _preflight_agent_projection( - session_dir: Path, - definitions: tuple[AgentDef, ...], - *, - exact_definitions: bool, -) -> tuple[AgentDef, ...]: - """Validate the complete role set and select roles safe to project.""" - names = tuple(definition.name for definition in definitions) - duplicates = sorted({name for name in names if names.count(name) > 1}) - if duplicates: - raise ValueError(f"duplicate Codex agent definitions: {duplicates}") - built_in_collisions = sorted(set(names) & _CODEX_AGENT_NAME_COLLISIONS) - if built_in_collisions: - raise ValueError(f"Codex built-in agent name collision: {built_in_collisions}") - config_path = session_dir / "config.toml" - config = tomllib.loads(config_path.read_text(encoding="utf-8")) - if exact_definitions and any(map(_direct_agent_mcp_tools, definitions)): - _canonical_explorer_mcp_transport(config_path) - configured_agents = config.get("agents", {}) - if not isinstance(configured_agents, dict): - raise ValueError("Codex config agents table must be a mapping") - protected_names = ( - set(names) - if exact_definitions - else {*BUNDLED_EXPLORER_ROLES, WEB_EVIDENCE_RESEARCHER_ROLE} - ) - ambient_collisions = sorted(set(names) & set(configured_agents) & protected_names) - if ambient_collisions: - raise ValueError(f"ambient Codex agent name collision: {ambient_collisions}") - - agents_dir = session_dir / "agents" - if agents_dir.exists() and not agents_dir.is_dir(): - raise ValueError(f"Codex agents path is not a directory: {agents_dir}") - artifact_collisions = sorted( - definition.name - for definition in definitions - if (agents_dir / f"{definition.name}.toml").exists() - ) - if artifact_collisions: - raise ValueError(f"ambient Codex agent artifact collision: {artifact_collisions}") - return tuple( - definition for definition in definitions if definition.name not in configured_agents - ) - - -def _render_agent_toml( - definition: AgentDef, - *, - explorer_binding_env: Mapping[str, str] | None = None, - explorer_mcp_transport: Mapping[str, object] | None = None, - project_explorer_mcp: bool = False, -) -> str: - """Render and parse one role before its output directory is touched.""" - direct_mcp_tools = _direct_agent_mcp_tools(definition) - digest = agent_definition_digest(definition) - lines = [ - f"name = {_format_toml_value(definition.name)}", - f"description = {_format_toml_value(definition.description)}", - f"sandbox_mode = {_format_toml_value(definition.codex.sandbox_mode)}", - ] - if definition.codex.model is not None: - lines.append(f"model = {_format_toml_value(definition.codex.model)}") - if definition.codex.reasoning_effort is not None: - lines.append( - f"model_reasoning_effort = {_format_toml_value(definition.codex.reasoning_effort)}" - ) - if definition.codex.web_search is not None: - lines.append(f"web_search = {_format_toml_value(definition.codex.web_search)}") - body = ( - f"{definition.body}\n\n" - f"AutoSkillit agent definition digest: {digest}\n\n" - f"{codex_discipline_suffix()}" - ) - lines.append(f"instructions = '''\n{body}\n'''") - lines.append(f"developer_instructions = '''\n{body}\n'''") - if definition.codex.disabled_features: - lines.append("[features]") - lines.extend(f"{feature} = false" for feature in definition.codex.disabled_features) - if not definition.codex.agents_enabled: - lines.extend(("[agents]", "enabled = false")) - if explorer_binding_env is not None and not project_explorer_mcp: - raise ValueError("an explorer binding requires an explorer MCP projection") - if explorer_mcp_transport is not None and not project_explorer_mcp and not direct_mcp_tools: - raise ValueError("an explorer MCP transport requires an explorer MCP projection") - if project_explorer_mcp: - if explorer_mcp_transport is None: - raise ValueError("an explorer MCP projection requires a canonical transport") - projection = _explorer_mcp_projection( - explorer_mcp_transport, - explorer_binding_env, - ) - lines.extend(_render_role_mcp_lines(projection, explorer_binding_env)) - elif direct_mcp_tools: - lines.extend(_render_direct_role_mcp_lines(explorer_mcp_transport, direct_mcp_tools)) - rendered = "\n".join(lines) + "\n" - tomllib.loads(rendered) - return rendered - - -def _eligible_agent_definitions( - definitions: tuple[AgentDef, ...], - bindings: Mapping[str, Mapping[str, str]], - *, - exact: bool, -) -> tuple[AgentDef, ...]: - definitions = tuple(d for d in definitions if not d.reader_tools) - if exact: - return definitions - return tuple( - definition - for definition in definitions - if definition.name not in BUNDLED_EXPLORER_ROLES or definition.name in bindings - ) - - -def _generate_agent_tomls( - session_dir: Path, - agent_defs: tuple[AgentDef, ...] | None = None, - *, - explorer_binding_envs: Mapping[str, Mapping[str, str]] | None = None, - explorer_mcp_transport: Mapping[str, object] | None = None, -) -> int: - definitions = _bundled_agent_definitions() if agent_defs is None else agent_defs - bindings = explorer_binding_envs or {} - eligible = _eligible_agent_definitions( - definitions, - bindings, - exact=agent_defs is not None, - ) - direct_mcp_transport = _resolve_role_mcp_transport( - session_dir, eligible, bindings, explorer_mcp_transport - ) - rendered = { - definition.name: _render_agent_toml( - definition, - explorer_binding_env=bindings.get(definition.name), - explorer_mcp_transport=( - direct_mcp_transport - if definition.name in bindings or _direct_agent_mcp_tools(definition) - else None - ), - project_explorer_mcp=definition.name in bindings, - ) - for definition in eligible - } - out_dir = session_dir / "agents" - out_dir.mkdir(exist_ok=True) - for definition in eligible: - toml_path = out_dir / f"{definition.name}.toml" - atomic_write(toml_path, rendered[definition.name]) - logger.debug("codex_agents_generated", count=len(eligible), dest=str(out_dir)) - return len(eligible) - - -def _register_agent_tomls( - session_dir: Path, - agent_defs: tuple[AgentDef, ...] | None = None, - *, - explorer_binding_envs: Mapping[str, Mapping[str, str]] | None = None, -) -> int: - config_path = session_dir / "config.toml" - config_text = config_path.read_text(encoding="utf-8") - tomllib.loads(config_text) - registrations: list[str] = [] - definitions = _bundled_agent_definitions() if agent_defs is None else agent_defs - bindings = explorer_binding_envs or {} - eligible = _eligible_agent_definitions( - definitions, - bindings, - exact=agent_defs is not None, - ) - for definition in eligible: - agent_path = session_dir / "agents" / f"{definition.name}.toml" - agent = tomllib.loads(agent_path.read_text(encoding="utf-8")) - if agent.get("name") != definition.name: - raise ValueError(f"generated agent identity mismatch: {agent_path}") - registrations.extend( - [ - f"[agents.{_format_toml_value(definition.name)}]", - f"description = {_format_toml_value(definition.description)}", - f"config_file = {_format_toml_value(f'agents/{agent_path.name}')}", - "", - ] - ) - if not registrations: - return 0 - separator = "\n" if config_text.endswith("\n") else "\n\n" - registration_text = "\n".join(registrations) - updated = f"{config_text}{separator}{registration_text}" - tomllib.loads(updated) - atomic_write(config_path, updated) - return len(registrations) // 4 - - -def _validate_existing_explorer_role_toml( - toml_path: Path, - definition: AgentDef, - *, - require_binding_env: bool, - explorer_mcp_transport: Mapping[str, object], -) -> dict[str, str] | None: - """Validate persisted role identity and recover its binding environment.""" - try: - current = tomllib.loads(toml_path.read_text(encoding="utf-8")) - except (OSError, tomllib.TOMLDecodeError) as exc: - raise ValueError(f"invalid materialized explorer role {toml_path}: {exc}") from exc - servers = current.get("mcp_servers") - if not isinstance(servers, dict) or set(servers) != {"autoskillit"}: - raise ValueError(f"materialized explorer role missing MCP projection: {toml_path}") - current_server = servers.get("autoskillit") - if not isinstance(current_server, dict): - raise ValueError(f"materialized explorer role missing MCP projection: {toml_path}") - current_server = dict(current_server) - current_env = current_server.pop("env", None) - expected_server = _explorer_mcp_projection(explorer_mcp_transport, None) - if current_server != expected_server: - raise ValueError(f"materialized explorer role has a divergent MCP projection: {toml_path}") - if current.get("name") != definition.name: - raise ValueError(f"materialized explorer role identity mismatch: {toml_path}") - if current_env is None and not require_binding_env: - return None - if not isinstance(current_env, dict): - raise ValueError( - f"materialized explorer role has an invalid binding environment: {toml_path}" - ) - return _validated_explorer_binding_env(definition.name, current_env) - - -def _validate_existing_parent_explorer_projection( - session_config: Mapping[str, object], - *, - require_binding_env: bool, - explorer_mcp_transport: Mapping[str, object], -) -> dict[str, str] | None: - """Validate the parent half of the shared-principal MCP projection.""" - if session_config.get("sandbox_mode") != "read-only": - raise ValueError("materialized explorer parent must be read-only") - servers = session_config.get("mcp_servers") - if not isinstance(servers, dict) or set(servers) != {"autoskillit"}: - raise ValueError("materialized explorer parent must configure exactly one MCP server") - current_server = servers.get("autoskillit") - if not isinstance(current_server, dict): - raise ValueError("materialized explorer parent is missing its MCP projection") - current_server = dict(current_server) - current_env = current_server.pop("env", None) - expected_server = _explorer_mcp_projection(explorer_mcp_transport, None) - if current_server != expected_server: - raise ValueError("materialized explorer parent has a divergent MCP projection") - if current_env is None and not require_binding_env: - return None - if not isinstance(current_env, dict): - raise ValueError("materialized explorer parent has an invalid binding environment") - return _validated_explorer_binding_env("parent", current_env) - - -def _validate_materialized_explorer_roles( - session_dir: Path, - definitions: tuple[AgentDef, ...], - roles: frozenset[str], - *, - require_binding_env: bool, -) -> tuple[dict[str, AgentDef], dict[str, object], str]: - """Validate registered persisted explorer artifacts before a grouped rewrite.""" - if any(type(role) is not str for role in roles): - raise ValueError("explorer role cleanup set must contain only text names") - definitions_by_name = {definition.name: definition for definition in definitions} - if roles != _EXPLORER_ROLE_NAMES or not roles <= set(definitions_by_name): - raise ValueError(f"unknown explorer roles: {sorted(roles - set(definitions_by_name))}") - agents_dir = session_dir / "agents" - if not agents_dir.is_dir(): - raise ValueError(f"materialized Codex agents directory is missing: {agents_dir}") - config_path = session_dir / "config.toml" - try: - config_text = config_path.read_text(encoding="utf-8") - session_config = tomllib.loads(config_text) - except (OSError, tomllib.TOMLDecodeError) as exc: - raise ValueError(f"invalid materialized Codex config: {exc}") from exc - registered_agents = session_config.get("agents") - if not isinstance(registered_agents, dict): - raise ValueError("materialized Codex config has no agent registrations") - explorer_mcp_transport = _canonical_explorer_mcp_transport(config_path) - projected_bindings = [ - _validate_existing_parent_explorer_projection( - session_config, - require_binding_env=require_binding_env, - explorer_mcp_transport=explorer_mcp_transport, - ) - ] - - selected: dict[str, AgentDef] = {} - for role in sorted(roles): - definition = definitions_by_name[role] - registration = registered_agents.get(role) - expected_path = f"agents/{role}.toml" - if not isinstance(registration, dict) or registration.get("config_file") != expected_path: - raise ValueError( - f"materialized Codex config has no canonical registration for {role!r}" - ) - projected_bindings.append( - _validate_existing_explorer_role_toml( - agents_dir / f"{role}.toml", - definition, - require_binding_env=require_binding_env, - explorer_mcp_transport=explorer_mcp_transport, - ) - ) - selected[role] = definition - if any(binding != projected_bindings[0] for binding in projected_bindings[1:]): - raise ValueError("materialized explorer bindings diverge from the shared principal") - return selected, explorer_mcp_transport, config_text - - -def _atomically_replace_explorer_projection( - session_dir: Path, - rendered_config: str, - rendered_roles: Mapping[str, str], -) -> None: - """Swap the parent and both roles as one staged session-root transaction.""" - stage_root = Path( - tempfile.mkdtemp(prefix=".autoskillit-explorer-refresh-", dir=session_dir.parent) - ) - staged_session = stage_root / "session" - backup_session = stage_root / "previous-session" - moved_original = False - try: - shutil.copytree(session_dir, staged_session, symlinks=True) - atomic_write(staged_session / "config.toml", rendered_config) - for role, content in rendered_roles.items(): - atomic_write(staged_session / "agents" / f"{role}.toml", content) - os.replace(session_dir, backup_session) - moved_original = True - try: - os.replace(staged_session, session_dir) - except OSError as install_error: - try: - os.replace(backup_session, session_dir) - except OSError as restore_error: - raise restore_error from install_error - moved_original = False - raise - moved_original = False - finally: - if not moved_original: - shutil.rmtree(stage_root, ignore_errors=True) - - -def refresh_explorer_binding_env( - session_dir: Path, - explorer_binding_env: Mapping[str, Mapping[str, str]], -) -> None: - """Atomically replace only server-issued explorer binding values on resume. - - The helper validates the persisted parent and both definition-derived role - layers before staging a replacement session root, so a failed refresh - cannot leave any of the three configs on a different principal. - """ - definitions = _bundled_agent_definitions() - binding_envs = _validated_explorer_binding_envs(definitions, explorer_binding_env) - if not binding_envs: - return - - definitions_by_name, explorer_mcp_transport, config_text = ( - _validate_materialized_explorer_roles( - session_dir, - definitions, - frozenset(binding_envs), - require_binding_env=True, - ) - ) - shared_binding = next(iter(binding_envs.values())) - rendered_config = _render_parent_explorer_config( - config_text, - explorer_mcp_transport=explorer_mcp_transport, - explorer_binding_env=shared_binding, - ) - rendered_roles: dict[str, str] = {} - for role, definition in definitions_by_name.items(): - rendered_roles[role] = _render_agent_toml( - definition, - explorer_binding_env=shared_binding, - explorer_mcp_transport=explorer_mcp_transport, - project_explorer_mcp=True, - ) - _atomically_replace_explorer_projection( - session_dir, - rendered_config, - rendered_roles, - ) - - -def clear_explorer_binding_env(session_dir: Path, roles: frozenset[str]) -> None: - """Atomically scrub persisted explorer secrets while retaining the broker allowlist.""" - if not isinstance(roles, frozenset): - raise ValueError("explorer role cleanup set must be a frozenset") - if not roles: - return - definitions = _bundled_agent_definitions() - definitions_by_name, explorer_mcp_transport, config_text = ( - _validate_materialized_explorer_roles( - session_dir, - definitions, - roles, - require_binding_env=False, - ) - ) - rendered_config = _render_parent_explorer_config( - config_text, - explorer_mcp_transport=explorer_mcp_transport, - explorer_binding_env=None, - ) - rendered_roles = { - role: _render_agent_toml( - definition, - explorer_mcp_transport=explorer_mcp_transport, - project_explorer_mcp=True, - ) - for role, definition in definitions_by_name.items() - } - _atomically_replace_explorer_projection( - session_dir, - rendered_config, - rendered_roles, - ) - - -def _render_parent_sandbox_config(config_text: str, sandbox_mode: str) -> str: - """Render the generated-home config with the normalized parent sandbox.""" - if sandbox_mode not in {"read-only", "workspace-write"}: - raise ValueError(f"unsupported parent sandbox mode: {sandbox_mode!r}") - lines = config_text.splitlines() - table_start = next( - (i for i, line in enumerate(lines) if line.lstrip().startswith("[")), len(lines) - ) - key_indexes = [ - i - for i, line in enumerate(lines[:table_start]) - if line.split("=", 1)[0].strip() == "sandbox_mode" - ] - if len(key_indexes) > 1: - raise ValueError("generated Codex config has duplicate top-level sandbox_mode keys") - if key_indexes: - del lines[key_indexes[0]] - if sandbox_mode == "read-only": - table_start = next( - (i for i, line in enumerate(lines) if line.lstrip().startswith("[")), len(lines) - ) - replacement = f"sandbox_mode = {_format_toml_value(sandbox_mode)}" - lines.insert(table_start, replacement) - updated = "\n".join(lines) + "\n" - parsed = tomllib.loads(updated) - if sandbox_mode == "read-only" and parsed.get("sandbox_mode") != sandbox_mode: - raise ValueError("generated Codex config did not retain the parent sandbox mode") - if sandbox_mode == "workspace-write" and "sandbox_mode" in parsed: - raise ValueError("generated Codex config retained a workspace-write sandbox pin") - return updated - - -def _render_cli_auth_store(config_text: str, execution_role: SkillExecutionRole) -> str: - """Pin ORCHESTRATOR homes to the durable file credential store.""" - if execution_role is not SkillExecutionRole.ORCHESTRATOR: - return config_text - lines = config_text.splitlines() - table_start = next( - (i for i, line in enumerate(lines) if line.lstrip().startswith("[")), len(lines) - ) - key_indexes = [ - i - for i, line in enumerate(lines[:table_start]) - if line.split("=", 1)[0].strip() == "cli_auth_credentials_store" - ] - if len(key_indexes) > 1: - raise ValueError( - "generated Codex config has duplicate top-level cli_auth_credentials_store keys" - ) - if key_indexes: - del lines[key_indexes[0]] - table_start -= 1 - lines.insert(table_start, 'cli_auth_credentials_store = "file"') - updated = "\n".join(lines) + "\n" - if tomllib.loads(updated).get("cli_auth_credentials_store") != "file": - raise ValueError("generated Codex config did not retain the file credential store") - return updated - - -def _materialize_profile_skills( - session_dir: Path, - *, - source_codex_home: Path | None = None, -) -> int: - """Symlink source-home profile skills into a generated Codex home. - - Scans the selected Codex home's ``skills`` for subdirectories containing - SKILL.md. Each is symlinked into session_dir/skills/. Falls back - to shutil.copytree if symlink creation fails. Subdirectories without - SKILL.md are skipped. Returns the number of skills materialized. - """ - source_home = Path.home() / ".codex" if source_codex_home is None else Path(source_codex_home) - profile_skills_root = source_home / "skills" - if not profile_skills_root.is_dir(): - return 0 - count = 0 - skills_base = session_dir / "skills" - skills_base.mkdir(parents=True, exist_ok=True) - entries = list(profile_skills_root.iterdir()) - for entry in entries: - if not entry.is_dir() or not (entry / "SKILL.md").is_file(): - continue - target = skills_base / entry.name - if target.exists() or target.is_symlink(): - continue - try: - target.symlink_to(entry.resolve()) - except OSError: - logger.debug( - "codex_profile_skill_symlink_failed_using_copytree", - skill=entry.name, - exc_info=True, - ) - shutil.copytree(entry, target) - count += 1 - return count - - @dataclass(frozen=True, slots=True) class CodexBackend(BackendCmdBuilderBase): source_codex_home: Path | None = None @@ -2339,7 +1110,7 @@ def adapt_skill_semantics(self, plan: SkillSemanticPlan) -> SkillSemanticAdaptat fragments.append( f"Call spawn_agent {spawn.count} time{'s' if spawn.count != 1 else ''} " f"with agent_type={native_role!r}, fork_turns='none'{policy_text}; " - "retain every returned child ID." + "retain every returned child terminal result before parent synthesis." ) if plan.concurrency is not None and plan.concurrency.required: fragments.append("Spawn all independent children before awaiting any result.") diff --git a/tests/arch/test_subpackage_isolation.py b/tests/arch/test_subpackage_isolation.py index 13d8f954c..3bb0c087c 100644 --- a/tests/arch/test_subpackage_isolation.py +++ b/tests/arch/test_subpackage_isolation.py @@ -1297,59 +1297,20 @@ def test_data_directories_are_not_python_packages() -> None: "run_skill launch denial paths before command construction (+139 net lines)", ), "execution/backends/codex.py": ( - 2444, - "REQ-CNST-010-E9: Codex backend — skill_sigil capability threading adds multi-line " - "keyword args to _ensure_skill_prefix call sites and _has_prefix guard; " - "write_guard_tool_names env injection adds 7 lines to _codex_exec_extras; " - "session_meta NDJSON support and process_name_aliases add ~8 lines; " - "explicit plugin_install_capable + supports_context_window_suffix kwargs for arch guard; " - "CodexSessionLocator nominally subclasses SessionLocator Protocol with codex_home " - "promoted from locate_session parameter to frozen dataclass field (3 net lines: " - "field declaration, blank line between field and method, SessionLocator in import block)" - "; project_log_dir method added to CodexSessionLocator (+3 net lines)" - "; session_log_path method added to CodexSessionLocator (+5 net lines)" - "; process_idle_timeout_ms field wired through build_skill_session_cmd and " - "build_food_truck_cmd CmdSpec constructors (+8 net lines)" - "; CapabilityNotSupportedError capability-gate in build_inspector_cmd (+1 net line); " - "AGENT_BACKEND_ENV_VAR injection in build_interactive_cmd merged_extras (+1 net line) " - "and build_resume_cmd _codex_exec_extras call expansion to multi-line (+2 net lines) " - "for T5-P4-A3-WP3 guard-hook backend dispatch" - "; #4585 excludes reader-only roles from ordinary Codex agent materialization (+1 line)" - "; _materialize_profile_skills function (~43 lines) for T5-P4-A4-WP2 profile skill " - "materialization into Codex session directories; registered-daemon launch identity " - "forwarding adds both required environment keys to all Codex command shapes (+8 net lines)" - "; debug-level symlink failure log in _materialize_profile_skills (+5 net lines)" - "; env-assembly consolidation via _assemble_shared_env_extras (T5-P4-A1-WP2)" - "; fleet inspector child-boundary defaults across Codex command builders (+5 net lines)" - "; explicit parameter dispositions for " - "plugin_source/output_format/exit_after_stop_delay_ms " - "replacing noqa:F841 silent discards (+18 net lines) for T5-P4-A2-WP1" - "; github_api_callable field + evidence comment in BackendCapabilities (+2 net lines)" - "; output-discipline delivery for fresh interactive sessions and generated agent " - "TOMLs (+12 net lines)" - "; _register_agent_tomls session-config registration for generated roles " - "(+39 net lines)" - "; REQ-SEM-ADAPT-001 semantic-plan adaptation remains on the registered Codex " - "backend so model and reasoning policy resolution has one authority; " - "interactive Codex startup validation, explicit generated-home construction, " - "profile probing, and durable cook-storage adapter integration remain co-located " - "with the backend whose command grammar they validate; managed native-shell " - "decision and lineage-reference injection remain adjacent to the Codex command " - "builders that own the protected environment boundary; #4443 adds canonical " - "agent-definition projection, parent/child sandbox precedence, and specialized " - "Codex explorer role registration and invocation wiring; #4478 review " - "remediation: build_skill_session_cmd/build_resume_cmd gain an " - "include_scope_discipline parameter and build_interactive_cmd's suffix call is " - "widened to codex_discipline_suffix(include_scope=True) so scope-discipline " - "delivery scoping stays adjacent to the same command builders that already own " - "prompt-injection composition (+14 net lines)" - "; #4488/#4489/#4492 explorer surface authority: setup_session_dir gains " - "explorer-role TOML exclusion filter for unbound sessions (+7 net lines) and " - "explicit session-scoped capability disposition (+1 net line); #4507 adds runtime " - "child cardinality rendering and protects the live-web bundled role (+12 net lines)" - "; #4566 pins ORCHESTRATOR file auth, durable auth linkage, and role-exact profile " - "materialization at the backend-owned generated-home setup boundary (+42 net lines); " - "launch/state MCP forwarding defaults remain in the Codex env policy (+2 net lines)", + 1300, + "REQ-CNST-010-E9-narrowed: CodexBackend class alone is 1037 lines (cmd/cmd-spec " + "grammar with build_skill_session_cmd/build_food_truck_cmd/build_interactive_cmd/validate_interactive_invocation/setup_session_dir), " + "with the four cmd-builder methods (≈465 lines) tightly coupled to CodexBackend " + "state. After #4664 decomposition, command vocabulary, env policy, state-readiness " + "probe, and session locator moved to _codex_cmd_builders.py; probes to " + "_codex_probes.py; explorer projection to _codex_explorer_projection.py. " + "CodexBackend retains all five cmd-builder methods because each touches " + "instance state (capabilities, env policy, flag vocabulary, session locator) " + "and the cmd-spec grammar is the backend's authority boundary — splitting these " + "would force a separate mutable state object and break the protocol. The " + "remaining slimmed CodexBackend is 1211 lines; cap raised to 1300 to acknowledge " + "the architectural seam that the decomposition could not cross without breaking " + "the backend dataclass invariant.", ), "execution/backends/claude.py": ( 1250, From 9ee422e8dfbd7aeac294b79b587923722f20d1b8 Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 17 Aug 2026 12:43:32 -0700 Subject: [PATCH 02/11] refactor(backends): decompose claude.py; fix _validate_codex_mcp_inventory re-export Extract ClaudeStreamParser + ClaudeResultParser to _claude_parse.py and ClaudeSessionLocator to _claude_session_locator.py. Re-export from claude.py so the public surface is unchanged. Also re-add _validate_codex_mcp_inventory to the _codex_probes re-export block (it was dropped by the linter since the import was not used inside codex.py itself; evidence_reader imports it via the canonical codex path). Refs: #4664 --- .../execution/backends/_claude_parse.py | 232 ++++++++++++ .../backends/_claude_session_locator.py | 118 +++++++ src/autoskillit/execution/backends/claude.py | 332 +----------------- src/autoskillit/execution/backends/codex.py | 1 + 4 files changed, 366 insertions(+), 317 deletions(-) create mode 100644 src/autoskillit/execution/backends/_claude_parse.py create mode 100644 src/autoskillit/execution/backends/_claude_session_locator.py diff --git a/src/autoskillit/execution/backends/_claude_parse.py b/src/autoskillit/execution/backends/_claude_parse.py new file mode 100644 index 000000000..1087e11d9 --- /dev/null +++ b/src/autoskillit/execution/backends/_claude_parse.py @@ -0,0 +1,232 @@ +"""Claude Code stream + result parsers. + +Extracted from `claude.py`. This module owns the NDJSON line classifier +(ClaudeStreamParser.parse_line) and the final-result reader +(ClaudeResultParser.parse_result / parse_stdout). The backend file imports +them and exposes them via its public surface. +""" + +from __future__ import annotations + +from collections.abc import Sequence +from dataclasses import dataclass + +from autoskillit.core import ( + AGENT_BACKEND_CLAUDE_CODE, + AgentSessionResult, + BackendEventKind, + ClaudeEventData, + CONTEXT_EXHAUSTION_MARKER, + SessionEvent, + fast_loads, +) +from autoskillit.execution.backends._claude_prompt import _extract_write_artifacts +from autoskillit.execution.process import _marker_is_standalone +from autoskillit.execution.session import parse_session_result + + +@dataclass(frozen=True, slots=True) +class ClaudeStreamParser: + completion_marker: str = "" + + def parse_line(self, line: str) -> SessionEvent | None: + line = line.strip() + if not line: + return None + try: + obj = fast_loads(line) + except (ValueError, TypeError): + return None + if not isinstance(obj, dict): + return None + + record_type = obj.get("type", "") + + if record_type in {"task_started", "task_progress", "task_notification", "task_updated"}: + task_id = obj.get("task_id") + if not isinstance(task_id, str) or not task_id.strip(): + return SessionEvent( + kind=BackendEventKind.IGNORED, + is_terminal=False, + has_marker=False, + ) + status: object = obj.get("status") + if record_type == "task_updated": + patch = obj.get("patch") + if not isinstance(patch, dict): + return SessionEvent( + kind=BackendEventKind.IGNORED, + is_terminal=False, + has_marker=False, + ) + status = patch.get("status") + active_statuses = {"pending", "running", "paused"} + terminal_statuses = {"completed", "failed", "stopped", "killed"} + if record_type in {"task_started", "task_progress"}: + task_active = True + elif status in active_statuses: + task_active = True + elif status in terminal_statuses: + task_active = False + else: + return SessionEvent( + kind=BackendEventKind.IGNORED, + is_terminal=False, + has_marker=False, + ) + return SessionEvent( + kind=BackendEventKind.TASK_LIFECYCLE, + is_terminal=False, + has_marker=False, + task_id=task_id.strip(), + task_active=task_active, + ) + + if record_type == "system": + subtype = obj.get("subtype", "") + session_id = obj.get("session_id", "") + if subtype == "api_retry": + return SessionEvent( + kind=BackendEventKind.API_RETRY, + is_terminal=False, + has_marker=False, + backend_data=ClaudeEventData( + record_type="system", + subtype="api_retry", + session_id=session_id, + raw=obj, + ), + ) + return SessionEvent( + kind=BackendEventKind.SESSION_META, + is_terminal=False, + has_marker=False, + session_id=session_id if subtype == "init" else None, + ) + + if record_type == "result": + result_field = obj.get("result", "") + if not (isinstance(result_field, str) and result_field.strip()): + return SessionEvent( + kind=BackendEventKind.IGNORED, + is_terminal=False, + has_marker=False, + ) + has_marker = bool( + self.completion_marker + and _marker_is_standalone(result_field, self.completion_marker) + ) + return SessionEvent( + kind=BackendEventKind.COMPLETION, + is_terminal=True, + has_marker=has_marker, + backend_data=ClaudeEventData( + record_type="result", + subtype=obj.get("subtype", ""), + session_id=obj.get("session_id", ""), + raw=obj, + ), + ) + + if record_type == "assistant": + if "message" not in obj and obj.get("output_tokens", -1) == 0: + flat_content = obj.get("content", []) + if isinstance(flat_content, list) and any( + isinstance(block, dict) + and block.get("type") == "text" + and CONTEXT_EXHAUSTION_MARKER in block.get("text", "").lower() + for block in flat_content + ): + return SessionEvent( + kind=BackendEventKind.TOOL_OUTPUT, + is_terminal=False, + has_marker=False, + backend_data=ClaudeEventData( + record_type="assistant", + subtype="context_exhaustion", + session_id="", + raw=obj, + ), + ) + message = obj.get("message") + content = message.get("content") if isinstance(message, dict) else None + if isinstance(content, list) and any( + isinstance(block, dict) + and block.get("type") == "tool_use" + and block.get("name") == "ScheduleWakeup" + for block in content + ): + return SessionEvent( + kind=BackendEventKind.SCHEDULE_WAKEUP, + is_terminal=False, + has_marker=False, + ) + return SessionEvent( + kind=BackendEventKind.IGNORED, + is_terminal=False, + has_marker=False, + ) + + return SessionEvent( + kind=BackendEventKind.IGNORED, + is_terminal=False, + has_marker=False, + ) + + +@dataclass(frozen=True, slots=True) +class ClaudeResultParser: + def parse_result(self, events: Sequence[SessionEvent]) -> AgentSessionResult: + session_id: str | None = None + has_completion = False + has_marker = False + last_backend_data: ClaudeEventData | None = None + for event in events: + if event.kind == BackendEventKind.SESSION_META and event.session_id: + session_id = event.session_id + if event.kind == BackendEventKind.COMPLETION: + has_completion = True + if event.has_marker: + has_marker = True + if isinstance(event.backend_data, ClaudeEventData): + last_backend_data = event.backend_data + output = "" + if last_backend_data and last_backend_data.raw: + output = last_backend_data.raw.get("result", "") + success = has_completion and has_marker + return AgentSessionResult( + success=success, + exit_code=0 if success else 1, + backend_name=AGENT_BACKEND_CLAUDE_CODE, + elapsed_seconds=0.0, + session_id=session_id, + output=output if isinstance(output, str) else "", + ) + + def parse_stdout(self, stdout: str, *, exit_code: int = 0) -> AgentSessionResult: + result = parse_session_result(stdout) + write_artifacts = _extract_write_artifacts(result.tool_uses) + return AgentSessionResult( + success=result.session_complete, + exit_code=0 if result.session_complete else 1, + backend_name=AGENT_BACKEND_CLAUDE_CODE, + elapsed_seconds=0.0, + session_id=result.session_id or None, + output=result.result, + error="\n".join(result.errors) if result.errors else "", + raw={ + "subtype": result.subtype.value, + "is_error": result.is_error, + "token_usage": result.token_usage, + "write_artifacts": write_artifacts, + "tool_uses": result.tool_uses, + "assistant_messages": result.assistant_messages, + "jsonl_context_exhausted": result.jsonl_context_exhausted, + "stop_reasons": result.stop_reasons, + "has_thinking_only_turn": result.has_thinking_only_turn, + "seen_block_types": list(result.seen_block_types), + }, + ) + + +__all__ = ["ClaudeResultParser", "ClaudeStreamParser"] \ No newline at end of file diff --git a/src/autoskillit/execution/backends/_claude_session_locator.py b/src/autoskillit/execution/backends/_claude_session_locator.py new file mode 100644 index 000000000..c75a97b4f --- /dev/null +++ b/src/autoskillit/execution/backends/_claude_session_locator.py @@ -0,0 +1,118 @@ +"""Claude Code session locator. + +Extracted from `claude.py` to keep the backend file focused on cmd/cmd-spec +grammar and parser concerns. This module owns the file-system walk that +maps a Claude session id to its persisted JSONL path and the index reader +that turns Claude's sessions-index.json into SessionSummary tuples. +""" + +from __future__ import annotations + +import json +from collections.abc import Mapping +from dataclasses import dataclass +from pathlib import Path + +from autoskillit.core import ( + AGENT_BACKEND_CLAUDE_CODE, + SessionLocator, + SessionSummary, + claude_code_log_path, + claude_code_project_dir, + read_registry, +) + + +_ORDER_GREETING_PREFIXES = ( + "Today's special:", + "Order up! Today's special:", + "Order up! The kitchen", + "Kitchen's open!", + "Table for one!", + "Fresh off the menu", + "Welcome to Good Burger, home of the Good Burger, can I take your order?", +) + + +@dataclass(frozen=True, slots=True) +class ClaudeSessionLocator(SessionLocator): + def locate_session(self, session_id: str) -> Path | None: + if not session_id or session_id.startswith(("no_session_", "crashed_")): + return None + base = Path.home() / ".claude" / "projects" + if not base.exists(): + return None + for project_dir in base.iterdir(): + if not project_dir.is_dir(): + continue + candidate = project_dir / f"{session_id}.jsonl" + if candidate.exists(): + return candidate + return None + + def project_log_dir(self, cwd: str) -> Path: + return claude_code_project_dir(cwd) + + def session_log_path(self, cwd: str, session_id: str) -> Path | None: + return claude_code_log_path(cwd, session_id) + + def list_sessions(self, cwd: str) -> tuple[SessionSummary, ...]: + normalized_cwd = str(Path(cwd).expanduser().resolve(strict=False)) + index_path = self.project_log_dir(normalized_cwd) / "sessions-index.json" + try: + entries = json.loads(index_path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError): + return () + if not isinstance(entries, list): + return () + + launch_ids_by_session_id = { + claude_session_id: launch_id + for launch_id, registry_entry in read_registry(Path(normalized_cwd)).items() + if isinstance(registry_entry, Mapping) + and isinstance( + claude_session_id := registry_entry.get("claude_session_id"), + str, + ) + } + summaries: list[SessionSummary] = [] + for entry in entries: + if not isinstance(entry, dict) or entry.get("isSidechain"): + continue + entry_cwd = entry.get("cwd") + if not isinstance(entry_cwd, str): + continue + resolved_entry_cwd = str(Path(entry_cwd).expanduser().resolve(strict=False)) + if resolved_entry_cwd != normalized_cwd: + continue + + session_id = entry.get("sessionId") + if not isinstance(session_id, str) or not session_id: + continue + first_prompt = entry.get("firstPrompt") + normalized_prompt = first_prompt if isinstance(first_prompt, str) else "" + summary = entry.get("summary") + git_branch = entry.get("gitBranch") + modified = entry.get("modified") + summaries.append( + SessionSummary( + backend_name=AGENT_BACKEND_CLAUDE_CODE, + session_id=session_id, + launch_id=launch_ids_by_session_id.get(session_id), + cwd=resolved_entry_cwd, + first_prompt=normalized_prompt, + summary=summary if isinstance(summary, str) else "", + git_branch=git_branch if isinstance(git_branch, str) else None, + modified=modified if isinstance(modified, str) else None, + is_sidechain=False, + session_type_hint=( + "order" + if normalized_prompt.startswith(_ORDER_GREETING_PREFIXES) + else "cook" + ), + ) + ) + return tuple(summaries) + + +__all__ = ["ClaudeSessionLocator"] \ No newline at end of file diff --git a/src/autoskillit/execution/backends/claude.py b/src/autoskillit/execution/backends/claude.py index b66a17dd9..d125671bf 100644 --- a/src/autoskillit/execution/backends/claude.py +++ b/src/autoskillit/execution/backends/claude.py @@ -1,3 +1,5 @@ +"""Claude Code backend implementation.""" + from __future__ import annotations import json @@ -26,7 +28,6 @@ CLAUDE_MCP_CONNECT_TIMEOUT_ENV_VAR, CLAUDE_MCP_CONNECT_TIMEOUT_MS, CLAUDE_MCP_CONNECTION_NONBLOCKING, - CONTEXT_EXHAUSTION_MARKER, NON_VARIADIC_CLAUDE_FLAGS, ORCHESTRATOR_SESSION_REQUIRED_ENV, PROVIDER_PROFILE_ENV_VAR, @@ -35,14 +36,11 @@ SESSION_TYPE_SKILL, SKILL_SESSION_REQUIRED_ENV, VARIADIC_CLAUDE_FLAGS, - AgentSessionResult, BackendCapabilities, BackendConventions, - BackendEventKind, BareResume, CapabilityNotSupportedError, ClaudeDirectoryConventions, - ClaudeEventData, ClaudeFlags, CmdSpec, CookSessionHandle, @@ -57,9 +55,6 @@ PreLaunchReadiness, ResumeSpec, SessionCheckpoint, - SessionEvent, - SessionLocator, - SessionSummary, SkillExecutionRole, SkillSemanticAdaptationResult, SkillSemanticPlan, @@ -67,14 +62,10 @@ ValidatedAddDir, YAMLError, build_agent_env, - claude_code_log_path, - claude_code_project_dir, executable_binding_matches_current_file, extract_skill_name, - fast_loads, load_yaml, pkg_root, - read_registry, truncate_text, ) from autoskillit.execution.backends._backend_cmd_builder_base import ( @@ -82,6 +73,10 @@ BackendCmdBuilderBase, FlagVocabulary, ) +from autoskillit.execution.backends._claude_parse import ( + ClaudeResultParser, + ClaudeStreamParser, +) from autoskillit.execution.backends._claude_prompt import ( _CLAUDE_SKILL_SESSION_HARDENING, _HEADLESS_ENV_HARDENING, @@ -93,15 +88,13 @@ _apply_output_format, _compose_resume_prompt, _ensure_skill_prefix, - _extract_write_artifacts, apply_prompt_injector_chain, ) +from autoskillit.execution.backends._claude_session_locator import ClaudeSessionLocator from autoskillit.execution.backends._cmd_builder import CmdBuilder from autoskillit.execution.backends._explorer_dispatch import ( CLAUDE_EXPLORATION_DISPATCH_RENDERER, ) -from autoskillit.execution.process import _marker_is_standalone -from autoskillit.execution.session import parse_session_result log = logging.getLogger(__name__) # noqa: TID251 — stdlib fallback: used before configure_logging(); structlog proxy would emit to stderr via import-time WriteLoggerFactory _EXPLORER_BINDING_REJECTION_MESSAGE = "Claude Code does not support explorer binding projection" @@ -145,25 +138,6 @@ def _claude_host_attestation_env( } -_ORDER_GREETING_PREFIXES = ( - "Today's special:", - "Order up! Today's special:", - "Order up! The kitchen", - "Kitchen's open!", - "Table for one!", - "Fresh off the menu", - "Welcome to Good Burger, home of the Good Burger, can I take your order?", -) - -__all__ = [ - "ClaudeCodeBackend", - "ClaudeEnvPolicy", - "ClaudeResultParser", - "ClaudeSessionLocator", - "ClaudeStreamParser", -] - - @dataclass(frozen=True, slots=True) class ClaudeEnvPolicy: def build_env( @@ -176,289 +150,13 @@ def build_env( return dict(build_agent_env(base=base_env, extras=extras, required=required)) -@dataclass(frozen=True, slots=True) -class ClaudeSessionLocator(SessionLocator): - def locate_session(self, session_id: str) -> Path | None: - if not session_id or session_id.startswith(("no_session_", "crashed_")): - return None - base = Path.home() / ".claude" / "projects" - if not base.exists(): - return None - for project_dir in base.iterdir(): - if not project_dir.is_dir(): - continue - candidate = project_dir / f"{session_id}.jsonl" - if candidate.exists(): - return candidate - return None - - def project_log_dir(self, cwd: str) -> Path: - return claude_code_project_dir(cwd) - - def session_log_path(self, cwd: str, session_id: str) -> Path | None: - return claude_code_log_path(cwd, session_id) - - def list_sessions(self, cwd: str) -> tuple[SessionSummary, ...]: - normalized_cwd = str(Path(cwd).expanduser().resolve(strict=False)) - index_path = self.project_log_dir(normalized_cwd) / "sessions-index.json" - try: - entries = json.loads(index_path.read_text(encoding="utf-8")) - except (OSError, json.JSONDecodeError): - return () - if not isinstance(entries, list): - return () - - launch_ids_by_session_id = { - claude_session_id: launch_id - for launch_id, registry_entry in read_registry(Path(normalized_cwd)).items() - if isinstance(registry_entry, Mapping) - and isinstance( - claude_session_id := registry_entry.get("claude_session_id"), - str, - ) - } - summaries: list[SessionSummary] = [] - for entry in entries: - if not isinstance(entry, dict) or entry.get("isSidechain"): - continue - entry_cwd = entry.get("cwd") - if not isinstance(entry_cwd, str): - continue - resolved_entry_cwd = str(Path(entry_cwd).expanduser().resolve(strict=False)) - if resolved_entry_cwd != normalized_cwd: - continue - - session_id = entry.get("sessionId") - if not isinstance(session_id, str) or not session_id: - continue - first_prompt = entry.get("firstPrompt") - normalized_prompt = first_prompt if isinstance(first_prompt, str) else "" - summary = entry.get("summary") - git_branch = entry.get("gitBranch") - modified = entry.get("modified") - summaries.append( - SessionSummary( - backend_name=AGENT_BACKEND_CLAUDE_CODE, - session_id=session_id, - launch_id=launch_ids_by_session_id.get(session_id), - cwd=resolved_entry_cwd, - first_prompt=normalized_prompt, - summary=summary if isinstance(summary, str) else "", - git_branch=git_branch if isinstance(git_branch, str) else None, - modified=modified if isinstance(modified, str) else None, - is_sidechain=False, - session_type_hint=( - "order" - if normalized_prompt.startswith(_ORDER_GREETING_PREFIXES) - else "cook" - ), - ) - ) - return tuple(summaries) - - -@dataclass(frozen=True, slots=True) -class ClaudeStreamParser: - completion_marker: str = "" - - def parse_line(self, line: str) -> SessionEvent | None: - line = line.strip() - if not line: - return None - try: - obj = fast_loads(line) - except (ValueError, TypeError): - return None - if not isinstance(obj, dict): - return None - - record_type = obj.get("type", "") - - if record_type in {"task_started", "task_progress", "task_notification", "task_updated"}: - task_id = obj.get("task_id") - if not isinstance(task_id, str) or not task_id.strip(): - return SessionEvent( - kind=BackendEventKind.IGNORED, - is_terminal=False, - has_marker=False, - ) - status: object = obj.get("status") - if record_type == "task_updated": - patch = obj.get("patch") - if not isinstance(patch, dict): - return SessionEvent( - kind=BackendEventKind.IGNORED, - is_terminal=False, - has_marker=False, - ) - status = patch.get("status") - active_statuses = {"pending", "running", "paused"} - terminal_statuses = {"completed", "failed", "stopped", "killed"} - if record_type in {"task_started", "task_progress"}: - task_active = True - elif status in active_statuses: - task_active = True - elif status in terminal_statuses: - task_active = False - else: - return SessionEvent( - kind=BackendEventKind.IGNORED, - is_terminal=False, - has_marker=False, - ) - return SessionEvent( - kind=BackendEventKind.TASK_LIFECYCLE, - is_terminal=False, - has_marker=False, - task_id=task_id.strip(), - task_active=task_active, - ) - - if record_type == "system": - subtype = obj.get("subtype", "") - session_id = obj.get("session_id", "") - if subtype == "api_retry": - return SessionEvent( - kind=BackendEventKind.API_RETRY, - is_terminal=False, - has_marker=False, - backend_data=ClaudeEventData( - record_type="system", - subtype="api_retry", - session_id=session_id, - raw=obj, - ), - ) - return SessionEvent( - kind=BackendEventKind.SESSION_META, - is_terminal=False, - has_marker=False, - session_id=session_id if subtype == "init" else None, - ) - - if record_type == "result": - result_field = obj.get("result", "") - if not (isinstance(result_field, str) and result_field.strip()): - return SessionEvent( - kind=BackendEventKind.IGNORED, - is_terminal=False, - has_marker=False, - ) - has_marker = bool( - self.completion_marker - and _marker_is_standalone(result_field, self.completion_marker) - ) - return SessionEvent( - kind=BackendEventKind.COMPLETION, - is_terminal=True, - has_marker=has_marker, - backend_data=ClaudeEventData( - record_type="result", - subtype=obj.get("subtype", ""), - session_id=obj.get("session_id", ""), - raw=obj, - ), - ) - - if record_type == "assistant": - if "message" not in obj and obj.get("output_tokens", -1) == 0: - flat_content = obj.get("content", []) - if isinstance(flat_content, list) and any( - isinstance(block, dict) - and block.get("type") == "text" - and CONTEXT_EXHAUSTION_MARKER in block.get("text", "").lower() - for block in flat_content - ): - return SessionEvent( - kind=BackendEventKind.TOOL_OUTPUT, - is_terminal=False, - has_marker=False, - backend_data=ClaudeEventData( - record_type="assistant", - subtype="context_exhaustion", - session_id="", - raw=obj, - ), - ) - message = obj.get("message") - content = message.get("content") if isinstance(message, dict) else None - if isinstance(content, list) and any( - isinstance(block, dict) - and block.get("type") == "tool_use" - and block.get("name") == "ScheduleWakeup" - for block in content - ): - return SessionEvent( - kind=BackendEventKind.SCHEDULE_WAKEUP, - is_terminal=False, - has_marker=False, - ) - return SessionEvent( - kind=BackendEventKind.IGNORED, - is_terminal=False, - has_marker=False, - ) - - return SessionEvent( - kind=BackendEventKind.IGNORED, - is_terminal=False, - has_marker=False, - ) - - -@dataclass(frozen=True, slots=True) -class ClaudeResultParser: - def parse_result(self, events: Sequence[SessionEvent]) -> AgentSessionResult: - session_id: str | None = None - has_completion = False - has_marker = False - last_backend_data: ClaudeEventData | None = None - for event in events: - if event.kind == BackendEventKind.SESSION_META and event.session_id: - session_id = event.session_id - if event.kind == BackendEventKind.COMPLETION: - has_completion = True - if event.has_marker: - has_marker = True - if isinstance(event.backend_data, ClaudeEventData): - last_backend_data = event.backend_data - output = "" - if last_backend_data and last_backend_data.raw: - output = last_backend_data.raw.get("result", "") - success = has_completion and has_marker - return AgentSessionResult( - success=success, - exit_code=0 if success else 1, - backend_name=AGENT_BACKEND_CLAUDE_CODE, - elapsed_seconds=0.0, - session_id=session_id, - output=output if isinstance(output, str) else "", - ) - - def parse_stdout(self, stdout: str, *, exit_code: int = 0) -> AgentSessionResult: - result = parse_session_result(stdout) - write_artifacts = _extract_write_artifacts(result.tool_uses) - return AgentSessionResult( - success=result.session_complete, - exit_code=0 if result.session_complete else 1, - backend_name=AGENT_BACKEND_CLAUDE_CODE, - elapsed_seconds=0.0, - session_id=result.session_id or None, - output=result.result, - error="\n".join(result.errors) if result.errors else "", - raw={ - "subtype": result.subtype.value, - "is_error": result.is_error, - "token_usage": result.token_usage, - "write_artifacts": write_artifacts, - "tool_uses": result.tool_uses, - "assistant_messages": result.assistant_messages, - "jsonl_context_exhausted": result.jsonl_context_exhausted, - "stop_reasons": result.stop_reasons, - "has_thinking_only_turn": result.has_thinking_only_turn, - "seen_block_types": list(result.seen_block_types), - }, - ) +__all__ = [ + "ClaudeCodeBackend", + "ClaudeEnvPolicy", + "ClaudeResultParser", + "ClaudeSessionLocator", + "ClaudeStreamParser", +] @dataclass(frozen=True, slots=True) @@ -1231,4 +929,4 @@ def build_inspector_cmd(self, prompt: str, *, model: str = "") -> CmdSpec: def _ignore_child_identity(pid: int, pgid: int) -> None: - del pid, pgid + del pid, pgid \ No newline at end of file diff --git a/src/autoskillit/execution/backends/codex.py b/src/autoskillit/execution/backends/codex.py index d1cf5a031..da353da56 100644 --- a/src/autoskillit/execution/backends/codex.py +++ b/src/autoskillit/execution/backends/codex.py @@ -135,6 +135,7 @@ # can keep importing them from the canonical codex module path. from autoskillit.execution.backends._codex_probes import ( _BoundedProbeResult, # noqa: F401 + _validate_codex_mcp_inventory, _validate_global_codex_home, _validate_inert_rollout_paths, _validate_mcp_probe, From 685fd8ed807d80e1629f6b81fb781f45e8e294fd Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 17 Aug 2026 12:45:08 -0700 Subject: [PATCH 03/11] refactor(backends): extract _codex_fs_atomic.py from _codex_session_storage.py Move stateless filesystem primitives (lexists/fsync/atomic_json/write_reconciliation_audit/ read_bounded/ensure_directory_chain/decode_mount_path/filesystem_mount_root/ filesystem_type/replace_symlink) to _codex_fs_atomic.py. The transaction- boundary core (CodexSessionStore, CodexInteractiveSessionLease, _FileLease) remains in _codex_session_storage.py. Update DURABLE_ARTIFACT_WRITERS registry and AST-target guard in test_durable_artifact_writers_guard.py to point at the new module location. Narrow E13 exemption to 1500 lines to accommodate the remaining core after extraction. Refs: #4664 --- src/autoskillit/core/types/_type_constants.py | 2 +- .../execution/backends/_codex_fs_atomic.py | 226 ++++++++++++++++++ .../backends/_codex_session_storage.py | 218 +---------------- .../test_durable_artifact_writers_guard.py | 4 +- tests/arch/test_subpackage_isolation.py | 15 +- 5 files changed, 251 insertions(+), 214 deletions(-) create mode 100644 src/autoskillit/execution/backends/_codex_fs_atomic.py diff --git a/src/autoskillit/core/types/_type_constants.py b/src/autoskillit/core/types/_type_constants.py index 303203c21..b5a98a84d 100644 --- a/src/autoskillit/core/types/_type_constants.py +++ b/src/autoskillit/core/types/_type_constants.py @@ -559,7 +559,7 @@ def _validate_durable_artifact_writer_defs( ), DurableArtifactWriterDef( writer=( - "autoskillit.execution.backends._codex_session_storage:_write_reconciliation_audit" + "autoskillit.execution.backends._codex_fs_atomic:_write_reconciliation_audit" ), artifact=( "immutable operator authorization records for explicit Codex attempt-view " diff --git a/src/autoskillit/execution/backends/_codex_fs_atomic.py b/src/autoskillit/execution/backends/_codex_fs_atomic.py new file mode 100644 index 000000000..eab3b9a5a --- /dev/null +++ b/src/autoskillit/execution/backends/_codex_fs_atomic.py @@ -0,0 +1,226 @@ +"""Stateless filesystem primitives used by the Codex session storage layer. + +Extracted from `_codex_session_storage.py`. These helpers are pure with +respect to module-level state — they only wrap the stdlib (`os`, `pathlib`, +`subprocess`) and the parsed `_codex_parse` helpers. The transaction-boundary +core (`CodexSessionStore`, `CodexInteractiveSessionLease`, `_FileLease`) +remains in `_codex_session_storage.py`. +""" + +from __future__ import annotations + +import json +import os +import plistlib +import re +import stat +import subprocess +import sys +import time +from pathlib import Path + +from autoskillit.core import get_logger + +logger = get_logger(__name__) + + +def _lexists(path: Path) -> bool: + return os.path.lexists(path) + + +def _require_real_directory(path: Path, *, label: str) -> None: + try: + mode = path.lstat().st_mode + except FileNotFoundError as exc: + raise RuntimeError(f"Missing {label}: {path}") from exc + if stat.S_ISLNK(mode) or not stat.S_ISDIR(mode): + raise RuntimeError(f"{label} must be a non-symlink directory: {path}") + + +def _fsync_directory(path: Path) -> None: + _require_real_directory(path, label="fsync directory") + flags = os.O_RDONLY | getattr(os, "O_DIRECTORY", 0) + flags |= getattr(os, "O_NOFOLLOW", 0) + fd = os.open(path, flags) + try: + os.fsync(fd) + finally: + os.close(fd) + + +def _atomic_json(path: Path, payload: object) -> None: + path.parent.mkdir(parents=True, exist_ok=True) + _require_real_directory(path.parent, label="JSON parent") + if _lexists(path) and path.is_symlink(): + raise RuntimeError(f"Refusing to replace symlink JSON destination: {path}") + encoded = (json.dumps(payload, sort_keys=True, separators=(",", ":")) + "\n").encode() + temporary = path.with_name(f".{path.name}.{os.getpid()}.{time.time_ns()}.tmp") + flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL + fd = os.open(temporary, flags, 0o600) + try: + view = memoryview(encoded) + while view: + written = os.write(fd, view) + view = view[written:] + os.fsync(fd) + except BaseException: + try: + temporary.unlink() + except FileNotFoundError: + pass + raise + finally: + os.close(fd) + os.replace(temporary, path) + _fsync_directory(path.parent) + + +def _write_reconciliation_audit(path: Path, payload: object) -> None: + """Publish one immutable, crash-safe reconciliation authorization.""" + _require_real_directory(path.parent, label="reconciliation audit root") + if _lexists(path): + raise FileExistsError(f"Reconciliation audit already exists: {path.name}") + encoded = (json.dumps(payload, sort_keys=True, separators=(",", ":")) + "\n").encode() + temporary = path.with_name(f".{path.name}.{os.getpid()}.{time.time_ns()}.tmp") + flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_NOFOLLOW", 0) + fd = os.open(temporary, flags, 0o600) + try: + view = memoryview(encoded) + while view: + written = os.write(fd, view) + view = view[written:] + os.fsync(fd) + except BaseException: + try: + temporary.unlink() + except FileNotFoundError: + pass + raise + finally: + os.close(fd) + try: + os.link(temporary, path, follow_symlinks=False) + _fsync_directory(path.parent) + finally: + try: + temporary.unlink() + finally: + _fsync_directory(path.parent) + + +def _read_bounded(path: Path, limit: int) -> bytes: + flags = os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0) + fd = os.open(path, flags) + try: + file_stat = os.fstat(fd) + if not stat.S_ISREG(file_stat.st_mode): + raise ValueError(f"{path} is not a regular file") + chunks: list[bytes] = [] + total = 0 + while total <= limit: + chunk = os.read(fd, min(64 * 1024, limit + 1 - total)) + if not chunk: + break + chunks.append(chunk) + total += len(chunk) + if total > limit: + raise ValueError(f"{path.name} exceeds the {limit}-byte bound") + return b"".join(chunks) + finally: + os.close(fd) + + +def _ensure_directory_chain(root: Path, relative: Path) -> Path: + _require_real_directory(root, label="storage root") + cursor = root + for part in relative.parts: + if part in {"", ".", ".."} or "/" in part or "\\" in part: + raise RuntimeError(f"Unsafe storage directory component: {part!r}") + next_path = cursor / part + created = False + try: + next_path.mkdir() + created = True + except FileExistsError: + pass + _require_real_directory(next_path, label="storage directory") + if created: + _fsync_directory(cursor) + cursor = next_path + return cursor + + +def _decode_mount_path(value: str) -> str: + return re.sub( + r"\\([0-7]{3})", + lambda match: chr(int(match.group(1), 8)), + value, + ) + + +def _filesystem_mount_root(path: Path) -> Path: + resolved = path.resolve(strict=True) + device = resolved.stat().st_dev + mount_root = resolved + while mount_root.parent != mount_root: + parent = mount_root.parent + if parent.stat().st_dev != device: + break + mount_root = parent + return mount_root + + +def _filesystem_type(path: Path) -> str: + if sys.platform == "darwin": + try: + mount_root = _filesystem_mount_root(path) + result = subprocess.run( + ("/usr/sbin/diskutil", "info", "-plist", str(mount_root)), + capture_output=True, + check=False, + timeout=5, + ) + if result.returncode != 0: + raise RuntimeError("Unable to classify the Codex storage filesystem with diskutil") + payload = plistlib.loads(result.stdout) + filesystem_type = payload.get("FilesystemType") + if not isinstance(filesystem_type, str) or not filesystem_type: + raise RuntimeError("diskutil did not report a filesystem type") + return filesystem_type.lower() + except (OSError, plistlib.InvalidFileException) as exc: + raise RuntimeError("Unable to classify the Codex storage filesystem") from exc + if not sys.platform.startswith("linux"): + raise RuntimeError(f"Codex durable views cannot classify filesystems on {sys.platform}") + try: + raw = _read_bounded(Path("/proc/self/mountinfo"), 4 * 1024 * 1024) + except (OSError, ValueError) as exc: + raise RuntimeError("Unable to classify the Codex storage filesystem") from exc + resolved = path.resolve(strict=True) + selected: tuple[int, str] | None = None + for raw_line in raw.decode("utf-8", errors="strict").splitlines(): + before, separator, after = raw_line.partition(" - ") + if not separator: + continue + fields = before.split() + trailing = after.split() + if len(fields) < 5 or not trailing: + continue + mount_path = Path(_decode_mount_path(fields[4])) + if resolved == mount_path or resolved.is_relative_to(mount_path): + length = len(mount_path.parts) + if selected is None or length > selected[0]: + selected = (length, trailing[0]) + if selected is None: + raise RuntimeError(f"Unable to classify Codex storage mount: {resolved}") + return selected[1] + + +def _replace_symlink(path: Path, target: Path) -> None: + temporary = path.with_name(f".{path.name}.{os.getpid()}.link") + try: + temporary.unlink() + except FileNotFoundError: + pass + temporary.symlink_to(target) + os.replace(temporary, path) + _fsync_directory(path.parent) \ No newline at end of file diff --git a/src/autoskillit/execution/backends/_codex_session_storage.py b/src/autoskillit/execution/backends/_codex_session_storage.py index 460a25715..8616ecbf4 100644 --- a/src/autoskillit/execution/backends/_codex_session_storage.py +++ b/src/autoskillit/execution/backends/_codex_session_storage.py @@ -6,12 +6,9 @@ import hashlib import json import os -import plistlib import shutil import socket import stat -import subprocess -import sys import time from collections.abc import Mapping, Sequence from contextlib import AbstractContextManager @@ -37,6 +34,19 @@ default_log_dir, get_logger, ) +from autoskillit.execution.backends._codex_fs_atomic import ( # noqa: F401 — re-exported + _atomic_json, + _decode_mount_path, + _ensure_directory_chain, + _filesystem_mount_root, + _filesystem_type, + _fsync_directory, + _lexists, + _read_bounded, + _replace_symlink, + _require_real_directory, + _write_reconciliation_audit, +) from autoskillit.execution.backends._codex_parse import ( _identity, _preserves_rollout_prefix, @@ -89,208 +99,6 @@ def codex_session_index_path(log_dir: Path | None = None) -> Path: return root.expanduser().resolve(strict=False) / _INDEX_NAME -def _lexists(path: Path) -> bool: - return os.path.lexists(path) - - -def _require_real_directory(path: Path, *, label: str) -> None: - try: - mode = path.lstat().st_mode - except FileNotFoundError as exc: - raise RuntimeError(f"Missing {label}: {path}") from exc - if stat.S_ISLNK(mode) or not stat.S_ISDIR(mode): - raise RuntimeError(f"{label} must be a non-symlink directory: {path}") - - -def _fsync_directory(path: Path) -> None: - _require_real_directory(path, label="fsync directory") - flags = os.O_RDONLY | getattr(os, "O_DIRECTORY", 0) - flags |= getattr(os, "O_NOFOLLOW", 0) - fd = os.open(path, flags) - try: - os.fsync(fd) - finally: - os.close(fd) - - -def _atomic_json(path: Path, payload: object) -> None: - path.parent.mkdir(parents=True, exist_ok=True) - _require_real_directory(path.parent, label="JSON parent") - if _lexists(path) and path.is_symlink(): - raise RuntimeError(f"Refusing to replace symlink JSON destination: {path}") - encoded = (json.dumps(payload, sort_keys=True, separators=(",", ":")) + "\n").encode() - temporary = path.with_name(f".{path.name}.{os.getpid()}.{time.time_ns()}.tmp") - flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL - fd = os.open(temporary, flags, 0o600) - try: - view = memoryview(encoded) - while view: - written = os.write(fd, view) - view = view[written:] - os.fsync(fd) - except BaseException: - try: - temporary.unlink() - except FileNotFoundError: - pass - raise - finally: - os.close(fd) - os.replace(temporary, path) - _fsync_directory(path.parent) - - -def _write_reconciliation_audit(path: Path, payload: object) -> None: - """Publish one immutable, crash-safe reconciliation authorization.""" - _require_real_directory(path.parent, label="reconciliation audit root") - if _lexists(path): - raise FileExistsError(f"Reconciliation audit already exists: {path.name}") - encoded = (json.dumps(payload, sort_keys=True, separators=(",", ":")) + "\n").encode() - temporary = path.with_name(f".{path.name}.{os.getpid()}.{time.time_ns()}.tmp") - flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_NOFOLLOW", 0) - fd = os.open(temporary, flags, 0o600) - try: - view = memoryview(encoded) - while view: - written = os.write(fd, view) - view = view[written:] - os.fsync(fd) - except BaseException: - try: - temporary.unlink() - except FileNotFoundError: - pass - raise - finally: - os.close(fd) - try: - os.link(temporary, path, follow_symlinks=False) - _fsync_directory(path.parent) - finally: - try: - temporary.unlink() - finally: - _fsync_directory(path.parent) - - -def _read_bounded(path: Path, limit: int) -> bytes: - flags = os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0) - fd = os.open(path, flags) - try: - file_stat = os.fstat(fd) - if not stat.S_ISREG(file_stat.st_mode): - raise ValueError(f"{path} is not a regular file") - chunks: list[bytes] = [] - total = 0 - while total <= limit: - chunk = os.read(fd, min(64 * 1024, limit + 1 - total)) - if not chunk: - break - chunks.append(chunk) - total += len(chunk) - if total > limit: - raise ValueError(f"{path.name} exceeds the {limit}-byte bound") - return b"".join(chunks) - finally: - os.close(fd) - - -def _ensure_directory_chain(root: Path, relative: Path) -> Path: - _require_real_directory(root, label="storage root") - cursor = root - for part in relative.parts: - if part in {"", ".", ".."} or "/" in part or "\\" in part: - raise RuntimeError(f"Unsafe storage directory component: {part!r}") - next_path = cursor / part - created = False - try: - next_path.mkdir() - created = True - except FileExistsError: - pass - _require_real_directory(next_path, label="storage directory") - if created: - _fsync_directory(cursor) - cursor = next_path - return cursor - - -def _decode_mount_path(value: str) -> str: - return re.sub( - r"\\([0-7]{3})", - lambda match: chr(int(match.group(1), 8)), - value, - ) - - -def _filesystem_mount_root(path: Path) -> Path: - resolved = path.resolve(strict=True) - device = resolved.stat().st_dev - mount_root = resolved - while mount_root.parent != mount_root: - parent = mount_root.parent - if parent.stat().st_dev != device: - break - mount_root = parent - return mount_root - - -def _filesystem_type(path: Path) -> str: - if sys.platform == "darwin": - try: - mount_root = _filesystem_mount_root(path) - result = subprocess.run( - ("/usr/sbin/diskutil", "info", "-plist", str(mount_root)), - capture_output=True, - check=False, - timeout=5, - ) - if result.returncode != 0: - raise RuntimeError("Unable to classify the Codex storage filesystem with diskutil") - payload = plistlib.loads(result.stdout) - filesystem_type = payload.get("FilesystemType") - if not isinstance(filesystem_type, str) or not filesystem_type: - raise RuntimeError("diskutil did not report a filesystem type") - return filesystem_type.lower() - except (OSError, plistlib.InvalidFileException) as exc: - raise RuntimeError("Unable to classify the Codex storage filesystem") from exc - if not sys.platform.startswith("linux"): - raise RuntimeError(f"Codex durable views cannot classify filesystems on {sys.platform}") - try: - raw = _read_bounded(Path("/proc/self/mountinfo"), 4 * 1024 * 1024) - except (OSError, ValueError) as exc: - raise RuntimeError("Unable to classify the Codex storage filesystem") from exc - resolved = path.resolve(strict=True) - selected: tuple[int, str] | None = None - for raw_line in raw.decode("utf-8", errors="strict").splitlines(): - before, separator, after = raw_line.partition(" - ") - if not separator: - continue - fields = before.split() - trailing = after.split() - if len(fields) < 5 or not trailing: - continue - mount_path = Path(_decode_mount_path(fields[4])) - if resolved == mount_path or resolved.is_relative_to(mount_path): - length = len(mount_path.parts) - if selected is None or length > selected[0]: - selected = (length, trailing[0]) - if selected is None: - raise RuntimeError(f"Unable to classify Codex storage mount: {resolved}") - return selected[1] - - -def _replace_symlink(path: Path, target: Path) -> None: - temporary = path.with_name(f".{path.name}.{os.getpid()}.link") - try: - temporary.unlink() - except FileNotFoundError: - pass - temporary.symlink_to(target) - os.replace(temporary, path) - _fsync_directory(path.parent) - - @dataclass(slots=True) class _FileLease: path: Path diff --git a/tests/arch/test_durable_artifact_writers_guard.py b/tests/arch/test_durable_artifact_writers_guard.py index fcfb5b806..5964725f2 100644 --- a/tests/arch/test_durable_artifact_writers_guard.py +++ b/tests/arch/test_durable_artifact_writers_guard.py @@ -222,7 +222,7 @@ def test_registered_writers_have_a_matching_call_site() -> None: def test_codex_reconciliation_audit_no_clobber_writer_is_registered() -> None: """The immutable audit hard-link publisher remains a registered durable writer.""" - rel = "execution/backends/_codex_session_storage.py" + rel = "execution/backends/_codex_fs_atomic.py" path = SRC_ROOT / rel tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) writer = next( @@ -238,5 +238,5 @@ def test_codex_reconciliation_audit_no_clobber_writer_is_registered() -> None: assert {"link", "fsync", "unlink"} <= calls assert ( - "autoskillit.execution.backends._codex_session_storage:_write_reconciliation_audit" + "autoskillit.execution.backends._codex_fs_atomic:_write_reconciliation_audit" ) in _REGISTERED_WRITERS diff --git a/tests/arch/test_subpackage_isolation.py b/tests/arch/test_subpackage_isolation.py index 3bb0c087c..3e3c79549 100644 --- a/tests/arch/test_subpackage_isolation.py +++ b/tests/arch/test_subpackage_isolation.py @@ -1360,12 +1360,15 @@ def test_data_directories_are_not_python_packages() -> None: "skill-invalidity threading and the completed explorer sidecar migration.", ), "execution/backends/_codex_session_storage.py": ( - 1650, - "REQ-CNST-010-E13: Codex interactive rollout storage is one transaction boundary " - "covering inode-preserving staging, process/thread/view leases, promotion, index " - "publication, manifest validation, crash recovery, and explicit legacy-view " - "reconciliation; splitting those lock-coupled state transitions would duplicate " - "invariants across independently mutable modules", + 1500, + "REQ-CNST-010-E13-narrowed: CodexSessionStore + CodexInteractiveSessionLease + " + "_FileLease transaction-boundary core only; stateless FS primitives extracted to " + "_codex_fs_atomic.py (RE: #4664). The transaction-boundary core remains one " + "lock-coupled module — splitting _FileLease / CodexInteractiveSessionLease / " + "CodexSessionStore across multiple files would duplicate the inode-preserving " + "staging, process/thread/view leases, promotion, index publication, manifest " + "validation, crash recovery, and explicit legacy-view reconciliation invariants. " + "Cap raised to 1500 lines to accommodate the core without the stateless helpers.", ), "workspace/session_skills.py": ( 1400, From 6d7427a2aa60ce5d4830b75eadfa47a1b2c1aa8f Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 17 Aug 2026 12:48:44 -0700 Subject: [PATCH 04/11] refactor(github_review): extract _reconcile_payload and _finalize to _poster_finalize.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move post-attempt finalize/reconcile helpers out of poster.py (997 → 854 lines) into _poster_finalize.py as module-level functions. The mutation authority remains in GitHubReviewLedger — these are pure shaping helpers that operate on receipts/result tuples post-attempt. _EXPECTED_REMOTE_STATES and _FINAL_STATES constants move with the helpers that consume them. Refs: #4664 --- .../github_review/_poster_finalize.py | 224 ++++++++++++++++++ .../execution/github_review/poster.py | 179 ++------------ 2 files changed, 242 insertions(+), 161 deletions(-) create mode 100644 src/autoskillit/execution/github_review/_poster_finalize.py diff --git a/src/autoskillit/execution/github_review/_poster_finalize.py b/src/autoskillit/execution/github_review/_poster_finalize.py new file mode 100644 index 000000000..05a43a164 --- /dev/null +++ b/src/autoskillit/execution/github_review/_poster_finalize.py @@ -0,0 +1,224 @@ +"""Module-level `_reconcile_payload` and `_finalize` helpers for the poster. + +Extracted from `poster.py`. These are post-attempt helpers that finalize +attempt + receipt publication; they are NOT mutation authorities (the +ledger retains all mutation authority). Moving them to module-level +functions keeps `poster.py` focused on the state machine. + +`_EXPECTED_REMOTE_STATES` and `_FINAL_STATES` also live here because +only these helpers consume them. +""" + +from __future__ import annotations + +from collections.abc import Mapping +from typing import Any + +from autoskillit.core import ( + GitHubReviewFindingDisposition, + GitHubReviewPostResult, + GitHubReviewReceipt, + GitHubReviewRequest, + ReviewFindingDispositionKind, + ReviewOperationState, + ReviewReconciliationResult, +) + +from . import _poster_support + +_EXPECTED_REMOTE_STATES = { + "APPROVE": "APPROVED", + "REQUEST_CHANGES": "CHANGES_REQUESTED", + "COMMENT": "COMMENTED", +} +_FINAL_STATES = frozenset({ReviewOperationState.SUCCEEDED, ReviewOperationState.RECONCILED}) + + +async def reconcile_payload( + *, + gateway, + request: GitHubReviewRequest, + operation_key: str, + payload: Mapping[str, Any], + findings: tuple[_poster_support.CanonicalFinding, ...], + authenticated_login: str, +) -> _poster_support.Reconciliation: + reviews = await gateway.list_reviews(request.repository, request.pr_number) + if not reviews.succeeded or not isinstance(reviews.data, list): + return _poster_support.Reconciliation( + ReviewReconciliationResult.UNCERTAIN, + error=reviews.error or "could not read pull-request reviews", + ) + marker = _poster_support.OPERATION_MARKER.format(key=operation_key) + expected_body = str(payload["body"]) + expected_event = str(payload["event"]) + expected_state = _EXPECTED_REMOTE_STATES[expected_event] + matches = [ + item + for item in reviews.data + if isinstance(item, Mapping) + and item.get("body") == expected_body + and marker in str(item.get("body", "")) + and item.get("commit_id") == request.head_sha + and item.get("state") == expected_state + and _poster_support.nested_string(item, "user", "login") == authenticated_login + and _poster_support.positive_int(item.get("id")) is not None + ] + if not matches: + return _poster_support.Reconciliation(ReviewReconciliationResult.NOT_FOUND) + if len(matches) != 1: + return _poster_support.Reconciliation( + ReviewReconciliationResult.UNCERTAIN, + error="multiple reviews matched one operation marker", + ) + review_id = _poster_support.positive_int(matches[0].get("id")) + if review_id is None: + return _poster_support.Reconciliation( + ReviewReconciliationResult.UNCERTAIN, + error="matched review omitted a valid id", + ) + comments = await gateway.list_review_comments( + request.repository, + request.pr_number, + review_id, + ) + if not comments.succeeded or not isinstance(comments.data, list): + return _poster_support.Reconciliation( + ReviewReconciliationResult.UNCERTAIN, + error=comments.error or "could not read review comments", + ) + expected_comments = payload.get("comments") + if not isinstance(expected_comments, list) or len(expected_comments) != len(findings): + return _poster_support.Reconciliation( + ReviewReconciliationResult.UNCERTAIN, + error="persisted attempt comments do not match canonical findings", + ) + matched_ids: list[tuple[int, int]] = [] + used_remote_ids: set[int] = set() + for finding, expected in zip(findings, expected_comments, strict=True): + if not isinstance(expected, Mapping): + return _poster_support.Reconciliation( + ReviewReconciliationResult.UNCERTAIN, + error="persisted attempt comment is malformed", + ) + candidates = [ + item + for item in comments.data + if isinstance(item, Mapping) + and _poster_support.remote_comment_matches(item, expected, finding.digest) + ] + if len(candidates) != 1: + return _poster_support.Reconciliation( + ReviewReconciliationResult.UNCERTAIN, + error="review comments did not exactly match the attempted finding set", + ) + comment_id = _poster_support.positive_int(candidates[0].get("id")) + if comment_id is None or comment_id in used_remote_ids: + return _poster_support.Reconciliation( + ReviewReconciliationResult.UNCERTAIN, + error="review comment ids were missing or duplicated", + ) + used_remote_ids.add(comment_id) + matched_ids.append((finding.canonical_index, comment_id)) + marker_count = sum( + isinstance(item, Mapping) + and _poster_support.FINDING_MARKER.split("{digest}", 1)[0] in str(item.get("body", "")) + for item in comments.data + ) + if marker_count != len(findings): + return _poster_support.Reconciliation( + ReviewReconciliationResult.UNCERTAIN, + error="review contained an unexpected AutoSkillit finding marker set", + ) + return _poster_support.Reconciliation( + ReviewReconciliationResult.MATCHED, + review_id=review_id, + comment_ids=tuple(matched_ids), + ) + + +def finalize( + *, + ledger, + wall_clock, + request: GitHubReviewRequest, + operation_key: str, + findings: tuple[_poster_support.CanonicalFinding, ...], + omitted: tuple[GitHubReviewFindingDisposition, ...], + effective_event: str, + attempt_digest: str, + response_class, + state: ReviewOperationState, + reconciliation: _poster_support.Reconciliation, + executed_mutations: int, +) -> GitHubReviewPostResult: + if state not in _FINAL_STATES or reconciliation.review_id is None: + raise ValueError("cannot finalize an unverified review operation") + ids_by_index = dict(reconciliation.comment_ids) + dispositions = list(omitted) + for finding in findings: + remote_comment_id = ids_by_index.get(finding.canonical_index) + if remote_comment_id is None: + raise ValueError("verified review omitted a posted comment id") + dispositions.append( + GitHubReviewFindingDisposition( + original_index=finding.original_index, + canonical_index=finding.canonical_index, + kind=ReviewFindingDispositionKind.POSTED, + remote_comment_id=remote_comment_id, + ) + ) + dispositions.sort(key=lambda item: item.original_index) + if len(dispositions) != len(request.comments): + raise ValueError("review finding accounting is not exhaustive") + now = wall_clock() + all_findings = _poster_support.canonical_findings(request) + receipt = GitHubReviewReceipt( + schema_version=1, + operation_key=operation_key, + repository=request.repository, + pr_number=request.pr_number, + head_sha=request.head_sha, + logical_iteration=request.logical_iteration, + requested_event=request.event, + effective_event=effective_event, + requested_body_digest=_poster_support.text_digest(request.body), + effective_body_digest=_poster_support.effective_body_digest( + request, operation_key, findings, effective_event + ), + canonical_finding_digest=_poster_support.finding_set_digest(all_findings), + state=state, + response_class=response_class, + review_id=reconciliation.review_id, + comment_ids=tuple( + item.remote_comment_id + for item in dispositions + if item.remote_comment_id is not None + ), + canonical_finding_count=len(all_findings), + reconciliation_result=reconciliation.result, + finding_dispositions=tuple(dispositions), + created_at=now, + updated_at=now, + final_attempt_digest=attempt_digest, + ) + ledger.save_receipt(receipt) + return GitHubReviewPostResult( + operation_key=operation_key, + head_sha=request.head_sha, + state=state, + response_class=response_class, + reconciliation_result=reconciliation.result, + review_id=reconciliation.review_id, + comment_ids=receipt.comment_ids, + planned_mutation_count=1, + planned_comment_count=len(all_findings), + executed_mutation_count=executed_mutations, + executed_comment_count=sum( + item.kind is ReviewFindingDispositionKind.POSTED for item in dispositions + ), + receipt=receipt, + ) + + +__all__ = ["reconcile_payload", "finalize"] \ No newline at end of file diff --git a/src/autoskillit/execution/github_review/poster.py b/src/autoskillit/execution/github_review/poster.py index 895a4489c..402d31fca 100644 --- a/src/autoskillit/execution/github_review/poster.py +++ b/src/autoskillit/execution/github_review/poster.py @@ -26,7 +26,7 @@ retry_after_seconds, ) -from . import _poster_boundary, _poster_retry, _poster_support +from . import _poster_boundary, _poster_finalize, _poster_retry, _poster_support from ._mutation_coordinator import GitHubReviewMutationCoordinator from .canonical import ( canonicalize_review_request, @@ -36,13 +36,6 @@ from .gateway import CredentialScopeMaterial, DefaultGitHubReviewGateway from .ledger import GitHubReviewLedger, ReviewAttemptRecord -_EXPECTED_REMOTE_STATES = { - "APPROVE": "APPROVED", - "REQUEST_CHANGES": "CHANGES_REQUESTED", - "COMMENT": "COMMENTED", -} -_FINAL_STATES = frozenset({ReviewOperationState.SUCCEEDED, ReviewOperationState.RECONCILED}) - class DefaultGitHubReviewPoster: def __init__( @@ -762,6 +755,7 @@ async def _reconcile_existing( executed_mutations=0, ) + async def _reconcile_payload( self, *, @@ -771,97 +765,13 @@ async def _reconcile_payload( findings: tuple[_poster_support.CanonicalFinding, ...], authenticated_login: str, ) -> _poster_support.Reconciliation: - reviews = await self.gateway.list_reviews(request.repository, request.pr_number) - if not reviews.succeeded or not isinstance(reviews.data, list): - return _poster_support.Reconciliation( - ReviewReconciliationResult.UNCERTAIN, - error=reviews.error or "could not read pull-request reviews", - ) - marker = _poster_support.OPERATION_MARKER.format(key=operation_key) - expected_body = str(payload["body"]) - expected_event = str(payload["event"]) - expected_state = _EXPECTED_REMOTE_STATES[expected_event] - matches = [ - item - for item in reviews.data - if isinstance(item, Mapping) - and item.get("body") == expected_body - and marker in str(item.get("body", "")) - and item.get("commit_id") == request.head_sha - and item.get("state") == expected_state - and _poster_support.nested_string(item, "user", "login") == authenticated_login - and _poster_support.positive_int(item.get("id")) is not None - ] - if not matches: - return _poster_support.Reconciliation(ReviewReconciliationResult.NOT_FOUND) - if len(matches) != 1: - return _poster_support.Reconciliation( - ReviewReconciliationResult.UNCERTAIN, - error="multiple reviews matched one operation marker", - ) - review_id = _poster_support.positive_int(matches[0].get("id")) - if review_id is None: - return _poster_support.Reconciliation( - ReviewReconciliationResult.UNCERTAIN, - error="matched review omitted a valid id", - ) - comments = await self.gateway.list_review_comments( - request.repository, - request.pr_number, - review_id, - ) - if not comments.succeeded or not isinstance(comments.data, list): - return _poster_support.Reconciliation( - ReviewReconciliationResult.UNCERTAIN, - error=comments.error or "could not read review comments", - ) - expected_comments = payload.get("comments") - if not isinstance(expected_comments, list) or len(expected_comments) != len(findings): - return _poster_support.Reconciliation( - ReviewReconciliationResult.UNCERTAIN, - error="persisted attempt comments do not match canonical findings", - ) - matched_ids: list[tuple[int, int]] = [] - used_remote_ids: set[int] = set() - for finding, expected in zip(findings, expected_comments, strict=True): - if not isinstance(expected, Mapping): - return _poster_support.Reconciliation( - ReviewReconciliationResult.UNCERTAIN, - error="persisted attempt comment is malformed", - ) - candidates = [ - item - for item in comments.data - if isinstance(item, Mapping) - and _poster_support.remote_comment_matches(item, expected, finding.digest) - ] - if len(candidates) != 1: - return _poster_support.Reconciliation( - ReviewReconciliationResult.UNCERTAIN, - error="review comments did not exactly match the attempted finding set", - ) - comment_id = _poster_support.positive_int(candidates[0].get("id")) - if comment_id is None or comment_id in used_remote_ids: - return _poster_support.Reconciliation( - ReviewReconciliationResult.UNCERTAIN, - error="review comment ids were missing or duplicated", - ) - used_remote_ids.add(comment_id) - matched_ids.append((finding.canonical_index, comment_id)) - marker_count = sum( - isinstance(item, Mapping) - and _poster_support.FINDING_MARKER.split("{digest}", 1)[0] in str(item.get("body", "")) - for item in comments.data - ) - if marker_count != len(findings): - return _poster_support.Reconciliation( - ReviewReconciliationResult.UNCERTAIN, - error="review contained an unexpected AutoSkillit finding marker set", - ) - return _poster_support.Reconciliation( - ReviewReconciliationResult.MATCHED, - review_id=review_id, - comment_ids=tuple(matched_ids), + return await _poster_finalize.reconcile_payload( + gateway=self.gateway, + request=request, + operation_key=operation_key, + payload=payload, + findings=findings, + authenticated_login=authenticated_login, ) async def _scan_remote_findings( @@ -928,70 +838,17 @@ def _finalize( reconciliation: _poster_support.Reconciliation, executed_mutations: int, ) -> GitHubReviewPostResult: - if state not in _FINAL_STATES or reconciliation.review_id is None: - raise ValueError("cannot finalize an unverified review operation") - ids_by_index = dict(reconciliation.comment_ids) - dispositions = list(omitted) - for finding in findings: - remote_comment_id = ids_by_index.get(finding.canonical_index) - if remote_comment_id is None: - raise ValueError("verified review omitted a posted comment id") - dispositions.append( - GitHubReviewFindingDisposition( - original_index=finding.original_index, - canonical_index=finding.canonical_index, - kind=ReviewFindingDispositionKind.POSTED, - remote_comment_id=remote_comment_id, - ) - ) - dispositions.sort(key=lambda item: item.original_index) - if len(dispositions) != len(request.comments): - raise ValueError("review finding accounting is not exhaustive") - now = self.wall_clock() - all_findings = _poster_support.canonical_findings(request) - receipt = GitHubReviewReceipt( - schema_version=1, + return _poster_finalize.finalize( + ledger=self.ledger, + wall_clock=self.wall_clock, + request=request, operation_key=operation_key, - repository=request.repository, - pr_number=request.pr_number, - head_sha=request.head_sha, - logical_iteration=request.logical_iteration, - requested_event=request.event, + findings=findings, + omitted=omitted, effective_event=effective_event, - requested_body_digest=_poster_support.text_digest(request.body), - effective_body_digest=_poster_support.effective_body_digest( - request, operation_key, findings, effective_event - ), - canonical_finding_digest=_poster_support.finding_set_digest(all_findings), - state=state, + attempt_digest=attempt_digest, response_class=response_class, - review_id=reconciliation.review_id, - comment_ids=tuple( - item.remote_comment_id - for item in dispositions - if item.remote_comment_id is not None - ), - canonical_finding_count=len(all_findings), - reconciliation_result=reconciliation.result, - finding_dispositions=tuple(dispositions), - created_at=now, - updated_at=now, - final_attempt_digest=attempt_digest, - ) - self.ledger.save_receipt(receipt) - return GitHubReviewPostResult( - operation_key=operation_key, - head_sha=request.head_sha, state=state, - response_class=response_class, - reconciliation_result=reconciliation.result, - review_id=reconciliation.review_id, - comment_ids=receipt.comment_ids, - planned_mutation_count=1, - planned_comment_count=len(all_findings), - executed_mutation_count=executed_mutations, - executed_comment_count=sum( - item.kind is ReviewFindingDispositionKind.POSTED for item in dispositions - ), - receipt=receipt, + reconciliation=reconciliation, + executed_mutations=executed_mutations, ) From 8cc7dd200260665f7de455cfd172aca2b37a5ef4 Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 17 Aug 2026 12:49:37 -0700 Subject: [PATCH 05/11] refactor(github_review): extract ledger schema to _ledger_schema.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move _SCHEMA_VERSION/_DIRECTORY_MODE/_DATABASE_MODE/_SCHEMA + dataclasses ReviewOperationRecord/ReviewAttemptRecord/MutationSlot from ledger.py (868 → 772 lines) into _ledger_schema.py. ledger.py imports them and re-exports for sibling modules; existing test imports keep working. Refs: #4664 --- .../execution/github_review/_ledger_schema.py | 128 ++++++++++++++++++ .../execution/github_review/ledger.py | 113 ++-------------- 2 files changed, 137 insertions(+), 104 deletions(-) create mode 100644 src/autoskillit/execution/github_review/_ledger_schema.py diff --git a/src/autoskillit/execution/github_review/_ledger_schema.py b/src/autoskillit/execution/github_review/_ledger_schema.py new file mode 100644 index 000000000..97fbc0a38 --- /dev/null +++ b/src/autoskillit/execution/github_review/_ledger_schema.py @@ -0,0 +1,128 @@ +"""Schema constants and dataclasses for the GitHub review ledger. + +Extracted from `ledger.py`. The schema (CREATE TABLE statements), the +file modes, and the three dataclasses (`ReviewOperationRecord`, +`ReviewAttemptRecord`, `MutationSlot`) live here. `GitHubReviewLedger` +imports them via re-export from this module. +""" + +from __future__ import annotations + +from dataclasses import dataclass + +from autoskillit.core import ( + GitHubReviewFindingDisposition, + ReviewOperationState, + ReviewResponseClass, +) + +_SCHEMA_VERSION = 1 +_DIRECTORY_MODE = 0o700 +_DATABASE_MODE = 0o600 +_SCHEMA = """ +CREATE TABLE metadata ( + key TEXT PRIMARY KEY, + value TEXT NOT NULL +) STRICT; +CREATE TABLE operations ( + operation_key TEXT PRIMARY KEY, + request_digest TEXT NOT NULL, + request_json BLOB NOT NULL, + state TEXT NOT NULL, + created_at REAL NOT NULL, + updated_at REAL NOT NULL +) STRICT; +CREATE TABLE operation_findings ( + operation_key TEXT NOT NULL, + canonical_index INTEGER NOT NULL, + original_index INTEGER NOT NULL, + finding_digest TEXT NOT NULL, + payload_json BLOB NOT NULL, + PRIMARY KEY (operation_key, canonical_index), + UNIQUE (operation_key, original_index), + UNIQUE (operation_key, finding_digest), + FOREIGN KEY (operation_key) REFERENCES operations(operation_key) +) STRICT; +CREATE TABLE attempts ( + operation_key TEXT NOT NULL, + attempt_number INTEGER NOT NULL, + attempt_digest TEXT NOT NULL, + payload_json BLOB NOT NULL, + canonical_indexes_json BLOB NOT NULL, + omitted_dispositions_json BLOB NOT NULL, + effective_event TEXT NOT NULL, + effective_body_digest TEXT NOT NULL, + state TEXT NOT NULL, + response_class TEXT NOT NULL, + status_code INTEGER, + error TEXT, + created_at REAL NOT NULL, + updated_at REAL NOT NULL, + PRIMARY KEY (operation_key, attempt_number), + UNIQUE (operation_key, attempt_digest), + FOREIGN KEY (operation_key) REFERENCES operations(operation_key) +) STRICT; +CREATE TABLE receipts ( + operation_key TEXT PRIMARY KEY, + receipt_json BLOB NOT NULL, + created_at REAL NOT NULL, + FOREIGN KEY (operation_key) REFERENCES operations(operation_key) +) STRICT; +CREATE TABLE rate_scopes ( + scope_id TEXT PRIMARY KEY, + lease_owner TEXT, + lease_generation INTEGER NOT NULL DEFAULT 0, + lease_expires_at REAL NOT NULL DEFAULT 0, + next_mutation_not_before REAL NOT NULL DEFAULT 0, + backoff_until REAL NOT NULL DEFAULT 0, + in_flight_operation_key TEXT +) STRICT; +""" + + +@dataclass(frozen=True, slots=True) +class ReviewOperationRecord: + operation_key: str + request_digest: str + request_json: bytes + state: ReviewOperationState + created_at: float + updated_at: float + + +@dataclass(frozen=True, slots=True) +class ReviewAttemptRecord: + operation_key: str + attempt_number: int + attempt_digest: str + payload_json: bytes + canonical_indexes: tuple[int, ...] + omitted_dispositions: tuple[GitHubReviewFindingDisposition, ...] + effective_event: str + effective_body_digest: str + state: str + response_class: ReviewResponseClass + status_code: int | None + error: str | None + created_at: float + updated_at: float + + +@dataclass(frozen=True, slots=True) +class MutationSlot: + ready: bool + delay: float + lease_owner: str + lease_generation: int + blocked_operation_key: str | None = None + + +__all__ = [ + "MutationSlot", + "ReviewAttemptRecord", + "ReviewOperationRecord", + "_DIRECTORY_MODE", + "_DATABASE_MODE", + "_SCHEMA", + "_SCHEMA_VERSION", +] \ No newline at end of file diff --git a/src/autoskillit/execution/github_review/ledger.py b/src/autoskillit/execution/github_review/ledger.py index ffeec5267..f35b4706b 100644 --- a/src/autoskillit/execution/github_review/ledger.py +++ b/src/autoskillit/execution/github_review/ledger.py @@ -10,15 +10,12 @@ import sqlite3 import stat import time -from dataclasses import dataclass from pathlib import Path from typing import Any from autoskillit.core import ( - GitHubReviewFindingDisposition, GitHubReviewReceipt, ReviewOperationState, - ReviewResponseClass, fsync_directory, private_file_identity, private_sidecar_issue, @@ -27,107 +24,15 @@ review_receipt_validation_error, unlink_sqlite_initialization_artifacts, ) - -_SCHEMA_VERSION = 1 -_DIRECTORY_MODE = 0o700 -_DATABASE_MODE = 0o600 -_SCHEMA = """ -CREATE TABLE metadata ( - key TEXT PRIMARY KEY, - value TEXT NOT NULL -) STRICT; -CREATE TABLE operations ( - operation_key TEXT PRIMARY KEY, - request_digest TEXT NOT NULL, - request_json BLOB NOT NULL, - state TEXT NOT NULL, - created_at REAL NOT NULL, - updated_at REAL NOT NULL -) STRICT; -CREATE TABLE operation_findings ( - operation_key TEXT NOT NULL, - canonical_index INTEGER NOT NULL, - original_index INTEGER NOT NULL, - finding_digest TEXT NOT NULL, - payload_json BLOB NOT NULL, - PRIMARY KEY (operation_key, canonical_index), - UNIQUE (operation_key, original_index), - UNIQUE (operation_key, finding_digest), - FOREIGN KEY (operation_key) REFERENCES operations(operation_key) -) STRICT; -CREATE TABLE attempts ( - operation_key TEXT NOT NULL, - attempt_number INTEGER NOT NULL, - attempt_digest TEXT NOT NULL, - payload_json BLOB NOT NULL, - canonical_indexes_json BLOB NOT NULL, - omitted_dispositions_json BLOB NOT NULL, - effective_event TEXT NOT NULL, - effective_body_digest TEXT NOT NULL, - state TEXT NOT NULL, - response_class TEXT NOT NULL, - status_code INTEGER, - error TEXT, - created_at REAL NOT NULL, - updated_at REAL NOT NULL, - PRIMARY KEY (operation_key, attempt_number), - UNIQUE (operation_key, attempt_digest), - FOREIGN KEY (operation_key) REFERENCES operations(operation_key) -) STRICT; -CREATE TABLE receipts ( - operation_key TEXT PRIMARY KEY, - receipt_json BLOB NOT NULL, - created_at REAL NOT NULL, - FOREIGN KEY (operation_key) REFERENCES operations(operation_key) -) STRICT; -CREATE TABLE rate_scopes ( - scope_id TEXT PRIMARY KEY, - lease_owner TEXT, - lease_generation INTEGER NOT NULL DEFAULT 0, - lease_expires_at REAL NOT NULL DEFAULT 0, - next_mutation_not_before REAL NOT NULL DEFAULT 0, - backoff_until REAL NOT NULL DEFAULT 0, - in_flight_operation_key TEXT -) STRICT; -""" - - -@dataclass(frozen=True, slots=True) -class ReviewOperationRecord: - operation_key: str - request_digest: str - request_json: bytes - state: ReviewOperationState - created_at: float - updated_at: float - - -@dataclass(frozen=True, slots=True) -class ReviewAttemptRecord: - operation_key: str - attempt_number: int - attempt_digest: str - payload_json: bytes - canonical_indexes: tuple[int, ...] - omitted_dispositions: tuple[GitHubReviewFindingDisposition, ...] - effective_event: str - effective_body_digest: str - state: str - response_class: ReviewResponseClass - status_code: int | None - error: str | None - created_at: float - updated_at: float - - -@dataclass(frozen=True, slots=True) -class MutationSlot: - ready: bool - delay: float - lease_owner: str - lease_generation: int - blocked_operation_key: str | None = None - +from ._ledger_schema import ( # noqa: F401 — re-exported for sibling modules + MutationSlot, + ReviewAttemptRecord, + ReviewOperationRecord, + _DIRECTORY_MODE, + _DATABASE_MODE, + _SCHEMA, + _SCHEMA_VERSION, +) class GitHubReviewLedger: """Durable operation, attempt, receipt, and cross-process pacing authority.""" From 80966797b657f44c2dabb5dcf415b03338f771f3 Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 17 Aug 2026 12:50:13 -0700 Subject: [PATCH 06/11] refactor(headless): extract adjudication helpers to _headless_adjudication.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move _resolve_skill_session_id/_parse_stdout/_build_api_retry_outcome/ _make_terminated_result/_has_out_of_cwd_file_change/_apply_post_session_adjudication/ _validate_declared_artifact from _headless_result.py (1030 → 846 lines) to _headless_adjudication.py. _build_skill_result remains as the headless orchestration authority. Re-export the moved symbols so existing imports keep working. Narrow E25 line-cap to acknowledge the residual 846-line _headless_result.py. Refs: #4664 --- .../headless/_headless_adjudication.py | 247 ++++++++++++++++++ .../execution/headless/_headless_result.py | 206 +-------------- 2 files changed, 258 insertions(+), 195 deletions(-) create mode 100644 src/autoskillit/execution/headless/_headless_adjudication.py diff --git a/src/autoskillit/execution/headless/_headless_adjudication.py b/src/autoskillit/execution/headless/_headless_adjudication.py new file mode 100644 index 000000000..01729eb57 --- /dev/null +++ b/src/autoskillit/execution/headless/_headless_adjudication.py @@ -0,0 +1,247 @@ +# Adjudication helpers extracted from _headless_result.py +"""Adjudication helpers for headless Claude session SkillResult construction. + +Extracted from `_headless_result.py`. This module owns the post-session +adjudication chain: parse-stdout, build-api-retry-outcome, make-terminated- +result, out-of-cwd-file-change detection, post-session-adjudication, and +declared-artifact validation. The SkillResult constructor itself +(`_build_skill_result`) remains in the parent module because it is +the headless orchestration authority. +""" + +from __future__ import annotations + +import dataclasses +import errno +import stat +from collections.abc import Sequence +from pathlib import Path +from typing import TYPE_CHECKING, cast + +from autoskillit.core import ( + AGENT_BACKEND_CLAUDE_CODE, + ApiRetryOutcome, + InfraOutcome, + NdjsonDriftOutcome, + ProviderOutcome, + RetryReason, + SkillResult, + WriteBehaviorSpec, + WriteEvidence, + get_logger, +) +from autoskillit.execution.headless._headless_evidence import _adapt_agent_result +from autoskillit.execution.headless._headless_outcome import ( + evaluate_outcome_invariants, + parse_outcome_fields, +) +from autoskillit.execution.headless._headless_path_tokens import _is_path_outside_cwd +from autoskillit.execution.session._session_content import _check_expected_patterns +from autoskillit.execution.session._session_model import ( + ClaudeSessionResult, + parse_session_result, +) + +if TYPE_CHECKING: + from autoskillit.core import CodingAgentBackend, SubprocessResult + from autoskillit.recipe._contracts_types import SkillContract + +logger = get_logger(__name__) + + +def _resolve_skill_session_id( + session: ClaudeSessionResult | None, + result: SubprocessResult, +) -> str: + """Return the best-available Claude session UUID.""" + if session is not None and session.session_id: + return session.session_id + return result.session_id or result.channel_b_session_id + + +def _parse_stdout(stdout: str, backend: CodingAgentBackend) -> ClaudeSessionResult: + if backend.name == AGENT_BACKEND_CLAUDE_CODE: + return parse_session_result(stdout) + agent_result = backend.result_parser().parse_stdout(stdout) + return _adapt_agent_result(agent_result) + + +def _build_api_retry_outcome(session: ClaudeSessionResult) -> ApiRetryOutcome: + return ApiRetryOutcome( + count=session.api_retry_count, + last_error=session.api_retry_last_error, + last_status=session.api_retry_last_status, + exhausted=session.api_retry_exhausted, + ) + + +def _make_terminated_result( + *, + result: SubprocessResult, + session: ClaudeSessionResult, + success: bool, + result_text: str, + subtype: str, + needs_retry: bool, + retry_reason: RetryReason, + evidence: WriteEvidence, + provider_used: str = "", + infra: InfraOutcome = InfraOutcome(), + api_retry: ApiRetryOutcome = ApiRetryOutcome(), +) -> SkillResult: + """Construct SkillResult for infrastructure-terminated sessions (stale/idle_stall).""" + return SkillResult( + success=success, + result=result_text, + session_id=session.session_id or _resolve_skill_session_id(session, result), + subtype=subtype, + is_error=session.is_error if success else False, + exit_code=result.returncode if result.returncode is not None else -1, + needs_retry=needs_retry, + retry_reason=retry_reason, + stderr=result.stderr if result.stderr else "", + token_usage=session.token_usage, + evidence=evidence, + kill_reason=result.kill_reason, + last_stop_reason=session.last_stop_reason, + lifespan_started=session.lifespan_started, + provider=ProviderOutcome(provider_used=provider_used, fallback_activated=False), + infra=infra, + api_retry=api_retry, + ndjson_drift=NdjsonDriftOutcome( + unknown_event_count=session.seen_ndjson_unknown_event_count, + unknown_item_count=session.seen_ndjson_unknown_item_count, + ), + ) + + +def _has_out_of_cwd_file_change(file_changes: Sequence[str], cwd: str) -> bool: + """Return True iff any raw Codex FILE_CHANGE path lexically resolves outside cwd. + + Empty/invalid entries are ignored. If cwd is missing, relative, or ``/``, + no boundary proof is produced — matching the validator's safety contract. + """ + for path in file_changes: + if not isinstance(path, str) or not path: + continue + if _is_path_outside_cwd(path, cwd, allow_relative=True): + return True + return False + + +def _apply_post_session_adjudication( + sr: SkillResult, + evidence: WriteEvidence, + write_behavior: WriteBehaviorSpec | None, + skill_contract: SkillContract | None, + cwd: str, +) -> SkillResult: + """Apply write, invariant, and declared-artifact contract checks. + + Invoked as the last adjudication step before each success-finalizing + return. Makes "a success path that skips adjudication" unrepresentable. + """ + fields = parse_outcome_fields(sr.result, skill_contract) if skill_contract else {} + if fields: + sr = dataclasses.replace(sr, outcome_fields=fields) + + if not sr.success: + return sr + + if not evidence.has_implementation_evidence and write_behavior is not None: + write_expected = False + if write_behavior.mode == "always": + write_expected = True + elif write_behavior.mode == "conditional" and write_behavior.expected_when: + write_expected = _check_expected_patterns( + sr.result, + write_behavior.expected_when, + ) + if write_expected: + return dataclasses.replace( + sr, + success=False, + subtype="zero_writes", + needs_retry=True, + retry_reason=RetryReason.ZERO_WRITES, + ) + + if skill_contract is not None and skill_contract.outcome_invariants: + violated, detail = evaluate_outcome_invariants(fields, skill_contract.outcome_invariants) + if violated: + logger.warning("outcome_invariant_violated", detail=detail) + return dataclasses.replace( + sr, + success=False, + subtype="outcome_invariant_violation", + needs_retry=True, + retry_reason=RetryReason.OUTCOME_INVARIANT, + outcome_fields=None, + ) + + if skill_contract is not None: + for output in skill_contract.outputs: + value = fields.get(output.name) + if output.type != "file_path" or value is None: + continue + failure = _validate_declared_artifact(cwd, output.name, cast(str, value)) + if failure is not None: + subtype, detail = failure + retry_reason = ( + RetryReason.CONTRACT_RECOVERY + if subtype == "artifact_contract_violation" + else RetryReason.RESUME + ) + return dataclasses.replace( + sr, + success=False, + is_error=True, + subtype=subtype, + needs_retry=True, + retry_reason=retry_reason, + result=detail, + outcome_fields=None, + ) + + return sr + + +def _validate_declared_artifact(cwd: str, field_name: str, value: str) -> tuple[str, str] | None: + """Validate one emitted ``file_path`` without exposing unsafe paths.""" + safe_name = "." + producer_detail = ( + f"Skill output '{field_name}' did not identify a contained regular file: {safe_name}" + ) + infrastructure_detail = ( + f"Could not validate skill output '{field_name}' because filesystem access failed." + ) + try: + root = Path(cwd).resolve() + candidate = Path(value) + safe_name = candidate.name or "." + producer_detail = ( + f"Skill output '{field_name}' did not identify a contained regular file: {safe_name}" + ) + target = ( + (root / candidate).resolve() if not candidate.is_absolute() else candidate.resolve() + ) + try: + target.relative_to(root) + except ValueError: + return "artifact_contract_violation", producer_detail + target_stat = target.stat() + if not stat.S_ISREG(target_stat.st_mode): + return "artifact_contract_violation", producer_detail + except (TypeError, ValueError, RuntimeError): + return "artifact_contract_violation", producer_detail + except OSError as exc: + if exc.errno in {errno.ENOENT, errno.ENOTDIR, errno.ELOOP}: + return "artifact_contract_violation", producer_detail + logger.warning( + "artifact_adjudication_error", + field_name=field_name, + artifact_name=safe_name, + exc_info=True, + ) + return "artifact_adjudication_error", infrastructure_detail + return None diff --git a/src/autoskillit/execution/headless/_headless_result.py b/src/autoskillit/execution/headless/_headless_result.py index 518bb5919..ad7fcf4c6 100644 --- a/src/autoskillit/execution/headless/_headless_result.py +++ b/src/autoskillit/execution/headless/_headless_result.py @@ -93,202 +93,18 @@ ] -def _resolve_skill_session_id( - session: ClaudeSessionResult | None, - result: SubprocessResult, -) -> str: - """Return the best-available Claude session UUID.""" - if session is not None and session.session_id: - return session.session_id - return result.session_id or result.channel_b_session_id - - -def _parse_stdout(stdout: str, backend: CodingAgentBackend) -> ClaudeSessionResult: - if backend.name == AGENT_BACKEND_CLAUDE_CODE: - return parse_session_result(stdout) - agent_result = backend.result_parser().parse_stdout(stdout) - return _adapt_agent_result(agent_result) - - -def _build_api_retry_outcome(session: ClaudeSessionResult) -> ApiRetryOutcome: - return ApiRetryOutcome( - count=session.api_retry_count, - last_error=session.api_retry_last_error, - last_status=session.api_retry_last_status, - exhausted=session.api_retry_exhausted, - ) - - -def _make_terminated_result( - *, - result: SubprocessResult, - session: ClaudeSessionResult, - success: bool, - result_text: str, - subtype: str, - needs_retry: bool, - retry_reason: RetryReason, - evidence: WriteEvidence, - provider_used: str = "", - infra: InfraOutcome = InfraOutcome(), - api_retry: ApiRetryOutcome = ApiRetryOutcome(), -) -> SkillResult: - """Construct SkillResult for infrastructure-terminated sessions (stale/idle_stall).""" - return SkillResult( - success=success, - result=result_text, - session_id=session.session_id or _resolve_skill_session_id(session, result), - subtype=subtype, - is_error=session.is_error if success else False, - exit_code=result.returncode if result.returncode is not None else -1, - needs_retry=needs_retry, - retry_reason=retry_reason, - stderr=result.stderr if result.stderr else "", - token_usage=session.token_usage, - evidence=evidence, - kill_reason=result.kill_reason, - last_stop_reason=session.last_stop_reason, - lifespan_started=session.lifespan_started, - provider=ProviderOutcome(provider_used=provider_used, fallback_activated=False), - infra=infra, - api_retry=api_retry, - ndjson_drift=NdjsonDriftOutcome( - unknown_event_count=session.seen_ndjson_unknown_event_count, - unknown_item_count=session.seen_ndjson_unknown_item_count, - ), - ) - - -def _has_out_of_cwd_file_change(file_changes: Sequence[str], cwd: str) -> bool: - """Return True iff any raw Codex FILE_CHANGE path lexically resolves outside cwd. - - Empty/invalid entries are ignored. If cwd is missing, relative, or ``/``, - no boundary proof is produced — matching the validator's safety contract. - """ - for path in file_changes: - if not isinstance(path, str) or not path: - continue - if _is_path_outside_cwd(path, cwd, allow_relative=True): - return True - return False - - -def _apply_post_session_adjudication( - sr: SkillResult, - evidence: WriteEvidence, - write_behavior: WriteBehaviorSpec | None, - skill_contract: SkillContract | None, - cwd: str, -) -> SkillResult: - """Apply write, invariant, and declared-artifact contract checks. - - Invoked as the last adjudication step before each success-finalizing - return. Makes "a success path that skips adjudication" unrepresentable. - """ - fields = parse_outcome_fields(sr.result, skill_contract) if skill_contract else {} - if fields: - sr = dataclasses.replace(sr, outcome_fields=fields) - - if not sr.success: - return sr - - if not evidence.has_implementation_evidence and write_behavior is not None: - write_expected = False - if write_behavior.mode == "always": - write_expected = True - elif write_behavior.mode == "conditional" and write_behavior.expected_when: - write_expected = _check_expected_patterns( - sr.result, - write_behavior.expected_when, - ) - if write_expected: - return dataclasses.replace( - sr, - success=False, - subtype="zero_writes", - needs_retry=True, - retry_reason=RetryReason.ZERO_WRITES, - ) - - if skill_contract is not None and skill_contract.outcome_invariants: - violated, detail = evaluate_outcome_invariants(fields, skill_contract.outcome_invariants) - if violated: - logger.warning("outcome_invariant_violated", detail=detail) - return dataclasses.replace( - sr, - success=False, - subtype="outcome_invariant_violation", - needs_retry=True, - retry_reason=RetryReason.OUTCOME_INVARIANT, - outcome_fields=None, - ) - - if skill_contract is not None: - for output in skill_contract.outputs: - value = fields.get(output.name) - if output.type != "file_path" or value is None: - continue - failure = _validate_declared_artifact(cwd, output.name, cast(str, value)) - if failure is not None: - subtype, detail = failure - retry_reason = ( - RetryReason.CONTRACT_RECOVERY - if subtype == "artifact_contract_violation" - else RetryReason.RESUME - ) - return dataclasses.replace( - sr, - success=False, - is_error=True, - subtype=subtype, - needs_retry=True, - retry_reason=retry_reason, - result=detail, - outcome_fields=None, - ) - - return sr - +# Adjudication helpers live in _headless_adjudication.py; re-exported for +# existing callers using the canonical _headless_result path. +from autoskillit.execution.headless._headless_adjudication import ( # noqa: F401 + _apply_post_session_adjudication, + _build_api_retry_outcome, + _has_out_of_cwd_file_change, + _make_terminated_result, + _parse_stdout, + _resolve_skill_session_id, + _validate_declared_artifact, +) -def _validate_declared_artifact(cwd: str, field_name: str, value: str) -> tuple[str, str] | None: - """Validate one emitted ``file_path`` without exposing unsafe paths.""" - safe_name = "." - producer_detail = ( - f"Skill output '{field_name}' did not identify a contained regular file: {safe_name}" - ) - infrastructure_detail = ( - f"Could not validate skill output '{field_name}' because filesystem access failed." - ) - try: - root = Path(cwd).resolve() - candidate = Path(value) - safe_name = candidate.name or "." - producer_detail = ( - f"Skill output '{field_name}' did not identify a contained regular file: {safe_name}" - ) - target = ( - (root / candidate).resolve() if not candidate.is_absolute() else candidate.resolve() - ) - try: - target.relative_to(root) - except ValueError: - return "artifact_contract_violation", producer_detail - target_stat = target.stat() - if not stat.S_ISREG(target_stat.st_mode): - return "artifact_contract_violation", producer_detail - except (TypeError, ValueError, RuntimeError): - return "artifact_contract_violation", producer_detail - except OSError as exc: - if exc.errno in {errno.ENOENT, errno.ENOTDIR, errno.ELOOP}: - return "artifact_contract_violation", producer_detail - logger.warning( - "artifact_adjudication_error", - field_name=field_name, - artifact_name=safe_name, - exc_info=True, - ) - return "artifact_adjudication_error", infrastructure_detail - return None def _build_skill_result( From 43f73ae319bd88580e2aa8ba3627b65532b4b695 Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 17 Aug 2026 12:50:26 -0700 Subject: [PATCH 07/11] test(arch): narrow E25 line-cap for _headless_result.py to 900 --- tests/arch/test_subpackage_isolation.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/arch/test_subpackage_isolation.py b/tests/arch/test_subpackage_isolation.py index 3e3c79549..3c5bb52f8 100644 --- a/tests/arch/test_subpackage_isolation.py +++ b/tests/arch/test_subpackage_isolation.py @@ -1331,9 +1331,12 @@ def test_data_directories_are_not_python_packages() -> None: "adds execution-role protocol parity while preserving Claude behavior (+3 net lines).", ), "execution/headless/_headless_result.py": ( - 1033, - "REQ-CNST-010-E25: #4233 keeps the async-obligation success gate adjacent to " - "the existing stale, idle, timeout, and content adjudication order it must preempt", + 900, + "REQ-CNST-010-E25-narrowed: #4233 keeps the async-obligation success gate adjacent to " + "the existing stale, idle, timeout, and content adjudication order it must preempt. " + "After #4664 decomposition, adjudication helpers live in _headless_adjudication.py; " + "_build_skill_result remains as the headless orchestration authority. The 846-line " + "residual is the single function that owns the success-gate adjacency rule.", ), "workspace/skill_capabilities.py": ( 1120, From 465321eba0d9290bf8fd282392abd08d53a9fb8a Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 17 Aug 2026 12:51:20 -0700 Subject: [PATCH 08/11] refactor(process): extract termination helpers to _termination.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move decide_termination_action (pure decision function) and execute_termination_action (sole authorized async kill executor) from process/__init__.py (942 → 821 lines) to process/_termination.py. The public process.__init__ facade re-exports both so existing callers keep working; _EXPECTED_PROCESS_SYMBOLS test continues to pass. Refs: #4664 --- src/autoskillit/execution/process/__init__.py | 133 +------------- .../execution/process/_termination.py | 170 ++++++++++++++++++ 2 files changed, 176 insertions(+), 127 deletions(-) create mode 100644 src/autoskillit/execution/process/_termination.py diff --git a/src/autoskillit/execution/process/__init__.py b/src/autoskillit/execution/process/__init__.py index 90c274e98..cd31369aa 100644 --- a/src/autoskillit/execution/process/__init__.py +++ b/src/autoskillit/execution/process/__init__.py @@ -166,134 +166,13 @@ def _normalize_pass_fds(pass_fds: tuple[int, ...]) -> tuple[int, ...]: return normalize_inherited_fds(pass_fds) -def decide_termination_action( - termination: TerminationReason, - *, - timeout_fired: bool, - process_exited: bool, - pending_task_ids: tuple[str, ...] = (), - schedule_wakeup_violation: bool = False, - completion_ceiling_expired: bool = False, -) -> TerminationAction: - """Pure decision function: maps race signals to a TerminationAction. - - Priority: - 1. timeout_fired → IMMEDIATE_KILL (always overrides) - 2. process_exited → NO_KILL (process already gone, no signal needed) - 3. termination-reason dispatch: - - COMPLETED: channel won but process alive → DRAIN_THEN_KILL_IF_ALIVE - - NATURAL_EXIT: fallback case → NO_KILL - - IDLE_STALL / STALE / TIMED_OUT: infra kill → IMMEDIATE_KILL - - The function is deliberately free of anyio and I/O so it can be tested - as a pure decision table without any async or process infrastructure. - """ - if timeout_fired: - return TerminationAction.IMMEDIATE_KILL - if process_exited and ( - pending_task_ids or schedule_wakeup_violation or completion_ceiling_expired - ): - return TerminationAction.IMMEDIATE_KILL - if process_exited: - return TerminationAction.NO_KILL - match termination: - case TerminationReason.NATURAL_EXIT | TerminationReason.SIGNAL_DEATH: - return TerminationAction.NO_KILL - case TerminationReason.COMPLETED: - return TerminationAction.DRAIN_THEN_KILL_IF_ALIVE - case ( - TerminationReason.IDLE_STALL - | TerminationReason.STALE - | TerminationReason.TIMED_OUT - | TerminationReason.HEALTH_INSPECTOR - ): - return TerminationAction.IMMEDIATE_KILL - case _ as unreachable: - assert_never(unreachable) - - -async def execute_termination_action( - action: TerminationAction, - *, - owner: OwnedProcessGroup, - process_exited_event: anyio.Event, - grace_seconds: float, - proc_log: structlog.BoundLogger, - pid: int | None = None, - marker_dir: Path | None = None, - session_id: str | None = None, - child_deferral_ceiling: float = 0.0, - process_observation_snapshot: ProcessObservationSnapshot | None = None, -) -> tuple[KillReason, int, ProcessCleanupResult]: - """Single authorized executor for all kill decisions in run_managed_async. - - This is the sole managed-async authority for drain, signal, settlement, and reap. - - On the DRAIN_THEN_KILL_IF_ALIVE path, when *pid* is provided and - *child_deferral_ceiling* > 0, the kill is deferred (bounded by the ceiling) - while child processes, an API connection, or an execution marker indicate - the subagent is still doing active work — mirroring the stale-kill - suppression pattern in _session_log_monitor. +# Termination decision/execution helpers live in _termination.py; re-exported +# so existing callers using the canonical process.__init__ path keep working. +from autoskillit.execution.process._termination import ( # noqa: F401 + decide_termination_action, + execute_termination_action, +) - Returns the kill reason, authoritative final return code, and cleanup evidence. - """ - if process_observation_snapshot is not None: - owner.merge_snapshot(process_observation_snapshot) - match action: - case TerminationAction.NO_KILL: - kill_reason = KillReason.NATURAL_EXIT - case TerminationAction.DRAIN_THEN_KILL_IF_ALIVE: - with anyio.move_on_after(grace_seconds): - await process_exited_event.wait() - if owner.returncode is not None: - proc_log.debug("natural_exit_after_drain", returncode=owner.returncode) - kill_reason = KillReason.NATURAL_EXIT - returncode, cleanup = await anyio.to_thread.run_sync( - owner.settle, abandon_on_cancel=False - ) - return kill_reason, returncode, cleanup - # Child-liveness deferral: same pattern as _session_log_monitor stale-kill suppression - if pid is not None and child_deferral_ceiling > 0: - deferral_start = anyio.current_time() - _poll_interval = 2.0 - while (anyio.current_time() - deferral_start) < child_deferral_ceiling: - if owner.returncode is not None: - proc_log.debug("natural_exit_during_deferral", returncode=owner.returncode) - kill_reason = KillReason.NATURAL_EXIT - returncode, cleanup = await anyio.to_thread.run_sync( - owner.settle, abandon_on_cancel=False - ) - return kill_reason, returncode, cleanup - active = ( - _has_active_child_processes(pid) - or _has_active_api_connection(pid) - or ( - marker_dir is not None - and _has_active_execution_marker(marker_dir, session_id=session_id) - ) - ) - if not active: - proc_log.debug("no_active_children_proceeding_to_kill") - break - proc_log.debug( - "child_liveness_deferral", - elapsed=anyio.current_time() - deferral_start, - ceiling=child_deferral_ceiling, - ) - await anyio.sleep(_poll_interval) - proc_log.debug("grace_expired_killing", grace_seconds=grace_seconds) - kill_reason = KillReason.KILL_AFTER_COMPLETION - case TerminationAction.IMMEDIATE_KILL: - if pid is not None and _has_active_child_processes(pid): - proc_log.warning( - "immediate_kill_with_active_children", - pid=pid, - ) - kill_reason = KillReason.INFRA_KILL - case _ as unreachable: - assert_never(unreachable) - returncode, cleanup = await anyio.to_thread.run_sync(owner.settle, abandon_on_cancel=False) - return kill_reason, returncode, cleanup async def run_managed_async( diff --git a/src/autoskillit/execution/process/_termination.py b/src/autoskillit/execution/process/_termination.py new file mode 100644 index 000000000..32926883b --- /dev/null +++ b/src/autoskillit/execution/process/_termination.py @@ -0,0 +1,170 @@ +"""Termination decision and execution helpers for managed subprocesses. + +Extracted from `process/__init__.py`. This module owns: + +- `decide_termination_action` — the pure decision function that maps race + signals to a `TerminationAction` (deliberately free of anyio and I/O + so it can be tested as a pure decision table). +- `execute_termination_action` — the sole authorized caller of + `async_kill_process_tree` for `run_managed_async`. Test-enforced. + +`_EXPECTED_PROCESS_SYMBOLS` keeps the public facade re-exports in +`process/__init__.py` so existing callers keep working. +""" + +from __future__ import annotations + +from pathlib import Path + +import anyio +import structlog + +from autoskillit.core import ( + KillReason, + ProcessCleanupResult, + TerminationAction, + TerminationReason, +) +from autoskillit.execution.process._process_kill import ( + OwnedProcessGroup, + ProcessObservationSnapshot, +) +from autoskillit.execution.process._process_monitor import ( + _has_active_api_connection, + _has_active_child_processes, + _has_active_execution_marker, +) +from typing_extensions import assert_never + + +def decide_termination_action( + termination: TerminationReason, + *, + timeout_fired: bool, + process_exited: bool, + pending_task_ids: tuple[str, ...] = (), + schedule_wakeup_violation: bool = False, + completion_ceiling_expired: bool = False, +) -> TerminationAction: + """Pure decision function: maps race signals to a TerminationAction. + + Priority: + 1. timeout_fired → IMMEDIATE_KILL (always overrides) + 2. process_exited → NO_KILL (process already gone, no signal needed) + 3. termination-reason dispatch: + - COMPLETED: channel won but process alive → DRAIN_THEN_KILL_IF_ALIVE + - NATURAL_EXIT: fallback case → NO_KILL + - IDLE_STALL / STALE / TIMED_OUT: infra kill → IMMEDIATE_KILL + + The function is deliberately free of anyio and I/O so it can be tested + as a pure decision table without any async or process infrastructure. + """ + if timeout_fired: + return TerminationAction.IMMEDIATE_KILL + if process_exited and ( + pending_task_ids or schedule_wakeup_violation or completion_ceiling_expired + ): + return TerminationAction.IMMEDIATE_KILL + if process_exited: + return TerminationAction.NO_KILL + match termination: + case TerminationReason.NATURAL_EXIT | TerminationReason.SIGNAL_DEATH: + return TerminationAction.NO_KILL + case TerminationReason.COMPLETED: + return TerminationAction.DRAIN_THEN_KILL_IF_ALIVE + case ( + TerminationReason.IDLE_STALL + | TerminationReason.STALE + | TerminationReason.TIMED_OUT + | TerminationReason.HEALTH_INSPECTOR + ): + return TerminationAction.IMMEDIATE_KILL + case _ as unreachable: + assert_never(unreachable) + + +async def execute_termination_action( + action: TerminationAction, + *, + owner: OwnedProcessGroup, + process_exited_event: anyio.Event, + grace_seconds: float, + proc_log: structlog.BoundLogger, + pid: int | None = None, + marker_dir: Path | None = None, + session_id: str | None = None, + child_deferral_ceiling: float = 0.0, + process_observation_snapshot: ProcessObservationSnapshot | None = None, +) -> tuple[KillReason, int, ProcessCleanupResult]: + """Single authorized executor for all kill decisions in run_managed_async. + + This is the sole managed-async authority for drain, signal, settlement, and reap. + + On the DRAIN_THEN_KILL_IF_ALIVE path, when *pid* is provided and + *child_deferral_ceiling* > 0, the kill is deferred (bounded by the ceiling) + while child processes, an API connection, or an execution marker indicate + the subagent is still doing active work — mirroring the stale-kill + suppression pattern in _session_log_monitor. + + Returns the kill reason, authoritative final return code, and cleanup evidence. + """ + if process_observation_snapshot is not None: + owner.merge_snapshot(process_observation_snapshot) + match action: + case TerminationAction.NO_KILL: + kill_reason = KillReason.NATURAL_EXIT + case TerminationAction.DRAIN_THEN_KILL_IF_ALIVE: + with anyio.move_on_after(grace_seconds): + await process_exited_event.wait() + if owner.returncode is not None: + proc_log.debug("natural_exit_after_drain", returncode=owner.returncode) + kill_reason = KillReason.NATURAL_EXIT + returncode, cleanup = await anyio.to_thread.run_sync( + owner.settle, abandon_on_cancel=False + ) + return kill_reason, returncode, cleanup + # Child-liveness deferral: same pattern as _session_log_monitor stale-kill suppression + if pid is not None and child_deferral_ceiling > 0: + deferral_start = anyio.current_time() + _poll_interval = 2.0 + while (anyio.current_time() - deferral_start) < child_deferral_ceiling: + if owner.returncode is not None: + proc_log.debug("natural_exit_during_deferral", returncode=owner.returncode) + kill_reason = KillReason.NATURAL_EXIT + returncode, cleanup = await anyio.to_thread.run_sync( + owner.settle, abandon_on_cancel=False + ) + return kill_reason, returncode, cleanup + active = ( + _has_active_child_processes(pid) + or _has_active_api_connection(pid) + or ( + marker_dir is not None + and _has_active_execution_marker(marker_dir, session_id=session_id) + ) + ) + if not active: + proc_log.debug("no_active_children_proceeding_to_kill") + break + proc_log.debug( + "child_liveness_deferral", + elapsed=anyio.current_time() - deferral_start, + ceiling=child_deferral_ceiling, + ) + await anyio.sleep(_poll_interval) + proc_log.debug("grace_expired_killing", grace_seconds=grace_seconds) + kill_reason = KillReason.KILL_AFTER_COMPLETION + case TerminationAction.IMMEDIATE_KILL: + if pid is not None and _has_active_child_processes(pid): + proc_log.warning( + "immediate_kill_with_active_children", + pid=pid, + ) + kill_reason = KillReason.INFRA_KILL + case _ as unreachable: + assert_never(unreachable) + returncode, cleanup = await anyio.to_thread.run_sync(owner.settle, abandon_on_cancel=False) + return kill_reason, returncode, cleanup + + +__all__ = ["decide_termination_action", "execute_termination_action"] \ No newline at end of file From 3722d19c0d0e4f1bf22f101e7ef8360f4db115e4 Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 17 Aug 2026 12:53:12 -0700 Subject: [PATCH 09/11] refactor(session): decompose _managed_headless_session_lineage.py Move per-record creation/projection/anchor helpers to _records.py, runner observation helpers to _runner.py, and index path helpers to _indexes.py. _managed_headless_session_lineage.py retains the conflict/CAS mismatch errors and the DefaultManagedHeadlessSessionLineageStore class plus the unaccounted _validate_anchor_identity/_require_cas/ _read_bounded helpers. Existing canonical-path imports keep working via re-export facade. Refs: #4664 --- .../_managed_headless_session_lineage.py | 348 ++---------------- ...anaged_headless_session_lineage_indexes.py | 76 ++++ ...anaged_headless_session_lineage_records.py | 192 ++++++++++ ...managed_headless_session_lineage_runner.py | 150 ++++++++ 4 files changed, 442 insertions(+), 324 deletions(-) create mode 100644 src/autoskillit/execution/session/_managed_headless_session_lineage_indexes.py create mode 100644 src/autoskillit/execution/session/_managed_headless_session_lineage_records.py create mode 100644 src/autoskillit/execution/session/_managed_headless_session_lineage_runner.py diff --git a/src/autoskillit/execution/session/_managed_headless_session_lineage.py b/src/autoskillit/execution/session/_managed_headless_session_lineage.py index b9eb0b699..42b13ec7c 100644 --- a/src/autoskillit/execution/session/_managed_headless_session_lineage.py +++ b/src/autoskillit/execution/session/_managed_headless_session_lineage.py @@ -549,330 +549,30 @@ def _mutate( return updated -def _new_lineage( - *, - launch_id: str, - decision: NativeShellCaptureDecision, - backend: str, - session_kind: ManagedHeadlessSessionKind, - lineage_anchor: Path, - anchor_device: int, - anchor_inode: int, - dispatch_id: str | None, -) -> ManagedHeadlessSessionLineage: - identity = { - "schema_version": MANAGED_HEADLESS_SESSION_LINEAGE_SCHEMA_VERSION, - "launch_id": launch_id, - "decision": decision.to_dict(), - "backend": backend, - "session_kind": session_kind.value, - "lineage_anchor": str(lineage_anchor), - "anchor_device": anchor_device, - "anchor_inode": anchor_inode, - } - lineage_digest = _digest(identity) - provisional = ManagedHeadlessSessionLineage( - launch_id=launch_id, - decision=decision, - backend=backend, - session_kind=session_kind, - lineage_anchor=str(lineage_anchor), - anchor_device=anchor_device, - anchor_inode=anchor_inode, - lineage_digest=lineage_digest, - generation=0, - record_digest="0" * 64, - dispatch_id=dispatch_id, - ) - return replace(provisional, record_digest=_digest(_record_payload(provisional))) - - -def _next_generation( - lineage: ManagedHeadlessSessionLineage, -) -> ManagedHeadlessSessionLineage: - provisional = replace( - lineage, - generation=lineage.generation + 1, - record_digest="0" * 64, - ) - return replace(provisional, record_digest=_digest(_record_payload(provisional))) - - -def _creation_projection(lineage: ManagedHeadlessSessionLineage) -> tuple[object, ...]: - return ( - lineage.launch_id, - lineage.decision, - lineage.backend, - lineage.session_kind, - lineage.lineage_anchor, - lineage.anchor_device, - lineage.anchor_inode, - lineage.dispatch_id, - ) - - -def _resolve_anchor(lineage_anchor: Path) -> tuple[Path, int, int]: - supplied = Path(lineage_anchor).expanduser() - if not supplied.is_absolute(): - raise ValueError("Managed lineage anchor must be absolute") - try: - anchor = supplied.resolve(strict=True) - stat_result = anchor.stat() - except OSError as exc: - raise ValueError("Managed lineage anchor is unavailable") from exc - if not anchor.is_dir(): - raise ValueError("Managed lineage anchor must be a directory") - return anchor, stat_result.st_dev, stat_result.st_ino - - -def _prepare_root(anchor: Path) -> Path: - current = anchor - for component in _NAMESPACE.parts: - current = current / component - if current.exists() and current.is_symlink(): - raise ValueError("Managed lineage namespace cannot contain symlinks") - current.mkdir(mode=0o700, exist_ok=True) - if current.is_symlink() or not current.is_dir(): - raise ValueError("Managed lineage namespace is not a regular directory") - for relative in ( - Path(_RECORDS_DIR), - Path(_INDEXES_DIR) / _FINAL_NATIVE_INDEX, - Path(_INDEXES_DIR) / _DISPATCH_INDEX, - ): - directory = current / relative - directory.mkdir(mode=0o700, parents=True, exist_ok=True) - if directory.is_symlink() or not directory.is_dir(): - raise ValueError("Managed lineage namespace is not a regular directory") - return current - - -@contextmanager -def _store_lock(root: Path, *, exclusive: bool) -> Iterator[None]: - lock_path = root / _LOCK_FILENAME - flags = os.O_CREAT | os.O_RDWR - if hasattr(os, "O_NOFOLLOW"): - flags |= os.O_NOFOLLOW - fd = os.open(lock_path, flags, 0o600) - try: - fcntl.flock(fd, fcntl.LOCK_EX if exclusive else fcntl.LOCK_SH) - yield - finally: - fcntl.flock(fd, fcntl.LOCK_UN) - os.close(fd) - - -def _record_path(root: Path, launch_id: str) -> Path: - # Construction validates the identity before this path can be written. - if ( - not isinstance(launch_id, str) - or len(launch_id) != 32 - or any(character not in "0123456789abcdef" for character in launch_id) - ): - raise ValueError("Invalid launch_id") - return root / _RECORDS_DIR / f"{launch_id}.json" - - -def _write_record(path: Path, lineage: ManagedHeadlessSessionLineage) -> None: - atomic_write( - path, - _canonical_json(_record_to_dict(lineage)), - strict_durability=True, - ) - - -def _read_record(path: Path) -> ManagedHeadlessSessionLineage: - raw = _read_bounded(path) - value = _strict_json_load(raw) - if _canonical_json(value).encode("utf-8") != raw: - raise ValueError("Managed lineage record is not canonical JSON") - return _lineage_from_dict(value) - - -def _read_runner_markers( - root: Path, - reference: ManagedHeadlessSessionLineageRef, - lineage: ManagedHeadlessSessionLineage, -) -> tuple[NativeShellCaptureObservation, ...]: - directory_flags = os.O_RDONLY | getattr(os, "O_DIRECTORY", 0) - nofollow = getattr(os, "O_NOFOLLOW", 0) - root_fd = os.open(root, directory_flags | nofollow) - observations_fd = -1 - launch_fd = -1 - try: - try: - observations_fd = os.open( - _RUNNER_OBSERVATIONS_DIR, - directory_flags | nofollow, - dir_fd=root_fd, - ) - launch_fd = os.open( - reference.launch_id, - directory_flags | nofollow, - dir_fd=observations_fd, - ) - except FileNotFoundError: - return () - parsed: list[NativeShellCaptureObservation] = [] - for name in sorted(os.listdir(launch_fd))[:_MAX_RUNNER_MARKERS]: - if not name.endswith(".json") or "/" in name or name in {".", ".."}: - continue - marker_fd = -1 - try: - marker_fd = os.open( - name, - os.O_RDONLY | nofollow, - dir_fd=launch_fd, - ) - metadata = os.fstat(marker_fd) - if ( - not stat.S_ISREG(metadata.st_mode) - or metadata.st_nlink != 1 - or metadata.st_size > _MAX_RUNNER_MARKER_BYTES - ): - continue - raw = os.read(marker_fd, _MAX_RUNNER_MARKER_BYTES + 1) - except OSError: - continue - finally: - if marker_fd >= 0: - os.close(marker_fd) - try: - marker = _strict_json_load(raw) - if _canonical_json(marker).encode("utf-8") != raw: - continue - if not isinstance(marker, dict) or set(marker) != { - "schema_version", - "launch_id", - "lineage_digest", - "observation", - }: - continue - if ( - marker["schema_version"] != MANAGED_HEADLESS_SESSION_LINEAGE_SCHEMA_VERSION - or marker["launch_id"] != reference.launch_id - or marker["lineage_digest"] != reference.lineage_digest - ): - continue - observation = NativeShellCaptureObservation.from_dict(marker["observation"]) - if observation.attempt_id not in lineage.attempt_ids: - continue - except (TypeError, ValueError): - continue - parsed.append(observation) - return tuple(dict.fromkeys(parsed)) - finally: - if launch_fd >= 0: - os.close(launch_fd) - if observations_fd >= 0: - os.close(observations_fd) - os.close(root_fd) - - -def _settle_runner_observation( - root: Path, - reference: ManagedHeadlessSessionLineageRef, - observation: NativeShellCaptureObservation, -) -> None: - """Durably consume one marker after its lineage mutation has settled.""" - marker = { - "schema_version": MANAGED_HEADLESS_SESSION_LINEAGE_SCHEMA_VERSION, - "launch_id": reference.launch_id, - "lineage_digest": reference.lineage_digest, - "observation": observation.to_dict(), - } - marker_name = f"{hashlib.sha256(_canonical_json(marker).encode('utf-8')).hexdigest()}.json" - directory_flags = os.O_RDONLY | getattr(os, "O_DIRECTORY", 0) - nofollow = getattr(os, "O_NOFOLLOW", 0) - root_fd = os.open(root, directory_flags | nofollow) - observations_fd = -1 - launch_fd = -1 - try: - try: - observations_fd = os.open( - _RUNNER_OBSERVATIONS_DIR, - directory_flags | nofollow, - dir_fd=root_fd, - ) - launch_fd = os.open( - reference.launch_id, - directory_flags | nofollow, - dir_fd=observations_fd, - ) - os.unlink(marker_name, dir_fd=launch_fd) - os.fsync(launch_fd) - except FileNotFoundError: - return - finally: - if launch_fd >= 0: - os.close(launch_fd) - if observations_fd >= 0: - os.close(observations_fd) - os.close(root_fd) - - -def _index_path(root: Path, index_name: str, key: str) -> Path: - if not isinstance(key, str) or not key or "\x00" in key: - raise ValueError(f"Invalid managed lineage {index_name} key") - if len(key.encode("utf-8")) > 512: - raise ValueError(f"Managed lineage {index_name} key is oversized") - digest = hashlib.sha256(key.encode("utf-8")).hexdigest() - return root / _INDEXES_DIR / index_name / f"{digest}.json" - - -def _write_index(root: Path, index_name: str, key: str, launch_id: str) -> None: - path = _index_path(root, index_name, key) - atomic_write( - path, - _canonical_json( - { - "schema_version": MANAGED_HEADLESS_SESSION_LINEAGE_SCHEMA_VERSION, - "key": key, - "launch_id": launch_id, - } - ), - strict_durability=True, - ) - - -def _remove_index(root: Path, index_name: str, key: str) -> None: - """Durably remove one index entry while its namespace lock is held.""" - path = _index_path(root, index_name, key) - path.unlink() - directory_fd = os.open(path.parent, os.O_RDONLY) - try: - os.fsync(directory_fd) - finally: - os.close(directory_fd) - - -def _read_index(root: Path, index_name: str, key: str) -> str: - path = _index_path(root, index_name, key) - value = _strict_json_load(_read_bounded(path)) - expected_fields = {"schema_version", "key", "launch_id"} - if not isinstance(value, dict) or set(value) != expected_fields: - raise ValueError("Invalid managed lineage index") - if value["schema_version"] != MANAGED_HEADLESS_SESSION_LINEAGE_SCHEMA_VERSION: - raise ValueError("Unsupported managed lineage index schema") - if value["key"] != key: - raise ValueError("Managed lineage index key mismatch") - return _strict_str(value["launch_id"], "launch_id") - - -def _assert_index_available( - root: Path, - index_name: str, - key: str, - launch_id: str, -) -> None: - path = _index_path(root, index_name, key) - if not path.exists(): - return - indexed_launch_id = _read_index(root, index_name, key) - if indexed_launch_id != launch_id: - raise ManagedHeadlessSessionLineageConflictError( - f"Managed lineage {index_name} identity is already owned" - ) - +# Index/runner/records helpers live in sibling modules; re-exported for +# existing callers using the canonical _managed_headless_session_lineage path. +from autoskillit.execution.session._managed_headless_session_lineage_records import ( # noqa: F401 + _creation_projection, + _new_lineage, + _next_generation, + _prepare_root, + _read_record, + _record_path, + _resolve_anchor, + _store_lock, + _write_record, +) +from autoskillit.execution.session._managed_headless_session_lineage_runner import ( # noqa: F401 + _read_runner_markers, + _settle_runner_observation, +) +from autoskillit.execution.session._managed_headless_session_lineage_indexes import ( # noqa: F401 + _assert_index_available, + _index_path, + _read_index, + _remove_index, + _write_index, +) def _validate_anchor_identity( lineage: ManagedHeadlessSessionLineage, diff --git a/src/autoskillit/execution/session/_managed_headless_session_lineage_indexes.py b/src/autoskillit/execution/session/_managed_headless_session_lineage_indexes.py new file mode 100644 index 000000000..6d2f21835 --- /dev/null +++ b/src/autoskillit/execution/session/_managed_headless_session_lineage_indexes.py @@ -0,0 +1,76 @@ +"""Index helpers for the managed headless session lineage store. + +Extracted from `_managed_headless_session_lineage.py`. +""" + +from __future__ import annotations + +from pathlib import Path + +from autoskillit.execution.session._managed_headless_session_lineage_codec import _strict_str + + +def _index_path(root: Path, index_name: str, key: str) -> Path: + if not isinstance(key, str) or not key or "\x00" in key: + raise ValueError(f"Invalid managed lineage {index_name} key") + if len(key.encode("utf-8")) > 512: + raise ValueError(f"Managed lineage {index_name} key is oversized") + digest = hashlib.sha256(key.encode("utf-8")).hexdigest() + return root / _INDEXES_DIR / index_name / f"{digest}.json" + + +def _write_index(root: Path, index_name: str, key: str, launch_id: str) -> None: + path = _index_path(root, index_name, key) + atomic_write( + path, + _canonical_json( + { + "schema_version": MANAGED_HEADLESS_SESSION_LINEAGE_SCHEMA_VERSION, + "key": key, + "launch_id": launch_id, + } + ), + strict_durability=True, + ) + + +def _remove_index(root: Path, index_name: str, key: str) -> None: + """Durably remove one index entry while its namespace lock is held.""" + path = _index_path(root, index_name, key) + path.unlink() + directory_fd = os.open(path.parent, os.O_RDONLY) + try: + os.fsync(directory_fd) + finally: + os.close(directory_fd) + + +def _read_index(root: Path, index_name: str, key: str) -> str: + path = _index_path(root, index_name, key) + value = _strict_json_load(_read_bounded(path)) + expected_fields = {"schema_version", "key", "launch_id"} + if not isinstance(value, dict) or set(value) != expected_fields: + raise ValueError("Invalid managed lineage index") + if value["schema_version"] != MANAGED_HEADLESS_SESSION_LINEAGE_SCHEMA_VERSION: + raise ValueError("Unsupported managed lineage index schema") + if value["key"] != key: + raise ValueError("Managed lineage index key mismatch") + return _strict_str(value["launch_id"], "launch_id") + + +def _assert_index_available( + root: Path, + index_name: str, + key: str, + launch_id: str, +) -> None: + path = _index_path(root, index_name, key) + if not path.exists(): + return + indexed_launch_id = _read_index(root, index_name, key) + if indexed_launch_id != launch_id: + raise ManagedHeadlessSessionLineageConflictError( + f"Managed lineage {index_name} identity is already owned" + ) + + diff --git a/src/autoskillit/execution/session/_managed_headless_session_lineage_records.py b/src/autoskillit/execution/session/_managed_headless_session_lineage_records.py new file mode 100644 index 000000000..203b323d1 --- /dev/null +++ b/src/autoskillit/execution/session/_managed_headless_session_lineage_records.py @@ -0,0 +1,192 @@ +"""Record I/O helpers for the managed headless session lineage store. + +Extracted from `_managed_headless_session_lineage.py`. These helpers +own the per-record creation-projection, anchor resolution, root +preparation, file-locked store context, record path resolution, and +read/write of one lineage record. +""" + +from __future__ import annotations + +import fcntl +import os +from collections.abc import Iterator +from contextlib import contextmanager +from dataclasses import replace +from pathlib import Path + +from autoskillit.core import ( + MANAGED_HEADLESS_SESSION_LINEAGE_SCHEMA_VERSION, + ManagedHeadlessSessionKind, + NativeShellCaptureDecision, + atomic_write, +) +from autoskillit.execution.session._managed_headless_session_lineage import ( + _DISPATCH_INDEX, + _FINAL_NATIVE_INDEX, + _INDEXES_DIR, + _LOCK_FILENAME, + _MAX_RECORD_BYTES, + _NAMESPACE, + _RECORDS_DIR, +) +from autoskillit.execution.session._managed_headless_session_lineage_codec import ( + canonical_json as _canonical_json, + digest as _digest, + lineage_from_dict as _lineage_from_dict, + record_payload as _record_payload, + record_to_dict as _record_to_dict, + strict_json_load as _strict_json_load, +) +from autoskillit.core import ManagedHeadlessSessionLineage + + +def _new_lineage( + *, + launch_id: str, + decision: NativeShellCaptureDecision, + backend: str, + session_kind: ManagedHeadlessSessionKind, + lineage_anchor: Path, + anchor_device: int, + anchor_inode: int, + dispatch_id: str | None, +) -> ManagedHeadlessSessionLineage: + identity = { + "schema_version": MANAGED_HEADLESS_SESSION_LINEAGE_SCHEMA_VERSION, + "launch_id": launch_id, + "decision": decision.to_dict(), + "backend": backend, + "session_kind": session_kind.value, + "lineage_anchor": str(lineage_anchor), + "anchor_device": anchor_device, + "anchor_inode": anchor_inode, + } + lineage_digest = _digest(identity) + provisional = ManagedHeadlessSessionLineage( + launch_id=launch_id, + decision=decision, + backend=backend, + session_kind=session_kind, + lineage_anchor=str(lineage_anchor), + anchor_device=anchor_device, + anchor_inode=anchor_inode, + lineage_digest=lineage_digest, + generation=0, + record_digest="0" * 64, + dispatch_id=dispatch_id, + ) + return replace(provisional, record_digest=_digest(_record_payload(provisional))) + + +def _next_generation( + lineage: ManagedHeadlessSessionLineage, +) -> ManagedHeadlessSessionLineage: + provisional = replace( + lineage, + generation=lineage.generation + 1, + record_digest="0" * 64, + ) + return replace(provisional, record_digest=_digest(_record_payload(provisional))) + + +def _creation_projection(lineage: ManagedHeadlessSessionLineage) -> tuple[object, ...]: + return ( + lineage.launch_id, + lineage.decision, + lineage.backend, + lineage.session_kind, + lineage.lineage_anchor, + lineage.anchor_device, + lineage.anchor_inode, + lineage.dispatch_id, + ) + + +def _resolve_anchor(lineage_anchor: Path) -> tuple[Path, int, int]: + supplied = Path(lineage_anchor).expanduser() + if not supplied.is_absolute(): + raise ValueError("Managed lineage anchor must be absolute") + try: + anchor = supplied.resolve(strict=True) + stat_result = anchor.stat() + except OSError as exc: + raise ValueError("Managed lineage anchor is unavailable") from exc + if not anchor.is_dir(): + raise ValueError("Managed lineage anchor must be a directory") + return anchor, stat_result.st_dev, stat_result.st_ino + + +def _prepare_root(anchor: Path) -> Path: + current = anchor + for component in _NAMESPACE.parts: + current = current / component + if current.exists() and current.is_symlink(): + raise ValueError("Managed lineage namespace cannot contain symlinks") + current.mkdir(mode=0o700, exist_ok=True) + if current.is_symlink() or not current.is_dir(): + raise ValueError("Managed lineage namespace is not a regular directory") + for relative in ( + Path(_RECORDS_DIR), + Path(_INDEXES_DIR) / _FINAL_NATIVE_INDEX, + Path(_INDEXES_DIR) / _DISPATCH_INDEX, + ): + directory = current / relative + directory.mkdir(mode=0o700, parents=True, exist_ok=True) + if directory.is_symlink() or not directory.is_dir(): + raise ValueError("Managed lineage namespace is not a regular directory") + return current + + +@contextmanager +def _store_lock(root: Path, *, exclusive: bool) -> Iterator[None]: + lock_path = root / _LOCK_FILENAME + flags = os.O_CREAT | os.O_RDWR + if hasattr(os, "O_NOFOLLOW"): + flags |= os.O_NOFOLLOW + fd = os.open(lock_path, flags, 0o600) + try: + fcntl.flock(fd, fcntl.LOCK_EX if exclusive else fcntl.LOCK_SH) + yield + finally: + fcntl.flock(fd, fcntl.LOCK_UN) + os.close(fd) + + +def _record_path(root: Path, launch_id: str) -> Path: + # Construction validates the identity before this path can be written. + if ( + not isinstance(launch_id, str) + or len(launch_id) != 32 + or any(character not in "0123456789abcdef" for character in launch_id) + ): + raise ValueError("Invalid launch_id") + return root / _RECORDS_DIR / f"{launch_id}.json" + + +def _write_record(path: Path, lineage: ManagedHeadlessSessionLineage) -> None: + atomic_write( + path, + _canonical_json(_record_to_dict(lineage)), + strict_durability=True, + ) + + +def _read_record(path: Path) -> ManagedHeadlessSessionLineage: + raw = _read_bounded(path) + value = _strict_json_load(raw) + if _canonical_json(value).encode("utf-8") != raw: + raise ValueError("Managed lineage record is not canonical JSON") + return _lineage_from_dict(value) + + +def _read_bounded(path: Path) -> bytes: + try: + with path.open("rb") as handle: + raw = handle.read(_MAX_RECORD_BYTES + 1) + except FileNotFoundError: + raise FileNotFoundError(f"Managed lineage record not found: {path.name}") from None + if len(raw) > _MAX_RECORD_BYTES: + raise ValueError("Managed lineage artifact is oversized") + return raw + diff --git a/src/autoskillit/execution/session/_managed_headless_session_lineage_runner.py b/src/autoskillit/execution/session/_managed_headless_session_lineage_runner.py new file mode 100644 index 000000000..43d292926 --- /dev/null +++ b/src/autoskillit/execution/session/_managed_headless_session_lineage_runner.py @@ -0,0 +1,150 @@ +"""Runner observation helpers for the managed headless session lineage store. + +Extracted from `_managed_headless_session_lineage.py`. +""" + +from __future__ import annotations + +import hashlib +import os +import stat +from pathlib import Path + +from autoskillit.core import ( + MANAGED_HEADLESS_SESSION_LINEAGE_SCHEMA_VERSION, + ManagedHeadlessSessionLineage, + ManagedHeadlessSessionLineageRef, + NativeShellCaptureObservation, +) +from autoskillit.execution.session._managed_headless_session_lineage import ( + _MAX_RUNNER_MARKER_BYTES, + _MAX_RUNNER_MARKERS, + _RUNNER_OBSERVATIONS_DIR, +) +from autoskillit.execution.session._managed_headless_session_lineage_codec import ( + canonical_json as _canonical_json, + strict_json_load as _strict_json_load, +) + + +def _read_runner_markers( + root: Path, + reference: ManagedHeadlessSessionLineageRef, + lineage: ManagedHeadlessSessionLineage, +) -> tuple[NativeShellCaptureObservation, ...]: + directory_flags = os.O_RDONLY | getattr(os, "O_DIRECTORY", 0) + nofollow = getattr(os, "O_NOFOLLOW", 0) + root_fd = os.open(root, directory_flags | nofollow) + observations_fd = -1 + launch_fd = -1 + try: + try: + observations_fd = os.open( + _RUNNER_OBSERVATIONS_DIR, + directory_flags | nofollow, + dir_fd=root_fd, + ) + launch_fd = os.open( + reference.launch_id, + directory_flags | nofollow, + dir_fd=observations_fd, + ) + except FileNotFoundError: + return () + parsed: list[NativeShellCaptureObservation] = [] + for name in sorted(os.listdir(launch_fd))[:_MAX_RUNNER_MARKERS]: + if not name.endswith(".json") or "/" in name or name in {".", ".."}: + continue + marker_fd = -1 + try: + marker_fd = os.open( + name, + os.O_RDONLY | nofollow, + dir_fd=launch_fd, + ) + metadata = os.fstat(marker_fd) + if ( + not stat.S_ISREG(metadata.st_mode) + or metadata.st_nlink != 1 + or metadata.st_size > _MAX_RUNNER_MARKER_BYTES + ): + continue + raw = os.read(marker_fd, _MAX_RUNNER_MARKER_BYTES + 1) + except OSError: + continue + finally: + if marker_fd >= 0: + os.close(marker_fd) + try: + marker = _strict_json_load(raw) + if _canonical_json(marker).encode("utf-8") != raw: + continue + if not isinstance(marker, dict) or set(marker) != { + "schema_version", + "launch_id", + "lineage_digest", + "observation", + }: + continue + if ( + marker["schema_version"] != MANAGED_HEADLESS_SESSION_LINEAGE_SCHEMA_VERSION + or marker["launch_id"] != reference.launch_id + or marker["lineage_digest"] != reference.lineage_digest + ): + continue + observation = NativeShellCaptureObservation.from_dict(marker["observation"]) + if observation.attempt_id not in lineage.attempt_ids: + continue + except (TypeError, ValueError): + continue + parsed.append(observation) + return tuple(dict.fromkeys(parsed)) + finally: + if launch_fd >= 0: + os.close(launch_fd) + if observations_fd >= 0: + os.close(observations_fd) + os.close(root_fd) + + +def _settle_runner_observation( + root: Path, + reference: ManagedHeadlessSessionLineageRef, + observation: NativeShellCaptureObservation, +) -> None: + """Durably consume one marker after its lineage mutation has settled.""" + marker = { + "schema_version": MANAGED_HEADLESS_SESSION_LINEAGE_SCHEMA_VERSION, + "launch_id": reference.launch_id, + "lineage_digest": reference.lineage_digest, + "observation": observation.to_dict(), + } + marker_name = f"{hashlib.sha256(_canonical_json(marker).encode('utf-8')).hexdigest()}.json" + directory_flags = os.O_RDONLY | getattr(os, "O_DIRECTORY", 0) + nofollow = getattr(os, "O_NOFOLLOW", 0) + root_fd = os.open(root, directory_flags | nofollow) + observations_fd = -1 + launch_fd = -1 + try: + try: + observations_fd = os.open( + _RUNNER_OBSERVATIONS_DIR, + directory_flags | nofollow, + dir_fd=root_fd, + ) + launch_fd = os.open( + reference.launch_id, + directory_flags | nofollow, + dir_fd=observations_fd, + ) + os.unlink(marker_name, dir_fd=launch_fd) + os.fsync(launch_fd) + except FileNotFoundError: + return + finally: + if launch_fd >= 0: + os.close(launch_fd) + if observations_fd >= 0: + os.close(observations_fd) + os.close(root_fd) + From 3ea195a865e77d59d5e8b53a29eafff4064395bc Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 17 Aug 2026 12:53:35 -0700 Subject: [PATCH 10/11] refactor(session): extract contract codec to _skill_session_contract_codec.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move validation, source/exploration-vector/execution-identity serialization, contract to/from dict, manifest construction, and _digest_json helper from _skill_session_contract_store.py (907 → 393 lines) into _skill_session_contract_codec.py. The parent re-exports _digest_json and all other moved symbols so existing imports keep working — notably tests/server/test_run_skill_resume.py:640 and tests/execution/test_skill_session_contract_store.py which import _digest_json from the canonical path. Refs: #4664 --- .../session/_skill_session_contract_codec.py | 571 ++++++++++++++++++ .../session/_skill_session_contract_store.py | 558 +---------------- 2 files changed, 593 insertions(+), 536 deletions(-) create mode 100644 src/autoskillit/execution/session/_skill_session_contract_codec.py diff --git a/src/autoskillit/execution/session/_skill_session_contract_codec.py b/src/autoskillit/execution/session/_skill_session_contract_codec.py new file mode 100644 index 000000000..278eb325f --- /dev/null +++ b/src/autoskillit/execution/session/_skill_session_contract_codec.py @@ -0,0 +1,571 @@ +"""Codec helpers for the skill session contract store. + +Extracted from `_skill_session_contract_store.py`. These helpers own +the validation, source/exploration-vector/execution-identity serialization, +contract to/from dict, manifest construction, and digest computation. + +The parent module re-exports `_digest_json` so existing callers using +`from autoskillit.execution.session._skill_session_contract_store import +_digest_json` continue to resolve. +""" + +from __future__ import annotations + +import hashlib +import json +from collections.abc import Mapping +from pathlib import Path +from typing import Any + +from autoskillit.core import ( + ExecutionIdentity, + ExplorationVectorDef, + ManagedHeadlessSessionLineageRef, + SkillSessionContract, + SkillSourceRef, + atomic_write, +) +from autoskillit.execution.session._skill_session_contract_store import ( + _STORE_MANIFEST_SCHEMA_VERSION, + _MANIFEST_FILENAME, +) + + +def _validate_raw_session_id(session_id: str) -> None: + if not isinstance(session_id, str) or not session_id or "\x00" in session_id: + raise ValueError(f"Invalid session ID: {session_id!r}") + + +def _finalized_contract_path(sessions_root: Path, session_id: str) -> Path: + _validate_raw_session_id(session_id) + key = hashlib.sha256(session_id.encode()).hexdigest() + path = sessions_root / key + if not path.resolve().is_relative_to(sessions_root.resolve()): + raise ValueError(f"Skill session contract path escapes sessions root: {path}") + return path + + +def _delete_finalized_contract(sessions_root: Path, session_id: str) -> None: + shutil.rmtree( + _finalized_contract_path(sessions_root, session_id), + ignore_errors=True, + ) + + +def _validate_digest_map( + field_name: str, + digests: Mapping[str, str], + closure: set[str], +) -> None: + if set(digests) != closure: + raise SkillContractError(f"{field_name} keys must exactly match closure") + for name, digest in digests.items(): + if not isinstance(digest, str) or not _SHA256_RE.fullmatch(digest): + raise SkillContractError(f"Invalid {field_name} digest for {name!r}") + + +def _validate_contract(contract: SkillSessionContract) -> None: + if contract.schema_version != SKILL_SESSION_CONTRACT_SCHEMA_VERSION: + raise SkillContractError("Unsupported skill session contract schema") + if contract.launch_contract is not None: + if contract.launch_contract_digest != contract.launch_contract.digest: + raise SkillContractError("Skill session launch contract digest mismatch") + if contract.launch_contract.effective_backend != contract.backend: + raise SkillContractError("Skill session launch backend mismatch") + if contract.launch_contract.cwd != contract.cwd: + raise SkillContractError("Skill session launch cwd mismatch") + if not contract.root_name or not contract.closure: + raise SkillContractError("Skill session contract requires a root and closure") + closure = set(contract.closure) + if len(closure) != len(contract.closure) or contract.root_name not in closure: + raise SkillContractError("Skill session contract closure is invalid") + if set(contract.source_refs) != closure: + raise SkillContractError("source_refs keys must exactly match closure") + if set(contract.member_roles) != closure: + raise SkillContractError("member_roles keys must exactly match closure") + if set(contract.member_capabilities) != closure: + raise SkillContractError("member_capabilities keys must exactly match closure") + if set(contract.member_activate_deps) != closure: + raise SkillContractError("member_activate_deps keys must exactly match closure") + if set(contract.canonical_contents) != closure: + raise SkillContractError("canonical_contents keys must exactly match closure") + if set(contract.exploration_vectors) != closure: + raise SkillContractError("exploration_vectors keys must exactly match closure") + if set(contract.exploration_sidecar_digests) != closure: + raise SkillContractError("exploration_sidecar_digests keys must exactly match closure") + for name in contract.closure: + source_ref = contract.source_refs[name] + if not isinstance(source_ref, SkillSourceRef): + raise SkillContractError(f"source reference for {name!r} must be typed") + if source_ref.logical_name != name: + raise SkillContractError(f"source reference logical name mismatch for {name!r}") + role = contract.member_roles[name] + capabilities = contract.member_capabilities[name] + validate_skill_capability_roles(capabilities, role) + if role is not contract.execution_role: + raise SkillContractError(f"member role mismatch for {name!r}") + canonical_digest = hashlib.sha256(contract.canonical_contents[name].encode()).hexdigest() + if canonical_digest != contract.canonical_digests[name]: + raise SkillContractError(f"canonical content digest mismatch for {name!r}") + vectors = contract.exploration_vectors[name] + if any(not isinstance(vector, ExplorationVectorDef) for vector in vectors): + raise SkillContractError(f"exploration vectors for {name!r} must be typed") + vector_ids = tuple(vector.id for vector in vectors) + if len(vector_ids) != len(set(vector_ids)): + raise SkillContractError(f"exploration vector ids for {name!r} must be unique") + member_capability_union = frozenset().union( + *(contract.member_capabilities[name] for name in contract.closure) + ) + if member_capability_union != contract.capability_union: + raise SkillContractError("member capability union does not match contract") + _validate_digest_map("canonical_digests", contract.canonical_digests, closure) + _validate_digest_map("projected_digests", contract.projected_digests, closure) + if contract.projection_version != SKILL_PROJECTION_VERSION: + raise SkillContractError( + f"unsupported projection_version {contract.projection_version}; " + f"expected {SKILL_PROJECTION_VERSION}" + ) + if not contract.project_root or not contract.cwd: + raise SkillContractError("project_root and cwd are required") + if not contract.backend or not contract.resolved_command: + raise SkillContractError("backend and resolved_command are required") + validate_skill_capability_roles(contract.capability_union, contract.execution_role) + + +def _validate_relative_path(value: str) -> Path: + if not isinstance(value, str) or not value: + raise ValueError("Projected snapshot path must be a non-empty relative path") + relative = PurePosixPath(value) + if relative.is_absolute() or any(part in ("", ".", "..") for part in relative.parts): + raise ValueError(f"Unsafe projected snapshot path: {value!r}") + return Path(*relative.parts) + + +def _validate_snapshot_mapping( + contract: SkillSessionContract, + snapshot: Mapping[str, str], +) -> dict[str, str]: + if not isinstance(snapshot, Mapping): + raise ValueError("Projected snapshot must be a mapping") + by_skill: dict[str, str] = {} + seen_paths: set[Path] = set() + for raw_relative_path, content in snapshot.items(): + relative_path = _validate_relative_path(raw_relative_path) + if relative_path in seen_paths: + raise ValueError(f"Duplicate projected snapshot path: {raw_relative_path!r}") + if relative_path.name != "SKILL.md" or len(relative_path.parts) < 2: + raise ValueError( + f"Projected snapshot path must end in /SKILL.md: {raw_relative_path!r}" + ) + if not isinstance(content, str): + raise ValueError(f"Projected snapshot content must be text: {raw_relative_path!r}") + skill_name = relative_path.parent.name + if skill_name in by_skill: + raise ValueError(f"Multiple projected documents for skill {skill_name!r}") + digest = hashlib.sha256(content.encode()).hexdigest() + if contract.projected_digests.get(skill_name) != digest: + raise ValueError(f"Projected snapshot digest mismatch for {skill_name!r}") + by_skill[skill_name] = relative_path.as_posix() + seen_paths.add(relative_path) + if set(by_skill) != set(contract.closure): + raise ValueError("Projected snapshot documents must exactly match closure") + return by_skill + + +def _source_ref_to_dict(source_ref: SkillSourceRef) -> dict[str, Any]: + return { + "origin": source_ref.origin.value, + "logical_name": source_ref.logical_name, + "skill_path": str(source_ref.skill_path), + "search_dir": source_ref.search_dir, + "precedence": source_ref.precedence, + } + + +def _source_ref_from_dict(data: Mapping[str, Any]) -> SkillSourceRef: + try: + return SkillSourceRef( + origin=SkillSource(str(data["origin"])), + logical_name=str(data["logical_name"]), + skill_path=Path(str(data["skill_path"])), + search_dir=(str(data["search_dir"]) if data.get("search_dir") is not None else None), + precedence=(int(data["precedence"]) if data.get("precedence") is not None else None), + ) + except (KeyError, TypeError, ValueError) as exc: + raise ValueError("Invalid skill source reference") from exc + + +_SERIALIZED_EXPLORATION_VECTOR_KEYS = frozenset( + { + "id", + "disposition", + "rationale", + "applicability", + "role", + "profile", + "relationship_classes", + "task_id", + "frontier_item_id", + "depends_on", + "scope", + "body", + "digest", + } +) +_EXECUTION_IDENTITY_KEYS = frozenset(ExecutionIdentity.empty().to_dict()) +_CHILD_EXECUTION_IDENTITY_KEYS = frozenset( + { + "task_id", + "role", + "plan_digest", + "definition_digest", + "requested_backend", + "effective_backend", + "requested_model", + "effective_model", + "requested_effort", + "effective_effort", + "session_id", + } +) + + +def _exploration_vector_to_dict(vector: ExplorationVectorDef) -> dict[str, Any]: + task = vector.task + return { + "id": vector.id, + "disposition": vector.disposition.value, + "rationale": vector.rationale, + "applicability": vector.applicability.value, + "role": vector.role, + "profile": vector.profile.value, + "relationship_classes": [item.value for item in vector.relationship_classes], + "task_id": task.task_id, + "frontier_item_id": task.frontier_item_id, + "depends_on": list(task.depends_on), + "scope": list(task.scope), + "body": vector.body, + "digest": vector.digest, + } + + +def _exploration_vector_from_dict(value: object) -> ExplorationVectorDef: + if not isinstance(value, dict) or set(value) != _SERIALIZED_EXPLORATION_VECTOR_KEYS: + raise ValueError("serialized exploration vector keys are invalid") + for field_name in ("relationship_classes", "depends_on", "scope"): + field_value = value[field_name] + if not isinstance(field_value, list) or any( + not isinstance(item, str) for item in field_value + ): + raise ValueError(f"serialized exploration vector {field_name} is invalid") + for field_name in ( + "id", + "disposition", + "rationale", + "applicability", + "profile", + "task_id", + "frontier_item_id", + "body", + "digest", + ): + if not isinstance(value[field_name], str): + raise ValueError(f"serialized exploration vector {field_name} is invalid") + if value["role"] is not None and not isinstance(value["role"], str): + raise ValueError("serialized exploration vector role is invalid") + profile = RepositoryProfileId(value["profile"]) + vector = ExplorationVectorDef( + id=value["id"], + disposition=ExplorationVectorDisposition(value["disposition"]), + rationale=value["rationale"], + applicability=ExplorationVectorApplicabilityId(value["applicability"]), + role=value["role"], + profile=profile, + relationship_classes=tuple( + RelationshipKind(item) for item in value["relationship_classes"] + ), + task=ExplorationTaskSpec( + task_id=value["task_id"], + frontier_item_id=value["frontier_item_id"], + profile=profile, + depends_on=tuple(value["depends_on"]), + scope=tuple(value["scope"]), + ), + body=value["body"], + ) + if value["digest"] != vector.digest: + raise ValueError("serialized exploration vector digest mismatch") + return vector + + +def _execution_identity_from_dict(value: object) -> ExecutionIdentity: + if not isinstance(value, dict) or set(value) != _EXECUTION_IDENTITY_KEYS: + raise ValueError("serialized execution identity keys are invalid") + children = value.get("children") + if not isinstance(children, list): + raise ValueError("serialized execution identity children must be a list") + scalar_values = {key: item for key, item in value.items() if key != "children"} + if any(not isinstance(item, str) for item in scalar_values.values()): + raise ValueError("serialized execution identity values must be text") + parsed_children: list[ChildExecutionIdentity] = [] + for child in children: + if ( + not isinstance(child, dict) + or set(child) != _CHILD_EXECUTION_IDENTITY_KEYS + or any(not isinstance(item, str) for item in child.values()) + ): + raise ValueError("serialized child execution identity is invalid") + parsed_children.append(ChildExecutionIdentity(**child)) + return ExecutionIdentity(**scalar_values, children=tuple(parsed_children)) + + +def _contract_to_dict(contract: SkillSessionContract) -> dict[str, Any]: + return { + "schema_version": contract.schema_version, + "root_name": contract.root_name, + "execution_role": contract.execution_role.value, + "source_refs": { + name: _source_ref_to_dict(source_ref) + for name, source_ref in sorted(contract.source_refs.items()) + }, + "closure": list(contract.closure), + "capability_union": sorted(contract.capability_union), + "canonical_digests": dict(sorted(contract.canonical_digests.items())), + "projected_digests": dict(sorted(contract.projected_digests.items())), + "projection_version": contract.projection_version, + "project_root": contract.project_root, + "cwd": contract.cwd, + "backend": contract.backend, + "resolved_command": contract.resolved_command, + "member_roles": {name: role.value for name, role in sorted(contract.member_roles.items())}, + "member_capabilities": { + name: sorted(capabilities) + for name, capabilities in sorted(contract.member_capabilities.items()) + }, + "member_activate_deps": { + name: list(dependencies) + for name, dependencies in sorted(contract.member_activate_deps.items()) + }, + "canonical_contents": dict(sorted(contract.canonical_contents.items())), + "exploration_vectors": { + name: [_exploration_vector_to_dict(vector) for vector in vectors] + for name, vectors in sorted(contract.exploration_vectors.items()) + }, + "exploration_sidecar_digests": dict(sorted(contract.exploration_sidecar_digests.items())), + "resolved_exploration_profile": ( + contract.resolved_exploration_profile.value + if contract.resolved_exploration_profile is not None + else None + ), + "active_exploration_applicabilities": sorted( + item.value for item in contract.active_exploration_applicabilities + ), + "expected_output_patterns": list(contract.expected_output_patterns), + "write_behavior": { + "mode": contract.write_behavior.mode, + "expected_when": list(contract.write_behavior.expected_when), + }, + "read_only": contract.read_only, + "scope_discipline": contract.scope_discipline, + "parent_sandbox_mode": contract.parent_sandbox_mode, + "completion_required": contract.completion_required, + "skill_contract_json": contract.skill_contract_json, + "projection_substitutions": [list(item) for item in contract.projection_substitutions], + "projection_gating": contract.projection_gating, + "projection_namespace": contract.projection_namespace, + "launch_contract": ( + json.loads(contract.launch_contract.canonical_json) + if contract.launch_contract is not None + else None + ), + "launch_contract_digest": contract.launch_contract_digest, + "execution_identity": contract.execution_identity.to_dict(), + } + + +def _contract_from_dict(data: Mapping[str, Any]) -> SkillSessionContract: + try: + source_refs_raw = data["source_refs"] + if not isinstance(source_refs_raw, dict): + raise ValueError("source_refs must be an object") + read_only = data.get("read_only", False) + if not isinstance(read_only, bool): + raise ValueError("read_only must be a boolean") + scope_discipline = data.get("scope_discipline", False) + if not isinstance(scope_discipline, bool): + raise ValueError("scope_discipline must be a boolean") + parent_sandbox_mode = data["parent_sandbox_mode"] + if not isinstance(parent_sandbox_mode, str): + raise ValueError("parent_sandbox_mode must be text") + completion_required = data.get("completion_required", False) + if not isinstance(completion_required, bool): + raise ValueError("completion_required must be a boolean") + projection_gating = data.get("projection_gating") + if projection_gating is not None and not isinstance(projection_gating, bool): + raise ValueError("projection_gating must be a boolean or null") + projection_substitutions = data.get("projection_substitutions", []) + if not isinstance(projection_substitutions, list) or any( + not isinstance(item, list) or len(item) != 2 for item in projection_substitutions + ): + raise ValueError("projection_substitutions entries must be two-element lists") + launch_payload = data.get("launch_contract") + launch_digest = data.get("launch_contract_digest", "") + if launch_payload is not None and not isinstance(launch_payload, dict): + raise ValueError("launch_contract must be an object or null") + if not isinstance(launch_digest, str): + raise ValueError("launch_contract_digest must be a string") + launch_contract = ( + ResolvedLaunchContract.from_payload( + launch_payload, + expected_digest=launch_digest, + ) + if launch_payload is not None + else None + ) + exploration_vectors_raw = data["exploration_vectors"] + if not isinstance(exploration_vectors_raw, dict) or any( + not isinstance(vectors, list) for vectors in exploration_vectors_raw.values() + ): + raise ValueError("exploration_vectors must be an object of lists") + resolved_exploration_profile_raw = data["resolved_exploration_profile"] + if resolved_exploration_profile_raw is not None and not isinstance( + resolved_exploration_profile_raw, str + ): + raise ValueError("resolved_exploration_profile must be text or null") + active_applicabilities_raw = data["active_exploration_applicabilities"] + if not isinstance(active_applicabilities_raw, list) or any( + not isinstance(item, str) for item in active_applicabilities_raw + ): + raise ValueError("active_exploration_applicabilities must be a list of text") + return SkillSessionContract( + root_name=str(data["root_name"]), + execution_role=SkillExecutionRole(str(data["execution_role"])), + source_refs={ + str(name): _source_ref_from_dict(source_ref) + for name, source_ref in source_refs_raw.items() + if isinstance(source_ref, dict) + }, + closure=tuple(str(name) for name in data["closure"]), + capability_union=frozenset(str(cap) for cap in data["capability_union"]), + canonical_digests={ + str(name): str(digest) for name, digest in data["canonical_digests"].items() + }, + projected_digests={ + str(name): str(digest) for name, digest in data["projected_digests"].items() + }, + projection_version=int(data["projection_version"]), + project_root=str(data["project_root"]), + cwd=str(data["cwd"]), + backend=str(data["backend"]), + resolved_command=str(data["resolved_command"]), + member_roles={ + str(name): SkillExecutionRole(str(role)) + for name, role in data["member_roles"].items() + }, + member_capabilities={ + str(name): frozenset(str(capability) for capability in capabilities) + for name, capabilities in data["member_capabilities"].items() + }, + member_activate_deps={ + str(name): tuple(str(dependency) for dependency in dependencies) + for name, dependencies in data["member_activate_deps"].items() + }, + canonical_contents={ + str(name): str(content) for name, content in data["canonical_contents"].items() + }, + exploration_vectors={ + str(name): tuple(_exploration_vector_from_dict(vector) for vector in vectors) + for name, vectors in exploration_vectors_raw.items() + }, + exploration_sidecar_digests={ + str(name): str(digest) + for name, digest in data.get("exploration_sidecar_digests", {}).items() + }, + resolved_exploration_profile=( + RepositoryProfileId(resolved_exploration_profile_raw) + if resolved_exploration_profile_raw is not None + else None + ), + active_exploration_applicabilities=frozenset( + ExplorationVectorApplicabilityId(item) for item in active_applicabilities_raw + ), + expected_output_patterns=tuple( + str(pattern) for pattern in data.get("expected_output_patterns", []) + ), + write_behavior=WriteBehaviorSpec( + mode=( + str(data.get("write_behavior", {}).get("mode")) + if data.get("write_behavior", {}).get("mode") is not None + else None + ), + expected_when=tuple( + str(pattern) + for pattern in data.get("write_behavior", {}).get("expected_when", []) + ), + ), + read_only=read_only, + scope_discipline=scope_discipline, + parent_sandbox_mode=parent_sandbox_mode, + completion_required=completion_required, + skill_contract_json=str(data.get("skill_contract_json", "")), + projection_substitutions=tuple( + (str(item[0]), str(item[1])) for item in projection_substitutions + ), + projection_gating=projection_gating, + projection_namespace=( + str(data["projection_namespace"]) + if data.get("projection_namespace") is not None + else None + ), + launch_contract=launch_contract, + launch_contract_digest=launch_digest, + execution_identity=_execution_identity_from_dict(data["execution_identity"]), + schema_version=int(data["schema_version"]), + ) + except (KeyError, TypeError, ValueError, AttributeError, SkillContractError) as exc: + raise ValueError("Invalid serialized skill session contract") from exc + + +def _digest_json(value: Mapping[str, Any]) -> str: + encoded = json.dumps( + dict(value), + sort_keys=True, + separators=(",", ":"), + ).encode() + return hashlib.sha256(encoded).hexdigest() + + +def _build_manifest( + *, + contract: SkillSessionContract, + raw_session_id: str | None, + candidate_session_ids: tuple[str, ...], + snapshot_paths: Mapping[str, str], + managed_lineage_ref: ManagedHeadlessSessionLineageRef | None, +) -> dict[str, Any]: + contract_data = _contract_to_dict(contract) + return { + "schema_version": _STORE_MANIFEST_SCHEMA_VERSION, + "raw_session_id": raw_session_id, + "candidate_session_ids": list(candidate_session_ids), + "managed_lineage_ref": ( + managed_lineage_ref.to_dict() if managed_lineage_ref is not None else None + ), + "contract": contract_data, + "contract_digest": _digest_json(contract_data), + "snapshot_paths": dict(sorted(snapshot_paths.items())), + } + + +def _managed_lineage_ref_from_manifest( + manifest: Mapping[str, Any], +) -> ManagedHeadlessSessionLineageRef | None: + if "managed_lineage_ref" not in manifest: + raise ValueError("Skill session manifest is missing managed lineage reference") + value = manifest.get("managed_lineage_ref") + if value is None: + return None + try: + return ManagedHeadlessSessionLineageRef.from_dict(value) + except (TypeError, ValueError) as exc: + raise ValueError("Invalid skill session managed lineage reference") from exc diff --git a/src/autoskillit/execution/session/_skill_session_contract_store.py b/src/autoskillit/execution/session/_skill_session_contract_store.py index 5ecc57774..f681b5c6f 100644 --- a/src/autoskillit/execution/session/_skill_session_contract_store.py +++ b/src/autoskillit/execution/session/_skill_session_contract_store.py @@ -367,541 +367,27 @@ def delete_skill_session_contracts( _delete_finalized_contract(sessions_root, session_id) -def _validate_raw_session_id(session_id: str) -> None: - if not isinstance(session_id, str) or not session_id or "\x00" in session_id: - raise ValueError(f"Invalid session ID: {session_id!r}") - - -def _finalized_contract_path(sessions_root: Path, session_id: str) -> Path: - _validate_raw_session_id(session_id) - key = hashlib.sha256(session_id.encode()).hexdigest() - path = sessions_root / key - if not path.resolve().is_relative_to(sessions_root.resolve()): - raise ValueError(f"Skill session contract path escapes sessions root: {path}") - return path - - -def _delete_finalized_contract(sessions_root: Path, session_id: str) -> None: - shutil.rmtree( - _finalized_contract_path(sessions_root, session_id), - ignore_errors=True, - ) - - -def _validate_digest_map( - field_name: str, - digests: Mapping[str, str], - closure: set[str], -) -> None: - if set(digests) != closure: - raise SkillContractError(f"{field_name} keys must exactly match closure") - for name, digest in digests.items(): - if not isinstance(digest, str) or not _SHA256_RE.fullmatch(digest): - raise SkillContractError(f"Invalid {field_name} digest for {name!r}") - - -def _validate_contract(contract: SkillSessionContract) -> None: - if contract.schema_version != SKILL_SESSION_CONTRACT_SCHEMA_VERSION: - raise SkillContractError("Unsupported skill session contract schema") - if contract.launch_contract is not None: - if contract.launch_contract_digest != contract.launch_contract.digest: - raise SkillContractError("Skill session launch contract digest mismatch") - if contract.launch_contract.effective_backend != contract.backend: - raise SkillContractError("Skill session launch backend mismatch") - if contract.launch_contract.cwd != contract.cwd: - raise SkillContractError("Skill session launch cwd mismatch") - if not contract.root_name or not contract.closure: - raise SkillContractError("Skill session contract requires a root and closure") - closure = set(contract.closure) - if len(closure) != len(contract.closure) or contract.root_name not in closure: - raise SkillContractError("Skill session contract closure is invalid") - if set(contract.source_refs) != closure: - raise SkillContractError("source_refs keys must exactly match closure") - if set(contract.member_roles) != closure: - raise SkillContractError("member_roles keys must exactly match closure") - if set(contract.member_capabilities) != closure: - raise SkillContractError("member_capabilities keys must exactly match closure") - if set(contract.member_activate_deps) != closure: - raise SkillContractError("member_activate_deps keys must exactly match closure") - if set(contract.canonical_contents) != closure: - raise SkillContractError("canonical_contents keys must exactly match closure") - if set(contract.exploration_vectors) != closure: - raise SkillContractError("exploration_vectors keys must exactly match closure") - if set(contract.exploration_sidecar_digests) != closure: - raise SkillContractError("exploration_sidecar_digests keys must exactly match closure") - for name in contract.closure: - source_ref = contract.source_refs[name] - if not isinstance(source_ref, SkillSourceRef): - raise SkillContractError(f"source reference for {name!r} must be typed") - if source_ref.logical_name != name: - raise SkillContractError(f"source reference logical name mismatch for {name!r}") - role = contract.member_roles[name] - capabilities = contract.member_capabilities[name] - validate_skill_capability_roles(capabilities, role) - if role is not contract.execution_role: - raise SkillContractError(f"member role mismatch for {name!r}") - canonical_digest = hashlib.sha256(contract.canonical_contents[name].encode()).hexdigest() - if canonical_digest != contract.canonical_digests[name]: - raise SkillContractError(f"canonical content digest mismatch for {name!r}") - vectors = contract.exploration_vectors[name] - if any(not isinstance(vector, ExplorationVectorDef) for vector in vectors): - raise SkillContractError(f"exploration vectors for {name!r} must be typed") - vector_ids = tuple(vector.id for vector in vectors) - if len(vector_ids) != len(set(vector_ids)): - raise SkillContractError(f"exploration vector ids for {name!r} must be unique") - member_capability_union = frozenset().union( - *(contract.member_capabilities[name] for name in contract.closure) - ) - if member_capability_union != contract.capability_union: - raise SkillContractError("member capability union does not match contract") - _validate_digest_map("canonical_digests", contract.canonical_digests, closure) - _validate_digest_map("projected_digests", contract.projected_digests, closure) - if contract.projection_version != SKILL_PROJECTION_VERSION: - raise SkillContractError( - f"unsupported projection_version {contract.projection_version}; " - f"expected {SKILL_PROJECTION_VERSION}" - ) - if not contract.project_root or not contract.cwd: - raise SkillContractError("project_root and cwd are required") - if not contract.backend or not contract.resolved_command: - raise SkillContractError("backend and resolved_command are required") - validate_skill_capability_roles(contract.capability_union, contract.execution_role) - - -def _validate_relative_path(value: str) -> Path: - if not isinstance(value, str) or not value: - raise ValueError("Projected snapshot path must be a non-empty relative path") - relative = PurePosixPath(value) - if relative.is_absolute() or any(part in ("", ".", "..") for part in relative.parts): - raise ValueError(f"Unsafe projected snapshot path: {value!r}") - return Path(*relative.parts) - - -def _validate_snapshot_mapping( - contract: SkillSessionContract, - snapshot: Mapping[str, str], -) -> dict[str, str]: - if not isinstance(snapshot, Mapping): - raise ValueError("Projected snapshot must be a mapping") - by_skill: dict[str, str] = {} - seen_paths: set[Path] = set() - for raw_relative_path, content in snapshot.items(): - relative_path = _validate_relative_path(raw_relative_path) - if relative_path in seen_paths: - raise ValueError(f"Duplicate projected snapshot path: {raw_relative_path!r}") - if relative_path.name != "SKILL.md" or len(relative_path.parts) < 2: - raise ValueError( - f"Projected snapshot path must end in /SKILL.md: {raw_relative_path!r}" - ) - if not isinstance(content, str): - raise ValueError(f"Projected snapshot content must be text: {raw_relative_path!r}") - skill_name = relative_path.parent.name - if skill_name in by_skill: - raise ValueError(f"Multiple projected documents for skill {skill_name!r}") - digest = hashlib.sha256(content.encode()).hexdigest() - if contract.projected_digests.get(skill_name) != digest: - raise ValueError(f"Projected snapshot digest mismatch for {skill_name!r}") - by_skill[skill_name] = relative_path.as_posix() - seen_paths.add(relative_path) - if set(by_skill) != set(contract.closure): - raise ValueError("Projected snapshot documents must exactly match closure") - return by_skill - - -def _source_ref_to_dict(source_ref: SkillSourceRef) -> dict[str, Any]: - return { - "origin": source_ref.origin.value, - "logical_name": source_ref.logical_name, - "skill_path": str(source_ref.skill_path), - "search_dir": source_ref.search_dir, - "precedence": source_ref.precedence, - } - - -def _source_ref_from_dict(data: Mapping[str, Any]) -> SkillSourceRef: - try: - return SkillSourceRef( - origin=SkillSource(str(data["origin"])), - logical_name=str(data["logical_name"]), - skill_path=Path(str(data["skill_path"])), - search_dir=(str(data["search_dir"]) if data.get("search_dir") is not None else None), - precedence=(int(data["precedence"]) if data.get("precedence") is not None else None), - ) - except (KeyError, TypeError, ValueError) as exc: - raise ValueError("Invalid skill source reference") from exc - - -_SERIALIZED_EXPLORATION_VECTOR_KEYS = frozenset( - { - "id", - "disposition", - "rationale", - "applicability", - "role", - "profile", - "relationship_classes", - "task_id", - "frontier_item_id", - "depends_on", - "scope", - "body", - "digest", - } +# Codec helpers live in _skill_session_contract_codec.py; re-exported for +# existing callers using the canonical _skill_session_contract_store path +# (notably tests at tests/execution/test_skill_session_contract_store.py and +# tests/server/test_run_skill_resume.py that import _digest_json). +from autoskillit.execution.session._skill_session_contract_codec import ( # noqa: F401 + _build_manifest, + _contract_from_dict, + _contract_to_dict, + _delete_finalized_contract, + _digest_json, + _execution_identity_from_dict, + _exploration_vector_from_dict, + _exploration_vector_to_dict, + _finalized_contract_path, + _managed_lineage_ref_from_manifest, + _source_ref_from_dict, + _source_ref_to_dict, + _validate_contract, + _validate_digest_map, + _validate_raw_session_id, + _validate_relative_path, + _validate_snapshot_mapping, ) -_EXECUTION_IDENTITY_KEYS = frozenset(ExecutionIdentity.empty().to_dict()) -_CHILD_EXECUTION_IDENTITY_KEYS = frozenset( - { - "task_id", - "role", - "plan_digest", - "definition_digest", - "requested_backend", - "effective_backend", - "requested_model", - "effective_model", - "requested_effort", - "effective_effort", - "session_id", - } -) - - -def _exploration_vector_to_dict(vector: ExplorationVectorDef) -> dict[str, Any]: - task = vector.task - return { - "id": vector.id, - "disposition": vector.disposition.value, - "rationale": vector.rationale, - "applicability": vector.applicability.value, - "role": vector.role, - "profile": vector.profile.value, - "relationship_classes": [item.value for item in vector.relationship_classes], - "task_id": task.task_id, - "frontier_item_id": task.frontier_item_id, - "depends_on": list(task.depends_on), - "scope": list(task.scope), - "body": vector.body, - "digest": vector.digest, - } - - -def _exploration_vector_from_dict(value: object) -> ExplorationVectorDef: - if not isinstance(value, dict) or set(value) != _SERIALIZED_EXPLORATION_VECTOR_KEYS: - raise ValueError("serialized exploration vector keys are invalid") - for field_name in ("relationship_classes", "depends_on", "scope"): - field_value = value[field_name] - if not isinstance(field_value, list) or any( - not isinstance(item, str) for item in field_value - ): - raise ValueError(f"serialized exploration vector {field_name} is invalid") - for field_name in ( - "id", - "disposition", - "rationale", - "applicability", - "profile", - "task_id", - "frontier_item_id", - "body", - "digest", - ): - if not isinstance(value[field_name], str): - raise ValueError(f"serialized exploration vector {field_name} is invalid") - if value["role"] is not None and not isinstance(value["role"], str): - raise ValueError("serialized exploration vector role is invalid") - profile = RepositoryProfileId(value["profile"]) - vector = ExplorationVectorDef( - id=value["id"], - disposition=ExplorationVectorDisposition(value["disposition"]), - rationale=value["rationale"], - applicability=ExplorationVectorApplicabilityId(value["applicability"]), - role=value["role"], - profile=profile, - relationship_classes=tuple( - RelationshipKind(item) for item in value["relationship_classes"] - ), - task=ExplorationTaskSpec( - task_id=value["task_id"], - frontier_item_id=value["frontier_item_id"], - profile=profile, - depends_on=tuple(value["depends_on"]), - scope=tuple(value["scope"]), - ), - body=value["body"], - ) - if value["digest"] != vector.digest: - raise ValueError("serialized exploration vector digest mismatch") - return vector - - -def _execution_identity_from_dict(value: object) -> ExecutionIdentity: - if not isinstance(value, dict) or set(value) != _EXECUTION_IDENTITY_KEYS: - raise ValueError("serialized execution identity keys are invalid") - children = value.get("children") - if not isinstance(children, list): - raise ValueError("serialized execution identity children must be a list") - scalar_values = {key: item for key, item in value.items() if key != "children"} - if any(not isinstance(item, str) for item in scalar_values.values()): - raise ValueError("serialized execution identity values must be text") - parsed_children: list[ChildExecutionIdentity] = [] - for child in children: - if ( - not isinstance(child, dict) - or set(child) != _CHILD_EXECUTION_IDENTITY_KEYS - or any(not isinstance(item, str) for item in child.values()) - ): - raise ValueError("serialized child execution identity is invalid") - parsed_children.append(ChildExecutionIdentity(**child)) - return ExecutionIdentity(**scalar_values, children=tuple(parsed_children)) - - -def _contract_to_dict(contract: SkillSessionContract) -> dict[str, Any]: - return { - "schema_version": contract.schema_version, - "root_name": contract.root_name, - "execution_role": contract.execution_role.value, - "source_refs": { - name: _source_ref_to_dict(source_ref) - for name, source_ref in sorted(contract.source_refs.items()) - }, - "closure": list(contract.closure), - "capability_union": sorted(contract.capability_union), - "canonical_digests": dict(sorted(contract.canonical_digests.items())), - "projected_digests": dict(sorted(contract.projected_digests.items())), - "projection_version": contract.projection_version, - "project_root": contract.project_root, - "cwd": contract.cwd, - "backend": contract.backend, - "resolved_command": contract.resolved_command, - "member_roles": {name: role.value for name, role in sorted(contract.member_roles.items())}, - "member_capabilities": { - name: sorted(capabilities) - for name, capabilities in sorted(contract.member_capabilities.items()) - }, - "member_activate_deps": { - name: list(dependencies) - for name, dependencies in sorted(contract.member_activate_deps.items()) - }, - "canonical_contents": dict(sorted(contract.canonical_contents.items())), - "exploration_vectors": { - name: [_exploration_vector_to_dict(vector) for vector in vectors] - for name, vectors in sorted(contract.exploration_vectors.items()) - }, - "exploration_sidecar_digests": dict(sorted(contract.exploration_sidecar_digests.items())), - "resolved_exploration_profile": ( - contract.resolved_exploration_profile.value - if contract.resolved_exploration_profile is not None - else None - ), - "active_exploration_applicabilities": sorted( - item.value for item in contract.active_exploration_applicabilities - ), - "expected_output_patterns": list(contract.expected_output_patterns), - "write_behavior": { - "mode": contract.write_behavior.mode, - "expected_when": list(contract.write_behavior.expected_when), - }, - "read_only": contract.read_only, - "scope_discipline": contract.scope_discipline, - "parent_sandbox_mode": contract.parent_sandbox_mode, - "completion_required": contract.completion_required, - "skill_contract_json": contract.skill_contract_json, - "projection_substitutions": [list(item) for item in contract.projection_substitutions], - "projection_gating": contract.projection_gating, - "projection_namespace": contract.projection_namespace, - "launch_contract": ( - json.loads(contract.launch_contract.canonical_json) - if contract.launch_contract is not None - else None - ), - "launch_contract_digest": contract.launch_contract_digest, - "execution_identity": contract.execution_identity.to_dict(), - } - - -def _contract_from_dict(data: Mapping[str, Any]) -> SkillSessionContract: - try: - source_refs_raw = data["source_refs"] - if not isinstance(source_refs_raw, dict): - raise ValueError("source_refs must be an object") - read_only = data.get("read_only", False) - if not isinstance(read_only, bool): - raise ValueError("read_only must be a boolean") - scope_discipline = data.get("scope_discipline", False) - if not isinstance(scope_discipline, bool): - raise ValueError("scope_discipline must be a boolean") - parent_sandbox_mode = data["parent_sandbox_mode"] - if not isinstance(parent_sandbox_mode, str): - raise ValueError("parent_sandbox_mode must be text") - completion_required = data.get("completion_required", False) - if not isinstance(completion_required, bool): - raise ValueError("completion_required must be a boolean") - projection_gating = data.get("projection_gating") - if projection_gating is not None and not isinstance(projection_gating, bool): - raise ValueError("projection_gating must be a boolean or null") - projection_substitutions = data.get("projection_substitutions", []) - if not isinstance(projection_substitutions, list) or any( - not isinstance(item, list) or len(item) != 2 for item in projection_substitutions - ): - raise ValueError("projection_substitutions entries must be two-element lists") - launch_payload = data.get("launch_contract") - launch_digest = data.get("launch_contract_digest", "") - if launch_payload is not None and not isinstance(launch_payload, dict): - raise ValueError("launch_contract must be an object or null") - if not isinstance(launch_digest, str): - raise ValueError("launch_contract_digest must be a string") - launch_contract = ( - ResolvedLaunchContract.from_payload( - launch_payload, - expected_digest=launch_digest, - ) - if launch_payload is not None - else None - ) - exploration_vectors_raw = data["exploration_vectors"] - if not isinstance(exploration_vectors_raw, dict) or any( - not isinstance(vectors, list) for vectors in exploration_vectors_raw.values() - ): - raise ValueError("exploration_vectors must be an object of lists") - resolved_exploration_profile_raw = data["resolved_exploration_profile"] - if resolved_exploration_profile_raw is not None and not isinstance( - resolved_exploration_profile_raw, str - ): - raise ValueError("resolved_exploration_profile must be text or null") - active_applicabilities_raw = data["active_exploration_applicabilities"] - if not isinstance(active_applicabilities_raw, list) or any( - not isinstance(item, str) for item in active_applicabilities_raw - ): - raise ValueError("active_exploration_applicabilities must be a list of text") - return SkillSessionContract( - root_name=str(data["root_name"]), - execution_role=SkillExecutionRole(str(data["execution_role"])), - source_refs={ - str(name): _source_ref_from_dict(source_ref) - for name, source_ref in source_refs_raw.items() - if isinstance(source_ref, dict) - }, - closure=tuple(str(name) for name in data["closure"]), - capability_union=frozenset(str(cap) for cap in data["capability_union"]), - canonical_digests={ - str(name): str(digest) for name, digest in data["canonical_digests"].items() - }, - projected_digests={ - str(name): str(digest) for name, digest in data["projected_digests"].items() - }, - projection_version=int(data["projection_version"]), - project_root=str(data["project_root"]), - cwd=str(data["cwd"]), - backend=str(data["backend"]), - resolved_command=str(data["resolved_command"]), - member_roles={ - str(name): SkillExecutionRole(str(role)) - for name, role in data["member_roles"].items() - }, - member_capabilities={ - str(name): frozenset(str(capability) for capability in capabilities) - for name, capabilities in data["member_capabilities"].items() - }, - member_activate_deps={ - str(name): tuple(str(dependency) for dependency in dependencies) - for name, dependencies in data["member_activate_deps"].items() - }, - canonical_contents={ - str(name): str(content) for name, content in data["canonical_contents"].items() - }, - exploration_vectors={ - str(name): tuple(_exploration_vector_from_dict(vector) for vector in vectors) - for name, vectors in exploration_vectors_raw.items() - }, - exploration_sidecar_digests={ - str(name): str(digest) - for name, digest in data.get("exploration_sidecar_digests", {}).items() - }, - resolved_exploration_profile=( - RepositoryProfileId(resolved_exploration_profile_raw) - if resolved_exploration_profile_raw is not None - else None - ), - active_exploration_applicabilities=frozenset( - ExplorationVectorApplicabilityId(item) for item in active_applicabilities_raw - ), - expected_output_patterns=tuple( - str(pattern) for pattern in data.get("expected_output_patterns", []) - ), - write_behavior=WriteBehaviorSpec( - mode=( - str(data.get("write_behavior", {}).get("mode")) - if data.get("write_behavior", {}).get("mode") is not None - else None - ), - expected_when=tuple( - str(pattern) - for pattern in data.get("write_behavior", {}).get("expected_when", []) - ), - ), - read_only=read_only, - scope_discipline=scope_discipline, - parent_sandbox_mode=parent_sandbox_mode, - completion_required=completion_required, - skill_contract_json=str(data.get("skill_contract_json", "")), - projection_substitutions=tuple( - (str(item[0]), str(item[1])) for item in projection_substitutions - ), - projection_gating=projection_gating, - projection_namespace=( - str(data["projection_namespace"]) - if data.get("projection_namespace") is not None - else None - ), - launch_contract=launch_contract, - launch_contract_digest=launch_digest, - execution_identity=_execution_identity_from_dict(data["execution_identity"]), - schema_version=int(data["schema_version"]), - ) - except (KeyError, TypeError, ValueError, AttributeError, SkillContractError) as exc: - raise ValueError("Invalid serialized skill session contract") from exc - -def _digest_json(value: Mapping[str, Any]) -> str: - encoded = json.dumps( - dict(value), - sort_keys=True, - separators=(",", ":"), - ).encode() - return hashlib.sha256(encoded).hexdigest() - - -def _build_manifest( - *, - contract: SkillSessionContract, - raw_session_id: str | None, - candidate_session_ids: tuple[str, ...], - snapshot_paths: Mapping[str, str], - managed_lineage_ref: ManagedHeadlessSessionLineageRef | None, -) -> dict[str, Any]: - contract_data = _contract_to_dict(contract) - return { - "schema_version": _STORE_MANIFEST_SCHEMA_VERSION, - "raw_session_id": raw_session_id, - "candidate_session_ids": list(candidate_session_ids), - "managed_lineage_ref": ( - managed_lineage_ref.to_dict() if managed_lineage_ref is not None else None - ), - "contract": contract_data, - "contract_digest": _digest_json(contract_data), - "snapshot_paths": dict(sorted(snapshot_paths.items())), - } - - -def _managed_lineage_ref_from_manifest( - manifest: Mapping[str, Any], -) -> ManagedHeadlessSessionLineageRef | None: - if "managed_lineage_ref" not in manifest: - raise ValueError("Skill session manifest is missing managed lineage reference") - value = manifest.get("managed_lineage_ref") - if value is None: - return None - try: - return ManagedHeadlessSessionLineageRef.from_dict(value) - except (TypeError, ValueError) as exc: - raise ValueError("Invalid skill session managed lineage reference") from exc From 4532cd966747c39511a8e62f162168b7b7bb6663 Mon Sep 17 00:00:00 2001 From: Trecek Date: Mon, 17 Aug 2026 12:58:38 -0700 Subject: [PATCH 11/11] style: apply pre-commit auto-fixes and add missing imports Final pass to make pre-commit (ruff, mypy, contract checks) pass on all decomposed files. Adds missing imports for _skill_session_contract_codec extracted types (SkillSource, RepositoryProfileId, ChildExecutionIdentity, etc.) and re-adds _validate_codex_mcp_inventory to codex.py re-export block after linter removed it. Refs: #4664 --- src/autoskillit/core/types/_type_constants.py | 4 +- .../execution/backends/_claude_parse.py | 4 +- .../backends/_claude_session_locator.py | 3 +- .../execution/backends/_codex_fs_atomic.py | 2 +- src/autoskillit/execution/backends/claude.py | 2 +- src/autoskillit/execution/backends/codex.py | 4 +- .../execution/github_review/_ledger_schema.py | 2 +- .../github_review/_poster_finalize.py | 6 +-- .../execution/github_review/ledger.py | 12 ++++-- .../execution/github_review/poster.py | 2 - .../execution/headless/_headless_result.py | 15 +------ src/autoskillit/execution/process/__init__.py | 1 - .../execution/process/_termination.py | 4 +- .../_managed_headless_session_lineage.py | 42 ++++--------------- ...anaged_headless_session_lineage_indexes.py | 25 +++++++++-- ...anaged_headless_session_lineage_records.py | 13 +++++- ...managed_headless_session_lineage_runner.py | 3 +- .../session/_skill_session_contract_codec.py | 20 +++++++-- .../session/_skill_session_contract_store.py | 17 -------- 19 files changed, 83 insertions(+), 98 deletions(-) diff --git a/src/autoskillit/core/types/_type_constants.py b/src/autoskillit/core/types/_type_constants.py index b5a98a84d..910f693ab 100644 --- a/src/autoskillit/core/types/_type_constants.py +++ b/src/autoskillit/core/types/_type_constants.py @@ -558,9 +558,7 @@ def _validate_durable_artifact_writer_defs( detection="autoskillit.execution.backends._codex_hooks:find_broken_codex_hook_commands", ), DurableArtifactWriterDef( - writer=( - "autoskillit.execution.backends._codex_fs_atomic:_write_reconciliation_audit" - ), + writer=("autoskillit.execution.backends._codex_fs_atomic:_write_reconciliation_audit"), artifact=( "immutable operator authorization records for explicit Codex attempt-view " "reconciliation" diff --git a/src/autoskillit/execution/backends/_claude_parse.py b/src/autoskillit/execution/backends/_claude_parse.py index 1087e11d9..a71c50f3e 100644 --- a/src/autoskillit/execution/backends/_claude_parse.py +++ b/src/autoskillit/execution/backends/_claude_parse.py @@ -13,10 +13,10 @@ from autoskillit.core import ( AGENT_BACKEND_CLAUDE_CODE, + CONTEXT_EXHAUSTION_MARKER, AgentSessionResult, BackendEventKind, ClaudeEventData, - CONTEXT_EXHAUSTION_MARKER, SessionEvent, fast_loads, ) @@ -229,4 +229,4 @@ def parse_stdout(self, stdout: str, *, exit_code: int = 0) -> AgentSessionResult ) -__all__ = ["ClaudeResultParser", "ClaudeStreamParser"] \ No newline at end of file +__all__ = ["ClaudeResultParser", "ClaudeStreamParser"] diff --git a/src/autoskillit/execution/backends/_claude_session_locator.py b/src/autoskillit/execution/backends/_claude_session_locator.py index c75a97b4f..43a30e8c9 100644 --- a/src/autoskillit/execution/backends/_claude_session_locator.py +++ b/src/autoskillit/execution/backends/_claude_session_locator.py @@ -22,7 +22,6 @@ read_registry, ) - _ORDER_GREETING_PREFIXES = ( "Today's special:", "Order up! Today's special:", @@ -115,4 +114,4 @@ def list_sessions(self, cwd: str) -> tuple[SessionSummary, ...]: return tuple(summaries) -__all__ = ["ClaudeSessionLocator"] \ No newline at end of file +__all__ = ["ClaudeSessionLocator"] diff --git a/src/autoskillit/execution/backends/_codex_fs_atomic.py b/src/autoskillit/execution/backends/_codex_fs_atomic.py index eab3b9a5a..5f3ba4579 100644 --- a/src/autoskillit/execution/backends/_codex_fs_atomic.py +++ b/src/autoskillit/execution/backends/_codex_fs_atomic.py @@ -223,4 +223,4 @@ def _replace_symlink(path: Path, target: Path) -> None: pass temporary.symlink_to(target) os.replace(temporary, path) - _fsync_directory(path.parent) \ No newline at end of file + _fsync_directory(path.parent) diff --git a/src/autoskillit/execution/backends/claude.py b/src/autoskillit/execution/backends/claude.py index d125671bf..8e08dbb26 100644 --- a/src/autoskillit/execution/backends/claude.py +++ b/src/autoskillit/execution/backends/claude.py @@ -929,4 +929,4 @@ def build_inspector_cmd(self, prompt: str, *, model: str = "") -> CmdSpec: def _ignore_child_identity(pid: int, pgid: int) -> None: - del pid, pgid \ No newline at end of file + del pid, pgid diff --git a/src/autoskillit/execution/backends/codex.py b/src/autoskillit/execution/backends/codex.py index da353da56..c0deced01 100644 --- a/src/autoskillit/execution/backends/codex.py +++ b/src/autoskillit/execution/backends/codex.py @@ -133,8 +133,8 @@ # Re-export probe helpers so existing consumers (e.g. evidence_reader, tests) # can keep importing them from the canonical codex module path. -from autoskillit.execution.backends._codex_probes import ( - _BoundedProbeResult, # noqa: F401 +from autoskillit.execution.backends._codex_probes import ( # noqa: F401 + _BoundedProbeResult, _validate_codex_mcp_inventory, _validate_global_codex_home, _validate_inert_rollout_paths, diff --git a/src/autoskillit/execution/github_review/_ledger_schema.py b/src/autoskillit/execution/github_review/_ledger_schema.py index 97fbc0a38..df905b04b 100644 --- a/src/autoskillit/execution/github_review/_ledger_schema.py +++ b/src/autoskillit/execution/github_review/_ledger_schema.py @@ -125,4 +125,4 @@ class MutationSlot: "_DATABASE_MODE", "_SCHEMA", "_SCHEMA_VERSION", -] \ No newline at end of file +] diff --git a/src/autoskillit/execution/github_review/_poster_finalize.py b/src/autoskillit/execution/github_review/_poster_finalize.py index 05a43a164..5b2188116 100644 --- a/src/autoskillit/execution/github_review/_poster_finalize.py +++ b/src/autoskillit/execution/github_review/_poster_finalize.py @@ -191,9 +191,7 @@ def finalize( response_class=response_class, review_id=reconciliation.review_id, comment_ids=tuple( - item.remote_comment_id - for item in dispositions - if item.remote_comment_id is not None + item.remote_comment_id for item in dispositions if item.remote_comment_id is not None ), canonical_finding_count=len(all_findings), reconciliation_result=reconciliation.result, @@ -221,4 +219,4 @@ def finalize( ) -__all__ = ["reconcile_payload", "finalize"] \ No newline at end of file +__all__ = ["reconcile_payload", "finalize"] diff --git a/src/autoskillit/execution/github_review/ledger.py b/src/autoskillit/execution/github_review/ledger.py index f35b4706b..3744a1307 100644 --- a/src/autoskillit/execution/github_review/ledger.py +++ b/src/autoskillit/execution/github_review/ledger.py @@ -14,8 +14,10 @@ from typing import Any from autoskillit.core import ( + GitHubReviewFindingDisposition, GitHubReviewReceipt, ReviewOperationState, + ReviewResponseClass, fsync_directory, private_file_identity, private_sidecar_issue, @@ -24,16 +26,18 @@ review_receipt_validation_error, unlink_sqlite_initialization_artifacts, ) + from ._ledger_schema import ( # noqa: F401 — re-exported for sibling modules - MutationSlot, - ReviewAttemptRecord, - ReviewOperationRecord, - _DIRECTORY_MODE, _DATABASE_MODE, + _DIRECTORY_MODE, _SCHEMA, _SCHEMA_VERSION, + MutationSlot, + ReviewAttemptRecord, + ReviewOperationRecord, ) + class GitHubReviewLedger: """Durable operation, attempt, receipt, and cross-process pacing authority.""" diff --git a/src/autoskillit/execution/github_review/poster.py b/src/autoskillit/execution/github_review/poster.py index 402d31fca..b503172d6 100644 --- a/src/autoskillit/execution/github_review/poster.py +++ b/src/autoskillit/execution/github_review/poster.py @@ -14,7 +14,6 @@ from autoskillit.core import ( GitHubReviewFindingDisposition, GitHubReviewPostResult, - GitHubReviewReceipt, GitHubReviewRequest, ReviewFindingDispositionKind, ReviewOperationState, @@ -755,7 +754,6 @@ async def _reconcile_existing( executed_mutations=0, ) - async def _reconcile_payload( self, *, diff --git a/src/autoskillit/execution/headless/_headless_result.py b/src/autoskillit/execution/headless/_headless_result.py index ad7fcf4c6..38131cc3b 100644 --- a/src/autoskillit/execution/headless/_headless_result.py +++ b/src/autoskillit/execution/headless/_headless_result.py @@ -3,15 +3,11 @@ from __future__ import annotations import dataclasses -import errno -import stat from collections.abc import Sequence from pathlib import Path -from typing import TYPE_CHECKING, cast +from typing import TYPE_CHECKING from autoskillit.core import ( - AGENT_BACKEND_CLAUDE_CODE, - ApiRetryOutcome, ChannelConfirmation, CliSubtype, ClosureAuthoritySpec, @@ -25,28 +21,21 @@ SkillResult, TerminationReason, WriteBehaviorSpec, - WriteEvidence, extract_skill_name, get_logger, validate_worktree_path, ) from autoskillit.execution.headless._headless_evidence import ( - _adapt_agent_result, _apply_budget_guard, _capture_failure, _compute_write_evidence, _extract_file_changes, _stdout_mentions_write_tools, ) -from autoskillit.execution.headless._headless_outcome import ( - evaluate_outcome_invariants, - parse_outcome_fields, -) from autoskillit.execution.headless._headless_path_tokens import ( _extract_branch_name, _extract_output_paths, _extract_worktree_path, - _is_path_outside_cwd, _normalize_messages, _select_output_path_tokens, _validate_output_paths, @@ -69,7 +58,6 @@ from autoskillit.execution.session._session_content import _check_expected_patterns from autoskillit.execution.session._session_model import ( ClaudeSessionResult, - parse_session_result, ) from autoskillit.execution.session._session_outcome import ( _compute_outcome, @@ -106,7 +94,6 @@ ) - def _build_skill_result( result: SubprocessResult, completion_marker: str = "", diff --git a/src/autoskillit/execution/process/__init__.py b/src/autoskillit/execution/process/__init__.py index cd31369aa..73f6d609e 100644 --- a/src/autoskillit/execution/process/__init__.py +++ b/src/autoskillit/execution/process/__init__.py @@ -174,7 +174,6 @@ def _normalize_pass_fds(pass_fds: tuple[int, ...]) -> tuple[int, ...]: ) - async def run_managed_async( cmd: list[str], *, diff --git a/src/autoskillit/execution/process/_termination.py b/src/autoskillit/execution/process/_termination.py index 32926883b..65519d867 100644 --- a/src/autoskillit/execution/process/_termination.py +++ b/src/autoskillit/execution/process/_termination.py @@ -15,6 +15,7 @@ from __future__ import annotations from pathlib import Path +from typing import assert_never import anyio import structlog @@ -34,7 +35,6 @@ _has_active_child_processes, _has_active_execution_marker, ) -from typing_extensions import assert_never def decide_termination_action( @@ -167,4 +167,4 @@ async def execute_termination_action( return kill_reason, returncode, cleanup -__all__ = ["decide_termination_action", "execute_termination_action"] \ No newline at end of file +__all__ = ["decide_termination_action", "execute_termination_action"] diff --git a/src/autoskillit/execution/session/_managed_headless_session_lineage.py b/src/autoskillit/execution/session/_managed_headless_session_lineage.py index 42b13ec7c..60f7adcd6 100644 --- a/src/autoskillit/execution/session/_managed_headless_session_lineage.py +++ b/src/autoskillit/execution/session/_managed_headless_session_lineage.py @@ -2,46 +2,21 @@ from __future__ import annotations -import fcntl -import hashlib -import os -import stat -from collections.abc import Callable, Iterator -from contextlib import contextmanager +from collections.abc import Callable from dataclasses import replace from pathlib import Path from autoskillit.core import ( - MANAGED_HEADLESS_SESSION_LINEAGE_SCHEMA_VERSION, ManagedHeadlessSessionKind, ManagedHeadlessSessionLineage, ManagedHeadlessSessionLineageRef, ManagedHeadlessSessionTerminalState, NativeShellCaptureDecision, NativeShellCaptureObservation, - atomic_write, -) -from autoskillit.execution.session._managed_headless_session_lineage_codec import ( - _strict_str, ) from autoskillit.execution.session._managed_headless_session_lineage_codec import ( canonical_json as _canonical_json, ) -from autoskillit.execution.session._managed_headless_session_lineage_codec import ( - digest as _digest, -) -from autoskillit.execution.session._managed_headless_session_lineage_codec import ( - lineage_from_dict as _lineage_from_dict, -) -from autoskillit.execution.session._managed_headless_session_lineage_codec import ( - record_payload as _record_payload, -) -from autoskillit.execution.session._managed_headless_session_lineage_codec import ( - record_to_dict as _record_to_dict, -) -from autoskillit.execution.session._managed_headless_session_lineage_codec import ( - strict_json_load as _strict_json_load, -) __all__ = [ "DefaultManagedHeadlessSessionLineageStore", @@ -551,6 +526,13 @@ def _mutate( # Index/runner/records helpers live in sibling modules; re-exported for # existing callers using the canonical _managed_headless_session_lineage path. +from autoskillit.execution.session._managed_headless_session_lineage_indexes import ( # noqa: F401 + _assert_index_available, + _index_path, + _read_index, + _remove_index, + _write_index, +) from autoskillit.execution.session._managed_headless_session_lineage_records import ( # noqa: F401 _creation_projection, _new_lineage, @@ -566,13 +548,7 @@ def _mutate( _read_runner_markers, _settle_runner_observation, ) -from autoskillit.execution.session._managed_headless_session_lineage_indexes import ( # noqa: F401 - _assert_index_available, - _index_path, - _read_index, - _remove_index, - _write_index, -) + def _validate_anchor_identity( lineage: ManagedHeadlessSessionLineage, diff --git a/src/autoskillit/execution/session/_managed_headless_session_lineage_indexes.py b/src/autoskillit/execution/session/_managed_headless_session_lineage_indexes.py index 6d2f21835..1b953aba8 100644 --- a/src/autoskillit/execution/session/_managed_headless_session_lineage_indexes.py +++ b/src/autoskillit/execution/session/_managed_headless_session_lineage_indexes.py @@ -5,9 +5,30 @@ from __future__ import annotations +import hashlib +import os from pathlib import Path -from autoskillit.execution.session._managed_headless_session_lineage_codec import _strict_str +from autoskillit.core import ( + MANAGED_HEADLESS_SESSION_LINEAGE_SCHEMA_VERSION, + atomic_write, +) +from autoskillit.execution.session._managed_headless_session_lineage import ( + _INDEXES_DIR, + ManagedHeadlessSessionLineageConflictError, +) +from autoskillit.execution.session._managed_headless_session_lineage_codec import ( + _strict_str, +) +from autoskillit.execution.session._managed_headless_session_lineage_codec import ( + canonical_json as _canonical_json, +) +from autoskillit.execution.session._managed_headless_session_lineage_codec import ( + strict_json_load as _strict_json_load, +) +from autoskillit.execution.session._managed_headless_session_lineage_records import ( + _read_bounded, +) def _index_path(root: Path, index_name: str, key: str) -> Path: @@ -72,5 +93,3 @@ def _assert_index_available( raise ManagedHeadlessSessionLineageConflictError( f"Managed lineage {index_name} identity is already owned" ) - - diff --git a/src/autoskillit/execution/session/_managed_headless_session_lineage_records.py b/src/autoskillit/execution/session/_managed_headless_session_lineage_records.py index 203b323d1..4bcdf9c10 100644 --- a/src/autoskillit/execution/session/_managed_headless_session_lineage_records.py +++ b/src/autoskillit/execution/session/_managed_headless_session_lineage_records.py @@ -18,6 +18,7 @@ from autoskillit.core import ( MANAGED_HEADLESS_SESSION_LINEAGE_SCHEMA_VERSION, ManagedHeadlessSessionKind, + ManagedHeadlessSessionLineage, NativeShellCaptureDecision, atomic_write, ) @@ -32,13 +33,22 @@ ) from autoskillit.execution.session._managed_headless_session_lineage_codec import ( canonical_json as _canonical_json, +) +from autoskillit.execution.session._managed_headless_session_lineage_codec import ( digest as _digest, +) +from autoskillit.execution.session._managed_headless_session_lineage_codec import ( lineage_from_dict as _lineage_from_dict, +) +from autoskillit.execution.session._managed_headless_session_lineage_codec import ( record_payload as _record_payload, +) +from autoskillit.execution.session._managed_headless_session_lineage_codec import ( record_to_dict as _record_to_dict, +) +from autoskillit.execution.session._managed_headless_session_lineage_codec import ( strict_json_load as _strict_json_load, ) -from autoskillit.core import ManagedHeadlessSessionLineage def _new_lineage( @@ -189,4 +199,3 @@ def _read_bounded(path: Path) -> bytes: if len(raw) > _MAX_RECORD_BYTES: raise ValueError("Managed lineage artifact is oversized") return raw - diff --git a/src/autoskillit/execution/session/_managed_headless_session_lineage_runner.py b/src/autoskillit/execution/session/_managed_headless_session_lineage_runner.py index 43d292926..097ee5d5b 100644 --- a/src/autoskillit/execution/session/_managed_headless_session_lineage_runner.py +++ b/src/autoskillit/execution/session/_managed_headless_session_lineage_runner.py @@ -23,6 +23,8 @@ ) from autoskillit.execution.session._managed_headless_session_lineage_codec import ( canonical_json as _canonical_json, +) +from autoskillit.execution.session._managed_headless_session_lineage_codec import ( strict_json_load as _strict_json_load, ) @@ -147,4 +149,3 @@ def _settle_runner_observation( if observations_fd >= 0: os.close(observations_fd) os.close(root_fd) - diff --git a/src/autoskillit/execution/session/_skill_session_contract_codec.py b/src/autoskillit/execution/session/_skill_session_contract_codec.py index 278eb325f..1e0b6d938 100644 --- a/src/autoskillit/execution/session/_skill_session_contract_codec.py +++ b/src/autoskillit/execution/session/_skill_session_contract_codec.py @@ -13,21 +13,35 @@ import hashlib import json +import shutil from collections.abc import Mapping -from pathlib import Path +from pathlib import Path, PurePosixPath from typing import Any from autoskillit.core import ( + SKILL_PROJECTION_VERSION, + SKILL_SESSION_CONTRACT_SCHEMA_VERSION, + ChildExecutionIdentity, ExecutionIdentity, + ExplorationTaskSpec, + ExplorationVectorApplicabilityId, ExplorationVectorDef, + ExplorationVectorDisposition, ManagedHeadlessSessionLineageRef, + RelationshipKind, + RepositoryProfileId, + ResolvedLaunchContract, + SkillContractError, + SkillExecutionRole, SkillSessionContract, + SkillSource, SkillSourceRef, - atomic_write, + WriteBehaviorSpec, + validate_skill_capability_roles, ) from autoskillit.execution.session._skill_session_contract_store import ( + _SHA256_RE, _STORE_MANIFEST_SCHEMA_VERSION, - _MANIFEST_FILENAME, ) diff --git a/src/autoskillit/execution/session/_skill_session_contract_store.py b/src/autoskillit/execution/session/_skill_session_contract_store.py index f681b5c6f..f2694fc4c 100644 --- a/src/autoskillit/execution/session/_skill_session_contract_store.py +++ b/src/autoskillit/execution/session/_skill_session_contract_store.py @@ -3,7 +3,6 @@ from __future__ import annotations import hashlib -import json import os import secrets import shutil @@ -18,28 +17,13 @@ from autoskillit.core import ( SKILL_PROJECTION_VERSION, - SKILL_SESSION_CONTRACT_SCHEMA_VERSION, - ChildExecutionIdentity, - ExecutionIdentity, - ExplorationTaskSpec, - ExplorationVectorApplicabilityId, - ExplorationVectorDef, - ExplorationVectorDisposition, ManagedHeadlessSessionLineageRef, - RelationshipKind, - RepositoryProfileId, ResolvedLaunchContract, - SkillContractError, - SkillExecutionRole, SkillSessionContract, - SkillSource, - SkillSourceRef, StoredSkillSessionContract, - WriteBehaviorSpec, atomic_write, default_log_dir, read_versioned_json, - validate_skill_capability_roles, write_versioned_json, ) @@ -390,4 +374,3 @@ def delete_skill_session_contracts( _validate_relative_path, _validate_snapshot_mapping, ) -