-
Notifications
You must be signed in to change notification settings - Fork 35
feat: add explicit run resume endpoints #63
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
94e7365 to
700851a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All changes are concentrated here. Other changes in this PR result from running codegen.
dc9e258 to
c576b51
Compare
700851a to
12d751d
Compare
- add PATCH /runs/resume, /runs/resume/wait, /runs/resume/stream to let clients resume interrupted threads (thread_id required, if_not_exists removed) - new RunResume/RunResumeStream schemas; 409 on non-interrupted threads, 404 on missing threads, same streaming shape as /runs/stream - regenerate server/client stubs from updated OpenAPI so resume handlers are wired up and return Run/RunWaitResponse/SSE with run_id in responses
12d751d to
dd9cd64
Compare
jdrogers940
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Some small callouts, but otherwise makes sense.
| @@ -0,0 +1,29 @@ | |||
| # StreamMode1 | |||
|
|
|||
| The stream mode(s) to use. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OOC: what's the reason for the new stream mode version?
| "title": "On Completion", | ||
| "description": "Whether to delete or keep the thread when run completes. Must be one of 'delete' or 'keep'. Defaults to 'keep'." | ||
| }, | ||
| "on_disconnect": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is applicable for the case of /runs/resume. Maybe pull out into stream and wait objects?
| "required": [ | ||
| "run_id", | ||
| "created_at", | ||
| "updated_at", | ||
| "status", | ||
| "metadata" | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OOC: why are we including these here and not the others?
Context:
Some implementations of the Agent Protocol support "interruptions" as a way to suspend execution of an agent workflow (for e.g., user authorization flows) and later resume it with some data. Currently, the protocol is ambiguous on how this should be supported, meaning different implementations condition resuming (vs. restarting the workflow) behavior based on implementation-specific fields or conventions in the input.
This PR proposes to remove that ambiguity via an explicit set of routes to resume interrupted threads.
Proposal:
More specifically, this proposal adds three new routes
/runs/resume,/runs/resume/streamand/runs/resume/waitto mirror the existing create/wait/stream routes.They accept a payload that is nearly identical to the corresponding run creation endpoints. However, they explicitly require an underlying thread_id. It follows that the 'if_not_exists' argument is omitted for these endpoints.
If an underlying thread is not in an interrupted state, the endpoint should raise a 409 error, since the operation has a conflict with the current state of the target thread resource.
Note:
This feature implies support for the threads stage of the protocol. It is expected that resumption of a thread via
/runs/resumeor the other methods would likely create a new run to continue execution of that thread, though this is not explicitly required by the spec in this pr.This feature also still leaves the following question up to the implementation: