Skip to content
Closed
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
28 changes: 18 additions & 10 deletions assistant/src/__tests__/host-transfer-proxy-targeted.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*
* Covers:
* - requestToHost() explicit valid targetClientId → validates, broadcasts with targetClientId
* - requestToHost() auto-resolve when exactly one host_file-capable client → auto-resolves
* - requestToHost() untargeted call with single capable client → broadcasts without targetClientId (no auto-resolve)
* - requestToHost() unknown targetClientId → early error, no broadcast
* - requestToHost() incapable client → early error, no broadcast
* - requestToSandbox() explicit valid targetClientId → same 4 cases
Expand Down Expand Up @@ -142,10 +142,16 @@ describe("HostTransferProxy — targetClientId", () => {
});
});

// ── requestToHost() — auto-resolve single capable client ─────────────
// ── requestToHost() — untargeted single client (no auto-resolve) ──────
//
// Rationale: auto-resolving targetClientId for single clients requires the
// macOS client to send x-vellum-client-id on content requests. Pre-Phase 3
// clients don't send this header, causing a 400 and a 120 s hang. For
// untargeted calls we broadcast without targetClientId (backward-compatible)
// regardless of how many host_file-capable clients are connected.

describe("requestToHost() — auto-resolve when exactly one capable client", () => {
test("auto-resolves targetClientId to the single capable client", async () => {
describe("requestToHost() — untargeted call with single capable client", () => {
test("broadcasts without targetClientId (no auto-resolve)", async () => {
setup();
setupSingleClient("client-solo");

Expand All @@ -162,10 +168,11 @@ describe("HostTransferProxy — targetClientId", () => {
await waitForMessages(sentMessages, 1);

const sent = sentMessages[0] as Record<string, unknown>;
expect(sent.targetClientId).toBe("client-solo");
// No auto-resolve: untargeted broadcast should have no targetClientId.
expect(sent.targetClientId).toBeUndefined();

const opts = sentMessageOptions[0] as Record<string, unknown> | undefined;
expect(opts?.targetClientId).toBe("client-solo");
expect(opts?.targetClientId).toBeUndefined();

const requestId = sent.requestId as string;
proxy.resolveTransferResult(requestId, { isError: false });
Expand Down Expand Up @@ -249,10 +256,10 @@ describe("HostTransferProxy — targetClientId", () => {
});
});

// ── requestToSandbox() — auto-resolve single capable client ──────────
// ── requestToSandbox() — untargeted single client (no auto-resolve) ────

describe("requestToSandbox() — auto-resolve when exactly one capable client", () => {
test("auto-resolves targetClientId", async () => {
describe("requestToSandbox() — untargeted call with single capable client", () => {
test("broadcasts without targetClientId (no auto-resolve)", async () => {
setup();
setupSingleClient("client-solo");

Expand All @@ -264,7 +271,8 @@ describe("HostTransferProxy — targetClientId", () => {

expect(sentMessages).toHaveLength(1);
const sent = sentMessages[0] as Record<string, unknown>;
expect(sent.targetClientId).toBe("client-solo");
// No auto-resolve: untargeted broadcast should have no targetClientId.
expect(sent.targetClientId).toBeUndefined();

proxy.cancel(sent.requestId as string);
await resultPromise;
Expand Down
6 changes: 0 additions & 6 deletions assistant/src/daemon/host-transfer-proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,6 @@ export class HostTransferProxy {
isError: true,
});
}
} else {
const capable = assistantEventHub.listClientsByCapability("host_file");
if (capable.length === 1) resolvedTargetClientId = capable[0].clientId;
}

const requestId = uuid();
Expand Down Expand Up @@ -311,9 +308,6 @@ export class HostTransferProxy {
isError: true,
});
}
} else {
const capable = assistantEventHub.listClientsByCapability("host_file");
if (capable.length === 1) resolvedTargetClientId = capable[0].clientId;
}

const requestId = uuid();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ struct ConversationTitleActionsControl: View {
let onArchive: () -> Void
let onRename: () -> Void
let onOpenForkParent: () -> Void
let onAnalyzeConversation: () -> Void
var onAnalyzeConversation: (() -> Void)? = nil
let onRefresh: () -> Void
var onOpenInNewWindow: (() -> Void)? = nil

Expand Down Expand Up @@ -167,7 +167,7 @@ struct ConversationActionsMenuContent: View {
let onUnpin: () -> Void
let onArchive: () -> Void
let onRename: () -> Void
let onAnalyzeConversation: () -> Void
var onAnalyzeConversation: (() -> Void)? = nil
let onRefresh: () -> Void
var onOpenInNewWindow: (() -> Void)? = nil

Expand All @@ -181,7 +181,7 @@ struct ConversationActionsMenuContent: View {
VMenuItem(icon: VIcon.gitBranch.rawValue, label: "Fork conversation", action: onForkConversation)
}

if presentation.isPersisted && !presentation.isChannelConversation {
if presentation.isPersisted && !presentation.isChannelConversation, let onAnalyzeConversation {
VMenuItem(
icon: VIcon.sparkles.rawValue,
label: "Analyze conversation",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ struct ConversationTitleOverlay: View {
let onArchive: () -> Void
let onRename: () -> Void
let onOpenForkParent: () -> Void
let onAnalyzeConversation: () -> Void
var onAnalyzeConversation: (() -> Void)? = nil
let onRefresh: () -> Void
var onOpenInNewWindow: (() -> Void)? = nil

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ struct ConversationSwitcherDrawer: View {
let selectConversation: (ConversationModel) -> Void
let onDismiss: () -> Void

@Environment(AssistantFeatureFlagStore.self) private var assistantFeatureFlagStore

/// Max conversations shown per section before "Show more".
private let maxPerSection = 5

Expand Down Expand Up @@ -91,7 +93,7 @@ struct ConversationSwitcherDrawer: View {
onDragStart: {
sidebar.beginConversationDrag(conversation.id)
},
onAnalyze: conversation.conversationId != nil && !conversation.isChannelConversation ? {
onAnalyze: conversation.conversationId != nil && !conversation.isChannelConversation && assistantFeatureFlagStore.isEnabled("analyze-conversation") ? {
selectConversation(conversation)
Task<Void, Never> { await conversationManager.analyzeActiveConversation() }
} : nil,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ struct SidebarView: View {
onDragStart: {
sidebar.beginConversationDrag(conversation.id)
},
onAnalyze: conversation.conversationId != nil && !conversation.isChannelConversation ? {
onAnalyze: conversation.conversationId != nil && !conversation.isChannelConversation && assistantFeatureFlagStore.isEnabled("analyze-conversation") ? {
selectConversation(conversation)
Task<Void, Never> { await conversationManager.analyzeActiveConversation() }
} : nil,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ struct TopBarView: View {
let onRenameConversation: () -> Void
let onOpenForkParent: () -> Void

@Environment(AssistantFeatureFlagStore.self) private var assistantFeatureFlagStore

@AppStorage("sidebarExpanded") private var sidebarExpanded: Bool = true
@AppStorage("sidebarToggleShortcut") private var sidebarToggleShortcut: String = "cmd+\\"
@AppStorage("homeShortcut") private var homeShortcut: String = "cmd+shift+h"
Expand Down Expand Up @@ -195,9 +197,9 @@ struct TopBarView: View {
},
onRename: onRenameConversation,
onOpenForkParent: onOpenForkParent,
onAnalyzeConversation: {
onAnalyzeConversation: assistantFeatureFlagStore.isEnabled("analyze-conversation") ? {
Task { await conversationManager.analyzeActiveConversation() }
},
} : nil,
onRefresh: {
conversationManager.refreshActiveConversation()
},
Expand Down
8 changes: 8 additions & 0 deletions meta/feature-flags/feature-flag-registry.json
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,14 @@
"label": "App Control",
"description": "Enable the app-control skill (per-app screenshot + raw input bypassing AX tree)",
"defaultEnabled": false
},
{
"id": "analyze-conversation",
"scope": "assistant",
"key": "analyze-conversation",
"label": "Analyze Conversation",
"description": "Show the 'Analyze' / 'Analyze conversation' option in conversation context menus and the conversation title actions dropdown.",
"defaultEnabled": false
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep analyze-conversation enabled by default

This change gates every manual Analyze entry point behind analyze-conversation, but the newly introduced registry default is false, so existing users lose the Analyze action immediately after upgrade unless a per-assistant override is already present. Because this commit does not add any bootstrap/migration path that seeds the flag to true, it introduces a user-facing behavior regression unrelated to the host-transfer fix.

Useful? React with 👍 / 👎.

}
]
}
Loading