diff --git a/AGENTS.md b/AGENTS.md index 06fb641b..a55921c5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,8 +55,8 @@ src/panopticon/ # → render skills + operations, point it at the /mcp server, deliver the workflow # overview to the agent's context → launch the CLI); cli/ = the agent-CLI adapter # package (ADR 0014): cli/base.py = the AgentCLI seam (ABC) + registry, - # cli/claude.py = ClaudeAgentCLI + cli/codex.py = CodexAgentCLI (M3.5: config, - # skills, MCP, AGENTS.md overview, launch/resume, auth; turn-flip hooks = M3.6) + # cli/claude.py = ClaudeAgentCLI + cli/codex.py = CodexAgentCLI (config, skills, + # MCP, AGENTS.md overview, launch/resume, auth, turn-flip hooks — full seam) # — the ONLY LLM pkg docker/Dockerfile # base task-container image (ADR 0005 base layer): python + git + bash + # the panopticon package + the `claude` CLI the agent execs; runs as the diff --git a/docs/design/decisions/0014-agent-cli-adapter.md b/docs/design/decisions/0014-agent-cli-adapter.md index 4bccedba..42c2d8e0 100644 --- a/docs/design/decisions/0014-agent-cli-adapter.md +++ b/docs/design/decisions/0014-agent-cli-adapter.md @@ -201,8 +201,15 @@ spike the implementer resolves against the installed codex version): 1. MCP transport — HTTP vs stdio support (we serve HTTP today; if codex is stdio-only we front it with a local proxy or add an stdio MCP entrypoint). -2. The exact codex **hooks config schema** and payload shape (for the turn-flip callback's - background-task gating — the `background_tasks` analogue). +2. ~~The exact codex **hooks config schema** and payload shape (for the turn-flip callback's + background-task gating — the `background_tasks` analogue).~~ **Resolved (M3.6):** hooks live under + a `[hooks]` table keyed by event, each an array of groups whose `hooks` array holds + `{type = "command", command = …}` (`[[hooks.Stop]]` → `[[hooks.Stop.hooks]]`); `Stop` / + `UserPromptSubmit` take no `matcher`. The `Stop` stdin payload is `session_id` / + `transcript_path` / `cwd` / `hook_event_name` / `model` / `permission_mode` / `turn_id` / + `stop_hook_active` / `last_assistant_message` — **no `background_tasks` analogue**, so the flip + degrades to the plain turn hand-back (claude's exact behaviour when the field is absent) and + `has_live_background_task` is always `False` for codex. 3. The `CODEX_HOME` config-dir override and its precedence. 4. The **instructions-merge precedence** — confirm `$CODEX_HOME/AGENTS.md` (or `experimental_instructions_file`) layers *additively* on top of the repo's root `AGENTS.md` @@ -211,12 +218,16 @@ spike the implementer resolves against the installed codex version): without touching the working tree. 5. The unattended / **skip-approvals sandbox** flag (the `--dangerously-skip-permissions` analogue — codex runs headless in a throwaway container on a per-task clone). -6. Whether codex **feeds hook stdout into the agent's context** (as claude does for - `UserPromptSubmit`). If not, the per-turn briefing + provisioning nudge need a different channel - (e.g. writing them into `$CODEX_HOME/AGENTS.md`, or a session-start injection). -7. The codex trigger for the **"agent is asking the user" turn state** — the `AskUserQuestion` - PreToolUse/PostToolUse flip has no direct codex analogue; find the equivalent (or accept that - the turn simply stays on the agent until the next `Stop`, a documented degradation). +6. ~~Whether codex **feeds hook stdout into the agent's context** (as claude does for + `UserPromptSubmit`).~~ **Resolved (M3.6): yes** — a `UserPromptSubmit` hook's plain-text stdout + is added to the agent as extra developer context, so the per-turn briefing + provisioning nudge + ride the same channel as claude, no alternate needed. (`Stop` is the opposite: plain-text stdout + is invalid there — JSON only — but our callback prints nothing on the stop path, so it's fine.) +7. ~~The codex trigger for the **"agent is asking the user" turn state** — the `AskUserQuestion` + PreToolUse/PostToolUse flip has no direct codex analogue.~~ **Resolved (M3.6):** codex has no + `AskUserQuestion` tool (and `Stop`/`UserPromptSubmit` take no `matcher`), so we wire no + `PreToolUse`/`PostToolUse` pair — the turn stays on the agent until the next `Stop`, the accepted + documented degradation. ### 6. The determinism invariant holds diff --git a/src/panopticon/container/cli/codex.py b/src/panopticon/container/cli/codex.py index 68c2176f..5d348321 100644 --- a/src/panopticon/container/cli/codex.py +++ b/src/panopticon/container/cli/codex.py @@ -15,11 +15,14 @@ - **launch / resume** → ``codex`` first-run vs ``codex resume --last`` (the ``claude --continue`` analogue), probing ``$CODEX_HOME/sessions`` for a prior transcript. -Scope is **M3.5**: everything needed to boot codex, reach the MCP server, see its skills + overview, -and resume. The **turn-flip hooks** (``write_settings`` wiring, the background-task gating payload) -are **M3.6** — the three hook seam methods are implemented here only enough to keep this class -concrete and degrade safely (see each method's docstring). The determinism invariant holds: this -lives in ``container/`` and only :meth:`launch` execs the real CLI (injected in tests). +Scope now includes the **turn-flip hooks** (M3.6): :meth:`~CodexAgentCLI.write_settings` wires +codex's ``[hooks]`` ``Stop`` / ``UserPromptSubmit`` block to the shared callback, and the hook-payload +seam (:meth:`~CodexAgentCLI.read_hook_payload` / :meth:`~CodexAgentCLI.has_live_background_task`) +parses codex's Stop payload. Codex feeds a ``UserPromptSubmit`` hook's stdout back as developer +context (ADR 0014 flag 6), so the briefing + provisioning nudge ride the same channel as claude; its +Stop payload has no background-task array so the flip always hands the turn back (flag 2), and it has +no ``AskUserQuestion`` analogue (flag 7) — both handled as documented. The determinism invariant +holds: this lives in ``container/`` and only :meth:`launch` execs the real CLI (injected in tests). """ from __future__ import annotations @@ -33,6 +36,7 @@ from panopticon.container.cli.base import AgentCLI, _Client from panopticon.container.config import update_toml_config +from panopticon.container.hooks import HOOK_COMMAND from panopticon.container.skills import write_commands, write_operation_commands from panopticon.core.models import Skill @@ -43,6 +47,16 @@ _MODEL_TIERS = {"primary": "gpt-5.6-codex"} +def _command_hook(actor: str, event: str) -> dict[str, Any]: + """One codex hook group: run the shared turn-flip callback with `` ``. + + Codex nests a command under an event as ``{"hooks": [{"type": "command", "command": …}]}`` — the + ``[[hooks.]]`` → ``[[hooks..hooks]]`` TOML shape. The command is the CLI-agnostic + callback (:data:`~panopticon.container.hooks.HOOK_COMMAND`) claude invokes too. + """ + return {"hooks": [{"type": "command", "command": f"{HOOK_COMMAND} {actor} {event}"}]} + + class CodexAgentCLI(AgentCLI): """The `codex` adapter (ADR 0014 §5). Config, skills, MCP, overview, trust, auth, launch/resume.""" @@ -71,17 +85,31 @@ def render_operations(self, client: _Client, task_id: str, home: Path) -> list[P ) def write_settings(self, home: Path) -> Path: - """Return codex's ``config.toml`` path; the turn-flip **hooks are M3.6**, not wired here. - - The launcher calls this to wire the Stop/UserPromptSubmit turn-flip hooks. Codex's hooks - config schema (and its background-task payload shape) is ADR 0014 flag 2, owned by the - **Codex turn-flip hooks** slice (M3.6) — until it lands a codex task's turn doesn't auto-flip - (a documented interim, ADR §5). So this only ensures the config dir exists and returns the - path other methods merge into; it writes no hook entries. When M3.6 lands, it merges codex's - ``[hooks]`` block invoking ``python -m panopticon.container.hook`` here. + """Wire codex's turn-flip hooks into ``config.toml``; return the path (ADR 0014 §5, M3.6). + + Codex's hooks live under a ``[hooks]`` table keyed by event, each event an array of groups + whose ``hooks`` array holds ``{type = "command", command = …}`` entries (the same shape + claude uses, just TOML). We wire the two turn-flip events the same callback + (:mod:`panopticon.container.hook`) serves for claude: + + - **Stop** → ``hook user stop`` (flip the ball to the user; the callback applies the + background-task guard). The callback prints nothing on the stop path, satisfying codex's + rule that plain-text stdout is invalid for ``Stop`` (JSON-only). + - **UserPromptSubmit** → ``hook agent prompt`` (flip to the agent, then print the phase + briefing + provisioning nudge — codex feeds a ``UserPromptSubmit`` hook's stdout back as + developer context, so the same channel claude relies on works here; ADR 0014 flag 6). + + Codex's ``Stop``/``UserPromptSubmit`` don't support a ``matcher``, and codex has no + ``AskUserQuestion`` tool, so — unlike claude — we wire *no* ``PreToolUse``/``PostToolUse`` + pair; the "agent is asking the user" turn state simply stays on the agent until the next Stop + (the documented degradation, ADR 0014 flag 7). Merged read-modify-write so it coexists with + the MCP / trust / overview keys already in ``config.toml``. """ config = home / self.config_dirname / self.CONFIG_FILE - config.parent.mkdir(parents=True, exist_ok=True) + with update_toml_config(config) as data: + hooks = data.setdefault("hooks", {}) + hooks["Stop"] = [_command_hook("user", "stop")] + hooks["UserPromptSubmit"] = [_command_hook("agent", "prompt")] return config def write_mcp_config(self, config_dir: Path, service_url: str) -> Path: @@ -173,14 +201,14 @@ def read_hook_payload(self, stdin: TextIO) -> dict[str, Any]: return data if isinstance(data, dict) else {} def has_live_background_task(self, payload: dict[str, Any]) -> bool: - """Whether the Stop payload reports still-running background work — **M3.6**, ``False`` for now. - - The turn-flip background-task gating needs codex's background-task payload shape (the - ``background_tasks`` analogue), which is ADR 0014 flag 2, owned by the Codex turn-flip hooks - slice (M3.6). Until then this degrades to the plain turn flip — exactly the safe degradation - claude already uses when the field is absent (an older CLI): the turn flips to the user on - Stop. Codex's hooks aren't wired yet either (see :meth:`write_settings`), so this isn't - reached in practice; it's implemented conservatively so it's correct the moment M3.6 wires it. + """Whether the Stop payload reports still-running background work (gates the turn flip). + + Always ``False`` for codex: its documented ``Stop`` payload (ADR 0014 flag 2, verified against + the hooks schema) is ``session_id`` / ``transcript_path`` / ``cwd`` / ``hook_event_name`` / + ``model`` / ``permission_mode`` / ``turn_id`` / ``stop_hook_active`` / ``last_assistant_message`` + — it carries **no** background-task array (unlike claude's ``background_tasks``), and codex's + ``Stop`` fires only when the turn has genuinely ended, so there's nothing in flight to strand. + The turn flips to the user, matching claude's exact behaviour when the field is absent. """ return False diff --git a/src/panopticon/container/hook.py b/src/panopticon/container/hook.py index 2363a739..3aad9abb 100644 --- a/src/panopticon/container/hook.py +++ b/src/panopticon/container/hook.py @@ -73,7 +73,8 @@ def main( return 0 client.set_turn(task_id, actor) # `prompt` (UserPromptSubmit): ground the agent in its current phase, and (while the task is - # unslugged) nudge toward provisioning. claude adds this hook's stdout to its context. + # unslugged) nudge toward provisioning. The CLI adds this hook's stdout to the agent's context + # (claude and codex both do — ADR 0014 flag 6). if event == "prompt": print(client.get_briefing(task_id)) if client.get_task(task_id).get("slug") is None: diff --git a/tests/container/test_codex.py b/tests/container/test_codex.py index 35e9bead..140ca54c 100644 --- a/tests/container/test_codex.py +++ b/tests/container/test_codex.py @@ -209,7 +209,7 @@ def test_launch_argv_passes_model_before_initial_prompt_on_first_run(tmp_path: P ] -# -- hook seam (M3.6 stubs; concrete + safe here) ----------------------------------------------- +# -- hook seam (M3.6) --------------------------------------------------------------------------- def test_read_hook_payload_tolerates_empty_and_invalid() -> None: @@ -220,20 +220,49 @@ def test_read_hook_payload_tolerates_empty_and_invalid() -> None: assert cli.read_hook_payload(io.StringIO('{"a": 1}')) == {"a": 1} -def test_has_live_background_task_degrades_to_false_until_m36() -> None: - # Codex's background-task payload shape (ADR flag 2) is wired in M3.6; until then the turn flips - # on Stop (the same degradation claude uses when the field is absent). +def test_has_live_background_task_always_false_for_codexs_stop_payload() -> None: + # Codex's documented Stop payload carries no background-task array, so a real Stop flips the turn. cli = CodexAgentCLI() + real_stop = {"hook_event_name": "Stop", "turn_id": "t", "stop_hook_active": False} + assert cli.has_live_background_task(real_stop) is False assert cli.has_live_background_task({}) is False - assert cli.has_live_background_task({"background_tasks": [{"status": "running"}]}) is False # -- settings / hooks (M3.6) -------------------------------------------------------------------- -def test_write_settings_returns_the_config_path_without_wiring_hooks_yet(tmp_path: Path) -> None: - # M3.5 keeps the class concrete; the actual turn-flip hook block is M3.6. +def _hook_command(entry: object) -> str: + # Unwrap codex's [[hooks.]] → [[hooks..hooks]] → {type, command} nesting. + assert isinstance(entry, list) and len(entry) == 1 + inner = entry[0]["hooks"] + assert isinstance(inner, list) and len(inner) == 1 and inner[0]["type"] == "command" + return str(inner[0]["command"]) + + +def test_write_settings_wires_the_turn_flip_hooks(tmp_path: Path) -> None: cli = CodexAgentCLI() path = cli.write_settings(tmp_path) assert path == tmp_path / cli.config_dirname / cli.CONFIG_FILE - assert path.parent.is_dir() # config dir ensured for the other writers + data = tomllib.loads(path.read_text()) + hooks = data["hooks"] + # Stop hands the ball to the user; UserPromptSubmit takes it back + prints briefing/nudge. + assert _hook_command(hooks["Stop"]) == "python -m panopticon.container.hook user stop" + assert ( + _hook_command(hooks["UserPromptSubmit"]) + == "python -m panopticon.container.hook agent prompt" + ) + # No AskUserQuestion analogue in codex → no PreToolUse/PostToolUse pair (ADR 0014 flag 7). + assert "PreToolUse" not in hooks and "PostToolUse" not in hooks + + +def test_hooks_coexist_with_mcp_and_trust_in_one_config_toml(tmp_path: Path) -> None: + # The launcher calls all three against the same config.toml; none may clobber another's keys. + cli = CodexAgentCLI() + config_dir = tmp_path / cli.config_dirname + cli.write_settings(tmp_path) # takes home; the others take the config dir + cli.write_mcp_config(config_dir, "http://svc:8000") + cli.trust_workspace(config_dir, Path("/workspace")) + data = tomllib.loads((config_dir / cli.CONFIG_FILE).read_text()) + assert _hook_command(data["hooks"]["Stop"]).endswith("user stop") # preserved + assert data["mcp_servers"]["panopticon"]["url"] == "http://svc:8000/mcp" + assert data["projects"]["/workspace"]["trust_level"] == "trusted"