Refactor/agentwolf v1

[Leoyzen]

No description provided.

[Leoyzen]

Bug: RunHandle.steer() skips mid-turn injection for ProtocolChannel sessions

Problem

RunHandle.steer() in orchestrator/run.py checks the CommChannel feedback path before the active_agent_run.enqueue("asap") path. Since ProtocolChannel.deliver_feedback() always returns True, steer() returns early and never reaches agent_run.enqueue(message, priority="asap").

This means for ProtocolChannel sessions (ACP, OpenCode, AG-UI servers), steer messages are not injected mid-turn — they only get queued in the CommChannel feedback queue, which is drained by _idle_loop() after the current turn ends.

This contradicts PydanticAI's asap semantics:

# PydanticAI AgentRun.enqueue() docstring:
# 'asap' (default) — at the earliest opportunity (next model request,
#     or a redirect if the agent would otherwise end).

Current code flow

def steer(self, message: str) -> bool:
    ...
    # CommChannel feedback path — ProtocolChannel returns True → EARLY RETURN
    if self._comm_channel is not None:
        feedback = Feedback(content=message, is_steer=True)
        if self._comm_channel.deliver_feedback(feedback):
            self._idle_event.set()
            return True  # ← Never reaches enqueue("asap") below

    # DirectChannel path — only reached when deliver_feedback() returns False
    if self._run_state == RunState.RUNNING:
        agent_run = self.active_agent_run
        if agent_run is not None:
            agent_run.enqueue(message, priority="asap")  # ← mid-turn injection
            return True
    ...

Impact

When a tool triggers an elicitation (awaiting human response) and a steer() is called (e.g., background task completion notification):

• DirectChannel (standalone execution): steer() → enqueue("asap") → model sees the message at before_model_request after the tool returns — ✅ correct
• ProtocolChannel (ACP/OpenCode servers): steer() → CommChannel feedback queue → only drained after the entire turn ends → model sees the message in the next turn, not mid-turn — ❌ wrong

Proposed fix

Reorder steer() to check active_agent_run.enqueue("asap") before the CommChannel feedback path:

def steer(self, message: str) -> bool:
    if self._closed:
        raise RuntimeError("Cannot steer after close()")
    if self._closing:
        return False

    # Priority 1: If running with active agent_run, inject mid-turn
    # via enqueue("asap") — aligns with PydanticAI's asap semantics.
    # This applies to BOTH DirectChannel and ProtocolChannel sessions.
    if self._run_state == RunState.RUNNING:
        agent_run = self.active_agent_run
        if agent_run is not None:
            agent_run.enqueue(message, priority="asap")
            return True

    # Priority 2: CommChannel feedback path (ProtocolChannel).
    # Used when IDLE or RUNNING without active_agent_run (turn gap).
    if self._comm_channel is not None:
        feedback = Feedback(content=message, is_steer=True)
        if self._comm_channel.deliver_feedback(feedback):
            self._idle_event.set()
            return True

    # Priority 3: DirectChannel fallback.
    if self._run_state == RunState.IDLE:
        self._message_queue.append(message)
        self._idle_event.set()
        return True

    # RUNNING without active_agent_run and no CommChannel.
    if self._run_state == RunState.RUNNING:
        self.run_ctx.queued_steer_messages.append(message)
        return True

    return False

This ensures no double delivery:

• RUNNING + active_agent_run → enqueue("asap") only (mid-turn, one delivery)
• IDLE / no active_agent_run + ProtocolChannel → CommChannel feedback (next-turn, one delivery)
• IDLE / no active_agent_run + DirectChannel → _message_queue (next-turn, one delivery)

followup() is not affected — it should remain next-turn-only (no mid-turn injection), which is correct for is_steer=False semantics.

[Leoyzen]

Steer() Bug — Dialectical Analysis & Fix

Bug Confirmation

Verified the steer() bug described in the prior comment against the actual source code:

• ProtocolChannel.deliver_feedback() (comm_channel.py L315-327): always returns True
• DirectChannel.deliver_feedback() (comm_channel.py L172-184): always returns False
• steer() (run.py L898-908): checks CommChannel feedback path first → ProtocolChannel returns True → early return → never reaches enqueue("asap") at L919

