Skip to content

Commit 6bc65db

Browse files
committed
fix(tasks): don't fake-clear MCP session with empty refresh on no-config transition
An empty mcpServers list is a notification-only no-op on the agent-server (it returns without rebuilding the session), so sending it on an actor transition that resolves no configs neither clears the previous actor's session nor rebinds — yet it reported success and falsely marked the new actor. Stop sending it and leave the binding on the previous actor, who still holds the live session. Only reachable on deployments without a resolvable MCP URL.
1 parent fe5e07b commit 6bc65db

2 files changed

Lines changed: 22 additions & 13 deletions

File tree

products/tasks/backend/temporal/process_task/activities/send_followup_to_sandbox.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -317,16 +317,24 @@ def _refresh_sandbox_mcp(
317317
if imported_mcp_configs:
318318
mcp_configs = mcp_configs + imported_mcp_configs
319319

320-
if not mcp_configs and not is_transition:
320+
if not mcp_configs:
321+
if is_transition:
322+
# The new actor resolves no MCP configs, and an empty-list refresh is
323+
# a notification-only no-op on the agent-server (see
324+
# send_refresh_session) — it cannot tear down the previous actor's
325+
# live session. So we neither send nor rebind: leave the binding on
326+
# the previous actor, which accurately describes the live session,
327+
# and re-attempt on their next message. Only reachable on deployments
328+
# without a resolvable MCP URL (get_sandbox_ph_mcp_configs is
329+
# otherwise never empty), so best-effort delivery is acceptable.
330+
logger.info("refresh_mcp_no_configs_on_transition", run_id=run_id, previous_user_id=bound_user_id)
331+
return True
321332
# First bind for this sandbox and the actor has no MCP configs: there is
322333
# no prior session to tear down, so just record the binding.
323334
mark_sandbox_mcp_session(scope, actor_user.id)
324335
logger.info("refresh_mcp_skipped_no_configs", run_id=run_id)
325336
return True
326337

327-
# An actor transition where the new actor resolves no configs still has to
328-
# clear the previous actor's live session — an empty server list replaces it
329-
# wholesale. Binding stays gated on a successful send below.
330338
mcp_servers = [config.to_dict() for config in mcp_configs]
331339

332340
result = send_refresh_session(

products/tasks/backend/temporal/process_task/tests/test_send_followup_to_sandbox.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -308,24 +308,25 @@ def test_replacement_sandbox_starts_unmarked(
308308
assert get_sandbox_mcp_session_user("sb-2") == 42
309309
assert cache.get(_sandbox_mcp_session_cache_key("run-1")) == 42 # untouched
310310

311-
def test_transition_with_no_configs_still_clears_previous_session(
311+
def test_transition_with_no_configs_leaves_previous_binding(
312312
self, mock_oauth, mock_ph_configs, mock_user_configs, mock_send_refresh
313313
):
314314
# The prior actor holds the live session, but the new actor resolves no
315-
# MCP configs. Rebinding without a refresh would leave the previous
316-
# actor's session live under the new actor's binding, so we still send a
317-
# refresh — an empty server list clears it wholesale.
315+
# MCP configs. An empty-list refresh is a no-op on the agent-server, so
316+
# we neither send it nor rebind: the binding stays on the previous actor
317+
# (who still holds the live session) rather than falsely flipping to the
318+
# new one.
318319
mock_oauth.return_value = "fresh-token"
319320
mock_ph_configs.return_value = []
320321
mock_user_configs.return_value = []
321-
mock_send_refresh.return_value = CommandResult(success=True, status_code=200)
322322
mark_sandbox_mcp_session("run-1", 99)
323323

324-
_refresh(_make_task_run_mock(), actor_id=42)
324+
actor = MagicMock(id=42)
325+
safe = _refresh_sandbox_mcp(_make_task_run_mock(), "read_only", None, actor_user=actor, state=None)
325326

326-
mock_send_refresh.assert_called_once()
327-
assert mock_send_refresh.call_args.args[1] == []
328-
assert get_sandbox_mcp_session_user("run-1") == 42
327+
assert safe is True # best-effort: delivery proceeds
328+
mock_send_refresh.assert_not_called()
329+
assert get_sandbox_mcp_session_user("run-1") == 99 # binding unchanged
329330

330331

331332
class TestSendFollowupActivityRefreshOrdering:

0 commit comments

Comments
 (0)