Is approval response rebinding optional when no AgentSession is passed?

[Mahiro Hirakawa (mahirhir)]

I've been reading the function-approval flow in _tools.py alongside the 0006-userapproval ADR. When a caller resumes a paused run with a FunctionApprovalResponseContent, _bind_approval_response_to_pending_request looks up the original request by id in session state and rebuilds the response from the stored function_call, so a caller can't quietly swap in different arguments before resuming. That rebinding only happens when an AgentSession is threaded through the call, though. Without one, _store_pending_approval_requests never records the original request, and the binding helper just returns the incoming response unchanged, arguments included. Is a bare, session-less resume meant to be a lower-assurance mode by design, or is the verification expected to live somewhere else in that path?

[Pham Manh Luc (MLuc24)]

Your reading holds on both halves. The storage side never records anything without a session:

def _store_pending_approval_requests(
    invocation_session: AgentSession | None,
    approval_requests: Sequence[Content],
) -> None:
    """Replace the active batch with immutable snapshots of surfaced approval requests."""
    if invocation_session is None:
        return

and the binding side hands the incoming response straight back:

def _bind_approval_response_to_pending_request(
    response: Content,
    invocation_session: AgentSession | None,
    *,
    consume: bool,
) -> Content | None:
    """Bind one approval response to a session-recorded request."""
    if invocation_session is None:
        return response

Worth adding one thing that goes further than argument rebinding, though, because the plural wrapper takes the same exit:

def _bind_approval_responses_to_pending_requests(
    messages: list[Message],
    invocation_session: AgentSession | None,
) -> None:
    """Rebind approval responses and remove unissued or duplicate responses."""
    if invocation_session is None:
        return

Its docstring promises three things, and without a session none of them run. So it is not only that arguments arrive unverified — responses carrying an id that was never issued, and duplicate responses for the same id, both survive the pass too.

That is what makes "lower-assurance mode" an uncomfortable description of it. The two paths do not differ in strength, they differ in direction: with a session an unrecognised id fails closed —

rebound = _bind_approval_response_to_pending_request(content, invocation_session, consume=True)
if rebound is None:
    logger.warning(
        "Ignored an approval response with request id %r because no pending approval request exists.",
        content.id,
    )
    continue

— and is dropped with a warning, while without one the identical input is accepted and acted on. Same bytes, opposite outcome, and the session-less side is the silent one.

As for where the verification is expected to live, the harness looks like the intended answer rather than the tool layer. ToolApprovalState is documented as "Session-backed state used by ToolApprovalMiddleware", so on that path a session exists by construction and the guards above are always in force. That would make the session-less call the bare low-level entry point rather than a supported resume mode.

If that is the intent, the gap is arguably documentation rather than code — but it would be worth being explicit, because the current shape lets a caller reach a genuinely unverified resume through a public-looking path without anything in the signature or the logs suggesting the checks were skipped. Whether that is by design is a maintainer call; the asymmetry above is the part I would want an answer on before relying on either path.