From 5fc99fd09eaa4078e0f7956977f3a4e66d12c581 Mon Sep 17 00:00:00 2001 From: Mihai Chirculescu Date: Fri, 7 Aug 2026 16:30:23 +0300 Subject: [PATCH 1/7] fix(agent): append system_prompt to the Claude Code preset instead of replacing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A plain-string ClaudeAgentOptions.system_prompt replaces Claude Code's entire default system prompt. Every experiment that sets even a one-line system_prompt silently strips the harness's behavioral guidance — observed in skills nightly runs as zero parallel tool calls (the batching instruction lives in the default prompt), heavy narration, and raw cat/sed over Read/Grep. Wrap the configured prompt in the SDK's claude_code preset with append so the default prompt survives. Co-Authored-By: Claude Fable 5 --- src/coder_eval/agents/claude_code_agent.py | 11 ++++++++- src/coder_eval/models/agent_config.py | 3 ++- tests/test_agent.py | 26 ++++++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/src/coder_eval/agents/claude_code_agent.py b/src/coder_eval/agents/claude_code_agent.py index 71cb2267..cfd5bc9a 100644 --- a/src/coder_eval/agents/claude_code_agent.py +++ b/src/coder_eval/agents/claude_code_agent.py @@ -20,6 +20,7 @@ TaskNotificationMessage, query, ) +from claude_agent_sdk.types import SystemPromptPreset # Private SDK import — the public `query()` API doesn't expose the subprocess # handle, but we need it to SIGKILL on timeout (the SDK's anyio task groups @@ -1173,6 +1174,14 @@ def _build_claude_query( if "ToolSearch" not in disallowed_tools: disallowed_tools.append("ToolSearch") + # A plain-string system_prompt would REPLACE Claude Code's default system + # prompt, dropping its behavioral guidance (parallel tool-call batching, + # conciseness). Always keep the default via the SDK preset and append the + # configured prompt after it. + system_prompt: SystemPromptPreset | None = None + if self.config.system_prompt is not None: + system_prompt = SystemPromptPreset(type="preset", preset="claude_code", append=self.config.system_prompt) + # as_posix(), not str(): bash on Windows strips backslashes from unquoted # paths, so a redirect like `> D:\foo\bar` ends up writing to "Dfoobar". options = ClaudeAgentOptions( @@ -1192,7 +1201,7 @@ def _build_claude_query( # summing per-message values undercounts by 10x+. Without this flag # StreamEvents are suppressed by the SDK. include_partial_messages=True, - system_prompt=self.config.system_prompt, + system_prompt=system_prompt, setting_sources=self.config.setting_sources if self.config.setting_sources is not None else ["project"], resume=self._session_id, settings=json.dumps(self.config.claude_settings) diff --git a/src/coder_eval/models/agent_config.py b/src/coder_eval/models/agent_config.py index b4ad98fd..b35d286d 100644 --- a/src/coder_eval/models/agent_config.py +++ b/src/coder_eval/models/agent_config.py @@ -151,7 +151,8 @@ class BaseAgentConfig(BaseModel): system_prompt: str | None = Field( default=None, description=( - "Custom system prompt. Replaces the default system prompt. " + "Custom system prompt, appended to the agent's default system prompt " + "(claude-code: the SDK 'claude_code' preset with append). " "Supports inline text or multi-line YAML strings. " "Mutually exclusive with system_prompt_file." ), diff --git a/tests/test_agent.py b/tests/test_agent.py index 2e4f7aaa..405d2996 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -377,6 +377,32 @@ async def test_claude_settings_none_default(): assert captured_options[0].settings is None +@pytest.mark.asyncio +async def test_system_prompt_appends_to_claude_code_preset(): + """system_prompt keeps the Claude Code default prompt and appends via the SDK preset.""" + config = parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt="You are a coding agent.") + agent = ClaudeCodeAgent(config) + + captured_options = await _capture_sdk_options(agent) + + assert captured_options[0].system_prompt == { + "type": "preset", + "preset": "claude_code", + "append": "You are a coding agent.", + } + + +@pytest.mark.asyncio +async def test_system_prompt_none_leaves_sdk_default(): + """No system_prompt -> ClaudeAgentOptions.system_prompt stays None (SDK default prompt).""" + config = parse_agent_config(type=AgentKind.CLAUDE_CODE) + agent = ClaudeCodeAgent(config) + + captured_options = await _capture_sdk_options(agent) + + assert captured_options[0].system_prompt is None + + @pytest.mark.asyncio async def test_sdk_options_forwarded_to_sdk(): """An sdk_options key (e.g. effort) is splatted into ClaudeAgentOptions.""" From f0668343cb7d300c5c68a34ed59a31b3a0bd190e Mon Sep 17 00:00:00 2001 From: Mihai Chirculescu Date: Fri, 7 Aug 2026 16:35:15 +0300 Subject: [PATCH 2/7] style: sort claude_agent_sdk.types import Co-Authored-By: Claude Fable 5 --- src/coder_eval/agents/claude_code_agent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coder_eval/agents/claude_code_agent.py b/src/coder_eval/agents/claude_code_agent.py index cfd5bc9a..208cc574 100644 --- a/src/coder_eval/agents/claude_code_agent.py +++ b/src/coder_eval/agents/claude_code_agent.py @@ -20,7 +20,6 @@ TaskNotificationMessage, query, ) -from claude_agent_sdk.types import SystemPromptPreset # Private SDK import — the public `query()` API doesn't expose the subprocess # handle, but we need it to SIGKILL on timeout (the SDK's anyio task groups @@ -28,6 +27,7 @@ # CLI). If this import breaks on an SDK upgrade, the threaded watchdog loses # its kill target and timeouts will no longer be enforced at the agent layer. from claude_agent_sdk._internal.transport.subprocess_cli import SubprocessCLITransport +from claude_agent_sdk.types import SystemPromptPreset from coder_eval.agent import Agent, AgentState from coder_eval.agents._logging import PrefixedAdapter, log_raw_sdk_event From b0113d36bb7782431f7e0a655b33cce9a7e8c19f Mon Sep 17 00:00:00 2001 From: Mihai Chirculescu Date: Sat, 8 Aug 2026 09:36:53 +0300 Subject: [PATCH 3/7] fix(agent): align Codex system_prompt with the append-only contract CodexAgent silently dropped config.system_prompt; forward it as developer_instructions (injected on top of the Codex base prompt) to match the append semantics of Claude Code (claude_code preset) and Antigravity (TemplatedSystemInstructions, which already appended). Also document the ripple effects of append-only system_prompt: - agent_judge: the reviewer prompt is now layered after the full Claude Code preset instead of replacing it (accepted trade-off, noted in code) - BaseAgentConfig.system_prompt description states per-agent semantics - docs: fix the stale "Replaces the default" claim in CLAUDE_CODE.md, add a System prompt row to CODEX.md, document Antigravity's append shorthand Co-Authored-By: Claude Fable 5 --- docs/agents/ANTIGRAVITY.md | 8 ++++++++ docs/agents/CLAUDE_CODE.md | 2 +- docs/agents/CODEX.md | 1 + src/coder_eval/agents/codex_agent.py | 8 ++++++++ src/coder_eval/criteria/agent_judge.py | 6 ++++++ src/coder_eval/models/agent_config.py | 5 +++-- tests/test_codex_agent.py | 16 ++++++++++++++++ 7 files changed, 43 insertions(+), 3 deletions(-) diff --git a/docs/agents/ANTIGRAVITY.md b/docs/agents/ANTIGRAVITY.md index 522dc8bf..ad96081b 100644 --- a/docs/agents/ANTIGRAVITY.md +++ b/docs/agents/ANTIGRAVITY.md @@ -109,6 +109,14 @@ Antigravity exposes a `thinking_level` field (`minimal` / `low` / `medium` / Antigravity-specific — Claude Code and Codex don't take this field. Thinking tokens are billed as **output** tokens (see [Telemetry](#telemetry)). +### `system_prompt` + +`agent.system_prompt` is passed to the SDK as `system_instructions`, whose string +shorthand maps to `TemplatedSystemInstructions` — a named section **appended** to +the harness's default system instructions, never a replacement. This matches the +append-only semantics of the shared config field across agents (Claude Code appends +via the `claude_code` preset; Codex via `developer_instructions`). + ### Skills (SKILL.md) Antigravity supports [Agent Skills](https://agentskills.io/specification) diff --git a/docs/agents/CLAUDE_CODE.md b/docs/agents/CLAUDE_CODE.md index 66670709..5614b499 100644 --- a/docs/agents/CLAUDE_CODE.md +++ b/docs/agents/CLAUDE_CODE.md @@ -99,7 +99,7 @@ agent: | `allowed_tools` | `list[str] \| null` | Tool allowlist. Unset ⇒ all tools allowed. | | `disallowed_tools` | `list[str] \| null` | Tool denylist. (`ToolSearch` is always appended for Bedrock parity.) | | `plugins` | `list[{type: local, path}]` | Local plugin/skill directories; `$VAR` in `path` is expanded and resolved to an absolute path. | -| `system_prompt` | `str \| null` | **Replaces** the default system prompt (there is no *append* seam). Mutually exclusive with `system_prompt_file`. | +| `system_prompt` | `str \| null` | **Appended** to the default Claude Code system prompt (via the SDK's `claude_code` preset with `append`) — the default's behavioral guidance is always kept. Mutually exclusive with `system_prompt_file`. | | `system_prompt_file` | `str \| null` | Path (relative to the task YAML) loaded into `system_prompt` at resolution. | | `setting_sources` | `list["user"\|"project"\|"local"] \| null` | Which host setting sources the SDK reads. Default resolves to `["project"]`. See [Sandbox isolation](#sandbox-isolation). | | `claude_settings` | `str \| dict \| null` | Passed to the SDK `--settings`. A dict is JSON-serialized; a str is a settings file path. Use `permissions.deny` to block tools/paths. | diff --git a/docs/agents/CODEX.md b/docs/agents/CODEX.md index b020e76b..7b6dc99f 100644 --- a/docs/agents/CODEX.md +++ b/docs/agents/CODEX.md @@ -213,6 +213,7 @@ The Codex SDK is synchronous. The agent uses `_run_async()` helper to detect and | **SDK Type** | Subprocess (CLI via JSON generator) | Sync client (app-server subprocess) | | **Command Tracking** | Full telemetry (tool name, params, duration) | Streamed telemetry: shell → `Bash`, apply_patch → `Write` | | **Model Selection** | Direct via `--model` or config | `agent.model` pinned into `thread_start` | +| **System prompt** | `system_prompt` appended to the default prompt (SDK `claude_code` preset) | `system_prompt` passed as `developer_instructions` on top of the Codex base prompt | | **Session Resume** | `--resume {session_id}` | Via thread ID | | **Permissions** | `permission_mode` + `allowed_tools` | `permission_mode` → sandbox/approval + `allowed_tools`/`disallowed_tools` → thread config | | **Tool Enforcement** | Not enforced by Coder Eval wrapper | `enabled_tools` honored; `disabled_tools` NOT enforced by the SDK | diff --git a/src/coder_eval/agents/codex_agent.py b/src/coder_eval/agents/codex_agent.py index c6e4b2d3..4b3af0d0 100644 --- a/src/coder_eval/agents/codex_agent.py +++ b/src/coder_eval/agents/codex_agent.py @@ -1276,6 +1276,14 @@ def _build_thread_options(self) -> dict[str, Any]: options["model"] = effective_model self._log.debug(f"Codex model pinned to {effective_model}") + # system_prompt maps to developer_instructions: injected ON TOP of Codex's + # base prompt, matching the append-only contract of the shared config field + # (Claude Code appends via the claude_code preset; Antigravity via + # TemplatedSystemInstructions). base_instructions (full replacement of the + # base prompt) is deliberately not exposed. + if self.config.system_prompt is not None: + options["developer_instructions"] = self.config.system_prompt + permission_mode = self.config.permission_mode.value approval_mode_str = _CODEX_APPROVAL_MODE diff --git a/src/coder_eval/criteria/agent_judge.py b/src/coder_eval/criteria/agent_judge.py index 20db8dab..3af1fea4 100644 --- a/src/coder_eval/criteria/agent_judge.py +++ b/src/coder_eval/criteria/agent_judge.py @@ -63,6 +63,12 @@ logger = logging.getLogger(__name__) +# system_prompt is append-only (see BaseAgentConfig.system_prompt), so this is +# layered AFTER the full Claude Code preset rather than replacing it: the judge +# carries the coding-agent identity plus this reviewer role, and pays the +# preset's prompt tokens on every call. Accepted trade-off — the preset's +# tool-usage guidance helps the investigation, and the submit_verdict contract +# below still governs the output. _SYSTEM_PROMPT = """\ You are a strict code reviewer evaluating a project generated by a coding agent. diff --git a/src/coder_eval/models/agent_config.py b/src/coder_eval/models/agent_config.py index b35d286d..8f389798 100644 --- a/src/coder_eval/models/agent_config.py +++ b/src/coder_eval/models/agent_config.py @@ -151,8 +151,9 @@ class BaseAgentConfig(BaseModel): system_prompt: str | None = Field( default=None, description=( - "Custom system prompt, appended to the agent's default system prompt " - "(claude-code: the SDK 'claude_code' preset with append). " + "Custom system prompt, appended to the agent's default system prompt — never a replacement " + "(claude-code: the SDK 'claude_code' preset with append; codex: developer_instructions " + "on top of the base prompt; antigravity: TemplatedSystemInstructions sections). " "Supports inline text or multi-line YAML strings. " "Mutually exclusive with system_prompt_file." ), diff --git a/tests/test_codex_agent.py b/tests/test_codex_agent.py index 95ee117f..f9f009dc 100644 --- a/tests/test_codex_agent.py +++ b/tests/test_codex_agent.py @@ -125,6 +125,22 @@ def test_sandbox_is_full_access(self, monkeypatch, mode, in_container, os_name): assert agent._build_thread_options()["sandbox"] == Sandbox("full-access") +class TestSystemPrompt: + """system_prompt travels as developer_instructions — injected on top of Codex's + base prompt, mirroring the append-only semantics of the other agents.""" + + def test_system_prompt_forwarded_as_developer_instructions(self): + agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX, system_prompt="You are a coding agent.")) + + assert agent._build_thread_options()["developer_instructions"] == "You are a coding agent." + + def test_no_system_prompt_omits_developer_instructions(self): + """No system_prompt -> the key is absent, leaving the SDK default untouched.""" + agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX)) + + assert "developer_instructions" not in agent._build_thread_options() + + class TestCodexEnvironmentConfiguration: """Test _build_codex_env: only CODEX_API_KEY travels via env.""" From 92ebce9b96ba612053bb734b23614bb9b2e6d871 Mon Sep 17 00:00:00 2001 From: Mihai Chirculescu Date: Sat, 8 Aug 2026 10:00:00 +0300 Subject: [PATCH 4/7] =?UTF-8?q?fix(agent):=20address=20PR=20review=20?= =?UTF-8?q?=E2=80=94=20unconditional=20preset,=20judge=20replace=20seam?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blockers from the PR #92 review: - system_prompt unset no longer loses the preset: the SDK maps None to --system-prompt "" (an explicit EMPTY prompt), so _build_options now always sends the claude_code preset — bare (CLI default prompt) when unset, with `append` when configured. This fixes the common no- system_prompt case, which previously ran without Claude Code's default behavioral guidance. - agent_judge no longer inherits the coding-agent preset: new ClaudeCodeAgentConfig.system_prompt_mode ("append" default / "replace"), forced to "replace" in _build_agent_config next to the existing security floors, so the judge prompt stays its entire identity and verdicts can't shift with the preset. Pinned by test. - exclude_dynamic_sections=True on the preset keeps the system prompt static across runs (no per-run tempdir path baked in); the SDK re-injects the stripped sections into the first user message. - Transport-level tests: captured options are rendered through SubprocessCLITransport._build_command() asserting the exact flag emitted (--append-system-prompt vs --system-prompt vs none) — the surface the original bug lived on. Also pins system_prompt: "" and the renamed unset-case test (the old name asserted a false SDK contract). - BaseAgentConfig.system_prompt description is agent-neutral again; the claude-specific mechanism lives on ClaudeCodeAgentConfig + docs/agents/. MIGRATION NOTE: system_prompt semantics on claude-code changed from replace to append, and runs WITHOUT system_prompt now get the real Claude Code default prompt instead of an empty one. Scores are comparable only within one semantics regime — re-baseline judged tasks (e.g. tasks/python_cli_simulated_judged/echo_simulated_judged.yaml, whose prompt was written against replace semantics) and pin runs to the CLI version recorded in environment_info.claude_code_cli. Co-Authored-By: Claude Fable 5 --- docs/agents/CLAUDE_CODE.md | 12 ++++- src/coder_eval/agents/claude_code_agent.py | 28 +++++++--- src/coder_eval/criteria/agent_judge.py | 15 +++--- src/coder_eval/models/agent_config.py | 14 +++-- tests/test_agent.py | 61 ++++++++++++++++++++-- tests/test_agent_judge_criterion.py | 16 ++++++ 6 files changed, 126 insertions(+), 20 deletions(-) diff --git a/docs/agents/CLAUDE_CODE.md b/docs/agents/CLAUDE_CODE.md index 5614b499..c752f15b 100644 --- a/docs/agents/CLAUDE_CODE.md +++ b/docs/agents/CLAUDE_CODE.md @@ -99,7 +99,8 @@ agent: | `allowed_tools` | `list[str] \| null` | Tool allowlist. Unset ⇒ all tools allowed. | | `disallowed_tools` | `list[str] \| null` | Tool denylist. (`ToolSearch` is always appended for Bedrock parity.) | | `plugins` | `list[{type: local, path}]` | Local plugin/skill directories; `$VAR` in `path` is expanded and resolved to an absolute path. | -| `system_prompt` | `str \| null` | **Appended** to the default Claude Code system prompt (via the SDK's `claude_code` preset with `append`) — the default's behavioral guidance is always kept. Mutually exclusive with `system_prompt_file`. | +| `system_prompt` | `str \| null` | **Appended** to the default Claude Code system prompt (via the SDK's `claude_code` preset) — the default's behavioral guidance is always kept, whether or not this is set. Mutually exclusive with `system_prompt_file`. | +| `system_prompt_mode` | `"append"` (default) / `"replace"` | `replace` sends `system_prompt` as the **entire** system prompt (no preset). Used by judge sub-agents, which must not carry the coding-agent persona; rarely needed in tasks. | | `system_prompt_file` | `str \| null` | Path (relative to the task YAML) loaded into `system_prompt` at resolution. | | `setting_sources` | `list["user"\|"project"\|"local"] \| null` | Which host setting sources the SDK reads. Default resolves to `["project"]`. See [Sandbox isolation](#sandbox-isolation). | | `claude_settings` | `str \| dict \| null` | Passed to the SDK `--settings`. A dict is JSON-serialized; a str is a settings file path. Use `permissions.deny` to block tools/paths. | @@ -111,6 +112,15 @@ agent: > `setting_sources`, `include_partial_messages`, …) are rejected there — set those > through their typed fields or `-D run_limits.*`. MCP servers are not a YAML field. +> **System-prompt reproducibility.** In `append` mode the preset's *dynamic +> sections* (working directory, git status, auto-memory) are excluded so the system +> prompt stays identical across runs — the per-run sandbox tempdir path would +> otherwise be baked into it, breaking prompt caching and run comparability. The +> SDK re-injects the stripped content into the first user message, so the agent +> loses nothing. Note the default-prompt baseline tracks the installed Claude Code +> CLI version; `environment_info.claude_code_cli` in `run.json` records which +> version a run used. + ### Setting fields from the CLI Any of these merge-resolve through `-D` / `--set` (see diff --git a/src/coder_eval/agents/claude_code_agent.py b/src/coder_eval/agents/claude_code_agent.py index 208cc574..6dccda46 100644 --- a/src/coder_eval/agents/claude_code_agent.py +++ b/src/coder_eval/agents/claude_code_agent.py @@ -27,6 +27,9 @@ # CLI). If this import breaks on an SDK upgrade, the threaded watchdog loses # its kill target and timeouts will no longer be enforced at the agent layer. from claude_agent_sdk._internal.transport.subprocess_cli import SubprocessCLITransport + +# SystemPromptPreset is not re-exported from the SDK root, so claude_agent_sdk.types +# is the only import route (same treatment as evaluation/verdict_tool.py). from claude_agent_sdk.types import SystemPromptPreset from coder_eval.agent import Agent, AgentState @@ -1174,13 +1177,24 @@ def _build_claude_query( if "ToolSearch" not in disallowed_tools: disallowed_tools.append("ToolSearch") - # A plain-string system_prompt would REPLACE Claude Code's default system - # prompt, dropping its behavioral guidance (parallel tool-call batching, - # conciseness). Always keep the default via the SDK preset and append the - # configured prompt after it. - system_prompt: SystemPromptPreset | None = None - if self.config.system_prompt is not None: - system_prompt = SystemPromptPreset(type="preset", preset="claude_code", append=self.config.system_prompt) + # The SDK maps system_prompt=None to `--system-prompt ""` (an explicit + # EMPTY custom prompt) and a plain string to a full replacement — either + # way Claude Code's default behavioral guidance (parallel tool-call + # batching, conciseness) is lost. So ALWAYS send the claude_code preset: + # without `append` the CLI runs its default prompt; with it the configured + # prompt is appended. exclude_dynamic_sections keeps the prompt static + # across runs (the per-run tempdir path would otherwise be baked into the + # system prompt, breaking prompt caching and run comparability); the SDK + # re-injects the stripped sections into the first user message. + # system_prompt_mode="replace" (judge sub-agents) opts out of the preset: + # the configured prompt IS the entire system prompt. + system_prompt: str | SystemPromptPreset + if self.config.system_prompt_mode == "replace" and self.config.system_prompt is not None: + system_prompt = self.config.system_prompt + else: + system_prompt = SystemPromptPreset(type="preset", preset="claude_code", exclude_dynamic_sections=True) + if self.config.system_prompt is not None: + system_prompt["append"] = self.config.system_prompt # as_posix(), not str(): bash on Windows strips backslashes from unquoted # paths, so a redirect like `> D:\foo\bar` ends up writing to "Dfoobar". diff --git a/src/coder_eval/criteria/agent_judge.py b/src/coder_eval/criteria/agent_judge.py index 3af1fea4..bbbc6e48 100644 --- a/src/coder_eval/criteria/agent_judge.py +++ b/src/coder_eval/criteria/agent_judge.py @@ -63,12 +63,11 @@ logger = logging.getLogger(__name__) -# system_prompt is append-only (see BaseAgentConfig.system_prompt), so this is -# layered AFTER the full Claude Code preset rather than replacing it: the judge -# carries the coding-agent identity plus this reviewer role, and pays the -# preset's prompt tokens on every call. Accepted trade-off — the preset's -# tool-usage guidance helps the investigation, and the submit_verdict contract -# below still governs the output. +# This is the judge's ENTIRE identity: _build_agent_config forces +# system_prompt_mode="replace" so the Claude Code coding-agent preset never +# reaches the scoring instrument — the judge must not carry an engineering +# persona (terse, proactively edits files) ahead of its grading role, and its +# verdicts must not shift when the preset does. _SYSTEM_PROMPT = """\ You are a strict code reviewer evaluating a project generated by a coding agent. @@ -269,6 +268,10 @@ def _build_agent_config( user_overrides["sdk_options"] = {**defaults.sdk_options, **user_overrides["sdk_options"]} config = defaults.model_copy(update=user_overrides, deep=True) config.system_prompt = system_prompt + # Force replace regardless of user YAML: the judge prompt is its entire + # identity — the coding-agent preset must never prefix the scoring + # instrument (see the note on _SYSTEM_PROMPT). + config.system_prompt_mode = "replace" # SECURITY: force setting_sources=[] regardless of user YAML so the SDK # does NOT load .claude/settings.json or .mcp.json from the judge's cwd. # Those files can install pre-LLM lifecycle hooks (SessionStart / diff --git a/src/coder_eval/models/agent_config.py b/src/coder_eval/models/agent_config.py index 8f389798..721997c1 100644 --- a/src/coder_eval/models/agent_config.py +++ b/src/coder_eval/models/agent_config.py @@ -151,9 +151,8 @@ class BaseAgentConfig(BaseModel): system_prompt: str | None = Field( default=None, description=( - "Custom system prompt, appended to the agent's default system prompt — never a replacement " - "(claude-code: the SDK 'claude_code' preset with append; codex: developer_instructions " - "on top of the base prompt; antigravity: TemplatedSystemInstructions sections). " + "Custom system prompt, appended to the agent's default system prompt — never a replacement. " + "Each agent's doc page (docs/agents/) states the exact mechanism. " "Supports inline text or multi-line YAML strings. " "Mutually exclusive with system_prompt_file." ), @@ -199,6 +198,15 @@ class ClaudeCodeAgentConfig(BaseAgentConfig): type: Literal[AgentKind.CLAUDE_CODE] # type: ignore[assignment] + system_prompt_mode: Literal["append", "replace"] = Field( + default="append", + description=( + "How system_prompt combines with the Claude Code default prompt: 'append' layers it " + "after the SDK 'claude_code' preset, keeping the default's behavioral guidance; " + "'replace' sends it as the ENTIRE system prompt. Judge sub-agents force 'replace' so " + "the scoring instrument never carries the coding-agent persona." + ), + ) claude_settings: str | dict[str, Any] | None = MergeField( strategy="deep", default=None, diff --git a/tests/test_agent.py b/tests/test_agent.py index 405d2996..59aa019e 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -377,6 +377,19 @@ async def test_claude_settings_none_default(): assert captured_options[0].settings is None +def _transport_command(options) -> list[str]: + """Render captured ClaudeAgentOptions into the actual CLI argv. + + The dict-shape assertions pin the values we set; this pins the SDK contract + (which flag the transport emits) — the surface the original replace-vs-append + bug lived on — and survives an SDK TypedDict reshape. + """ + from claude_agent_sdk._internal.transport.subprocess_cli import SubprocessCLITransport + + options.cli_path = "claude" + return SubprocessCLITransport(prompt="x", options=options)._build_command() + + @pytest.mark.asyncio async def test_system_prompt_appends_to_claude_code_preset(): """system_prompt keeps the Claude Code default prompt and appends via the SDK preset.""" @@ -388,19 +401,61 @@ async def test_system_prompt_appends_to_claude_code_preset(): assert captured_options[0].system_prompt == { "type": "preset", "preset": "claude_code", + "exclude_dynamic_sections": True, "append": "You are a coding agent.", } + cmd = _transport_command(captured_options[0]) + assert "--append-system-prompt" in cmd + assert "--system-prompt" not in cmd @pytest.mark.asyncio -async def test_system_prompt_none_leaves_sdk_default(): - """No system_prompt -> ClaudeAgentOptions.system_prompt stays None (SDK default prompt).""" +async def test_system_prompt_unset_sends_bare_preset(): + """No system_prompt -> the bare claude_code preset, which the transport renders + as NO system-prompt flag (the CLI default). Passing None instead would emit + `--system-prompt \"\"` — an explicit EMPTY prompt that loses the default.""" config = parse_agent_config(type=AgentKind.CLAUDE_CODE) agent = ClaudeCodeAgent(config) captured_options = await _capture_sdk_options(agent) - assert captured_options[0].system_prompt is None + assert captured_options[0].system_prompt == { + "type": "preset", + "preset": "claude_code", + "exclude_dynamic_sections": True, + } + cmd = _transport_command(captured_options[0]) + assert "--append-system-prompt" not in cmd + assert "--system-prompt" not in cmd + + +@pytest.mark.asyncio +async def test_system_prompt_empty_string_appends_empty(): + """system_prompt: \"\" is configured, not unset — it appends (harmlessly), and a + future truthiness refactor must not route it into the preset-loss path.""" + config = parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt="") + agent = ClaudeCodeAgent(config) + + captured_options = await _capture_sdk_options(agent) + + assert captured_options[0].system_prompt["append"] == "" + + +@pytest.mark.asyncio +async def test_system_prompt_mode_replace_sends_plain_string(): + """system_prompt_mode='replace' (the judge seam) sends the configured prompt as + the ENTIRE system prompt — no preset, no coding-agent persona.""" + config = parse_agent_config( + type=AgentKind.CLAUDE_CODE, system_prompt="You are a strict grader.", system_prompt_mode="replace" + ) + agent = ClaudeCodeAgent(config) + + captured_options = await _capture_sdk_options(agent) + + assert captured_options[0].system_prompt == "You are a strict grader." + cmd = _transport_command(captured_options[0]) + assert "--system-prompt" in cmd + assert "--append-system-prompt" not in cmd @pytest.mark.asyncio diff --git a/tests/test_agent_judge_criterion.py b/tests/test_agent_judge_criterion.py index cfd6b720..593764bc 100644 --- a/tests/test_agent_judge_criterion.py +++ b/tests/test_agent_judge_criterion.py @@ -715,6 +715,22 @@ def test_agent_judge_prompt_requires_findings(sandbox: Sandbox, direct_route: Di assert "findings" in user_msg.lower() +def test_agent_judge_system_prompt_replaces_not_appends(sandbox: Sandbox, direct_route: DirectRoute) -> None: + """The judge prompt is its ENTIRE identity: system_prompt_mode must be 'replace' + so the Claude Code coding-agent preset never prefixes the scoring instrument — + forced even when the user's YAML says 'append'.""" + criterion = AgentJudgeCriterion( + description="x", prompt="grade", agent={"type": "claude-code", "system_prompt_mode": "append"} + ) + mock_agent = _make_mock_agent('{"score": 0.5, "rationale": "ok"}') + with patch(_AGENT_PATCH_PATH, return_value=mock_agent) as mock_cls: + SuccessChecker(sandbox, init_registry=False, route=direct_route).check(criterion) + + (agent_config,) = mock_cls.call_args.args + assert agent_config.system_prompt_mode == "replace" + assert agent_config.system_prompt.startswith("You are a strict code reviewer") + + def test_agent_judge_transcript_captures_tool_calls(sandbox: Sandbox, direct_route: DirectRoute) -> None: """Tool calls made by the judge sub-agent must surface on the transcript so reviewers can audit the verdict.""" From 3083fc87b1773921ac1a0339a0a41ec10b433b53 Mon Sep 17 00:00:00 2001 From: Mihai Chirculescu Date: Sat, 8 Aug 2026 10:19:50 +0300 Subject: [PATCH 5/7] feat(agent): record system_prompt_semantics marker in environment_info Trend dashboards need to segment runs by system-prompt regime instead of silently pooling pre-/post-append-semantics scores (PR #92 review, cross-run comparability blocker). Each built-in agent now emits system_prompt_semantics via get_environment_info(), merged into run.json: - claude-code: the resolved system_prompt_mode ("append" / "replace") - codex: "append" (developer_instructions; previously the field was silently dropped, so codex runs also cross a semantics boundary here) - antigravity: "append" (unchanged behavior, emitted for uniformity) Runs without the marker predate the change and used replace-on-set / empty-on-unset (claude-code) or dropped (codex) semantics. Co-Authored-By: Claude Fable 5 --- docs/agents/CLAUDE_CODE.md | 4 +++- src/coder_eval/agents/antigravity_agent.py | 3 +++ src/coder_eval/agents/claude_code_agent.py | 11 +++++++++++ src/coder_eval/agents/codex_agent.py | 8 +++++++- tests/test_agent.py | 13 +++++++++++++ tests/test_antigravity_agent.py | 8 ++++++++ tests/test_codex_agent.py | 7 +++++-- 7 files changed, 50 insertions(+), 4 deletions(-) diff --git a/docs/agents/CLAUDE_CODE.md b/docs/agents/CLAUDE_CODE.md index c752f15b..8b7aab0d 100644 --- a/docs/agents/CLAUDE_CODE.md +++ b/docs/agents/CLAUDE_CODE.md @@ -119,7 +119,9 @@ agent: > SDK re-injects the stripped content into the first user message, so the agent > loses nothing. Note the default-prompt baseline tracks the installed Claude Code > CLI version; `environment_info.claude_code_cli` in `run.json` records which -> version a run used. +> version a run used, and `environment_info.system_prompt_semantics` +> (`append` / `replace`) records the prompt regime — runs predating that marker +> used replace-on-set / empty-on-unset semantics and are not score-comparable. ### Setting fields from the CLI diff --git a/src/coder_eval/agents/antigravity_agent.py b/src/coder_eval/agents/antigravity_agent.py index 4e1475be..9c1d58ce 100644 --- a/src/coder_eval/agents/antigravity_agent.py +++ b/src/coder_eval/agents/antigravity_agent.py @@ -561,6 +561,9 @@ def get_environment_info(self) -> dict[str, Any]: return { "antigravity_model": self._effective_model(), "antigravity_thinking_level": self.config.thinking_level, + # Antigravity has always appended (TemplatedSystemInstructions); + # emitted for cross-agent uniformity of the marker. + "system_prompt_semantics": "append", } def _conversation_or_none(self) -> Any: diff --git a/src/coder_eval/agents/claude_code_agent.py b/src/coder_eval/agents/claude_code_agent.py index 6dccda46..3671fa7f 100644 --- a/src/coder_eval/agents/claude_code_agent.py +++ b/src/coder_eval/agents/claude_code_agent.py @@ -1239,6 +1239,17 @@ def _build_claude_query( return options, transport, effective_model + def get_environment_info(self) -> dict[str, Any]: + """Record which system-prompt regime built this run's prompts. + + ``append`` = the claude_code preset (dynamic sections excluded) with the + configured system_prompt, if any, appended; ``replace`` = the configured + prompt is the ENTIRE system prompt (judge sub-agents). Runs from before + this marker existed used replace-on-set / empty-on-unset semantics — + trend dashboards must not pool scores across that boundary. + """ + return {"system_prompt_semantics": self.config.system_prompt_mode} + async def stop(self) -> None: """Stop the agent and clean up resources.""" self.client = None diff --git a/src/coder_eval/agents/codex_agent.py b/src/coder_eval/agents/codex_agent.py index 4b3af0d0..e3f40f7d 100644 --- a/src/coder_eval/agents/codex_agent.py +++ b/src/coder_eval/agents/codex_agent.py @@ -944,10 +944,16 @@ def get_environment_info(self) -> dict[str, Any]: recorded to avoid leaking any embedded credentials; the API key is never recorded. """ + # system_prompt_semantics: Codex appends system_prompt as + # developer_instructions on top of its base prompt. Runs from before this + # marker existed silently DROPPED the field — dashboards must not pool + # system_prompt-setting tasks across that boundary. + info: dict[str, Any] = {"system_prompt_semantics": "append"} base_url = self._resolve_base_url() if not base_url: - return {} + return info return { + **info, "codex_base_url_host": urlparse(base_url).hostname or "", "codex_wire_api": _CODEX_WIRE_API, "codex_api_version": self._resolve_api_version() or "", diff --git a/tests/test_agent.py b/tests/test_agent.py index 59aa019e..1005a8e9 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -458,6 +458,19 @@ async def test_system_prompt_mode_replace_sends_plain_string(): assert "--append-system-prompt" not in cmd +def test_environment_info_reports_system_prompt_semantics(): + """The resolved system_prompt_mode lands in run.json (environment_info) so + trend dashboards can segment runs by prompt regime instead of pooling + pre-/post-append-semantics scores.""" + default_agent = ClaudeCodeAgent(parse_agent_config(type=AgentKind.CLAUDE_CODE)) + assert default_agent.get_environment_info() == {"system_prompt_semantics": "append"} + + judge_like = ClaudeCodeAgent( + parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt="grader", system_prompt_mode="replace") + ) + assert judge_like.get_environment_info() == {"system_prompt_semantics": "replace"} + + @pytest.mark.asyncio async def test_sdk_options_forwarded_to_sdk(): """An sdk_options key (e.g. effort) is splatted into ClaudeAgentOptions.""" diff --git a/tests/test_antigravity_agent.py b/tests/test_antigravity_agent.py index 99747950..0f989b07 100644 --- a/tests/test_antigravity_agent.py +++ b/tests/test_antigravity_agent.py @@ -64,6 +64,14 @@ def test_effective_model_prefers_config_then_default(): assert unpinned._effective_model() == _DEFAULT_MODEL +def test_environment_info_reports_append_prompt_semantics(): + """Antigravity always appends system_prompt (TemplatedSystemInstructions); + the cross-agent marker in run.json records that regime.""" + agent = AntigravityAgent(parse_agent_config(type="antigravity")) + + assert agent.get_environment_info()["system_prompt_semantics"] == "append" + + def _make_skill(parent, name: str) -> None: d = parent / name d.mkdir(parents=True) diff --git a/tests/test_codex_agent.py b/tests/test_codex_agent.py index f9f009dc..33690b0a 100644 --- a/tests/test_codex_agent.py +++ b/tests/test_codex_agent.py @@ -331,10 +331,12 @@ def test_empty_api_version_falls_back(self, monkeypatch): class TestCodexEnvironmentInfo: """get_environment_info surfaces resolved custom-endpoint routing for run artifacts.""" - def test_no_base_url_emits_nothing(self, monkeypatch): + def test_no_base_url_emits_only_prompt_semantics(self, monkeypatch): + """Without a custom endpoint, only the cross-agent system-prompt marker is + emitted (Codex appends system_prompt as developer_instructions).""" monkeypatch.delenv("CODEX_BASE_URL", raising=False) agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX, model="gpt-5-codex")) - assert agent.get_environment_info() == {} + assert agent.get_environment_info() == {"system_prompt_semantics": "append"} def test_azure_routing_recorded(self, monkeypatch): """Host (not full URL), wire_api, api-version, and the deployment-name marker @@ -345,6 +347,7 @@ def test_azure_routing_recorded(self, monkeypatch): agent = CodexAgent(parse_agent_config(type=AgentKind.CODEX, model="my-gpt5-deployment")) info = agent.get_environment_info() assert info == { + "system_prompt_semantics": "append", "codex_base_url_host": "my-res.openai.azure.com", "codex_wire_api": "responses", "codex_api_version": "2025-04-01-preview", From 0e444f003da9f59ac1cdd8231f4bf6d082b98b82 Mon Sep 17 00:00:00 2001 From: Mihai Chirculescu Date: Mon, 10 Aug 2026 11:21:18 +0300 Subject: [PATCH 6/7] =?UTF-8?q?fix(agent):=20address=20PR=20review=20?= =?UTF-8?q?=E2=80=94=20simulator=20replace=20mode,=20preset-aware=20report?= =?UTF-8?q?s,=20replace=20validator?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blockers: - UserSimulator now sets system_prompt_mode="replace" so the roleplay persona is the simulator's ENTIRE system prompt (the claude_code coding-agent preset no longer prefixes it on dialog-mode runs), and SubAgentRunner fail-louds on any identity-prompt config left in append mode (mirrors the setting_sources guard). - reports.collect_agent_settings_rows understands the persisted SystemPromptPreset dict: renders the appended prompt text (never the dict repr), omits the row for a bare preset, and surfaces a "System Prompt Mode: replace" row for plain strings. REPORT_SCHEMA.md documents the sdk_options.system_prompt type change and the environment_info.system_prompt_semantics segmentation rule. Non-blocking: - ClaudeCodeAgentConfig rejects system_prompt_mode="replace" with no system_prompt/system_prompt_file; _effective_prompt_mode() is the single source of truth for both the options builder and the system_prompt_semantics marker, so run.json can never disagree with the wire. - Softened the false "never a replacement" prose on BaseAgentConfig.system_prompt. Nits: - Antigravity forwards system_prompt verbatim ("" no longer dropped by `or None`), matching Claude Code / Codex `is not None` semantics. - Corrected the stale Codex get_environment_info docstring and the "always kept" row in docs/agents/CLAUDE_CODE.md; AB_EXPERIMENTS.md lists system_prompt_mode as a variant lever. - Typed the test helpers (_transport_command / _capture_sdk_options), asserted the whole preset dict in the empty-string test, narrowed _transport_command's docstring to argv-pinning only. Tests: simulator replace-mode assertion, SubAgentRunner guard, replace-with-unset-prompt validator case, environment_info merge survival at the orchestrator seam, and report tests fed from a real dump_dataclass(ClaudeAgentOptions). Co-Authored-By: Claude Fable 5 --- docs/AB_EXPERIMENTS.md | 4 +- docs/REPORT_SCHEMA.md | 8 +++ docs/agents/CLAUDE_CODE.md | 4 +- src/coder_eval/agents/antigravity_agent.py | 5 +- src/coder_eval/agents/claude_code_agent.py | 20 +++++- src/coder_eval/agents/codex_agent.py | 14 ++-- src/coder_eval/evaluation/sub_agent.py | 11 +++ src/coder_eval/models/agent_config.py | 20 +++++- src/coder_eval/reports.py | 25 ++++++- src/coder_eval/simulation/user_simulator.py | 6 ++ tests/test_agent.py | 41 ++++++++--- tests/test_orchestrator.py | 9 ++- tests/test_reports.py | 78 +++++++++++++++++++++ tests/test_sub_agent_runner.py | 22 ++++++ tests/test_user_simulator.py | 13 ++++ 15 files changed, 254 insertions(+), 26 deletions(-) diff --git a/docs/AB_EXPERIMENTS.md b/docs/AB_EXPERIMENTS.md index 040d9b97..3c7612e5 100644 --- a/docs/AB_EXPERIMENTS.md +++ b/docs/AB_EXPERIMENTS.md @@ -133,8 +133,8 @@ From `ExperimentVariant` (`coder_eval/models/experiment.py`): The `agent` dict is the lever for most A/B tests. Anything on `AgentConfig` is fair game: `model`, `permission_mode`, `allowed_tools`, `disallowed_tools`, -`plugins`, `system_prompt` / `system_prompt_file`, `setting_sources`, -`claude_settings`, `sdk_options`. +`plugins`, `system_prompt` / `system_prompt_file`, `system_prompt_mode` +(append-vs-replace arms), `setting_sources`, `claude_settings`, `sdk_options`. > **Path-resolution gotcha.** Relative file paths in variant config resolve > against _different_ base directories depending on the field: diff --git a/docs/REPORT_SCHEMA.md b/docs/REPORT_SCHEMA.md index df4bf740..53f1469d 100644 --- a/docs/REPORT_SCHEMA.md +++ b/docs/REPORT_SCHEMA.md @@ -137,6 +137,14 @@ The authoritative per-replicate record. `ClaudeAgentOptions` dump), `sandbox_path`, `task_config` (`{resolved, source_yaml, source_file, lineage}` — `lineage` maps each field to `{value, source, source_detail}` so you can trace which config layer set it). +`environment_info.system_prompt_semantics` (`"append"` / `"replace"`) records the +system-prompt regime the agent ran with; runs predating the marker used +replace-on-set / empty-on-unset semantics and are not score-comparable, so +consumers should segment on it (absent key ⇒ pre-append regime). +`sdk_options.system_prompt` is a `SystemPromptPreset` dict +(`{type: "preset", preset: "claude_code", exclude_dynamic_sections: true, append?: str}`) +on append-mode Claude Code runs and a plain string only in replace mode — it is +no longer `str | null`, so consumers must not string-handle it unconditionally. **Telemetry/totals:** `total_token_usage` ([TokenUsage](#tokenusage)), `command_stats` (`CommandStatistics`), `total_assistant_turns`, `expected_commands` / diff --git a/docs/agents/CLAUDE_CODE.md b/docs/agents/CLAUDE_CODE.md index 8b7aab0d..a84a7ccd 100644 --- a/docs/agents/CLAUDE_CODE.md +++ b/docs/agents/CLAUDE_CODE.md @@ -99,8 +99,8 @@ agent: | `allowed_tools` | `list[str] \| null` | Tool allowlist. Unset ⇒ all tools allowed. | | `disallowed_tools` | `list[str] \| null` | Tool denylist. (`ToolSearch` is always appended for Bedrock parity.) | | `plugins` | `list[{type: local, path}]` | Local plugin/skill directories; `$VAR` in `path` is expanded and resolved to an absolute path. | -| `system_prompt` | `str \| null` | **Appended** to the default Claude Code system prompt (via the SDK's `claude_code` preset) — the default's behavioral guidance is always kept, whether or not this is set. Mutually exclusive with `system_prompt_file`. | -| `system_prompt_mode` | `"append"` (default) / `"replace"` | `replace` sends `system_prompt` as the **entire** system prompt (no preset). Used by judge sub-agents, which must not carry the coding-agent persona; rarely needed in tasks. | +| `system_prompt` | `str \| null` | **Appended** to the default Claude Code system prompt (via the SDK's `claude_code` preset) — the default's behavioral guidance is kept unless `system_prompt_mode: replace` opts out. Mutually exclusive with `system_prompt_file`. | +| `system_prompt_mode` | `"append"` (default) / `"replace"` | `replace` sends `system_prompt` as the **entire** system prompt (no preset) and requires `system_prompt` / `system_prompt_file` to be set (validated at load). Used by judge sub-agents and the user simulator, which must not carry the coding-agent persona; rarely needed in tasks. | | `system_prompt_file` | `str \| null` | Path (relative to the task YAML) loaded into `system_prompt` at resolution. | | `setting_sources` | `list["user"\|"project"\|"local"] \| null` | Which host setting sources the SDK reads. Default resolves to `["project"]`. See [Sandbox isolation](#sandbox-isolation). | | `claude_settings` | `str \| dict \| null` | Passed to the SDK `--settings`. A dict is JSON-serialized; a str is a settings file path. Use `permissions.deny` to block tools/paths. | diff --git a/src/coder_eval/agents/antigravity_agent.py b/src/coder_eval/agents/antigravity_agent.py index 9c1d58ce..84bfcca7 100644 --- a/src/coder_eval/agents/antigravity_agent.py +++ b/src/coder_eval/agents/antigravity_agent.py @@ -348,7 +348,10 @@ async def start( # Autonomous execution: approve every tool call (incl. run_command), # which the default LocalAgentConfig policy would otherwise deny. policies=[policy.allow_all()], - system_instructions=self.config.system_prompt or None, + # Forward verbatim (None stays None): an explicit "" is a + # configured-but-empty prompt and must be forwarded, matching + # the `is not None` semantics in the Claude Code / Codex agents. + system_instructions=self.config.system_prompt, # Skill discovery: hand the harness the search-path roots that parent # the UiPath skill dirs. Unlike Codex (which symlinks into # .agents/skills/), Antigravity takes skill search paths natively. diff --git a/src/coder_eval/agents/claude_code_agent.py b/src/coder_eval/agents/claude_code_agent.py index 3671fa7f..f0d94425 100644 --- a/src/coder_eval/agents/claude_code_agent.py +++ b/src/coder_eval/agents/claude_code_agent.py @@ -10,7 +10,7 @@ from contextlib import suppress from datetime import datetime, timedelta from pathlib import Path -from typing import Any, ClassVar +from typing import Any, ClassVar, Literal from claude_agent_sdk import ( ClaudeAgentOptions, @@ -1189,7 +1189,8 @@ def _build_claude_query( # system_prompt_mode="replace" (judge sub-agents) opts out of the preset: # the configured prompt IS the entire system prompt. system_prompt: str | SystemPromptPreset - if self.config.system_prompt_mode == "replace" and self.config.system_prompt is not None: + if self._effective_prompt_mode() == "replace": + assert self.config.system_prompt is not None # guaranteed by _effective_prompt_mode system_prompt = self.config.system_prompt else: system_prompt = SystemPromptPreset(type="preset", preset="claude_code", exclude_dynamic_sections=True) @@ -1239,6 +1240,19 @@ def _build_claude_query( return options, transport, effective_model + def _effective_prompt_mode(self) -> Literal["append", "replace"]: + """The system-prompt regime that actually goes on the wire. + + Single source of truth for both the options builder and the + ``system_prompt_semantics`` run-record marker, so the persisted regime + can never disagree with what was sent. ``replace`` requires a configured + prompt (the config validator rejects the pair at load, but a mutated or + hand-built config falls back to the preset here — fail open to append). + """ + if self.config.system_prompt_mode == "replace" and self.config.system_prompt is not None: + return "replace" + return "append" + def get_environment_info(self) -> dict[str, Any]: """Record which system-prompt regime built this run's prompts. @@ -1248,7 +1262,7 @@ def get_environment_info(self) -> dict[str, Any]: this marker existed used replace-on-set / empty-on-unset semantics — trend dashboards must not pool scores across that boundary. """ - return {"system_prompt_semantics": self.config.system_prompt_mode} + return {"system_prompt_semantics": self._effective_prompt_mode()} async def stop(self) -> None: """Stop the agent and clean up resources.""" diff --git a/src/coder_eval/agents/codex_agent.py b/src/coder_eval/agents/codex_agent.py index e3f40f7d..2a33441b 100644 --- a/src/coder_eval/agents/codex_agent.py +++ b/src/coder_eval/agents/codex_agent.py @@ -937,12 +937,14 @@ def _close_client(self) -> None: def get_environment_info(self) -> dict[str, Any]: """Record the resolved Codex routing so runs are auditable/comparable. - Only emits when a custom endpoint is configured (CODEX_BASE_URL). On a - custom endpoint the model is an operator-chosen alias (a deployment name - on Azure), so two operators' ``gpt-5-codex`` deployments are otherwise - indistinguishable in run artifacts. The host (not the full URL) is - recorded to avoid leaking any embedded credentials; the API key is never - recorded. + Always emits ``system_prompt_semantics``. The routing keys + (``codex_base_url_host`` / ``codex_wire_api`` / ``codex_api_version`` / + ``codex_model_is_deployment``) are added only when a custom endpoint is + configured (CODEX_BASE_URL): on a custom endpoint the model is an + operator-chosen alias (a deployment name on Azure), so two operators' + ``gpt-5-codex`` deployments are otherwise indistinguishable in run + artifacts. The host (not the full URL) is recorded to avoid leaking any + embedded credentials; the API key is never recorded. """ # system_prompt_semantics: Codex appends system_prompt as # developer_instructions on top of its base prompt. Runs from before this diff --git a/src/coder_eval/evaluation/sub_agent.py b/src/coder_eval/evaluation/sub_agent.py index 88d1bc2e..3e82facc 100644 --- a/src/coder_eval/evaluation/sub_agent.py +++ b/src/coder_eval/evaluation/sub_agent.py @@ -87,6 +87,17 @@ def __init__( "SubAgentRunner requires agent_config.setting_sources=[] so the SDK does not " + "load .claude/settings.json or .mcp.json from the sub-agent's working directory." ) + # A sub-agent's system_prompt is its entire identity (judge instructions, + # simulator persona) — the claude_code coding-agent preset must never + # prefix it. Same fail-loud contract as setting_sources above: callers + # own their config, so a misconfigured one raises instead of being + # silently mutated. + if agent_config.system_prompt is not None and agent_config.system_prompt_mode != "replace": + raise ValueError( + "SubAgentRunner requires agent_config.system_prompt_mode='replace' when a " + + "system_prompt is set: the sub-agent prompt is its entire identity and must " + + "not be appended to the claude_code coding-agent preset." + ) assert sandbox.sandbox_dir is not None, "sandbox not initialized" self._sandbox = sandbox self._agent_config = agent_config diff --git a/src/coder_eval/models/agent_config.py b/src/coder_eval/models/agent_config.py index 721997c1..c7eda5a2 100644 --- a/src/coder_eval/models/agent_config.py +++ b/src/coder_eval/models/agent_config.py @@ -151,7 +151,8 @@ class BaseAgentConfig(BaseModel): system_prompt: str | None = Field( default=None, description=( - "Custom system prompt, appended to the agent's default system prompt — never a replacement. " + "Custom system prompt. Built-in agents layer it on top of their default system prompt " + "rather than replacing it; Claude Code can opt out via system_prompt_mode: replace. " "Each agent's doc page (docs/agents/) states the exact mechanism. " "Supports inline text or multi-line YAML strings. " "Mutually exclusive with system_prompt_file." @@ -254,6 +255,23 @@ def _validate_sdk_options_keys(cls, v: dict[str, Any]) -> dict[str, Any]: ) return v + @model_validator(mode="after") + def check_replace_mode_has_prompt(self) -> Self: + """Reject ``system_prompt_mode: replace`` with no prompt to replace with. + + Without a configured prompt the options builder would fall back to the + claude_code preset (the append regime) while run.json's + ``system_prompt_semantics`` marker could label the run 'replace' — + silently mis-bucketing trend dashboards. ``system_prompt_file`` counts: + the task loader inlines it into ``system_prompt`` at resolution time. + """ + if self.system_prompt_mode == "replace" and self.system_prompt is None and self.system_prompt_file is None: + raise ValueError( + "system_prompt_mode='replace' requires system_prompt (or system_prompt_file) to be set — " + + "there is no prompt to replace the Claude Code default with" + ) + return self + class CodexAgentConfig(BaseAgentConfig): """Codex agent configuration.""" diff --git a/src/coder_eval/reports.py b/src/coder_eval/reports.py index 5ff79eab..180c7fa9 100644 --- a/src/coder_eval/reports.py +++ b/src/coder_eval/reports.py @@ -57,6 +57,24 @@ def resolve_agent_settings(task_dicts: list[dict[str, Any]]) -> tuple[dict[str, return None, False +def _unwrap_system_prompt(value: Any) -> tuple[str | None, str | None]: + """Reduce a persisted ``sdk_options.system_prompt`` value to (text, mode). + + Claude Code append-mode runs persist a ``SystemPromptPreset`` dict + (``{'type': 'preset', 'preset': 'claude_code', ..., 'append': }``); + replace-mode runs persist a plain string. Returns the configured prompt + text (None when nothing was configured — a bare preset dict carries no + custom prompt and gets no row) and the regime worth surfacing ('replace' + for a plain string; None for the default append regime). + """ + if isinstance(value, dict): + append = value.get("append") + return (str(append) if append is not None else None), None + if isinstance(value, str): + return value, "replace" + return None, None + + def collect_agent_settings_rows(settings_source: dict[str, Any], is_sdk: bool) -> list[tuple[str, str]]: """Extract ordered label/value pairs from an agent settings dict. @@ -87,11 +105,14 @@ def collect_agent_settings_rows(settings_source: dict[str, Any], is_sdk: bool) - betas = settings_source.get("betas") if betas: rows.append(("Betas", ", ".join(betas))) - if settings_source.get("system_prompt") is not None: - prompt_str = str(settings_source["system_prompt"]).replace("\n", " ") + prompt_text, prompt_mode = _unwrap_system_prompt(settings_source.get("system_prompt")) + if prompt_text is not None: + prompt_str = prompt_text.replace("\n", " ") if len(prompt_str) > SYSTEM_PROMPT_PREVIEW_CHARS: prompt_str = prompt_str[:SYSTEM_PROMPT_PREVIEW_CHARS] + "..." rows.append(("System Prompt", prompt_str)) + if prompt_mode is not None: + rows.append(("System Prompt Mode", prompt_mode)) plugins = settings_source.get("plugins") if isinstance(plugins, list): diff --git a/src/coder_eval/simulation/user_simulator.py b/src/coder_eval/simulation/user_simulator.py index dca83d57..2001a073 100644 --- a/src/coder_eval/simulation/user_simulator.py +++ b/src/coder_eval/simulation/user_simulator.py @@ -211,6 +211,12 @@ def __init__( setting_sources=[], permission_mode="default", system_prompt=self._system_prompt, + # The roleplay persona IS the simulator's entire identity: 'replace' + # keeps the claude_code coding-agent preset from prefixing it (which + # would contradict the persona's own "stay in character" instruction + # and change every dialog-mode evaluation). Mirrors the judge seam + # in criteria/agent_judge.py. + system_prompt_mode="replace", ) # parse_agent_config returns a union, but type=CLAUDE_CODE guarantees ClaudeCodeAgentConfig assert isinstance(agent_config, ClaudeCodeAgentConfig) diff --git a/tests/test_agent.py b/tests/test_agent.py index 1005a8e9..6cec3156 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -6,7 +6,7 @@ from unittest.mock import MagicMock, patch import pytest -from claude_agent_sdk import ProcessError +from claude_agent_sdk import ClaudeAgentOptions, ProcessError from coder_eval.agent import AgentState from coder_eval.agents.claude_code_agent import ClaudeCodeAgent @@ -181,11 +181,11 @@ async def _capture_sdk_options( *, env_path_prepend: list[str] | None = None, max_turns: int | None = None, -) -> "list": +) -> list[ClaudeAgentOptions]: """Run one communicate() turn with a mocked query() and return captured options list.""" import tempfile - captured_options: list = [] + captured_options: list[ClaudeAgentOptions] = [] class ResultMessage: def __init__(self, session_id: str = "s-1") -> None: @@ -377,12 +377,15 @@ async def test_claude_settings_none_default(): assert captured_options[0].settings is None -def _transport_command(options) -> list[str]: +def _transport_command(options: ClaudeAgentOptions) -> list[str]: """Render captured ClaudeAgentOptions into the actual CLI argv. - The dict-shape assertions pin the values we set; this pins the SDK contract - (which flag the transport emits) — the surface the original replace-vs-append - bug lived on — and survives an SDK TypedDict reshape. + The dict-shape assertions pin the values we set; this pins the argv half of + the SDK contract (which flag the transport emits) — the surface the original + replace-vs-append bug lived on — and survives an SDK TypedDict reshape. + Note ``exclude_dynamic_sections`` never reaches argv: the SDK sends it as a + control-protocol ``excludeDynamicSections`` initialize field, which this + helper cannot see. """ from claude_agent_sdk._internal.transport.subprocess_cli import SubprocessCLITransport @@ -438,7 +441,12 @@ async def test_system_prompt_empty_string_appends_empty(): captured_options = await _capture_sdk_options(agent) - assert captured_options[0].system_prompt["append"] == "" + assert captured_options[0].system_prompt == { + "type": "preset", + "preset": "claude_code", + "exclude_dynamic_sections": True, + "append": "", + } @pytest.mark.asyncio @@ -471,6 +479,23 @@ def test_environment_info_reports_system_prompt_semantics(): assert judge_like.get_environment_info() == {"system_prompt_semantics": "replace"} +def test_system_prompt_mode_replace_requires_prompt(): + """The fourth cell of the mode x prompt matrix: 'replace' with no prompt is + rejected at config validation — otherwise the options builder would fall + back to the preset (append regime) while run.json recorded 'replace'.""" + import pydantic + + with pytest.raises(pydantic.ValidationError, match="system_prompt_mode='replace' requires"): + parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt_mode="replace") + + # system_prompt_file satisfies the requirement: task_loader inlines it into + # system_prompt at resolution time. + config = parse_agent_config( + type=AgentKind.CLAUDE_CODE, system_prompt_file="prompt.md", system_prompt_mode="replace" + ) + assert config.system_prompt_mode == "replace" + + @pytest.mark.asyncio async def test_sdk_options_forwarded_to_sdk(): """An sdk_options key (e.g. effort) is splatted into ClaudeAgentOptions.""" diff --git a/tests/test_orchestrator.py b/tests/test_orchestrator.py index 7eee4c19..0b0ed816 100644 --- a/tests/test_orchestrator.py +++ b/tests/test_orchestrator.py @@ -603,7 +603,10 @@ def get_sdk_options(self): return {"env": {"PATH": os.environ.get("PATH", "")}} def get_environment_info(self): - return {} + # Non-empty on purpose: pins the orchestrator merge seam + # (environment_info.update(agent.get_environment_info())) that + # carries agent markers like system_prompt_semantics into run.json. + return {"system_prompt_semantics": "append"} async def create_dummy_agent(_self): return DummyAgent() @@ -635,6 +638,10 @@ async def create_dummy_agent(_self): await orchestrator._setup() + # An agent-supplied environment_info key survives the merge into the + # run record (the cross-repo contract seam external consumers read). + assert orchestrator.result.environment_info["system_prompt_semantics"] == "append" + assert isinstance(orchestrator.sandbox, Sandbox) assert orchestrator.sandbox.sandbox_dir is not None assert not orchestrator.sandbox.is_persistent diff --git a/tests/test_reports.py b/tests/test_reports.py index 756a345f..fb3dc559 100644 --- a/tests/test_reports.py +++ b/tests/test_reports.py @@ -904,6 +904,84 @@ def test_generate_markdown_sdk_options_defaults_hidden(): assert "**System Prompt**" not in report_md +def test_agent_settings_rows_system_prompt_preset_shapes(): + """Feed a REAL dump_dataclass(ClaudeAgentOptions) — the shape a Claude Code run + actually persists into sdk_options — not a hand-built dict: append-mode runs + carry a SystemPromptPreset dict, and the report must render the appended + prompt text (never the dict repr), omit the row for a bare preset, and + surface the regime for a replace-mode plain string.""" + from claude_agent_sdk import ClaudeAgentOptions + from claude_agent_sdk.types import SystemPromptPreset + + from coder_eval.reports import collect_agent_settings_rows + from coder_eval.utils import dump_dataclass + + appended = SystemPromptPreset( + type="preset", preset="claude_code", exclude_dynamic_sections=True, append="Be terse." + ) + rows = dict(collect_agent_settings_rows(dump_dataclass(ClaudeAgentOptions(system_prompt=appended)), is_sdk=True)) + assert rows["System Prompt"] == "Be terse." + assert "System Prompt Mode" not in rows + + bare = SystemPromptPreset(type="preset", preset="claude_code", exclude_dynamic_sections=True) + rows = dict(collect_agent_settings_rows(dump_dataclass(ClaudeAgentOptions(system_prompt=bare)), is_sdk=True)) + assert "System Prompt" not in rows + + rows = dict(collect_agent_settings_rows(dump_dataclass(ClaudeAgentOptions(system_prompt="Grader.")), is_sdk=True)) + assert rows["System Prompt"] == "Grader." + assert rows["System Prompt Mode"] == "replace" + + +def test_generate_markdown_system_prompt_preset_not_dict_repr(): + """End-to-end: a preset-shaped sdk_options.system_prompt renders as prompt text + in the Markdown report, and a bare preset (no configured prompt) keeps the + System Prompt row absent — the pre-preset behavior for unset prompts.""" + summary = RunSummary( + run_id="test-run", + start_time=datetime(2025, 10, 11, 12, 0, 0), + end_time=datetime(2025, 10, 11, 12, 1, 0), + total_duration_seconds=60.0, + tasks_run=1, + tasks_succeeded=1, + tasks_failed=0, + tasks_error=0, + task_results=[ + _make_task_result( + "task1", + "SUCCESS", + 1.0, + 30.0, + iteration_count=1, + sdk_options={ + "permission_mode": "bypassPermissions", + "allowed_tools": [], + "system_prompt": { + "type": "preset", + "preset": "claude_code", + "exclude_dynamic_sections": True, + "append": "You are a careful engineer.", + }, + }, + ), + ], + framework_version="0.1.0", + environment_info={}, + ) + + report_md = ReportGenerator.generate_markdown(summary) + assert "**System Prompt**: You are a careful engineer." in report_md + assert "{'type': 'preset'" not in report_md + + # Bare preset == no configured prompt: the row stays absent. + summary.task_results[0]["sdk_options"]["system_prompt"] = { + "type": "preset", + "preset": "claude_code", + "exclude_dynamic_sections": True, + } + report_md = ReportGenerator.generate_markdown(summary) + assert "**System Prompt**" not in report_md + + def test_generate_markdown_no_agent_settings(): """Test that Agent Settings section is omitted when no task has agent_config.""" summary = RunSummary( diff --git a/tests/test_sub_agent_runner.py b/tests/test_sub_agent_runner.py index a40a31aa..68a55ba2 100644 --- a/tests/test_sub_agent_runner.py +++ b/tests/test_sub_agent_runner.py @@ -46,6 +46,7 @@ def _make_agent_config() -> ClaudeCodeAgentConfig: permission_mode="bypassPermissions", allowed_tools=["Read"], system_prompt="x", + system_prompt_mode="replace", # identity-prompt contract setting_sources=[], # security contract ), ) @@ -438,6 +439,27 @@ def test_runner_asserts_setting_sources_empty(sandbox: Sandbox, bad_sources: lis ) +def test_runner_asserts_replace_mode_for_identity_prompt(sandbox: Sandbox) -> None: + """Identity-prompt contract: a sub-agent's system_prompt is its entire identity, + so append mode (which would prefix the claude_code coding-agent preset) is + rejected at construction rather than silently changing the sub-agent's persona.""" + bad_config = parse_agent_config( + type=AgentKind.CLAUDE_CODE, + model="claude-opus-4-6", + permission_mode="bypassPermissions", + allowed_tools=["Read"], + system_prompt="x", # mode defaults to 'append' + setting_sources=[], + ) + with pytest.raises(ValueError, match="system_prompt_mode"): + SubAgentRunner( + sandbox=sandbox, + agent_config=bad_config, + ignore_patterns=[], + route=DirectRoute(), + ) + + # --- symlink + pattern filtering (unit + end-to-end) --- diff --git a/tests/test_user_simulator.py b/tests/test_user_simulator.py index 3adb9647..ca1e5aa4 100644 --- a/tests/test_user_simulator.py +++ b/tests/test_user_simulator.py @@ -70,6 +70,19 @@ def test_system_prompt_override_used_verbatim(self): ) assert sim.system_prompt == "CUSTOM TEMPLATE BODY" + def test_persona_replaces_coding_agent_preset(self): + """The persona is the simulator's ENTIRE system prompt: the agent config + must use system_prompt_mode='replace' so the claude_code coding-agent + preset never prefixes the roleplay identity (which would contradict its + own "stay in character" instruction on every dialog-mode run).""" + sim = UserSimulator( + config=_sim_cfg(), + task_description="A task", + initial_prompt="Start", + ) + assert sim._agent_config.system_prompt_mode == "replace" + assert sim._agent_config.system_prompt == sim.system_prompt + def test_opener_wording_when_no_initial_prompt(self): sim = UserSimulator( config=_sim_cfg(persona="BA", goal="build dice roller"), From 377e8e34632625223bf7227ea93ab00b3c65a27b Mon Sep 17 00:00:00 2001 From: Mihai Chirculescu Date: Tue, 11 Aug 2026 01:58:23 +0300 Subject: [PATCH 7/7] fix(agent): resolve system_prompt_file atomically and reject blank prompts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the PR review's blockers. The two prompt fields have no valid sequential assignment order under BaseAgentConfig's validate_assignment=True: clearing system_prompt_file first leaves (mode='replace', prompt=None, file=None), which check_replace_mode_has_prompt rejects, while setting system_prompt first leaves both populated, which check_prompt_exclusivity rejects. So `system_prompt_mode: replace` + `system_prompt_file:` — a combination both CLAUDE_CODE.md and the validator's own docstring advertise — hard-failed at task load, with an error naming the field the user did set. resolve_agent_system_prompt now returns a new config built by one model_copy(update=...) so no half-updated state is ever validated. Deliberately model_copy and not a validated reconstruction: experiment.py builds the task's merge layer with model_dump(exclude_unset=True), so a rebuilt config would mark every field as set and let the task layer override variant and CLI defaults. A regression test pins that. _build_agent_config in agent_judge.py had the same defect, reachable whenever a user's agent_judge YAML set system_prompt_file; its forced fields now travel in a single model_copy update too. Also from the review: - A blank or whitespace-only system_prompt now normalizes to None at one seam. Every agent branches on `is not None`, so a blank string was the one value that read as configured while carrying nothing — producing an empty ENTIRE system prompt under replace, and empty system_instructions on Antigravity. - SubAgentRunner enforces both halves of the invariant it states: a sub-agent must have a system_prompt AND be in replace mode. An omitted prompt reached the bare coding-agent preset through the other branch. - _effective_prompt_mode warns on its fail-open branch, so a silently downgraded judge is visible in task.log, not only in run.json. - Migration sections in CLAUDE_CODE.md (append semantics, how to preserve the old behavior) and CODEX.md (ignored -> injected, with no opt-out), plus a caveat on the AB_EXPERIMENTS system_prompt_mode lever. - echo_simulated_judged.yaml pinned to system_prompt_mode: replace: its prompt was written to BE the entire system prompt, so this preserves its judged baseline. Test coverage moves to the loader: a construction-only test is what let the crash ship green, so the system_prompt x system_prompt_file x system_prompt_mode matrix now runs through the real load_task. BREAKING CHANGE: agent.system_prompt now appends to the agent's default system prompt instead of replacing it (Claude Code), and is no longer silently dropped (Codex). Scores are not comparable across this boundary. Set system_prompt_mode: replace to preserve the old Claude Code behavior; Codex has no opt-out. sdk_options.system_prompt changes shape from str to a SystemPromptPreset dict in append mode. Co-Authored-By: Claude Opus 5 (1M context) --- docs/AB_EXPERIMENTS.md | 9 +- docs/agents/CLAUDE_CODE.md | 36 ++++- docs/agents/CODEX.md | 14 ++ src/coder_eval/agents/claude_code_agent.py | 11 +- src/coder_eval/criteria/agent_judge.py | 33 +++-- src/coder_eval/evaluation/sub_agent.py | 16 +- src/coder_eval/models/agent_config.py | 19 ++- src/coder_eval/orchestration/experiment.py | 7 +- src/coder_eval/orchestration/task_loader.py | 52 +++++-- .../echo_simulated_judged.yaml | 6 + tests/test_agent.py | 52 +++++-- tests/test_agent_judge_criterion.py | 72 +++++++++ tests/test_resolve_task_files.py | 139 +++++++++++++++++- 13 files changed, 397 insertions(+), 69 deletions(-) diff --git a/docs/AB_EXPERIMENTS.md b/docs/AB_EXPERIMENTS.md index 3c7612e5..060773f5 100644 --- a/docs/AB_EXPERIMENTS.md +++ b/docs/AB_EXPERIMENTS.md @@ -133,8 +133,13 @@ From `ExperimentVariant` (`coder_eval/models/experiment.py`): The `agent` dict is the lever for most A/B tests. Anything on `AgentConfig` is fair game: `model`, `permission_mode`, `allowed_tools`, `disallowed_tools`, -`plugins`, `system_prompt` / `system_prompt_file`, `system_prompt_mode` -(append-vs-replace arms), `setting_sources`, `claude_settings`, `sdk_options`. +`plugins`, `system_prompt` / `system_prompt_file`, `setting_sources`, +`claude_settings`, `sdk_options`. + +> **`system_prompt_mode` is a poor A/B lever.** A `replace` arm sends no default +> Claude Code prompt *at all*, so the delta measures the missing behavioral guidance +> (tool-call batching, conciseness), not your prompt text. To A/B prompt *content*, +> vary `system_prompt` and leave both arms on `append`. > **Path-resolution gotcha.** Relative file paths in variant config resolve > against _different_ base directories depending on the field: diff --git a/docs/agents/CLAUDE_CODE.md b/docs/agents/CLAUDE_CODE.md index a84a7ccd..cd1f8b94 100644 --- a/docs/agents/CLAUDE_CODE.md +++ b/docs/agents/CLAUDE_CODE.md @@ -99,9 +99,9 @@ agent: | `allowed_tools` | `list[str] \| null` | Tool allowlist. Unset ⇒ all tools allowed. | | `disallowed_tools` | `list[str] \| null` | Tool denylist. (`ToolSearch` is always appended for Bedrock parity.) | | `plugins` | `list[{type: local, path}]` | Local plugin/skill directories; `$VAR` in `path` is expanded and resolved to an absolute path. | -| `system_prompt` | `str \| null` | **Appended** to the default Claude Code system prompt (via the SDK's `claude_code` preset) — the default's behavioral guidance is kept unless `system_prompt_mode: replace` opts out. Mutually exclusive with `system_prompt_file`. | -| `system_prompt_mode` | `"append"` (default) / `"replace"` | `replace` sends `system_prompt` as the **entire** system prompt (no preset) and requires `system_prompt` / `system_prompt_file` to be set (validated at load). Used by judge sub-agents and the user simulator, which must not carry the coding-agent persona; rarely needed in tasks. | -| `system_prompt_file` | `str \| null` | Path (relative to the task YAML) loaded into `system_prompt` at resolution. | +| `system_prompt` | `str \| null` | **Appended** to the default Claude Code system prompt (via the SDK's `claude_code` preset) — the default's behavioral guidance is kept unless `system_prompt_mode: replace` opts out. Mutually exclusive with `system_prompt_file`. An empty or whitespace-only value is treated as unset. | +| `system_prompt_mode` | `"append"` (default) / `"replace"` | `replace` sends `system_prompt` as the **entire** system prompt (no preset) and requires a non-blank `system_prompt` / `system_prompt_file` (validated at load). Used by judge sub-agents and the user simulator, which must not carry the coding-agent persona; rarely needed in tasks — see [the migration note](#migrating-tasks-that-set-system_prompt). | +| `system_prompt_file` | `str \| null` | Path (relative to the task YAML) loaded into `system_prompt` at resolution. Works with either `system_prompt_mode`. | | `setting_sources` | `list["user"\|"project"\|"local"] \| null` | Which host setting sources the SDK reads. Default resolves to `["project"]`. See [Sandbox isolation](#sandbox-isolation). | | `claude_settings` | `str \| dict \| null` | Passed to the SDK `--settings`. A dict is JSON-serialized; a str is a settings file path. Use `permissions.deny` to block tools/paths. | | `sdk_options` | `dict` (default `{}`) | Pass-through for `ClaudeAgentOptions` fields Coder Eval doesn't own (e.g. `effort`). Validated at load — an unknown or framework-owned key is a hard error. | @@ -123,6 +123,36 @@ agent: > (`append` / `replace`) records the prompt regime — runs predating that marker > used replace-on-set / empty-on-unset semantics and are not score-comparable. +### Migrating tasks that set `system_prompt` + +`system_prompt` used to **replace** Claude Code's default system prompt. It now +appends to it. If your task or experiment sets `system_prompt`, the agent gains back +every default behavioral instruction it was previously running without — parallel +tool-call batching, conciseness rules, the `Read`/`Grep`/`Glob` tool preferences, and +the default security guardrails. + +That is a genuine behavior change, so **scores are not comparable across this +boundary**. Pick one: + +- **Keep the repair (recommended).** Do nothing. Re-baseline any threshold or + reference score the task gates on, and expect turn counts to drop on tasks that + depend on batched tool calls. +- **Preserve the old behavior.** Add `system_prompt_mode: replace` to the `agent:` + block. The configured prompt again becomes the entire system prompt. Only do this + if the task *intends* to run without the default guidance — a prompt that merely + adds sandbox policy or a persona does not. + +Segment dashboards on `environment_info.system_prompt_semantics` to keep the two +regimes in separate cohorts. Note the append-mode baseline also tracks the installed +CLI version, so a `CLAUDE_CODE_VERSION` bump becomes a score-affecting change +(attributable via `environment_info.claude_code_cli`). + +> **Consumers reading `sdk_options.system_prompt`.** The persisted value changed +> shape: a plain `str` in the old regime, a `SystemPromptPreset` dict +> (`{type, preset, exclude_dynamic_sections, append}`) in append mode. Code that +> string-handles that field needs to branch on the type — see +> [Report schema](../REPORT_SCHEMA.md). + ### Setting fields from the CLI Any of these merge-resolve through `-D` / `--set` (see diff --git a/docs/agents/CODEX.md b/docs/agents/CODEX.md index 7b6dc99f..fe230294 100644 --- a/docs/agents/CODEX.md +++ b/docs/agents/CODEX.md @@ -227,6 +227,20 @@ The Codex SDK is synchronous. The agent uses `_run_async()` helper to detect and 4. **Authentication** - Requires `CODEX_API_KEY` in the environment (point it at whichever endpoint's key you use — OpenAI, gateway, or Azure); the agent calls `login_api_key` when a key is present. `OPENAI_API_KEY`/`AZURE_OPENAI_API_KEY` are NOT read. 5. **Model field** - `TurnRecord.model_used` reflects the pinned `agent.model`; the Codex `Turn` payload itself doesn't carry the resolved model. 6. **Skills with Windows paths** - Symlink creation may fail on Windows; agent falls back to copying (slower). +7. **No `system_prompt_mode`** - `replace` semantics are Claude-Code-only. `system_prompt` is always appended as `developer_instructions`; setting `system_prompt_mode` on a Codex `agent:` block is a validation error (unknown field). + +## Migrating tasks that set `system_prompt` + +`system_prompt` was previously **ignored** on Codex tasks — silently dropped, so the +task ran on Codex's base prompt alone. It is now forwarded as +`developer_instructions`, layered on top of that base prompt. Any Codex task setting +the field now actually receives those instructions, so **scores are not comparable +across this boundary**. Two things to check: + +- A prompt written for Claude (naming `Read`/`Grep`/`Glob`, or Claude tool etiquette) + is now live on Codex, where those tool names don't exist. +- There is **no opt-out** (see Known Limitations #7). To restore the old behavior, + remove `system_prompt` from the Codex variant — otherwise re-baseline. ## Future Enhancements diff --git a/src/coder_eval/agents/claude_code_agent.py b/src/coder_eval/agents/claude_code_agent.py index f0d94425..1c3b6ff6 100644 --- a/src/coder_eval/agents/claude_code_agent.py +++ b/src/coder_eval/agents/claude_code_agent.py @@ -1249,8 +1249,15 @@ def _effective_prompt_mode(self) -> Literal["append", "replace"]: prompt (the config validator rejects the pair at load, but a mutated or hand-built config falls back to the preset here — fail open to append). """ - if self.config.system_prompt_mode == "replace" and self.config.system_prompt is not None: - return "replace" + if self.config.system_prompt_mode == "replace": + if self.config.system_prompt is not None: + return "replace" + # Warn on the downgrade so it's visible in task.log rather than only + # inferable from run.json's system_prompt_semantics marker. + logger.warning( + "system_prompt_mode='replace' with no system_prompt — falling back to the claude_code " + + "preset (append regime). run.json records the regime actually used." + ) return "append" def get_environment_info(self) -> dict[str, Any]: diff --git a/src/coder_eval/criteria/agent_judge.py b/src/coder_eval/criteria/agent_judge.py index bbbc6e48..7a650313 100644 --- a/src/coder_eval/criteria/agent_judge.py +++ b/src/coder_eval/criteria/agent_judge.py @@ -266,18 +266,27 @@ def _build_agent_config( user_overrides = criterion.agent.model_dump(exclude_unset=True) if "sdk_options" in user_overrides: user_overrides["sdk_options"] = {**defaults.sdk_options, **user_overrides["sdk_options"]} - config = defaults.model_copy(update=user_overrides, deep=True) - config.system_prompt = system_prompt - # Force replace regardless of user YAML: the judge prompt is its entire - # identity — the coding-agent preset must never prefix the scoring - # instrument (see the note on _SYSTEM_PROMPT). - config.system_prompt_mode = "replace" - # SECURITY: force setting_sources=[] regardless of user YAML so the SDK - # does NOT load .claude/settings.json or .mcp.json from the judge's cwd. - # Those files can install pre-LLM lifecycle hooks (SessionStart / - # PreToolUse) or MCP subprocesses that run with the evaluator's - # credentials BEFORE the allowed_tools gate kicks in. - config.setting_sources = [] + config = defaults.model_copy( + update={ + **user_overrides, + # Force the judge's own prompt in replace mode regardless of user YAML + # (see the note on _SYSTEM_PROMPT for why, and SubAgentRunner.__init__ + # for the enforcement point). system_prompt_file must be cleared in the + # SAME update: it is mutually exclusive with system_prompt, and + # BaseAgentConfig has validate_assignment=True, so a user YAML that set + # it would make a sequential assignment raise on the intermediate state. + "system_prompt": system_prompt, + "system_prompt_file": None, + "system_prompt_mode": "replace", + # SECURITY: force setting_sources=[] regardless of user YAML so the SDK + # does NOT load .claude/settings.json or .mcp.json from the judge's cwd. + # Those files can install pre-LLM lifecycle hooks (SessionStart / + # PreToolUse) or MCP subprocesses that run with the evaluator's + # credentials BEFORE the allowed_tools gate kicks in. + "setting_sources": [], + }, + deep=True, + ) # SECURITY: ensure the ignore_patterns floor is present even if the # user supplied their own list. Set-union guarantees idempotence and # doesn't depend on order. diff --git a/src/coder_eval/evaluation/sub_agent.py b/src/coder_eval/evaluation/sub_agent.py index 3e82facc..0c9e80f8 100644 --- a/src/coder_eval/evaluation/sub_agent.py +++ b/src/coder_eval/evaluation/sub_agent.py @@ -89,14 +89,16 @@ def __init__( ) # A sub-agent's system_prompt is its entire identity (judge instructions, # simulator persona) — the claude_code coding-agent preset must never - # prefix it. Same fail-loud contract as setting_sources above: callers - # own their config, so a misconfigured one raises instead of being - # silently mutated. - if agent_config.system_prompt is not None and agent_config.system_prompt_mode != "replace": + # prefix it. That takes BOTH halves: an omitted prompt gets the bare + # preset, which is the same failure reached via the other branch, so + # neither is accepted. Same fail-loud contract as setting_sources above: + # callers own their config, so a misconfigured one raises instead of + # being silently mutated. + if agent_config.system_prompt is None or agent_config.system_prompt_mode != "replace": raise ValueError( - "SubAgentRunner requires agent_config.system_prompt_mode='replace' when a " - + "system_prompt is set: the sub-agent prompt is its entire identity and must " - + "not be appended to the claude_code coding-agent preset." + "SubAgentRunner requires agent_config.system_prompt to be set with " + + "system_prompt_mode='replace': the sub-agent prompt is its entire identity " + + "and must not be appended to (or replaced by) the claude_code coding-agent preset." ) assert sandbox.sandbox_dir is not None, "sandbox not initialized" self._sandbox = sandbox diff --git a/src/coder_eval/models/agent_config.py b/src/coder_eval/models/agent_config.py index c7eda5a2..fff138f3 100644 --- a/src/coder_eval/models/agent_config.py +++ b/src/coder_eval/models/agent_config.py @@ -186,6 +186,20 @@ def _validate_ignore_patterns(cls, values: list[str]) -> list[str]: return [normalize_ignore_pattern_entry(v) for v in values] + @field_validator("system_prompt", mode="after") + @classmethod + def _blank_prompt_is_no_prompt(cls, v: str | None) -> str | None: + """Collapse an empty or whitespace-only ``system_prompt`` to ``None``. + + Every agent branches on ``system_prompt is not None`` to decide whether a + prompt was configured, so a blank string is the one value that reads as + "configured" while carrying nothing — producing an empty *entire* system + prompt under ``replace``, and empty ``system_instructions`` on Antigravity. + Normalizing here fixes both, and makes ``check_replace_mode_has_prompt`` + reject ``replace`` + blank instead of silently honoring it. + """ + return v if v is None or v.strip() else None + @model_validator(mode="after") def check_prompt_exclusivity(self) -> Self: """Ensure system_prompt and system_prompt_file are mutually exclusive.""" @@ -263,7 +277,10 @@ def check_replace_mode_has_prompt(self) -> Self: claude_code preset (the append regime) while run.json's ``system_prompt_semantics`` marker could label the run 'replace' — silently mis-bucketing trend dashboards. ``system_prompt_file`` counts: - the task loader inlines it into ``system_prompt`` at resolution time. + the task loader inlines it into ``system_prompt`` at resolution time + (atomically — see ``resolve_agent_system_prompt``, which must never leave a + half-updated config for this validator to see). A blank prompt does NOT + count: ``_blank_prompt_is_no_prompt`` has already collapsed it to ``None``. """ if self.system_prompt_mode == "replace" and self.system_prompt is None and self.system_prompt_file is None: raise ValueError( diff --git a/src/coder_eval/orchestration/experiment.py b/src/coder_eval/orchestration/experiment.py index f75c585e..cea13f1c 100644 --- a/src/coder_eval/orchestration/experiment.py +++ b/src/coder_eval/orchestration/experiment.py @@ -524,9 +524,10 @@ def resolve_task_files( """ exp_dir = experiment_file.parent if experiment_file is not None else task_file.parent - # Resolve system_prompt_file (may be injected by variant as relative or absolute path) - if task.agent is not None and task.agent.system_prompt_file is not None: - resolve_agent_system_prompt(task.agent, exp_dir) + # Resolve system_prompt_file (may be injected by variant as relative or absolute path). + # Rebind: the resolver returns a new config so the prompt/file swap is atomic + # (see resolve_agent_system_prompt). + task.agent = resolve_agent_system_prompt(task.agent, exp_dir) # Resolve relative template_sources paths if task.sandbox.template_sources: diff --git a/src/coder_eval/orchestration/task_loader.py b/src/coder_eval/orchestration/task_loader.py index bff0c13a..ea33abe8 100644 --- a/src/coder_eval/orchestration/task_loader.py +++ b/src/coder_eval/orchestration/task_loader.py @@ -226,26 +226,46 @@ def resolve_variant_initial_prompt_file(variant: ExperimentVariant, base_dir: Pa variant.initial_prompt = content -def resolve_agent_system_prompt(agent_config: AgentConfig | BaseAgentConfig | None, base_dir: Path) -> None: - """Resolve system_prompt_file to inline system_prompt. Mutates in place.""" - if agent_config is None: - return - if agent_config.system_prompt_file is not None: - prompt_path = Path(agent_config.system_prompt_file) - if not prompt_path.is_absolute(): - prompt_path = (base_dir / prompt_path).resolve() - if not prompt_path.exists(): - raise FileNotFoundError(f"system_prompt_file not found: {prompt_path}") - content = prompt_path.read_text(encoding="utf-8").strip() - # Clear file field BEFORE setting inline to avoid mutual-exclusivity validator - agent_config.system_prompt_file = None - agent_config.system_prompt = content +def resolve_agent_system_prompt[T: AgentConfig | BaseAgentConfig | None](agent_config: T, base_dir: Path) -> T: + """Inline ``system_prompt_file`` into ``system_prompt``, returning the resolved config. + + Returns a NEW config rather than mutating in place because the swap has no + valid sequential order: ``BaseAgentConfig`` sets ``validate_assignment=True``, + and the two prompt fields are constrained against each other in both + directions — clearing the file first leaves ``(mode='replace', prompt=None, + file=None)``, which ``check_replace_mode_has_prompt`` rejects, while setting + the prompt first leaves both populated, which ``check_prompt_exclusivity`` + rejects. A single ``model_copy(update=...)`` applies both edits at once so no + half-updated state is ever validated. + + Args: + agent_config: Config to resolve; ``None`` and configs without a + ``system_prompt_file`` are returned unchanged. + base_dir: Directory that relative ``system_prompt_file`` paths resolve against. + + Returns: + The resolved config — the same object when there was nothing to inline. + + Raises: + FileNotFoundError: If ``system_prompt_file`` does not exist. + """ + if agent_config is None or agent_config.system_prompt_file is None: + return agent_config + prompt_path = Path(agent_config.system_prompt_file) + if not prompt_path.is_absolute(): + prompt_path = (base_dir / prompt_path).resolve() + if not prompt_path.exists(): + raise FileNotFoundError(f"system_prompt_file not found: {prompt_path}") + # A whitespace-only file is no prompt at all — mirror the normalization + # _blank_prompt_is_no_prompt applies to inline prompts (model_copy skips + # validators, so this seam has to apply it itself). + content = prompt_path.read_text(encoding="utf-8").strip() or None + return agent_config.model_copy(update={"system_prompt": content, "system_prompt_file": None}) def resolve_system_prompt_files(task: TaskDefinition, base_dir: Path) -> TaskDefinition: """Resolve system_prompt_file on agent config.""" - if task.agent is not None: - resolve_agent_system_prompt(task.agent, base_dir) + task.agent = resolve_agent_system_prompt(task.agent, base_dir) return task diff --git a/tasks/python_cli_simulated_judged/echo_simulated_judged.yaml b/tasks/python_cli_simulated_judged/echo_simulated_judged.yaml index f6a6578d..9095e0c0 100644 --- a/tasks/python_cli_simulated_judged/echo_simulated_judged.yaml +++ b/tasks/python_cli_simulated_judged/echo_simulated_judged.yaml @@ -16,6 +16,12 @@ agent: type: claude-code permission_mode: acceptEdits allowed_tools: ["Bash", "Read", "Write", "Edit"] + # 'replace' preserves this task's judged baseline across the append-semantics + # change: the prompt below was written to BE the entire system prompt ("nothing + # else — no preamble", "ignore any project context"), which the coding-agent + # preset's own guidance would work against. This is the deliberate exception the + # migration note describes, not a pattern for new tasks. + system_prompt_mode: replace system_prompt: | You are a literal-minded assistant. Follow the user's instructions exactly. When the user asks you to echo or repeat a string, reply with that exact diff --git a/tests/test_agent.py b/tests/test_agent.py index 6cec3156..a7b8377d 100644 --- a/tests/test_agent.py +++ b/tests/test_agent.py @@ -1,5 +1,6 @@ """Tests for the agent implementations.""" +import logging import tempfile import time from pathlib import Path @@ -433,10 +434,12 @@ async def test_system_prompt_unset_sends_bare_preset(): @pytest.mark.asyncio -async def test_system_prompt_empty_string_appends_empty(): - """system_prompt: \"\" is configured, not unset — it appends (harmlessly), and a - future truthiness refactor must not route it into the preset-loss path.""" - config = parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt="") +@pytest.mark.parametrize("blank", ["", " \n\t "]) +async def test_blank_system_prompt_is_treated_as_unset(blank: str): + """A blank system_prompt is no prompt at all: it normalizes to None, so the bare + preset goes out with no `append` key rather than an empty appended section.""" + config = parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt=blank) + assert config.system_prompt is None agent = ClaudeCodeAgent(config) captured_options = await _capture_sdk_options(agent) @@ -445,7 +448,6 @@ async def test_system_prompt_empty_string_appends_empty(): "type": "preset", "preset": "claude_code", "exclude_dynamic_sections": True, - "append": "", } @@ -479,21 +481,41 @@ def test_environment_info_reports_system_prompt_semantics(): assert judge_like.get_environment_info() == {"system_prompt_semantics": "replace"} -def test_system_prompt_mode_replace_requires_prompt(): - """The fourth cell of the mode x prompt matrix: 'replace' with no prompt is - rejected at config validation — otherwise the options builder would fall - back to the preset (append regime) while run.json recorded 'replace'.""" +def test_replace_mode_without_prompt_fails_open_and_warns(caplog): + """Defense in depth: the config validator rejects this pair at load, so reaching + the agent means a mutated or hand-built config. Fail OPEN to append — never send + an empty entire system prompt — and warn so the downgrade shows up in task.log + rather than only in run.json.""" + config = parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt="x", system_prompt_mode="replace") + # Bypass validate_assignment the way only a hand-built/mutated config could. + object.__setattr__(config, "system_prompt", None) + agent = ClaudeCodeAgent(config) + + with caplog.at_level(logging.WARNING): + assert agent._effective_prompt_mode() == "append" + + assert "falling back to the claude_code preset" in caplog.text + + +@pytest.mark.parametrize("blank", ["", " "]) +def test_system_prompt_mode_replace_requires_a_non_blank_prompt(blank: str): + """The fourth cell of the mode x prompt matrix: 'replace' with no usable prompt is + rejected at config validation — otherwise the options builder would fall back to + the preset (append regime) while run.json recorded 'replace'. A blank prompt is + normalized to None first, so it is rejected here too rather than sending an empty + entire system prompt. + + The system_prompt_file cell is covered end-to-end through load_task in + tests/test_resolve_task_files.py — asserting construction alone would not exercise + the loader's field swap, which is where this combination actually resolves. + """ import pydantic with pytest.raises(pydantic.ValidationError, match="system_prompt_mode='replace' requires"): parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt_mode="replace") - # system_prompt_file satisfies the requirement: task_loader inlines it into - # system_prompt at resolution time. - config = parse_agent_config( - type=AgentKind.CLAUDE_CODE, system_prompt_file="prompt.md", system_prompt_mode="replace" - ) - assert config.system_prompt_mode == "replace" + with pytest.raises(pydantic.ValidationError, match="system_prompt_mode='replace' requires"): + parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt=blank, system_prompt_mode="replace") @pytest.mark.asyncio diff --git a/tests/test_agent_judge_criterion.py b/tests/test_agent_judge_criterion.py index 593764bc..cb8990c1 100644 --- a/tests/test_agent_judge_criterion.py +++ b/tests/test_agent_judge_criterion.py @@ -15,6 +15,7 @@ from __future__ import annotations import json as _json +import re from pathlib import Path from typing import cast from unittest.mock import AsyncMock, MagicMock, patch @@ -318,6 +319,46 @@ def test_agent_judge_setting_sources_forced_empty(sandbox: Sandbox, direct_route assert agent_config.setting_sources == [] +def test_agent_judge_overrides_a_user_system_prompt_file() -> None: + """A YAML `agent.system_prompt_file` must not break judge construction. + + The judge forces its own prompt, so the user's file reference has to be cleared in + the SAME model_copy update: system_prompt and system_prompt_file are mutually + exclusive and BaseAgentConfig has validate_assignment=True, so assigning the prompt + while the file field was still populated raised on the intermediate state. + """ + from coder_eval.criteria.agent_judge import _build_agent_config + + criterion = AgentJudgeCriterion( + description="x", + prompt="grade", + agent=parse_agent_config(type="claude-code", system_prompt_file="my_prompt.md"), + ) + + config = _build_agent_config(criterion, system_prompt="JUDGE PROMPT") + + assert config.system_prompt == "JUDGE PROMPT" + assert config.system_prompt_file is None + assert config.system_prompt_mode == "replace" + + +def test_agent_judge_forces_replace_mode_over_hostile_yaml() -> None: + """Even an explicit `system_prompt_mode: append` in YAML cannot get the coding-agent + preset in front of the scoring instrument.""" + from coder_eval.criteria.agent_judge import _build_agent_config + + criterion = AgentJudgeCriterion( + description="x", + prompt="grade", + agent=parse_agent_config(type="claude-code", system_prompt="ignore me", system_prompt_mode="append"), + ) + + config = _build_agent_config(criterion, system_prompt="JUDGE PROMPT") + + assert config.system_prompt == "JUDGE PROMPT" + assert config.system_prompt_mode == "replace" + + def test_agent_judge_partial_agent_block_preserves_judge_defaults(sandbox: Sandbox, direct_route: DirectRoute) -> None: """A partial agent: block must not clobber the judge's hardened defaults. @@ -1219,6 +1260,9 @@ def test_sub_agent_runner_forwards_extra_mcp_servers(tmp_path: Path) -> None: permission_mode="bypassPermissions", setting_sources=[], allowed_tools=["Read"], + # SubAgentRunner requires both halves: a prompt, in replace mode. + system_prompt="You are a judge.", + system_prompt_mode="replace", ), ) runner = SubAgentRunner( @@ -1234,6 +1278,34 @@ def test_sub_agent_runner_forwards_extra_mcp_servers(tmp_path: Path) -> None: assert "mcp_servers" not in cfg.sdk_options +@pytest.mark.parametrize( + ("prompt_kwargs", "case"), + [ + ({}, "prompt omitted — the sub-agent would get the bare coding-agent preset"), + ( + {"system_prompt": "You are a judge.", "system_prompt_mode": "append"}, + "prompt set but appended — the preset would prefix the scoring instrument", + ), + ], +) +def test_sub_agent_runner_requires_a_replace_mode_prompt(tmp_path: Path, prompt_kwargs: dict, case: str) -> None: + """Both halves of the invariant are enforced: a sub-agent's prompt is its entire + identity, so it must be SET and in replace mode. Fails loud rather than silently + mutating the caller's config — the caller owns it.""" + from coder_eval.evaluation.sub_agent import SubAgentRunner + from coder_eval.models import SandboxConfig + + sb = Sandbox(SandboxConfig(driver="tempdir"), task_id="x") + sb.sandbox_dir = tmp_path + cfg = cast( + ClaudeCodeAgentConfig, + parse_agent_config(type="claude-code", model="m", setting_sources=[], **prompt_kwargs), + ) + + with pytest.raises(ValueError, match=re.escape("requires agent_config.system_prompt to be set")): + SubAgentRunner(sandbox=sb, agent_config=cfg, ignore_patterns=[], route=DirectRoute()) + + def test_claude_code_agent_accepts_extra_mcp_servers() -> None: """ClaudeCodeAgent stores extra_mcp_servers; merging into ClaudeAgentOptions is tested live.""" from coder_eval.agents.claude_code_agent import ClaudeCodeAgent diff --git a/tests/test_resolve_task_files.py b/tests/test_resolve_task_files.py index 441352d3..01fd4cd7 100644 --- a/tests/test_resolve_task_files.py +++ b/tests/test_resolve_task_files.py @@ -3,11 +3,13 @@ from pathlib import Path import pytest +import yaml from pydantic import ValidationError from coder_eval.models import ( AgentConfig, AgentKind, + ClaudeCodeAgentConfig, SandboxConfig, TaskDefinition, TemplateDirSource, @@ -15,6 +17,7 @@ ) from coder_eval.orchestration.experiment import resolve_task_files from coder_eval.orchestration.task_loader import ( + load_task, resolve_agent_system_prompt, resolve_initial_prompt_file, ) @@ -164,9 +167,9 @@ def test_resolves_relative_path(self, tmp_path): prompt_file = tmp_path / "system.md" prompt_file.write_text("System prompt content\n") agent = parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt_file="system.md") - resolve_agent_system_prompt(agent, tmp_path) - assert agent.system_prompt == "System prompt content" - assert agent.system_prompt_file is None + resolved = resolve_agent_system_prompt(agent, tmp_path) + assert resolved.system_prompt == "System prompt content" + assert resolved.system_prompt_file is None def test_missing_file_raises(self, tmp_path): agent = parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt_file="missing.md") @@ -175,19 +178,139 @@ def test_missing_file_raises(self, tmp_path): def test_no_file_field_is_noop(self, tmp_path): agent = parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt="inline") - resolve_agent_system_prompt(agent, tmp_path) - assert agent.system_prompt == "inline" + assert resolve_agent_system_prompt(agent, tmp_path) is agent def test_none_agent_is_noop(self, tmp_path): """Passing None should not raise.""" - resolve_agent_system_prompt(None, tmp_path) + assert resolve_agent_system_prompt(None, tmp_path) is None def test_multiline_content_stripped(self, tmp_path): prompt_file = tmp_path / "system.md" prompt_file.write_text("\n Line 1\n Line 2\n\n") agent = parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt_file="system.md") - resolve_agent_system_prompt(agent, tmp_path) - assert agent.system_prompt == "Line 1\n Line 2" + resolved = resolve_agent_system_prompt(agent, tmp_path) + assert resolved.system_prompt == "Line 1\n Line 2" + + def test_preserves_fields_set_for_the_merge_layer(self, tmp_path): + """The resolved copy must keep __pydantic_fields_set__: experiment.py builds the + task's merge layer with model_dump(exclude_unset=True), so a config that marked + every field as set would override variant and CLI layers with its defaults.""" + prompt_file = tmp_path / "system.md" + prompt_file.write_text("content") + agent = parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt_file="system.md") + resolved = resolve_agent_system_prompt(agent, tmp_path) + assert "model" not in resolved.model_fields_set + assert resolved.model_dump(exclude_unset=True).keys() <= { + "type", + "system_prompt", + "system_prompt_file", + } + + @pytest.mark.parametrize("mode", ["append", "replace"]) + def test_replace_mode_with_prompt_file_resolves(self, tmp_path, mode: str): + """Regression: the two prompt fields have no valid sequential assignment order + under validate_assignment, so a non-atomic swap raised on the intermediate + state — for 'replace' via check_replace_mode_has_prompt (both fields momentarily + None) and for either mode via check_prompt_exclusivity (both momentarily set).""" + prompt_file = tmp_path / "system.md" + prompt_file.write_text("You are a judge.") + agent = parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt_file="system.md", system_prompt_mode=mode) + resolved = resolve_agent_system_prompt(agent, tmp_path) + assert resolved.system_prompt == "You are a judge." + assert resolved.system_prompt_file is None + assert resolved.system_prompt_mode == mode + + def test_blank_file_resolves_to_no_prompt(self, tmp_path): + """A whitespace-only file is no prompt at all — same normalization inline + prompts get, so 'replace' can never send an empty entire system prompt.""" + prompt_file = tmp_path / "system.md" + prompt_file.write_text(" \n\t\n") + agent = parse_agent_config(type=AgentKind.CLAUDE_CODE, system_prompt_file="system.md") + assert resolve_agent_system_prompt(agent, tmp_path).system_prompt is None + + +def _write_task_with_agent(path: Path, agent: dict) -> Path: + """Write a minimal task YAML carrying the given ``agent:`` block.""" + task_file = path / "t.yaml" + task_file.write_text( + yaml.dump( + { + "task_id": "t", + "description": "Test task", + "initial_prompt": "Do something", + "sandbox": {"driver": "tempdir"}, + "success_criteria": [{"type": "file_exists", "path": "x.py", "description": "exists"}], + "agent": agent, + } + ) + ) + return task_file + + +class TestSystemPromptMatrixThroughLoadTask: + """The system_prompt x system_prompt_file x system_prompt_mode matrix, exercised + through the real ``load_task`` entry point. + + Construction-only coverage is what let a load-time crash ship green: the config + validated fine and only the loader's field swap blew up. These go through the + loader so the resolved end state is what gets asserted. + """ + + @pytest.mark.parametrize("mode", ["append", "replace"]) + def test_prompt_file_is_inlined(self, tmp_path: Path, mode: str): + (tmp_path / "sp.md").write_text("You are a careful engineer.") + task_file = _write_task_with_agent( + tmp_path, {"type": "claude-code", "system_prompt_file": "sp.md", "system_prompt_mode": mode} + ) + + task, _ = load_task(task_file) + + assert isinstance(task.agent, ClaudeCodeAgentConfig) + assert task.agent.system_prompt == "You are a careful engineer." + assert task.agent.system_prompt_file is None + assert task.agent.system_prompt_mode == mode + + @pytest.mark.parametrize("mode", ["append", "replace"]) + def test_inline_prompt_survives(self, tmp_path: Path, mode: str): + task_file = _write_task_with_agent( + tmp_path, {"type": "claude-code", "system_prompt": "Be terse.", "system_prompt_mode": mode} + ) + + task, _ = load_task(task_file) + + assert isinstance(task.agent, ClaudeCodeAgentConfig) + assert task.agent.system_prompt == "Be terse." + assert task.agent.system_prompt_mode == mode + + def test_replace_without_any_prompt_is_rejected(self, tmp_path: Path): + task_file = _write_task_with_agent(tmp_path, {"type": "claude-code", "system_prompt_mode": "replace"}) + + with pytest.raises(ValueError, match="system_prompt_mode='replace' requires"): + load_task(task_file) + + def test_both_prompt_fields_is_rejected(self, tmp_path: Path): + (tmp_path / "sp.md").write_text("prompt") + task_file = _write_task_with_agent( + tmp_path, {"type": "claude-code", "system_prompt": "inline", "system_prompt_file": "sp.md"} + ) + + with pytest.raises(ValueError, match="Only one of"): + load_task(task_file) + + def test_neither_prompt_field_leaves_defaults(self, tmp_path: Path): + task_file = _write_task_with_agent(tmp_path, {"type": "claude-code"}) + + task, _ = load_task(task_file) + + assert isinstance(task.agent, ClaudeCodeAgentConfig) + assert task.agent.system_prompt is None + assert task.agent.system_prompt_mode == "append" + + def test_missing_prompt_file_names_the_path(self, tmp_path: Path): + task_file = _write_task_with_agent(tmp_path, {"type": "claude-code", "system_prompt_file": "nope.md"}) + + with pytest.raises(ValueError, match="system_prompt_file not found"): + load_task(task_file) def _make_task(agent: AgentConfig | None = None, template_sources: list | None = None) -> TaskDefinition: