Skip to content

Conversation

pgrayy
Copy link
Member

@pgrayy pgrayy commented Oct 6, 2025

Description

Support interrupting before tool calls to ask for human feedback.

  • Users can interrupt from their BeforeToolInvocationEvent hooks.
  • Interrupting places the agent in an interrupt state.
  • The result returned from the agent contains an "interrupt" stop reason along with a list of the interrupt instances.
  • Users parse the reasons from the interrupt instances to construct a list of interruptResponse prompts.
  • Users pass the responses to agent invoke when ready.

For more details, see Usage section below and comments under "Files changed".

Usage

import json
from typing import Any

from strands import Agent, tool
from strands.agent import AgentResult
from strands.experimental.hooks import BeforeToolInvocationEvent
from strands.hooks import HookProvider, HookRegistry
from strands.types.agent import AgentInput


@tool
def delete_tool(key: str) -> bool:
    print("DELETE_TOOL | deleting")
    return True


class ToolInterruptHook(HookProvider):
    def register_hooks(self, registry: HookRegistry, **kwargs: Any) -> None:
        registry.add_callback(BeforeToolInvocationEvent, self.approve)

    def approve(self, event: BeforeToolInvocationEvent):
        if event.tool_use["name"] != "delete_tool":
            return

        approval = event.interrupt("APPROVAL")
        if approval != "A":
            event.cancel_tool = "approval was not granted"


def server(agent: Agent, prompt: AgentInput) -> AgentResult:
    return agent(prompt)


def client(agent: Agent, key: str) -> dict[str, Any]:
    prompt = f"Can you delete object with key {key}"

    while True:
        result = server(agent, prompt)

        match result.stop_reason:
            case "interrupt":
                interrupts = result.interrupts
                print(f"CLIENT | interrupts=<{interrupts}> | processing interrupts")

                prompt = [
                  {
                    "interruptResponse": {
                      "name": interrupt.name,
                      "event_name": interrupt.event_name,
                      "response": input(f"(A)PPROVE or (R)EJECT {interrupt.name}: "),
                    },
                  },
                ]

            case _:
                return json.dumps(result.message, indent=2)


def app() -> None:
    agent = Agent(
        hooks=[ToolInterruptHook()],
        tools=[delete_tool],
        system_prompt="You delete objects given their keys.",
        callback_handler=None,
    )
    response = client(agent, key="X")
    print(f"APP | {response}")


if __name__ == "__main__":
    app()

A streaming example:

for event in agent.stream("Call my tool"):
  if "tool_interrupt_event" in event:
    interrupt = event["tool_interrupt_event"]["interrupt"]
    print(interrupt.reasons)

Related Issues

#204

Documentation PR

Noted for follow up.

Type of Change

New feature

Testing

How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli

  • I ran hatch run prepare: Wrote new unit tests
  • hatch test tests_integ/test_interrupt.py: Wrote new integ tests

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

Follow Ups

  • Support interruption from tool definitions
  • Would like to consider allowing customers to define tool result content in their BeforeToolCallEvent hooks.
    • This would allow customers to provide additional tool result content through the interrupt response mechanism.
  • Allow customers to provide interrupt responses preemptively as part of agent invoke while in a non-interrupt state.
    • Something akin to agent("Call my tool", interrupt_responses=[...]).
    • As an alternative, customers can build conditions around the event.interrupt(...) call to avoid execution.
  • Support same interrupt mechanism for multi-agent workflows.
  • Signal that interrupts are not supported for direct tool calls.
  • Support elicitation in MCP tool.
    • This will entail raising an interrupt exception given an elicitation response. The mechanics should be very similar.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

self.hooks.invoke_callbacks(AgentInitializedEvent(agent=self))

# Map of active interrupt instances
self._interrupts: dict[tuple[str, str], Interrupt] = {}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only populated when resuming from an interrupt (see _resume_interrupt below). It is cleared out after the interrupt has been processed.

