-
Notifications
You must be signed in to change notification settings - Fork 85
feat(tools): add target_client_id to host_file_* and computer_use_* tools #29397
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1780,6 +1780,10 @@ export async function surfaceProxyResolver( | |
| // Record the action and proxy to the connected desktop client | ||
| const reasoning = | ||
| typeof input.reasoning === "string" ? input.reasoning : undefined; | ||
| const targetClientId = | ||
| typeof input.target_client_id === "string" && input.target_client_id !== "" | ||
| ? input.target_client_id | ||
| : undefined; | ||
| ctx.hostCuProxy.recordAction(toolName, input, reasoning); | ||
| return ctx.hostCuProxy.request( | ||
| toolName, | ||
|
|
@@ -1788,6 +1792,7 @@ export async function surfaceProxyResolver( | |
| ctx.hostCuProxy.stepCount, | ||
| reasoning, | ||
| signal, | ||
| targetClientId, | ||
|
Comment on lines
1792
to
+1795
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Passing Useful? React with 👍 / 👎. |
||
| ); | ||
| } | ||
|
|
||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Missing The PR adds (Refers to lines 496-499) Was this helpful? React with 👍 or 👎 to provide feedback. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,10 @@ | ||
| import { extname } from "node:path"; | ||
|
|
||
| import { supportsHostProxy } from "../../channels/types.js"; | ||
| import { HostFileProxy } from "../../daemon/host-file-proxy.js"; | ||
| import { RiskLevel } from "../../permissions/types.js"; | ||
| import type { ToolDefinition } from "../../providers/types.js"; | ||
| import { assistantEventHub } from "../../runtime/assistant-event-hub.js"; | ||
| import { FileSystemOps } from "../shared/filesystem/file-ops-service.js"; | ||
| import { | ||
| IMAGE_EXTENSIONS, | ||
|
|
@@ -37,6 +39,11 @@ class HostFileReadTool implements Tool { | |
| type: "number", | ||
| description: "Maximum number of lines to read", | ||
| }, | ||
| target_client_id: { | ||
| type: "string", | ||
| description: | ||
| "ID of the specific client to execute this on. Required when multiple clients support host_file; omit when only one is connected. Obtain IDs from `assistant clients list --capability host_file`.", | ||
| }, | ||
| }, | ||
| required: ["path"], | ||
| }, | ||
|
|
@@ -55,6 +62,24 @@ class HostFileReadTool implements Tool { | |
| }; | ||
| } | ||
|
|
||
| const targetClientId = | ||
| typeof input.target_client_id === "string" && input.target_client_id !== "" | ||
| ? input.target_client_id | ||
| : undefined; | ||
|
|
||
| const transportInterface = context.transportInterface; | ||
| if ( | ||
| targetClientId == null && | ||
| transportInterface != null && | ||
| !supportsHostProxy(transportInterface) && | ||
| assistantEventHub.listClientsByCapability("host_file").length > 1 | ||
| ) { | ||
|
Comment on lines
+72
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The new Useful? React with 👍 / 👎. |
||
| return { | ||
| content: `Error: multiple clients support host_file. Specify which client to use with \`target_client_id\`. Run \`assistant clients list --capability host_file\` to see client IDs and labels.`, | ||
| isError: true, | ||
| }; | ||
| } | ||
|
|
||
| // Proxy to connected client for execution on the user's machine | ||
| // when a capable client is available (managed/cloud-hosted mode), | ||
| // including image reads that need the host filesystem view. | ||
|
|
@@ -65,6 +90,7 @@ class HostFileReadTool implements Tool { | |
| path: rawPath, | ||
| offset: typeof input.offset === "number" ? input.offset : undefined, | ||
| limit: typeof input.limit === "number" ? input.limit : undefined, | ||
| targetClientId, | ||
| }, | ||
| context.conversationId, | ||
| context.signal, | ||
|
|
||
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.
🚩 Pre-existing: buildUserFacingLabel reads cancelLabel for deny action
At
conversation-surfaces.ts:1724, thedenybranch readssurfaceData?.cancelLabelinstead ofsurfaceData?.denyLabel. This means the deny action's user-facing label falls back to the cancel button's label rather than a dedicated deny label. This is a pre-existing issue not introduced by this PR, but it's in adjacent unchanged code within the same function.(Refers to lines 1724-1726)
Was this helpful? React with 👍 or 👎 to provide feedback.