This means ProtocolChannel sessions (ACP, OpenCode, AG-UI servers) get next-turn-only delivery for steer messages, while DirectChannel sessions (standalone execution) get correct mid-turn injection. Bug confirmed.

Context: ACP session/inject RFD

Researched the upstream ACP v2 RFD PR #1261 (session/inject with queue/steer modes). Key findings:

1. ACP v1 has no steer protocol — only session/cancel + new session/prompt (destructive interrupt-then-reprompt)
2. ACP v2 RFD #1261 proposes session/inject with mode: "queue" | "steer", agent-owned messageId, user_message echo notification, and mandatory session/revoke_inject
3. Production blocker: Raft team reports steer absence is an ACP adoption blocker (tested Grok CLI 0.2.93, confirmed no mid-turn injection on public ACP transport)
4. RFD depends on v2 prompt lifecycle (non-blocking session/prompt, state_change notifications) — v2 is still in RFD stage

Architectural Gap Analysis

Capability	ACP v1	ACP v2 RFD #1261	AgentPool CommChannel (current)	AgentPool CommChannel (after fix)
Mid-turn steer	❌	✅ (at next tool boundary)	⚠️ broken for ProtocolChannel	✅ (at next model request — finer-grained)
Queue (next-turn)	❌	✅	✅ via followup()	✅
messageId handle	❌	✅ (mandatory)	❌	❌ (tracked as follow-up)
Revoke pending	❌	✅ (mandatory)	❌	❌ (tracked as follow-up)
Replace pending	❌	✅ (opt-in)	❌	❌ (tracked as follow-up)
steer_in_stream capability	❌	✅	❌	❌ (tracked as follow-up)
cancel() interaction with pending	N/A	defined (injects survive cancel)	undefined	undefined (tracked)

Proposed Fix

Reorder steer() to check enqueue("asap") before the CommChannel feedback path:

def steer(self, message: str) -> bool:
    if self._closed:
        msg = "Cannot steer after close()"
        raise RuntimeError(msg)
    if self._closing:
        return False

    # Priority 1: If running with active agent_run, inject mid-turn
    # via enqueue("asap") — aligns with PydanticAI's asap semantics.
    # Applies to BOTH DirectChannel and ProtocolChannel sessions.
    if self._run_state == RunState.RUNNING:
        agent_run = self.active_agent_run
        if agent_run is not None:
            agent_run.enqueue(message, priority="asap")
            self._idle_event.set()  # race safety: cover turn-end window
            return True

    # Priority 2: CommChannel feedback path (ProtocolChannel).
    # Used when IDLE or RUNNING without active_agent_run (turn gap).
    if self._comm_channel is not None:
        feedback = Feedback(content=message, is_steer=True)
        if self._comm_channel.deliver_feedback(feedback):
            self._idle_event.set()
            return True

    # Priority 3: DirectChannel fallback.
    if self._run_state == RunState.IDLE:
        self._message_queue.append(message)
        self._idle_event.set()
        return True

    # RUNNING without active_agent_run and no CommChannel.
    if self._run_state == RunState.RUNNING:
        self.run_ctx.queued_steer_messages.append(message)
        return True

    return False

Double-delivery safety: enqueue("asap") path does NOT call deliver_feedback(), so the CommChannel feedback queue stays empty — the post-turn drain (L853-866) won't pick up a duplicate.

Race safety: _idle_event.set() after enqueue() covers the window where the turn ends between the state check and the enqueue call. _idle_event.set() is idempotent.

Backward compatibility: DirectChannel's deliver_feedback() returns False, so it falls through to the existing logic unchanged.

Follow-up Tasks (Tracked)

The messageId / revoke / replace / steer_in_stream capability machinery from RFD #1261 requires extending the Feedback dataclass and adding a pending message management subsystem. These are tracked as separate tasks:

• Extend Feedback type with message_id: str | None and content_blocks field
• Implement session/revoke_inject semantics (pending message tombstoning)
• Define cancel() interaction with pending steer/followup (RFD: injects survive cancel)
• Add steer_in_stream capability declaration ("interrupt" vs "finish")
• Evaluate _session/inject extension method on ACP v1 as early adapter
• Map RunState → ACP v2 state_change notifications (running/idle/requires_action)