ValueError: If any interrupts are missing corresponding responses.
"""
# Currently, users can only interrupt tool calls. Thus, to determine if the agent was interrupted in the
# previous invocation, we look for tool results with interrupt context in the messages array.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: If we setup dedicated interrupt session management, this logic becomes much simpler. For now though, we are trying to build a solution that doesn't involve changes to session management.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need a dedicated session management? can't we handle this as new message type in existing system?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now though, we are trying to build a solution that doesn't involve changes to session management.

This would be good to call out in the PR description - that a goal for this PR is to re-use message state

(actually I'm not sure how we're persisting interrupts - that definitely needs a callout in the PR description)

Copy link
Member Author

@pgrayy pgrayy Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make sense to have dedicated session management entries for interrupts to support more complex use cases. For example, if we support interrupts on other event types, it wouldn't then make sense to continue storing state in the tool results.

Also, more generally, dedicated session management entries can simplify retrieval logic as we can directly parse interrupt context instead of having to parse the messages array.

I can add details about this to the PR description.

if "json" in tool_result["content"][0] and "interrupt" in tool_result["content"][0]["json"]
]
if not reasons:
return
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During an interrupt, we store the context as tool results in the agent messages array. Consequently, for interrupts to work, users must either have session management enabled or maintain an agent instance in memory. LangGraph has the same requirement.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for interrupts to work, users must either have session management enabled or maintain an agent instance in memory

I can't imagine an implementation that doesn't have these limitations - was there a design that didn't have this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were getting there with the idea of allowing customers to pass in event type into agent invoke. Given the event type, Strands would jump straight to the location in code where emitted. This is pretty much an entirely different design though. With us utilizing the messages array and tool results for context, session management is a necessity.

yield model_event

stop_reason, message, *_ = model_event["stop"]
yield ModelMessageEvent(message=message)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We skip over model invocation if resuming from an interrupt and head straight to tool execution.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add that as a comment in the code

"role": "user",
"content": [{"toolResult": result} for result in tool_results],
}
tool_result_message = _convert_tool_results_to_message(agent, tool_results)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a _convert_prompt_to_messages method in Agent and so went with a similar name here for consistency.


name: str
event_name: str
reasons: list[Any]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We allow users to provide multiple reasons for raising an interrupt as multiple hooks can be configured on BeforeToolCallEvent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that considered a single interrupt or multiple interrupt? It feels to me that these would be differently named items (e.g. name is always 1:1 with reason)

Copy link
Member Author

@pgrayy pgrayy Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is considered a single interrupt with multiple reasons. So as an example, maybe a customer wants to configure an approval interrupt and an authorization interrupt for their tool call. They could then collect these two reasons and send them back to the user as a single interrupt.

except InterruptException:
# All callbacks are allowed to finish executing during an interrupt. The state of the interrupt is
# stored as an instance variable on the hook event.
pass
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Users can raise one interrupt per registered hook. Each reason is appended to the reasons array of a single interrupt instance.

"toolUseId": str(tool_use.get("toolUseId")),
"status": "error",
"content": [{"text": cancel_message}],
"content": [*interrupt.to_tool_result_content(), {"text": cancel_message}],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When resuming, we provide the interrupt reasons in the tool result content array.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When resuming we provide the responses or the reasons?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, could have explained this better. After completing the interrupt (a.k.a. resuming), we want to take the interrupt reasons gathered at the time of interrupt and place them into the tool result content alongside the completed results. To a final result for a tool that was interrupted would look something like:

tool_result = {
  "toolUseId": "t1",
  "status": "success",
  "content": [
    {"json": {"interrupt": {"name": "calculator_tool", "event_name": "BeforeToolCallEvent", "reasons": ["approval required"]}},
    {"text": "5"},
  ],
}

Comment on lines +319 to +324
return cast(str, cast(ToolUse, cast(dict, self.get("tool_cancel_event")).get("tool_use")).get("toolUseId"))

@property
def message(self) -> str:
"""The tool cancellation message."""
return cast(str, self["message"])
return cast(str, self["tool_cancel_event"]["message"])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typos from #964. We didn't end up using the properties however and so this is not causing issues yet. I included them though for consistency with other event definitions in this module.

from .interrupt import InterruptContent

AgentInput: TypeAlias = str | list[ContentBlock] | Messages | None
AgentInput: TypeAlias = str | list[ContentBlock] | list[InterruptContent] | Messages | None
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setup a dedicated InterruptContent type so it does not get mixed with model content blocks. Interrupt content is not to be processed by the models.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are updating the agent invoke method to start accepting kwargs. Should we instead accept this as a kwarg?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally think it is more intuitive to treat it as a prompt type. So it would be the difference between agent(responses) vs agent(interrupt_responses=responses). I know we talked about supporting preemptive responses. I think in that case, we could utilize kwargs, but I am planning that work in a follow up PR.

@pgrayy pgrayy marked this pull request as ready for review October 6, 2025 20:02
@pgrayy pgrayy requested review from zastrowm, mkmeral and Unshure October 6, 2025 20:05
interruptResponse: User response to an interrupt event.
"""

