From 2d30963f6fed97ca8ff61696a5f6d6742d12c28b Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Fri, 18 Sep 2026 15:22:20 -0400 Subject: [PATCH] fix(session-search): answer consent and readiness before an unknown scope The registry short-circuited an unresolvable scope before current.search ran, and current.search is where disabled and not-ready are decided. A host with indexing off that lacks the project told the user it did not have the workspace, which they cannot act on. The verdict now travels to the service beside the request, and the service answers it after its own checks. --- .../session-search-child-service.test.ts | 7 ++- .../session-search-child-service.ts | 4 +- .../session-search-in-process-service.ts | 2 +- .../session-search-instance.test.ts | 45 +++++++++++++++ .../session-search-instance.ts | 8 ++- .../session-search-scope-entry-points.test.ts | 14 +++-- .../session-search-scope-resolution.ts | 9 +-- .../session-search-scope-service.test.ts | 57 ++++++++++++------- .../session-search-service-registry.ts | 49 ++++------------ .../ai-vault-search/session-search-service.ts | 39 ++++++++----- .../session-scanner-service-protocol.ts | 5 +- .../session-scanner-service-search.ts | 2 +- .../ai-vault/session-scanner-service-spawn.ts | 5 +- src/shared/ai-vault-search-test-fixture.ts | 5 +- 14 files changed, 150 insertions(+), 101 deletions(-) diff --git a/src/main/ai-vault-search/session-search-child-service.test.ts b/src/main/ai-vault-search/session-search-child-service.test.ts index 29f436ca857..af3a23442f8 100644 --- a/src/main/ai-vault-search/session-search-child-service.test.ts +++ b/src/main/ai-vault-search/session-search-child-service.test.ts @@ -24,12 +24,13 @@ it('forwards every call to the child and returns what it answered', async () => const calls = stubCalls() const service = createChildSessionSearchService(calls) - expect(await service.search({ query: 'ledger' }, ['/work/app'])).toEqual({ + const hostScope = { kind: 'resolved', paths: ['/work/app'] } as const + expect(await service.search({ query: 'ledger' }, hostScope)).toEqual({ kind: 'unavailable', reason: 'disabled' }) - // Host-resolved paths ride beside the request, never inside it. - expect(calls.search).toHaveBeenCalledWith({ query: 'ledger' }, ['/work/app']) + // The scope verdict rides beside the request, never inside it. + expect(calls.search).toHaveBeenCalledWith({ query: 'ledger' }, hostScope) expect(await service.status()).toEqual(indexingStatus) await service.reconcile() expect(calls.reconcile).toHaveBeenCalledTimes(1) diff --git a/src/main/ai-vault-search/session-search-child-service.ts b/src/main/ai-vault-search/session-search-child-service.ts index de50657ffe8..92ad066d417 100644 --- a/src/main/ai-vault-search/session-search-child-service.ts +++ b/src/main/ai-vault-search/session-search-child-service.ts @@ -24,9 +24,9 @@ export function createChildSessionSearchService( } ): SessionSearchService { return { - search: async (request, hostScopePaths) => { + search: async (request, hostScope) => { try { - return await calls.search(request, hostScopePaths) + return await calls.search(request, hostScope) } catch { return { kind: 'unavailable', reason: 'not-ready' } } diff --git a/src/main/ai-vault-search/session-search-in-process-service.ts b/src/main/ai-vault-search/session-search-in-process-service.ts index 0180eefe4f4..8cb0adbe3e4 100644 --- a/src/main/ai-vault-search/session-search-in-process-service.ts +++ b/src/main/ai-vault-search/session-search-in-process-service.ts @@ -40,7 +40,7 @@ export function installInProcessSessionSearchService(args: { }) instance.apply(args.settings) setSessionSearchService({ - search: (request, hostScopePaths) => instance.search(request, hostScopePaths), + search: (request, hostScope) => instance.search(request, hostScope), status: async () => instance.status(), reconcile: () => instance.reconcile() }) diff --git a/src/main/ai-vault-search/session-search-instance.test.ts b/src/main/ai-vault-search/session-search-instance.test.ts index 3f0c4ff3880..40aa3bc6ff2 100644 --- a/src/main/ai-vault-search/session-search-instance.test.ts +++ b/src/main/ai-vault-search/session-search-instance.test.ts @@ -214,3 +214,48 @@ it('keeps pagination stable when the clock crosses retention before a purge', as expect(second.hits[0].sessionId).not.toBe(first.hits[0].sessionId) expect(errors).toEqual([]) }) + +// An unresolvable scope is the host's last word, not its first: consent and +// readiness are what the reader can act on, so they have to answer first. +it('reports being switched off before blaming a scope it does not know', async () => { + const subject = newInstance() + subject.apply({ enabled: false, historyDays: null }) + await subject.settled() + + expect(await subject.search({ query: 'anything' }, { kind: 'unknown' })).toEqual({ + kind: 'unavailable', + reason: 'disabled' + }) +}) + +it('reports not being ready before blaming a scope it does not know', async () => { + const subject = newInstance() + subject.apply({ enabled: true, historyDays: null }) + await subject.settled() + // Consent stands while no index does, which is what `not-ready` names. + subject.close() + + expect(await subject.search({ query: 'anything' }, { kind: 'unknown' })).toEqual({ + kind: 'unavailable', + reason: 'not-ready' + }) +}) + +it('answers scope-unknown once it is switched on and ready', async () => { + await writeClaudeTranscript( + transcriptPath(RECENT_SESSION_ID), + ['a distinctive conversation'], + RECENT_SESSION_ID + ) + const subject = newInstance() + subject.apply({ enabled: true, historyDays: null }) + await subject.settled() + + expect(await subject.search({ query: 'distinctive' }, { kind: 'unknown' })).toEqual({ + kind: 'unavailable', + reason: 'scope-unknown' + }) + // The same query answers with hits when no scope is in the way, so the refusal + // above is the scope's and not an empty or unreadable index. + expect(await searchFor('distinctive')).toEqual([RECENT_SESSION_ID]) +}) diff --git a/src/main/ai-vault-search/session-search-instance.ts b/src/main/ai-vault-search/session-search-instance.ts index a6d7e835689..c9db351fa6d 100644 --- a/src/main/ai-vault-search/session-search-instance.ts +++ b/src/main/ai-vault-search/session-search-instance.ts @@ -14,7 +14,7 @@ import type { SessionSearchScanRoots } from './session-search-scan-roots' import type { SessionSearchIndexerOptions } from './session-search-indexer-options' import { createSessionSearchService, - type SessionSearchHostScopePaths, + type SessionSearchHostScope, type SessionSearchService } from './session-search-service' @@ -83,13 +83,15 @@ export class SessionSearchInstance { async search( request: AiVaultSearchRequest, - hostScopePaths?: SessionSearchHostScopePaths + hostScope?: SessionSearchHostScope ): Promise { const live = this.live + // Consent and readiness first, for a scoped request exactly as for an + // unscoped one: a host the user can switch on must say so, not blame a scope. if (!live) { return { kind: 'unavailable', reason: this.settings.enabled ? 'not-ready' : 'disabled' } } - return live.service.search(request, hostScopePaths) + return live.service.search(request, hostScope) } status(): AiVaultSearchStatus { diff --git a/src/main/ai-vault-search/session-search-scope-entry-points.test.ts b/src/main/ai-vault-search/session-search-scope-entry-points.test.ts index 2e445b62d9e..52718c98f8a 100644 --- a/src/main/ai-vault-search/session-search-scope-entry-points.test.ts +++ b/src/main/ai-vault-search/session-search-scope-entry-points.test.ts @@ -78,14 +78,16 @@ describe('every search entry point carries the scope identity through', () => { }) }) - it('answers scope-unknown over the relay, which carries no repo catalog of its own', async () => { + it('hands the relay’s own verdict down, that host carrying no repo catalog', async () => { const service = fakeSearchService() - service.status.mockResolvedValue({ ...(await service.status()), enabled: true }) setSessionSearchService(service) - expect(await relayHandler()({ query: 'needle', within: WITHIN })).toEqual({ - kind: 'unavailable', - reason: 'scope-unknown' - }) + await relayHandler()({ query: 'needle', within: WITHIN }) + expect(service.search).toHaveBeenCalledWith( + { query: 'needle', limit: 20 }, + { + kind: 'unknown' + } + ) }) }) diff --git a/src/main/ai-vault-search/session-search-scope-resolution.ts b/src/main/ai-vault-search/session-search-scope-resolution.ts index c7e69ab67c2..f94369705f6 100644 --- a/src/main/ai-vault-search/session-search-scope-resolution.ts +++ b/src/main/ai-vault-search/session-search-scope-resolution.ts @@ -8,19 +8,16 @@ import { } from '../../shared/worktree/id' import { areRuntimePathsEqual } from '../../shared/worktree/ownership' import type { SessionSearchScopeCatalog } from './session-search-scope-catalog' +import type { SessionSearchHostScope } from './session-search-service' import { managedWorktreeDirectories, ScopePathSet } from './session-search-scope-paths' -export type SessionSearchScopeResolution = - | { kind: 'resolved'; paths: string[] } - /** This host has no such workspace or project. Never a reason to search everything. */ - | { kind: 'unknown' } - type ScopeRepo = SessionSearchScopeCatalog['repos'][number] +/** `unknown` says this host has no such workspace or project. Never a reason to search everything. */ export function resolveSessionSearchScope( within: AiVaultSearchScopeIdentity, catalog: SessionSearchScopeCatalog | null -): SessionSearchScopeResolution { +): SessionSearchHostScope { if (!catalog) { return { kind: 'unknown' } } diff --git a/src/main/ai-vault-search/session-search-scope-service.test.ts b/src/main/ai-vault-search/session-search-scope-service.test.ts index 32f4583506a..ab6fc355225 100644 --- a/src/main/ai-vault-search/session-search-scope-service.test.ts +++ b/src/main/ai-vault-search/session-search-scope-service.test.ts @@ -30,7 +30,10 @@ describe('scope identity at the search choke point', () => { 'ipc' ) // Beside the request, not inside `filters.scopePaths`, which carries a wire cap. - expect(service.search).toHaveBeenCalledWith({ query: 'needle', limit: 20 }, ['/work/app']) + expect(service.search).toHaveBeenCalledWith( + { query: 'needle', limit: 20 }, + { kind: 'resolved', paths: ['/work/app'] } + ) expect(response).toMatchObject({ resolvedWithin: true }) }) @@ -43,35 +46,44 @@ describe('scope identity at the search choke point', () => { 'ipc' ) // An exact match, so a leaked `within` would fail here as an extra key. - expect(service.search).toHaveBeenCalledWith({ query: 'needle', limit: 20 }, [ - '/work/app', - '/home/me/orca/workspaces/app' - ]) + expect(service.search).toHaveBeenCalledWith( + { query: 'needle', limit: 20 }, + { kind: 'resolved', paths: ['/work/app', '/home/me/orca/workspaces/app'] } + ) }) - it('answers scope-unknown rather than searching everything it has', async () => { + it('hands an unresolvable scope to the service rather than answering for it', async () => { const service = fakeSearchService() setSessionSearchService(service) installSessionSearchScopeCatalogSource(() => CATALOG) - expect( - await searchSessionService( - { query: 'needle', within: { kind: 'project', projectKey: 'repo:elsewhere' } }, - 'ipc' - ) - ).toEqual({ kind: 'unavailable', reason: 'scope-unknown' }) - expect(service.search).not.toHaveBeenCalled() + await searchSessionService( + { query: 'needle', within: { kind: 'project', projectKey: 'repo:elsewhere' } }, + 'ipc' + ) + // The service owns the answer, because it owns the consent and readiness + // checks that have to come first. + expect(service.search).toHaveBeenCalledWith( + { query: 'needle', limit: 20 }, + { + kind: 'unknown' + } + ) + expect(service.status).not.toHaveBeenCalled() }) - it('answers scope-unknown on a host with no catalog at all, such as the relay', async () => { + it('says unknown on a host with no catalog at all, such as the relay', async () => { const service = fakeSearchService() setSessionSearchService(service) - expect( - await searchSessionService( - { query: 'needle', within: { kind: 'workspace', worktreeId: 'repo-1::/work/app' } }, - 'ipc' - ) - ).toEqual({ kind: 'unavailable', reason: 'scope-unknown' }) - expect(service.status).not.toHaveBeenCalled() + await searchSessionService( + { query: 'needle', within: { kind: 'workspace', worktreeId: 'repo-1::/work/app' } }, + 'ipc' + ) + expect(service.search).toHaveBeenCalledWith( + { query: 'needle', limit: 20 }, + { + kind: 'unknown' + } + ) }) it('carries more paths than the request field could hold, and the request still re-parses', async () => { @@ -91,7 +103,8 @@ describe('scope identity at the search choke point', () => { 'ipc' ) const call = service.search.mock.lastCall - expect(call?.[1]).toHaveLength(101) + expect(call?.[1]).toMatchObject({ kind: 'resolved', paths: expect.any(Array) }) + expect(Object(call?.[1]).paths).toHaveLength(101) // The scanner child re-parses the request it is handed; 101 paths inside // `filters.scopePaths` would be refused there and surface as "not ready". expect(() => AiVaultSearchRequestSchema.parse(call?.[0])).not.toThrow() diff --git a/src/main/ai-vault-search/session-search-service-registry.ts b/src/main/ai-vault-search/session-search-service-registry.ts index 7779e2d513a..6c8633fb406 100644 --- a/src/main/ai-vault-search/session-search-service-registry.ts +++ b/src/main/ai-vault-search/session-search-service-registry.ts @@ -7,11 +7,7 @@ import { import { unavailableSessionSearchStatus } from '../../shared/ai-vault-search-client' import { sessionSearchScopeCatalog } from './session-search-scope-catalog' import { resolveSessionSearchScope } from './session-search-scope-resolution' -import type { - AiVaultSearchRequest, - AiVaultSearchResponse, - AiVaultSearchStatus -} from '../../shared/ai-vault-search-types' +import type { AiVaultSearchResponse, AiVaultSearchStatus } from '../../shared/ai-vault-search-types' import { redactForTransport, redactStatusForTransport, @@ -36,17 +32,19 @@ export async function searchSessionService( return { kind: 'unavailable', reason: 'no-service' } } // The one place every entry point funnels through, so native, WSL, SSH and - // relay hosts all turn an identity into paths the same way, exactly once. - const scoped = applySessionSearchScope(parsed) - if (scoped === null) { - return { kind: 'unavailable', reason: 'scope-unknown' } - } - const { request, hostScopePaths } = scoped + // relay hosts all turn an identity into paths the same way, exactly once. The + // verdict is handed to the service rather than answered here: a host that is + // off or still starting owes the reader that answer first, and it is the + // service that makes it. + const { within, ...request } = parsed + const hostScope = within + ? resolveSessionSearchScope(within, sessionSearchScopeCatalog()) + : undefined const freshness = request.freshness === 'wait-until-current' ? await reconcileWithin(current, freshnessTimeoutMs) : false - const result = AiVaultSearchResponseSchema.parse(await current.search(request, hostScopePaths)) + const result = AiVaultSearchResponseSchema.parse(await current.search(request, hostScope)) if (result.kind !== 'results') { return result } @@ -55,36 +53,11 @@ export async function searchSessionService( ...fields, hits: result.hits.map((hit) => redactForTransport(hit, transport)), truncated: { ...result.truncated, freshness: result.truncated.freshness || freshness }, - ...(hostScopePaths ? { resolvedWithin: true as const } : {}), + ...(hostScope ? { resolvedWithin: true as const } : {}), ...(request.debug && debug ? { debug } : {}) } } -type ScopedSessionSearch = { - request: AiVaultSearchRequest - /** Absent for an unscoped request, which still searches everything. */ - hostScopePaths?: readonly string[] -} - -/** - * Turns this host's scope identity into this host's paths. Null means the host - * does not know the workspace or project, which is an answer — never a reason to - * fall back to searching everything. - */ -function applySessionSearchScope(parsed: AiVaultSearchRequest): ScopedSessionSearch | null { - const { within, ...request } = parsed - if (!within) { - return { request } - } - const resolution = resolveSessionSearchScope(within, sessionSearchScopeCatalog()) - if (resolution.kind === 'unknown') { - return null - } - // The paths ride beside the request, never inside `filters.scopePaths`: that - // field is capped for the clients that write it, and a host's own answer is not. - return { request, hostScopePaths: resolution.paths } -} - export async function sessionSearchServiceStatus( raw: unknown, transport: SessionSearchTransport diff --git a/src/main/ai-vault-search/session-search-service.ts b/src/main/ai-vault-search/session-search-service.ts index 9f60bd190f2..71870a5c101 100644 --- a/src/main/ai-vault-search/session-search-service.ts +++ b/src/main/ai-vault-search/session-search-service.ts @@ -8,22 +8,30 @@ import type { SessionSearchIndexer } from './session-search-indexer' import { SessionSearchCursorError } from './session-search-page-cursor' /** - * Paths a host resolved from a scope identity, handed to the engine beside the - * request rather than inside `filters.scopePaths`. + * What the answering host made of a scope identity. Absent means the request + * carried none and still searches everything. * - * Why not that field: it is a wire field, capped at 64 entries for the clients - * that fill it in by hand. A project whose worktrees do not share one managed - * directory resolves to one path per worktree, and 100 of them would be refused - * by the very schema the request is re-parsed with inside the scanner child. - * These paths never cross a wire — the host that resolved them is the host that - * searches — so no cap applies to them. + * Why the paths ride here and not in `filters.scopePaths`: that is a wire field, + * capped at 64 entries for the clients that fill it in by hand. A project whose + * worktrees do not share one managed directory resolves to one path per + * worktree, and 100 of them would be refused by the very schema the request is + * re-parsed with inside the scanner child. These paths never cross a wire — the + * host that resolved them is the host that searches — so no cap applies. + * + * Why `unknown` travels here rather than being answered by the caller: a host + * that is switched off or still starting owes the reader that answer, for a + * scoped request exactly as for an unscoped one. Those answers are made below, + * after consent and readiness are checked, so the verdict has to arrive where + * they are made and not before. */ -export type SessionSearchHostScopePaths = readonly string[] +export type SessionSearchHostScope = + | { kind: 'resolved'; paths: readonly string[] } + | { kind: 'unknown' } export type SessionSearchService = { search( req: AiVaultSearchRequest, - hostScopePaths?: SessionSearchHostScopePaths + hostScope?: SessionSearchHostScope ): Promise status(): Promise reconcile(): Promise @@ -39,14 +47,19 @@ export function createSessionSearchService({ return { reconcile: () => indexer.reconcile({ full: true }), status: async () => ({ enabled: true, ...indexer.status(), generation: engine.generation() }), - search: async (request, hostScopePaths) => { + search: async (request, hostScope) => { + // Reached only through a live index, so consent and readiness are already + // answered: an unresolvable scope is this host's last word, not a fallback. + if (hostScope?.kind === 'unknown') { + return { kind: 'unavailable', reason: 'scope-unknown' } + } if (request.cursor === '') { return { kind: 'malformed-cursor' } } try { const result = engine.search( - hostScopePaths - ? { ...request, filters: { ...request.filters, scopePaths: hostScopePaths } } + hostScope + ? { ...request, filters: { ...request.filters, scopePaths: hostScope.paths } } : request ) return { diff --git a/src/main/ai-vault/session-scanner-service-protocol.ts b/src/main/ai-vault/session-scanner-service-protocol.ts index 86a4917e350..faff7a29896 100644 --- a/src/main/ai-vault/session-scanner-service-protocol.ts +++ b/src/main/ai-vault/session-scanner-service-protocol.ts @@ -10,6 +10,7 @@ import type { AiVaultSearchStatus } from '../../shared/ai-vault-search-types' import type { AiVaultSearchSettings } from '../../shared/ai-vault-search-settings' +import type { SessionSearchHostScope } from '../ai-vault-search/session-search-service' import type { SessionSearchScanRoots } from '../ai-vault-search/session-search-scan-roots' import type { ReadAiVaultFirstUserPromptArgs } from './session-first-user-prompt-read' import type { SessionParseCachePersistenceOptions } from './session-parse-cache-persistence' @@ -86,8 +87,8 @@ export type AiVaultServiceRequestBody = type: 'request' operation: 'searchSessions' request: AiVaultSearchRequest - /** Host-resolved scope paths; deliberately outside `request` so no wire cap applies. */ - hostScopePaths?: readonly string[] + /** What the host made of a scope identity; outside `request` so no wire cap applies. */ + hostScope?: SessionSearchHostScope } | { type: 'request'; operation: 'searchStatus' } | { type: 'request'; operation: 'searchReconcile' } diff --git a/src/main/ai-vault/session-scanner-service-search.ts b/src/main/ai-vault/session-scanner-service-search.ts index b5b4ef3826d..fd747d1ab9d 100644 --- a/src/main/ai-vault/session-scanner-service-search.ts +++ b/src/main/ai-vault/session-scanner-service-search.ts @@ -90,7 +90,7 @@ export class SessionScannerServiceSearch { value: instance ? await instance.search( AiVaultSearchRequestSchema.parse(request.request), - request.hostScopePaths + request.hostScope ) : { kind: 'unavailable', reason: 'disabled' } } diff --git a/src/main/ai-vault/session-scanner-service-spawn.ts b/src/main/ai-vault/session-scanner-service-spawn.ts index 15bc9e5a68b..a923e904023 100644 --- a/src/main/ai-vault/session-scanner-service-spawn.ts +++ b/src/main/ai-vault/session-scanner-service-spawn.ts @@ -1,4 +1,5 @@ import { localAiVaultScanRoots } from './cached-session-list' +import type { SessionSearchHostScope } from '../ai-vault-search/session-search-service' import { fork, type ChildProcess } from 'node:child_process' import { existsSync } from 'node:fs' import type { @@ -97,13 +98,13 @@ export function readAiVaultFirstUserPromptInService( export function searchSessionsInService( request: AiVaultSearchRequest, - hostScopePaths?: readonly string[] + hostScope?: SessionSearchHostScope ): Promise { return getSharedClient().request({ type: 'request', operation: 'searchSessions', request, - ...(hostScopePaths ? { hostScopePaths } : {}) + ...(hostScope ? { hostScope } : {}) }) } diff --git a/src/shared/ai-vault-search-test-fixture.ts b/src/shared/ai-vault-search-test-fixture.ts index 7029eb761a3..3729d7dc229 100644 --- a/src/shared/ai-vault-search-test-fixture.ts +++ b/src/shared/ai-vault-search-test-fixture.ts @@ -39,8 +39,9 @@ export function fakeSearchService() { search: vi.fn( async ( _request: AiVaultSearchRequest, - // Host-resolved scope paths; typed here so a caller's arguments are visible to `mock.calls`. - _hostScopePaths?: readonly string[] + // The host's scope verdict, declared so `mock.calls` records it. Typed + // loosely because its type is a host-side one and this fixture is shared. + _hostScope?: unknown ): Promise => searchResults() ), status: vi.fn(async () => ({