Skip to content

Commit 116839d

Browse files
fix(tasks): rebind sandbox MCP session on actor transitions (#70454)
1 parent c5cde18 commit 116839d

4 files changed

Lines changed: 248 additions & 219 deletions

File tree

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

Lines changed: 57 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,14 @@
2929
from products.tasks.backend.temporal.process_task.utils import (
3030
get_actor_distinct_id,
3131
get_imported_mcp_server_configs,
32+
get_sandbox_mcp_session_user,
3233
get_sandbox_ph_mcp_configs,
3334
get_task_run_credential_user,
3435
get_user_mcp_server_configs,
3536
is_slack_interaction_state,
36-
mark_mcp_token_issued,
37+
mark_sandbox_mcp_session,
3738
record_message_actor,
38-
should_refresh_mcp_token,
39+
sandbox_identity_scope,
3940
)
4041

4142
from ee.hogai.sandbox import STOP_REASON_END_TURN, TURN_COMPLETE_METHOD
@@ -158,10 +159,13 @@ def _deliver_followup(input: SendFollowupToSandboxInput) -> None:
158159
task_run, user_id=actor_user.id, distinct_id=get_actor_distinct_id(actor_user)
159160
)
160161

161-
# Push a fresh MCP config before the turn so the agent-server rebinds its
162-
# ACP session to a non-stale OAuth token. Non-fatal: if refresh fails we
163-
# still deliver the follow-up with the existing (possibly stale) creds.
164-
_refresh_sandbox_mcp(task_run, input.posthog_mcp_scopes, auth_token, actor_user=actor_user, state=state)
162+
# Rebind the sandbox's MCP session to this actor before the turn. On an
163+
# actor transition this must rebind or clear the prior session; if it can't,
164+
# fail closed rather than run the turn under the previous actor's creds.
165+
# Same-actor and first-bind refreshes stay best-effort.
166+
if not _refresh_sandbox_mcp(task_run, input.posthog_mcp_scopes, auth_token, actor_user=actor_user, state=state):
167+
error_msg = "Could not rebind sandbox MCP credentials for the follow-up actor"
168+
raise RuntimeError(f"send_followup failed: {error_msg}")
165169
artifacts = None
166170
artifact_ids = input.artifact_ids or []
167171
if artifact_ids:
@@ -256,28 +260,43 @@ def _refresh_sandbox_mcp(
256260
*,
257261
actor_user: Any,
258262
state: dict[str, Any] | None,
259-
) -> None:
260-
"""Mint a fresh OAuth token for the actor and push updated MCP configs to
261-
the sandbox.
262-
263-
Best-effort: retries once on failure, then logs and returns — a failed
264-
refresh must not block an otherwise-valid follow-up. Skipped when a token
265-
was already issued for this run within MCP_TOKEN_REFRESH_INTERVAL_SECONDS.
263+
) -> bool:
264+
"""Rebind the sandbox's MCP session to this message's actor.
265+
266+
Returns ``True`` when the session is safe to use (unchanged actor or a
267+
successful rebind) and ``False`` when a rebind could not be confirmed — the
268+
caller then fails the follow-up closed. A rebind is unconfirmed whenever the
269+
mint or refresh fails and the binding is not known to be this actor's,
270+
including an *unknown* binding: the marker self-expires at half the token
271+
lifetime, so an absent marker can mean the previous actor's session is still
272+
live, not that the sandbox is fresh. Retries the refresh once before giving
273+
up.
266274
"""
267275
run_id = str(task_run.id)
268-
if not should_refresh_mcp_token(run_id):
269-
logger.info("refresh_mcp_skipped_within_interval", run_id=run_id)
270-
return
271276
if actor_user is None:
272277
# Without a credential user the mint is guaranteed to fail; skip
273278
# quietly rather than warn on every message.
274-
return
279+
return True
280+
281+
scope = sandbox_identity_scope(run_id, state)
282+
bound_user_id = get_sandbox_mcp_session_user(scope)
283+
if bound_user_id == actor_user.id:
284+
logger.info("refresh_mcp_skipped_within_interval", run_id=run_id, user_id=actor_user.id)
285+
return True
286+
is_transition = bound_user_id is not None
287+
if is_transition:
288+
logger.info(
289+
"refresh_mcp_identity_transition",
290+
run_id=run_id,
291+
previous_user_id=bound_user_id,
292+
user_id=actor_user.id,
293+
)
275294

276295
try:
277296
access_token = create_oauth_access_token_for_run(task_run.task, state, scopes=scopes)
278297
except Exception as e:
279298
logger.warning("refresh_mcp_token_mint_failed", run_id=run_id, error=str(e))
280-
return
299+
return False # rebind unconfirmed → fail closed (unknown binding may hide a live session)
281300

282301
mcp_configs = get_sandbox_ph_mcp_configs(
283302
token=access_token,
@@ -302,8 +321,22 @@ def _refresh_sandbox_mcp(
302321
mcp_configs = mcp_configs + imported_mcp_configs
303322

304323
if not mcp_configs:
324+
if is_transition:
325+
# A prior actor holds the live session and this actor resolves no MCP
326+
# configs, so an empty-list refresh (a no-op on the agent-server)
327+
# can neither rebind it nor tear it down. Fail closed rather than run
328+
# the turn against the previous actor's retained session.
329+
logger.info(
330+
"refresh_mcp_no_configs_on_transition_fail_closed", run_id=run_id, previous_user_id=bound_user_id
331+
)
332+
return False
333+
# No recorded prior actor and no MCP configs to establish a session:
334+
# there is nothing to leak, so let the turn run rather than block the
335+
# agent just because MCP is unavailable. Record the binding so a later
336+
# actor transition is still detected.
337+
mark_sandbox_mcp_session(scope, actor_user.id)
305338
logger.info("refresh_mcp_skipped_no_configs", run_id=run_id)
306-
return
339+
return True
307340

308341
mcp_servers = [config.to_dict() for config in mcp_configs]
309342

@@ -314,9 +347,9 @@ def _refresh_sandbox_mcp(
314347
timeout=REFRESH_TIMEOUT_SECONDS,
315348
)
316349
if result.success:
317-
mark_mcp_token_issued(run_id)
350+
mark_sandbox_mcp_session(scope, actor_user.id)
318351
logger.info("refresh_mcp_delivered", run_id=run_id, attempts=1)
319-
return
352+
return True
320353

321354
logger.info(
322355
"refresh_mcp_retrying",
@@ -332,16 +365,17 @@ def _refresh_sandbox_mcp(
332365
timeout=REFRESH_TIMEOUT_SECONDS,
333366
)
334367
if retry.success:
335-
mark_mcp_token_issued(run_id)
368+
mark_sandbox_mcp_session(scope, actor_user.id)
336369
logger.info("refresh_mcp_delivered", run_id=run_id, attempts=2)
337-
return
370+
return True
338371

339372
logger.warning(
340373
"refresh_mcp_failed",
341374
run_id=run_id,
342375
error=retry.error,
343376
status_code=retry.status_code,
344377
)
378+
return False # rebind never confirmed → fail closed (unknown binding may hide a live session)
345379

346380

347381
def _get_stop_reason(result_data: dict[str, Any] | None) -> str:

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
get_sandbox_ph_mcp_configs,
3131
get_task_run_credential_user,
3232
get_user_mcp_server_configs,
33-
mark_mcp_token_issued,
33+
mark_sandbox_mcp_session,
3434
)
3535

3636
from .get_task_processing_context import TaskProcessingContext
@@ -167,6 +167,9 @@ class StartAgentServerOutput:
167167
class _LaunchParams:
168168
mcp_configs: list[McpServerConfig]
169169
relayed_mcp_servers: list[str]
170+
# The user the boot-time credentials were minted for, recorded as the
171+
# sandbox's initial session identity.
172+
actor_user_id: int | None
170173
agentsh_domains: list[str] | None
171174
protected_base_branch: str | None
172175
event_ingest_token: str | None
@@ -298,6 +301,7 @@ def _prepare_launch(ctx: TaskProcessingContext, scopes: PosthogMcpScopes) -> _La
298301
return _LaunchParams(
299302
mcp_configs=mcp_configs,
300303
relayed_mcp_servers=relayed_names,
304+
actor_user_id=actor_user.id if actor_user else None,
301305
agentsh_domains=agentsh_domains,
302306
protected_base_branch=protected_base_branch,
303307
event_ingest_token=event_ingest_token,
@@ -340,10 +344,10 @@ def _invoke_start_agent_server(
340344
rtk_enabled=ctx.rtk_enabled,
341345
)
342346

343-
# Mark startup-time token issuance so follow-ups within the next
344-
# 30m window skip the redundant refresh.
345-
if params.mcp_configs:
346-
mark_mcp_token_issued(ctx.run_id)
347+
# Record the boot identity so same-actor follow-ups within the
348+
# freshness window skip the redundant refresh.
349+
if params.mcp_configs and params.actor_user_id is not None:
350+
mark_sandbox_mcp_session(sandbox.id, params.actor_user_id)
347351

348352
# Persist the effective rtk posture the agent launched with, so terminal
349353
# analytics can cohort runs by it (the state override alone misses the

0 commit comments

Comments
 (0)