feat(runner): client tools on Claude in remote sandboxes#5383
Conversation
Plan-only workspace for delivering browser-fulfilled client tools to the Claude harness in remote Daytona sandboxes. Reuses the existing file relay (the Pi-on-Daytona pattern); adds no new runner-to-sandbox network path. Plans #5256 and closes the residual mixed-set silent-drop from #4984. Claude-Session: https://claude.ai/code/session_01DnWRxU3dCJ11hgDidm26vW
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis change enables Claude client tools on Daytona by advertising them through the in-sandbox shim, adding paused relay responses, returning benign MCP results, and preserving cold-replay resumption. It removes the Daytona client-only refusal and adds documentation and unit-test coverage. ChangesDaytona client-tool delivery
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Claude
participant DaytonaShim
participant RunnerRelay
participant Browser
Claude->>DaytonaShim: Call advertised client tool
DaytonaShim->>RunnerRelay: Submit relay request
RunnerRelay-->>DaytonaShim: Write paused relay answer
DaytonaShim-->>Claude: Return non-error wait result
Browser->>RunnerRelay: Complete client interaction
RunnerRelay-->>Claude: Resume with browser result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
Plan review: approved with four decisions on the open questions, binding for the implementation.
-
The pause-answer gate: name the capability, not the harness. Use a
writePausedAnswerflag on the plan (set true for non-Pi today) rather than branching on!plan.isPi. A third Daytona harness then changes one assignment, not a scattered condition. -
Correlation of identical calls: the live QA MUST force the case of two client tools with identical name and arguments in one turn. Implementation uses first-in-first-out matching per (name, arguments) and documents that limitation in the code. Do not build extra id plumbing preemptively; if the forced test shows a widget attaching to the wrong call, add the shim's request id as the correlating hint then, as its own small commit.
-
The paused tool result text: a fixed template that includes the tool name, along the lines of 'The request is waiting for the user's response in the app. Do not retry it; continue or end your reply.' The model reasons more coherently when it knows which call is parked, and instructing it not to retry avoids loop-shaped transcripts.
-
Relay file cleanup: explicit delete of the request/response pair at the moment the paused answer is written. Deterministic cleanup beats relying on the sweep; the existing stale-file sweep stays as the backstop for crashes.
Also noting for the record: the planner's two corrections to the research (the spec-file assembly lives in environment.ts, and the Daytona pause seam is already armed) were verified claims, and the plan is better for them. Implementation may start once CodeRabbit's pass is in and Mahmoud has had his look.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4bae63cd-3ff9-42c4-ac61-f1be0f9fa782
📒 Files selected for processing (2)
docs/design/agent-workflows/projects/client-tools-daytona/PLAN.mddocs/design/agent-workflows/projects/client-tools-daytona/README.md
| - **Paused relay answer (Step 3)**: when `executeRelayedTool` returns `PAUSED` on the non-Pi | ||
| Daytona path, the loop writes `{ ok: true, paused: true }`; on the Pi path it writes no answer | ||
| file (unchanged). `relayToolCall` maps a paused answer to its paused result, not a throw. The | ||
| shim's `tools/call` maps a paused result to a benign, non-error content result. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Add regression assertions for late timeout frames and stale relay files.
The proposed tests verify that a paused answer exists and that resume succeeds, but they do not prove the core #5256 requirement: no late timeout/error frame arrives after teardown, and a paused .req.json/.res.json pair cannot satisfy a fresh call. Add these assertions to the unit and release-gate coverage.
Also applies to: 245-259
| combination. Each step is revertable on its own: | ||
|
|
||
| - **Step 1** is revertable: restore the `executableToolSpecs` filter at the upload and | ||
| advertisement gates. Client tools stop being advertised again. | ||
| - **Step 2** adds no code (or a small correlation refinement), so there is nothing to roll back | ||
| beyond that refinement. | ||
| - **Step 3** is the load-bearing change and is revertable: remove the `paused` field, restore | ||
| `if (text === PAUSED) return;`, and revert the shim's benign-result branch. Behavior returns | ||
| to Block B and Block C. | ||
| - **Step 4** adds no code. | ||
| - **Step 5** is revertable: restore the `DAYTONA_CLIENT_ONLY_TOOLS_UNSUPPORTED_MESSAGE` refusal. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace “revertable” with “revertible” or “reversible”.
The rollback section repeats the misspelling at Lines 285, 291, and 295.
🧰 Tools
🪛 LanguageTool
[grammar] ~285-~285: Ensure spelling is correct
Context: ...ches no other combination. Each step is revertable on its own: - Step 1 is revertable...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~291-~291: Ensure spelling is correct
Context: ...p 3** is the load-bearing change and is revertable: remove the paused field, restore `i...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
Source: Linters/SAST tools
|
|
||
| ## 9. Open questions for review | ||
|
|
||
| 1. **The paused answer's gating flag.** The plan gates the paused-answer write on `!plan.isPi`. |
There was a problem hiding this comment.
I think we need to solve this today, no?
| identical arguments, the lookup could attach a widget to the wrong bubble. The seam already | ||
| consumes stored outputs first-in-first-out per key; confirm the live test exercises the | ||
| identical-args case, and decide whether the shim should pass a correlating hint if it does not. | ||
| 3. **The benign wait text.** The shim returns a short "waiting for the user" text result to |
There was a problem hiding this comment.
I thought we really paused. I didn't think that we would send a message to Claude Code in this case. Why can't we use the native weighting behavior here im lost?
|
Design record from Mahmoud's review, binding for the implementation. The question he raised: why return a synthetic waiting-for-the-user message to Claude and end the turn, instead of the native pattern where the tool call simply stays open and the harness waits, as a warm session naturally could? The answer we settled on: the message-plus-replay pattern is the cold-path contract, inherited from the Pi relay model, and it must exist regardless because a cold session has no process to hold anything open. The native hold is real and already exists in this codebase for approvals on warm sessions (the ACP permission request stays pending inside the live turn). Client tools do not get it yet because the relay has no turn-boundary model (open issue S5.2), the frontend answers by posting a new request rather than into an open turn, and a blocking MCP call faces Claude Code's client timeout (escape hatch identified: MCP progress notifications reset that timeout). The decision: ship this plan as the cold path that works everywhere, and treat the warm-path native hold as an explicit planned extension, coupled to the S5.2 turn-boundary work and the pending-interaction-records direction. THE IMPLEMENTATION MUST BE SHAPED FOR THAT FUTURE: there are two paths, cold and warm, and the code must say so. Concretely: the relay's pause handling models a closed set of outcomes (answered, error, paused-cold today; paused-hold reserved for the warm extension), the writePausedAnswer capability flag stays the single switch point, and the shim's pause branch carries a comment pointing at the extension issue. No behavior for the warm hold gets built now, but no door gets closed either. An extension issue with the user-facing problem and this research will be filed when this ships. |
mmabrouk
changed the title
Live walkthrough: Claude on Daytona calls the client tool, the widget renders, the user answers, the run resumesOn the video: I could not capture a GIF/MP4 of the playground. A video needs a browser driving the live app, but the only browser connected to the extension is on a remote machine that cannot reach this dev box's Client tools on Claude in a remote (Daytona) sandbox — frame-by-frame walkthroughFALLBACK EVIDENCE (not a GIF): the only browser connected to the extension is on a remote Turn 1 — the agent calls the client tool and the widget PARKS
The user answers the widget in their browser with output Turn 2 — the session RESUMES with the browser output and finishes
Result
|
| // the non-Pi in-sandbox shim gets a benign paused answer so it ends its blocking | ||
| // `tools/call` at once rather than waiting out the per-tool timeout and emitting a late | ||
| // error frame. The browser result returns on the cold-replay resume turn. | ||
| if (!writePausedAnswer) return; |
There was a problem hiding this comment.
Reviewer note (pause/teardown ordering): onPause fires pause.pause() before this write, which tears down the ACP session (cold-replay model), not the sandbox. stop() drains this in-flight write, and the worst case is the shim being killed cleanly before it reads the answer — no late error frame either way. So the paused answer removes the old full-timeout error; it is a best-effort improvement, not a race removed "by construction". I deliberately did NOT add a shim-ack-before-cancel handshake (Codex suggested it): the shim is the thing being cancelled, so waiting on its ack would risk hanging teardown. Live QA confirmed no late error frame.
| * new disposition (the warm hold) cannot silently fall through to the cold behavior; it forces a | ||
| * decision here, which is what keeps the warm extension a local change rather than a rework. | ||
| */ | ||
| export function relayWritesPausedAnswer( |
There was a problem hiding this comment.
Reviewer note (the closed set): the warm hold is a lifecycle decision, so the closed set lives here at the client-tool boundary, not in the relay's response shape. relayWritesPausedAnswer is exhaustive — adding warm-hold behavior forces a decision here rather than silently falling through to the cold path. The relay only consumes the derived writePausedAnswer boolean.
| * dispatch. A pause reaching here is a contract violation, so fail loud instead of laundering the | ||
| * sentinel into a string. The cold-pause protocol is handled only by the in-sandbox shim. | ||
| */ | ||
| function assertNotPaused( |
There was a problem hiding this comment.
Reviewer note: this throw is defensive, not a live path. relayToolCall can now return the pause sentinel, but the Pi extension (writePausedAnswer=false) and the local loopback MCP (parks client tools in-process) never receive it. Failing loud beats laundering the sentinel into a string if that invariant ever breaks.
|
Reviewer note on identical-argument correlation (the pre-existing FIFO seam in |
|
@coderabbitai review |
✅ Action performedReview finished.
|
78f5b72 to
91260c9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d54477c1-129b-450a-831d-fba4257d9fa6
📒 Files selected for processing (21)
docs/design/agent-workflows/documentation/tools.mddocs/design/agent-workflows/interfaces/README.mddocs/design/agent-workflows/interfaces/cross-service/runner-to-mcp-server.mdservices/runner/src/engines/sandbox_agent/client-tools.tsservices/runner/src/engines/sandbox_agent/environment-setup.tsservices/runner/src/engines/sandbox_agent/environment.tsservices/runner/src/engines/sandbox_agent/mcp.tsservices/runner/src/engines/sandbox_agent/run-plan.tsservices/runner/src/engines/sandbox_agent/run-turn.tsservices/runner/src/engines/sandbox_agent/tool-mcp-assets.tsservices/runner/src/tools/dispatch.tsservices/runner/src/tools/relay-client.tsservices/runner/src/tools/relay-protocol.tsservices/runner/src/tools/relay.tsservices/runner/src/tools/tool-mcp-stdio.tsservices/runner/tests/unit/client-tools.test.tsservices/runner/tests/unit/relay-client.test.tsservices/runner/tests/unit/relay-loop.test.tsservices/runner/tests/unit/sandbox-agent-run-plan.test.tsservices/runner/tests/unit/session-mcp-layering.test.tsservices/runner/tests/unit/tool-mcp-stdio.test.ts
| it("non-Pi shim path (writePausedAnswer) writes { ok: true, paused: true } when a client tool parks", async () => { | ||
| const { host, files } = fakeHost(); | ||
| const relay = startToolRelay( | ||
| host, | ||
| DIR, | ||
| [clientSpec], | ||
| undefined, | ||
| undefined, | ||
| parkingRelay(), | ||
| undefined, | ||
| { writePausedAnswer: true }, | ||
| ); | ||
|
|
||
| putClientRequest(files, "call-1"); | ||
| await until(() => files.has(`${DIR}/call-1.res.json`), "the paused answer"); | ||
| const res = JSON.parse(files.get(`${DIR}/call-1.res.json`) ?? "{}"); | ||
| assert.deepEqual( | ||
| res, | ||
| { ok: true, paused: true }, | ||
| "the shim path gets a benign paused answer, not an error or an empty success", | ||
| ); | ||
|
|
||
| await relay.stop(); | ||
| }); | ||
|
|
||
| it("Pi path (no writePausedAnswer) writes NO answer file when a client tool parks", async () => { | ||
| const { host, files } = fakeHost(); | ||
| let parked = false; | ||
| const relay = startToolRelay( | ||
| host, | ||
| DIR, | ||
| [clientSpec], | ||
| undefined, | ||
| undefined, | ||
| parkingRelay(() => { | ||
| parked = true; | ||
| }), | ||
| undefined, | ||
| {}, // writePausedAnswer defaults OFF -> Pi's native pause, no answer file | ||
| ); | ||
|
|
||
| putClientRequest(files, "call-1"); | ||
| await until(() => parked, "the client tool parked"); | ||
| // Settle long enough that a (wrong) answer write would have landed. | ||
| await sleep(RELAY_POLL_MS + 50); | ||
| assert.ok( | ||
| !files.has(`${DIR}/call-1.res.json`), | ||
| "Pi parks through its own extension; the loop writes no answer file", | ||
| ); | ||
|
|
||
| await relay.stop(); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Stop the relay in finally blocks.
Any failed wait or assertion skips relay.stop(), leaving its polling loop active and potentially hanging the test suite. Wrap each test body after relay creation in try/finally.
Proposed structure
const relay = startToolRelay(/* ... */);
-
- putClientRequest(files, "call-1");
- // assertions...
- await relay.stop();
+ try {
+ putClientRequest(files, "call-1");
+ // assertions...
+ } finally {
+ await relay.stop();
+ }📝 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.
| it("non-Pi shim path (writePausedAnswer) writes { ok: true, paused: true } when a client tool parks", async () => { | |
| const { host, files } = fakeHost(); | |
| const relay = startToolRelay( | |
| host, | |
| DIR, | |
| [clientSpec], | |
| undefined, | |
| undefined, | |
| parkingRelay(), | |
| undefined, | |
| { writePausedAnswer: true }, | |
| ); | |
| putClientRequest(files, "call-1"); | |
| await until(() => files.has(`${DIR}/call-1.res.json`), "the paused answer"); | |
| const res = JSON.parse(files.get(`${DIR}/call-1.res.json`) ?? "{}"); | |
| assert.deepEqual( | |
| res, | |
| { ok: true, paused: true }, | |
| "the shim path gets a benign paused answer, not an error or an empty success", | |
| ); | |
| await relay.stop(); | |
| }); | |
| it("Pi path (no writePausedAnswer) writes NO answer file when a client tool parks", async () => { | |
| const { host, files } = fakeHost(); | |
| let parked = false; | |
| const relay = startToolRelay( | |
| host, | |
| DIR, | |
| [clientSpec], | |
| undefined, | |
| undefined, | |
| parkingRelay(() => { | |
| parked = true; | |
| }), | |
| undefined, | |
| {}, // writePausedAnswer defaults OFF -> Pi's native pause, no answer file | |
| ); | |
| putClientRequest(files, "call-1"); | |
| await until(() => parked, "the client tool parked"); | |
| // Settle long enough that a (wrong) answer write would have landed. | |
| await sleep(RELAY_POLL_MS + 50); | |
| assert.ok( | |
| !files.has(`${DIR}/call-1.res.json`), | |
| "Pi parks through its own extension; the loop writes no answer file", | |
| ); | |
| await relay.stop(); | |
| }); | |
| it("non-Pi shim path (writePausedAnswer) writes { ok: true, paused: true } when a client tool parks", async () => { | |
| const { host, files } = fakeHost(); | |
| const relay = startToolRelay( | |
| host, | |
| DIR, | |
| [clientSpec], | |
| undefined, | |
| undefined, | |
| parkingRelay(), | |
| undefined, | |
| { writePausedAnswer: true }, | |
| ); | |
| try { | |
| putClientRequest(files, "call-1"); | |
| await until(() => files.has(`${DIR}/call-1.res.json`), "the paused answer"); | |
| const res = JSON.parse(files.get(`${DIR}/call-1.res.json`) ?? "{}"); | |
| assert.deepEqual( | |
| res, | |
| { ok: true, paused: true }, | |
| "the shim path gets a benign paused answer, not an error or an empty success", | |
| ); | |
| } finally { | |
| await relay.stop(); | |
| } | |
| }); | |
| it("Pi path (no writePausedAnswer) writes NO answer file when a client tool parks", async () => { | |
| const { host, files } = fakeHost(); | |
| let parked = false; | |
| const relay = startToolRelay( | |
| host, | |
| DIR, | |
| [clientSpec], | |
| undefined, | |
| undefined, | |
| parkingRelay(() => { | |
| parked = true; | |
| }), | |
| undefined, | |
| {}, // writePausedAnswer defaults OFF -> Pi's native pause, no answer file | |
| ); | |
| try { | |
| putClientRequest(files, "call-1"); | |
| await until(() => parked, "the client tool parked"); | |
| // Settle long enough that a (wrong) answer write would have landed. | |
| await sleep(RELAY_POLL_MS + 50); | |
| assert.ok( | |
| !files.has(`${DIR}/call-1.res.json`), | |
| "Pi parks through its own extension; the loop writes no answer file", | |
| ); | |
| } finally { | |
| await relay.stop(); | |
| } | |
| }); |
mmabrouk
left a comment
mmabrouk
left a comment
There was a problem hiding this comment.
My commit-level review of the feature commit (91260c9) after the plan-level review and Codex's pass. Verdict: this is the design we agreed, correctly built, and it moves to Mahmoud's queue.
What I verified line by line:
- The closed set is real, not decorative. ClientToolPauseDisposition lives at the client-tool boundary with an exhaustive switch (a compile-time never-check), so the reserved warm-hold variant cannot silently fall through to cold behavior; when #5384 gets built, the compiler forces the decision. This is exactly the extendability condition from Mahmoud's review, honored in the type system rather than in a comment.
- The pause sentinel is a Symbol, not a string. A legitimate tool result can be any string, including empty, so a string sentinel would have been a collision waiting to happen. The Symbol makes the pause structurally unmistakable, and the shim checks it before the ok/text path so a paused answer can never read as an empty success.
- Pi's behavior is preserved by default. writePausedAnswer defaults off; only the non-Pi shim path flips it. The relay's Pi contract (write no answer, park through the extension) is untouched, which the unchanged Pi gate journeys confirm.
- Cleanup is deterministic with the sweep as backstop, per the plan decision: the shim removes both relay files for every answer variant including the pause, and the turn-start sweep only covers a torn-down shim.
- Test surface matches the risk: 488 added/151 removed lines across five new or extended unit suites (relay loop pause, relay client sentinel, shim mapping, run-plan gate removal, correlation), plus the live evidence and the new gate journey.
Recorded disagreement with Codex finding #1 (ack-handshake on teardown) reviewed and endorsed: the handshake would couple teardown to a process being killed; the live evidence shows the worst case is a clean shim shutdown with no late error frame.
One expectation for the merge sequence: this PR and #5381 (the denied frame) were verified together on one tree; merge them in the same release window so the gate's client and deny journeys stay green together.
| * warm session, instead of writing an answer and tearing the session down. | ||
| * NOT built; nothing produces it yet. See the warm-hold extension #5384. | ||
| */ | ||
| export type ClientToolPauseDisposition = |
There was a problem hiding this comment.
Honestly, quite weird structuring of this. I can't say why, but it feels very weird to have pi-native, cold-acknowledge, and warm-hold as pose this position. Looks to me the right way is to have maybe port and then a version for pi version for others. I don't know, but this is weird. Maybe it's very hard to kind of improve, and that's all right, but I just say I wouldn't call this good code.
| throw new Error(PI_PERMISSION_EXTENSION_UNAVAILABLE_MESSAGE); | ||
| } | ||
| if (!plan.isPi && plan.executableToolSpecs.length > 0) { | ||
| if (!plan.isPi && plan.toolSpecs.length > 0) { |
There was a problem hiding this comment.
I mean, is pi and is at each place not very good programming? I know it's maybe outside of scope here, but just maybe add it to your memory. I don't think this is the right way to have the logic. I think the problem is there is some sort of port adapter for dealing with the logic, where you don't have to have if-then-else for the harness type within the code. It does not look like the environment TS should know about that. It looks like the responsibility is all over the place.
Advertise browser-fulfilled client tools to the in-sandbox stdio MCP shim and add a paused relay answer so a parked client tool ends the shim's tools/call cleanly while the runner ends the turn. The browser result returns on the cold-replay resume. Deletes the interim #5366 client-only Daytona refusal. Closes #5256; closes the residual mixed-set drop of #4984. The client-tool pause disposition is a closed set (pi-native / cold-acknowledge, with warm-hold reserved) at the client-tool boundary; the relay consumes the derived writePausedAnswer switch. Claude-Session: https://claude.ai/code/session_01DnWRxU3dCJ11hgDidm26vW
91260c9 to
b2e8419
Compare
dosubot
Bot
added
size:XL
Railway Preview Environment
Updated at 2026-07-18T23:40:20.977Z |
1 participant
Context
On the Claude harness in a Daytona remote sandbox, a browser-fulfilled client tool (like
request_connection) did not work. Two wrong outcomes were possible:The same tool already worked on Pi (any sandbox) and on Claude in the local sandbox. Three mechanisms blocked the Claude-plus-Daytona path: the in-sandbox stdio MCP shim was only ever advertised the executable subset of tools, its
tools/callhad no "pause" outcome, and when a client tool parked the relay wrote no answer, so the shim waited out its full per-tool timeout and then emitted a late error frame.This reuses the delivery path Pi already uses on Daytona (the existing file relay). It adds no new network route between the runner and the sandbox.
Changes
Advertise client tools to the shim. The upload gate and the advertisement gate no longer filter client specs out.
environment.ts: uploadadvertisedToolSpecs(plan.toolSpecs)(wasplan.executableToolSpecs), gated onplan.toolSpecs.length > 0.mcp.ts: build the shim entry whentoolSpecs.length > 0(wasexecutableToolSpecs(toolSpecs).length > 0), and count every advertised tool in the honest log.Add one new "paused" relay answer. When a client tool parks, the runner writes a benign paused answer so the shim ends its
tools/callcleanly while the runner ends the turn. The browser result returns on the cold-replay resume.relay-protocol.ts:ExecuteRelayResponsegains an optionalpaused?: true. An answer is now one of three: success (ok: true, text), failure (ok: false, error), or pause (ok: true, paused: true).relay.ts: onPAUSED, the loop writes{ ok: true, paused: true }whenwritePausedAnsweris set (non-Pi shim path); Pi still writes nothing.relay-client.ts:relayToolCallrecognizes the paused answer and returns aRELAY_PAUSEDsentinel (not a throw, not an empty string), and still unlinks the req/res pair.tool-mcp-stdio.ts: the shim maps the sentinel to a benign, NON-error tool result that names the tool and tells the model not to retry.Before, the shim's
tools/callhad two outcomes:After, a pause is a third, distinct, non-error outcome:
Delete the interim #5366 refusal.
run-plan.tsno longer refuses a client-only Daytona run (DAYTONA_CLIENT_ONLY_TOOLS_UNSUPPORTED_MESSAGEis gone). TheREMOTE_TOOLS_UNSUPPORTED_MESSAGErefusal stays for a non-Daytona remote provider, which is still unproven and must fail closed.The cold/warm design record
The pause message plus cold-replay resume is the cold-path contract, and it must exist regardless because a cold session has no live process to hold anything open. A warm-path native hold (keep the shim's call open inside the live turn, the way an ACP approval already holds on a warm session) is a real future extension, coupled to the relay's missing turn-boundary model (S5.2) and pending-interaction records.
The code is shaped for that future without building it. The client-tool pause disposition is a closed set at the client-tool boundary (
ClientToolPauseDisposition = "pi-native" | "cold-acknowledge" | "warm-hold"inclient-tools.ts), withwarm-holdreserved and the mapping to the relay'swritePausedAnswerswitch made exhaustive, so adding the warm hold is a local change rather than a rework. The follow-up issue captures the user-facing problem and the research.Tests
services/runner:pnpm run typecheckclean,pnpm testgreen at 1214 (1209 baseline + 5 new for the paused path and the disposition mapping)./services/agent/v0/invokeendpoint and asserting on the SSE wire:data-renderframe,finish=other); the answer resumes and the run completes with no re-park and no error. This is the release gate for the feature.request_connection('slack')calls across turns each parked their own widget and resumed with their own distinct token, no misattachment (the per-key FIFO correlation holds). The concurrent same-turn case could not be provoked (the model deduplicates identical connection requests), so no id plumbing was added, per the plan's decision.Scope / risk
/runwire-contract change: the relay files are internal to the runner, so no golden fixture insdks/pythonmoves.clientjourney lives in the local-onlyagent-release-gateskill, so it is not in this diff.What to QA
request_connectionclient tool, and prompt it to connect an integration. The connect widget should appear in the chat, the run should pause, and answering it should resume the agent to completion. Before this change the run either refused up front or produced no tool at all.Plans #5383. Closes #5256. Closes #4984.