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
11 changes: 1 addition & 10 deletions apps/desktop/src/electron/ElectronMenu.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,7 @@ describe("ElectronMenu", () => {
const electronMenu = yield* ElectronMenu.ElectronMenu;
const selectedItemId = yield* electronMenu.showContextMenu({
window: makeWindow(2),
items: [
{ id: "copy", label: "Copy" },
{ id: "delete", label: "Delete", destructive: true, separatorBefore: true },
],
items: [{ id: "copy", label: "Copy" }],
position: Option.some({ x: 10.8, y: 20.2 }),
});

Expand All @@ -113,12 +110,6 @@ describe("ElectronMenu", () => {
enabled: true,
click: buildFromTemplateMock.mock.calls[0]?.[0][0].click,
});
assert.deepEqual(
buildFromTemplateMock.mock.calls[0]?.[0].map(
(item: Electron.MenuItemConstructorOptions) => item.type ?? item.label,
),
["Copy", "separator", "Delete"],
);
}).pipe(Effect.provide(TestLayer)),
);

Expand Down
10 changes: 1 addition & 9 deletions apps/desktop/src/electron/ElectronMenu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ function normalizeContextMenuItems(source: readonly ContextMenuItem[]): ContextM
label: sourceItem.label,
destructive: sourceItem.destructive === true,
disabled: sourceItem.disabled === true,
...(sourceItem.separatorBefore === true ? { separatorBefore: true } : {}),
};

