From 943426785bdc5b669622bf053a07059d50913f56 Mon Sep 17 00:00:00 2001 From: Kalvin Chau Date: Mon, 31 Aug 2026 20:56:06 -0700 Subject: [PATCH] fix(search): search full session set via server-side session/list query Cmd-K and Session History search only covered sessions already loaded in the renderer: content search exported each loaded session client-side, so anything past the first session/list page was never searched. Discovery now runs server-side: listSessionsPage/acpListSessionsPage send _meta.query, which goose translates to a SQL keyword filter over message text with cursor pagination. The export sweep remains, but only to enrich server-matched loaded sessions with snippet/messageId/matchCount; every server match rows as a content hit even without enrichment. - searchSessions walks the filtered list (cursor-cycle and page-cap are errors, never silent truncation) and returns the full match set - useSessionSearch treats the server match set as authoritative for content membership, with per-query retention, generation-gated commits, and snippet preservation across failed enrichments - admission predicates keep scope policies: Cmd-K excludes archived, History applies its scope/project filters to discovered sessions - selecting a discovered result hydrates it into the session store synchronously (with persisted workspace metadata) before activation; store-backed card actions are hidden until a row is hydrated - coverage/progress now reflects export enrichment only Fixes #257 --- src/app/AppShell.navigation.test.tsx | 56 ++++ src/app/AppShell.tsx | 23 +- src/features/chat/lib/acpSessionMapping.ts | 23 +- src/features/search/ui/SearchView.tsx | 7 + .../search/ui/__tests__/SearchView.test.tsx | 27 +- .../hooks/__tests__/useSessionSearch.test.ts | 263 +++++++++++++++++- .../sessions/hooks/useSessionSearch.ts | 224 ++++++++++++--- .../sessions/lib/buildSessionSearchResults.ts | 23 +- .../sessions/lib/sessionListFilters.ts | 13 + .../sessions/ui/SessionHistoryView.tsx | 79 ++++-- .../ui/__tests__/SessionHistoryView.test.tsx | 218 ++++++++++++++- src/shared/api/__tests__/acp.test.ts | 2 +- src/shared/api/__tests__/acpApi.test.ts | 31 +++ .../api/__tests__/sessionSearch.test.ts | 192 ++++++++++++- src/shared/api/acp.ts | 20 +- src/shared/api/acpApi.ts | 19 +- src/shared/api/sessionSearch.ts | 125 ++++++++- 17 files changed, 1246 insertions(+), 99 deletions(-) diff --git a/src/app/AppShell.navigation.test.tsx b/src/app/AppShell.navigation.test.tsx index a2a2f53ec..53821fa94 100644 --- a/src/app/AppShell.navigation.test.tsx +++ b/src/app/AppShell.navigation.test.tsx @@ -93,6 +93,7 @@ const mockAcpCreateSession = vi.hoisted(() => vi.fn()); const mockAcpPrepareSession = vi.hoisted(() => vi.fn()); const mockAcpSetSessionConfigOption = vi.hoisted(() => vi.fn()); const mockAcpListSessionsPage = vi.hoisted(() => vi.fn()); +const mockAcpSearchSessions = vi.hoisted(() => vi.fn()); const mockBuildFeatures = vi.hoisted(() => ({ byoKeyProviders: false, voiceConversation: false, @@ -492,6 +493,7 @@ vi.mock("@/shared/api/acp", () => ({ acpGetSessionInfo: (...args: unknown[]) => mockAcpGetSessionInfo(...args), acpListSessionsPage: (...args: unknown[]) => mockAcpListSessionsPage(...args), acpLoadSession: (...args: unknown[]) => mockAcpLoadSession(...args), + acpSearchSessions: (...args: unknown[]) => mockAcpSearchSessions(...args), discoverAcpProviders: vi.fn().mockResolvedValue([]), })); @@ -1003,6 +1005,15 @@ describe("AppShell global navigation", () => { mockAcpGetSessionInfo.mockResolvedValue(null); mockAcpLoadSession.mockReset(); mockAcpLoadSession.mockResolvedValue(undefined); + mockAcpSearchSessions.mockReset(); + // Default: the server matches nothing; tests that exercise discovery + // override with their own match set. + mockAcpSearchSessions.mockImplementation(async () => ({ + results: [], + searchedIds: [], + failedIds: [], + matchedInfos: [], + })); mockToastError.mockReset(); mockListenSessionDeepLinkErrors.mockReset(); mockListenSessionDeepLinkErrors.mockResolvedValue(vi.fn()); @@ -5365,6 +5376,51 @@ describe("AppShell global navigation", () => { ); }); + it("hydrates a server-discovered session into the store when selected from search", async () => { + // The store starts EMPTY: the discovered session is known only to the + // server. Clicking its result must insert it synchronously — activation + // renders the chat only for store sessions. + useChatSessionStore.setState({ sessions: [] }); + mockAcpSearchSessions.mockImplementation(async () => ({ + results: [], + searchedIds: [], + failedIds: [], + matchedInfos: [ + { + sessionId: "server-1", + title: "Server match", + updatedAt: "2026-07-28T12:00:00.000Z", + createdAt: "2026-07-28T11:00:00.000Z", + lastMessageAt: null, + archivedAt: null, + userSetName: false, + messageCount: 2, + subtitle: null, + workingDir: "/tmp/project", + projectId: null, + providerId: null, + modelId: null, + personaId: null, + }, + ], + })); + const user = userEvent.setup(); + renderAppShell(); + + await user.click(screen.getByRole("button", { name: "Search" })); + const search = screen.getByRole("textbox", { name: "Universal search" }); + await user.type(search, "server match"); + + await user.click( + await screen.findByRole("button", { name: /Open chat Server match/ }), + ); + + const stored = useChatSessionStore.getState().getSession("server-1"); + expect(stored).toBeDefined(); + expect(useChatSessionStore.getState().activeSessionId).toBe("server-1"); + expect(screen.getByTestId("active-view")).toHaveTextContent("chat"); + }); + it("focuses a detached chat selected from search", async () => { mockSessionWindowSupport.supported = true; useSessionWindowStore diff --git a/src/app/AppShell.tsx b/src/app/AppShell.tsx index a73c2d299..b8dc277e4 100644 --- a/src/app/AppShell.tsx +++ b/src/app/AppShell.tsx @@ -82,6 +82,7 @@ import { import { selectLocalMessageCountsBySession } from "@/features/chat/stores/chatSelectors"; import { resolveSessionCycleTarget } from "@/features/sessions/lib/sessionCycle"; import { useSessionWindowStore } from "@/features/chat/stores/sessionWindowStore"; +import { loadPersistedChatWorkspaceMetadata } from "@/features/chat/stores/workspaceAttachmentPersistence"; import { selectActiveSessionId, selectHasHydratedSessions, @@ -4152,7 +4153,12 @@ export function AppShell({ }, [handleSelectSession]); const handleSelectSearchResult = useCallback( - (sessionId: string, messageId?: string, query?: string) => { + ( + sessionId: string, + messageId?: string, + query?: string, + session?: ChatSession, + ) => { guardAppNavigation(() => { setSearchDialogOpen(false); if (messageId) { @@ -4177,6 +4183,21 @@ export function AppShell({ } return; } + // Server-discovered sessions are not in the store yet; hydrate from + // the row's own metadata synchronously so activation renders the chat + // instead of falling through to home. The row mapping skipped + // workspace persistence, so restore it here — opening the session + // must land in the workspace the user last worked in. + if (session && !useChatSessionStore.getState().getSession(sessionId)) { + const persisted = loadPersistedChatWorkspaceMetadata(sessionId); + useChatSessionStore.getState().addSession({ + ...session, + workspaceAttachments: + persisted?.workspaceAttachments ?? session.workspaceAttachments, + activeWorkspaceId: + persisted?.activeWorkspaceId ?? session.activeWorkspaceId, + }); + } selectSessionDirect(sessionId); }); }, diff --git a/src/features/chat/lib/acpSessionMapping.ts b/src/features/chat/lib/acpSessionMapping.ts index dbbde4b78..3845654ae 100644 --- a/src/features/chat/lib/acpSessionMapping.ts +++ b/src/features/chat/lib/acpSessionMapping.ts @@ -18,15 +18,30 @@ interface SessionPageState { } export function acpSessionToChatSession(session: AcpSessionInfo): ChatSession { - const now = new Date().toISOString(); const persistedWorkspaceMetadata = loadPersistedChatWorkspaceMetadata( session.sessionId, ); + return withWorkspaceBackfill({ + ...chatSessionFromAcpInfo(session), + workspaceAttachments: persistedWorkspaceMetadata?.workspaceAttachments, + activeWorkspaceId: persistedWorkspaceMetadata?.activeWorkspaceId, + }); +} + +/** + * The ACP→ChatSession field mapping without workspace hydration or backfill. + * Transient rows (server-discovered search results) skip both: the + * localStorage read is wasted on sessions this renderer never opened, and the + * workingDir backfill invents attachments for sessions that exist only as + * search rows. + */ +export function chatSessionFromAcpInfo(session: AcpSessionInfo): ChatSession { + const now = new Date().toISOString(); const executionTarget = executionTargetFromGooseServeSession({ providerId: session.providerId ?? undefined, modelId: session.modelId ?? undefined, }); - return withWorkspaceBackfill({ + return { id: session.sessionId, title: normalizeAcpTitle(session.title) ?? "Untitled", projectId: session.projectId ?? undefined, @@ -34,8 +49,6 @@ export function acpSessionToChatSession(session: AcpSessionInfo): ChatSession { executionTargetSource: executionTarget ? "acp" : undefined, personaId: session.personaId ?? undefined, workingDir: session.workingDir ?? undefined, - workspaceAttachments: persistedWorkspaceMetadata?.workspaceAttachments, - activeWorkspaceId: persistedWorkspaceMetadata?.activeWorkspaceId, createdAt: session.createdAt ?? session.updatedAt ?? now, updatedAt: session.updatedAt ?? now, lastMessageAt: session.lastMessageAt ?? undefined, @@ -43,7 +56,7 @@ export function acpSessionToChatSession(session: AcpSessionInfo): ChatSession { messageCount: session.messageCount, subtitle: session.subtitle ?? undefined, userSetName: session.userSetName, - }); + }; } function mergeSessionMetadata( diff --git a/src/features/search/ui/SearchView.tsx b/src/features/search/ui/SearchView.tsx index 6e04fe5a0..06c39a160 100644 --- a/src/features/search/ui/SearchView.tsx +++ b/src/features/search/ui/SearchView.tsx @@ -74,6 +74,9 @@ interface SearchViewProps { sessionId: string, messageId?: string, query?: string, + /** The result row's own session, passed so the caller can hydrate + * server-discovered sessions into the store before activating them. */ + session?: ChatSession, ) => void; onOpenExtension: (entry: ExtensionEntry) => void; onOpenAgent: (agentId: string) => void; @@ -167,6 +170,9 @@ export function SearchView({ locale: i18n.resolvedLanguage, getDisplayTitle, visibleMetadataOnly: true, + // Cmd-K's loaded slice excludes archived sessions; server-discovered + // matches must follow the same policy. + includeDiscoveredSession: (session) => !session.archivedAt, }); const { clear: clearChatSearch, @@ -540,6 +546,7 @@ export function SearchView({ sessionId, messageId, submittedQuery || trimmedDebouncedQuery, + result.session, ) } /> diff --git a/src/features/search/ui/__tests__/SearchView.test.tsx b/src/features/search/ui/__tests__/SearchView.test.tsx index 9423adeb8..fe438a8d5 100644 --- a/src/features/search/ui/__tests__/SearchView.test.tsx +++ b/src/features/search/ui/__tests__/SearchView.test.tsx @@ -61,6 +61,25 @@ function render(ui: ReactElement) { ); } +function matchedInfo(sessionId: string) { + return { + sessionId, + title: "Server match", + updatedAt: "2026-04-12T12:00:00Z", + createdAt: "2026-04-12T12:00:00Z", + lastMessageAt: null, + archivedAt: null, + userSetName: false, + messageCount: 3, + subtitle: null, + workingDir: null, + projectId: null, + providerId: null, + modelId: null, + personaId: null, + }; +} + describe("SearchView", () => { beforeEach(() => { vi.stubEnv("VITE_AUTOMATIONS", "1"); @@ -69,13 +88,14 @@ describe("SearchView", () => { mockGetAutomationTiles.mockReset(); mockGetAutomationTiles.mockResolvedValue({ tiles: [] }); mockAcpSearchSessions.mockReset(); - // Coverage is reported per sweep, derived from the targets the boundary was - // handed, so tests never have to restate which sessions a sweep covered. + // Production shape: the server matches every target handed to it here, and + // searchedIds ⊆ matchedInfos (only matched targets are export-enriched). mockAcpSearchSessions.mockImplementation( async (_query: string, targets: { id: string }[]) => ({ results: [], searchedIds: targets.map((target) => target.id), failedIds: [], + matchedInfos: targets.map((target) => matchedInfo(target.id)), }), ); mockListSkills.mockReset(); @@ -612,6 +632,7 @@ describe("SearchView", () => { results: [messageMatch], searchedIds: targets.map((target) => target.id), failedIds: [], + matchedInfos: targets.map((target) => matchedInfo(target.id)), }), ); @@ -648,6 +669,7 @@ describe("SearchView", () => { results: (typeof messageMatch)[]; searchedIds: string[]; failedIds: string[]; + matchedInfos: ReturnType[]; }; let resolveSweep: (sweep: Sweep) => void = () => {}; mockAcpSearchSessions.mockReturnValueOnce( @@ -679,6 +701,7 @@ describe("SearchView", () => { results: [messageMatch], searchedIds: ["session-1", "session-2"], failedIds: [], + matchedInfos: [matchedInfo("session-1"), matchedInfo("session-2")], }); }); diff --git a/src/features/sessions/hooks/__tests__/useSessionSearch.test.ts b/src/features/sessions/hooks/__tests__/useSessionSearch.test.ts index 10d574d72..239bd2aac 100644 --- a/src/features/sessions/hooks/__tests__/useSessionSearch.test.ts +++ b/src/features/sessions/hooks/__tests__/useSessionSearch.test.ts @@ -17,12 +17,52 @@ type SearchSweep = { results: MessageSearchResult[]; searchedIds: string[]; failedIds: string[]; + /** Server-discovered match set; production always returns it for a content + * query. Tests model it explicitly: any session the server matched, whether + * or not it is also an enrichment target. */ + matchedInfos: MatchedInfo[]; }; +type MatchedInfo = { + sessionId: string; + title: string; + updatedAt: string; + createdAt: string | null; + lastMessageAt: string | null; + archivedAt: string | null; + userSetName: boolean; + messageCount: number; + subtitle: string | null; + workingDir: string | null; + projectId: string | null; + providerId: string | null; + modelId: string | null; + personaId: string | null; +}; + +function matchedInfo(sessionId: string, title = "Server match"): MatchedInfo { + return { + sessionId, + title, + updatedAt: "2026-04-12T12:00:00Z", + createdAt: "2026-04-12T12:00:00Z", + lastMessageAt: null, + archivedAt: null, + userSetName: false, + messageCount: 3, + subtitle: "preview text", + workingDir: null, + projectId: null, + providerId: null, + modelId: null, + personaId: null, + }; +} + /** - * A sweep that read every target it was given. The boundary reports coverage - * per target, so tests that only care about matches still have to say which - * sessions were read — otherwise the hook would rightly report them unsearched. + * A sweep where the server matched every target it was handed. Production + * searchedIds are exactly the matched targets that were export-read, so + * modeling all targets as matched and read is the reachable shape. */ function sweptAll(results: MessageSearchResult[] = []) { return async ( @@ -32,18 +72,22 @@ function sweptAll(results: MessageSearchResult[] = []) { results, searchedIds: targets.map((target) => target.id), failedIds: [], + matchedInfos: targets.map((target) => matchedInfo(target.id)), }); } /** * An explicit sweep result, for deferred mocks that cannot see their targets. - * `searchedIds` must name the sessions this sweep actually covered. + * `searchedIds` must name the sessions this sweep actually covered, and every + * searched id must also appear in `matchedInfos` — the boundary only reads + * targets the server matched. */ function sweep( searchedIds: string[], results: MessageSearchResult[] = [], + matchedInfos: MatchedInfo[] = searchedIds.map((id) => matchedInfo(id)), ): SearchSweep { - return { results, searchedIds, failedIds: [] }; + return { results, searchedIds, failedIds: [], matchedInfos }; } vi.mock("@/shared/api/acp", () => ({ @@ -181,18 +225,23 @@ describe("useSessionSearch", () => { }); it("searches only new sessions incrementally and merges message results newest first", async () => { + // The server answers with the full match set on every call, not only the + // page's targets: page two's set includes acp-1 again. mockAcpSearchSessions .mockImplementationOnce(sweptAll()) - .mockImplementationOnce( - sweptAll([ + .mockImplementationOnce(async () => ({ + results: [ { sessionId: "acp-2", snippet: "needle in message", messageId: "message-2", matchCount: 2, }, - ]), - ); + ], + searchedIds: ["acp-2"], + failedIds: [], + matchedInfos: [matchedInfo("acp-1"), matchedInfo("acp-2")], + })); const { result } = renderSessionSearch(); @@ -233,6 +282,13 @@ describe("useSessionSearch", () => { await searchFor(result, "needle"); const staleSearchMore = result.current.searchMore; + // "follow" matches nothing on the server: an empty match set. + mockAcpSearchSessions.mockImplementationOnce(async () => ({ + results: [], + searchedIds: [], + failedIds: [], + matchedInfos: [], + })); await searchFor(result, "follow"); await act(async () => { await staleSearchMore([oldQueryOnlySession]); @@ -505,6 +561,7 @@ describe("useSessionSearch", () => { results: [], searchedIds: ["acp-1"], failedIds: ["acp-2"], + matchedInfos: [matchedInfo("acp-1"), matchedInfo("acp-2")], }), ); @@ -528,6 +585,7 @@ describe("useSessionSearch", () => { results: [], searchedIds: [], failedIds: ["acp-1"], + matchedInfos: [matchedInfo("acp-1")], }), ) .mockImplementationOnce(sweptAll()); @@ -620,6 +678,7 @@ describe("useSessionSearch", () => { results: [], searchedIds: [], failedIds: ["acp-2"], + matchedInfos: [matchedInfo("acp-2")], }), ) // The next page sweep must target acp-2 again, and this time it reads. @@ -683,6 +742,7 @@ describe("useSessionSearch", () => { results: [], searchedIds: [], failedIds: ["acp-1"], + matchedInfos: [matchedInfo("acp-1")], }), ); @@ -728,6 +788,7 @@ describe("useSessionSearch", () => { results: [], searchedIds: [], failedIds: ["acp-1"], + matchedInfos: [matchedInfo("acp-1")], }), ) // The next incremental sweep must target acp-1 again, and it reads. @@ -805,3 +866,187 @@ describe("useSessionSearch", () => { ); }); }); + +function serverSweep( + matchedInfos: MatchedInfo[], + searchedIds: string[], + results: MessageSearchResult[] = [], + failedIds: string[] = [], +): SearchSweep { + return { results, searchedIds, failedIds, matchedInfos }; +} + +describe("server-side discovery", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("surfaces content matches for sessions that are not loaded", async () => { + mockAcpSearchSessions.mockResolvedValueOnce( + serverSweep([matchedInfo("acp-1"), matchedInfo("old-1")], ["acp-1"]), + ); + + const { result } = renderSessionSearch(); + await searchFor(result, "needle"); + + expect(result.current.results.map((item) => item.session.id)).toEqual([ + "old-1", + "acp-1", + ]); + const discovered = result.current.results.find( + (item) => item.session.id === "old-1", + ); + // A server-discovered match rows as a content hit with no client-derived + // snippet until its corpus is exported. + expect(discovered).toMatchObject({ + matchType: "message", + session: { id: "old-1", title: "Server match" }, + }); + expect(discovered?.snippet).toBeUndefined(); + expect(discovered?.messageId).toBeUndefined(); + }); + + it("drops a loaded content match the server no longer matches", async () => { + const contentOnlySession: ChatSession = { + id: "acp-9", + title: "Untitled", + createdAt: "2026-04-10T12:00:00Z", + updatedAt: "2026-04-10T12:00:00Z", + messageCount: 1, + }; + mockAcpSearchSessions.mockResolvedValueOnce( + serverSweep( + [matchedInfo("acp-9")], + ["acp-9"], + [ + { + sessionId: "acp-9", + snippet: "needle in message", + messageId: "message-9", + matchCount: 1, + }, + ], + ), + ); + + const { result } = renderSessionSearch([contentOnlySession]); + await searchFor(result, "needle"); + expect(result.current.results.map((item) => item.session.id)).toEqual([ + "acp-9", + ]); + + // The session's content changed between sweeps: the server's empty match + // set is authoritative, so the stale content row must go even though no + // enrichment ran for it. + mockAcpSearchSessions.mockResolvedValueOnce(serverSweep([], [])); + await submitCurrentSearch(result); + + expect(result.current.results).toEqual([]); + }); + + it("keeps a matched target on screen when its enrichment export fails", async () => { + // Title deliberately does NOT match the query: if the row survives only + // as a metadata hit, the test proves nothing about content retention. + const contentOnlySession: ChatSession = { + id: "acp-9", + title: "Untitled", + createdAt: "2026-04-10T12:00:00Z", + updatedAt: "2026-04-10T12:00:00Z", + messageCount: 1, + }; + mockAcpSearchSessions.mockResolvedValueOnce({ + ...serverSweep([matchedInfo("acp-9")], []), + failedIds: ["acp-9"], + }); + + const { result } = renderSessionSearch([contentOnlySession]); + await searchFor(result, "needle"); + + // The export could not be read, but the server confirmed the match: the + // row degrades to snippet-less rather than vanishing. + expect(result.current.results).toHaveLength(1); + expect(result.current.results[0]).toMatchObject({ + session: { id: "acp-9" }, + matchType: "message", + }); + expect(result.current.progress).toMatchObject({ + searched: 0, + unreadable: 1, + }); + }); + + it("honors the admission check for discovered sessions", async () => { + mockAcpSearchSessions.mockResolvedValueOnce( + serverSweep([matchedInfo("acp-1"), matchedInfo("old-1")], ["acp-1"]), + ); + + const queryClient = new QueryClient(); + const { result } = renderHook( + () => + useSessionSearch({ + sessions, + resolvers, + includeDiscoveredSession: (session) => session.id !== "old-1", + }), + { + wrapper: ({ children }: { children: ReactNode }) => + createElement(QueryClientProvider, { client: queryClient }, children), + }, + ); + await searchFor(result, "needle"); + + expect(result.current.results.map((item) => item.session.id)).toEqual([ + "acp-1", + ]); + }); + + it("discovers matches with no loaded sessions at all", async () => { + // An empty loaded slice (fresh store, or a project filter with nothing on + // screen) must still reach the server: discovery needs no export targets. + mockAcpSearchSessions.mockResolvedValueOnce( + serverSweep([matchedInfo("old-1")], []), + ); + + const { result } = renderSessionSearch([]); + await searchFor(result, "needle"); + + expect(mockAcpSearchSessions).toHaveBeenCalledWith( + "needle", + [], + searchOptions, + ); + expect(result.current.results.map((item) => item.session.id)).toEqual([ + "old-1", + ]); + }); + + it("keeps earlier server-discovered rows across a searchMore page sweep", async () => { + mockAcpSearchSessions.mockResolvedValue( + serverSweep([matchedInfo("acp-1"), matchedInfo("old-1")], ["acp-1"]), + ); + + const { result } = renderSessionSearch(); + await searchFor(result, "needle"); + expect(result.current.results.map((item) => item.session.id)).toEqual([ + "old-1", + "acp-1", + ]); + + // A page sweep re-runs discovery; the server answers with the full match + // set again (now including the newly loaded session), so every earlier + // row must survive the rebuild. + mockAcpSearchSessions.mockResolvedValueOnce( + serverSweep( + [matchedInfo("acp-1"), matchedInfo("acp-2"), matchedInfo("old-1")], + ["acp-2"], + ), + ); + await searchMore(result, [...sessions, newerSession]); + + expect(result.current.results.map((item) => item.session.id)).toEqual([ + "old-1", + "acp-2", + "acp-1", + ]); + }); +}); diff --git a/src/features/sessions/hooks/useSessionSearch.ts b/src/features/sessions/hooks/useSessionSearch.ts index efa517729..25363dadc 100644 --- a/src/features/sessions/hooks/useSessionSearch.ts +++ b/src/features/sessions/hooks/useSessionSearch.ts @@ -1,12 +1,17 @@ import { useCallback, useContext, useRef, useState } from "react"; import { QueryClientContext } from "@tanstack/react-query"; import type { ChatSession } from "@/features/chat/stores/chatSessionStore"; +import { chatSessionFromAcpInfo } from "@/features/chat/lib/acpSessionMapping"; import { acpSearchSessions, + type AcpSessionInfo, type AcpSessionSearchResult, } from "@/shared/api/acp"; import { formatAcpErrorMessage } from "@/shared/api/acpErrors"; -import { sessionSearchStamp } from "@/shared/api/sessionSearch"; +import { + SERVER_CONTENT_SEARCH_MIN_CHARS, + sessionSearchStamp, +} from "@/shared/api/sessionSearch"; import { buildSessionSearchResults, mergeSessionSearchResults, @@ -20,14 +25,20 @@ interface UseSessionSearchOptions { locale?: string; getDisplayTitle?: (session: ChatSession) => string; visibleMetadataOnly?: boolean; + /** Admission check for server-discovered sessions outside `sessions`. When + * the caller's view is a filtered slice of the store (history's scope tab + + * project filter), a content hit that fails the check is not surfaced. + * Default admits everything. */ + includeDiscoveredSession?: (session: ChatSession) => boolean; } /** * Shortest query that gets a conversation-text sweep. Below it only metadata * is matched, so anything narrating the search scope must read this rather - * than assume every submitted query reached message content. + * than assume every submitted query reached message content. The value lives + * at the API boundary; the hook re-exports it so UI copy reads one policy. */ -export const SESSION_CONTENT_SEARCH_MIN_CHARS = 2; +export const SESSION_CONTENT_SEARCH_MIN_CHARS = SERVER_CONTENT_SEARCH_MIN_CHARS; function searchErrorMessage(error: unknown): string { return formatAcpErrorMessage(error, "Search failed"); @@ -67,12 +78,12 @@ export type SessionSearchProgress = { }; /** - * Whether a query reaches conversation text at all. Sessions are only counted - * toward content coverage when a sweep will really run, so a query too short to - * sweep reports no coverage instead of a vacuous "n of n". + * Whether a query reaches conversation text at all. Content discovery runs + * server-side over the whole store, so length is the only gate — an empty + * loaded slice still discovers matches. */ -function sweepsContent(trimmed: string, targetCount: number): boolean { - return trimmed.length >= SESSION_CONTENT_SEARCH_MIN_CHARS && targetCount > 0; +function sweepsContent(trimmed: string): boolean { + return trimmed.length >= SESSION_CONTENT_SEARCH_MIN_CHARS; } /** What one sweep settled, for callers deciding what may be marked done. */ @@ -93,6 +104,7 @@ export function useSessionSearch({ locale, getDisplayTitle, visibleMetadataOnly, + includeDiscoveredSession, }: UseSessionSearchOptions) { // Optional so provider-less mounts (tests) fall back to uncached exports; // with a client, sweeps share one corpus export per (session, stamp) across @@ -125,6 +137,15 @@ export function useSessionSearch({ const targetedContentIdsRef = useRef>(new Set()); const searchedContentIdsRef = useRef>(new Set()); const unreadableContentIdsRef = useRef>(new Set()); + // Server-matched sessions outside the loaded list, kept across the pages of + // one query so a `searchMore` rebuild cannot drop the rows the query sweep + // discovered. Cleared with the coverage sets on a new query/clear. + const syntheticContentSessionsRef = useRef>( + new Map(), + ); + // Monotonic generation for overlapping sweeps of one query; only the newest + // may commit authoritative state (see runSearchPage). + const sweepGenerationRef = useRef(0); // `search` reads sessions and query through refs so its identity stays // stable across store churn and query state updates: consumers key sweep // effects on that identity, and an unstable callback used to re-fire a full @@ -146,12 +167,14 @@ export function useSessionSearch({ locale, getDisplayTitle, visibleMetadataOnly, + includeDiscoveredSession, }); displayOptionsRef.current = { resolvers, locale, getDisplayTitle, visibleMetadataOnly, + includeDiscoveredSession, }; const syncProgress = useCallback(() => { @@ -168,6 +191,10 @@ export function useSessionSearch({ unreadableContentIdsRef.current = new Set(); }, []); + const resetDiscovered = useCallback(() => { + syntheticContentSessionsRef.current = new Map(); + }, []); + /** * Fold one sweep's reported coverage in. A session that failed to export is * moved out of `searched` — a later retry can promote it back — so the @@ -192,6 +219,7 @@ export function useSessionSearch({ targetSessions: ChatSession[], trimmed: string, messageResults: AcpSessionSearchResult[] = [], + extraSessions?: ChatSession[], ) => { const options = displayOptionsRef.current; return buildSessionSearchResults( @@ -203,12 +231,44 @@ export function useSessionSearch({ locale: options.locale, getDisplayTitle: options.getDisplayTitle, visibleMetadataOnly: options.visibleMetadataOnly, + extraSessions, }, ); }, [], ); + /** + * ChatSessions for the sessions the server matched, looked up against every + * session the caller tracks (not only the current sweep's targets, so a page + * sweep keeps the store's live copies for earlier hits). Sessions the store + * has never loaded are mapped from the server's metadata, cheaply and + * without the workspace backfill, and must pass the caller's + * `includeDiscoveredSession` admission check; archive scope is the caller's + * decision (History's Archived tab admits archived sessions, Cmd-K does not). + */ + const mapContentHitSessions = useCallback( + (matchedInfos: AcpSessionInfo[]): ChatSession[] => { + const include = displayOptionsRef.current.includeDiscoveredSession; + const storeById = new Map( + sessionsRef.current.map((session) => [session.id, session]), + ); + const hitSessions: ChatSession[] = []; + for (const info of matchedInfos) { + const stored = storeById.get(info.sessionId); + if (stored) { + hitSessions.push(stored); + continue; + } + const mapped = chatSessionFromAcpInfo(info); + if (include && !include(mapped)) continue; + hitSessions.push(mapped); + } + return hitSessions; + }, + [], + ); + /** * Metadata matches, applied before the export sweep resolves so title and * filter hits render immediately. Only a new query may clear the screen: for @@ -235,11 +295,18 @@ export function useSessionSearch({ // Re-sweep: keep what is on screen for the sessions still in the list // (merged last, so an existing content match is not downgraded to a // metadata one), add metadata hits for sessions that just joined, and - // drop the ones that left. + // drop the ones that left. Server-discovered rows survive too: the + // fresh answer has not landed, so the last authoritative match set is + // still the best one on screen. + const discoveredIds = new Set(syntheticContentSessionsRef.current.keys()); setResults((current) => mergeSessionSearchResults( metadataResults, - current.filter((result) => sweptSessionIds.has(result.session.id)), + current.filter( + (result) => + sweptSessionIds.has(result.session.id) || + discoveredIds.has(result.session.id), + ), ), ); }, @@ -260,13 +327,38 @@ export function useSessionSearch({ (nextResults: SessionSearchDisplayResult[], searchedIds: Set) => { setResults((current) => { const currentIds = new Set(current.map((result) => result.session.id)); + const enrichedById = new Map( + current + .filter((result) => result.snippet) + .map((result) => [result.session.id, result]), + ); return mergeSessionSearchResults( current.filter((result) => !searchedIds.has(result.session.id)), - nextResults.filter( - (result) => - searchedIds.has(result.session.id) || - !currentIds.has(result.session.id), - ), + nextResults + .filter( + (result) => + searchedIds.has(result.session.id) || + !currentIds.has(result.session.id), + ) + // A still-matching session whose enrichment failed this sweep is + // rebuilt as a snippet-less placeholder; keep the snippet an + // earlier successful read produced rather than degrading the row. + // Message placeholders only: a row that fell back to a metadata + // match must not keep navigating to the old message. + .map((result) => { + if (result.snippet || result.matchType !== "message") { + return result; + } + const enriched = enrichedById.get(result.session.id); + if (!enriched) return result; + return { + ...result, + snippet: enriched.snippet, + messageId: enriched.messageId, + messageRole: enriched.messageRole, + matchCount: enriched.matchCount, + }; + }), ); }); }, @@ -285,7 +377,12 @@ export function useSessionSearch({ targetSessions: ChatSession[]; mode: SweepMode; }): Promise => { - const metadataResults = buildResults(targetSessions, trimmed); + const metadataResults = buildResults( + targetSessions, + trimmed, + [], + [...syntheticContentSessionsRef.current.values()], + ); const targets = targetSessions.map((session) => ({ id: session.id, stamp: sessionSearchStamp(session), @@ -295,11 +392,19 @@ export function useSessionSearch({ setError(null); applyInterimResults(metadataResults, mode, sweptSessionIds); - if (!sweepsContent(trimmed, targets.length)) { - // No content sweep was owed, so there is nothing unread to retry. + // Content search needs no loaded targets: the server answers for the + // whole store, so an empty loaded slice (a project filter or archive tab + // with nothing on screen yet) still discovers matches. + if (!sweepsContent(trimmed)) { return { ok: true, unreadableIds: [] }; } + // Overlapping sweeps of one query (a resweep racing a page load) must + // not let the slower answer overwrite the newer one: each sweep takes a + // generation number, and only the latest generation may commit the + // authoritative match set and its rebuilt rows. + const sweepGeneration = ++sweepGenerationRef.current; + activeSearchesRef.current += 1; setIsSearching(true); @@ -310,20 +415,65 @@ export function useSessionSearch({ if (requestIdRef.current !== requestId) { return { ok: false, unreadableIds: [] }; } + if (sweepGenerationRef.current !== sweepGeneration) { + // A newer sweep of this query owns the commit; this one's answer is + // stale before it is applied. + return { ok: false, unreadableIds: [] }; + } + + // The server's match set is authoritative for content membership: it + // read the whole store, so a session absent from it does not match, + // whatever an earlier sweep (or a failed export) left on screen. + const hitSessions = mapContentHitSessions(sweep.matchedInfos); + const hitIds = new Set(hitSessions.map((session) => session.id)); + const targetIds = new Set(targets.map((target) => target.id)); + + // Union the page's targets with every admitted server match; loaded + // sessions win over mapped copies (live store state). + const extras = hitSessions.filter( + (session) => !targetIds.has(session.id), + ); + const unionSessions = [...targetSessions, ...extras]; + + // Swap the retained match set for the fresh one, remembering the ids + // it replaces: a match the server no longer returns must leave the + // screen, and only naming it here can invalidate its old row. + const previousHitIds = new Set( + syntheticContentSessionsRef.current.keys(), + ); + syntheticContentSessionsRef.current = new Map( + hitSessions.map((session) => [session.id, session]), + ); - // Coverage comes from the sweep, not from the target list: the boundary - // resolves even when individual corpus exports fail, so counting every - // target as searched here is what let the UI claim it had read - // conversations it never opened. recordCoverage(sweep.searchedIds, sweep.failedIds); syncProgress(); + // Every admitted match rows as a content hit; export enrichment only + // overlays snippet/messageId/matchCount where it succeeded. A match + // with no enrichment keeps a snippet-less content row rather than + // vanishing behind a transient export failure. + const enrichmentById = new Map( + sweep.results.map((result) => [result.sessionId, result]), + ); + const messageResults: AcpSessionSearchResult[] = hitSessions.map( + (session) => + enrichmentById.get(session.id) ?? { + sessionId: session.id, + snippet: "", + messageId: "", + matchCount: 0, + }, + ); + + // Replace content state for everything this answer speaks for: every + // target the server searched (matched or not — an unmatched target + // has provably lost any prior content row), every admitted match, and + // every previously retained match (its absence from the fresh set + // removes its row). Rows outside the set survive untouched. applySweptResults( - buildResults(targetSessions, trimmed, sweep.results), - new Set(sweep.searchedIds), + buildResults(unionSessions, trimmed, messageResults), + new Set([...targetIds, ...hitIds, ...previousHitIds]), ); - // Partial coverage still counts as a completed sweep, but the skipped - // sessions travel back so the caller does not mark them done. return { ok: true, unreadableIds: sweep.failedIds }; } catch (searchError) { if (requestIdRef.current !== requestId) { @@ -331,9 +481,6 @@ export function useSessionSearch({ } setError(searchErrorMessage(searchError)); - // Nothing was read, so every target of this sweep is unsearched. The - // ids stay in `targeted`, so a retry promotes them rather than adding - // a second set of denominators. recordCoverage( [], targets.map((target) => target.id), @@ -362,6 +509,7 @@ export function useSessionSearch({ applyInterimResults, applySweptResults, buildResults, + mapContentHitSessions, queryClient, recordCoverage, syncProgress, @@ -374,6 +522,7 @@ export function useSessionSearch({ pendingSessionIdsRef.current = new Set(); activeSearchesRef.current = 0; resetCoverage(); + resetDiscovered(); queryRef.current = ""; submittedSearchRef.current = null; setQuery(""); @@ -382,7 +531,7 @@ export function useSessionSearch({ setIsSearching(false); setError(null); setProgress(null); - }, [resetCoverage]); + }, [resetCoverage, resetDiscovered]); const updateQuery = useCallback( (nextQuery: string) => { @@ -398,6 +547,7 @@ export function useSessionSearch({ pendingSessionIdsRef.current = new Set(); activeSearchesRef.current = 0; resetCoverage(); + resetDiscovered(); // Synced here as well as on render so a submit in the same tick as the // update (setQuery("x"); search()) already sees the new query. queryRef.current = nextQuery; @@ -409,7 +559,7 @@ export function useSessionSearch({ setError(null); setProgress(null); }, - [resetCoverage], + [resetCoverage, resetDiscovered], ); const search = useCallback( @@ -439,10 +589,16 @@ export function useSessionSearch({ ); activeSearchesRef.current = 0; resetCoverage(); + // A new query abandons the previous server answer; a resweep keeps it + // until the fresh one lands, so discovered rows don't blink out (and a + // failed resweep can't lose them). + if (mode === "query") { + resetDiscovered(); + } // A new query targets exactly the sessions it is about to sweep — and // only if it will actually reach conversation text, so a one-character // query reports no content coverage rather than a vacuous "n of n". - if (sweepsContent(trimmed, targetSessions.length)) { + if (sweepsContent(trimmed)) { targetedContentIdsRef.current = new Set( targetSessions.map((session) => session.id), ); @@ -474,7 +630,7 @@ export function useSessionSearch({ } } }, - [clear, resetCoverage, runSearchPage, syncProgress], + [clear, resetCoverage, resetDiscovered, runSearchPage, syncProgress], ); const searchMore = useCallback( @@ -503,7 +659,7 @@ export function useSessionSearch({ } // Union, not addition: a retry of a previously failed page re-adds ids // that are already counted, and `Set` makes that idempotent. - if (sweepsContent(trimmed, unsearchedSessions.length)) { + if (sweepsContent(trimmed)) { for (const session of unsearchedSessions) { targetedContentIdsRef.current.add(session.id); } diff --git a/src/features/sessions/lib/buildSessionSearchResults.ts b/src/features/sessions/lib/buildSessionSearchResults.ts index 70a365712..739e7167c 100644 --- a/src/features/sessions/lib/buildSessionSearchResults.ts +++ b/src/features/sessions/lib/buildSessionSearchResults.ts @@ -7,6 +7,12 @@ interface BuildSessionSearchResultsOptions { locale?: string; getDisplayTitle?: (session: ChatSession) => string; visibleMetadataOnly?: boolean; + /** Sessions matched server-side that are not in the loaded `sessions` list. + * Metadata filtering still runs against the loaded list alone (its + * resolvers only know loaded personas/projects); extras enter purely as + * message matches. Loaded sessions win on id overlap — the store's copy + * carries live state (workspace, pins) the server's metadata lacks. */ + extraSessions?: ChatSession[]; } export interface SessionSearchDisplayResult { @@ -61,7 +67,13 @@ export function buildSessionSearchResults( messageMatches.map((match) => [match.sessionId, match]), ); - return sortByActivityDesc(sessions) + const loadedIds = new Set(sessions.map((session) => session.id)); + const extras = (options.extraSessions ?? []).filter( + (session) => + !loadedIds.has(session.id) && messageMatchesBySessionId.has(session.id), + ); + + return sortByActivityDesc([...sessions, ...extras]) .filter((session) => { return ( metadataMatchIds.has(session.id) || @@ -80,10 +92,13 @@ export function buildSessionSearchResults( return { session, matchType: "message" as const, - snippet: messageMatch.snippet, - messageId: messageMatch.messageId, + // Server-discovered matches arrive with no snippet/messageId until an + // export enriches them; the empty string must not win over the card's + // own preview fallback. + snippet: messageMatch.snippet || undefined, + messageId: messageMatch.messageId || undefined, messageRole: messageMatch.messageRole, - matchCount: messageMatch.matchCount, + matchCount: messageMatch.matchCount || undefined, }; }); } diff --git a/src/features/sessions/lib/sessionListFilters.ts b/src/features/sessions/lib/sessionListFilters.ts index 749658641..81cfeb07a 100644 --- a/src/features/sessions/lib/sessionListFilters.ts +++ b/src/features/sessions/lib/sessionListFilters.ts @@ -39,6 +39,19 @@ export function selectSessionsForScope( ); } +/** + * Whether one session passes a scope tab and project filter — the per-session + * form of `selectSessionsForScope`, for admitting server-discovered search + * results that are not in any loaded list. + */ +export function sessionMatchesScope( + session: ChatSession, + scope: SessionScope, + projectIds: ReadonlySet, +): boolean { + return selectSessionsForScope([session], scope, projectIds).length > 0; +} + /** * Filter sessions to those whose `projectId` is in `projectIds`. * diff --git a/src/features/sessions/ui/SessionHistoryView.tsx b/src/features/sessions/ui/SessionHistoryView.tsx index 73ef13a6d..8849fc6d3 100644 --- a/src/features/sessions/ui/SessionHistoryView.tsx +++ b/src/features/sessions/ui/SessionHistoryView.tsx @@ -63,6 +63,7 @@ import { useForkSession } from "../hooks/useForkSession"; import { useSessionSearch } from "../hooks/useSessionSearch"; import { selectSessionsForScope, + sessionMatchesScope, type SessionScope, } from "../lib/sessionListFilters"; import { @@ -127,6 +128,7 @@ interface SessionHistoryViewProps { sessionId: string, messageId?: string, query?: string, + session?: ChatSession, ) => void; onRenameChat?: (sessionId: string, nextTitle: string) => void; onArchiveChat?: SessionAction; @@ -388,6 +390,8 @@ export function SessionHistoryView({ resolvers, locale: i18n.resolvedLanguage, getDisplayTitle, + includeDiscoveredSession: (session) => + sessionMatchesScope(session, scope, selectedProjectIds), }); const { error: searchError, @@ -724,17 +728,30 @@ export function SessionHistoryView({ ); // Results are only as current as the last sweep, but membership can change // under them without touching the query, the scope, or the project filter — - // restoring a session from the Archived tab is exactly that. Gate them on the - // live session set so a row that no longer belongs cannot linger (still - // offering its Restore action) until something else triggers a resweep. + // restoring a session from the Archived tab is exactly that. Gate loaded rows + // on the live session set so a row that no longer belongs cannot linger + // (still offering its Restore action) until something else triggers a + // resweep. Server-discovered sessions are not in the loaded set; the hook + // already admitted them through the live scope/project filters, and an + // archive/restore flip lands in the store (loaded) or in the next resweep. const activeSessionIds = useMemo( () => new Set(activeSessions.map((session) => session.id)), [activeSessions], ); + const allSessionIds = useMemo( + () => new Set(sessions.map((session) => session.id)), + [sessions], + ); const visibleSearchResults = useMemo( () => - searchResults.filter((result) => activeSessionIds.has(result.session.id)), - [activeSessionIds, searchResults], + searchResults.filter((result) => { + if (activeSessionIds.has(result.session.id)) return true; + // The store knows this session but the current scope excludes it + // (wrong tab, wrong project, archived under the Active tab): the + // server-discovered copy must not smuggle it back in. + return !allSessionIds.has(result.session.id); + }), + [activeSessionIds, allSessionIds, searchResults], ); const searchRows = useMemo( () => flattenFlatSessionRows(visibleSearchResults, columns), @@ -1121,14 +1138,13 @@ export function SessionHistoryView({ }, [setTopBarActions, t, handleTriggerImport, isImporting]); const handleSelectResult = useCallback( - (sessionId: string, messageId?: string) => { - if (messageId) { - onSelectSearchResult?.(sessionId, messageId, submittedQuery); - return; - } - onSelectSession?.(sessionId); + (sessionId: string, messageId?: string, session?: ChatSession) => { + // Search rows go through the search-result path: discovered sessions + // need the caller's pre-activation hydration, which `onSelectSession` + // does not do. + onSelectSearchResult?.(sessionId, messageId, submittedQuery, session); }, - [onSelectSearchResult, onSelectSession, submittedQuery], + [onSelectSearchResult, submittedQuery], ); const renderSessionCard = useCallback( @@ -1138,16 +1154,28 @@ export function SessionHistoryView({ snippet?: string; matchCount?: number; messageId?: string; - isSearchResult?: boolean; + /** Present only for search rows; distinguishes metadata hits from + * content hits. */ + matchType?: "metadata" | "message"; }, ) => { - const isSearchResult = options?.isSearchResult ?? false; + const isSearchResult = options?.matchType !== undefined; const messageId = options?.messageId; + const isMetadataMatch = options?.matchType === "metadata"; + // Server-discovered rows are not in the session store, so store-backed + // mutations (rename, fork, archive/restore) would silently no-op; + // export and open work straight off the session id and stay available. + const isHydrated = allSessionIds.has(session.id); // Browse cards preview the latest message text; search cards show the - // matched snippet instead. - const snippet = isSearchResult - ? options?.snippet - : (session.subtitle ?? undefined); + // matched snippet instead. A server-discovered content match has no + // snippet until its corpus is exported — fall back to the session + // preview. A metadata-only match has no content snippet to show, and + // the preview must not pose as one. + const snippet = isMetadataMatch + ? undefined + : isSearchResult + ? options?.snippet || (session.subtitle ?? undefined) + : (session.subtitle ?? undefined); return ( handleSelectResult(session.id, messageId) + isSearchResult + ? () => handleSelectResult(session.id, messageId, session) : onSelectSession } selected={selectedSessionIds.has(session.id)} @@ -1186,10 +1214,10 @@ export function SessionHistoryView({ selectionCount={selectedCount} onSelectionClear={clearSelection} onSelectionChange={toggleSessionSelection} - onRename={onRenameChat} - onFork={handleFork} - onArchive={handleArchive} - onUnarchive={handleUnarchive} + onRename={isHydrated ? onRenameChat : undefined} + onFork={isHydrated ? handleFork : undefined} + onArchive={isHydrated ? handleArchive : undefined} + onUnarchive={isHydrated ? handleUnarchive : undefined} onUnarchiveSelected={handleUnarchiveSelected} onArchiveSelected={requestArchiveSelected} onExport={handleExport} @@ -1210,6 +1238,7 @@ export function SessionHistoryView({ ); }, [ + allSessionIds, getPersonaName, getProjectColor, getProjectIcon, @@ -1291,7 +1320,7 @@ export function SessionHistoryView({ snippet: result.snippet, matchCount: result.matchCount, messageId: result.messageId, - isSearchResult: true, + matchType: result.matchType, }), )} diff --git a/src/features/sessions/ui/__tests__/SessionHistoryView.test.tsx b/src/features/sessions/ui/__tests__/SessionHistoryView.test.tsx index b07a6cd5d..2ccd14f52 100644 --- a/src/features/sessions/ui/__tests__/SessionHistoryView.test.tsx +++ b/src/features/sessions/ui/__tests__/SessionHistoryView.test.tsx @@ -118,25 +118,33 @@ vi.mock("../SessionCard", () => ({ SessionCard: ({ id, title, + onSelect, onExport, onOpenInWindow, isOpenInWindow, snippet, snippetLineClamp, onSelectionChange, + onArchive, onArchiveSelected, + onFork, + onRename, onUnarchive, onUnarchiveSelected, }: { id: string; title: string; + onSelect?: (id: string) => void; onExport?: (id: string) => void; onOpenInWindow?: (id: string) => void; isOpenInWindow?: boolean; snippet?: string; snippetLineClamp?: 1 | 3; onSelectionChange?: (id: string, selected: boolean) => void; + onArchive?: (id: string) => void; onArchiveSelected?: () => void; + onFork?: (id: string) => void; + onRename?: (id: string, nextTitle: string) => void; onUnarchive?: (id: string) => void; onUnarchiveSelected?: () => void; }) => ( @@ -153,6 +161,24 @@ vi.mock("../SessionCard", () => ({ + + {onArchive ? ( + + ) : null} + {onFork ? ( + + ) : null} + {onRename ? ( + + ) : null} @@ -240,6 +266,25 @@ function scrollHistoryTo(scrollTop: number) { }); } +function historyMatchedInfo(sessionId: string) { + return { + sessionId, + title: "Server match", + updatedAt: "2026-04-12T12:00:00Z", + createdAt: "2026-04-12T12:00:00Z", + lastMessageAt: null, + archivedAt: null, + userSetName: false, + messageCount: 3, + subtitle: null, + workingDir: null, + projectId: null, + providerId: null, + modelId: null, + personaId: null, + }; +} + describe("SessionHistoryView", () => { beforeEach(() => { mocks.sessionWindowSupport.supported = true; @@ -272,13 +317,14 @@ describe("SessionHistoryView", () => { isLoadingMoreSessions: false, loadMoreSessions: undefined, }); - // Coverage is reported per sweep, derived from the targets the boundary was - // handed, so tests never have to restate which sessions a sweep covered. + // Production shape: the server matches every target handed to it here, and + // searchedIds ⊆ matchedInfos (only matched targets are export-enriched). mocks.acpSearchSessions.mockImplementation( async (_query: string, targets: { id: string }[]) => ({ results: [], searchedIds: targets.map((target) => target.id), failedIds: [], + matchedInfos: targets.map((target) => historyMatchedInfo(target.id)), }), ); }); @@ -348,6 +394,13 @@ describe("SessionHistoryView", () => { }), ], }); + // Title match only: the server finds nothing in the message content. + mocks.acpSearchSessions.mockImplementation(async () => ({ + results: [], + searchedIds: [], + failedIds: [], + matchedInfos: [], + })); renderHistory(); @@ -932,6 +985,15 @@ describe("SessionHistoryView", () => { ], }); + // "needle" matches nothing in content; only the archived session's title + // matches, and it is outside the active scope. + mocks.acpSearchSessions.mockImplementation(async () => ({ + results: [], + searchedIds: [], + failedIds: [], + matchedInfos: [], + })); + renderHistory(); await user.type(screen.getByRole("searchbox"), "needle{Enter}"); @@ -966,6 +1028,15 @@ describe("SessionHistoryView", () => { ], }); + // Title match only, so the premise (a metadata hit with no content match + // behind it) does not depend on the default match-everything sweep. + mocks.acpSearchSessions.mockImplementation(async () => ({ + results: [], + searchedIds: [], + failedIds: [], + matchedInfos: [], + })); + renderHistory(); await user.type(screen.getByRole("searchbox"), "needle{Enter}"); @@ -1287,3 +1358,146 @@ describe("SessionHistoryView", () => { }); }); }); + +describe("server-discovered matches", () => { + function matchedInfo(sessionId: string, title: string) { + return { + sessionId, + title, + updatedAt: "2026-04-12T12:00:00Z", + createdAt: "2026-04-12T12:00:00Z", + lastMessageAt: null, + archivedAt: null, + userSetName: false, + messageCount: 3, + subtitle: null, + workingDir: null, + projectId: null, + providerId: null, + modelId: null, + personaId: null, + }; + } + + it("renders a content match for a session that is not loaded", async () => { + const user = userEvent.setup(); + setSessionStoreState({ sessions: [session()] }); + mocks.acpSearchSessions.mockImplementation(async () => ({ + results: [], + // Production searchedIds ⊆ matchedInfos: only server-matched targets + // are export-enriched. + searchedIds: [], + failedIds: [], + matchedInfos: [matchedInfo("old-1", "Old Needle Chat")], + })); + + renderHistory(); + + await user.type(screen.getByRole("searchbox"), "needle{Enter}"); + + expect(await screen.findByText("Old Needle Chat")).toBeInTheDocument(); + }); + + it("excludes a discovered match the project filter rules out", async () => { + const user = userEvent.setup(); + projectState.projects = [ + { id: "project-a", name: "Project A", workingDirs: ["/a"] }, + ]; + setSessionStoreState({ + sessions: [session({ projectId: "project-a" })], + }); + mocks.acpSearchSessions.mockImplementation(async () => ({ + results: [], + searchedIds: [], + failedIds: [], + matchedInfos: [ + // An admitted in-project discovery alongside the excluded one, so the + // test proves the response was applied rather than ignored wholesale. + { ...matchedInfo("old-2", "Project A Needle"), projectId: "project-a" }, + { + ...matchedInfo("old-1", "Other Project Needle"), + projectId: "other", + }, + ], + })); + + renderHistory(); + + await user.click(screen.getByRole("button", { name: "All projects" })); + await user.click( + screen.getByRole("menuitemcheckbox", { name: "Project A" }), + ); + await user.keyboard("{Escape}"); + + await user.type(screen.getByRole("searchbox"), "needle{Enter}"); + + expect(await screen.findByText("Project A Needle")).toBeInTheDocument(); + expect(screen.queryByText("Other Project Needle")).not.toBeInTheDocument(); + }); + + it("routes a discovered row's selection through onSelectSearchResult with its session", async () => { + const user = userEvent.setup(); + const onSelectSession = vi.fn(); + const onSelectSearchResult = vi.fn(); + setSessionStoreState({ sessions: [session()] }); + mocks.acpSearchSessions.mockImplementation(async () => ({ + results: [], + searchedIds: [], + failedIds: [], + matchedInfos: [matchedInfo("old-1", "Old Needle Chat")], + })); + + render( + , + ); + + await user.type(screen.getByRole("searchbox"), "needle{Enter}"); + await user.click( + await screen.findByRole("button", { name: "Open Old Needle Chat" }), + ); + + // Discovered rows must not use plain session selection: the caller needs + // the row's session to hydrate the store before activating. + expect(onSelectSession).not.toHaveBeenCalled(); + expect(onSelectSearchResult).toHaveBeenCalledWith( + "old-1", + undefined, + "needle", + expect.objectContaining({ id: "old-1", title: "Old Needle Chat" }), + ); + }); + + it("hides store-backed actions on a discovered row but keeps export and open", async () => { + const user = userEvent.setup(); + setSessionStoreState({ sessions: [session()] }); + mocks.acpSearchSessions.mockImplementation(async () => ({ + results: [], + searchedIds: [], + failedIds: [], + matchedInfos: [matchedInfo("old-1", "Old Needle Chat")], + })); + + renderHistory(); + + await user.type(screen.getByRole("searchbox"), "needle{Enter}"); + await screen.findByText("Old Needle Chat"); + + expect( + screen.getByRole("button", { name: "Open Old Needle Chat" }), + ).toBeInTheDocument(); + // Not in the store: rename/fork/archive would silently no-op, so the row + // must not offer them. + expect( + screen.queryByRole("button", { name: "Rename Old Needle Chat" }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Fork Old Needle Chat" }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Archive Old Needle Chat" }), + ).not.toBeInTheDocument(); + }); +}); diff --git a/src/shared/api/__tests__/acp.test.ts b/src/shared/api/__tests__/acp.test.ts index 31b8d224e..3c66393c8 100644 --- a/src/shared/api/__tests__/acp.test.ts +++ b/src/shared/api/__tests__/acp.test.ts @@ -146,7 +146,7 @@ vi.mock("../acpActiveMessageTracking", () => ({ })); vi.mock("../sessionSearch", () => ({ - searchSessionsViaExports: vi.fn(), + searchSessions: vi.fn(), })); describe("acpSteerMessage", () => { diff --git a/src/shared/api/__tests__/acpApi.test.ts b/src/shared/api/__tests__/acpApi.test.ts index 3f5e739fe..323d7728d 100644 --- a/src/shared/api/__tests__/acpApi.test.ts +++ b/src/shared/api/__tests__/acpApi.test.ts @@ -199,6 +199,37 @@ describe("listSessionsPage", () => { }); }); + it("sends the keyword filter as top-level _meta.query", async () => { + mocks.listSessions.mockResolvedValueOnce({ + sessions: [], + nextCursor: null, + }); + + const { listSessionsPage } = await import("../acpApi"); + + await expect( + listSessionsPage({ query: " refactor plan " }), + ).resolves.toEqual({ sessions: [], nextCursor: null }); + expect(mocks.listSessions).toHaveBeenCalledWith({ + _meta: { + goose: { includeLastMessageSnippet: true }, + query: "refactor plan", + }, + }); + }); + + it("omits the keyword filter for blank or absent queries", async () => { + mocks.listSessions.mockResolvedValue({ sessions: [], nextCursor: null }); + + const { listSessionsPage } = await import("../acpApi"); + + await listSessionsPage({ query: " " }); + await listSessionsPage(); + for (const call of mocks.listSessions.mock.calls) { + expect(call[0]).toEqual(includeLastMessageSnippetMeta); + } + }); + it("trims the cursor and maps session info", async () => { mocks.listSessions.mockResolvedValueOnce({ sessions: [ diff --git a/src/shared/api/__tests__/sessionSearch.test.ts b/src/shared/api/__tests__/sessionSearch.test.ts index 4076e2d6a..abc0afacb 100644 --- a/src/shared/api/__tests__/sessionSearch.test.ts +++ b/src/shared/api/__tests__/sessionSearch.test.ts @@ -2,12 +2,18 @@ import { QueryClient } from "@tanstack/react-query"; import { beforeEach, describe, expect, it, vi } from "vitest"; const mockExportSession = vi.hoisted(() => vi.fn()); +const mockListSessionsPage = vi.hoisted(() => vi.fn()); vi.mock("../acpApi", () => ({ exportSession: mockExportSession, + listSessionsPage: (...args: unknown[]) => mockListSessionsPage(...args), })); -import { searchSessionsViaExports, sessionSearchStamp } from "../sessionSearch"; +import { + searchSessions, + searchSessionsViaExports, + sessionSearchStamp, +} from "../sessionSearch"; function exportedNeedleConversation(sessionId: string): string { return JSON.stringify({ @@ -313,3 +319,187 @@ describe("searchSessionsViaExports", () => { expect(retried.failedIds).toEqual([]); }); }); + +function serverSession( + sessionId: string, + overrides: Record = {}, +) { + return { + sessionId, + title: `Session ${sessionId}`, + updatedAt: "2026-04-10T12:00:00Z", + createdAt: null, + lastMessageAt: null, + archivedAt: null, + userSetName: false, + messageCount: 2, + subtitle: null, + workingDir: null, + projectId: null, + providerId: null, + modelId: null, + personaId: null, + ...overrides, + }; +} + +describe("searchSessions", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("does not hit the server for queries below the content threshold", async () => { + await expect( + searchSessions("n", [{ id: "session-1", stamp: "v1" }]), + ).resolves.toEqual({ + results: [], + searchedIds: [], + failedIds: [], + matchedInfos: [], + }); + expect(mockListSessionsPage).not.toHaveBeenCalled(); + expect(mockExportSession).not.toHaveBeenCalled(); + }); + + it("discovers matches server-side across pages and enriches only matched targets", async () => { + mockListSessionsPage + .mockResolvedValueOnce({ + sessions: [serverSession("session-1"), serverSession("old-1")], + nextCursor: "cursor-2", + }) + .mockResolvedValueOnce({ + sessions: [serverSession("old-2")], + nextCursor: null, + }); + mockExportSession.mockResolvedValue( + exportedNeedleConversation("session-1"), + ); + + const sweep = await searchSessions("needle", [ + { id: "session-1", stamp: "v1" }, + { id: "session-2", stamp: "v1" }, + ]); + + // The query filter rides every page request, and pagination follows the + // returned cursor until the server stops handing one out. + expect(mockListSessionsPage).toHaveBeenNthCalledWith(1, { + cursor: null, + query: "needle", + }); + expect(mockListSessionsPage).toHaveBeenNthCalledWith(2, { + cursor: "cursor-2", + query: "needle", + }); + + // Every server match — including sessions outside the loaded targets — + // comes back in matchedInfos so the caller can surface them. + expect(sweep.matchedInfos?.map((info) => info.sessionId)).toEqual([ + "session-1", + "old-1", + "old-2", + ]); + + // Only matched targets get an export; the unmatched one is already + // answered by the server and must not pay for a corpus read. + expect(mockExportSession).toHaveBeenCalledTimes(1); + expect(mockExportSession).toHaveBeenCalledWith("session-1"); + expect(sweep.results).toMatchObject([ + { sessionId: "session-1", matchCount: 1 }, + ]); + expect(sweep.searchedIds).toEqual(["session-1"]); + expect(sweep.failedIds).toEqual([]); + }); + + it("matches any whitespace-separated keyword, mirroring the server filter", async () => { + mockListSessionsPage.mockResolvedValueOnce({ + sessions: [serverSession("session-1")], + nextCursor: null, + }); + mockExportSession.mockResolvedValue( + JSON.stringify({ + conversation: [ + { + id: "m1", + role: "user", + content: "only the second word appears here", + }, + ], + }), + ); + + // The full phrase never appears, but the server's OR over words matched + // this session — the enrichment must agree or the match would be erased. + const sweep = await searchSessions("missing word", [ + { id: "session-1", stamp: "v1" }, + ]); + + expect(sweep.results).toMatchObject([ + { sessionId: "session-1", matchCount: 1 }, + ]); + expect(sweep.failedIds).toEqual([]); + }); + + it("reports matched targets whose export fails as unread, keeping the match", async () => { + mockListSessionsPage.mockResolvedValueOnce({ + sessions: [serverSession("session-1")], + nextCursor: null, + }); + mockExportSession.mockRejectedValue(new Error("export failed")); + + const sweep = await searchSessions("needle", [ + { id: "session-1", stamp: "v1" }, + ]); + + expect(sweep.results).toEqual([]); + expect(sweep.failedIds).toEqual(["session-1"]); + // The server already established the match; matchedInfos carries it so + // the caller can degrade to a snippet-less row instead of hiding it. + expect(sweep.matchedInfos?.map((info) => info.sessionId)).toEqual([ + "session-1", + ]); + }); + + it("fails on a repeated pagination cursor instead of looping", async () => { + mockListSessionsPage.mockResolvedValue({ + sessions: [serverSession("loop-1")], + nextCursor: "cursor-forever", + }); + + // A server that hands back the same cursor is cycling; the search must + // error rather than storm requests or treat duplicates as the full set. + await expect(searchSessions("needle", [])).rejects.toThrow( + "repeated pagination cursor", + ); + expect(mockListSessionsPage).toHaveBeenCalledTimes(2); + }); + + it("fails rather than truncate when the page cap is reached", async () => { + let page = 0; + mockListSessionsPage.mockImplementation(async () => { + page += 1; + return { + sessions: [serverSession(`session-${page}`)], + nextCursor: `cursor-${page}`, + }; + }); + + await expect(searchSessions("needle", [])).rejects.toThrow( + "exceeded 100 pages", + ); + expect(mockListSessionsPage).toHaveBeenCalledTimes(100); + }); + + it("propagates a mid-pagination failure", async () => { + mockListSessionsPage + .mockResolvedValueOnce({ + sessions: [serverSession("session-1")], + nextCursor: "cursor-2", + }) + .mockRejectedValueOnce(new Error("connection closed")); + + await expect(searchSessions("needle", [])).rejects.toThrow( + "connection closed", + ); + expect(mockExportSession).not.toHaveBeenCalled(); + }); +}); diff --git a/src/shared/api/acp.ts b/src/shared/api/acp.ts index 4817b03fb..5edd92628 100644 --- a/src/shared/api/acp.ts +++ b/src/shared/api/acp.ts @@ -18,7 +18,7 @@ import { clearActiveMessageId, } from "./acpActiveMessageTracking"; import { - searchSessionsViaExports, + searchSessions, type SessionSearchOptions, type SessionSearchTarget, } from "./sessionSearch"; @@ -527,23 +527,37 @@ export interface AcpSessionSearchSweep { results: AcpSessionSearchResult[]; searchedIds: string[]; failedIds: string[]; + /** Metadata of every session whose message content matched the query on the + * server — including sessions not among `targets`, so callers can surface + * matches beyond the sessions currently loaded in the renderer. Empty when + * nothing matched or no server-side discovery ran (short query). */ + matchedInfos: AcpSessionInfo[]; } /** List one page of sessions known to the goose binary. */ export async function acpListSessionsPage({ cursor, + query, }: { cursor?: string | null; + query?: string | null; } = {}): Promise { - return directAcp.listSessionsPage({ cursor }); + return directAcp.listSessionsPage({ cursor, query }); } +/** + * Search session content. A query that meets the content-search threshold is + * discovered server-side (goose's `_meta.query` SQL filter over message text, + * cursor-paginated) so the whole session store is covered; `targets` are then + * export-swept for snippet/match-count enrichment and coverage. Below the + * threshold no content search runs at all and the sweep is empty. + */ export async function acpSearchSessions( query: string, targets: SessionSearchTarget[], options: SessionSearchOptions = {}, ): Promise { - return searchSessionsViaExports(query, targets, options); + return searchSessions(query, targets, options); } /** diff --git a/src/shared/api/acpApi.ts b/src/shared/api/acpApi.ts index 497de6a33..ed2bdcf43 100644 --- a/src/shared/api/acpApi.ts +++ b/src/shared/api/acpApi.ts @@ -63,6 +63,16 @@ const LIST_SESSIONS_META = { }, } satisfies NonNullable; +/** `_meta` for a keyword-filtered session/list: goose reads top-level + * `_meta.query` as a whitespace-split, case-insensitive keyword OR over + * message text, so discovery runs server-side over the whole session store + * instead of only the sessions the renderer has loaded. Case folding is + * SQLite `LOWER()` — ASCII-only, so "CAFÉ" will not match a query of "café"; + * a Unicode-aware collation belongs to goose, not this client. */ +function listSessionsMeta(query: string): ListSessionsRequest["_meta"] { + return { ...LIST_SESSIONS_META, query }; +} + export async function listProviders(): Promise { return getCuratedAgentProviders(); } @@ -132,16 +142,23 @@ export async function getSessionInfo( export async function listSessionsPage({ cursor, + query, }: { cursor?: string | null; + /** Keyword filter for goose's server-side message-content search + * (`_meta.query`). Only set when searching; omit for plain listing. */ + query?: string | null; } = {}): Promise { const client = await getClient(); const normalizedCursor = cursor?.trim() || null; + const normalizedQuery = query?.trim() || null; // ACP session/list only standardizes cwd and cursor filters. Goose project // membership lives in _meta.projectId, so callers must paginate globally and // group by projectId client-side instead of using cwd as a proxy. const params: ListSessionsRequest = { - _meta: LIST_SESSIONS_META, + _meta: normalizedQuery + ? listSessionsMeta(normalizedQuery) + : LIST_SESSIONS_META, }; if (normalizedCursor != null) { params.cursor = normalizedCursor; diff --git a/src/shared/api/sessionSearch.ts b/src/shared/api/sessionSearch.ts index 6bff41fbe..a86b94c4f 100644 --- a/src/shared/api/sessionSearch.ts +++ b/src/shared/api/sessionSearch.ts @@ -1,5 +1,5 @@ import type { QueryClient } from "@tanstack/react-query"; -import { exportSession } from "./acpApi"; +import { exportSession, listSessionsPage, type AcpSessionInfo } from "./acpApi"; const SNIPPET_PREFIX = 40; const SNIPPET_SUFFIX = 60; @@ -69,6 +69,15 @@ export interface SessionSearchSweep { failedIds: string[]; } +/** The full-store sweep: an export sweep plus the server-discovered match + * set. `matchedInfos` covers every session whose message content matched the + * query on the server — including sessions outside `targets`, so callers can + * surface matches beyond the sessions currently loaded in the renderer. + * Empty when nothing matched or no server-side discovery ran (short query). */ +export interface SessionSearchStoreSweep extends SessionSearchSweep { + matchedInfos: AcpSessionInfo[]; +} + interface ParsedMessage { id: string; role: MessageRole | null; @@ -103,6 +112,17 @@ export interface SessionSearchOptions { queryClient?: QueryClient; } +/** Minimum query length for server-side content search. Re-exported by the + * search hook as `SESSION_CONTENT_SEARCH_MIN_CHARS` so the API boundary, the + * hook, and the status line share one policy. */ +export const SERVER_CONTENT_SEARCH_MIN_CHARS = 2; + +/** Safety bound on server-driven page walks: a backend that keeps handing out + * cursors must not turn one search into an unbounded request loop. Generous + * against the 50-row server page size; reaching it with a cursor pending is + * treated as an error (see `searchSessions`), never as a complete answer. */ +const MAX_SERVER_SEARCH_PAGES = 100; + export async function searchSessionsViaExports( query: string, targets: SessionSearchTarget[], @@ -119,6 +139,14 @@ export async function searchSessionsViaExports( unique.push(target); } + // Match semantics mirror goose's server-side keyword filter — the query is + // split on whitespace and a text matches when ANY word appears — so export + // enrichment agrees with server discovery. Words are deduped so "foo foo" + // does not double-count its occurrences. + const needles = [ + ...new Set(trimmed.toLowerCase().split(/\s+/).filter(Boolean)), + ]; + const results: (SessionSearchResult | null)[] = unique.map(() => null); // Per-target coverage, kept positionally so concurrent workers never race: // a slot is written only by the worker that claimed that index. @@ -132,7 +160,7 @@ export async function searchSessionsViaExports( const target = unique[index]; try { const messages = await fetchCorpus(target, options.queryClient); - results[index] = searchSession(target.id, messages, trimmed); + results[index] = searchSession(target.id, messages, needles); } catch { // A session whose corpus cannot be read is not a session without // matches. Record it so callers can say so instead of counting it as @@ -161,6 +189,79 @@ export async function searchSessionsViaExports( }; } +/** + * Full session-store content search. Discovery runs server-side: goose's + * `session/list` `_meta.query` filter is a SQL keyword match over message + * text, cursor-paginated, so matches surface no matter how far back they sit + * — the old export-every-loaded-session sweep could never leave the first + * page. The export sweep then runs only over the page targets that actually + * matched, purely to enrich them with snippet/messageId/matchCount (and to + * keep coverage honest); a target whose export fails degrades to a snippet-less + * row instead of vanishing. Matches outside `targets` travel in + * `matchedInfos` for the caller to render with generic content rows. + */ +export async function searchSessions( + query: string, + targets: SessionSearchTarget[], + options: SessionSearchOptions = {}, +): Promise { + const trimmed = query.trim(); + if (trimmed.length < SERVER_CONTENT_SEARCH_MIN_CHARS) { + return { results: [], searchedIds: [], failedIds: [], matchedInfos: [] }; + } + + const matchedInfos: AcpSessionInfo[] = []; + const seenCursors = new Set(); + let cursor: string | null = null; + for (let page = 0; page < MAX_SERVER_SEARCH_PAGES; page += 1) { + const { sessions, nextCursor } = await listSessionsPage({ + cursor, + query: trimmed, + }); + matchedInfos.push(...sessions); + if (!nextCursor) { + cursor = null; + break; + } + // A cursor the server has already handed out means the walk is cycling; + // continuing would duplicate matches and never terminate honestly. + if (seenCursors.has(nextCursor)) { + throw new Error("session/list returned a repeated pagination cursor"); + } + seenCursors.add(nextCursor); + cursor = nextCursor; + } + // The walk must end on a null cursor: a truncated match set cannot be the + // authoritative full-store answer the caller treats it as. + if (cursor !== null) { + throw new Error( + `session/list search exceeded ${MAX_SERVER_SEARCH_PAGES} pages`, + ); + } + + // Match the server's keyword semantics client-side before spending exports: + // goose splits the query on whitespace and ORs the words as substrings, so a + // multi-word query matches sessions no single full-string sweep would find — + // and the client substring check would then erase a real server match. + const matchedTargetIds = new Set(matchedInfos.map((info) => info.sessionId)); + const matchedTargets = targets.filter((target) => + matchedTargetIds.has(target.id), + ); + const enrichment = + matchedTargets.length > 0 + ? await searchSessionsViaExports(trimmed, matchedTargets, { + queryClient: options.queryClient, + }) + : { results: [], searchedIds: [], failedIds: [] }; + + // Evict superseded corpora for every target, not only the exported few: a + // session that stopped matching never reaches the sweep above, but its old + // stamp's corpus is just as dead. + if (options.queryClient) evictSupersededCorpora(options.queryClient, targets); + + return { ...enrichment, matchedInfos }; +} + /** * Drops the corpora of stamps this sweep superseded. Once a session's stamp * changes nothing will ever read its old corpus again, so leaving it to gc @@ -230,7 +331,7 @@ async function exportCorpus(sessionId: string): Promise { function searchSession( sessionId: string, messages: ParsedMessage[], - query: string, + needles: string[], ): SessionSearchResult | null { if (!messages.length) return null; @@ -243,14 +344,16 @@ function searchSession( for (const msg of messages) { for (const text of msg.texts) { - const count = countMatches(text, query); - if (!count) continue; - matchCount += count; - firstMatch ??= { - messageId: msg.id, - role: msg.role, - snippet: buildSnippet(text, query), - }; + for (const needle of needles) { + const count = countMatches(text, needle); + if (!count) continue; + matchCount += count; + firstMatch ??= { + messageId: msg.id, + role: msg.role, + snippet: buildSnippet(text, needle), + }; + } } }