Skip to content

fix(steer): deliver /steer as a user turn so hardened models stop rejecting it - #37618

Closed
beardthelion wants to merge 4 commits into
NousResearch:mainfrom
beardthelion:fix/steer-as-user-turn
Closed

fix(steer): deliver /steer as a user turn so hardened models stop rejecting it#37618
beardthelion wants to merge 4 commits into
NousResearch:mainfrom
beardthelion:fix/steer-as-user-turn

Conversation

@beardthelion

Copy link
Copy Markdown
Contributor

What does this PR do?

Alternative fix for the /steer false-positive on injection-resistant models, offered alongside #36938 at the issue author's request to compare both approaches head-to-head.

/steer is flagged as a possible prompt-injection attempt by hardened models because the steer text is appended into the last role:"tool" message, immediately after the <untrusted_tool_result> fence. The model sees an imperative claiming user authority inside the channel it is trained to distrust, so its indirect-injection defense fires on legitimate operator input. Confirmed independently on minimax-m3 (rejected a steer as a "user-injected instruction in the tool output ... not from you directly"), in addition to the Opus 4.8 report in #36934, so this is model-class behavior rather than one model's quirk.

Where #36938 keeps the steer in the tool channel and relabels the marker (lexical provenance), this PR removes the steer from that channel entirely and delivers it as a real role:"user" message after the tool batch (structural provenance). The two are complementary; #36938 remains the minimal-risk option.

Why it is wire-safe: delivering after every tool_call_id is answered is valid OpenAI alternation. On the Anthropic wire, _merge_consecutive_roles folds the trailing user message into the same user turn as the tool_result blocks, yielding one valid turn [tool_result, ..., {type:"text", text: steer}]. repair_message_sequence already treats a user turn as closing a tool-result run. This addresses the role-alternation concern raised in #36934 / #36938 (the issue author has acknowledged it does not hold for post-batch delivery). Prompt-cache impact is symmetric with the relabel approach, since editing the last tool message also invalidates the suffix from that point.

Related Issue

Refs #36934. Alternative to #36938 (same issue, different approach).

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • agent/agent_runtime_helpers.py: rename apply_pending_steer_to_tool_results to deliver_pending_steer_as_user_turn. When the message tail is a completed tool result, append {"role":"user", "content": "[The user sent this mid-task via /steer]\n<text>"}; otherwise restash the steer for the next-turn fallback.
  • agent/tool_executor.py: route both aggregate drain sites (parallel + sequential) through the renamed method at the batch boundary; remove the two per-tool drains. Mid-batch delivery would insert a user message before later tool_call_ids are answered, which repair_message_sequence then drops as orphans. The model only reads at the next API call (after the whole batch), so per-tool vs aggregate delivery is indistinguishable to it.
  • agent/conversation_loop.py: replace the inline pre-API tool-content append with the same helper call.
  • run_agent.py: rename the AIAgent forwarder method to match.
  • tests/run_agent/test_steer.py: rewrite TestSteerInjection and TestPreApiCallSteerDrain to assert user-turn delivery; add TestSteerCallSites guarding that the old name is gone and delivery happens only at the two batch boundaries.
  • tests/run_agent/test_concurrent_interrupt.py: rename the stubbed attribute.
  • tests/agent/test_steer_anthropic_wire.py (new): integration test that convert_messages_to_anthropic merges the steer user turn into the tool-result turn.

How to Test

  1. scripts/run_tests.sh tests/run_agent/test_steer.py tests/run_agent/test_concurrent_interrupt.py tests/agent/test_steer_anthropic_wire.py (22 tests pass).
  2. Broader steer/gateway/CLI/ACP surface: scripts/run_tests.sh tests/gateway/test_steer_command.py tests/gateway/test_busy_session_ack.py tests/cli/test_cli_steer_busy_path.py tests/acp_adapter/test_acp_commands.py (35 tests pass).
  3. Live: on a hardened model, send /steer <instruction> mid-tool-batch; the steer is honored instead of being flagged as injection. The new Anthropic-wire test pins the serialization that makes this safe.

Checklist

Code

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings): docstrings on the renamed helper explain the why; no user-facing docs affected
  • I've updated cli-config.yaml.example if I added/changed config keys: N/A (no config keys)
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows: N/A
  • I've considered cross-platform impact (Windows, macOS): N/A (pure message-list manipulation, no platform-specific code)
  • I've updated tool descriptions/schemas if I changed tool behavior: N/A (no tool behavior changed)

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Superseded by #40240 (merged 2026-06-06, commit 0f45509), which shipped the fix for #36934.

The maintainer took the structural-provenance goal this PR argued for but kept the steer in the alternation-safe tool slot rather than moving it to a user turn: STEER_CHANNEL_NOTE in the core system prompt makes the model expect and trust the exact [OUT-OF-BAND USER MESSAGE] marker, so provenance is signaled structurally (in the system prompt) without dropping the per-tool drains or risking mid-batch user insertion orphaning later tool results. The prompt-cache-neutrality this thread established is honored — the note is static text.

Verified fixed on upstream/main (88b720ebb): steer + threat-pattern tests green (69 passed), scanner clean on the reporter's phrasings, User guidance: label removed with a regression test.

Closing as superseded — thanks for the wire-safety analysis and the minimax-m3 corroboration, both of which fed the framing that landed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants