Skip to content

refactor: discriminated union for transport metadata, remove iOS proxy setup #23776

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
noanflaherty merged 1 commit into from
Apr 6, 2026
Merged

refactor: discriminated union for transport metadata, remove iOS proxy setup #23776

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
11 changes: 4 additions & 7 deletions assistant/src/daemon/handlers/conversations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,10 @@ export function makeEventSender(params: {
guardianPrincipalId: trustContext?.guardianPrincipalId ?? undefined,
toolName: event.toolName,
commandPreview:
redactSecrets(
summarizeToolInput(event.toolName, inputRecord),
) || undefined,
redactSecrets(summarizeToolInput(event.toolName, inputRecord)) ||
undefined,
riskLevel: event.riskLevel,
activityText: activityRaw
? redactSecrets(activityRaw)
: undefined,
activityText: activityRaw ? redactSecrets(activityRaw) : undefined,
executionTarget: event.executionTarget,
status: "pending",
requestCode: generateCanonicalRequestCode(),
Expand Down Expand Up @@ -304,7 +301,7 @@ export async function handleConversationCreate(
// Only create the host bash proxy for desktop client interfaces that can
// execute commands on the user's machine. Set before updateClient so
// updateClient's call to hostBashProxy.updateSender targets the new proxy.
if (transportInterface === "macos" || transportInterface === "ios") {
if (transportInterface === "macos") {
const proxy = new HostBashProxy(sendEvent, (requestId) => {
pendingInteractions.resolve(requestId);
});
Expand Down
27 changes: 23 additions & 4 deletions assistant/src/daemon/message-types/conversations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@ export interface ConversationListRequest {
limit?: number;
}

/** Lightweight conversation transport metadata for channel identity and natural-language guidance. */
export interface ConversationTransportMetadata {
/** Shared fields for all transport metadata variants. */
interface BaseTransportMetadata {
/** Logical channel identifier (e.g. "desktop", "telegram", "mobile"). */
channelId: ChannelId;
/** Interface identifier for this transport (e.g. "macos", "ios", "cli"). */
interfaceId?: InterfaceId;
/** Optional natural-language hints for channel-specific UX behavior. */
hints?: string[];
/** Optional concise UX brief for this channel. */
Expand All @@ -28,6 +26,27 @@ export interface ConversationTransportMetadata {
chatType?: string;
}

/** Transport metadata for macOS desktop clients, including host environment fields. */
export interface MacosTransportMetadata extends BaseTransportMetadata {
/** Interface identifier for macOS transport. */
interfaceId: "macos";
/** Home directory of the host macOS user. */
hostHomeDir?: string;
/** Username of the host macOS user. */
hostUsername?: string;
Comment on lines +34 to +36

@devin-ai-integration devin-ai-integration Bot Apr 6, 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.

🚩 New hostHomeDir/hostUsername fields are defined but not consumed server-side

The MacosTransportMetadata type adds hostHomeDir and hostUsername optional fields (assistant/src/daemon/message-types/conversations.ts:34-36), and the Swift client adds matching properties (clients/shared/Network/Generated/GeneratedAPITypes.swift:3591-3602). However, no server-side code currently reads these fields from the transport metadata. This is likely intentional scaffolding for future use — the fields are optional so nothing breaks — but the reviewer should confirm that a follow-up PR will consume them.

Open in Devin Review

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

}

/** Transport metadata for non-macOS transports. */
export interface NonMacosTransportMetadata extends BaseTransportMetadata {
/** Interface identifier for this transport (e.g. "ios", "cli"). */
interfaceId?: Exclude<InterfaceId, "macos">;
}

/** Lightweight conversation transport metadata for channel identity and natural-language guidance. */
export type ConversationTransportMetadata =
| MacosTransportMetadata
| NonMacosTransportMetadata;

export interface ConversationCreateRequest {
type: "conversation_create";
title?: string;
Expand Down
2 changes: 1 addition & 1 deletion assistant/src/daemon/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1075,7 +1075,7 @@ export class DaemonServer {
// Guard: don't replace an active proxy during concurrent turn races —
// another request may have started processing between the isProcessing()
// check above and the await on ensureActorScopedHistory().
if (resolvedInterface === "macos" || resolvedInterface === "ios") {
if (resolvedInterface === "macos") {
if (!conversation.isProcessing() || !conversation.hostBashProxy) {
conversation.setHostBashProxy(
new HostBashProxy(conversation.getCurrentSender(), (requestId) => {
Expand Down
10 changes: 5 additions & 5 deletions assistant/src/runtime/routes/conversation-routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,9 @@ function isToolResultType(type: string): boolean {
function isSystemNoticeText(block: Record<string, unknown>): boolean {
if (block.type !== "text") return false;
const text = typeof block.text === "string" ? block.text : "";
return text.startsWith("<system_notice>") && text.endsWith("</system_notice>");
return (
text.startsWith("<system_notice>") && text.endsWith("</system_notice>")
);
}

/**
Expand Down Expand Up @@ -1115,7 +1117,7 @@ export async function handleSendMessage(
// channels, headless) fall back to local execution.
// Set the proxy BEFORE updateClient so updateClient's call to
// hostBashProxy.updateSender targets the correct (new) proxy.
if (sourceInterface === "macos" || sourceInterface === "ios") {
if (sourceInterface === "macos") {
// Reuse the existing proxy if the conversation is actively processing a
// host bash request to avoid orphaning in-flight requests.
if (!conversation.isProcessing() || !conversation.hostBashProxy) {
Expand Down Expand Up @@ -1152,9 +1154,7 @@ export async function handleSendMessage(
// When proxies are preserved during an active turn (non-desktop request while
// processing), skip updating proxy senders to avoid degrading them.
const preservingProxies =
conversation.isProcessing() &&
sourceInterface !== "macos" &&
sourceInterface !== "ios";
conversation.isProcessing() && sourceInterface !== "macos";
Comment on lines 1156 to +1157

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 6, 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 Prevent queued iOS turns from re-enabling host proxies

This update removes iOS proxy setup, but this condition now preserves existing proxies for any non-macOS interface during active turns. In a mixed-client scenario (macOS turn in flight, then iOS message queued on the same conversation), those preserved proxies are later re-enabled because drainQueueImpl still treats "ios" as desktop (assistant/src/daemon/conversation-process.ts around sourceInterface === "macos" || sourceInterface === "ios"). The queued iOS turn can therefore still execute host_bash/host_file/computer-use through the preserved desktop proxy, which contradicts the iOS removal and can run tools on the host unexpectedly.

Useful? React with 👍 / 👎.

conversation.updateClient(onEvent, !isInteractive, {
skipProxySenderUpdate: preservingProxies,
});
Expand Down
8 changes: 7 additions & 1 deletion clients/shared/Network/Generated/GeneratedAPITypes.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3588,12 +3588,18 @@ public struct ConversationTransportMetadata: Codable, Sendable {
public let hints: [String]?
/// Optional concise UX brief for this channel.
public let uxBrief: String?
/// Home directory of the host macOS user. Only populated when interfaceId == "macos".
public let hostHomeDir: String?
/// Username of the host macOS user. Only populated when interfaceId == "macos".
public let hostUsername: String?

public init(channelId: String, interfaceId: String? = nil, hints: [String]? = nil, uxBrief: String? = nil) {
public init(channelId: String, interfaceId: String? = nil, hints: [String]? = nil, uxBrief: String? = nil, hostHomeDir: String? = nil, hostUsername: String? = nil) {
self.channelId = channelId
self.interfaceId = interfaceId
self.hints = hints
self.uxBrief = uxBrief
self.hostHomeDir = hostHomeDir
self.hostUsername = hostUsername
}
}

Expand Down
Loading