interruptResponse: InterruptResponse
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In code, this will look like:

prompt = [
  {
    "interruptResponse": {
      "name": "my_tool",
      "event_name": "BeforeToolCallEvent",
      "response": "approved",
    },
  },
  # And additional responses if multiple tools interrupted during concurrent tool execution.
]
agent(prompt)

Went with this approach for consistency. This is similar to how users would pass in a list of content blocks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two call outs with this:

(1) I feel like the combination of name & event_name make this more complicated than it should be - could we simplify to name only and allow the integrator/hook to decide if they need to disambiguate between different hooks?

(2) If the above change is made, could we simplify to using a dictionary where the key is the name of the interrupt and the value is the interrupt response? I feel like that will cut down on the verbose-ness quite a bit:

 {
    "interruptResponse": {
      ["my_tool"]: "approved"
    },
  },

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would we need to differentiate between a ToolInterrupt and some other kind of interrupt

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still like having distinct fields for clarity and consistency with the other content block types. As an example ToolResultBlock separates the content from the tool use id.

What if we provide convenience methods for constructing these responses. The result object returned from invoke includes the list of interrupt instances. We could provide something like a create_response method:

result = agent("Call my tool")
if result.stop_reason == "interrupt":
    responses = [interrupt.create_response("my response") for interrupt in result.interrupts]
    result = agent(responses)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And to be clear, we have name and event_name because we need to distinguish between an interrupt raised from a hook vs tool definition (for when we support raising in tool definitions).

Comment on lines +24 to +25
name: str
event_name: str
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using both name and event_name to help distinguish when customers raise an interrupt from a hook vs tool definition.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The naming here is a bit off - when I read name and event_name I'm not sure of the difference.

Maybe source for where the interrupt was fired from?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ I think disambiguation can be useful, but not sure how much this moves the needle compared to the user providing something like this themselves

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When customers go to use this system, the docs will clearly identify for them what the fields mean. So for example, when responding to an interrupt, we will have an example like:

