fix(tool-projection): expose host_file_* on cross-client transports when capable client connected#29613
Merged
credence-the-bot[bot] merged 6 commits intoMay 5, 2026
Conversation
Generalizes the cross-client tool-exposure carve-out in isToolActiveForContext from a hardcoded host_bash check to a Set<HostProxyCapability> containing host_bash and host_file. Phase 2 (PR #29398) and Phase 3 (PR #29440) shipped the routing infrastructure for cross-client host_file_* execution but never extended this exposure gate, so web/iphone turns silently lacked all four host_file_* tools even when a macOS client was connected. Also fixes a latent miscount: the hub query now uses the actual capability under inspection instead of the hardcoded "host_bash" string. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Generalizes the per-capability client-count mock from a single
host_bash counter to a Map keyed by capability, then adds 16 new
tests (4 host_file_* tools × {exposed-with-client, blocked-no-client,
blocked-on-chrome-extension, blocked-when-hasNoClient}) plus a
regression guard verifying listClientsByCapability is queried with
the actual capability under inspection (D5 latent-bug fix).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Updates the test file's top-of-file docstring to describe the generalized CROSS_CLIENT_EXPOSED_CAPABILITIES carve-out (host_bash + host_file) instead of the Phase-1-only host_bash framing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…t transports Adds the 'no-client' and 'stale-targetClientId' runtime guards to host_file_read / write / edit (and the missing no-client guard to host_file_transfer), mirroring the existing host_bash guards in host-shell.ts. Without these guards, exposing host_file_* on non-host-proxy transports (web, iphone) via PR #29613's cross-client carve-out could silently fall through to the daemon container's filesystem if the macOS client disconnects between tool projection and tool execution — reading or modifying files inside the cloud daemon instead of the user's host machine. Addresses Codex P1 on PR #29613. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
Contributor
Author
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Contributor
Author
|
@chatgpt-codex-connector review Testing the executor-guard fix in commit 4ec0e1a. Previous |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…non-host-proxy turns Codex P2 on PR #29613: the disconnected-target guard added in commit 4ec0e1a rejects any host_file_{read,write,edit} call with target_client_id when HostFileProxy is unavailable, even on macos turns where local-fs fallback IS the intended offline behavior. If a stale or auto-filled target_client_id leaks in from a prior cross-client turn, a macos turn now errors instead of writing locally — a regression vs the pre-PR behavior. Scope the guard to !supportsHostProxy(transport) so it only fires on web/iphone, where local fs would be the daemon container's filesystem and falling through is genuinely unsafe. macos turns silently ignore a stale target_client_id and fall through to FileSystemOps as before. Adds one regression test per executor (read/write/edit) covering macos + stale target_client_id + unavailable proxy → falls through to local fs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
|
@chatgpt-codex-connector review P2 from review-2 addressed in commit da11f13 — disconnected-target guard scoped to non-host-proxy transports only. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ard to non-host-proxy turns Devin review on PR #29613: transfer.ts had the same scope drift the prior commit fixed in read/write/edit. The pre-existing post-proxy guard `if (targetClientId != null) { ... no host client is available }` fired on macOS too, so a stale/auto-filled target_client_id from a prior cross-client turn caused host_file_transfer to error on macOS even though host_file_{read,write,edit} silently fell through to local fs (the desired offline-mode behavior). Replaces the unscoped guard with a scoped guard #3 placed at the top of execute() (matching the read/write/edit pattern), keyed on !supportsHostProxy(transport). The non-host-proxy + stale-target case now produces the same "target client ... is no longer connected" message as the other tools; on macOS the stale target_client_id is silently ignored and the call falls through to executeLocal as before PR #29613. Adds two tests in transfer.test.ts: web + stale target → error, and macos + stale target → falls through to local copy. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
Author
|
@chatgpt-codex-connector review Cycle 3 fix in commit 54a2e39 — transfer.ts now scopes the disconnected-target guard to non-host-proxy transports and removes the pre-existing unscoped post-proxy reject. macOS with stale target_client_id now falls through to executeLocal, matching read/write/edit. Both prior threads (Devin transfer.ts BUG + Codex P2) addressed by the same commit. |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
credence-the-bot
Bot
merged commit a4d75aa
into
credence-the-bot/host-file-cross-client-exposure-plan
13 checks passed
credence-the-bot Bot
added a commit
that referenced
this pull request
May 5, 2026
…29621) Phase 4 round 1: addresses minor integration-review gaps from the self-review pass on PR #29613. - Replace stale "iphone" references with the actual InterfaceId "ios" in 6 comment locations (conversation-tool-setup.ts, the 4 host_file_* executors). - Add an explanatory note in each host_file_* executor's disconnected-target guard pointing out the deliberate divergence from host_bash (host-shell.ts:239-247) and linking to the PR #29613 review for rationale, so future readers understand the asymmetry is intentional. - Align transfer.ts's "no client connected" guard message with the read/write/edit pattern by referring to "host_file" (the capability) rather than "host_file_transfer" (the tool name). - Add a stub mock for assistant-event-hub.js in transfer.test.ts so the multi-client guard test runs against an isolated stub rather than the live process-wide singleton, matching the read/write/edit test pattern. Co-authored-by: credence-the-bot[bot] <credence-the-bot[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
credence-the-bot Bot
added a commit
that referenced
this pull request
May 5, 2026
…t with inline rationale (#29628) Phase 4 round 2: Devin BUG findings on PR #29621 flagged the "see PR #29613 review discussion for rationale" tail of the new divergence comments as a violation of assistant/AGENTS.md "Code comments" rule (don't narrate history; describe the current state). Replaces the PR cross-reference in each of the 4 host_file_* executors with a short inline statement of the asymmetry — that host_bash rejects unconditionally for any stale target_client_id regardless of transport — keeping the divergence call-out without relying on PR-discussion archaeology. Co-authored-by: credence-the-bot[bot] <credence-the-bot[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
credence-the-bot Bot
added a commit
that referenced
this pull request
May 5, 2026
…onnected-target guard Codex P1 on the feature-branch PR (#29632): the disconnected-target guard added in PR #29613 / cycle 2 only fires when \`context.transportInterface != null\`. On the documented legacy/ backwards-compat path where transport metadata is missing, a call with target_client_id and no connected host client skips this error path and falls through to local FileSystemOps / executeLocal — silently targeting the daemon container's filesystem instead of the intended host client. This was a real regression introduced by cycle 3's removal of transfer.ts's pre-existing unscoped guard. Restructure the condition so the guard fires for both non-host-proxy transports AND undefined transport, skipping only when transport is explicitly host-proxy-capable (macos, where local-fs fallback IS the intended offline behavior). Keeps the cycle-2 behavior for macos fall-through while restoring the safety guarantee for legacy callers. Adds one regression test per executor (read/write/edit/transfer) for the undefined-transport + target_client_id + no-proxy → reject path. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
noanflaherty
pushed a commit
that referenced
this pull request
May 5, 2026
…hen capable client connected (#29632) * fix(tool-projection): expose host_file_* on cross-client transports when capable client connected (#29613) * feat(tool-projection): add CROSS_CLIENT_EXPOSED_CAPABILITIES set Generalizes the cross-client tool-exposure carve-out in isToolActiveForContext from a hardcoded host_bash check to a Set<HostProxyCapability> containing host_bash and host_file. Phase 2 (PR #29398) and Phase 3 (PR #29440) shipped the routing infrastructure for cross-client host_file_* execution but never extended this exposure gate, so web/iphone turns silently lacked all four host_file_* tools even when a macOS client was connected. Also fixes a latent miscount: the hub query now uses the actual capability under inspection instead of the hardcoded "host_bash" string. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(tool-projection): cover host_file_* cross-client exposure paths Generalizes the per-capability client-count mock from a single host_bash counter to a Map keyed by capability, then adds 16 new tests (4 host_file_* tools × {exposed-with-client, blocked-no-client, blocked-on-chrome-extension, blocked-when-hasNoClient}) plus a regression guard verifying listClientsByCapability is queried with the actual capability under inspection (D5 latent-bug fix). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(tool-projection): update file docstring for cross-client carve-out Updates the test file's top-of-file docstring to describe the generalized CROSS_CLIENT_EXPOSED_CAPABILITIES carve-out (host_bash + host_file) instead of the Phase-1-only host_bash framing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(tool-projection): add host_file_* executor guards for cross-client transports Adds the 'no-client' and 'stale-targetClientId' runtime guards to host_file_read / write / edit (and the missing no-client guard to host_file_transfer), mirroring the existing host_bash guards in host-shell.ts. Without these guards, exposing host_file_* on non-host-proxy transports (web, iphone) via PR #29613's cross-client carve-out could silently fall through to the daemon container's filesystem if the macOS client disconnects between tool projection and tool execution — reading or modifying files inside the cloud daemon instead of the user's host machine. Addresses Codex P1 on PR #29613. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(tool-projection): scope host_file_* disconnected-target guard to non-host-proxy turns Codex P2 on PR #29613: the disconnected-target guard added in commit 4ec0e1a rejects any host_file_{read,write,edit} call with target_client_id when HostFileProxy is unavailable, even on macos turns where local-fs fallback IS the intended offline behavior. If a stale or auto-filled target_client_id leaks in from a prior cross-client turn, a macos turn now errors instead of writing locally — a regression vs the pre-PR behavior. Scope the guard to !supportsHostProxy(transport) so it only fires on web/iphone, where local fs would be the daemon container's filesystem and falling through is genuinely unsafe. macos turns silently ignore a stale target_client_id and fall through to FileSystemOps as before. Adds one regression test per executor (read/write/edit) covering macos + stale target_client_id + unavailable proxy → falls through to local fs. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(tool-projection): scope host_file_transfer disconnected-target guard to non-host-proxy turns Devin review on PR #29613: transfer.ts had the same scope drift the prior commit fixed in read/write/edit. The pre-existing post-proxy guard `if (targetClientId != null) { ... no host client is available }` fired on macOS too, so a stale/auto-filled target_client_id from a prior cross-client turn caused host_file_transfer to error on macOS even though host_file_{read,write,edit} silently fell through to local fs (the desired offline-mode behavior). Replaces the unscoped guard with a scoped guard #3 placed at the top of execute() (matching the read/write/edit pattern), keyed on !supportsHostProxy(transport). The non-host-proxy + stale-target case now produces the same "target client ... is no longer connected" message as the other tools; on macOS the stale target_client_id is silently ignored and the call falls through to executeLocal as before PR #29613. Adds two tests in transfer.test.ts: web + stale target → error, and macos + stale target → falls through to local copy. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: credence-the-bot[bot] <credence-the-bot[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> * fix(tool-projection): self-review cleanup for host_file_* exposure (#29621) Phase 4 round 1: addresses minor integration-review gaps from the self-review pass on PR #29613. - Replace stale "iphone" references with the actual InterfaceId "ios" in 6 comment locations (conversation-tool-setup.ts, the 4 host_file_* executors). - Add an explanatory note in each host_file_* executor's disconnected-target guard pointing out the deliberate divergence from host_bash (host-shell.ts:239-247) and linking to the PR #29613 review for rationale, so future readers understand the asymmetry is intentional. - Align transfer.ts's "no client connected" guard message with the read/write/edit pattern by referring to "host_file" (the capability) rather than "host_file_transfer" (the tool name). - Add a stub mock for assistant-event-hub.js in transfer.test.ts so the multi-client guard test runs against an isolated stub rather than the live process-wide singleton, matching the read/write/edit test pattern. Co-authored-by: credence-the-bot[bot] <credence-the-bot[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> * fix(tool-projection): replace PR cross-reference in divergence comment with inline rationale (#29628) Phase 4 round 2: Devin BUG findings on PR #29621 flagged the "see PR #29613 review discussion for rationale" tail of the new divergence comments as a violation of assistant/AGENTS.md "Code comments" rule (don't narrate history; describe the current state). Replaces the PR cross-reference in each of the 4 host_file_* executors with a short inline statement of the asymmetry — that host_bash rejects unconditionally for any stale target_client_id regardless of transport — keeping the divergence call-out without relying on PR-discussion archaeology. Co-authored-by: credence-the-bot[bot] <credence-the-bot[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> * fix(tool-projection): close backwards-compat hole in host_file_* disconnected-target guard Codex P1 on the feature-branch PR (#29632): the disconnected-target guard added in PR #29613 / cycle 2 only fires when \`context.transportInterface != null\`. On the documented legacy/ backwards-compat path where transport metadata is missing, a call with target_client_id and no connected host client skips this error path and falls through to local FileSystemOps / executeLocal — silently targeting the daemon container's filesystem instead of the intended host client. This was a real regression introduced by cycle 3's removal of transfer.ts's pre-existing unscoped guard. Restructure the condition so the guard fires for both non-host-proxy transports AND undefined transport, skipping only when transport is explicitly host-proxy-capable (macos, where local-fs fallback IS the intended offline behavior). Keeps the cycle-2 behavior for macos fall-through while restoring the safety guarantee for legacy callers. Adds one regression test per executor (read/write/edit/transfer) for the undefined-transport + target_client_id + no-proxy → reject path. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: credence-the-bot[bot] <277301654+credence-the-bot[bot]@users.noreply.github.com> Co-authored-by: credence-the-bot[bot] <credence-the-bot[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
isToolActiveForContextfrom a hardcodedhost_bashcheck to aSet<HostProxyCapability>containinghost_bashandhost_file.host_file_*but never extended this exposure gate, so web/iphone turns silently lacked all fourhost_file_*tools even with a macOS client connected.Design Doc
.private/plans/host-file-cross-client-exposure-design.mdBehavior Change
host_file_read/write/edit/transfernow appear in the LLM's tool list and route to the macOS client via existing Phase 2/3 proxies.Tests
host_file_*cross-client exposure tests (one per tool)host_file_*chrome-extension exclusion testshost_file_*hasNoClient exclusion testshost_file_*no-client exclusion testsRisk
Low — additive change. Re-uses the same chrome-extension and hasNoClient gates as the existing host_bash carve-out. No new security surface.
Part of plan: host-file-cross-client-exposure-plan.md (PR 1 of 1)