Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,24 @@ extension AppDelegate {
}
HostToolExecutor.executeHostBashRequest(msg)
case .hostFileRequest(let msg):
let localClientId = DeviceIdStore.getOrCreate()
let isLocalConversation = self.mainWindow?.conversationManager
.conversations.contains(where: { $0.conversationId == msg.conversationId }) ?? false
let isTargeted = msg.targetClientId == localClientId
let isUntargetedLocal = msg.targetClientId == nil && isLocalConversation
guard isTargeted || isUntargetedLocal else {
Comment on lines +384 to +388
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 Do not gate untargeted host_file execution on UI conversation list

isUntargetedLocal now depends on mainWindow?.conversationManager.conversations, which can be empty/nil before the main window is created or before conversations hydrate. In that state, untargeted host_file_request events that already passed EventStreamClient's locallyOwnedConversationIds check are incorrectly dropped, so legitimate local host-file operations never run. This is a regression from the previous behavior (execute after transport-level ownership filtering) and the same pattern is duplicated in the host_cu_request branch.

Useful? React with 👍 / 👎.

break
}
HostToolExecutor.executeHostFileRequest(msg)
case .hostCuRequest(let msg):
let localClientId = DeviceIdStore.getOrCreate()
let isLocalConversation = self.mainWindow?.conversationManager
.conversations.contains(where: { $0.conversationId == msg.conversationId }) ?? false
let isTargeted = msg.targetClientId == localClientId
let isUntargetedLocal = msg.targetClientId == nil && isLocalConversation
guard isTargeted || isUntargetedLocal else {
break
}
let proxy = self.getOrCreateHostCuOverlay(conversationId: msg.conversationId, request: msg)
let task = Task { @MainActor in
defer { self.inFlightCuTasks.removeValue(forKey: msg.requestId) }
Expand Down
4 changes: 4 additions & 0 deletions clients/shared/Network/EventStreamClient.swift
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.

🚩 hostAppControlRequest and hostBrowserRequest lack targetClientId support — intentional scope boundary

This PR adds targetClientId to HostFileRequest and HostCuRequest but does not add it to HostAppControlRequest (MessageTypes.swift:1687-1719) or HostBrowserRequest. Their shouldIgnoreHostToolRequest cases (EventStreamClient.swift:628-635) still lack the targetClientId != nil bypass, and postAppControlResult (HostProxyClient.swift:96-99) still omits the X-Vellum-Client-Id header. This appears intentional (app-control and browser don't have cross-client proxy routing yet), but worth confirming these request types don't need cross-client targeting in this iteration.

(Refers to lines 628-635)

Open in Devin Review

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

Original file line number Diff line number Diff line change
Expand Up @@ -614,10 +614,14 @@ public final class EventStreamClient {
log.warning("Ignoring host_bash_request for non-local conversation \(msg.conversationId, privacy: .public)")
return true
case .hostFileRequest(let msg):
// Targeted cross-client requests carry a non-local conversationId by design.
// Pass them through so AppDelegate+ConnectionSetup can perform the targetClientId check.
if msg.targetClientId != nil { return false }
if locallyOwnedConversationIds.contains(msg.conversationId) { return false }
log.warning("Ignoring host_file_request for non-local conversation \(msg.conversationId, privacy: .public)")
return true
case .hostCuRequest(let msg):
if msg.targetClientId != nil { return false }
if locallyOwnedConversationIds.contains(msg.conversationId) { return false }
log.warning("Ignoring host_cu_request for non-local conversation \(msg.conversationId, privacy: .public)")
return true
Expand Down
2 changes: 2 additions & 0 deletions clients/shared/Network/HostProxyClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ public struct HostProxyClient: HostProxyClientProtocol {
let response = try await GatewayHTTPClient.post(
path: "host-file-result",
body: body,
extraHeaders: ["X-Vellum-Client-Id": DeviceIdStore.getOrCreate()],
timeout: timeout
)
guard response.isSuccess else {
Expand All @@ -69,6 +70,7 @@ public struct HostProxyClient: HostProxyClientProtocol {
let response = try await GatewayHTTPClient.post(
path: "host-cu-result",
body: body,
extraHeaders: ["X-Vellum-Client-Id": DeviceIdStore.getOrCreate()],
timeout: 30
)
guard response.isSuccess else {
Expand Down
8 changes: 8 additions & 0 deletions clients/shared/Network/MessageTypes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1614,13 +1614,17 @@ public struct HostFileRequest: Decodable, Sendable {
public let oldString: String?
public let newString: String?
public let replaceAll: Bool?
/// When set, this request is targeted at a specific client ID. Non-nil only for
/// cross-client proxy requests routed through HostFileProxy.
public let targetClientId: String?

private enum CodingKeys: String, CodingKey {
case type, requestId, conversationId, operation, path
case offset, limit, content
case oldString = "old_string"
case newString = "new_string"
case replaceAll = "replace_all"
case targetClientId
}
}

Expand Down Expand Up @@ -1659,6 +1663,9 @@ public struct HostCuRequest: Decodable, Sendable {
public let input: [String: AnyCodable]
public let stepNumber: Int
public let reasoning: String?
/// When set, this request is targeted at a specific client ID. Non-nil only for
/// cross-client proxy requests routed through HostCuProxy.
public let targetClientId: String?

private enum CodingKeys: String, CodingKey {
case type
Expand All @@ -1668,6 +1675,7 @@ public struct HostCuRequest: Decodable, Sendable {
case input
case stepNumber
case reasoning
case targetClientId
}
}

Expand Down