diff --git a/src/autoskillit/execution/backends/_backend_cmd_builder_base.py b/src/autoskillit/execution/backends/_backend_cmd_builder_base.py index 5ebfb54b8..ea4f059f6 100644 --- a/src/autoskillit/execution/backends/_backend_cmd_builder_base.py +++ b/src/autoskillit/execution/backends/_backend_cmd_builder_base.py @@ -20,6 +20,8 @@ from typing import TYPE_CHECKING, Any, NamedTuple from autoskillit.core import ( + AUTOSKILLIT_APPLICABLE_GUARDS, + AUTOSKILLIT_WRITE_GUARD_TOOL_NAMES, CAMPAIGN_ID_ENV_VAR, KITCHEN_SESSION_ID_ENV_VAR, SkillSessionConfig, @@ -88,22 +90,34 @@ def _flag_vocabulary(self) -> FlagVocabulary: @staticmethod def _assemble_shared_env_extras( *, + session_type: str = "", + applicable_guards: frozenset[str] = frozenset(), + write_guard_tool_names: frozenset[str] = frozenset(), write_prefix: str = "", write_prefixes: tuple[str, ...] = (), cwd: str = "", scenario_step_name: str = "", ) -> dict[str, str]: - """Assemble the eight shared env keys consumed by both backends. + """Assemble the shared env keys consumed by both backends. - Always-on keys (two): ``MAX_MCP_OUTPUT_TOKENS``, ``MCP_CONNECTION_NONBLOCKING``. + Always-on keys (three): ``MAX_MCP_OUTPUT_TOKENS``, ``MCP_CONNECTION_NONBLOCKING``, + ``AUTOSKILLIT_HEADLESS``. - Conditional keys (six): ``SCENARIO_STEP_NAME``, - ``CAMPAIGN_ID_ENV_VAR``, ``KITCHEN_SESSION_ID_ENV_VAR``, + Conditional keys (nine): ``AUTOSKILLIT_SESSION_TYPE``, + ``AUTOSKILLIT_APPLICABLE_GUARDS``, ``AUTOSKILLIT_WRITE_GUARD_TOOL_NAMES``, + ``SCENARIO_STEP_NAME``, ``CAMPAIGN_ID_ENV_VAR``, ``KITCHEN_SESSION_ID_ENV_VAR``, ``AUTOSKILLIT_ALLOWED_WRITE_PREFIX``, ``AUTOSKILLIT_ALLOWED_WRITE_PREFIXES``, ``AUTOSKILLIT_CWD``. Each is included only when its input is non-empty (campaign/kitchen IDs are also read from the ambient ``os.environ``). """ extras: dict[str, str] = dict(SHARED_BASELINE_ENV) + extras["AUTOSKILLIT_HEADLESS"] = "1" + if session_type: + extras["AUTOSKILLIT_SESSION_TYPE"] = session_type + if applicable_guards: + extras[AUTOSKILLIT_APPLICABLE_GUARDS] = ",".join(sorted(applicable_guards)) + if write_guard_tool_names: + extras[AUTOSKILLIT_WRITE_GUARD_TOOL_NAMES] = ",".join(sorted(write_guard_tool_names)) if scenario_step_name: extras["SCENARIO_STEP_NAME"] = scenario_step_name campaign_id = os.environ.get(CAMPAIGN_ID_ENV_VAR) diff --git a/src/autoskillit/execution/backends/_claude_prompt.py b/src/autoskillit/execution/backends/_claude_prompt.py index 50e011ba3..92074bc88 100644 --- a/src/autoskillit/execution/backends/_claude_prompt.py +++ b/src/autoskillit/execution/backends/_claude_prompt.py @@ -51,7 +51,7 @@ def _extract_write_artifacts(tool_uses: list[dict[str, Any]]) -> list[str]: } ) -# Variables that _build_skill_session_cmd_impl controls exclusively. They must not +# Variables that build_skill_session_cmd controls exclusively. They must not # leak from the host process environment — the caller opts in via explicit # parameters (exit_after_stop_delay_ms, scenario_step_name, allowed_write_prefix, etc.). # Note: CLAUDE_CODE_EXIT_AFTER_STOP_DELAY, SCENARIO_STEP_NAME, and diff --git a/src/autoskillit/execution/backends/claude.py b/src/autoskillit/execution/backends/claude.py index 7aa4a04dd..7bdc23edd 100644 --- a/src/autoskillit/execution/backends/claude.py +++ b/src/autoskillit/execution/backends/claude.py @@ -13,15 +13,15 @@ AGENT_BACKEND_CLAUDE_CODE, AGENT_BACKEND_DYNACONF_ENV_VAR, AGENT_BACKEND_ENV_VAR, - AUTOSKILLIT_APPLICABLE_GUARDS, - AUTOSKILLIT_WRITE_GUARD_TOOL_NAMES, CAMPAIGN_ID_ENV_VAR, CLAUDE_CODE_CAPABILITIES, CONTEXT_EXHAUSTION_MARKER, + NON_VARIADIC_CLAUDE_FLAGS, ORCHESTRATOR_SESSION_REQUIRED_ENV, SESSION_TYPE_ORCHESTRATOR, SESSION_TYPE_SKILL, SKILL_SESSION_REQUIRED_ENV, + VARIADIC_CLAUDE_FLAGS, AgentSessionResult, BackendCapabilities, BackendConventions, @@ -285,17 +285,8 @@ def _env_policy(self) -> ClaudeEnvPolicy: def _flag_vocabulary(self) -> FlagVocabulary: return FlagVocabulary( - variadic_flags=frozenset( - {ClaudeFlags.ADD_DIR, ClaudeFlags.PLUGIN_DIR, ClaudeFlags.TOOLS} - ), - non_variadic_flags=frozenset( - { - ClaudeFlags.PRINT, - ClaudeFlags.MODEL, - ClaudeFlags.RESUME, - ClaudeFlags.DANGEROUSLY_SKIP_PERMISSIONS, - } - ), + variadic_flags=VARIADIC_CLAUDE_FLAGS, + non_variadic_flags=NON_VARIADIC_CLAUDE_FLAGS, model_flag=ClaudeFlags.MODEL, add_dir_flag=ClaudeFlags.ADD_DIR, resume_flag=ClaudeFlags.RESUME, @@ -534,71 +525,25 @@ def build_skill_session_cmd( resume_message: str | None = None, ) -> CmdSpec: if config is not None: - return self._build_skill_session_cmd_impl( - skill_command, - cwd=cwd, - completion_marker=config.completion_marker, - model=config.model, - plugin_source=config.plugin_source, - output_format=config.output_format, - add_dirs=config.add_dirs, - exit_after_stop_delay_ms=config.exit_after_stop_delay_ms, - stream_idle_timeout_ms=config.stream_idle_timeout_ms, - scenario_step_name=config.scenario_step_name, - temp_dir_relpath=config.temp_dir_relpath, - allowed_write_prefix=config.allowed_write_prefix, - allowed_write_prefixes=config.allowed_write_prefixes, - provider_extras=config.provider_extras, - profile_name=config.profile_name, - resume_session_id=config.resume_session_id, - resume_checkpoint=config.resume_checkpoint, - resume_message=config.resume_message, - sandbox_mode=config.sandbox_mode, - ) - return self._build_skill_session_cmd_impl( - skill_command, - cwd=cwd, - completion_marker=completion_marker, - model=model, - plugin_source=plugin_source, - output_format=output_format, - add_dirs=add_dirs, - exit_after_stop_delay_ms=exit_after_stop_delay_ms, - stream_idle_timeout_ms=stream_idle_timeout_ms, - scenario_step_name=scenario_step_name, - temp_dir_relpath=temp_dir_relpath, - allowed_write_prefix=allowed_write_prefix, - allowed_write_prefixes=allowed_write_prefixes, - provider_extras=provider_extras, - profile_name=profile_name, - resume_session_id=resume_session_id, - resume_checkpoint=resume_checkpoint, - resume_message=resume_message, - ) + cfg = self._apply_config(config) + completion_marker = cfg["completion_marker"] + model = cfg["model"] + plugin_source = cfg["plugin_source"] + output_format = cfg["output_format"] + add_dirs = cfg["add_dirs"] + exit_after_stop_delay_ms = cfg["exit_after_stop_delay_ms"] + stream_idle_timeout_ms = cfg["stream_idle_timeout_ms"] + scenario_step_name = cfg["scenario_step_name"] + temp_dir_relpath = cfg["temp_dir_relpath"] + allowed_write_prefix = cfg["allowed_write_prefix"] + allowed_write_prefixes = cfg["allowed_write_prefixes"] + provider_extras = cfg["provider_extras"] + profile_name = cfg["profile_name"] + resume_session_id = cfg["resume_session_id"] + resume_checkpoint = cfg["resume_checkpoint"] + resume_message = cfg["resume_message"] + sandbox_mode = cfg["sandbox_mode"] # noqa: F841 - def _build_skill_session_cmd_impl( - self, - skill_command: str, - *, - cwd: str, - completion_marker: str, - model: str | None, - plugin_source: PluginSource | None, - output_format: OutputFormat, - add_dirs: Sequence[ValidatedAddDir] = (), - exit_after_stop_delay_ms: int = 0, - stream_idle_timeout_ms: int = 0, - scenario_step_name: str = "", - temp_dir_relpath: str | None = None, - allowed_write_prefix: str = "", - allowed_write_prefixes: tuple[str, ...] = (), - provider_extras: Mapping[str, str] | None = None, - profile_name: str = "", - resume_session_id: str = "", - resume_checkpoint: SessionCheckpoint | None = None, - resume_message: str | None = None, - sandbox_mode: str = "workspace-write", - ) -> CmdSpec: _has_prefix = ( bool(profile_name) and skill_command.strip().startswith("/") @@ -632,28 +577,21 @@ def _build_skill_session_cmd_impl( profile_name=profile_name, ), ) - extras: dict[str, str] = { - "AUTOSKILLIT_HEADLESS": "1", - "AUTOSKILLIT_SESSION_TYPE": SESSION_TYPE_SKILL, - AGENT_BACKEND_ENV_VAR: AGENT_BACKEND_CLAUDE_CODE, - AGENT_BACKEND_DYNACONF_ENV_VAR: AGENT_BACKEND_CLAUDE_CODE, - AUTOSKILLIT_APPLICABLE_GUARDS: ",".join(sorted(self.capabilities.applicable_guards)), - AUTOSKILLIT_WRITE_GUARD_TOOL_NAMES: ",".join( - sorted(self.capabilities.write_guard_tool_names) - ), - } + extras = self._assemble_shared_env_extras( + session_type=SESSION_TYPE_SKILL, + applicable_guards=self.capabilities.applicable_guards, + write_guard_tool_names=self.capabilities.write_guard_tool_names, + write_prefix=allowed_write_prefix, + write_prefixes=allowed_write_prefixes, + cwd=cwd, + scenario_step_name=scenario_step_name, + ) + extras[AGENT_BACKEND_DYNACONF_ENV_VAR] = AGENT_BACKEND_CLAUDE_CODE + extras[AGENT_BACKEND_ENV_VAR] = AGENT_BACKEND_CLAUDE_CODE if exit_after_stop_delay_ms > 0: extras["CLAUDE_CODE_EXIT_AFTER_STOP_DELAY"] = str(exit_after_stop_delay_ms) if stream_idle_timeout_ms > 0: extras["CLAUDE_STREAM_IDLE_TIMEOUT_MS"] = str(stream_idle_timeout_ms) - extras.update( - self._assemble_shared_env_extras( - write_prefix=allowed_write_prefix, - write_prefixes=allowed_write_prefixes, - cwd=cwd, - scenario_step_name=scenario_step_name, - ) - ) extras["AUTOSKILLIT_SKILL_NAME"] = extract_skill_name(skill_command) or "" if provider_extras: for k, v in provider_extras.items(): @@ -729,28 +667,21 @@ def build_food_truck_cmd( ), ) - extras: dict[str, str] = { - "AUTOSKILLIT_HEADLESS": "1", - "AUTOSKILLIT_SESSION_TYPE": SESSION_TYPE_ORCHESTRATOR, - AGENT_BACKEND_ENV_VAR: AGENT_BACKEND_CLAUDE_CODE, - AGENT_BACKEND_DYNACONF_ENV_VAR: AGENT_BACKEND_CLAUDE_CODE, - AUTOSKILLIT_APPLICABLE_GUARDS: ",".join(sorted(self.capabilities.applicable_guards)), - AUTOSKILLIT_WRITE_GUARD_TOOL_NAMES: ",".join( - sorted(self.capabilities.write_guard_tool_names) - ), - } + extras = self._assemble_shared_env_extras( + session_type=SESSION_TYPE_ORCHESTRATOR, + applicable_guards=self.capabilities.applicable_guards, + write_guard_tool_names=self.capabilities.write_guard_tool_names, + write_prefix=allowed_write_prefix, + write_prefixes=allowed_write_prefixes, + cwd=cwd, + scenario_step_name=scenario_step_name, + ) + extras[AGENT_BACKEND_ENV_VAR] = AGENT_BACKEND_CLAUDE_CODE + extras[AGENT_BACKEND_DYNACONF_ENV_VAR] = AGENT_BACKEND_CLAUDE_CODE if exit_after_stop_delay_ms > 0: extras["CLAUDE_CODE_EXIT_AFTER_STOP_DELAY"] = str(exit_after_stop_delay_ms) if stream_idle_timeout_ms > 0: extras["CLAUDE_STREAM_IDLE_TIMEOUT_MS"] = str(stream_idle_timeout_ms) - extras.update( - self._assemble_shared_env_extras( - write_prefix=allowed_write_prefix, - write_prefixes=allowed_write_prefixes, - cwd=cwd, - scenario_step_name=scenario_step_name, - ) - ) extras.pop(CAMPAIGN_ID_ENV_VAR, None) # food truck does not propagate campaign ID if env_extras: for k, v in env_extras.items(): diff --git a/src/autoskillit/execution/backends/codex.py b/src/autoskillit/execution/backends/codex.py index 05f41b293..2105ee435 100644 --- a/src/autoskillit/execution/backends/codex.py +++ b/src/autoskillit/execution/backends/codex.py @@ -693,23 +693,24 @@ def build_skill_session_cmd( sandbox_mode: str = "workspace-write", ) -> CmdSpec: if config is not None: - completion_marker = config.completion_marker - model = config.model - plugin_source = config.plugin_source # noqa: F841 # no-op: Codex has no --plugin-dir equivalent - output_format = config.output_format # noqa: F841 # no-op: --json is unconditional for Codex - add_dirs = config.add_dirs - exit_after_stop_delay_ms = config.exit_after_stop_delay_ms # noqa: F841 # no-op: Claude-only - stream_idle_timeout_ms = config.stream_idle_timeout_ms - scenario_step_name = config.scenario_step_name - temp_dir_relpath = config.temp_dir_relpath - allowed_write_prefix = config.allowed_write_prefix - allowed_write_prefixes = config.allowed_write_prefixes - provider_extras = config.provider_extras - profile_name = config.profile_name - resume_session_id = config.resume_session_id - resume_checkpoint = config.resume_checkpoint - resume_message = config.resume_message - sandbox_mode = config.sandbox_mode + cfg = self._apply_config(config) + completion_marker = cfg["completion_marker"] + model = cfg["model"] + plugin_source = cfg["plugin_source"] # noqa: F841 # no-op: Codex has no --plugin-dir equivalent + output_format = cfg["output_format"] # noqa: F841 # no-op: --json is unconditional for Codex + add_dirs = cfg["add_dirs"] + exit_after_stop_delay_ms = cfg["exit_after_stop_delay_ms"] # noqa: F841 # no-op: Claude-only + stream_idle_timeout_ms = cfg["stream_idle_timeout_ms"] + scenario_step_name = cfg["scenario_step_name"] + temp_dir_relpath = cfg["temp_dir_relpath"] + allowed_write_prefix = cfg["allowed_write_prefix"] + allowed_write_prefixes = cfg["allowed_write_prefixes"] + provider_extras = cfg["provider_extras"] + profile_name = cfg["profile_name"] + resume_session_id = cfg["resume_session_id"] + resume_checkpoint = cfg["resume_checkpoint"] + resume_message = cfg["resume_message"] + sandbox_mode = cfg["sandbox_mode"] _has_prefix = ( bool(profile_name) and skill_command.strip().startswith("/") @@ -744,20 +745,20 @@ def build_skill_session_cmd( ), ) - extras = _codex_exec_extras( + extras = self._assemble_shared_env_extras( session_type=SESSION_TYPE_SKILL, - include_agent_backend_flat=True, applicable_guards=self.capabilities.applicable_guards, write_guard_tool_names=self.capabilities.write_guard_tool_names, + write_prefix=allowed_write_prefix, + write_prefixes=allowed_write_prefixes, + cwd=cwd, + scenario_step_name=scenario_step_name, ) - extras.update( - self._assemble_shared_env_extras( - write_prefix=allowed_write_prefix, - write_prefixes=allowed_write_prefixes, - cwd=cwd, - scenario_step_name=scenario_step_name, - ) - ) + extras["AUTOSKILLIT_HEADLESS_AUTO_GATE"] = "1" + extras[AGENT_BACKEND_DYNACONF_ENV_VAR] = AGENT_BACKEND_CODEX + extras[AGENT_BACKEND_ENV_VAR] = AGENT_BACKEND_CODEX + extras[MCP_CLIENT_BACKEND_ENV_VAR] = AGENT_BACKEND_CODEX + extras[FOOD_TRUCK_TOOL_TAGS_ENV_VAR] = "" extras["AUTOSKILLIT_SKILL_NAME"] = extract_skill_name(skill_command) or "" if provider_extras: for k, v in provider_extras.items(): @@ -843,20 +844,20 @@ def build_food_truck_cmd( ), ) - extras = _codex_exec_extras( + extras = self._assemble_shared_env_extras( session_type=SESSION_TYPE_ORCHESTRATOR, - include_agent_backend_flat=True, applicable_guards=self.capabilities.applicable_guards, write_guard_tool_names=self.capabilities.write_guard_tool_names, + write_prefix=allowed_write_prefix, + write_prefixes=allowed_write_prefixes, + cwd=cwd, + scenario_step_name=scenario_step_name, ) - extras.update( - self._assemble_shared_env_extras( - write_prefix=allowed_write_prefix, - write_prefixes=allowed_write_prefixes, - cwd=cwd, - scenario_step_name=scenario_step_name, - ) - ) + extras["AUTOSKILLIT_HEADLESS_AUTO_GATE"] = "1" + extras[AGENT_BACKEND_DYNACONF_ENV_VAR] = AGENT_BACKEND_CODEX + extras[AGENT_BACKEND_ENV_VAR] = AGENT_BACKEND_CODEX + extras[MCP_CLIENT_BACKEND_ENV_VAR] = AGENT_BACKEND_CODEX + extras[FOOD_TRUCK_TOOL_TAGS_ENV_VAR] = "" if completion_marker: extras["AUTOSKILLIT_COMPLETION_MARKER"] = completion_marker if env_extras: diff --git a/tests/arch/AGENTS.md b/tests/arch/AGENTS.md index befaf8e42..ca14df874 100644 --- a/tests/arch/AGENTS.md +++ b/tests/arch/AGENTS.md @@ -126,6 +126,7 @@ AST enforcement, sub-package layer contracts, and architectural invariant tests. | `test_doc_fence_filter.py` | Unit and functional tests for `_strip_doc_fenced_blocks` section-aware code fence filter | | `test_capability_scanner_fence_immunity.py` | AST regression guard: capability scanners must import and call `_strip_doc_fenced_blocks` | | `test_backend_stdlib_boundaries.py` | Stdlib-boundary equality tests: write_guard fallback frozenset ↔ CLAUDE_CODE_CAPABILITIES, session type hook string literals ↔ SessionType enum | +| `test_backend_builder_no_independent_copies.py` | AST and isinstance guards: shared env-key literals absent from per-backend files; BACKEND_REGISTRY entries inherit from BackendCmdBuilderBase; FlagVocabulary parity for Claude and Codex | ## Architecture Notes diff --git a/tests/arch/test_backend_builder_no_independent_copies.py b/tests/arch/test_backend_builder_no_independent_copies.py new file mode 100644 index 000000000..6ddeb5a62 --- /dev/null +++ b/tests/arch/test_backend_builder_no_independent_copies.py @@ -0,0 +1,126 @@ +"""AST and structural guards: shared env-key literals absent from per-backend files.""" + +from __future__ import annotations + +import ast +from pathlib import Path + +import pytest + +from autoskillit.execution.backends import BACKEND_REGISTRY +from autoskillit.execution.backends._backend_cmd_builder_base import BackendCmdBuilderBase + +pytestmark = [pytest.mark.layer("arch"), pytest.mark.small] + +_BACKENDS_DIR = ( + Path(__file__).resolve().parents[2] / "src" / "autoskillit" / "execution" / "backends" +) + +_CLAUDE_PATH = _BACKENDS_DIR / "claude.py" +_CODEX_PATH = _BACKENDS_DIR / "codex.py" +_BASE_PATH = _BACKENDS_DIR / "_backend_cmd_builder_base.py" + +SHARED_ENV_LITERAL_KEYS: frozenset[str] = frozenset( + { + "AUTOSKILLIT_ALLOWED_WRITE_PREFIX", + "AUTOSKILLIT_ALLOWED_WRITE_PREFIXES", + "AUTOSKILLIT_CWD", + } +) + +SHARED_ENV_NAME_REFS: frozenset[str] = frozenset( + { + "CAMPAIGN_ID_ENV_VAR", + "KITCHEN_SESSION_ID_ENV_VAR", + } +) + + +def _collect_env_key_string_literals(path: Path) -> set[str]: + """Collect string constants used as dict subscript keys in assignments.""" + tree = ast.parse(path.read_text()) + keys: set[str] = set() + for node in ast.walk(tree): + if not isinstance(node, (ast.Assign, ast.AugAssign)): + continue + targets = node.targets if isinstance(node, ast.Assign) else [node.target] + for target in targets: + if ( + isinstance(target, ast.Subscript) + and isinstance(target.slice, ast.Constant) + and isinstance(target.slice.value, str) + ): + keys.add(target.slice.value) + return keys + + +def _collect_env_name_subscripts(path: Path) -> set[str]: + """Collect Name-node identifiers used as dict subscript keys in assignments.""" + tree = ast.parse(path.read_text()) + names: set[str] = set() + for node in ast.walk(tree): + if not isinstance(node, (ast.Assign, ast.AugAssign)): + continue + targets = node.targets if isinstance(node, ast.Assign) else [node.target] + for target in targets: + if isinstance(target, ast.Subscript) and isinstance(target.slice, ast.Name): + names.add(target.slice.id) + return names + + +def test_shared_env_literals_absent_from_per_backend_files() -> None: + for path in (_CLAUDE_PATH, _CODEX_PATH): + found = _collect_env_key_string_literals(path) & SHARED_ENV_LITERAL_KEYS + assert not found, f"{path.name} still has shared env literals as subscript keys: {found}" + + base_keys = _collect_env_key_string_literals(_BASE_PATH) + missing = SHARED_ENV_LITERAL_KEYS - base_keys + assert not missing, f"_backend_cmd_builder_base.py missing shared env literals: {missing}" + + +def test_shared_env_name_refs_absent_from_per_backend_files() -> None: + for path in (_CLAUDE_PATH, _CODEX_PATH): + found = _collect_env_name_subscripts(path) & SHARED_ENV_NAME_REFS + assert not found, f"{path.name} still has shared env Name refs as subscript keys: {found}" + + base_names = _collect_env_name_subscripts(_BASE_PATH) + missing = SHARED_ENV_NAME_REFS - base_names + assert not missing, f"_backend_cmd_builder_base.py missing shared env Name refs: {missing}" + + +def test_all_backend_registry_entries_use_base_assemble() -> None: + assert BACKEND_REGISTRY, "BACKEND_REGISTRY is empty" + for name, cls in BACKEND_REGISTRY.items(): + assert issubclass(cls, BackendCmdBuilderBase), ( + f"{name}: {cls.__name__} does not inherit from BackendCmdBuilderBase" + ) + + +def test_flag_vocabulary_covers_all_claude_flags() -> None: + from autoskillit.core import ( + NON_VARIADIC_CLAUDE_FLAGS, + VARIADIC_CLAUDE_FLAGS, + ClaudeFlags, + ) + from autoskillit.execution.backends.claude import ClaudeCodeBackend + + backend = ClaudeCodeBackend() + vocab = backend._flag_vocabulary() + assert vocab.variadic_flags == VARIADIC_CLAUDE_FLAGS + assert vocab.non_variadic_flags == NON_VARIADIC_CLAUDE_FLAGS + assert vocab.variadic_flags | vocab.non_variadic_flags == frozenset(ClaudeFlags) + + +def test_flag_vocabulary_covers_all_codex_flags() -> None: + from autoskillit.execution.backends.codex import ( + NON_VARIADIC_CODEX_FLAGS, + VARIADIC_CODEX_FLAGS, + CodexBackend, + CodexFlags, + ) + + backend = CodexBackend() + vocab = backend._flag_vocabulary() + assert vocab.variadic_flags == VARIADIC_CODEX_FLAGS + assert vocab.non_variadic_flags == NON_VARIADIC_CODEX_FLAGS + assert vocab.variadic_flags | vocab.non_variadic_flags == frozenset(CodexFlags) diff --git a/tests/arch/test_subpackage_isolation.py b/tests/arch/test_subpackage_isolation.py index 41b5d418d..01bf851de 100644 --- a/tests/arch/test_subpackage_isolation.py +++ b/tests/arch/test_subpackage_isolation.py @@ -1017,7 +1017,8 @@ def test_data_directories_are_not_python_packages() -> None: "materialization into Codex session directories" "; debug-level symlink failure log in _materialize_profile_skills (+5 net lines)" "; evidence comment above git_metadata_writable=False citing permissions.rs sandbox " - "protection and consumer path (+7 net lines) for T5-P6-A11-WP1", + "protection and consumer path (+7 net lines) for T5-P6-A11-WP1" + "; env-assembly consolidation via _assemble_shared_env_extras (T5-P4-A1-WP2)", ), } diff --git a/tests/contracts/test_backend_protocol.py b/tests/contracts/test_backend_protocol.py index 39d736007..6d1e6a00d 100644 --- a/tests/contracts/test_backend_protocol.py +++ b/tests/contracts/test_backend_protocol.py @@ -187,12 +187,10 @@ def test_build_skill_session_cmd_protocol_shape_call_succeeds(): assert isinstance(result, CmdSpec) -def test_build_skill_session_cmd_config_delegates_to_impl(): +def test_build_skill_session_cmd_config_produces_same_output(): from pathlib import Path - from unittest.mock import patch from autoskillit.core import ( - CmdSpec, DirectInstall, OutputFormat, SessionCheckpoint, @@ -200,6 +198,7 @@ def test_build_skill_session_cmd_config_delegates_to_impl(): ) from autoskillit.execution.backends import ClaudeCodeBackend + chk = SessionCheckpoint(step_name="chk") config = SkillSessionConfig( completion_marker="%%VERIFY%%", model="sonnet", @@ -214,48 +213,35 @@ def test_build_skill_session_cmd_config_delegates_to_impl(): provider_extras={"EXTRA": "val"}, profile_name="verify-profile", resume_session_id="sess-1", - resume_checkpoint=SessionCheckpoint(step_name="chk"), + resume_checkpoint=chk, resume_message="resume-msg", sandbox_mode="read-only", ) - sentinel = CmdSpec(cmd=("sentinel",), env={}) - - with patch.object( - ClaudeCodeBackend, "_build_skill_session_cmd_impl", return_value=sentinel - ) as mock_impl: - backend = ClaudeCodeBackend() - result = backend.build_skill_session_cmd("/test", "/work", config) - - assert result is sentinel - mock_impl.assert_called_once() - args = mock_impl.call_args.args - assert args == ("/test",), f"skill_command not forwarded: {args}" - kw = mock_impl.call_args.kwargs - assert kw["cwd"] == "/work" - assert kw["completion_marker"] == config.completion_marker - assert kw["model"] == config.model - assert kw["plugin_source"] == config.plugin_source - assert kw["output_format"] == config.output_format - assert kw["add_dirs"] == config.add_dirs - assert kw["exit_after_stop_delay_ms"] == config.exit_after_stop_delay_ms - assert kw["stream_idle_timeout_ms"] == config.stream_idle_timeout_ms - assert kw["scenario_step_name"] == config.scenario_step_name - assert kw["temp_dir_relpath"] == config.temp_dir_relpath - assert kw["allowed_write_prefix"] == config.allowed_write_prefix - assert kw["allowed_write_prefixes"] == config.allowed_write_prefixes - assert kw["provider_extras"] == config.provider_extras - assert kw["profile_name"] == config.profile_name - assert kw["resume_session_id"] == config.resume_session_id - assert kw["resume_checkpoint"] == config.resume_checkpoint - assert kw["resume_message"] == config.resume_message - assert kw["sandbox_mode"] == config.sandbox_mode - - -def test_build_skill_session_cmd_impl_exists(): - from autoskillit.execution.backends import ClaudeCodeBackend - assert hasattr(ClaudeCodeBackend, "_build_skill_session_cmd_impl") - assert callable(getattr(ClaudeCodeBackend, "_build_skill_session_cmd_impl")) + backend = ClaudeCodeBackend() + via_config = backend.build_skill_session_cmd("/test", "/work", config=config) + via_flat = backend.build_skill_session_cmd( + "/test", + "/work", + completion_marker=config.completion_marker, + model=config.model, + plugin_source=config.plugin_source, + output_format=config.output_format, + add_dirs=config.add_dirs, + exit_after_stop_delay_ms=config.exit_after_stop_delay_ms, + stream_idle_timeout_ms=config.stream_idle_timeout_ms, + scenario_step_name=config.scenario_step_name, + temp_dir_relpath=config.temp_dir_relpath, + allowed_write_prefix=config.allowed_write_prefix, + allowed_write_prefixes=config.allowed_write_prefixes, + provider_extras=config.provider_extras, + profile_name=config.profile_name, + resume_session_id=config.resume_session_id, + resume_checkpoint=config.resume_checkpoint, + resume_message=config.resume_message, + ) + assert via_config.cmd == via_flat.cmd + assert via_config.env == via_flat.env def test_build_interactive_cmd_satisfies_protocol_claude(): diff --git a/tests/execution/backends/test_backend_cmd_builder_base.py b/tests/execution/backends/test_backend_cmd_builder_base.py index 3b27cf188..6dc0c8ead 100644 --- a/tests/execution/backends/test_backend_cmd_builder_base.py +++ b/tests/execution/backends/test_backend_cmd_builder_base.py @@ -73,14 +73,21 @@ def _clean_env(self, monkeypatch: pytest.MonkeyPatch) -> None: def test_all_eight_keys(self) -> None: result = BackendCmdBuilderBase._assemble_shared_env_extras( + session_type="skill", + applicable_guards=frozenset({"write_guard"}), + write_guard_tool_names=frozenset({"Write", "Edit"}), write_prefix="/tmp/wp", write_prefixes=("/tmp/a", "/tmp/b"), cwd="/work", scenario_step_name="step1", ) - assert len(result) == 8 + assert len(result) == 12 assert result["MAX_MCP_OUTPUT_TOKENS"] == "50000" assert result["MCP_CONNECTION_NONBLOCKING"] == "0" + assert result["AUTOSKILLIT_HEADLESS"] == "1" + assert result["AUTOSKILLIT_SESSION_TYPE"] == "skill" + assert result["AUTOSKILLIT_APPLICABLE_GUARDS"] == "write_guard" + assert result["AUTOSKILLIT_WRITE_GUARD_TOOL_NAMES"] == "Edit,Write" assert result[CAMPAIGN_ID_ENV_VAR] == "camp-123" assert result[KITCHEN_SESSION_ID_ENV_VAR] == "kitchen-456" assert result["AUTOSKILLIT_ALLOWED_WRITE_PREFIX"] == "/tmp/wp" @@ -94,12 +101,16 @@ def test_conditional_keys_absent(self, monkeypatch: pytest.MonkeyPatch) -> None: result = BackendCmdBuilderBase._assemble_shared_env_extras() assert "MAX_MCP_OUTPUT_TOKENS" in result assert "MCP_CONNECTION_NONBLOCKING" in result + assert result["AUTOSKILLIT_HEADLESS"] == "1" assert CAMPAIGN_ID_ENV_VAR not in result assert KITCHEN_SESSION_ID_ENV_VAR not in result assert "AUTOSKILLIT_ALLOWED_WRITE_PREFIX" not in result assert "AUTOSKILLIT_ALLOWED_WRITE_PREFIXES" not in result assert "AUTOSKILLIT_CWD" not in result assert "SCENARIO_STEP_NAME" not in result + assert "AUTOSKILLIT_SESSION_TYPE" not in result + assert "AUTOSKILLIT_APPLICABLE_GUARDS" not in result + assert "AUTOSKILLIT_WRITE_GUARD_TOOL_NAMES" not in result class TestApplyConfig: diff --git a/tests/execution/backends/test_claude_backend.py b/tests/execution/backends/test_claude_backend.py index f7e19a739..a757f0c3e 100644 --- a/tests/execution/backends/test_claude_backend.py +++ b/tests/execution/backends/test_claude_backend.py @@ -191,7 +191,7 @@ def test_config_adapter_matches_impl(self) -> None: output_format=OutputFormat.JSON, ) via_config = backend.build_skill_session_cmd("/plan", cwd="/tmp", config=config) - via_impl = backend._build_skill_session_cmd_impl( + via_flat = backend.build_skill_session_cmd( "/plan", cwd="/tmp", completion_marker="%%DONE%%", @@ -199,8 +199,8 @@ def test_config_adapter_matches_impl(self) -> None: plugin_source=None, output_format=OutputFormat.JSON, ) - assert via_config.cmd == via_impl.cmd - assert via_config.env == via_impl.env + assert via_config.cmd == via_flat.cmd + assert via_config.env == via_flat.env def test_config_adapter_forwards_all_fields(self) -> None: backend = ClaudeCodeBackend() @@ -223,7 +223,7 @@ def test_config_adapter_forwards_all_fields(self) -> None: resume_message="resume-msg", ) via_config = backend.build_skill_session_cmd("/plan", cwd="/tmp", config=config) - via_impl = backend._build_skill_session_cmd_impl( + via_flat = backend.build_skill_session_cmd( "/plan", cwd="/tmp", completion_marker="%%MARKER%%", @@ -242,8 +242,8 @@ def test_config_adapter_forwards_all_fields(self) -> None: resume_checkpoint=chk, resume_message="resume-msg", ) - assert via_config.cmd == via_impl.cmd - assert via_config.env == via_impl.env + assert via_config.cmd == via_flat.cmd + assert via_config.env == via_flat.env def test_legacy_flat_params_still_work(self) -> None: backend = ClaudeCodeBackend()