Skip to content

feat(macos): decode targetClientId from HostBashRequest and accept targeted non-local requests #29315

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.

Sign up for GitHub

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

Merged
credence-the-bot merged 1 commit into from
May 3, 2026
Merged

feat(macos): decode targetClientId from HostBashRequest and accept targeted non-local requests #29315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions clients/macos/vellum-assistant/App/AppDelegate+ConnectionSetup.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,15 @@ extension AppDelegate {
self.featureFlagStore.reloadFromGateway()
// Host tool execution — run locally and post results back
case .hostBashRequest(let msg):
// Accept if this conversation is locally owned, OR if the request
// is explicitly targeted at this client (cross-client proxy routing).
let localClientId = DeviceIdStore.getOrCreate()
let isLocalConversation = self.mainWindow?.conversationManager
.conversations.contains(where: { $0.conversationId == msg.conversationId }) ?? false
let isTargeted = msg.targetClientId == localClientId
guard isLocalConversation || isTargeted else {
break
Comment on lines +374 to +375

@chatgpt-codex-connector chatgpt-codex-connector Bot May 3, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Route targeted host_bash requests through SSE filter

This acceptance guard cannot make cross-client host bash work by itself, because EventStreamClient.shouldIgnoreHostToolRequest still drops every host_bash_request whose conversationId is not locally owned before this switch ever runs (clients/shared/Network/EventStreamClient.swift, case .hostBashRequest). In the targeted-proxy path, the conversation is intentionally non-local, so the message is filtered out and isTargeted is never evaluated, leaving targeted host bash requests non-functional.

Useful? React with 👍 / 👎.

}
Comment on lines +368 to +376

@devin-ai-integration devin-ai-integration Bot May 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Upstream SSE filter drops cross-client proxy bash requests before targetClientId guard can evaluate

The new targetClientId guard in AppDelegate+ConnectionSetup.swift:374 is unreachable for its intended cross-client proxy use case. EventStreamClient.shouldIgnoreHostToolRequest (clients/shared/Network/EventStreamClient.swift:607-632) runs first and drops any hostBashRequest whose conversationId is not in locallyOwnedConversationIds (line 610-612). For cross-client proxy routing, the conversation is owned by a different client, so locallyOwnedConversationIds.contains(msg.conversationId) is false, and the message is silently discarded with a warning log before handleParsedMessage ever broadcasts it to subscribers. The new guard at line 374 that checks msg.targetClientId == localClientId never gets a chance to execute, completely breaking the cross-client proxy feature.

Upstream filter that blocks the message

EventStreamClient.swift:609-612:

case .hostBashRequest(let msg):
    if locallyOwnedConversationIds.contains(msg.conversationId) { return false }
    log.warning("Ignoring host_bash_request for non-local conversation ...")
    return true

This needs to also pass through messages where msg.targetClientId matches the local client ID.

Prompt for agents 
The cross-client proxy feature for hostBashRequest is broken because EventStreamClient.shouldIgnoreHostToolRequest (clients/shared/Network/EventStreamClient.swift, lines 607-632) drops all host_bash_request messages whose conversationId is not in locallyOwnedConversationIds — before the message reaches the new targetClientId guard in AppDelegate+ConnectionSetup.swift.

For cross-client proxy routing, the conversation is owned by a different client, so the conversation ID won't be in locallyOwnedConversationIds. The fix needs to be in shouldIgnoreHostToolRequest: the hostBashRequest case (line 609-612) should also allow through messages where msg.targetClientId matches the local client ID (DeviceIdStore.getOrCreate()). Something like:

case .hostBashRequest(let msg):
    if locallyOwnedConversationIds.contains(msg.conversationId) { return false }
    if msg.targetClientId == DeviceIdStore.getOrCreate() { return false }
    log.warning(...)
    return true

Note: The guard in AppDelegate+ConnectionSetup.swift (lines 368-376) may then be redundant with this upstream fix, or can serve as defense-in-depth. Consider whether the duplicate check in AppDelegate is still needed.
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

HostToolExecutor.executeHostBashRequest(msg)
case .hostFileRequest(let msg):
HostToolExecutor.executeHostFileRequest(msg)
Comment on lines 378 to 379

@devin-ai-integration devin-ai-integration Bot May 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Inconsistent cross-client routing guard across host tool types

The new targetClientId-based routing guard is only added for .hostBashRequest but not for .hostFileRequest, .hostCuRequest, .hostBrowserRequest, or .hostTransferRequest (AppDelegate+ConnectionSetup.swift:378-412). This may be intentional if cross-client proxy routing is only planned for bash requests, but it's worth confirming the intent — if the feature is expected to extend to other host tool types, the same pattern (and the same upstream shouldIgnoreHostToolRequest fix) would be needed for those cases as well.

(Refers to lines 378-412)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Expand Down
4 changes: 4 additions & 0 deletions clients/shared/Network/MessageTypes.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1538,6 +1538,9 @@ public struct HostBashRequest: Decodable, Sendable {
public let timeoutSeconds: Double?
/// Extra environment variables to inject into the subprocess (e.g. VELLUM_UNTRUSTED_SHELL).
public let env: [String: String]?
/// When set, this request is targeted at a specific client ID. Non-nil only for
/// cross-client proxy requests routed through HostBashProxy.
public let targetClientId: String?

private enum CodingKeys: String, CodingKey {
case type
Expand All @@ -1547,6 +1550,7 @@ public struct HostBashRequest: Decodable, Sendable {
case workingDir = "working_dir"
case timeoutSeconds = "timeout_seconds"
case env
case targetClientId
}
}

Expand Down