handoff is silently discarded when it shares a tool-call batch with transfer_task

[bernard-code-lab]

Description

handleHandoff mutates the runtime's shared current agent directly:

docker-agent/pkg/runtime/agent_delegation.go

Lines 783 to 784 in 5683379

	r.executeOnAgentSwitchHooks(ctx, currentAgent, sess.ID, ca, next.Name(), agentSwitchKindHandoff)
	r.setCurrentAgent(next.Name())

r.executeOnAgentSwitchHooks(ctx, currentAgent, sess.ID, ca, next.Name(), agentSwitchKindHandoff)
r.setCurrentAgent(next.Name())

This happens outside the pin/switch machinery that runForwarding uses for transfer_task. When the model emits both tools in a single assistant response, the dispatcher runs them in parallel, and transfer_task swaps the current agent with a deferred restore:

func (r *LocalRuntime) swapCurrentAgent(...) func() {
	r.setCurrentAgent(to.Name())
	return func() {
		r.setCurrentAgent(from.Name())   // unconditional
		...
	}
}

The restore writes the pre-swap agent back unconditionally. It has no way to know that a sibling handoff legitimately changed the current agent in the meantime, so it overwrites it. The handoff is discarded with no error, no event, and no log line — the conversation simply continues with the original agent.

The two tools disagree about what the current agent means. For transfer_task it is call-scoped state to be swapped and restored; for handoff it is session state that must persist into later turns. Restoring it is correct for one and destructive for the other.

Expected Behavior

After a batch containing a handoff, the session is routed to the handoff target, whatever else shared that batch.

Actual Behavior

The current agent is back at the caller. The handoff has no effect and nothing reports the loss.

Steps to Reproduce

Root emits [transfer_task(worker), handoff(specialist)] in one response:

b := newStreamBuilder()
b.AddToolCallName("call_t", transfertask.ToolNameTransferTask).
	AddToolCallArguments("call_t", `{"agent":"worker","task":"chunk","expected_output":"r"}`)
b.AddToolCallName("call_h", handoff.ToolNameHandoff).
	AddToolCallArguments("call_h", `{"agent":"specialist"}`)

root := agent.New("root", "root agent",
	agent.WithModel(&queueProvider{id: "test/mock-model", streams: []chat.MessageStream{
		b.AddToolCallStopWithUsage(10, 5).Build(),
		newStreamBuilder().AddContent("root done").AddStopWithUsage(10, 5).Build(),
	}}),
	agent.WithSubAgents(worker),
	agent.WithHandoffs(specialist),
	agent.WithToolSets(transfertask.New(), handoff.New()),
)

rt := newDelegationRuntime(t, root, worker, specialist)
sess := session.New(session.WithUserMessage("go"), session.WithToolsApproved(true))
_, err := rt.Run(t.Context(), sess)
require.NoError(t, err)

// want "specialist", got "root"
fmt.Println(rt.CurrentAgentName(t.Context()))

Result over 5 consecutive runs:

agent after batch = "root"   (expected "specialist")

Control: dropping the transfer_task call and leaving the handoff alone in the batch yields "specialist" on every run. The batch is the trigger, not the handoff itself.

Docker Agent version

main @ 5683379 (also reproduces on the branch for #4156).

Additional context

Found while fixing #4156 in #4180. That PR fixes the caller resolution half for handoff — it used to read the shared current agent to decide who was handing off, which a sibling transfer_task could have already swapped — by resolving from the dispatcher's pre-fan-out snapshot instead.

The mutation half is deliberately left out of that PR. transfer_task's swap is call-scoped and safe to route through the pin/switch machinery; handoff rewires the session for every later turn, so giving it the same treatment is a routing design decision rather than a bug fix, and it does not belong in a concurrency patch.

Rough options, in case they're useful:

1. Have the transfer_task restore be a compare-and-restore: only write from back if the current agent is still the value the swap installed. Smallest change; makes the restore non-destructive without moving handoff.
2. Give handoff an explicit session-level routing field that resolveSessionAgent prefers, so it stops competing for the same mutable field.
3. Serialize tools that mutate the current agent within a batch.

Happy to send a PR for whichever direction you prefer — (1) is the smallest and I have the failing test ready.

[bernard-code-lab]

Implemented option (1) to check it. It does not hold, and the reproduction section above is wrong on one point. Both corrected below.

1. Correction: this does not reproduce on main

The report says it reproduces on main @ 5683379. It does not. On main the batched handoff is rejected before it ever runs:

Agent worker cannot hand off to specialist: target agent not in handoffs list.
No agents are configured in this list.

handleHandoff resolves its caller with r.currentAgentName(), which the sibling transfer_task has already swapped to its own target — the #4156 defect on the sibling tool. The call never reaches setCurrentAgent, so there is nothing for the restore to overwrite.

The clobber only becomes observable once caller resolution is fixed, i.e. on top of #4180. My original trace was taken on that branch and I attributed it to main without re-checking. Steps to reproduce should read: on top of #4180.

2. Option (1) is not viable — two variants, both reproduced

I tried both readings of "compare-and-restore":

variant	handoff survives the batch	child's force_handoff stays contained	60 runs, -race
restore only while the swap still owns the field	yes	no	—
restore the value the swap displaced, not the caller	yes	yes	3 failures
both	yes	no	0 failures

The guard variant breaks force_handoff inside a delegated child. The child calls setCurrentAgent(finisher) and the delegation's restore is what contains that change to the child session. Guarding the restore removes the containment: finisher leaks into the parent session, which then runs its own next turn as finisher. In TestTransferTask_PinningByBatchShape the target's model is invoked twice instead of once.

That is the actual obstacle. The field carries three writes with three lifetimes:

writer	scope	restore must
transfer_task	the call	undo
child's force_handoff	the child session	undo
sibling handoff	the parent session, every later turn	preserve

The last two are indistinguishable by value — both are "the field changed while the delegation ran". One has to survive and the other has to be undone, and the restore cannot tell which it is looking at. That is why the displaced-value variant still fails 3 in 60: when the handoff lands during the swap rather than before it, it looks exactly like the child's force_handoff.

The missing information is not the value. It is which session the write belongs to.

3. Option (2) is cheaper than I made it sound

I described it as giving handoff a session-level routing field. The fallback already exists — agentRouter.ResolveSession prefers a session-scoped agent over the shared one, and the type comment already calls it "the session-pin-vs-current-agent fallback".

What blocks reuse is that sess.AgentName is not only a routing field. It also drives:

• agent_router.go:77 — resolve to this agent (the part option (2) wants)
• agent_delegation.go:361 — later delegations pin their children instead of switching
• loop.go:1042 — force_handoff is skipped entirely
• skill_runner.go:144 — skill forks inherit the pin

Pointing handoff at that field would silently disable force_handoff for the rest of the session and change how every later delegation behaves — one conflation traded for another. So option (2) is not new infrastructure, it is splitting a field that already exists into "pinned" and "routed", with ResolveSession preferring either over the shared current agent.

4. How would you like to proceed?

Option (3) — serialising current-agent mutations within a batch — I have not tried; it looks like it would serialise the wrong axis, since the conflict is between scopes rather than between concurrent writers.

Happy to take option (2) with a distinct session field, or to leave it until @rumpl or @dgageot have weighed in, given this changes routing rather than fixing a race. The failing test (handoff batched with transfer_task, plus the handoff-alone control) is written and I can bring it either way.

Line references are against d1d596f.