diff --git a/src/autoskillit/execution/backends/codex.py b/src/autoskillit/execution/backends/codex.py index 2105ee435..23e1bf1a0 100644 --- a/src/autoskillit/execution/backends/codex.py +++ b/src/autoskillit/execution/backends/codex.py @@ -696,10 +696,10 @@ def build_skill_session_cmd( 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 + 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"] # noqa: F841 # no-op: Claude-only + 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"] @@ -711,6 +711,10 @@ def build_skill_session_cmd( resume_checkpoint = cfg["resume_checkpoint"] resume_message = cfg["resume_message"] sandbox_mode = cfg["sandbox_mode"] + if plugin_source is not None: + logger.warning("codex_plugin_source_discarded", plugin_source=str(plugin_source)) + if output_format != OutputFormat.JSON: + logger.warning("codex_output_format_coerced") _has_prefix = ( bool(profile_name) and skill_command.strip().startswith("/") @@ -769,6 +773,14 @@ def build_skill_session_cmd( extras["AUTOSKILLIT_COMPLETION_MARKER"] = completion_marker if add_dirs: extras["CODEX_HOME"] = add_dirs[0].path + if exit_after_stop_delay_ms: + extras.setdefault( + "AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT", str(exit_after_stop_delay_ms / 1000) + ) + if stream_idle_timeout_ms: + extras.setdefault( + "AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT", str(stream_idle_timeout_ms / 1000) + ) filtered_base = {k: v for k, v in os.environ.items() if k not in _HEADLESS_EXCLUSIVE_VARS} env = CodexEnvPolicy().build_env( @@ -819,9 +831,10 @@ def build_food_truck_cmd( sentinel_contract: str = "", resume_message: str | None = None, ) -> CmdSpec: - _plugin_source = plugin_source # noqa: F841 # no-op: Codex has no --plugin-dir - _output_format = output_format # noqa: F841 # no-op: --json is unconditional - _exit_ms = exit_after_stop_delay_ms # noqa: F841 # no-op: Claude-only + if plugin_source is not None: + logger.warning("codex_plugin_source_discarded", plugin_source=str(plugin_source)) + if output_format != OutputFormat.STREAM_JSON: + logger.warning("codex_output_format_coerced") if resume_session_id: effective_prompt = _compose_resume_prompt( @@ -864,6 +877,14 @@ def build_food_truck_cmd( for k, v in env_extras.items(): if k not in _PROVIDER_EXTRAS_BASE_DENYLIST: extras[k] = v + if exit_after_stop_delay_ms: + extras.setdefault( + "AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT", str(exit_after_stop_delay_ms / 1000) + ) + if stream_idle_timeout_ms: + extras.setdefault( + "AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT", str(stream_idle_timeout_ms / 1000) + ) filtered_base = {k: v for k, v in os.environ.items() if k not in _HEADLESS_EXCLUSIVE_VARS} env = CodexEnvPolicy().build_env( diff --git a/tests/arch/test_subpackage_isolation.py b/tests/arch/test_subpackage_isolation.py index 01bf851de..82f66d079 100644 --- a/tests/arch/test_subpackage_isolation.py +++ b/tests/arch/test_subpackage_isolation.py @@ -996,7 +996,7 @@ def test_data_directories_are_not_python_packages() -> None: "crash-close before executor when /dev/shm path has been reclaimed (+26 net lines)", ), "execution/backends/codex.py": ( - 1125, + 1150, "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; " @@ -1018,7 +1018,10 @@ def test_data_directories_are_not_python_packages() -> None: "; 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" - "; env-assembly consolidation via _assemble_shared_env_extras (T5-P4-A1-WP2)", + "; env-assembly consolidation via _assemble_shared_env_extras (T5-P4-A1-WP2)" + "; 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", ), } diff --git a/tests/execution/backends/test_codex_backend.py b/tests/execution/backends/test_codex_backend.py index 247a26c62..6146982c6 100644 --- a/tests/execution/backends/test_codex_backend.py +++ b/tests/execution/backends/test_codex_backend.py @@ -611,20 +611,17 @@ def test_completion_marker_with_profile(self) -> None: spec2 = CodexBackend().build_skill_session_cmd(**self.BASE) assert "AUTOSKILLIT_COMPLETION_MARKER" not in spec2.env - def test_claude_only_params_accepted_but_ignored(self) -> None: - spec = CodexBackend().build_skill_session_cmd( - **{ - **self.BASE, - "exit_after_stop_delay_ms": 5000, - "stream_idle_timeout_ms": 3000, - } - ) - cmd_str = " ".join(spec.cmd) - assert "--output-format" not in cmd_str - assert "--plugin-dir" not in cmd_str + def test_default_params_emit_no_warnings(self) -> None: + with structlog.testing.capture_logs() as cap_logs: + spec = CodexBackend().build_skill_session_cmd(**self.BASE) + discard_events = [ + e + for e in cap_logs + if e.get("event") in ("codex_plugin_source_discarded", "codex_output_format_coerced") + ] + assert discard_events == [] assert "CLAUDE_CODE_EXIT_AFTER_STOP_DELAY" not in spec.env assert "CLAUDE_STREAM_IDLE_TIMEOUT_MS" not in spec.env - assert spec.process_idle_timeout_ms == 3000 def test_stream_idle_timeout_routed_to_cmdspec(self) -> None: spec = CodexBackend().build_skill_session_cmd( @@ -1540,58 +1537,144 @@ def test_claude_code_backend_process_idle_default_zero(self) -> None: class TestCodexDiscardDispositions: - """Document the complete noqa:F841 discard disposition picture in codex.py. + """Codex builder parameter disposition contracts. - stream_idle_timeout_ms -> routed to CmdSpec.process_idle_timeout_ms (P1-A6-WP1). - plugin_source, output_format, exit_after_stop_delay_ms -> intentional - no-op discards (P2-A2 scope). + plugin_source -> logged warning when non-None. + output_format -> logged warning when != JSON. + exit_after_stop_delay_ms -> AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT env injection via setdefault. + stream_idle_timeout_ms -> routed to CmdSpec.process_idle_timeout_ms + env injection. """ - def test_stream_idle_timeout_routed_in_skill_builder(self) -> None: + SKILL_BASE: dict[str, object] = { + "skill_command": "/test", + "cwd": "/work", + "completion_marker": "%%DONE%%", + } + FOOD_TRUCK_BASE: dict[str, object] = { + "orchestrator_prompt": "go", + "plugin_source": DirectInstall(plugin_dir=Path("/pkg")), + "cwd": "/work", + "completion_marker": "%%DONE%%", + } + + def test_plugin_source_warning_skill_builder(self) -> None: + with structlog.testing.capture_logs() as cap_logs: + CodexBackend().build_skill_session_cmd( + **self.SKILL_BASE, + plugin_source=DirectInstall(plugin_dir=Path("/pkg")), + ) + events = [e for e in cap_logs if e.get("event") == "codex_plugin_source_discarded"] + assert len(events) == 1 + assert events[0]["log_level"] == "warning" + + def test_plugin_source_warning_food_truck_builder(self) -> None: + with structlog.testing.capture_logs() as cap_logs: + CodexBackend().build_food_truck_cmd(**self.FOOD_TRUCK_BASE) + events = [e for e in cap_logs if e.get("event") == "codex_plugin_source_discarded"] + assert len(events) == 1 + assert events[0]["log_level"] == "warning" + + def test_output_format_warning_skill_builder(self) -> None: + with structlog.testing.capture_logs() as cap_logs: + CodexBackend().build_skill_session_cmd( + **self.SKILL_BASE, + output_format=OutputFormat.STREAM_JSON, + ) + events = [e for e in cap_logs if e.get("event") == "codex_output_format_coerced"] + assert len(events) == 1 + assert events[0]["log_level"] == "warning" + + def test_output_format_warning_food_truck_builder(self) -> None: + with structlog.testing.capture_logs() as cap_logs: + CodexBackend().build_food_truck_cmd( + **self.FOOD_TRUCK_BASE, + output_format=OutputFormat.JSON, + ) + events = [e for e in cap_logs if e.get("event") == "codex_output_format_coerced"] + assert len(events) == 1 + assert events[0]["log_level"] == "warning" + + def test_exit_delay_injects_idle_timeout_skill_builder(self) -> None: spec = CodexBackend().build_skill_session_cmd( - skill_command="/test", - cwd="/work", - completion_marker="%%DONE%%", - stream_idle_timeout_ms=10000, + **self.SKILL_BASE, + exit_after_stop_delay_ms=5000, ) - assert spec.process_idle_timeout_ms == 10000 + assert spec.env["AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT"] == "5.0" - def test_stream_idle_timeout_routed_in_food_truck_builder(self) -> None: + def test_exit_delay_injects_idle_timeout_food_truck_builder(self) -> None: spec = CodexBackend().build_food_truck_cmd( - orchestrator_prompt="go", - plugin_source=DirectInstall(plugin_dir=Path("/pkg")), - cwd="/work", - completion_marker="%%DONE%%", - stream_idle_timeout_ms=20000, + **self.FOOD_TRUCK_BASE, + exit_after_stop_delay_ms=5000, ) - assert spec.process_idle_timeout_ms == 20000 + assert spec.env["AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT"] == "5.0" - def test_exit_after_stop_delay_still_discarded(self) -> None: + def test_stream_idle_injects_idle_timeout_skill_builder(self) -> None: spec = CodexBackend().build_skill_session_cmd( - skill_command="/test", - cwd="/work", - completion_marker="%%DONE%%", - exit_after_stop_delay_ms=5000, + **self.SKILL_BASE, + stream_idle_timeout_ms=3000, ) - assert "CLAUDE_CODE_EXIT_AFTER_STOP_DELAY" not in spec.env + assert spec.env["AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT"] == "3.0" + assert spec.process_idle_timeout_ms == 3000 - def test_plugin_source_still_discarded(self) -> None: + def test_stream_idle_injects_idle_timeout_food_truck_builder(self) -> None: + spec = CodexBackend().build_food_truck_cmd( + **self.FOOD_TRUCK_BASE, + stream_idle_timeout_ms=3000, + ) + assert spec.env["AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT"] == "3.0" + assert spec.process_idle_timeout_ms == 3000 + + def test_stream_idle_routed_to_process_idle_skill_builder(self) -> None: spec = CodexBackend().build_skill_session_cmd( - skill_command="/test", - cwd="/work", - completion_marker="%%DONE%%", - plugin_source=DirectInstall(plugin_dir=Path("/pkg")), + **self.SKILL_BASE, + stream_idle_timeout_ms=10000, + ) + assert spec.process_idle_timeout_ms == 10000 + + def test_stream_idle_routed_to_process_idle_food_truck_builder(self) -> None: + spec = CodexBackend().build_food_truck_cmd( + **self.FOOD_TRUCK_BASE, + stream_idle_timeout_ms=10000, ) - assert "--plugin-dir" not in " ".join(spec.cmd) + assert spec.process_idle_timeout_ms == 10000 + + def test_zero_ms_no_idle_timeout_skill_builder(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT", raising=False) + spec = CodexBackend().build_skill_session_cmd(**self.SKILL_BASE) + assert "AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT" not in spec.env + + def test_zero_ms_no_idle_timeout_food_truck_builder( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.delenv("AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT", raising=False) + spec = CodexBackend().build_food_truck_cmd(**self.FOOD_TRUCK_BASE) + assert "AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT" not in spec.env - def test_output_format_still_discarded(self) -> None: + def test_existing_idle_timeout_not_overwritten_skill_builder(self) -> None: spec = CodexBackend().build_skill_session_cmd( - skill_command="/test", - cwd="/work", - completion_marker="%%DONE%%", - output_format=OutputFormat.STREAM_JSON, + **self.SKILL_BASE, + exit_after_stop_delay_ms=5000, + provider_extras={"AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT": "99.0"}, ) - assert "--json" in spec.cmd + assert spec.env["AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT"] == "99.0" + + def test_existing_idle_timeout_not_overwritten_food_truck_builder(self) -> None: + spec = CodexBackend().build_food_truck_cmd( + **self.FOOD_TRUCK_BASE, + exit_after_stop_delay_ms=5000, + env_extras={"AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT": "99.0"}, + ) + assert spec.env["AUTOSKILLIT_IDLE_OUTPUT_TIMEOUT"] == "99.0" + + def test_no_warnings_on_defaults_skill_builder(self) -> None: + with structlog.testing.capture_logs() as cap_logs: + CodexBackend().build_skill_session_cmd(**self.SKILL_BASE) + discard_events = [ + e + for e in cap_logs + if e.get("event") in ("codex_plugin_source_discarded", "codex_output_format_coerced") + ] + assert discard_events == [] class TestCodexBackendSetupSessionDir: