Repository navigation
Claude Agent SDK integration - #169
Conversation
641971c to
649d785
Compare
649d785 to
177fb62
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 177fb623f8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@patbeqo thanks for the decision log, it made the design easy to test. I crash-tested how it behaves when a Worker dies. I haven't run this PR's own code, so I tested what it is built on: claude-agent-sdk 0.2.163 with its bundled Claude Code 2.1.286 (this PR's lock), the SDK's 1. A Worker killed with SIGKILL leaves Claude Code running until its turn ends. After a SIGKILL of only the Worker process (for example 2. Then the README's resume can wait forever, or run next to the orphan. The README notes that a resumed session has nothing to finish if the turn's result was lost. After a kill that is the usual case, since the orphan finishes the turn: the new Claude Code sent no messages and waited for input, so the loop as written would never get a 3. A command cut off mid-run can run again, and the model isn't told. When the whole machine dies during a Bash call (simulated by SIGKILL of the Worker, Claude Code and the command at once; 6 of 6), or the Worker shuts down (the plugin's SIGTERM) and Claude Code exits before it writes the result to the session (6 of 8 shutdowns), the session has the On Claude Code 2.1.273 (claude-agent-sdk 0.2.153) and 2.1.294 (claude-agent-sdk 0.2.165, the newest this PR allows), one run each with pre-approved commands, the Worker kill (1, and the endless wait in 2) and the machine crash (3) gave the same results. What I can help with, as small follow-ups:
Happy to open any of these after this lands, or to turn the cases into tests in your harness. |
|
@osamastro7-droid Thanks, this was very useful.
|
Claude Code treats the end of its input as the end of the session and fails a pending tool approval, so resuming the session after a Worker crash finished the turn as if the approval were denied. Recover by resuming the session at the prompt with resume_session_at and telling Claude Code to continue, which asks about each tool call again however the old Claude Code stopped. - Drop ClaudeCodeProcess.terminate. Worker shutdown now closes Claude Code. - Pass control responses for requests the SDK never made through to the SDK, which ignores them, as replay-user-messages echoes the SDK's own answers. - Show the recovery in the README and the restart test.
brianstrauch
left a comment
There was a problem hiding this comment.
Thank you! Can you add this plugin to the README? You could also consider bumping the version of claude-agent-sdk to latest.
In short: users keep plain Claude Agent SDK code (
ClaudeSDKClientorquery) in a Workflowand pass it a
TemporalTransport. Claude Code runs unmodified and owns the agent loop. TemporalActivities carry its stdin and stdout, and approvals, hooks and custom tools run as Workflow code.
What was changed
Adds
python/claude_agent_sdk: thetemporalio-claude-agent-sdkpackage (Pre-release), with rootAPI
temporalio.claude_agent_sdk. Requirestemporalio>=1.35.0.TemporalTransportTransport. Pass it to your ownClaudeSDKClientorqueryin the Workflow. Its Activities areconnect,write,end_input,readandclose.ClaudeAgentSDKPluginConnector(connector=, defaultSubprocessConnector).SubprocessConnectorSubprocessCLITransport.Connector,ClaudeCodeProcessconnectreturns a process withwrite,end_input,read,closeandterminate.ConnectActivityInput,Output,TransportActivityOptionsDesign
stream-json protocol.
@tooltools.Connectordecides where Claude Code runs(Activity dependency injection),
and its Activities stay on the Worker that holds it
(Worker execution affinity).
Decision log
Approaches considered, and why this design won
options.session_storedoesn't work through a custom transportSandboxinterface + named factoriesConnectorleaves both to the usershscripts that kept Claude Code's output in files, a watch Activity that Signalled each line, heartbeated byte offsetsConnectorand Worker affinity: Temporal's own patterns instead of plugin plumbingworkflow.uuid4()init, as Claude Agent SDK apps doCLAUDE_CODE_RESUME_INTERRUPTED_TURNresume_session_at: when the Worker dies, Claude Code records a pending approval as failed, so the turn no longer looks cut offChecklist
Related: Proposal: Claude Agent SDK plugin (python/claude_agent_sdk) #31 (proposal), Add the Claude Agent SDK plugin (python/claude_agent_sdk) #33, Add the Claude Agent SDK plugin #103. Samples: Add Claude Agent SDK samples samples-python#403
How was this tested:
64 unit and integration tests on a real dev server, plus a Worker-restart run over every sample
in Add Claude Agent SDK samples samples-python#403: each resumed with the correct result.
Any docs updates needed?
The README covers setup, connectors (default and custom), recovery and Worker shutdown, long
conversations, secrets, callbacks, progress, observability and hardening. A docs.temporal.io page
is a follow-up.