if (sourceItem.children) {
Expand Down Expand Up @@ -142,17 +141,10 @@ export const make = Effect.gen(function* () {
): Electron.MenuItemConstructorOptions[] => {
const template: Electron.MenuItemConstructorOptions[] = [];
let hasInsertedDestructiveSeparator = false;
const appendSeparator = () => {
if (template.length === 0 || template.at(-1)?.type === "separator") return;
template.push({ type: "separator" });
};

for (const item of entries) {
if (item.separatorBefore) {
appendSeparator();
}
if (item.destructive && !hasInsertedDestructiveSeparator && template.length > 0) {
appendSeparator();
template.push({ type: "separator" });
hasInsertedDestructiveSeparator = true;
}

Expand Down
41 changes: 1 addition & 40 deletions apps/server/src/orchestration/ActivityPayloadProjection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function activity(payload: Record<string, unknown>): OrchestrationThreadActivity
* If slimming ever moves to an allowlist over the whole payload, these
* assertions are the tripwire.
*/
describe("projectActivityPayload", () => {
describe("projectActivityPayload agent-field survival", () => {
it("preserves tool attribution (agentId/parentToolUseId) through data slimming", () => {
const projected = projectActivityPayload(
activity({
Expand All @@ -44,45 +44,6 @@ describe("projectActivityPayload", () => {
expect(data.somethingClientNeverReads).toBeUndefined();
});

it("normalizes Claude and OpenCode command inputs before slimming provider data", () => {
const claude = projectActivityPayload(
activity({
itemType: "command_execution",
toolCallId: "claude-call-1",
data: {
toolName: "Bash",
input: { command: "vp test run" },
result: { content: "x".repeat(5_000) },
},
}),
);
const openCode = projectActivityPayload(
activity({
itemType: "command_execution",
toolCallId: "opencode-call-1",
data: {
tool: "bash",
state: {
status: "running",
input: { command: "vp lint" },
output: "x".repeat(5_000),
},
},
}),
);

expect(claude.payload).toMatchObject({
toolCallId: "claude-call-1",
data: { command: "vp test run" },
});
expect(openCode.payload).toMatchObject({
toolCallId: "opencode-call-1",
data: { command: "vp lint" },
});
expect(JSON.stringify(claude.payload).length).toBeLessThan(200);
expect(JSON.stringify(openCode.payload).length).toBeLessThan(200);
});

it("slims Codex-shaped mcp_tool_call items to rendered fields plus a result summary", () => {
const projected = projectActivityPayload(
activity({
Expand Down
34 changes: 7 additions & 27 deletions apps/server/src/orchestration/ActivityPayloadProjection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,24 +104,6 @@ function projectCommandData(data: Record<string, unknown>): Record<string, unkno
return Object.keys(projectedItem).length > 0 ? projectedItem : undefined;
}

function projectCommandValue(data: Record<string, unknown>): unknown {
if (data.command !== undefined) {
return data.command;
}

const input = asRecord(data.input);
if (input?.command !== undefined) {
return input.command;
}

const stateInput = asRecord(asRecord(data.state)?.input);
if (stateInput?.command !== undefined) {
return stateInput.command;
}

return undefined;
}

function summarizeToolTextOutput(value: string): string | null {
const lines: string[] = [];
for (const rawLine of value.split(/\r?\n/u)) {
Expand Down Expand Up @@ -305,9 +287,8 @@ export function projectActivityPayload(
if (item) {
projectedData.item = item;
}
const command = projectCommandValue(data);
if (command !== undefined) {
projectedData.command = command;
if ("command" in data) {

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.

🟡 Medium orchestration/ActivityPayloadProjection.ts:290

Provider command activities lose their command text during projection, so clients render incomplete tool activity details. The new top-level-only check preserves data.command but drops valid data.input.command and data.state.input.command values after their containing objects are discarded; restore projectCommandValue(data) as the fallback extraction.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/ActivityPayloadProjection.ts around line 290:

Provider command activities lose their command text during projection, so clients render incomplete tool activity details. The new top-level-only check preserves `data.command` but drops valid `data.input.command` and `data.state.input.command` values after their containing objects are discarded; restore `projectCommandValue(data)` as the fallback extraction.

projectedData.command = data.command;
}

const changedFiles: string[] = [];
Expand Down Expand Up @@ -387,19 +368,18 @@ function dropStaleContextWindowActivities(
/**
* Identity both clients use to fold a tool lifecycle row into the call it
* belongs to (`deriveToolLifecycleCollapseKey` in web's `session-logic` and
* mobile's `threadActivity`): the runtime item id ingestion stamps as
* `toolCallId`, a legacy `data.toolCallId`, or the itemType/title/detail triple.
* Returns null for rows with no identity at all — those never collapse on the
* client either, so they must not be dropped here.
* mobile's `threadActivity`): an explicit `data.toolCallId` when the adapter
* emits one, otherwise the itemType/title/detail triple. Returns null for rows
* with no identity at all — those never collapse on the client either, so they
* must not be dropped here.
*/
function toolLifecycleIdentity(activity: OrchestrationThreadActivity): string | null {
const payload = asRecord(activity.payload);
if (!payload) {
return null;
}

const toolCallId =
asTrimmedString(payload.toolCallId) ?? asTrimmedString(asRecord(payload.data)?.toolCallId);
const toolCallId = asTrimmedString(asRecord(payload.data)?.toolCallId);
if (toolCallId) {
return `id:${toolCallId}`;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2811,16 +2811,11 @@ describe("ProviderRuntimeIngestion", () => {
createdAt: now,
threadId: asThreadId("thread-1"),
turnId: asTurnId("turn-9"),
itemId: asItemId("tool-call-9"),
payload: {
itemType: "command_execution",
status: "inProgress",
title: "Command run",
detail: "Bash: vp test run",
data: {
toolName: "Bash",
input: { command: "vp test run" },
},
status: "in_progress",
title: "Read file",
detail: "/tmp/file.ts",
},
});

Expand All @@ -2835,20 +2830,11 @@ describe("ProviderRuntimeIngestion", () => {
);

expect(thread.session?.status).toBe("ready");
const activity = thread.activities.find(
(entry: ProviderRuntimeTestActivity) => entry.kind === "tool.started",
);
const payload = activity?.payload as Record<string, unknown> | undefined;
expect(payload).toMatchObject({
itemType: "command_execution",
toolCallId: "tool-call-9",
status: "inProgress",
detail: "Bash: vp test run",
data: {
toolName: "Bash",
input: { command: "vp test run" },
},
});
expect(
thread.activities.some(
(activity: ProviderRuntimeTestActivity) => activity.kind === "tool.started",
),
).toBe(true);
});

it("consumes P1 runtime events into thread metadata, diff checkpoints, and activities", async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,6 @@ export function runtimeEventToActivities(
summary: event.payload.title ?? "Tool updated",
payload: {
itemType: event.payload.itemType,
...(event.itemId !== undefined ? { toolCallId: event.itemId } : {}),
...(event.payload.status ? { status: event.payload.status } : {}),
...(event.payload.detail ? { detail: truncateDetail(event.payload.detail) } : {}),
...(event.payload.data !== undefined ? { data: event.payload.data } : {}),
Expand Down Expand Up @@ -822,8 +821,6 @@ export function runtimeEventToActivities(
summary: event.payload.title ?? "Tool",
payload: {
itemType: event.payload.itemType,
...(event.itemId !== undefined ? { toolCallId: event.itemId } : {}),
...(event.payload.status ? { status: event.payload.status } : {}),
...(event.payload.detail ? { detail: truncateDetail(event.payload.detail) } : {}),

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.

🟡 Medium Layers/ProviderRuntimeIngestion.ts:824

Failed item.completed events are projected without status, so the web client renders them as successfully completed instead of failed. Restore the event.payload.status field in the tool.completed payload.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts around line 824:

Failed `item.completed` events are projected without `status`, so the web client renders them as successfully completed instead of failed. Restore the `event.payload.status` field in the `tool.completed` payload.

...(event.payload.data !== undefined ? { data: event.payload.data } : {}),
...(event.payload.agentId ? { agentId: event.payload.agentId } : {}),
Expand All @@ -850,10 +847,7 @@ export function runtimeEventToActivities(
summary: `${event.payload.title ?? "Tool"} started`,
payload: {
itemType: event.payload.itemType,
...(event.itemId !== undefined ? { toolCallId: event.itemId } : {}),
...(event.payload.status ? { status: event.payload.status } : {}),
...(event.payload.detail ? { detail: truncateDetail(event.payload.detail) } : {}),
...(event.payload.data !== undefined ? { data: event.payload.data } : {}),
...(event.payload.agentId ? { agentId: event.payload.agentId } : {}),
...(event.payload.parentToolUseId
? { parentToolUseId: event.payload.parentToolUseId }
Expand Down
58 changes: 26 additions & 32 deletions apps/web/src/components/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ import {
WifiOffIcon,
} from "lucide-react";
import { cn, randomHex } from "~/lib/utils";
import { COLLAPSED_SIDEBAR_TITLEBAR_INSET_CLASS } from "~/workspaceTitlebar";
import { stackedThreadToast, toastManager } from "./ui/toast";
import { decodeProjectScriptKeybindingRule } from "~/lib/projectScriptKeybindings";
import { type NewProjectScriptInput } from "./ProjectScriptsControl";
Expand Down Expand Up @@ -219,11 +220,7 @@ import {
import { appendPreviewAnnotationPrompt } from "../lib/previewAnnotation";
import { appendReviewCommentsToPrompt, type ReviewCommentContext } from "../reviewCommentContext";
import { environmentCatalog } from "../connection/catalog";
import {
selectThreadTerminalCustomLabels,
selectThreadTerminalUiState,
useTerminalUiStateStore,
} from "../terminalUiStateStore";
import { selectThreadTerminalUiState, useTerminalUiStateStore } from "../terminalUiStateStore";
import { useKnownTerminalSessions, useThreadRunningTerminalIds } from "../state/terminalSessions";
import { projectEnvironment } from "../state/projects";
import { useEnvironmentQuery } from "../state/query";
Expand Down Expand Up @@ -259,7 +256,6 @@ import { ChatHeader } from "./chat/ChatHeader";
import { PanelLayoutControls, RightPanelMaximizeControl } from "./chat/PanelLayoutControls";
import { type ExpandedImagePreview } from "./chat/ExpandedImagePreview";
import { NoActiveThreadState } from "./NoActiveThreadState";
import { WorkspacePageHeader } from "./WorkspacePageContainer";
import {
resolveEffectiveEnvMode,
resolveLocalCheckoutBranchMismatch,
Expand Down Expand Up @@ -657,7 +653,6 @@ interface PersistentThreadTerminalDrawerProps {
newShortcutLabel: string | undefined;
closeShortcutLabel: string | undefined;
keybindings: ResolvedKeybindingsConfig;
onHide: () => void;
onAddTerminalContext: (selection: TerminalContextSelection) => void;
}

Expand All @@ -672,7 +667,6 @@ const PersistentThreadTerminalDrawer = memo(function PersistentThreadTerminalDra
newShortcutLabel,
closeShortcutLabel,
keybindings,
onHide,
onAddTerminalContext,
}: PersistentThreadTerminalDrawerProps) {
const openTerminal = useAtomCommand(terminalEnvironment.open, "terminal open");
Expand Down Expand Up @@ -996,7 +990,6 @@ const PersistentThreadTerminalDrawer = memo(function PersistentThreadTerminalDra
onSplitTerminal={splitTerminal}
onSplitTerminalVertical={splitTerminalVertical}
onNewTerminal={createNewTerminal}
onHide={onHide}
splitShortcutLabel={visible ? splitShortcutLabel : undefined}
splitVerticalShortcutLabel={visible ? splitVerticalShortcutLabel : undefined}
newShortcutLabel={visible ? newShortcutLabel : undefined}
Expand Down Expand Up @@ -1547,16 +1540,6 @@ function ChatViewContent(props: ChatViewProps) {
const canCheckoutPullRequestIntoThread = isLocalDraftThread;
const activeThreadId = activeThread?.id ?? null;
const activeThreadEnvironmentId = activeThread?.environmentId ?? null;
const activeThreadRef = useMemo(
() =>
activeThreadEnvironmentId && activeThreadId
? scopeThreadRef(activeThreadEnvironmentId, activeThreadId)
: null,
[activeThreadEnvironmentId, activeThreadId],
);
const activeTerminalCustomLabels = useTerminalUiStateStore((state) =>
selectThreadTerminalCustomLabels(state.terminalCustomLabelsByThreadKey, activeThreadRef),
);
const runningTerminalIds = useThreadRunningTerminalIds({
environmentId: activeThread?.environmentId ?? null,
threadId: activeThreadId,
Expand Down Expand Up @@ -1586,15 +1569,18 @@ function ChatViewContent(props: ChatViewProps) {
for (const session of activeThreadKnownSessions) {
labels.set(
session.target.terminalId,
activeTerminalCustomLabels[session.target.terminalId] ??
resolveTerminalSessionLabel(session.target.terminalId, session.state.summary),
resolveTerminalSessionLabel(session.target.terminalId, session.state.summary),
);
}
for (const [terminalId, label] of Object.entries(activeTerminalCustomLabels)) {
if (!labels.has(terminalId)) labels.set(terminalId, label);
}
return labels;
}, [activeTerminalCustomLabels, activeThreadKnownSessions]);
}, [activeThreadKnownSessions]);
const activeThreadRef = useMemo(
() =>
activeThreadEnvironmentId && activeThreadId
? scopeThreadRef(activeThreadEnvironmentId, activeThreadId)
: null,
[activeThreadEnvironmentId, activeThreadId],
);
const activeThreadKey = activeThreadRef ? scopedThreadKey(activeThreadRef) : null;
const [timelineAnchor, setTimelineAnchor] = useState<{
readonly threadKey: string | null;
Expand Down Expand Up @@ -2822,7 +2808,6 @@ function ChatViewContent(props: ChatViewProps) {
},
[activeThreadRef, storeSetTerminalOpen],
);
const hideTerminal = useCallback(() => setTerminalOpen(false), [setTerminalOpen]);
const toggleTerminalVisibility = useCallback(() => {
if (!activeThreadRef) return;
const nextOpen = !terminalUiState.terminalOpen;
Expand Down Expand Up @@ -6129,6 +6114,7 @@ function ChatViewContent(props: ChatViewProps) {
? "thread"
: "page"
}
chromeVariant="collapse"
composerDraftTarget={composerDraftTarget}
onStateChange={handlePullRequestTabStatusChange}
/>
Expand Down Expand Up @@ -6174,11 +6160,20 @@ function ChatViewContent(props: ChatViewProps) {
data-chat-column-maximized-away={rightPanelMaximized ? "true" : "false"}
>
{/* Top bar */}
<WorkspacePageHeader
<header
data-chat-header
electron={isElectron}
reserveNativeControls={reserveTitleBarControlInset && !inlineRightPanelOwnsTitleBar}
className="relative bg-background"
className={cn(
"bg-background transition-[padding-left] duration-200 ease-linear motion-reduce:transition-none",
isElectron
? cn(
"drag-region relative flex h-[var(--workspace-topbar-height)] min-h-[var(--workspace-topbar-height)] shrink-0 items-center px-3 sm:px-5",
reserveTitleBarControlInset &&
!inlineRightPanelOwnsTitleBar &&
"wco:pr-[var(--workspace-native-controls-inset)]",
)
: "flex h-[var(--workspace-topbar-height)] min-h-[var(--workspace-topbar-height)] shrink-0 items-center pl-[calc(env(safe-area-inset-left)+0.75rem)] pr-[calc(env(safe-area-inset-right)+0.75rem)] sm:pl-[calc(env(safe-area-inset-left)+1.25rem)] sm:pr-[calc(env(safe-area-inset-right)+1.25rem)]",
COLLAPSED_SIDEBAR_TITLEBAR_INSET_CLASS,
)}
>
{!rightPanelOpen ? panelLayoutControls : null}
<ChatHeader
Expand Down Expand Up @@ -6209,7 +6204,7 @@ function ChatViewContent(props: ChatViewProps) {
onUpdateProjectScript={updateProjectScript}
onDeleteProjectScript={deleteProjectScript}
/>
</WorkspacePageHeader>
</header>

<ThreadErrorBanner
error={visibleThreadError}
Expand Down Expand Up @@ -6547,7 +6542,6 @@ function ChatViewContent(props: ChatViewProps) {
newShortcutLabel={newTerminalShortcutLabel ?? undefined}
closeShortcutLabel={closeTerminalShortcutLabel ?? undefined}
keybindings={keybindings}
onHide={hideTerminal}
onAddTerminalContext={addTerminalContextToDraft}
/>
))}
Expand Down
Loading
Loading