Conversation
…kend The deepagents extra now requires deepagents>=0.7.12,<0.8 (0.7.14 exists but is inside the two-week exclude-newer window). deepagents 0.7 needs langsmith>=0.11.2, so the langsmith extra allows langsmith<0.13; the RunTree.patch overrides follow langsmith 0.12's Optional exclude_inputs so LangSmith's own default applies on every supported version. deepagents 0.7 made delete an optional protocol op and checks support on the wrapper's class (type(backend).delete against the protocol default), which raised AttributeError for TemporalBackend's __getattr__-based ops. The I/O ops are now real class-level methods, and the wrapper picks a subclass mirroring the inner backend's delete support, so deepagents offers the delete tool exactly when the unwrapped backend would.
There was a problem hiding this comment.
🟡 Changes recommended
The new LangSmith default-forwarding behavior needs focused regression coverage.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates Deep Agents integration to 0.7 and preserves optional backend capability detection.
Changes:
- Mirrors
delete/adeletesupport at class level and routes supported operations through Activities. - Updates Deep Agents and LangSmith compatibility ranges.
- Adds backend capability and Activity-routing tests.
File summaries
| File | Description |
|---|---|
temporalio/contrib/deepagents/_tools.py |
Implements class-level optional operation mirroring. |
tests/contrib/deepagents/test_backends.py |
Tests delete detection and Activity routing. |
temporalio/contrib/langsmith/_interceptor.py |
Preserves LangSmith’s patch-input default. |
pyproject.toml |
Updates optional dependency constraints. |
uv.lock |
Locks updated transitive dependencies. |
CHANGELOG.md |
Documents dependency and backend changes. |
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| def patch(self, *, exclude_inputs: bool = False) -> None: | ||
| """Patch the run to LangSmith, skipping during replay.""" | ||
| def patch(self, *, exclude_inputs: bool | None = None) -> None: |
There was a problem hiding this comment.
Added in b9c13f1: test_patch_forwards_langsmith_default_outside_workflow asserts the no-argument call reaches the wrapped RunTree.patch as exclude_inputs=None, and the in-workflow test now asserts the same instead of a bare assert_called_once().
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e2c94c622
ℹ️ 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".
| for _op_name in _BACKEND_OPS: | ||
| if _op_name not in _OPTIONAL_OPS: | ||
| setattr(TemporalBackend, _op_name, _make_backend_op(_op_name)) |
There was a problem hiding this comment.
When wrapping a non-sandbox backend such as StoreBackend, this loop installs execute and aexecute on the wrapper class even though those operations belong to SandboxBackendProtocol. Deepagents' class/protocol capability checks can consequently treat every TemporalBackend as shell-capable and expose the execute tool; if the model invokes it, backend_op reaches an inner backend with no such method and fails with DeepAgentsUnknownBackendOp. Mirror the inner backend's sandbox support for these two methods, as is done for delete.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Partly right, and the fix is in b9c13f1 — but the stated mechanism doesn't hold on deepagents 0.7: supports_execution is a nominal isinstance check against SandboxBackendProtocol, which is a plain ABC subclass rather than a runtime-checkable Protocol, so class-level execute/aexecute methods never made any TemporalBackend look shell-capable (verified: a wrapper around LocalShellBackend returned supports_execution() == False). The real gap ran the other way: wrapping a shell backend silently lost its shell tool. Now the mirror class derives from SandboxBackendProtocol only when deepagents itself reports the inner backend as execution-capable, with execute/aexecute dispatchers and id forwarded to the inner backend; wrappers around filesystem/store backends stay non-sandbox classes. Tests: a wrapped FilesystemBackend is not execution-capable, a wrapped LocalShellBackend is, and an end-to-end aexecute runs as a deepagents.backend_op Activity.
| target: type[TemporalBackend] = ( | ||
| _wrapper_class_for(inner) if cls is TemporalBackend else cls | ||
| ) |
There was a problem hiding this comment.
When a user subclasses the public TemporalBackend, this branch deliberately skips _wrapper_class_for, so even a subclass wrapping a delete-capable backend has no class-level delete implementation. Deepagents therefore reports delete as unsupported and omits the delete tool, whereas constructing TemporalBackend directly with the same inner backend enables it. Ensure subclasses participate in capability mirroring rather than bypassing it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Agreed, fixed in b9c13f1: mirroring now applies to any TemporalBackend subclass — the generated class derives from the user's class (so isinstance and its methods are preserved) and any capability op the subclass defines itself is left alone. Covered by test_temporal_backend_subclass_keeps_capability_mirroring.
… default Per review. deepagents decides execution support with a nominal isinstance check against SandboxBackendProtocol (an ABC subclass, not a structural Protocol), so no wrapper was ever mistaken for shell-capable; the real gap ran the other way: a wrapped LocalShellBackend lost its shell tool. The mirror class now derives from SandboxBackendProtocol only when deepagents reports the inner backend as execution-capable, with execute/aexecute dispatchers and id forwarded; plain backends stay non-sandbox classes. Mirroring also applies to user subclasses of TemporalBackend (generated as a subclass of theirs; ops they define are left alone) instead of being skipped. The langsmith RunTree.patch tests now pin that a no-argument call forwards exclude_inputs=None so LangSmith's own default applies.
TLDR:
temporalio[deepagents]now requiresdeepagents>=0.7.12,<0.8. deepagents 0.7 decides a backend's optional capabilities (delete, shellexecute) from the wrapper's class, soTemporalBackendnow mirrors the inner backend at class level instead of answering every op through__getattr__— including for user subclasses. Everything else in the plugin worked unchanged.Pins
deepagents>=0.7.12,<0.8. 0.7.14 is the latest release but falls inside the repo's two-weekexclude-newerwindow, so 0.7.12 is the newest version the lock can take today.langsmith<0.13(was<0.9): deepagents 0.7 requireslangsmith>=0.11.2. The langsmith contrib passes unchanged on 0.12.1; its twoRunTree.patchoverrides follow 0.12'sexclude_inputs: bool | None = None, and a test pins that a no-argument call forwardsNoneso LangSmith's own default applies.TemporalBackend
deleteoptional and gates its delete tool ontype(backend).delete is not BackendProtocol.delete. The old__getattr__-based ops made that class-level lookup raiseAttributeError, breaking every agent using built-in file tools. The I/O ops are now real class-level methods, andTemporalBackend(inner)picks a subclass that mirrorsinner's delete support.SandboxBackendProtocolinstance (a plain ABC subclass, not a structural Protocol). A wrappedLocalShellBackendtherefore silently lost its shell tool. When deepagents reports the inner backend as execution-capable, the mirror class also derives fromSandboxBackendProtocol, withexecute/aexecutedispatchers andidforwarded; wrappers around filesystem/store backends stay non-sandbox classes.class MyBackend(TemporalBackend)gets the same mirroring (generated as a subclass ofMyBackend; ops it defines itself are left alone).Verification
tests/contrib/deepagents: 45 passed on deepagents 0.7.12, including new tests for delete/execute mirroring, subclass participation, andaexecuterunning as abackend_opActivity through a realLocalShellBackend.tests/contrib/langsmith: 88 passed on langsmith 0.12.1.