feat(agent): client-tool round-trip backend (#4920)#4925
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@ardaerzin — this is your counterpart for the FE round-trip design. The backend is done (see PR description). Your frontend slice needs:
The design file has the full detail: docs/design/agent-workflows/projects/agent-fe-roundtrip/design.md (section "Work split -> Arda (FE)"). |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
services/agent/tests/unit/sandbox-agent-permissions.test.ts (1)
36-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd direct coverage for the client-tool branch.
These stubs keep the expanded
Responderinterface satisfied, but the newattachPermissionResponderpath should also assert a request withtoolCall.spec.kind === "client"emitskind: "client_tool", parks without replying, and maps deny/output outcomes throughclientToolReply().Also applies to: 108-110, 147-149
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8737fc66-7334-4508-82bf-da22b81679d9
📒 Files selected for processing (11)
api/oss/src/apis/fastapi/tools/router.pyapi/oss/src/apis/fastapi/workflows/router.pyapi/oss/src/core/workflows/static_catalog.pysdks/python/agenta/sdk/agents/adapters/vercel/messages.pysdks/python/agenta/sdk/agents/platform/workflow.pyservices/agent/src/engines/sandbox_agent/permissions.tsservices/agent/src/responder.tsservices/agent/src/tools/dispatch.tsservices/agent/tests/unit/responder.test.tsservices/agent/tests/unit/sandbox-agent-orchestration.test.tsservices/agent/tests/unit/sandbox-agent-permissions.test.ts
| async def _emit_data_event( | ||
| *, | ||
| request: Request, | ||
| name: str, | ||
| data: dict, | ||
| ) -> None: | ||
| emit = getattr(request.state, "emit", None) or getattr( | ||
| request.state, "emit_event", None | ||
| ) | ||
| if not callable(emit): | ||
| return | ||
|
|
||
| result = emit({"type": "data", "name": name, "data": data}) | ||
| if isawaitable(result): | ||
| await result |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Don’t let UI event delivery fail a successful tool call.
If the request-state emitter throws here, _call_workflow_tool() returns an error even though the workflow already ran and produced outputs. This helper should be best-effort for the same reason as the workflow router variant.
| await _emit_committed_revision_data_event( | ||
| request=request, | ||
| workflow_revision=workflow_revision, | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Emit the same event from the normal commit endpoint.
This only covers create_workflow_revision(). The regular commit path at Line 1500 still returns without a committed-revision event, so follow-up commits will not trigger the frontend revision refresh that this PR is adding.
Minimal follow-up
# In commit_workflow_revision(), after invalidate_cache(...)
await _emit_committed_revision_data_event(
request=request,
workflow_revision=workflow_revision,
)| async def _emit_data_event( | ||
| *, | ||
| request: Request, | ||
| name: str, | ||
| data: dict, | ||
| ) -> None: | ||
| emit = getattr(request.state, "emit", None) or getattr( | ||
| request.state, "emit_event", None | ||
| ) | ||
| if not callable(emit): | ||
| return | ||
|
|
||
| result = emit({"type": "data", "name": name, "data": data}) | ||
| if isawaitable(result): | ||
| await result |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make data-event emission best-effort.
If request.state.emit fails here after the revision was already committed, the API turns a successful mutation into a 5xx and invites duplicate retries. Swallow/log emitter failures instead of letting them escape the request.
| if ( | ||
| tool_name is None | ||
| and ptype.startswith("tool-") | ||
| and ptype != TOOL_OUTPUT_AVAILABLE | ||
| and ptype not in TOOL_OUTPUT_AVAILABLE_TYPES | ||
| ): | ||
| tool_name = ptype[len("tool-") :] | ||
|
|
||
| blocks: List[ContentBlock] = [] | ||
| if ptype != TOOL_OUTPUT_AVAILABLE or "input" in part: | ||
| if ptype not in TOOL_OUTPUT_AVAILABLE_TYPES or "input" in part: |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Don’t synthesize tool names for generic result parts.
Line 100 only exempts tool-output-available. tool-output-error and tool-output-denied still fall through the tool- prefix path, so parts emitted by sdks/python/agenta/sdk/agents/adapters/vercel/stream.py:137-157 without toolName get parsed as tool_name="output-error" / "output-denied" and also produce a bogus tool_call at Line 105. That breaks the documented neutral tool_result mapping for non-success tool results.
Suggested fix
-TOOL_OUTPUT_AVAILABLE_TYPES = {TOOL_OUTPUT_AVAILABLE, "TOOL_OUTPUT_AVAILABLE"}
+TOOL_OUTPUT_AVAILABLE_TYPES = {TOOL_OUTPUT_AVAILABLE, "TOOL_OUTPUT_AVAILABLE"}
+GENERIC_TOOL_RESULT_TYPES = {
+ *TOOL_OUTPUT_AVAILABLE_TYPES,
+ "tool-output-error",
+ "tool-output-denied",
+}
@@
- if (
- tool_name is None
- and ptype.startswith("tool-")
- and ptype not in TOOL_OUTPUT_AVAILABLE_TYPES
- ):
+ if (
+ tool_name is None
+ and ptype.startswith("tool-")
+ and ptype not in GENERIC_TOOL_RESULT_TYPES
+ ):
tool_name = ptype[len("tool-") :]
@@
- if ptype not in TOOL_OUTPUT_AVAILABLE_TYPES or "input" in part:
+ if ptype not in GENERIC_TOOL_RESULT_TYPES or "input" in part:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( | |
| tool_name is None | |
| and ptype.startswith("tool-") | |
| and ptype != TOOL_OUTPUT_AVAILABLE | |
| and ptype not in TOOL_OUTPUT_AVAILABLE_TYPES | |
| ): | |
| tool_name = ptype[len("tool-") :] | |
| blocks: List[ContentBlock] = [] | |
| if ptype != TOOL_OUTPUT_AVAILABLE or "input" in part: | |
| if ptype not in TOOL_OUTPUT_AVAILABLE_TYPES or "input" in part: | |
| if ( | |
| tool_name is None | |
| and ptype.startswith("tool-") | |
| and ptype not in GENERIC_TOOL_RESULT_TYPES | |
| ): | |
| tool_name = ptype[len("tool-") :] | |
| blocks: List[ContentBlock] = [] | |
| if ptype not in GENERIC_TOOL_RESULT_TYPES or "input" in part: |
| def _is_request_connection_workflow(tool_config: ReferenceToolConfig) -> bool: | ||
| workflow = getattr(tool_config, "workflow", None) | ||
| if getattr(workflow, "slug", None) == REQUEST_CONNECTION_WORKFLOW_SLUG: | ||
| return True | ||
|
|
||
| call_ref = tool_config.call_ref | ||
| return ( | ||
| call_ref == f"workflow.variant.{REQUEST_CONNECTION_WORKFLOW_SLUG}" | ||
| or call_ref.startswith(f"workflow.variant.{REQUEST_CONNECTION_WORKFLOW_SLUG}.") | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Handle environment-scoped request-connection references too.
Line 116 is checking tool_config.workflow.slug, but ReferenceToolConfig exposes slug directly. As written, the special case only matches workflow.variant... call refs, so ref_by="environment" tools targeting __ag__request_connection resolve as ordinary callback tools and bypass the client-tool flow.
Suggested fix
def _is_request_connection_workflow(tool_config: ReferenceToolConfig) -> bool:
- workflow = getattr(tool_config, "workflow", None)
- if getattr(workflow, "slug", None) == REQUEST_CONNECTION_WORKFLOW_SLUG:
+ if tool_config.slug == REQUEST_CONNECTION_WORKFLOW_SLUG:
return True
call_ref = tool_config.call_ref
return (
call_ref == f"workflow.variant.{REQUEST_CONNECTION_WORKFLOW_SLUG}"
or call_ref.startswith(f"workflow.variant.{REQUEST_CONNECTION_WORKFLOW_SLUG}.")
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def _is_request_connection_workflow(tool_config: ReferenceToolConfig) -> bool: | |
| workflow = getattr(tool_config, "workflow", None) | |
| if getattr(workflow, "slug", None) == REQUEST_CONNECTION_WORKFLOW_SLUG: | |
| return True | |
| call_ref = tool_config.call_ref | |
| return ( | |
| call_ref == f"workflow.variant.{REQUEST_CONNECTION_WORKFLOW_SLUG}" | |
| or call_ref.startswith(f"workflow.variant.{REQUEST_CONNECTION_WORKFLOW_SLUG}.") | |
| ) | |
| def _is_request_connection_workflow(tool_config: ReferenceToolConfig) -> bool: | |
| if tool_config.slug == REQUEST_CONNECTION_WORKFLOW_SLUG: | |
| return True | |
| call_ref = tool_config.call_ref | |
| return ( | |
| call_ref == f"workflow.variant.{REQUEST_CONNECTION_WORKFLOW_SLUG}" | |
| or call_ref.startswith(f"workflow.variant.{REQUEST_CONNECTION_WORKFLOW_SLUG}.") | |
| ) |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
🤖 The AI agent says: What to review on this PR:
|
be5bd45 to
76b89f4
Compare
| return workflow_revision_resolve_response | ||
|
|
||
|
|
||
| async def _emit_committed_revision_data_event( |
There was a problem hiding this comment.
Again, I'm confused by this whole part.
There was a problem hiding this comment.
🤖 The AI agent says:
This is the frontend refresh signal for the agent-builds-an-app flow, and it pairs with the FE slice in #4934.
When the agent commits a workflow revision on the user's behalf (it self-builds the app while the chat is streaming), the browser needs to refresh the active variant/revision without a full page reload. _emit_committed_revision_data_event is how the backend tells it to: after create_workflow_revision commits, it emits a stream "data" event named committed-revision carrying {variantId, revisionId, version}.
_emit_data_event looks up request.state.emit (or emit_event). That callback is attached only when the request runs inside an agent streaming run — the runner installs it so server-side commits made mid-turn can push frames out on the same stream the chat is using. On a plain REST call there is no emitter, so it silently no-ops (a normal API caller gets nothing extra). The FE (#4934, item #5 in the handoff) listens for data-committed-revision and refreshes the active revision in place.
Two related notes so the picture is complete:
- The tools router has a sibling,
_emit_committed_revision_data_event_from_outputs, that fires the same event when a workflow-tool run commits a revision (the agent committing via a reference tool rather than the direct commit endpoint). - CodeRabbit flagged that the regular commit endpoint (
commit_workflow_revision, ~line 1500) does not emit this yet — only the create/initial path does. If we want the refresh on every commit and not just the first, that's a one-line follow-up. I left it out of feat(agent): client-tool round-trip backend (#4920) #4925 on purpose since the agent-build path goes through create; tell me if you want it on the regular commit path too.
There was a problem hiding this comment.
🤖 The AI agent says: Thanks — no change needed here.
|
|
||
| if ( | ||
| ptype == TOOL_OUTPUT_AVAILABLE | ||
| ptype in TOOL_OUTPUT_AVAILABLE_TYPES |
There was a problem hiding this comment.
I don't understand this change.
There was a problem hiding this comment.
🤖 The AI agent says:
This is in the Vercel → neutral-content-block adapter. A Vercel message "part" carries a type string. Tool-related parts arrive in three shapes: a typed tool call tool-<name> (e.g. tool-get_weather), a dynamic-tool part, and a generic result part tool-output-available. The dispatcher routes all three to _tool_part_blocks.
The change turns a single string equality into a set membership check (ptype in TOOL_OUTPUT_AVAILABLE_TYPES, where the set holds both spellings of the marker — the wire value tool-output-available and its uppercased enum-name form). It's used in two places inside _tool_part_blocks:
- Name derivation. When a part has no explicit
toolName, we strip thetool-prefix to recover the name (tool-get_weather→get_weather). Buttool-output-availableis a result envelope, not a tool named "output-available" — its real toolName lives elsewhere. The set excludes it from prefix-stripping so we don't synthesize a bogus name. - Whether to emit a
tool_callblock. A result-only part should map to atool_result, not also fabricate atool_call. The set guards that too.
So the casing/role mapping is: "treat either spelling of the tool-output-available marker as a result, never as a tool name or a call." Functionally the same intent as before; the set just makes it tolerate both spellings instead of one exact string.
One honest follow-up: CodeRabbit pointed out that tool-output-error and tool-output-denied still fall through the tool- prefix path and would get parsed as tool_name="output-error" / "output-denied". That's a real edge for non-success client-tool results. It's a separate, narrow fix (widen the guard set to include those two); I can fold it in here or take it as a follow-up — your call.
There was a problem hiding this comment.
please fix that point here before mergin
There was a problem hiding this comment.
🤖 The AI agent says:
Fixed in af0e4d9. The guard now covers all three result-envelope markers, not just tool-output-available. I introduced TOOL_OUTPUT_MARKER_TYPES = {tool-output-available, tool-output-error, tool-output-denied} and use it at the three guard points. So on the round-trip ingest path none of these tool- prefixed result markers get parsed into a tool name or fabricate a spurious tool_call — they are treated as result envelopes: tool-output-error (carrying errorText) becomes a tool_result with is_error=True, and tool-output-denied no longer produces a phantom tool call.
mmabrouk
left a comment
There was a problem hiding this comment.
Please address my comments and answer them, both here in the comments and in chat.
Address review on PR #4925: add an inline comment explaining why a kind:"client" permission request is forwarded to the browser as a client_tool interaction_request instead of being gated in the sandbox. Claude-Session: https://claude.ai/code/session_01LUTGjUDzSX1tiJwqnqACtj
5957270 to
aa385fa
Compare
Frontend for the client-tool round-trip backend (#4925): the runner emits-and-parks a browser-fulfilled tool, the playground fulfills it and resumes the run. - Generalize the resume predicate (agentApprovalResume) so a settled client-tool output resumes the run, not only an approval response — covers request_connection and any future client tool. Guards against an approved-then-run tool being mistaken for a client-tool result. Unit-tested for both paths. - Client-tool dispatcher (clientTools/): registry keyed render.kind -> toolName -> generic fallback, plus an explicit "can't handle that" surface that settles the part so a parked call never hangs silently. v1 ships the connect widget. - Connect widget (request_connection): reuses the existing connection machinery (useToolsConnections + OAuth popup); validates the callback postMessage origin against the Agenta API origin before trusting it; settles on every terminal path (success, cancel, abandon, timeout). Result UX is a compact inline status chip. - Refresh on commit: listen for the data-committed-revision stream part and invalidate the latest-revision + inspect caches, so the config panel refreshes in both the gated and direct commit paths. Verified: tsc clean (entities/playground), oss tsc no net-new errors, eslint + prettier clean, resume-predicate unit tests pass.
RUNNER: add onClientTool to Responder; detect client specs in
permissions.ts and emit interaction_request(kind=client_tool) with park;
remove the throw in dispatch.ts (return pending signal instead);
rename approvalKey -> parkedCallKey everywhere; generalize
extractApprovalDecisions to handle both {approved} and structured output.
SDK: widen tool-output-available parsing in messages.py; add
__ag__request_connection static workflow resolving to ClientToolSpec
(kind=client, render={kind:connect}) in platform/workflow.py.
API: register __ag__request_connection in static_catalog; emit
data-committed-revision event after successful commit_revision in both
tools/router.py and workflows/router.py.
All checks: ruff format+check (API+SDK), tsc --noEmit, vitest (44 tests pass).
Frontend (message-part dispatcher, connect widget, resume predicate,
result UX, config refresh) is NOT included — owned by @ardaerzin.
…4920) The reserved __ag__request_connection workflow stored its inline tool with the resolved-spec key `kind:"client"` instead of the tool-config discriminator `type:"client"`. The `tools` field holds configs (discriminated by `type`), so after embed resolution the value coerced to a builtin tool named request_connection rather than a ClientToolConfig. Store `type:"client"` so it resolves to a ClientToolSpec (render: connect). Claude-Session: https://claude.ai/code/session_01GYo3UEfvsZpncagqb28Mbc
Address review on PR #4925: add an inline comment explaining why a kind:"client" permission request is forwarded to the browser as a client_tool interaction_request instead of being gated in the sandbox. Claude-Session: https://claude.ai/code/session_01LUTGjUDzSX1tiJwqnqACtj
…tted-revision on the regular commit endpoint
Frontend for the client-tool round-trip backend (#4925): the runner emits-and-parks a browser-fulfilled tool, the playground fulfills it and resumes the run. - Generalize the resume predicate (agentApprovalResume) so a settled client-tool output resumes the run, not only an approval response — covers request_connection and any future client tool. Guards against an approved-then-run tool being mistaken for a client-tool result. Unit-tested for both paths. - Client-tool dispatcher (clientTools/): registry keyed render.kind -> toolName -> generic fallback, plus an explicit "can't handle that" surface that settles the part so a parked call never hangs silently. v1 ships the connect widget. - Connect widget (request_connection): reuses the existing connection machinery (useToolsConnections + OAuth popup); validates the callback postMessage origin against the Agenta API origin before trusting it; settles on every terminal path (success, cancel, abandon, timeout). Result UX is a compact inline status chip. - Refresh on commit: listen for the data-committed-revision stream part and invalidate the latest-revision + inspect caches, so the config panel refreshes in both the gated and direct commit paths. Verified: tsc clean (entities/playground), oss tsc no net-new errors, eslint + prettier clean, resume-predicate unit tests pass.
Frontend for the client-tool round-trip backend (#4925): the runner emits-and-parks a browser-fulfilled tool, the playground fulfills it and resumes the run. - Generalize the resume predicate (agentApprovalResume) so a settled client-tool output resumes the run, not only an approval response — covers request_connection and any future client tool. Guards against an approved-then-run tool being mistaken for a client-tool result. Unit-tested for both paths. - Client-tool dispatcher (clientTools/): registry keyed render.kind -> toolName -> generic fallback, plus an explicit "can't handle that" surface that settles the part so a parked call never hangs silently. v1 ships the connect widget. - Connect widget (request_connection): reuses the existing connection machinery (useToolsConnections + OAuth popup); validates the callback postMessage origin against the Agenta API origin before trusting it; settles on every terminal path (success, cancel, abandon, timeout). Result UX is a compact inline status chip. - Refresh on commit: listen for the data-committed-revision stream part and invalidate the latest-revision + inspect caches, so the config panel refreshes in both the gated and direct commit paths. Verified: tsc clean (entities/playground), oss tsc no net-new errors, eslint + prettier clean, resume-predicate unit tests pass.
af0e4d9 to
d366270
Compare
Frontend for the client-tool round-trip backend (#4925): the runner emits-and-parks a browser-fulfilled tool, the playground fulfills it and resumes the run. - Generalize the resume predicate (agentApprovalResume) so a settled client-tool output resumes the run, not only an approval response — covers request_connection and any future client tool. Guards against an approved-then-run tool being mistaken for a client-tool result. Unit-tested for both paths. - Client-tool dispatcher (clientTools/): registry keyed render.kind -> toolName -> generic fallback, plus an explicit "can't handle that" surface that settles the part so a parked call never hangs silently. v1 ships the connect widget. - Connect widget (request_connection): reuses the existing connection machinery (useToolsConnections + OAuth popup); validates the callback postMessage origin against the Agenta API origin before trusting it; settles on every terminal path (success, cancel, abandon, timeout). Result UX is a compact inline status chip. - Refresh on commit: listen for the data-committed-revision stream part and invalidate the latest-revision + inspect caches, so the config panel refreshes in both the gated and direct commit paths. Verified: tsc clean (entities/playground), oss tsc no net-new errors, eslint + prettier clean, resume-predicate unit tests pass.
Context
Some tools the agent uses require user interaction before they can complete. The OAuth connection flow is the main case: the agent asks the user to connect an integration (GitHub, Slack, etc.), the user finishes sign-in in the browser, and the run must resume with the result. The existing runner handled only
approved/deniedpermission decisions and had no path for structured output from a client-side tool call.This PR wires the backend side of that round-trip. The frontend slice (message-part dispatcher, connect widget, resume predicate, result UX, config refresh on commit) is a separate deliverable, coordinated with @ardaerzin.
Design:
docs/design/agent-workflows/projects/agent-fe-roundtrip/design.mdChanges
TS runner (
services/agent/):responder.ts—RespondergainsonClientTool(req).ApprovalDecisionsnow holds{ output: unknown }in addition to permission decisions.parkedCallKeyreplacesapprovalKeyeverywhere (the old name implied approval-only semantics).permissions.ts— Before dispatching a tool, checksspec.kind === "client". If so, emitsinteraction_request(kind: "client_tool")and parks viaresponder.onClientTool. The tool call stays parked until the frontend sends a result.dispatch.ts— Client tools no longer throw. They return a{type: "client_tool_pending"}signal; the permission responder owns the park.SDK (
sdks/python/):adapters/vercel/messages.py—TOOL_OUTPUT_AVAILABLE_TYPESis now a set containing both"tool-output-available"and"TOOL_OUTPUT_AVAILABLE". The previous check was case-exact and dropped uppercase variants from some clients.agents/platform/workflow.py— Registers__ag__request_connectionas a hard-coded client tool that resolves toClientToolSpec(kind="client", render={"kind": "connect"}). Agents that include this workflow as a tool get the connect affordance without any backend call.API:
core/workflows/static_catalog.py— Registers__ag__request_connectionunder URIclient:tool:request_connection:v0alongside the getting-started skill.apis/fastapi/tools/router.pyandworkflows/router.py— Bothcommit_revisionpaths now emit a one-waydata-committed-revisionevent{variantId, revisionId, version}after a successful commit. The frontend uses this to refresh the build-kit state without a separate poll.Scope / risk
This is the backend slice only. The interaction loop is incomplete until the frontend dispatches
client_toolinteraction events and sends results back. Until that lands, akind: "client"tool call will emit the event and park but never resume. The agent will appear to hang at that point.The
_emit_data_eventhelper in both API routers resolvesrequest.state.emitwith a fallback toemit_event. If neither attribute exists (e.g., outside a streaming context), it exits silently. Reviewers should confirm theemitattribute is reliably present onrequest.statein the tool-call and workflow-commit code paths, or that silent failure is the right behavior.The
parkedCallKeyrename is purely cosmetic. It does not change behavior. The test files were updated to match.Tests
responder.test.ts,sandbox-agent-orchestration.test.ts,sandbox-agent-permissions.test.tsupdated forparkedCallKeyrename and expandedResponderinterface.ruff format+checkclean on API + SDK.tsc --noEmitclean.How to QA
Prerequisites: local dev stack with the agent sidecar running. The frontend slice is not yet merged, so the round-trip cannot be exercised end-to-end. This PR can be verified at the unit-test level only.
Automated tests:
What to verify: Run the TS tests and confirm the
parkedCallKeyrename did not break any assertion. Confirm the API router tests still pass after the_emit_data_eventaddition.Edge cases: The
_emit_data_eventcall in the tools router readsoutputsas a dict and exits ifvariantId/revisionId/versionare absent. Confirm acommit_revisioncall that returns a non-dict output does not raise.https://claude.ai/code/session_01GYo3UEfvsZpncagqb28Mbc