diff --git a/agent/AGENTS.md b/agent/AGENTS.md index e1f162fa77..edd8072bcc 100644 --- a/agent/AGENTS.md +++ b/agent/AGENTS.md @@ -59,7 +59,10 @@ Adding one: register in that table (no `if name == ...` chain); `tools/todo_tool commands (`agent/skill_commands.py`) inject as a user message; subdirectory `AGENTS.md` hints (`agent/subdirectory_hints.py`) append to the tool result (head+tail truncated past `_MAX_HINT_CHARS = 32_000`, with a warning). - **Strict role alternation.** Never two same-role messages in a row; never a synthetic user - message injected mid-loop. Cron deliveries live in their own session for this reason. + message injected mid-loop. The one exception is `/steer`, delivered as a standalone user row + after a tool result (`assistant(tool_calls) → tool → user` is legal on every provider path) — + never smeared onto the already-persisted tool row, which append-only persistence would leave + divergent from the live request. Cron deliveries live in their own session for this reason. - **Context files** (`agent/prompt_builder.py`) load from the CWD only at startup and are capped (`CONTEXT_FILE_MAX_CHARS` / dynamic cap from the context window / `context_file_max_chars`). Never load an install-tree `AGENTS.md` as project context (PR #64611); subdirectory hints reject diff --git a/agent/turn_iteration_prep.py b/agent/turn_iteration_prep.py index a48299ea3d..189c0c6b8f 100644 --- a/agent/turn_iteration_prep.py +++ b/agent/turn_iteration_prep.py @@ -1,7 +1,7 @@ """Outer-iteration bookkeeping for the conversation turn loop, in call order: ``begin_iteration`` (pending redirect, interrupt / review-budget / iteration-budget exits), -``prepare_iteration`` (``agent:step`` callback, skill-nudge counter, pre-API ``/steer`` drain -into the newest tool result — never a user message —, run-budget wrap-up notice, tool_call +``prepare_iteration`` (``agent:step`` callback, skill-nudge counter, pre-API ``/steer`` drain as a +standalone user row after the newest tool result, run-budget wrap-up notice, tool_call argument sanitization, interrupt-scaffold ghost-row drop, role-alternation repair), ``announce_api_call`` (verbose summary / quiet spinner) and, after the retry loop, ``apply_retry_restarts`` (consumes the ``TurnRetryState`` restart flags). Nothing here @@ -96,7 +96,7 @@ def prepare_iteration( agent: Any, *, messages: Any, api_call_count: Any, user_message: Any = None, current_turn_user_idx: Any = None, ) -> IterationPrep: """Prepare ``messages`` for this iteration in the original order. Every mutation here is - cache-safe by construction: steer text lands in the newest tool result, the ghost-row + cache-safe by construction: steer text is appended as a new (not yet persisted) user row, the ghost-row filter only drops hidden scaffold placeholders, and repair runs BEFORE the request build.""" from agent.conversation_loop import ( _INTERRUPT_SCAFFOLD_MARKER, _maybe_inject_run_budget_wrapup @@ -128,18 +128,20 @@ def prepare_iteration( except Exception: logger.debug("Nous key pre-expiry adoption failed", exc_info=True) - # Drain a /steer sent during the last API call into the newest tool message so - # it lands THIS iteration. Never put in a user message (breaks alternation). + # Drain a /steer sent during the last API call so it lands THIS iteration. Delivered as a + # standalone user row after the newest tool result (never smeared onto the tool row: that + # row is already persisted append-only, so replay would diverge from the live request and + # break the prompt cache — same contract as apply_pending_steer_to_tool_results). _pre_api_steer = agent._drain_pending_steer() if _pre_api_steer: - _inject_steer_into_newest_tool_result(agent, messages, _pre_api_steer) + _inject_steer_after_newest_tool_result(agent, messages, _pre_api_steer) - # One-shot run-budget wrap-up notice at 80% of agent.run_budget_seconds, via the - # same cache-safe channel as /steer (newest tool result); off with no budget. + # One-shot run-budget wrap-up notice at 80% of agent.run_budget_seconds, appended to the + # newest tool result; off with no budget. if getattr(agent, "run_budget_seconds", None): _maybe_inject_run_budget_wrapup(agent, messages) - # Use the same cache-safe channel as /steer; never add a synthetic user/system row. + # Appended to the newest tool result; never a synthetic user/system row. _maybe_inject_iteration_budget_warning(agent, messages) request_logger = getattr(agent, "logger", None) or logger # same name as the origin module @@ -227,26 +229,15 @@ def _previous_tool_round(messages: Any) -> list: return [] -def _inject_steer_into_newest_tool_result(agent: Any, messages: Any, steer_text: str) -> None: - """Append the steer marker to the newest tool message; with no tool message, put the - text back so the post-tool-execution drain delivers it later.""" +def _inject_steer_after_newest_tool_result(agent: Any, messages: Any, steer_text: str) -> None: + """Append the steer marker as a standalone user row after the newest tool message; with no + tool message, put the text back so the post-tool-execution drain delivers it later.""" for _si in range(len(messages) - 1, -1, -1): _sm = messages[_si] if isinstance(_sm, dict) and _sm.get("role") == "tool": from agent.prompt_builder import format_steer_marker - marker = format_steer_marker(steer_text) - existing = _sm.get("content", "") - if isinstance(existing, str): - _sm["content"] = existing + marker - else: - # Multimodal content blocks — append a text block. - with suppress(Exception): - blocks = list(existing) if existing else [] - blocks.append({"type": "text", "text": marker}) - _sm["content"] = blocks - logger.debug( - "Pre-API-call steer drain: injected into tool msg at index %d", _si - ) + messages.insert(_si + 1, {"role": "user", "content": format_steer_marker(steer_text)}) + logger.debug("Pre-API-call steer drain: appended user row after tool msg at index %d", _si) return _lock = getattr(agent, "_pending_steer_lock", None) if _lock is not None: diff --git a/tests/run_agent/test_steer.py b/tests/run_agent/test_steer.py index 929914fea4..144ae4a811 100644 --- a/tests/run_agent/test_steer.py +++ b/tests/run_agent/test_steer.py @@ -621,29 +621,27 @@ class TestPreApiCallSteerDrain: fix for the scenario where /steer sent during model thinking only lands after the agent is completely done.""" - def test_pre_api_drain_injects_into_last_tool_result(self): - """If a steer is pending when the main loop starts building - api_messages, it should be injected into the last tool result - in the messages list.""" + def test_pre_api_drain_appends_user_row_and_leaves_tool_row_untouched(self): + """A steer pending when the loop builds api_messages lands THIS iteration as a + standalone user row after the newest tool result; the (already persisted, append-only) + tool row is byte-identical afterwards so replay cannot diverge from the live request.""" + from agent.turn_iteration_prep import _inject_steer_after_newest_tool_result + agent = _bare_agent() - # Simulate messages after a tool batch completed + tool_row = {"role": "tool", "content": "output here", "tool_call_id": "tc1"} + before = dict(tool_row) messages = [ {"role": "user", "content": "do something"}, {"role": "assistant", "content": "ok", "tool_calls": [ {"id": "tc1", "function": {"name": "terminal", "arguments": "{}"}} ]}, - {"role": "tool", "content": "output here", "tool_call_id": "tc1"}, + tool_row, ] - # Steer arrives during API call (set after tool execution) agent.steer("focus on error handling") - # Simulate what the pre-API-call drain does: - _pre_api_steer = agent._drain_pending_steer() - assert _pre_api_steer == "focus on error handling" - # Inject into last tool msg (mirrors the new code in run_conversation) - for _si in range(len(messages) - 1, -1, -1): - if messages[_si].get("role") == "tool": - messages[_si]["content"] += format_steer_marker(_pre_api_steer) - break + _inject_steer_after_newest_tool_result(agent, messages, agent._drain_pending_steer()) + assert tool_row == before + assert messages[-2] is tool_row + assert messages[-1]["role"] == "user" assert STEER_MARKER_OPEN in messages[-1]["content"] assert "focus on error handling" in messages[-1]["content"] assert agent._pending_steer is None