responses = [{"interruptResponse": {"name": "my_tool", "event_name": "BeforeToolCallEvent", "response": "approve"}]

Do you think given this extra context, the names work? I feel source is more ambiguous personally. I'm wanting to make it clear that each interrupt has its own name along with the name of the event under which it was raised.

Copy link
Member

@zastrowm zastrowm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pointed out some items to discuss - haven't reviewed tests yet

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Support interrupting tool calls to ask for human feedback.

Nit: This is implementing support for interruption before the tool call - or at least that's what the example shows

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

ValueError: If any interrupts are missing corresponding responses.
"""
# Currently, users can only interrupt tool calls. Thus, to determine if the agent was interrupted in the
# previous invocation, we look for tool results with interrupt context in the messages array.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now though, we are trying to build a solution that doesn't involve changes to session management.

This would be good to call out in the PR description - that a goal for this PR is to re-use message state

(actually I'm not sure how we're persisting interrupts - that definitely needs a callout in the PR description)

tool_results = [
content["toolResult"]
for content in message["content"]
if "toolResult" in content and content["toolResult"]["status"] == "error"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why net set the status to interrupt?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose we could since we will alter the status again before passing to the model. That was the initial hesitation, tool result status is a bedrock field and only success and error are supported. As long as we switch to those values before resuming from interrupt, we are good. I'll update.

if "json" in tool_result["content"][0] and "interrupt" in tool_result["content"][0]["json"]
]
if not reasons:
return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for interrupts to work, users must either have session management enabled or maintain an agent instance in memory

I can't imagine an implementation that doesn't have these limitations - was there a design that didn't have this?

reasons_map = {(reason["name"], reason["event_name"]): reason for reason in reasons}
responses_map = {(response["name"], response["event_name"]): response for response in responses}
missing_keys = reasons_map.keys() - responses_map.keys()
if missing_keys:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This indicates that all interrupts need to be resolved before resuming, is that correct?

Could/should we support partial interrupts to move forward a "little".

I'm thinking of the folllowing use case:

  • Agent wants to invoke 3 tools
  • Hook runs with interrupts for all 3 tools - user must accept all 3
  • Agent resumes with approval for first tool - could/should we store the interrupt-result for that first tool and return back to the user that the other 2 need to be fulfilled too?

User experience-wise, it'd be beneficial to be able to approve tools one-by-one - if we don't support that in the agent class, do we have a way for the caller/application to indicate "tool1 was provided a response" and/or a way to know "these are all the interrupts that need to be fulfilled and we don't have all 3 yet?"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively written, instead of throwing here, should we return with stop_reason=interrupt again until we're fulfilled?

That I think would fit both use cases:

  • Application wants to always present all interrupts to the user right away
  • Application wants to present interrupts one by one (so as not to overwhelm the user)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open to that idea.


name: str
event_name: str
reasons: list[Any]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have any other name suggestions? reason leads me to think of a string, whereas the intent is for this to be anything JSON-serialiable, right?

Copy link
Member Author

@pgrayy pgrayy Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally really like reason. It is a concise variable name. And there is some symmetry with response. So you have interrupt reasons and interrupt response. I'm open to suggestions though.

Regarding what users would think, we will have examples and the type hints that make it clear this could be of any value including something JSON serializable.

name: str
event_name: str
reasons: list[Any]
response: Any = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to indicate that N things can trigger an interrupt, but they must all use the same response to resolve the interrupt - am I understanding this correctly?

If so, that seems to indicate that all interrupts for a specific name/event/name need to coordinate their Responses to be compatiable?

I'm thinking specifically one interrupt asking:

  • "Are you sure you want to invoke this tool?"
  • "Your username must be configured before invoking this tool - what is it?"

These are two interrupts which take different resolutions I think.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want one reason per interrupt, we would need to add on another identifier. There are a few options:

  • We could automatically include an interrupt index that correlates with its position in the sequence of hooks executed.
    • With this approach though, customers would have to call the hooks in the exact same sequence. This would be fine for now since we control the order of execution for hooks. Maybe one day though we allow customers to decide execution order. Even still, they probably should be executing in the same order upon resuming from an interrupt.
  • Allow customers to pass in an interrupt name themselves.
    • A problem I see with this approach though is if they use third party hooks and so there is a risk of name clashing.
  • Automatically generate an id.
    • We would have to associate the id with the hook under which the interrupt was raised. To make this association, we would need to probably get the name of the hook, but then we again run the risk of name clashes.

I think the index approach is probably the most robust. But these issues are the reason why I took the approach of setting up a reasons array.

"content": interrupt.to_tool_result_content(),
}
after_event = agent.hooks.invoke_callbacks(
AfterToolCallEvent(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long - term - perhaps after we're no longer experimental, we should consider indicating in AfterToolCallEvent if an interrupt was triggered

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yes. Actually, let me add that here in this PR.

"toolUseId": str(tool_use.get("toolUseId")),
"status": "error",
"content": [{"text": cancel_message}],
"content": [*interrupt.to_tool_result_content(), {"text": cancel_message}],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When resuming we provide the responses or the reasons?

interruptResponse: User response to an interrupt event.
"""

interruptResponse: InterruptResponse
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two call outs with this:

(1) I feel like the combination of name & event_name make this more complicated than it should be - could we simplify to name only and allow the integrator/hook to decide if they need to disambiguate between different hooks?

(2) If the above change is made, could we simplify to using a dictionary where the key is the name of the interrupt and the value is the interrupt response? I feel like that will cut down on the verbose-ness quite a bit:

 {
    "interruptResponse": {
      ["my_tool"]: "approved"
    },
  },


responses = [
{
"interruptResponse": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to mix camel case and snake here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was done for consistency with the existing content block types (example).

message: Message
metrics: EventLoopMetrics
state: Any
interrupts: list[Interrupt] | None = None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider Sequence[Interrupt]? Might be more flexible in the future if we have some HumanInterrupt or EnhancedInterrupt

yield model_event
if agent._interrupts:
stop_reason: StopReason = "tool_use"
message = agent.messages[-2]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: not clear why -2 is used

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, we only support interrupts on tools. This means that in an interrupt state, the latest message will be a tool result and the preceding message a tool use. We need to get the tool use message so we can collect the tool use requests for execution. I'll add an inline comment to make this clear.

yield model_event

stop_reason, message, *_ = model_event["stop"]
yield ModelMessageEvent(message=message)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add that as a comment in the code

self.hooks.invoke_callbacks(AgentInitializedEvent(agent=self))

# Map of active interrupt instances
self._interrupts: dict[tuple[str, str], Interrupt] = {}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use a hashable object here instead so its clear what the key is

Copy link
Member Author

@pgrayy pgrayy Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about a TypeAlias?

InterruptKey: TypeAlias = tuple[str, str]

@property
def tool_use_id(self) -> str:
"""The id of the tool cancelled."""
return cast(str, cast(ToolUse, cast(dict, self.get("tool_cancelled_event")).get("tool_use")).get("toolUseId"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bug????

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, see comment below.

message: Message,
metrics: "EventLoopMetrics",
request_state: Any,
interrupts: Optional[list[Interrupt]] = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same list v Sequence

import logging
import time
from typing import TYPE_CHECKING, Any, AsyncGenerator, cast

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

concurrent later?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you asking about a mechanism to support asking for human input concurrently or special handling in the concurrent executor.

  • If its the former, I was thinking about how we could extend the Interrupt class with an async pause method that would concurrently ask for human input and so avoid the full interrupt cycle.
  • If its the latter, there isn't any special handling needed in the concurrent executor. Interrupts are collected into tool results and tools that are not interrupted are free to finish their execution. This is in contrast to the sequential executor which breaks as soon as an interrupt is raised.

yield ToolInterruptEvent(interrupt)

interrupt_result: ToolResult = {
"toolUseId": str(tool_use.get("toolUseId")),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't tool_use['toolUseId'] already have the type str?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm just copying the pattern used elsewhere in code. Figured I wouldn't question it right now since there are so many other changes happening right now.

Comment on lines +24 to +25
name: str
event_name: str
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^ I think disambiguation can be useful, but not sure how much this moves the needle compared to the user providing something like this themselves

message: Message,
metrics: "EventLoopMetrics",
request_state: Any,
interrupts: Optional[list[Interrupt]] = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
interrupts: Optional[list[Interrupt]] = None,
interrupts:list[Interrupt] | None = None,

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used Optional for consistency but since | None is the new recommendation, I can start using it going forward and we can clean up the other uses of Optional over time.

"role": "user",
"content": [{"toolResult": result} for result in results],
}
agent.messages.append(message)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Session currently does not support overwriting a message that has already been persisted. If a tool has the json content for interrupt, and then the user continues from the interrupt, is that tool content overwritten?

from .interrupt import InterruptContent

AgentInput: TypeAlias = str | list[ContentBlock] | Messages | None
AgentInput: TypeAlias = str | list[ContentBlock] | list[InterruptContent] | Messages | None
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are updating the agent invoke method to start accepting kwargs. Should we instead accept this as a kwarg?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants