From 33b7d7b3ae2a7b1c3d1bd58c04cb9bf581ec2fe7 Mon Sep 17 00:00:00 2001 From: maria Date: Sun, 4 Oct 2026 07:58:30 -0300 Subject: [PATCH 01/47] feat(web): add shift-held pull request quick actions (#15549) Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com> --- .../PullRequestDetailPanel.test.tsx | 1 + .../pullRequest/PullRequestDetailPanel.tsx | 60 +--- .../components/pullRequest/PullRequestRow.tsx | 267 +++++++++--------- .../pullRequest/PullRequestSpeedActions.tsx | 135 +++++++++ .../pullRequest/pullRequestChecks.test.tsx | 2 + .../pullRequest/usePullRequestActions.ts | 89 ++++-- apps/web/src/routes/_chat.pull-requests.tsx | 21 +- apps/web/src/shortcutModifierState.ts | 16 +- 8 files changed, 392 insertions(+), 199 deletions(-) create mode 100644 apps/web/src/components/pullRequest/PullRequestSpeedActions.tsx diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx index 884bef0bf0b8..a39e144ca382 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.test.tsx @@ -32,6 +32,7 @@ vi.mock("~/state/environments", () => ({ usePrimaryEnvironmentId: () => EnvironmentId.make("env-1"), })); vi.mock("~/hooks/useSettings", () => ({ + useEnvironmentSettings: () => undefined, useClientSettings: (select: (settings: typeof DEFAULT_CLIENT_SETTINGS) => unknown) => select(DEFAULT_CLIENT_SETTINGS), })); diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index bf5bd62de91c..53dc7642d9be 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -6,7 +6,6 @@ import { scopedThreadKey, scopeProjectRef } from "@t3tools/client-runtime/enviro import { squashAtomCommandFailure } from "@t3tools/client-runtime/state/runtime"; import { type EnvironmentId, - DEFAULT_SERVER_SETTINGS, type PullRequestAction, type PullRequestMergeMethod, type PullRequestListEntry, @@ -15,7 +14,6 @@ import { resolveEnvironmentMachineKind, type ScopedThreadRef, } from "@t3tools/contracts"; -import { resolveProjectSettings } from "@t3tools/shared/projectSettings"; import { ArrowDownUpIcon, ArrowLeftIcon, @@ -63,20 +61,14 @@ import { type ShortcutMatchContext, } from "~/keybindings"; import { primaryServerKeybindingsAtom } from "~/state/server"; -import { useClientSettings } from "~/hooks/useSettings"; -import { - deriveLogicalProjectKeyFromSettings, - derivePhysicalProjectKey, - selectProjectGroupingSettings, -} from "~/logicalProject"; +import { usePullRequestDefaultMergeMethodResolver } from "./usePullRequestActions"; import { changeRequestRepositoryUrl, gitHubPullRequestBrowserUrl } from "~/lib/openPullRequestLink"; import { usePreparePullRequestThreadAction } from "~/lib/sourceControlActions"; import { cn } from "~/lib/utils"; import { readLocalApi } from "~/localApi"; import type { ReviewCommentContext } from "~/reviewCommentContext"; -import { buildPhysicalToLogicalProjectKeyMap } from "~/sidebarProjectGrouping"; import { useProjects, useServerConfigs } from "~/state/entities"; -import { useEnvironments, usePrimaryEnvironmentId } from "~/state/environments"; +import { useEnvironments } from "~/state/environments"; import { useEnvironmentQuery } from "~/state/query"; import { useLiveRefresh } from "~/hooks/useLiveRefresh"; import { @@ -563,19 +555,14 @@ export function PullRequestDetailPanel({ }, [condensed]); const lastSelectedMergeMethod = useUiStateStore((state) => state.pullRequestMergeMethod); const setLastSelectedMergeMethod = useUiStateStore((state) => state.setPullRequestMergeMethod); - // Server-side and per project, like every other project setting. The - // client-local per-project map from before still answers when the server - // has no value, so a choice made on an older release keeps applying until - // it is set (or reset) in Settings. - const legacyMergeMethodOverrides = useClientSettings( - (settings) => settings.pullRequestMergeMethodOverrides, + const resolveProjectDefaultMergeMethod = usePullRequestDefaultMergeMethodResolver( + environmentId, + reference.projectId, + ); + const projectDefaultMergeMethod = useMemo( + () => resolveProjectDefaultMergeMethod(), + [resolveProjectDefaultMergeMethod], ); - const projectGroupingSettings = useClientSettings(selectProjectGroupingSettings); - const projectDefaultMergeMethod = - resolveProjectSettings( - environmentConfigs.get(environmentId)?.settings ?? DEFAULT_SERVER_SETTINGS, - reference.projectId, - ).settings.pullRequestMergeMethod ?? undefined; const [mergeMethodSelection, setMergeMethodSelection] = useState<{ readonly pullRequestKey: string; readonly method: PullRequestMergeMethod; @@ -887,39 +874,12 @@ export function PullRequestDetailPanel({ const [titleSaving, setTitleSaving] = useState(false); const newThread = useNewThreadHandler(); const { environments } = useEnvironments(); - const primaryEnvironmentId = usePrimaryEnvironmentId(); const unavailableGitHubUrl = useMemo(() => { const identity = projects.find( (project) => project.id === reference.projectId && project.environmentId === environmentId, )?.repositoryIdentity; return gitHubPullRequestBrowserUrl(identity, reference.repository, reference.number); }, [environmentId, projects, reference.number, reference.projectId, reference.repository]); - // Project settings stored the override under the sidebar group's key, which a duplicate row - // borrows from its siblings, so the project alone does not always name the same key. - const legacyProjectDefaultMergeMethod = useMemo(() => { - if (projectDefaultMergeMethod !== undefined) return undefined; - const project = projects.find( - (candidate) => - candidate.environmentId === environmentId && candidate.id === reference.projectId, - ); - if (!project) return undefined; - const projectKey = - buildPhysicalToLogicalProjectKeyMap({ - projects, - settings: projectGroupingSettings, - primaryEnvironmentId, - }).get(derivePhysicalProjectKey(project)) ?? - deriveLogicalProjectKeyFromSettings(project, projectGroupingSettings); - return legacyMergeMethodOverrides[projectKey]; - }, [ - environmentId, - legacyMergeMethodOverrides, - primaryEnvironmentId, - projectDefaultMergeMethod, - projectGroupingSettings, - projects, - reference.projectId, - ]); // Beside a thread there is nothing to pick: the hand-offs land in that thread's composer, and // the thread is already on one server's copy of the branch. const pickableEnvironments = useMemo( @@ -1421,7 +1381,7 @@ export function PullRequestDetailPanel({ const selectedMergeMethod = resolvePullRequestMergeMethod( allowedMergeMethods, currentMergeMethod, - projectDefaultMergeMethod ?? legacyProjectDefaultMergeMethod, + projectDefaultMergeMethod, lastSelectedMergeMethod, ); const selectedMergeMethodLabel = PULL_REQUEST_MERGE_METHOD_LABELS[selectedMergeMethod]; diff --git a/apps/web/src/components/pullRequest/PullRequestRow.tsx b/apps/web/src/components/pullRequest/PullRequestRow.tsx index 04008281f5e7..1f4de90f55b8 100644 --- a/apps/web/src/components/pullRequest/PullRequestRow.tsx +++ b/apps/web/src/components/pullRequest/PullRequestRow.tsx @@ -1,5 +1,9 @@ import { SearchIcon } from "lucide-react"; import { PullRequestStackPopover } from "./PullRequestStackPopover"; +import { + PullRequestSpeedActions, + type PullRequestSpeedActionResult, +} from "./PullRequestSpeedActions"; import { memo, type RefCallback } from "react"; import { cn } from "~/lib/utils"; @@ -66,10 +70,9 @@ function PullRequestRowLabels({ labels }: { labels: EnvironmentPullRequestEntry[ /** * The page row keeps a little more room around the shared lines than the panel, which sits in - * a narrow column. The intrinsic size is the content box a skipped row reserves, which is the - * two lines without the padding: a 56px row less 20px of `py-2.5`. + * a narrow column. Its outer wrapper reserves the full row height when offscreen. */ -const PAGE_ROW_CLASS = "px-3 py-2.5 [contain-intrinsic-block-size:36.5px]"; +const PAGE_ROW_CLASS = "px-3 py-2.5"; export type PullRequestRowTarget = Pick< EnvironmentPullRequestEntry, @@ -86,6 +89,8 @@ function PullRequestRowImpl({ statsKey, statsRef, onSelect, + speedMode, + onActed, }: { entry: EnvironmentPullRequestEntry; selected: boolean; @@ -101,144 +106,152 @@ function PullRequestRowImpl({ matchedElsewhere?: boolean; /** Used by the list's shared visibility observer to defer optional line-count reads. */ statsKey?: string; - statsRef?: RefCallback; + statsRef?: RefCallback; onSelect: (entry: PullRequestRowTarget) => void; + speedMode: boolean; + onActed: (result: PullRequestSpeedActionResult) => void; }) { const { Icon, providerName } = getSourceControlPresentationForKind(entry.provider); return ( - + /> + ) : null} + + + } + metaClassName="@container/pr-row-meta" + meta={ + <> + {matchedElsewhere ? ( + + + } + > + matched in the description + + + matched in the description + + + Matched in the description + + ) : null} + {showProvider ? ( + + }> + + + {providerName} + + ) : null} + + {showProjectTitle ? {entry.repository} : null} + {environmentLabel ? ( + {environmentLabel} + ) : null} + {entry.labels.length > 0 ? : null} + + } + updatedAt={entry.updatedAt} + /> + + {entry.state !== "merged" && entry.provider === "github" ? ( + + ) : null} + ); } diff --git a/apps/web/src/components/pullRequest/PullRequestSpeedActions.tsx b/apps/web/src/components/pullRequest/PullRequestSpeedActions.tsx new file mode 100644 index 000000000000..613928733d37 --- /dev/null +++ b/apps/web/src/components/pullRequest/PullRequestSpeedActions.tsx @@ -0,0 +1,135 @@ +import type { PullRequestAction } from "@t3tools/contracts"; +import { Effect } from "effect"; +import { AtomRegistry } from "effect/unstable/reactivity"; +import { appAtomRegistry } from "~/rpc/atomRegistry"; +import { pullRequestEnvironment, pullRequestStackAtom } from "~/state/pullRequests"; +import { useUiStateStore } from "~/uiStateStore"; +import { Button } from "../ui/button"; +import { Spinner } from "../ui/spinner"; +import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; +import { resolvePullRequestMergeMethod } from "./pullRequestDetail.logic"; +import { PullRequestGlyph } from "./pullRequestIcons"; +import type { EnvironmentPullRequestEntry } from "./pullRequestList.logic"; +import { + usePullRequestActionRunner, + usePullRequestDefaultMergeMethodResolver, +} from "./usePullRequestActions"; + +export interface PullRequestSpeedActionResult { + readonly entry: EnvironmentPullRequestEntry; + readonly action: PullRequestAction; +} + +/** No detail or stack reads until a merge is clicked, even on a long list. */ +export function PullRequestSpeedActions({ + entry, + visible, + onActed, +}: { + entry: EnvironmentPullRequestEntry; + visible: boolean; + onActed: (result: PullRequestSpeedActionResult) => void; +}) { + const resolveProjectDefault = usePullRequestDefaultMergeMethodResolver( + entry.environmentId, + entry.projectId, + ); + const reference = { + projectId: entry.projectId, + host: entry.host, + repository: entry.repository, + number: entry.number, + }; + const { actionPending, perform } = usePullRequestActionRunner({ + environmentId: entry.environmentId, + reference, + onSuccess: (action) => onActed({ entry, action }), + resolveMergeMethod: async () => { + const target = { environmentId: entry.environmentId, input: reference }; + const detailAtom = pullRequestEnvironment.detail({ + ...target, + input: { ...reference, allowStale: false }, + }); + appAtomRegistry.refresh(detailAtom); + const detail = await Effect.runPromise( + AtomRegistry.getResult(appAtomRegistry, detailAtom, { suspendOnWaiting: true }), + ); + if ( + detail.state !== "open" || + detail.isDraft || + !detail.capabilities.actions.includes("merge") || + !detail.viewerPermissions.actions.includes("merge") + ) { + throw new Error("This pull request cannot be merged."); + } + if (detail.capabilities.stackActions) { + const stackAtom = pullRequestStackAtom(target); + appAtomRegistry.refresh(stackAtom); + const stack = await Effect.runPromise( + AtomRegistry.getResult(appAtomRegistry, stackAtom, { suspendOnWaiting: true }), + ); + if (stack !== null) throw new Error("Open this pull request to merge its stack."); + } + const allowed = detail.capabilities.mergeMethods.filter( + (method) => detail.mergeCapabilities[method], + ); + if (allowed.length === 0) + throw new Error("No merge method is available for this repository."); + return resolvePullRequestMergeMethod( + allowed, + null, + resolveProjectDefault(), + useUiStateStore.getState().pullRequestMergeMethod, + ); + }, + }); + const actions = + entry.state === "closed" + ? (["reopen"] as const) + : entry.isDraft + ? (["close", "ready"] as const) + : (["close", "merge"] as const); + return ( +
+ {actions.map((action) => { + const label = ACTIONS[action].label; + const Icon = ACTIONS[action].Icon; + return ( + + void perform(action)} + /> + } + > + {actionPending ? : } + {label} + + + {action === "merge" && entry.stack + ? "Open this pull request to merge its stack" + : `${label} immediately`} + + + ); + })} +
+ ); +} + +const ACTIONS = { + close: { label: "Close", Icon: PullRequestGlyph.closed }, + merge: { label: "Merge", Icon: PullRequestGlyph.merged }, + ready: { label: "Ready for review", Icon: PullRequestGlyph.pullRequest }, + reopen: { label: "Reopen", Icon: PullRequestGlyph.reopen }, +} as const; diff --git a/apps/web/src/components/pullRequest/pullRequestChecks.test.tsx b/apps/web/src/components/pullRequest/pullRequestChecks.test.tsx index 48a371a7a9c0..b0e8c69517bc 100644 --- a/apps/web/src/components/pullRequest/pullRequestChecks.test.tsx +++ b/apps/web/src/components/pullRequest/pullRequestChecks.test.tsx @@ -102,6 +102,8 @@ function row(overrides: Partial): ReactNode { showProjectTitle: false, showProvider: false, onSelect: () => {}, + speedMode: false, + onActed: () => {}, }); } diff --git a/apps/web/src/components/pullRequest/usePullRequestActions.ts b/apps/web/src/components/pullRequest/usePullRequestActions.ts index 3ba3b60bd90e..01c2789e8209 100644 --- a/apps/web/src/components/pullRequest/usePullRequestActions.ts +++ b/apps/web/src/components/pullRequest/usePullRequestActions.ts @@ -8,12 +8,23 @@ import { scopeProjectRef } from "@t3tools/client-runtime/environment"; import { squashAtomCommandFailure } from "@t3tools/client-runtime/state/runtime"; import type { EnvironmentId, + ProjectId, PullRequestAction, PullRequestDetail, PullRequestMergeMethod, PullRequestRef, } from "@t3tools/contracts"; -import { useState } from "react"; +import { useCallback, useRef, useState } from "react"; +import { resolveProjectSettings } from "@t3tools/shared/projectSettings"; +import { useClientSettings, useEnvironmentSettings } from "~/hooks/useSettings"; +import { + deriveLogicalProjectKeyFromSettings, + derivePhysicalProjectKey, + selectProjectGroupingSettings, +} from "~/logicalProject"; +import { buildPhysicalToLogicalProjectKeyMap } from "~/sidebarProjectGrouping"; +import { useProjects } from "~/state/entities"; +import { usePrimaryEnvironmentId } from "~/state/environments"; import { type DraftId, useComposerDraftStore } from "~/composerDraftStore"; import { useNewThreadHandler } from "~/hooks/useHandleNewThread"; @@ -25,8 +36,48 @@ import { useAtomCommand } from "~/state/use-atom-command"; import { toastManager } from "../ui/toast"; import { handoffPrompt, handoffReviewComments, readableFailure } from "./pullRequestDetail.logic"; +/** Resolve on demand so hidden quick actions do not rebuild the legacy project grouping. */ +export function usePullRequestDefaultMergeMethodResolver( + environmentId: EnvironmentId, + projectId: ProjectId, +) { + const projectDefault = useEnvironmentSettings( + environmentId, + (settings) => resolveProjectSettings(settings, projectId).settings.pullRequestMergeMethod, + ); + const legacyOverrides = useClientSettings((settings) => settings.pullRequestMergeMethodOverrides); + const grouping = useClientSettings(selectProjectGroupingSettings); + const projects = useProjects(); + const primaryEnvironmentId = usePrimaryEnvironmentId(); + return useCallback(() => { + if (projectDefault != null) return projectDefault; + if (Object.keys(legacyOverrides).length === 0) return undefined; + const project = projects.find( + (candidate) => candidate.environmentId === environmentId && candidate.id === projectId, + ); + if (!project) return undefined; + // Duplicate sidebar rows borrow their logical group key from their siblings. + const key = + buildPhysicalToLogicalProjectKeyMap({ + projects, + settings: grouping, + primaryEnvironmentId, + }).get(derivePhysicalProjectKey(project)) ?? + deriveLogicalProjectKeyFromSettings(project, grouping); + return legacyOverrides[key]; + }, [ + projectDefault, + projects, + environmentId, + projectId, + grouping, + primaryEnvironmentId, + legacyOverrides, + ]); +} + const ACTION_SUCCESS_LABELS: Record = { - merge: "Pull request merged", + merge: "Merge requested", ready: "Marked ready for review", draft: "Converted to draft", close: "Pull request closed", @@ -80,37 +131,41 @@ export function usePullRequestActionRunner({ environmentId, reference, onSuccess, + resolveMergeMethod, }: { environmentId: EnvironmentId; reference: PullRequestRef | null; onSuccess?: (action: PullRequestAction) => void; + /** Small surfaces resolve repository settings on the click, not for every visible row. */ + resolveMergeMethod?: () => Promise; }) { const runAction = useAtomCommand(pullRequestEnvironment.runAction, { reportFailure: false }); const [actionPending, setActionPending] = useState(false); + const pendingRef = useRef(false); const perform = async (action: PullRequestAction, method?: PullRequestMergeMethod) => { - if (actionPending || reference === null) return; + if (pendingRef.current || reference === null) return; + pendingRef.current = true; setActionPending(true); - const result = await runAction({ - environmentId, - input: { ...reference, action, ...(method ? { mergeMethod: method } : {}) }, - }); - setActionPending(false); - if (result._tag === "Failure") { - // The host's own sentence, because it is the only thing that says why. A merge strategy a - // branch policy forbids is refused at completion and nowhere earlier — Azure DevOps - // publishes no per-strategy availability to hide the control with — so "action failed" - // would leave the reader pressing the same button again. - const failure = squashAtomCommandFailure(result); + try { + const mergeMethod = method ?? (action === "merge" ? await resolveMergeMethod?.() : undefined); + const result = await runAction({ + environmentId, + input: { ...reference, action, ...(mergeMethod ? { mergeMethod } : {}) }, + }); + if (result._tag === "Failure") throw squashAtomCommandFailure(result); + toastManager.add({ type: "success", title: ACTION_SUCCESS_LABELS[action] }); + onSuccess?.(action); + } catch (failure) { toastManager.add({ type: "error", title: ACTION_FAILURE_LABELS[action], description: readableFailure(failure, ACTION_FAILURE_HINTS[action]), }); - return; + } finally { + pendingRef.current = false; + setActionPending(false); } - toastManager.add({ type: "success", title: ACTION_SUCCESS_LABELS[action] }); - onSuccess?.(action); }; return { actionPending, perform }; diff --git a/apps/web/src/routes/_chat.pull-requests.tsx b/apps/web/src/routes/_chat.pull-requests.tsx index 8f75fcb66cac..b67f79234813 100644 --- a/apps/web/src/routes/_chat.pull-requests.tsx +++ b/apps/web/src/routes/_chat.pull-requests.tsx @@ -1,5 +1,7 @@ import { RefreshIcon } from "~/components/ui/refresh-icon"; import { Spinner } from "~/components/ui/spinner"; +import { useShortcutModifierState } from "~/shortcutModifierState"; +import type { PullRequestSpeedActionResult } from "~/components/pullRequest/PullRequestSpeedActions"; import { pullRequestHostOf, resolveEnvironmentMachineKind } from "@t3tools/contracts"; import type { EnvironmentId, @@ -342,6 +344,9 @@ export const Route = createFileRoute("/_chat/pull-requests")({ function PullRequestsRouteView() { useEscapeToGoBack(); + const modifiers = useShortcutModifierState(true); + const speedMode = + modifiers.shiftKey && !modifiers.metaKey && !modifiers.ctrlKey && !modifiers.altKey; const search = Route.useSearch(); const sort = search.sort ?? "ready"; const statsPolicy: PullRequestStatsPolicy = @@ -952,6 +957,16 @@ function PullRequestsRouteView() { }; /** The detail panel's own writes, by row, so its failure takes back its own note. */ const detailOverrideTokens = useRef(new Map()); + const speedActionRef = useRef<(result: PullRequestSpeedActionResult) => void>(() => {}); + speedActionRef.current = ({ entry, action }) => { + // Some hosts accept a merge before it completes. Let the next host read declare it merged. + if (action !== "merge") overrideEntry(entry, action); + setDetailRefreshToken((token) => token + 1); + refreshListAndStats(undefined, entry.environmentId); + }; + const onSpeedAction = useCallback((result: PullRequestSpeedActionResult) => { + speedActionRef.current(result); + }, []); // A reload recreates the registry the queries live in, so with nothing held the page would // cold-start into skeletons even though almost every row is unchanged. The last answer for // this set of environments is kept across reloads and hydrated here as the carried rows: they @@ -1395,11 +1410,11 @@ function PullRequestsRouteView() { [statsBatches], ); const statsObserver = useRef(null); - const statsRows = useRef(new Set()); + const statsRows = useRef(new Set()); const statsPending = useRef(true); const statsPolicyRef = useRef(statsPolicy); statsPolicyRef.current = statsPolicy; - const registerStatsRow = useCallback((node: HTMLButtonElement | null) => { + const registerStatsRow = useCallback((node: HTMLDivElement | null) => { if (node === null || typeof IntersectionObserver === "undefined") return; statsRows.current.add(node); statsObserver.current?.observe(node); @@ -1817,6 +1832,8 @@ function PullRequestsRouteView() { selected.number === entry.number } onSelect={selectEntry} + speedMode={speedMode} + onActed={onSpeedAction} /> ); })} diff --git a/apps/web/src/shortcutModifierState.ts b/apps/web/src/shortcutModifierState.ts index 15d6e0a1bcae..20091622158e 100644 --- a/apps/web/src/shortcutModifierState.ts +++ b/apps/web/src/shortcutModifierState.ts @@ -1,4 +1,5 @@ import { useEffect, useRef, useState } from "react"; +import { isEditableFocused } from "./lib/editableFocus"; export interface ShortcutModifierState { metaKey: boolean; @@ -26,7 +27,7 @@ export function areShortcutModifierStatesEqual( ); } -export function useShortcutModifierState(): ShortcutModifierState { +export function useShortcutModifierState(ignoreEditable = false): ShortcutModifierState { const [state, setState] = useState(EMPTY_SHORTCUT_MODIFIER_STATE); const stateRef = useRef(EMPTY_SHORTCUT_MODIFIER_STATE); @@ -39,7 +40,11 @@ export function useShortcutModifierState(): ShortcutModifierState { setState(next); }; const onKeyboardEvent = (event: KeyboardEvent) => { - updateState(shortcutModifierStateAfterKeyboardEvent(stateRef.current, event)); + updateState( + ignoreEditable && isEditableFocused(event.target) + ? EMPTY_SHORTCUT_MODIFIER_STATE + : shortcutModifierStateAfterKeyboardEvent(stateRef.current, event), + ); }; // Dictation tools (Wispr Flow) paste with a synthetic ⌘V whose Meta keyup // never reaches the page, so the tracked state stays "⌘ held" forever and @@ -50,17 +55,22 @@ export function useShortcutModifierState(): ShortcutModifierState { updateState(EMPTY_SHORTCUT_MODIFIER_STATE); }; + const onFocus = (event: FocusEvent) => { + if (ignoreEditable && isEditableFocused(event.target)) onResetEvent(); + }; + window.addEventListener("focusin", onFocus); window.addEventListener("keydown", onKeyboardEvent, true); window.addEventListener("keyup", onKeyboardEvent, true); window.addEventListener("paste", onResetEvent, true); window.addEventListener("blur", onResetEvent); return () => { + window.removeEventListener("focusin", onFocus); window.removeEventListener("keydown", onKeyboardEvent, true); window.removeEventListener("keyup", onKeyboardEvent, true); window.removeEventListener("paste", onResetEvent, true); window.removeEventListener("blur", onResetEvent); }; - }, []); + }, [ignoreEditable]); return state; } From 5a96895a85b43c838da6c89cf21068b99c003d5a Mon Sep 17 00:00:00 2001 From: maria Date: Sun, 4 Oct 2026 07:58:54 -0300 Subject: [PATCH 02/47] fix: expanded tool calls show their output, empty ones don't expand (#15505) Co-authored-by: maria-rcks <254055478+maria-rcks@users.noreply.github.com> --- .../src/features/threads/thread-work-log.tsx | 45 ++++- apps/mobile/src/lib/threadActivity.test.ts | 30 +++ apps/mobile/src/lib/threadActivity.ts | 49 +++-- apps/mobile/src/state/queries.ts | 23 +++ apps/server/scripts/acp-mock-agent.ts | 64 +++++++ apps/server/src/auth/RpcAuthorization.ts | 1 + .../Adapters/AcpAdapterV2.test.ts | 35 ++++ .../orchestration-v2/Adapters/AcpAdapterV2.ts | 81 +++++++- .../Adapters/ClaudeAdapterV2.test.ts | 83 ++++++++ .../Adapters/ClaudeAdapterV2.ts | 46 ++++- .../Adapters/CursorAdapterV2.test.ts | 34 +++- .../Adapters/CursorAdapterV2.ts | 19 +- .../Adapters/OpenCodeAdapterV2.test.ts | 28 ++- .../Adapters/OpenCodeToolItems.ts | 29 ++- .../Adapters/PiAdapterV2.test.ts | 96 ++++++++++ .../orchestration-v2/Adapters/PiAdapterV2.ts | 8 + .../src/orchestration-v2/Orchestrator.ts | 14 ++ .../orchestration-v2/ProjectionStore.test.ts | 8 + .../src/orchestration-v2/ProjectionStore.ts | 25 +++ .../ProviderTurnControlService.test.ts | 1 + .../ThreadManagementService.ts | 16 ++ .../orchestration-v2/WireProjection.test.ts | 30 ++- .../src/orchestration-v2/WireProjection.ts | 83 +++++++- .../src/relay/AgentAwarenessRelay.test.ts | 1 + apps/server/src/ws.ts | 15 ++ .../src/components/chat/MessagesTimeline.tsx | 45 ++++- .../src/components/chat/V2ItemInspector.tsx | 127 ++++++++++++- apps/web/src/state/queries.ts | 21 ++ packages/client-runtime/package.json | 4 + .../client-runtime/src/state/orchestration.ts | 7 + .../client-runtime/src/work-log/itemDetail.ts | 179 ++++++++++++++++++ .../src/work-log/presentation.ts | 7 +- packages/contracts/src/orchestrationV2.ts | 27 +++ packages/contracts/src/rpc.ts | 7 + 34 files changed, 1225 insertions(+), 63 deletions(-) create mode 100644 packages/client-runtime/src/work-log/itemDetail.ts diff --git a/apps/mobile/src/features/threads/thread-work-log.tsx b/apps/mobile/src/features/threads/thread-work-log.tsx index 57287c81eda1..3fed0c1a7154 100644 --- a/apps/mobile/src/features/threads/thread-work-log.tsx +++ b/apps/mobile/src/features/threads/thread-work-log.tsx @@ -60,9 +60,12 @@ import { cn } from "../../lib/cn"; import { THREAD_WORK_ROW_MIN_HEIGHT, type deriveThreadWorkLogSizing } from "../../lib/layout"; import { type AgentSpawnSummary, + formatItemFullDetail, type ThreadFeedActivity, workEntryRowLabel, } from "../../lib/threadActivity"; +import { turnItemOutputText } from "@t3tools/client-runtime/work-log/item-detail"; +import { useTurnItemDetail } from "../../state/queries"; import { resolveThreadWorkGroupInitialScroll, shouldFollowThreadWorkGroupAppend, @@ -70,6 +73,7 @@ import { } from "./thread-feed-live-follow"; import { resolveWorkEntryToolPresentation, + toolGroupAction, type ToolGroupSummaryKind, workEntryViewedImagePath, } from "@t3tools/client-runtime/work-log/presentation"; @@ -836,6 +840,11 @@ const ThreadWorkLogRow = memo(function ThreadWorkLogRow( ) { const { row, expanded } = props; const navigation = useNavigation(); + const fetchedDetail = useTurnItemDetail( + expanded && row.fetchesDetail + ? { environmentId: props.environmentId, row: row.projectedItem } + : null, + ); const failureItem = row.projectedItem.item; if (failureItem.type === "error" && failureItem.status === "failed") { const warning = failureItem.failure.class === "usage_limit"; @@ -917,7 +926,26 @@ const ThreadWorkLogRow = memo(function ThreadWorkLogRow( : undefined; const canExpand = row.canExpand && notifiedSubagentThreadId === undefined; const reasoning = row.projectedItem.item.type === "reasoning" ? row.projectedItem.item : null; - const fullDetail = expanded && !reasoning ? row.getFullDetail() : null; + const fetchedItem = fetchedDetail.data?.item ?? null; + // Reads keep their path list; the fetched file contents show as output. + const isRead = toolGroupAction(row.workEntry) === "read"; + const fullDetail = + expanded && !reasoning + ? fetchedItem && !isRead + ? formatItemFullDetail(row.projectedItem, fetchedItem) + : row.getFullDetail() + : null; + const fetchedOutput = !expanded + ? null + : fetchedItem + ? (turnItemOutputText(fetchedItem) ?? "No output.") + : fetchedDetail.error + ? `Couldn't load output: ${fetchedDetail.error}` + : row.fetchesDetail + ? fetchedDetail.data + ? "Output is no longer available." + : "Loading output…" + : null; const viewedImagePath = workEntryViewedImagePath(row.workEntry); const toolPresentation = resolveWorkEntryToolPresentation(row.workEntry); const previewText = workEntryRowLabel(row.workEntry); @@ -1060,7 +1088,12 @@ const ThreadWorkLogRow = memo(function ThreadWorkLogRow( - {expanded && (reasoning || fullDetail || viewedImagePath || row.workEntry.questionAnswer) ? ( + {expanded && + (reasoning || + fullDetail || + fetchedOutput || + viewedImagePath || + row.workEntry.questionAnswer) ? ( )} + {fetchedOutput ? ( + + {fetchedOutput} + + ) : null} ) : null} diff --git a/apps/mobile/src/lib/threadActivity.test.ts b/apps/mobile/src/lib/threadActivity.test.ts index 16fd407f537b..5471f24704a9 100644 --- a/apps/mobile/src/lib/threadActivity.test.ts +++ b/apps/mobile/src/lib/threadActivity.test.ts @@ -271,6 +271,35 @@ describe("buildThreadFeed", () => { expect(items[0]).toMatchObject({ output: rawOutput }); }); + it("expands tool rows only when they have detail or withheld output", () => { + const items: OrchestrationV2TurnItem[] = [ + { ...command(), input: "", outputOmitted: true }, + { + ...base("dynamic-empty", "2026-06-20T00:00:03.000Z", 2), + type: "dynamic_tool", + toolName: "example", + input: {}, + }, + { + ...base("read-omitted", "2026-06-20T00:00:04.000Z", 3), + type: "dynamic_tool", + toolName: "Read", + input: { path: "src/env.ts" }, + outputOmitted: true, + }, + ]; + const activities = buildThreadFeed(items.map((item, index) => projected(item, index))).flatMap( + (entry) => (entry.type === "activity-group" ? entry.activities : []), + ); + expect( + activities.map(({ canExpand, fetchesDetail }) => ({ canExpand, fetchesDetail })), + ).toEqual([ + { canExpand: true, fetchesDetail: true }, + { canExpand: false, fetchesDetail: false }, + { canExpand: true, fetchesDetail: true }, + ]); + }); + it("recognizes automation attribution after projecting a user message", () => { const feed = buildThreadFeed([ projected( @@ -1236,6 +1265,7 @@ describe("buildThreadFeed", () => { summary: `Tool ${id}`, detail: null, canExpand: false, + fetchesDetail: false, getFullDetail: () => null, getCopyText: () => id, icon: "command", diff --git a/apps/mobile/src/lib/threadActivity.ts b/apps/mobile/src/lib/threadActivity.ts index 3a77324a7ac8..a77491795704 100644 --- a/apps/mobile/src/lib/threadActivity.ts +++ b/apps/mobile/src/lib/threadActivity.ts @@ -6,6 +6,10 @@ import type { import { turnItemIsWorkspacePreparation } from "@t3tools/client-runtime/state/turn-item-presentation"; import { formatSubagentDisplayTitle } from "@t3tools/client-runtime/state/subagent-display"; import { extractToolActivityPresentation } from "@t3tools/client-runtime/work-log/tool-presentation"; +import { + turnItemHasDetail, + turnItemNeedsDetailFetch, +} from "@t3tools/client-runtime/work-log/item-detail"; import { commandDisplayText, commandProgramName, @@ -74,6 +78,8 @@ export interface ThreadFeedActivity { readonly summary: string; readonly detail: string | null; readonly canExpand: boolean; + /** Expanding fetches the withheld input and output with getTurnItem. */ + readonly fetchesDetail: boolean; readonly getFullDetail: () => string | null; readonly getCopyText: () => string; readonly icon: @@ -721,6 +727,23 @@ function toWorkLogEntry( } } +/** Expanded detail for a row, from its wire item or the full item from getTurnItem. */ +export function formatItemFullDetail( + row: OrchestrationV2ProjectedTurnItem, + item: OrchestrationV2TurnItem, +): string { + return JSON.stringify( + { + visibility: row.visibility, + sourceThreadId: row.sourceThreadId, + sourceItemId: row.sourceItemId, + item: toolItemForDisplay(item), + }, + null, + 2, + ); +} + function toFeedActivity( row: OrchestrationV2ProjectedTurnItem, attemptId: RunAttemptId | null, @@ -735,21 +758,9 @@ function toFeedActivity( item.type === "dynamic_tool" && toolGroupAction(workEntry) === "read" ? collectToolFilePaths(item) : null; - const getFullDetail = memoizeValue(() => { - if (readPaths) { - return readPaths.join("\n") || null; - } - return JSON.stringify( - { - visibility: row.visibility, - sourceThreadId: row.sourceThreadId, - sourceItemId: row.sourceItemId, - item: toolItemForDisplay(item), - }, - null, - 2, - ); - }); + const getFullDetail = memoizeValue(() => + readPaths ? readPaths.join("\n") || null : formatItemFullDetail(row, item), + ); const getCopyText = memoizeValue(() => [summary, detail, getFullDetail()] .filter( @@ -765,7 +776,13 @@ function toFeedActivity( attemptId, summary, detail, - canExpand: !(item.type === "error" && item.status === "failed") && (readPaths?.length ?? 1) > 0, + canExpand: + !(item.type === "error" && item.status === "failed") && + (readPaths + ? readPaths.length > 0 || turnItemNeedsDetailFetch(item) + : turnItemHasDetail(item) || workEntry.questionAnswer !== undefined), + // Read rows show their paths, then the fetched file contents. + fetchesDetail: turnItemNeedsDetailFetch(item), getFullDetail, getCopyText, icon: workEntry.toolSurface ?? itemIcon(item), diff --git a/apps/mobile/src/state/queries.ts b/apps/mobile/src/state/queries.ts index d9cce63b65ac..db8269ff130b 100644 --- a/apps/mobile/src/state/queries.ts +++ b/apps/mobile/src/state/queries.ts @@ -2,6 +2,7 @@ import { filterComposerPullRequestMatches } from "@t3tools/shared/composerPullRe import type { VcsRefTarget } from "@t3tools/client-runtime/state/vcs"; import type { EnvironmentId, + OrchestrationV2ProjectedTurnItem, ProjectId, ThreadId, VcsListRefsResult, @@ -14,6 +15,7 @@ import { } from "@t3tools/client-runtime/state/thread-search"; import { useAtomValue } from "@effect/atom-react"; import * as Cause from "effect/Cause"; +import { turnItemDetailRevision } from "@t3tools/client-runtime/work-log/item-detail"; import * as Option from "effect/Option"; import { AsyncResult, Atom } from "effect/unstable/reactivity"; import { useCallback, useEffect, useMemo, useState } from "react"; @@ -351,3 +353,24 @@ export function useCheckpointDiff(target: CheckpointDiffTarget) { ); return targets.fullThread === null ? turn : fullThread; } + +/** Full input and output for one tool row; pass null to skip fetching. */ +export function useTurnItemDetail( + target: { + readonly environmentId: EnvironmentId; + readonly row: OrchestrationV2ProjectedTurnItem; + } | null, +) { + return useEnvironmentQuery( + target === null + ? null + : orchestrationEnvironment.turnItem({ + environmentId: target.environmentId, + input: { + threadId: target.row.sourceThreadId, + itemId: target.row.sourceItemId, + revision: turnItemDetailRevision(target.row.item), + }, + }), + ); +} diff --git a/apps/server/scripts/acp-mock-agent.ts b/apps/server/scripts/acp-mock-agent.ts index 6e426b52c4e0..5548bebac84c 100644 --- a/apps/server/scripts/acp-mock-agent.ts +++ b/apps/server/scripts/acp-mock-agent.ts @@ -1077,6 +1077,70 @@ const program = Effect.gen(function* () { status: "completed", rawInput: { query: "TODO", path: "apps/web" }, }, + // Grok backend searches: the query only arrives in the completed rawOutput. + { + sessionUpdate: "tool_call_update", + toolCallId: "grok-x-search", + title: "X search:", + kind: "search", + status: "in_progress", + rawInput: { variant: "XSearch", backend: true }, + }, + { + sessionUpdate: "tool_call_update", + toolCallId: "grok-x-search", + title: "X search:", + status: "completed", + rawOutput: { + call_id: "xs_call-1", + input: '{"query":"conversation_id:42","limit":"10","mode":"Latest"}', + name: "x_keyword_search", + id: "grok-x-search", + }, + }, + { + sessionUpdate: "tool_call_update", + toolCallId: "grok-web-search", + title: "Web search:", + kind: "search", + status: "completed", + rawInput: { variant: "WebSearch", backend: true }, + rawOutput: { + action: { + type: "search", + query: "t3 code", + sources: [ + { type: "url", url: "https://t3.codes" }, + { type: "url", url: "https://t3.codes" }, + { type: "url", url: "https://github.com/pingdotgg/t3code" }, + ], + }, + id: "grok-web-search", + status: "completed", + }, + }, + { + sessionUpdate: "tool_call_update", + toolCallId: "grok-web-fetch", + title: "Fetch: https://t3.codes", + kind: "fetch", + status: "completed", + rawInput: { variant: "WebFetch", url: "https://t3.codes" }, + rawOutput: { + type: "WebFetch", + Content: { url: "https://t3.codes", content: "T3 Code page" }, + }, + content: [{ type: "content", content: { type: "text", text: "T3 Code page" } }], + }, + { + sessionUpdate: "tool_call_update", + toolCallId: "antigravity-shell", + title: "run_command", + kind: "execute", + status: "completed", + rawInput: { command: "cat probe.txt" }, + rawOutput: { commandLine: "cat probe.txt", exitCode: 0, combinedOutput: "after\n" }, + }, { sessionUpdate: "compaction_update", compactionId: "compact-1", diff --git a/apps/server/src/auth/RpcAuthorization.ts b/apps/server/src/auth/RpcAuthorization.ts index bba03dd2d600..54d8e2a906bf 100644 --- a/apps/server/src/auth/RpcAuthorization.ts +++ b/apps/server/src/auth/RpcAuthorization.ts @@ -33,6 +33,7 @@ export const RPC_REQUIRED_SCOPES = { [ORCHESTRATION_V2_WS_METHODS.searchThreads]: AuthOrchestrationReadScope, [ORCHESTRATION_V2_WS_METHODS.getArchivedShellSnapshot]: AuthOrchestrationReadScope, [ORCHESTRATION_V2_WS_METHODS.getThreadProjection]: AuthOrchestrationReadScope, + [ORCHESTRATION_V2_WS_METHODS.getTurnItem]: AuthOrchestrationReadScope, [ORCHESTRATION_V2_WS_METHODS.launchThread]: AuthOrchestrationOperateScope, [ORCHESTRATION_V2_WS_METHODS.subscribeArchivedShell]: AuthOrchestrationReadScope, [ORCHESTRATION_V2_WS_METHODS.subscribeShell]: AuthOrchestrationReadScope, diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts index b6cb0be45acf..8f981fb3384d 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.test.ts @@ -1324,6 +1324,12 @@ describe("AcpAdapterV2", () => { ), { input: "printf proof", output: "proof" }, ); + assert.deepInclude( + items.flatMap((item) => + item.type === "command_execution" ? [{ input: item.input, output: item.output }] : [], + ), + { input: "cat probe.txt", output: "after\n" }, + ); assert.isTrue( items.some((item) => item.title === "Action required" && item.status === "waiting"), ); @@ -1345,6 +1351,35 @@ describe("AcpAdapterV2", () => { search?.type === "file_search" ? { title: search.title, pattern: search.pattern } : null, { title: "Searched TODO in web", pattern: "apps/web" }, ); + const webItem = (nativeId: string, status: string) => { + const item = items.findLast( + (candidate) => + candidate.type === "web_search" && + candidate.status === status && + candidate.nativeItemRef?.nativeId?.endsWith(nativeId) === true, + ); + return item?.type === "web_search" + ? { title: item.title, patterns: item.patterns, results: item.results } + : null; + }; + assert.deepEqual(webItem("grok-x-search", "running"), { + title: "X search", + patterns: undefined, + results: undefined, + }); + assert.deepEqual(webItem("grok-x-search", "completed"), { + title: "X search: conversation_id:42", + patterns: ["conversation_id:42"], + results: undefined, + }); + assert.deepEqual(webItem("grok-web-search", "completed"), { + title: "Web search: t3 code", + patterns: ["t3 code"], + results: [{ url: "https://t3.codes" }, { url: "https://github.com/pingdotgg/t3code" }], + }); + assert.deepEqual(webItem("grok-web-fetch", "completed")?.results, [ + { url: "https://t3.codes", snippet: "T3 Code page" }, + ]); const completedCompaction = items.find( (item) => item.type === "compaction" && diff --git a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts index bf01d9cb2d7d..aac767f8c886 100644 --- a/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts +++ b/apps/server/src/orchestration-v2/Adapters/AcpAdapterV2.ts @@ -19,6 +19,7 @@ import { type OrchestrationV2Subagent, type OrchestrationV2TurnItem, type OrchestrationV2UserInputQuestion, + type OrchestrationV2WebSearchResult, type ProviderApprovalDecision, type ProviderApprovalOption, type ProviderInstanceId, @@ -846,11 +847,14 @@ function textFromUnknown(value: unknown): string | undefined { return undefined; } // Prefer prompt-facing Grok fields before nested envelopes. + // Antigravity reports shell output as combinedOutput. for (const key of [ "output_for_prompt", "stdout", "stderr", "output", + "combinedOutput", + "combined_output", "content", "text", "message", @@ -989,6 +993,46 @@ function pathFromToolCall(toolCall: AcpToolCallState): string | undefined { return undefined; } +/** + * Grok runs X and web searches server-side as `search` tools whose rawInput is + * only `{ variant: "XSearch" | "WebSearch", backend: true }`. The query arrives + * with completion: web searches report `action: { query, sources }`, X searches + * the backend call `{ name, input }` with JSON-encoded arguments. + */ +function acpBackendWebSearch( + rawInput: Record | undefined, + rawOutput: Record | undefined, +): + | { readonly query: string | undefined; readonly results: OrchestrationV2WebSearchResult[] } + | undefined { + const variant = typeof rawInput?.variant === "string" ? rawInput.variant.toLowerCase() : ""; + const action = unknownRecord(rawOutput?.action); + if (variant !== "xsearch" && variant !== "websearch" && action?.type !== "search") { + return undefined; + } + let args: Record | undefined; + if (typeof rawOutput?.input === "string") { + try { + args = unknownRecord(JSON.parse(rawOutput.input)); + } catch { + args = undefined; + } + } + const argsText = Object.entries(args ?? {}) + .filter(([, value]) => typeof value === "string" || typeof value === "number") + .map(([key, value]) => `${key}: ${value}`) + .join(", "); + const query = [action?.query, args?.query, argsText] + .find((value): value is string => typeof value === "string" && value.trim().length > 0) + ?.trim(); + const urls = new Set(); + for (const source of Array.isArray(action?.sources) ? action.sources : []) { + const url = unknownRecord(source)?.url; + if (typeof url === "string" && url.trim().length > 0) urls.add(url.trim()); + } + return { query, results: [...urls].map((url) => ({ url })) }; +} + function providerRequestKind(kind: string | "unknown"): ProviderRequestKind { switch (kind) { case "execute": @@ -3297,7 +3341,30 @@ export function makeAcpAdapterV2( ...(rawOutput === undefined ? {} : { output: rawOutput }), }; break; - case "search": + case "search": { + const backendSearch = acpBackendWebSearch(rawInputRecord, rawOutputRecord); + if (backendSearch !== undefined) { + // Grok titles these "X search:" / "Web search:" awaiting the query. + const label = nonEmptyText(toolCall.data.title, title ?? "Web search").replace( + /:\s*$/u, + "", + ); + turnItem = { + ...base, + title: + backendSearch.query === undefined + ? label + : `${label}: ${backendSearch.query}`, + type: "web_search", + ...(backendSearch.query === undefined + ? {} + : { patterns: [backendSearch.query] }), + ...(backendSearch.results.length === 0 + ? {} + : { results: backendSearch.results }), + }; + break; + } turnItem = { ...base, title: @@ -3322,6 +3389,7 @@ export function makeAcpAdapterV2( }), }; break; + } case "execute": { const exitCode = acpProjectedCommandExitCode(status, rawOutput); turnItem = { @@ -3345,7 +3413,11 @@ export function makeAcpAdapterV2( ...(diffText === undefined ? {} : { diffStr: diffText }), }; break; - case "fetch": + case "fetch": { + // Grok nests the page under rawOutput.Content, which textFromUnknown + // cannot read; the (bounded) content blocks carry the same text. + const snippet = + textFromUnknown(toolCall.data.content) ?? textFromUnknown(rawOutput); turnItem = { ...base, type: "web_search", @@ -3356,14 +3428,13 @@ export function makeAcpAdapterV2( results: [ { url: path, - ...(textFromUnknown(rawOutput) === undefined - ? {} - : { snippet: textFromUnknown(rawOutput) }), + ...(snippet === undefined ? {} : { snippet }), }, ], }), }; break; + } default: if (projectAsCommandExecution) { const exitCode = acpProjectedCommandExitCode(status, rawOutput); diff --git a/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts index b837575c1faf..2e237bf3bb4c 100644 --- a/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.test.ts @@ -4325,6 +4325,89 @@ describe("ClaudeAdapterV2 background wake turns", () => { ), ); + it.effect("stores a Bash result's stdout and stderr as command output", () => + Effect.scoped( + Effect.gen(function* () { + const harness = yield* makeWakeHarness; + const now = yield* DateTime.now; + const attemptId = RunAttemptId.make("attempt-claude-bash-output"); + const bashToolUseId = "toolu_01BashOutput"; + + yield* harness.runtime.startTurn( + makeClaudeTestTurnInput({ + threadId: harness.threadId, + providerThread: harness.providerThread, + now, + attemptId, + text: "Run it.", + attachments: [], + }), + ); + yield* Queue.offer( + harness.sdkMessages, + claudeSdkFrame({ + type: "assistant", + message: { + model: "claude-sonnet-4-6", + id: "msg_bash_output", + type: "message", + role: "assistant", + content: [ + { + type: "tool_use", + id: bashToolUseId, + name: "Bash", + input: { command: "git status" }, + }, + ], + }, + parent_tool_use_id: null, + uuid: "00000000-0000-4000-8000-000000000790", + session_id: WAKE_NATIVE_SESSION, + }), + ); + yield* Queue.offer( + harness.sdkMessages, + claudeSdkFrame({ + type: "user", + message: { + role: "user", + content: [ + { type: "tool_result", tool_use_id: bashToolUseId, content: "On branch main" }, + ], + }, + parent_tool_use_id: null, + uuid: "00000000-0000-4000-8000-000000000791", + session_id: WAKE_NATIVE_SESSION, + tool_use_result: { + stdout: "On branch main", + stderr: "warning: dirty", + interrupted: false, + isImage: false, + }, + }), + ); + yield* Queue.offer( + harness.sdkMessages, + makeResultFrame({ uuid: "00000000-0000-4000-8000-000000000792", result: "Done." }), + ); + yield* awaitUntil(() => harness.terminalEvents().length === 1, "turn terminal"); + + const bash = harness.events.findLast( + (event) => + event.type === "turn_item.updated" && + event.turnItem.nativeItemRef?.nativeId === bashToolUseId, + ); + assert.equal( + bash?.type === "turn_item.updated" && bash.turnItem.type === "command_execution" + ? bash.turnItem.output + : undefined, + "On branch main\nwarning: dirty", + ); + }).pipe(Effect.provide(Layer.merge(IdAllocator.layer, NodeServices.layer))), + ), + ); + it.effect("answers an approval a held wake turn raises without waiting for the echo", () => Effect.scoped( Effect.gen(function* () { diff --git a/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts b/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts index 0aca36abe77c..ba0cfbd8fbd5 100644 --- a/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts +++ b/apps/server/src/orchestration-v2/Adapters/ClaudeAdapterV2.ts @@ -1864,6 +1864,28 @@ function claudeNativeToolOutputText(output: ClaudeNativeToolOutput): string { return typeof value === "string" ? value : value === undefined ? "" : jsonStringifyForTool(value); } +/** + * Bash results arrive as `{ stdout, stderr, interrupted, ... }`; keep only the + * text. A background run has empty streams, so keep its acknowledgement instead. + */ +function claudeCommandOutputText(output: ClaudeNativeToolOutput): string { + const value = claudeNativeToolOutputValue(output); + if (typeof value === "object" && value !== null) { + const stdout = Reflect.get(value, "stdout"); + const stderr = Reflect.get(value, "stderr"); + if (typeof stdout === "string" || typeof stderr === "string") { + const text = [stdout, stderr] + .filter((part): part is string => typeof part === "string" && part.trim().length > 0) + .join("\n"); + if (text.length > 0) return text; + return output.type === "structured_tool_use_result" && output.fallbackValue !== undefined + ? claudeSubagentResultText({ type: "content_block", value: output.fallbackValue }) + : ""; + } + } + return claudeNativeToolOutputText(output); +} + function claudeSubagentResultText(output: ClaudeNativeToolOutput): string { const value = claudeNativeToolOutputValue(output); const content = Array.isArray(value) @@ -1904,6 +1926,8 @@ function isClaudeSubagentAsyncLaunchAck(output: ClaudeNativeToolOutput): boolean return claudeSubagentResultText(output).startsWith("Async agent launched successfully."); } +const WEB_FETCH_SNIPPET_MAX_CHARS = 8_000; + function webSearchPatternsFromClaudeTool(input: { readonly toolInput: ClaudeNativeToolInput; readonly output: ClaudeNativeToolOutput; @@ -3807,8 +3831,12 @@ export function makeClaudeAdapterV2( output: input.output, }); const webSearchResults = webSearchResultsFromClaudeOutput(input.output); + const webFetchUrl = firstStringInputField(input.toolInput, ["url"])?.trim(); const outputValue = claudeNativeToolOutputValue(input.output); - const outputText = claudeNativeToolOutputText(input.output); + const outputText = + itemType === "command_execution" + ? claudeCommandOutputText(input.output) + : claudeNativeToolOutputText(input.output); const turnItem: OrchestrationV2TurnItem = itemType === "command_execution" ? { @@ -3831,7 +3859,21 @@ export function makeClaudeAdapterV2( ...(webSearchPatterns.length === 0 ? {} : { patterns: [...webSearchPatterns] }), - ...(webSearchResults.length === 0 ? {} : { results: [...webSearchResults] }), + ...(webSearchResults.length > 0 + ? { results: [...webSearchResults] } + : input.classification.normalizedName === "webfetch" && + outputText.trim().length > 0 + ? { + // WebFetch returns page text, not search hits. Keep a + // bounded preview so the row has something to show. + results: [ + { + ...(webFetchUrl === undefined ? {} : { url: webFetchUrl }), + snippet: outputText.slice(0, WEB_FETCH_SNIPPET_MAX_CHARS), + }, + ], + } + : {}), } : { ...itemBase, diff --git a/apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.test.ts index 3a172baa67c8..1756d6322b4c 100644 --- a/apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.test.ts @@ -430,6 +430,26 @@ describe("CursorAdapterV2", () => { result: { status: "error", error: "ENOENT" }, }, }, + { + type: "tool-call-completed", + modelCallId: "native-model-call", + callId: "grep-failed", + toolCall: { + type: "grep", + args: { pattern: "TODO", path: "src" }, + result: { status: "error", error: "search failed" }, + }, + }, + { + type: "tool-call-completed", + modelCallId: "native-model-call", + callId: "glob-failed", + toolCall: { + type: "glob", + args: { globPattern: "*.ts" }, + result: { status: "error", error: "search failed" }, + }, + }, { type: "tool-call-completed", modelCallId: "native-model-call", @@ -648,7 +668,17 @@ describe("CursorAdapterV2", () => { { pattern: path.join(workspace, "missing"), status: "failed", - results: undefined, + results: [{ fileName: path.join(workspace, "missing"), preview: "ENOENT" }], + }, + { + pattern: "TODO", + status: "failed", + results: [{ fileName: "src", preview: "search failed" }], + }, + { + pattern: "*.ts", + status: "failed", + results: [{ fileName: ".", preview: "search failed" }], }, { pattern: "src/a.ts, src/b.ts", @@ -667,7 +697,7 @@ describe("CursorAdapterV2", () => { { pattern: "src/a.ts", status: "failed", - results: undefined, + results: [{ fileName: "src/a.ts", preview: "lint failed" }], }, ], ); diff --git a/apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.ts b/apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.ts index bb9de20da64f..dd77ac688370 100644 --- a/apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.ts +++ b/apps/server/src/orchestration-v2/Adapters/CursorAdapterV2.ts @@ -1227,7 +1227,10 @@ export function makeCursorAdapterV2( toolCall.result?.status === "success" && toolCall.result.value.diffString !== undefined ? { diffStr: toolCall.result.value.diffString } - : {}), + : // A failed change keeps its error where the diff would be. + toolCall.result?.status === "error" && outputText.trim().length > 0 + ? { diffStr: outputText } + : {}), ...(toolCall.type === "write" ? { newStr: toolCall.args.fileText } : {}), }; break; @@ -1248,8 +1251,20 @@ export function makeCursorAdapterV2( case "ls": case "readLints": case "semSearch": { - const results = cursorToolSearchResults(toolCall, path); const pattern = cursorToolSearchPattern(toolCall); + const searchPath = + toolCall.type === "grep" + ? toolCall.args.path + : toolCall.type === "glob" + ? toolCall.args.targetDirectory + : toolCall.type === "semSearch" + ? toolCall.args.targetDirectories?.join(", ") + : pattern; + // A failed search keeps its error as one row under the searched path. + const results = + toolCall.result?.status === "error" && outputText.trim().length > 0 + ? [{ fileName: searchPath?.trim() || ".", preview: outputText }] + : cursorToolSearchResults(toolCall, path); turnItem = { ...base, title: diff --git a/apps/server/src/orchestration-v2/Adapters/OpenCodeAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/OpenCodeAdapterV2.test.ts index eca4aa9115a4..a53c9b20bf41 100644 --- a/apps/server/src/orchestration-v2/Adapters/OpenCodeAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/OpenCodeAdapterV2.test.ts @@ -833,10 +833,12 @@ describe("OpenCodeAdapterV2", () => { Stream.runCollect, Effect.forkScoped, ); - for (const [tool, input] of [ - ["read", { filePath: "src/env.ts" }], - ["grep", { pattern: "TODO", path: "apps/web" }], - ["websearch", { query: "OpenCode documentation" }], + for (const [tool, input, output] of [ + ["read", { filePath: "src/env.ts" }, "---\nfile body"], + ["grep", { pattern: "TODO", path: "apps/web" }, "---\nfile body"], + ["websearch", { query: "OpenCode documentation" }, "---\nfile body"], + ["glob", { pattern: "missing", path: "apps/web" }, ""], + ["codesearch", {}, " \n\t"], ] as const) { yield* Effect.promise(() => nativeEvents.push({ @@ -853,7 +855,7 @@ describe("OpenCodeAdapterV2", () => { state: { status: "completed", input, - output: "---\nfile body", + output, title: tool, metadata: {}, time: { start: 1, end: 2 }, @@ -877,10 +879,26 @@ describe("OpenCodeAdapterV2", () => { const grep = items.find((item) => item.type === "file_search"); assert.equal(grep?.title, "Searched TODO in web"); assert.equal(grep?.type === "file_search" ? grep.pattern : null, "TODO"); + assert.deepEqual(grep?.type === "file_search" ? grep.results : null, [ + { fileName: "apps/web", preview: "---\nfile body" }, + ]); const webSearch = items.find((item) => item.type === "web_search"); assert.deepEqual(webSearch?.type === "web_search" ? webSearch.patterns : null, [ "OpenCode documentation", ]); + assert.deepEqual(webSearch?.type === "web_search" ? webSearch.results : null, [ + { snippet: "---\nfile body" }, + ]); + const emptyFileSearch = items.find( + (item) => item.type === "file_search" && item.pattern === "missing", + ); + assert.ok(emptyFileSearch?.type === "file_search"); + assert.equal(emptyFileSearch.results, undefined); + const emptyWebSearch = items.find( + (item) => item.type === "web_search" && item.patterns === undefined, + ); + assert.ok(emptyWebSearch?.type === "web_search"); + assert.equal(emptyWebSearch.results, undefined); }).pipe(Effect.provide(IdAllocator.layer), Effect.scoped), ); diff --git a/apps/server/src/orchestration-v2/Adapters/OpenCodeToolItems.ts b/apps/server/src/orchestration-v2/Adapters/OpenCodeToolItems.ts index d4d8c86df654..1b01b2b8007c 100644 --- a/apps/server/src/orchestration-v2/Adapters/OpenCodeToolItems.ts +++ b/apps/server/src/orchestration-v2/Adapters/OpenCodeToolItems.ts @@ -6,6 +6,9 @@ import type { OrchestrationV2TurnItem } from "@t3tools/contracts"; import { formatReadToolLabel, formatSearchToolLabel } from "@t3tools/shared/toolActivity"; +// Search results stay on the timeline wire, so keep their text a preview. +const SEARCH_PREVIEW_MAX_CHARS = 8_000; + type ToolItemBase = Omit< Extract, "type" | "toolName" | "input" | "output" @@ -96,7 +99,10 @@ export function openCodeToolTurnItem( case "file_change": { const oldStr = recordString(input, "oldString", "oldText"); const newStr = recordString(input, "newString", "content", "newText"); - const diffStr = recordString(tool.completedMetadata, "diff", "patch"); + // A failed edit has no diff; keep its error where the diff would be. + const diffStr = + recordString(tool.completedMetadata, "diff", "patch") ?? + (base.status === "failed" && output?.trim() ? output : undefined); return { ...base, type: "file_change", @@ -108,6 +114,9 @@ export function openCodeToolTurnItem( } case "file_search": { const pattern = recordString(input, "pattern", "query", "path", "filePath"); + // OpenCode reports matches as plain text, so keep it as one result row + // under the searched path, like the ACP search projection. + const searchRoot = (recordString(input, "path", "filePath") ?? pattern)?.trim(); return { ...base, title: @@ -115,14 +124,32 @@ export function openCodeToolTurnItem( base.title, type: "file_search", ...(pattern === undefined ? {} : { pattern }), + ...(!output?.trim() || searchRoot === undefined + ? {} + : { + results: [ + { fileName: searchRoot, preview: output.slice(0, SEARCH_PREVIEW_MAX_CHARS) }, + ], + }), }; } case "web_search": { const pattern = recordString(input, "query", "url", "pattern"); + const url = recordString(input, "url")?.trim(); return { ...base, type: "web_search", ...(pattern === undefined ? {} : { patterns: [pattern] }), + ...(!output?.trim() + ? {} + : { + results: [ + { + ...(url === undefined ? {} : { url }), + snippet: output.slice(0, SEARCH_PREVIEW_MAX_CHARS), + }, + ], + }), }; } case "dynamic_tool": { diff --git a/apps/server/src/orchestration-v2/Adapters/PiAdapterV2.test.ts b/apps/server/src/orchestration-v2/Adapters/PiAdapterV2.test.ts index fb912714f12f..df2bade7fc06 100644 --- a/apps/server/src/orchestration-v2/Adapters/PiAdapterV2.test.ts +++ b/apps/server/src/orchestration-v2/Adapters/PiAdapterV2.test.ts @@ -1155,6 +1155,102 @@ describe("PiAdapterV2", () => { }).pipe(Effect.scoped, Effect.provide(testLayer)), ); + it.effect("persists edit patches and write content on file change items", () => + Effect.gen(function* () { + const fake = yield* makeFakePi; + const { runtime, takeEvent } = yield* openRuntime(fake); + const providerThread = yield* runtime.ensureThread({ + threadId: THREAD_ID, + modelSelection: modelSelection("default"), + runtimePolicy, + }); + yield* startTurn(runtime, providerThread); + yield* fake.takeRequest("prompt"); + yield* fake.emit({ type: "agent_start" }); + const patch = "--- a.ts\n+++ a.ts\n@@ -1 +1 @@\n-old\n+new\n"; + yield* fake.emit({ + type: "tool_execution_start", + toolCallId: "call_edit", + toolName: "edit", + args: { path: "a.ts", edits: [{ oldText: "old", newText: "new" }] }, + }); + yield* fake.emit({ + type: "tool_execution_end", + toolCallId: "call_edit", + toolName: "edit", + isError: false, + result: { + content: [{ type: "text", text: "Successfully replaced 1 block(s) in a.ts." }], + details: { diff: "-1 old\n+1 new", patch, firstChangedLine: 1 }, + }, + }); + const edit = yield* takeEvent( + (event) => + event.type === "turn_item.updated" && + event.turnItem.type === "file_change" && + event.turnItem.status === "completed", + ); + assert.isTrue( + edit.type === "turn_item.updated" && + edit.turnItem.type === "file_change" && + edit.turnItem.fileName === "a.ts" && + edit.turnItem.diffStr === patch, + ); + + yield* fake.emit({ + type: "tool_execution_start", + toolCallId: "call_write", + toolName: "write", + args: { path: "b.ts", content: "export {};\n" }, + }); + yield* fake.emit({ + type: "tool_execution_end", + toolCallId: "call_write", + toolName: "write", + isError: false, + result: { content: [{ type: "text", text: "Successfully wrote to b.ts" }] }, + }); + const write = yield* takeEvent( + (event) => + event.type === "turn_item.updated" && + event.turnItem.type === "file_change" && + event.turnItem.status === "completed", + ); + assert.isTrue( + write.type === "turn_item.updated" && + write.turnItem.type === "file_change" && + write.turnItem.fileName === "b.ts" && + write.turnItem.newStr === "export {};\n", + ); + + // A failed edit has no patch, so it keeps the error to show when expanded. + yield* fake.emit({ + type: "tool_execution_start", + toolCallId: "call_edit_failed", + toolName: "edit", + args: { path: "c.ts", edits: [{ oldText: "missing", newText: "new" }] }, + }); + yield* fake.emit({ + type: "tool_execution_end", + toolCallId: "call_edit_failed", + toolName: "edit", + isError: true, + result: { content: [{ type: "text", text: "Could not find the text in c.ts." }] }, + }); + const failedEdit = yield* takeEvent( + (event) => + event.type === "turn_item.updated" && + event.turnItem.type === "file_change" && + event.turnItem.status === "failed", + ); + assert.isTrue( + failedEdit.type === "turn_item.updated" && + failedEdit.turnItem.type === "file_change" && + failedEdit.turnItem.diffStr === "Could not find the text in c.ts.", + ); + }).pipe(Effect.scoped, Effect.provide(testLayer)), + ); + it.effect("settles a command-only prompt from its deferred ack and idle probe", () => Effect.gen(function* () { const fake = yield* makeFakePi; diff --git a/apps/server/src/orchestration-v2/Adapters/PiAdapterV2.ts b/apps/server/src/orchestration-v2/Adapters/PiAdapterV2.ts index 6f0bcf258acb..56f3d7b28796 100644 --- a/apps/server/src/orchestration-v2/Adapters/PiAdapterV2.ts +++ b/apps/server/src/orchestration-v2/Adapters/PiAdapterV2.ts @@ -988,6 +988,12 @@ export function makePiAdapterV2( if (toolName === "edit" || toolName === "write") { const fileName = recordString(args, "path") ?? recordString(args, "file_path"); if (fileName !== undefined) { + // edit reports a unified patch in its result details; write only + // carries the new content in its args. A failed call keeps its error. + const diffStr = + recordString(recordField(resultRecord, "details"), "patch") ?? + (isError && outputText.trim().length > 0 ? outputText : undefined); + const newStr = toolName === "write" ? recordString(args, "content") : undefined; yield* emit({ type: "turn_item.updated", driver: PI_PROVIDER, @@ -996,6 +1002,8 @@ export function makePiAdapterV2( title: toolName, type: "file_change", fileName, + ...(diffStr === undefined ? {} : { diffStr }), + ...(newStr === undefined ? {} : { newStr }), }, }); return; diff --git a/apps/server/src/orchestration-v2/Orchestrator.ts b/apps/server/src/orchestration-v2/Orchestrator.ts index 42c55371d5f7..0a913831890d 100644 --- a/apps/server/src/orchestration-v2/Orchestrator.ts +++ b/apps/server/src/orchestration-v2/Orchestrator.ts @@ -49,6 +49,7 @@ import { RunId, ThreadLinkedPullRequest, ThreadId, + type TurnItemId, } from "@t3tools/contracts"; import { modelSelectionsEqual } from "@t3tools/shared/model"; import { @@ -264,6 +265,10 @@ export interface OrchestratorV2Shape { options: ProjectionTimelinePageOptions, ) => Effect.Effect; readonly getMessageCount: (threadId: ThreadId) => Effect.Effect; + readonly getTurnItem: (input: { + readonly threadId: ThreadId; + readonly itemId: TurnItemId; + }) => Effect.Effect; readonly getThreadRecords: ( threadId: ThreadId, fields: ReadonlyArray, @@ -10067,6 +10072,14 @@ const makeOrchestrator = Effect.fn("orchestrationV2.Orchestrator.layer")(functio projectionStore .getMessageCount(threadId) .pipe(Effect.mapError((cause) => new OrchestratorProjectionError({ threadId, cause }))), + getTurnItem: (input) => + projectionStore + .getTurnItem(input) + .pipe( + Effect.mapError( + (cause) => new OrchestratorProjectionError({ threadId: input.threadId, cause }), + ), + ), getThreadRecords: (threadId, fields, filter) => projectionStore .getThreadRecords(threadId, fields, filter) @@ -10188,6 +10201,7 @@ const layerUnavailable: Layer.Layer = Layer.succeed( ), getTimelinePage: (threadId) => Effect.fail(new OrchestratorProjectionError({ threadId })), getMessageCount: (threadId) => Effect.fail(new OrchestratorProjectionError({ threadId })), + getTurnItem: ({ threadId }) => Effect.fail(new OrchestratorProjectionError({ threadId })), getThreadRecords: (threadId) => Effect.fail(new OrchestratorProjectionError({ threadId })), getThreadProjection: (threadId) => Effect.fail( diff --git a/apps/server/src/orchestration-v2/ProjectionStore.test.ts b/apps/server/src/orchestration-v2/ProjectionStore.test.ts index 3e979544c87b..3bb7617eef0f 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.test.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.test.ts @@ -959,6 +959,14 @@ it.layer(TestLayer)("ProjectionStoreV2", (it) => { assert.strictEqual(older.projection.turnItems[0]?.ordinal, 850); assert.strictEqual(older.projection.turnItems.at(-1)?.ordinal, 925); + // A single item reads back with its full output, scoped to its thread. + const itemId = TurnItemId.make("turn-item:bounded-sql-history:925"); + const stored = yield* projectionStore.getTurnItem({ threadId, itemId }); + assert.strictEqual(stored?.type === "command_execution" ? stored.output : undefined, "ok"); + assert.isNull( + yield* projectionStore.getTurnItem({ threadId: ThreadId.make("thread:other"), itemId }), + ); + const sqlPageLimit = THREAD_HISTORY_PAGE_POLICY.maxItems + 2; const initialSnapshot = yield* projectionStore.getThreadSnapshotWindow(threadId, { rowLimit: sqlPageLimit, diff --git a/apps/server/src/orchestration-v2/ProjectionStore.ts b/apps/server/src/orchestration-v2/ProjectionStore.ts index 93f1e1e90de2..a6be3079946f 100644 --- a/apps/server/src/orchestration-v2/ProjectionStore.ts +++ b/apps/server/src/orchestration-v2/ProjectionStore.ts @@ -318,6 +318,11 @@ export interface ProjectionStoreV2Shape { readonly getNextTurnItemOrdinal: ( threadId: ThreadId, ) => Effect.Effect; + /** One persisted turn item, or null when the thread has no such item. */ + readonly getTurnItem: (input: { + readonly threadId: ThreadId; + readonly itemId: TurnItemId; + }) => Effect.Effect; readonly getThreadRecords: ( threadId: ThreadId, fields: ReadonlyArray, @@ -4504,6 +4509,18 @@ export const layer: Layer.Layer = Effect.mapError(controlReadError(threadId)), ); + const getTurnItem: ProjectionStoreV2Shape["getTurnItem"] = ({ threadId, itemId }) => + sql<{ payload_json: string }>`SELECT payload_json + FROM orchestration_v2_projection_turn_items + WHERE turn_item_id = ${itemId} AND thread_id = ${threadId}`.pipe( + Effect.flatMap((rows) => + rows[0] === undefined + ? Effect.succeed(null) + : decodeTurnItemPayload(rows[0].payload_json), + ), + Effect.mapError(controlReadError(threadId)), + ); + const getThreadAttachmentIds: ProjectionStoreV2Shape["getThreadAttachmentIds"] = (threadId) => sql<{ id: string }>` SELECT DISTINCT json_extract(attachment.value, '$.id') AS id @@ -5539,6 +5556,7 @@ export const layer: Layer.Layer = hasUnpairedRunInterruptRequest, getMessageCount, getNextTurnItemOrdinal, + getTurnItem, getThreadRecords, getRuntimeRequest, getPlan, @@ -5791,6 +5809,13 @@ export const layerMemory: Layer.Layer = Layer.effect( ?.turnItems.reduce((max, item) => Math.max(max, item.ordinal), 0) ?? 0) + 1, ), ), + getTurnItem: ({ threadId, itemId }) => + Ref.get(replayState).pipe( + Effect.map( + (state) => + state.projections.get(threadId)?.turnItems.find((item) => item.id === itemId) ?? null, + ), + ), getThreadAttachmentIds: (threadId) => service .getThreadProjection(threadId) diff --git a/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts b/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts index eba830319aaf..91fe20459ca2 100644 --- a/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts +++ b/apps/server/src/orchestration-v2/ProviderTurnControlService.test.ts @@ -227,6 +227,7 @@ it.effect( getTimelinePage: () => Effect.die("Unused timeline read"), getMessageCount: () => Effect.die("unused message count"), getNextTurnItemOrdinal: () => Effect.die("unused ordinal read"), + getTurnItem: () => Effect.die("unused turn item read"), getThreadRecords: () => Effect.die("unused record read"), getRuntimeRequest: () => Effect.die("unused getRuntimeRequest"), getRunningTurnContext: () => Effect.die("unused getRunningTurnContext"), diff --git a/apps/server/src/orchestration-v2/ThreadManagementService.ts b/apps/server/src/orchestration-v2/ThreadManagementService.ts index 3bb397728060..f0468364a25c 100644 --- a/apps/server/src/orchestration-v2/ThreadManagementService.ts +++ b/apps/server/src/orchestration-v2/ThreadManagementService.ts @@ -10,6 +10,7 @@ import { type ModelSelection, type OrchestrationV2Actor, type OrchestrationV2Command, + type OrchestrationV2GetTurnItemResult, type OrchestrationV2ServerCommand, type OrchestrationV2ConversationMessage, type OrchestrationV2CreationSource, @@ -22,6 +23,7 @@ import { RunId, type ScheduledTaskId, ThreadId, + type TurnItemId, } from "@t3tools/contracts"; import * as Context from "effect/Context"; import * as DateTime from "effect/DateTime"; @@ -32,6 +34,7 @@ import * as Option from "effect/Option"; import * as Schema from "effect/Schema"; import * as Orchestrator from "./Orchestrator.ts"; +import { projectTurnItemForDetail } from "./WireProjection.ts"; import * as LegacyV1ThreadImporter from "./legacy/LegacyV1ThreadImporter.ts"; export type ThreadManagementSendMode = "auto" | "queue" | "steer" | "restart"; @@ -276,6 +279,14 @@ export interface ThreadManagementServiceShape { ) => Effect.Effect; readonly getTimelinePage: Orchestrator.OrchestratorV2["Service"]["getTimelinePage"]; readonly getMessageCount: Orchestrator.OrchestratorV2["Service"]["getMessageCount"]; + /** + * One turn item with the input and output the thread stream withholds, + * bounded for the wire. Clients fetch it when a tool row is expanded. + */ + readonly getTurnItem: (input: { + readonly threadId: ThreadId; + readonly itemId: TurnItemId; + }) => Effect.Effect; readonly getThreadRecords: Orchestrator.OrchestratorV2["Service"]["getThreadRecords"]; readonly getThreadProjection: ( threadId: ThreadId, @@ -721,6 +732,11 @@ const make = Effect.gen(function* () { ensureProjectionTranscript(threadId).pipe( Effect.andThen(orchestrator.getMessageCount(threadId)), ), + getTurnItem: (input) => + ensureProjectionTranscript(input.threadId).pipe( + Effect.andThen(orchestrator.getTurnItem(input)), + Effect.map((item) => ({ item: item === null ? null : projectTurnItemForDetail(item) })), + ), getThreadRecords: (threadId, fields, filter) => ensureProjectionTranscript(threadId).pipe( Effect.andThen(orchestrator.getThreadRecords(threadId, fields, filter)), diff --git a/apps/server/src/orchestration-v2/WireProjection.test.ts b/apps/server/src/orchestration-v2/WireProjection.test.ts index 846ed1f1a1f1..48a4881bdfb4 100644 --- a/apps/server/src/orchestration-v2/WireProjection.test.ts +++ b/apps/server/src/orchestration-v2/WireProjection.test.ts @@ -17,7 +17,11 @@ import { describe, expect, it } from "@effect/vitest"; import * as DateTime from "effect/DateTime"; import * as Schema from "effect/Schema"; -import { projectTurnItemForWire, projectDomainEventForWire } from "./WireProjection.ts"; +import { + projectTurnItemForWire, + projectTurnItemForDetail, + projectDomainEventForWire, +} from "./WireProjection.ts"; import { threadShellFromProjection } from "./ProjectionStore.ts"; const decodeTurnItem = Schema.decodeUnknownSync(OrchestrationV2TurnItem); @@ -173,7 +177,7 @@ describe("orchestration V2 wire projection", () => { it("omits even small dynamic tool results while retaining input", () => { const item = { ...base, output: { ok: true } } satisfies OrchestrationV2TurnItem; - expect(projectTurnItemForWire(item)).toEqual(base); + expect(projectTurnItemForWire(item)).toEqual({ ...base, outputOmitted: true }); expect(item.output).toEqual({ ok: true }); }); @@ -252,10 +256,24 @@ describe("orchestration V2 wire projection", () => { const projected = projectTurnItemForWire(item); expect(projected).not.toHaveProperty("output"); expect(projected).toMatchObject({ input: "test", status: "completed" }); + // Clients fetch withheld output on demand, so they need to know it exists. + expect(projected.type === "command_execution" ? projected.outputOmitted : null).toBe( + output ? true : undefined, + ); expect(item.output).toBe(output); }, ); + it.each([ + ["echo ok", "echo ok"], + ["a".repeat(262_143) + "😀", "a".repeat(262_143) + "\n… output truncated for transport"], + ])("bounds fetched command input without changing persistence, case %#", (input, expected) => { + const item = { ...base, type: "command_execution" as const, input, output: "ok" }; + const projected = projectTurnItemForDetail(item); + expect(projected).toMatchObject({ input: expected, output: "ok" }); + expect(item.input).toBe(input); + }); + it("keeps failure evidence without retaining command output", () => { const item = { ...base, @@ -290,6 +308,14 @@ describe("orchestration V2 wire projection", () => { expect(projected).not.toHaveProperty("newStr"); expect(projected).toMatchObject({ fileName: "src/main.ts", additions: 3, deletions: 1 }); expect(item.diffStr).toBe("+new code"); + // A failed edit keeps the provider's error so expanding the row can show it. + const failed = projectTurnItemForWire({ + ...item, + status: "failed", + diffStr: "String to replace not found", + }); + expect(failed).toMatchObject({ diffStr: "String to replace not found" }); + expect(failed).not.toHaveProperty("newStr"); }); it("retains only result identities and failure metadata in live tool events", () => { diff --git a/apps/server/src/orchestration-v2/WireProjection.ts b/apps/server/src/orchestration-v2/WireProjection.ts index b72cd0542457..7c17147d220a 100644 --- a/apps/server/src/orchestration-v2/WireProjection.ts +++ b/apps/server/src/orchestration-v2/WireProjection.ts @@ -8,19 +8,22 @@ import { compactDynamicToolOutput, toolOutputIndicatesFailure } from "@t3tools/s const MAX_DETAIL_STRING_BYTES = 32_768; const MAX_DYNAMIC_VALUE_BYTES = 16_384; +const MAX_ON_DEMAND_BYTES = 256 * 1024; -function truncateDetail(value: string | undefined): string | undefined { +function truncateDetail( + value: string | undefined, + maxBytes = MAX_DETAIL_STRING_BYTES, +): string | undefined { if ( value === undefined || - (value.length <= MAX_DETAIL_STRING_BYTES && - Buffer.byteLength(value, "utf8") <= MAX_DETAIL_STRING_BYTES) + (value.length <= maxBytes && Buffer.byteLength(value, "utf8") <= maxBytes) ) { return value; } // UTF-8 needs at least one byte per UTF-16 code unit. Only encode the prefix // that could fit, rather than allocating a buffer for the complete output. - const prefix = Buffer.from(value.slice(0, MAX_DETAIL_STRING_BYTES), "utf8") - .subarray(0, MAX_DETAIL_STRING_BYTES) + const prefix = Buffer.from(value.slice(0, maxBytes), "utf8") + .subarray(0, maxBytes) .toString("utf8") .replace(/\uFFFD$/u, ""); return `${prefix}\n… output truncated for transport`; @@ -80,13 +83,20 @@ export function projectTurnItemForWire(item: OrchestrationV2TurnItem): Orchestra (item.exitCode !== undefined && item.exitCode !== 0) || (output !== undefined && toolOutputIndicatesFailure(output.slice(0, MAX_DETAIL_STRING_BYTES))); - return failed ? { ...projected, outputIndicatesFailure: true } : projected; + return { + ...projected, + ...(failed ? { outputIndicatesFailure: true } : {}), + ...(output?.trim() ? { outputOmitted: true } : {}), + }; } case "file_change": { // File identity and counts are enough for activity. Full diffs already // have a dedicated read path and remain intact in persistence. - const { diffStr: _diff, oldStr: _old, newStr: _new, ...projected } = item; - return projected; + const { diffStr, oldStr: _old, newStr: _new, ...projected } = item; + // A failed edit stores the provider's error where the diff would be. + return item.status === "failed" && diffStr?.trim() + ? { ...projected, diffStr: truncateDetail(diffStr) } + : projected; } case "subagent": return { @@ -102,6 +112,7 @@ export function projectTurnItemForWire(item: OrchestrationV2TurnItem): Orchestra ...projected, input: summarizeDynamicValue(item.input), ...(output === undefined ? {} : { output }), + ...(hasDynamicValue(rawOutput) ? { outputOmitted: true } : {}), }; } default: @@ -109,6 +120,62 @@ export function projectTurnItemForWire(item: OrchestrationV2TurnItem): Orchestra } } +function hasDynamicValue(value: unknown): boolean { + if (value === undefined || value === null) return false; + if (typeof value === "string") return value.trim().length > 0; + if (Array.isArray(value)) return value.length > 0; + return typeof value !== "object" || Object.keys(value).length > 0; +} + +function boundDynamicValue(value: unknown): unknown { + if (value === undefined) return value; + if (typeof value === "string") return truncateDetail(value, MAX_ON_DEMAND_BYTES); + let json: string; + try { + // Compact, so measuring does not inflate the value; clients indent it. + json = JSON.stringify(value) ?? String(value); + } catch { + return "Unserializable tool value"; + } + return Buffer.byteLength(json, "utf8") <= MAX_ON_DEMAND_BYTES + ? value + : truncateDetail(json, MAX_ON_DEMAND_BYTES); +} + +/** + * Projects one item for an on-demand detail read: keeps the input and output + * the timeline withholds, bounded so a huge result cannot stall the socket. + */ +export function projectTurnItemForDetail(item: OrchestrationV2TurnItem): OrchestrationV2TurnItem { + switch (item.type) { + case "command_execution": + return { + ...item, + input: truncateDetail(item.input, MAX_ON_DEMAND_BYTES) ?? "", + output: truncateDetail(item.output, MAX_ON_DEMAND_BYTES), + }; + case "dynamic_tool": + return { + ...item, + input: boundDynamicValue(item.input), + output: boundDynamicValue(item.output), + }; + case "subagent": + return { + ...item, + prompt: truncateDetail(item.prompt, MAX_ON_DEMAND_BYTES) ?? "", + progress: truncateDetail(item.progress, MAX_ON_DEMAND_BYTES), + result: + item.result === null ? null : (truncateDetail(item.result, MAX_ON_DEMAND_BYTES) ?? null), + }; + case "handoff": + case "file_change": + return projectTurnItemForWire(item); + default: + return item; + } +} + export function projectContextHandoffForWire( handoff: OrchestrationV2ContextHandoff, ): OrchestrationV2ContextHandoff { diff --git a/apps/server/src/relay/AgentAwarenessRelay.test.ts b/apps/server/src/relay/AgentAwarenessRelay.test.ts index 003403fc5c09..4467fd9768ea 100644 --- a/apps/server/src/relay/AgentAwarenessRelay.test.ts +++ b/apps/server/src/relay/AgentAwarenessRelay.test.ts @@ -194,6 +194,7 @@ const makeTestRelay = Effect.fnUntraced(function* ( dispatch: unused, getTimelinePage: () => Effect.die("Unused timeline read"), getMessageCount: () => Effect.die("unused message count"), + getTurnItem: () => Effect.die("unused turn item read"), getThreadRecords: () => Effect.die("unused record read"), getThreadProjection: unused, getCheckpointContext: unused, diff --git a/apps/server/src/ws.ts b/apps/server/src/ws.ts index ae331995d201..7800603f2818 100644 --- a/apps/server/src/ws.ts +++ b/apps/server/src/ws.ts @@ -1847,6 +1847,21 @@ const makeWsRpcLayer = ( readWorkflowScript({ scriptPath: input.scriptPath }), { "rpc.aggregate": "orchestration" }, ), + [ORCHESTRATION_V2_WS_METHODS.getTurnItem]: (input) => + observeRpcEffect( + ORCHESTRATION_V2_WS_METHODS.getTurnItem, + threadManagement.getTurnItem(input).pipe( + Effect.mapError( + (cause) => + new OrchestrationV2GetThreadProjectionError({ + threadId: input.threadId, + message: "Failed to load turn item", + cause, + }), + ), + ), + { "rpc.aggregate": "orchestration" }, + ), [ORCHESTRATION_V2_WS_METHODS.getTurnDiff]: (input) => observeRpcEffect( ORCHESTRATION_V2_WS_METHODS.getTurnDiff, diff --git a/apps/web/src/components/chat/MessagesTimeline.tsx b/apps/web/src/components/chat/MessagesTimeline.tsx index 3b80e65bb285..bcaaa61b0b10 100644 --- a/apps/web/src/components/chat/MessagesTimeline.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.tsx @@ -45,6 +45,10 @@ import { workEntryViewedImagePath, } from "@t3tools/client-runtime/work-log/presentation"; import { resolveWorkGroupScrollAnchor } from "@t3tools/client-runtime/work-log/scroll-anchor"; +import { + turnItemHasDetail, + turnItemNeedsDetailFetch, +} from "@t3tools/client-runtime/work-log/item-detail"; import { formatAttachmentSize } from "@t3tools/client-runtime/state/attachments"; import { subagentGroupSummary, @@ -260,7 +264,7 @@ import { formatDayAwareTimestamp, formatUpcomingTimestamp, } from "../../timestampFormat"; -import { V2ItemInspector } from "./V2ItemInspector"; +import { FetchedToolOutput, V2ItemInspector } from "./V2ItemInspector"; import { useV2ItemSupport } from "../../state/v2ItemSupport"; import { Collapsible, CollapsibleTrigger, CollapsiblePanel } from "../ui/collapsible"; import { @@ -5151,10 +5155,25 @@ function WorkEntryLogRow(props: WorkEntryRowProps) { viewedImage ? viewedImagePath : null, ) : null; + // Projected rows expand to the item inspector, so only offer a disclosure + // when it has something to show, even if that output still has to load. + // Reads and skills still fetch the output the timeline withheld. + const plainOutputFetches = + plainOutput !== undefined && + workEntry.projectedItem !== undefined && + turnItemNeedsDetailFetch(workEntry.projectedItem.item); const canExpandProjectedItem = plainOutput !== undefined - ? Boolean(plainOutput || viewedImage || workEntry.questionAnswer) - : canExpand || workEntry.projectedItem !== undefined; + ? Boolean(plainOutput || viewedImage || workEntry.questionAnswer || plainOutputFetches) + : workEntry.projectedItem === undefined + ? canExpand + : isReasoning + ? Boolean(workEntry.detail?.trim()) + : Boolean( + viewedImage || + workEntry.questionAnswer || + turnItemHasDetail(workEntry.projectedItem.item), + ); // Reserve destructive row styling for severe failures, not routine tool errors. const iconWrapperClass = cn( "flex size-4 items-center justify-center", @@ -5325,7 +5344,9 @@ function WorkEntryLogRow(props: WorkEntryRowProps) { !isReasoning && !workEntry.questionAnswer && canExpandProjectedItem && - (expandedBody || (workEntry.projectedItem && plainOutput === undefined)) ? ( + (expandedBody || + plainOutputFetches || + (workEntry.projectedItem && plainOutput === undefined)) ? ( {workEntry.projectedItem && plainOutput === undefined ? ( - ) : expandedBody ? ( - 
{expandedBody}
- ) : null} + ) : ( + <> + {expandedBody ? ( + 
{expandedBody}
+ ) : null} + {plainOutputFetches && workEntry.projectedItem ? ( + + ) : null} + + )} ) : null} diff --git a/apps/web/src/components/chat/V2ItemInspector.tsx b/apps/web/src/components/chat/V2ItemInspector.tsx index b2f0f6b9e762..4f33a7f5b1a7 100644 --- a/apps/web/src/components/chat/V2ItemInspector.tsx +++ b/apps/web/src/components/chat/V2ItemInspector.tsx @@ -4,12 +4,19 @@ import type { RunId, ThreadId, } from "@t3tools/contracts"; +import { + formatToolValue, + turnItemDetailRevision, + turnItemNeedsDetailFetch, + turnItemOutputText, +} from "@t3tools/client-runtime/work-log/item-detail"; import { ExternalLinkIcon, GitBranchIcon, RotateCcwIcon } from "lucide-react"; -import { memo, Suspense, use, useMemo } from "react"; +import { memo, type ReactNode, Suspense, use, useMemo } from "react"; import { useTheme } from "../../hooks/useTheme"; import { resolveDiffThemeName } from "../../lib/diffRendering"; import { getSyntaxHighlighterPromise } from "../../lib/syntaxHighlighting"; +import { useTurnItemDetail } from "../../state/queries"; import { useV2ItemSupport } from "../../state/v2ItemSupport"; import { formatWorkspaceRelativePath } from "../../filePathDisplay"; import { Button } from "../ui/button"; @@ -81,8 +88,94 @@ function StructuredValue({ ); } +function SectionLabel({ children }: { readonly children: ReactNode }) { + return ( +

+ {children} +

+ ); +} + +/** + * The item behind a projected row, with the output the timeline withheld + * fetched while the row is open. + */ +function useFetchedTurnItem( + projectedItem: OrchestrationV2ProjectedTurnItem, + environmentId: EnvironmentId, +) { + const wireItem = projectedItem.item; + const fetches = turnItemNeedsDetailFetch(wireItem); + const detail = useTurnItemDetail( + fetches + ? { + environmentId, + threadId: projectedItem.sourceThreadId, + itemId: projectedItem.sourceItemId, + revision: turnItemDetailRevision(wireItem), + } + : null, + ); + const fetchedItem = detail.data?.item; + const item = fetchedItem?.type === wireItem.type ? fetchedItem : wireItem; + return { + item, + output: { + text: turnItemOutputText(item), + pending: item === wireItem && detail.isPending, + error: + item !== wireItem + ? null + : detail.data?.item === null + ? "Output is no longer available." + : detail.error, + empty: fetches && item !== wireItem, + }, + }; +} + +/** Output the timeline withheld, fetched while the row is open. */ +function ToolOutput(props: { + readonly text: string | null; + readonly pending: boolean; + readonly error: string | null; + readonly empty: boolean; +}) { + const body = props.text ? ( + + ) : props.pending ? ( +

Loading output…

+ ) : props.error ? ( +

Couldn't load output: {props.error}

+ ) : props.empty ? ( +

No output.

+ ) : null; + if (body === null) return null; + return ( +
+ Output + {body} +
+ ); +} + +/** Fetched output for rows that show their own plain text instead of the inspector. */ +export function FetchedToolOutput(props: { + readonly projectedItem: OrchestrationV2ProjectedTurnItem; + readonly environmentId: EnvironmentId; +}) { + const { output } = useFetchedTurnItem(props.projectedItem, props.environmentId); + return ( +
+ +
+ ); +} + export const V2ItemInspector = memo(function V2ItemInspector(props: V2ItemInspectorProps) { - const { item } = props.projectedItem; + const fetched = useFetchedTurnItem(props.projectedItem, props.environmentId); + const item = fetched.item; + const output = ; const support = useV2ItemSupport({ environmentId: props.environmentId, sourceThreadId: props.projectedItem.sourceThreadId, @@ -107,6 +200,7 @@ export const V2ItemInspector = memo(function V2ItemInspector(props: V2ItemInspec {item.type === "command_execution" ? (
+ {output} {item.exitCode !== undefined ? (

Process exited with code {item.exitCode} @@ -137,6 +231,9 @@ export const V2ItemInspector = memo(function V2ItemInspector(props: V2ItemInspec ) : null}

+ {item.status === "failed" && item.diffStr?.trim() ? ( + + ) : null} {item.changes !== undefined && item.changes.length > 0 ? (