Keep unknown search domains out of Sentry, and stop the batch carve-out over-suppressing

packages/worker/src/mcp/capabilities/packages/get-git-remote.node.test.ts

@@ -3,7 +3,7 @@ import { expect, test, vi } from 'vitest'
 const mockModule = vi.hoisted(() => ({
 	getSavedPackageById: vi.fn(),
 	getSavedPackageByKodyId: vi.fn(),
-	getEntitySourceById: vi.fn(),
+	getEntitySourceByIdForUser: vi.fn(),
 	resolveArtifactDefaultBranchHead: vi.fn(),
 	resolveExistingArtifactSourceRepo: vi.fn(),
 	createStubSavedPackage: vi.fn(),
@@ -24,8 +24,8 @@ vi.mock('#worker/package-registry/package-owner.ts', () => ({
 }))
 
 vi.mock('#worker/repo/entity-sources.ts', () => ({
-	getEntitySourceById: (...args: Array<unknown>) =>
-		mockModule.getEntitySourceById(...args),
+	getEntitySourceByIdForUser: (...args: Array<unknown>) =>
+		mockModule.getEntitySourceByIdForUser(...args),
 }))
 
 vi.mock('#worker/repo/artifacts.ts', async () => {
@@ -122,20 +122,25 @@ function mockPackageSource(sourceUserId = 'user-1') {
 		name: '@kentcdodds/unleashed-wifi',
 		sourceId: 'source-1',
 	})
-	mockModule.getEntitySourceById.mockResolvedValue({
-		id: 'source-1',
-		user_id: sourceUserId,
-		entity_kind: 'package',
-		entity_id: 'package-1',
-		repo_id: 'package-package-1',
-		published_commit: 'commit-1',
-		indexed_commit: null,
-		manifest_path: 'package.json',
-		source_root: '/',
-		last_external_check_at: null,
-		created_at: '2026-05-04T00:00:00.000Z',
-		updated_at: '2026-05-04T00:00:00.000Z',
-	})
+	mockModule.getEntitySourceByIdForUser.mockImplementation(
+		async (_db: unknown, input: { id: string; userId: string }) => {
+			if (input.userId !== sourceUserId) return null
+			return {
+				id: 'source-1',
+				user_id: sourceUserId,
+				entity_kind: 'package',
+				entity_id: 'package-1',
+				repo_id: 'package-package-1',
+				published_commit: 'commit-1',
+				indexed_commit: null,
+				manifest_path: 'package.json',
+				source_root: '/',
+				last_external_check_at: null,
+				created_at: '2026-05-04T00:00:00.000Z',
+				updated_at: '2026-05-04T00:00:00.000Z',
+			}
+		},
+	)
 	const createToken = vi.fn(async (scope: 'read' | 'write', ttl: number) => ({
 		id: 'token-1',
 		plaintext: `art_v1_${scope}_token?expires=${Math.floor(Date.now() / 1000) + ttl}`,

packages/worker/src/mcp/capabilities/packages/publish-external-push.node.test.ts

@@ -5,7 +5,7 @@ const mockModule = vi.hoisted(() => ({
 	captureException: vi.fn(),
 	getSavedPackageById: vi.fn(),
 	getSavedPackageByKodyId: vi.fn(),
-	getEntitySourceById: vi.fn(),
+	getEntitySourceByIdForUser: vi.fn(),
 	resolveArtifactSourceHead: vi.fn(),
 	publishFromExternalRef: vi.fn(),
 	listPublishedPackageArtifactTargets: vi.fn(),
@@ -26,8 +26,8 @@ vi.mock('#worker/package-registry/repo.ts', () => ({
 }))
 
 vi.mock('#worker/repo/entity-sources.ts', () => ({
-	getEntitySourceById: (...args: Array<unknown>) =>
-		mockModule.getEntitySourceById(...args),
+	getEntitySourceByIdForUser: (...args: Array<unknown>) =>
+		mockModule.getEntitySourceByIdForUser(...args),
 }))
 
 vi.mock('#worker/repo/artifacts.ts', () => ({
@@ -67,7 +67,7 @@ function setupDefaultMocks() {
 		name: '@kentcdodds/demo-package',
 		sourceId: 'source-1',
 	})
-	mockModule.getEntitySourceById.mockResolvedValue({
+	mockModule.getEntitySourceByIdForUser.mockResolvedValue({
 		id: 'source-1',
 		user_id: 'user-1',
 		entity_kind: 'package',
@@ -470,7 +470,7 @@ test('publishExternalPush recovers from transient Durable Object resets', async
 	expect(consoleWarn).toHaveBeenCalledTimes(1)
 
 	setupDefaultMocks()
-	mockModule.getEntitySourceById
+	mockModule.getEntitySourceByIdForUser
 		.mockResolvedValueOnce({
 			id: 'source-1',
 			user_id: 'user-1',

packages/worker/src/mcp/capabilities/packages/resolve-package-source.ts

@@ -3,7 +3,7 @@ import {
 	getSavedPackageById,
 	getSavedPackageByKodyId,
 } from '#worker/package-registry/repo.ts'
-import { getEntitySourceById } from '#worker/repo/entity-sources.ts'
+import { getEntitySourceByIdForUser } from '#worker/repo/entity-sources.ts'
 import { type EntitySourceRow } from '#worker/repo/types.ts'
 
 export type PackageSourceIdentity = {
@@ -47,8 +47,11 @@ export async function resolveOwnedPackageSource(input: {
 		const missingId = input.args.package_id ?? input.args.kody_id
 		throw new McpCallerError(`Saved package "${missingId}" was not found.`)
 	}
-	const source = await getEntitySourceById(input.db, savedPackage.sourceId)
-	if (!source || source.user_id !== input.userId) {
+	const source = await getEntitySourceByIdForUser(input.db, {
+		id: savedPackage.sourceId,
+		userId: input.userId,
+	})
+	if (!source) {
 		throw new McpCallerError('Repo source was not found for this user.')
 	}
 	return {

packages/worker/src/mcp/observability.node.test.ts

@@ -124,6 +124,18 @@ test('logMcpEvent keeps caller mistakes out of Sentry', () => {
 			cause: new McpCallerError('Provide "query" or "domain".'),
 		})
 
+		logMcpEvent({
+			...callerFailureBase,
+			tool: 'search',
+			toolName: 'search',
+			errorName: 'McpCallerError',
+			errorMessage:
+				'Unknown domain "skills". Available domains: account, packages.',
+			cause: new McpCallerError(
+				'Unknown domain "skills". Available domains: account, packages.',
+			),
+		})
+
 		logMcpEvent({
 			...callerFailureBase,
 			capabilityName: 'repo_open_session',
@@ -154,7 +166,7 @@ test('logMcpEvent keeps caller mistakes out of Sentry', () => {
 		})
 	})
 
-	expect(payloads).toHaveLength(4)
+	expect(payloads).toHaveLength(5)
 	expect(sentryMock.captureException).not.toHaveBeenCalled()
 	expect(sentryMock.captureMessage).not.toHaveBeenCalled()
 })

packages/worker/src/mcp/tools/search-core.ts

@@ -1,3 +1,4 @@
+import { McpCallerError } from '#mcp/caller-error.ts'
 import {
 	deterministicEmbedding,
 	embedTextForVectorize,
@@ -174,7 +175,9 @@ export async function searchUnified(input: {
 	if (domainFilter) {
 		const availableDomains = listSearchDomainNames(input.registry)
 		if (!availableDomains.includes(domainFilter)) {
-			throw new Error(
+			// Caller passed a non-domain id (often a package kody id such as
+			// "skills"). Clear from the message alone — keep it off Sentry.
+			throw new McpCallerError(
 				`Unknown domain "${domainFilter}". Available domains: ${[...availableDomains].sort().join(', ')}.`,
 			)
 		}

packages/worker/src/mcp/tools/search-handler.node.test.ts

@@ -643,24 +643,96 @@ test('search tool batches entity detail with per-ref isolation and preserves sin
 	])
 
 	mockPerformanceNow.mockReturnValueOnce(400).mockReturnValueOnce(410)
-	const allFailed = await handler({
-		entity: ['missing_a:capability', 'missing_b:capability'],
-		conversationId: 'conv-batch-all-failed',
-	})
-	expect(allFailed.isError).toBe(true)
-	expect(allFailed.structuredContent.error).toMatch(
-		/all entity lookups failed/i,
-	)
-	expect(allFailed.structuredContent.result).toEqual([
-		expect.objectContaining({
-			entityRef: 'missing_a:capability',
-			error: expect.any(String),
-		}),
-		expect.objectContaining({
-			entityRef: 'missing_b:capability',
-			error: expect.any(String),
+	const observability = await import('#mcp/observability.ts')
+	const logMcpEventSpy = vi.spyOn(observability, 'logMcpEvent')
+	try {
+		const allFailed = await handler({
+			entity: ['missing_a:capability', 'missing_b:capability'],
+			conversationId: 'conv-batch-all-failed',
+		})
+		expect(allFailed.isError).toBe(true)
+		expect(allFailed.structuredContent.error).toMatch(
+			/all entity lookups failed/i,
+		)
+		expect(allFailed.structuredContent.result).toEqual([
+			expect.objectContaining({
+				entityRef: 'missing_a:capability',
+				error: expect.any(String),
+			}),
+			expect.objectContaining({
+				entityRef: 'missing_b:capability',
+				error: expect.any(String),
+			}),
+		])
+		expect(logMcpEventSpy).toHaveBeenCalledWith(
+			expect.objectContaining({
+				outcome: 'failure',
+				callerError: true,
+				errorName: 'EntityBatchError',
+			}),
+		)
+	} finally {
+		logMcpEventSpy.mockRestore()
+	}
+})
+
+test('entity batch all-fail reports platform errors to Sentry (no callerError)', async () => {
+	const observability = await import('#mcp/observability.ts')
+	const logMcpEventSpy = vi.spyOn(observability, 'logMcpEvent')
+	mockModule.getSavedPackageById.mockImplementation(
+		async (_db: unknown, input: { packageId: string }) => ({
+			id: input.packageId,
+			userId: 'user-1',
+			name: input.packageId,
+			kodyId: input.packageId,
+			description: 'pkg',
+			tags: [],
+			searchText: 'pkg',
+			sourceId: `source-${input.packageId}`,
+			hasApp: false,
+			hidden: false,
+			isPrivate: true,
+			createdAt: '2026-01-01T00:00:00.000Z',
+			updatedAt: '2026-01-01T00:00:00.000Z',
 		}),
-	])
+	)
+	mockModule.loadPackageSourceBySourceId.mockRejectedValue(
+		new Error('D1 read failed'),
+	)
+	try {
+		const { handler } = await getSearchRegistration({
+			user: {
+				userId: 'user-1',
+				email: 'user@example.com',
+				displayName: 'User',
+				username: 'user',
+			},
+		})
+		mockPerformanceNow.mockReturnValueOnce(500).mockReturnValueOnce(510)
+		const response = await handler({
+			entity: ['pkg-a:package', 'pkg-b:package'],
+			conversationId: 'conv-batch-platform-fail',
+		})
+		expect(response.isError).toBe(true)
+		expect(logMcpEventSpy).toHaveBeenCalledWith(
+			expect.objectContaining({
+				outcome: 'failure',
+				errorName: 'EntityBatchError',
+				cause: expect.objectContaining({
+					message: 'All entity lookups failed.',
+				}),
+			}),
+		)
+		const failureCall = logMcpEventSpy.mock.calls.find(
+			(call) =>
+				(call[0] as { errorName?: string }).errorName === 'EntityBatchError',
+		)
+		expect(failureCall?.[0]).not.toHaveProperty('callerError', true)
+	} finally {
+		logMcpEventSpy.mockRestore()
+		mockModule.getSavedPackageById.mockReset()
+		mockModule.loadPackageSourceBySourceId.mockReset()
+	}
 })
 
 test('integration entity detail enriches related packages without bloating ranked search', async () => {

packages/worker/src/mcp/tools/search-tool-runner.ts

@@ -324,18 +324,9 @@ export async function runSearchTool(input: {
 				markdownParts.push(entityResult.markdown)
 			}
 			const allFailed = successCount === 0
-			const failedEntries = outcome.results.filter(
-				(
-					entry,
-				): entry is Extract<(typeof outcome.results)[number], { ok: false }> =>
-					!entry.ok,
-			)
-			// Only treat a total batch failure as caller-caused when every
-			// failed lookup was a caller mistake. Shared platform failures
-			// (DB/load) still reach Sentry.
-			const allCallerFailures =
-				failedEntries.length > 0 &&
-				failedEntries.every((entry) => entry.callerError)
+			const allFailuresAreCallerErrors =
+				allFailed &&
+				outcome.results.every((entry) => !entry.ok && entry.callerError)
 			logMcpEvent({
 				category: 'mcp',
 				tool: 'search',
@@ -348,7 +339,14 @@ export async function runSearchTool(input: {
 				...(allFailed
 					? {
 							sandboxError: false,
-							...(allCallerFailures ? { callerError: true } : {}),
+							// Only treat the batch as a caller mistake when every
+							// entry failed with McpCallerError. Mixed/platform
+							// failures must still reach Sentry.
+							...(allFailuresAreCallerErrors
+								? { callerError: true }
+								: {
+										cause: new Error('All entity lookups failed.'),
+									}),
 							errorName: 'EntityBatchError',
 							errorMessage: 'All entity lookups failed.',
 						}

packages/worker/src/mcp/tools/search.node.test.ts

@@ -1,5 +1,6 @@
 import { expect, test, vi } from 'vitest'
 import { consoleWarn } from '#worker/test-support/console-spies.ts'
+import { McpCallerError } from '#mcp/caller-error.ts'
 import { buildCapabilityRegistry } from '#mcp/capabilities/build-capability-registry.ts'
 import {
 	CAPABILITY_EMBEDDING_DIMENSIONS,
@@ -2001,17 +2002,19 @@ test('searchUnified domain scoping: filter, browse, reject unknown, and overview
 	expect(names).toContain('email_send')
 	expect(names).not.toContain('job_schedule')
 
-	await expect(
-		searchUnified({
-			env: {} as Env,
-			query: 'send email',
-			limit: 10,
-			userId: 'user-1',
-			registry,
-			optionalRows: emptyOptionalSearchRows,
-			domain: 'nope',
-		}),
-	).rejects.toThrow(/Unknown domain "nope"/)
+	const unknownDomain = await searchUnified({
+		env: {} as Env,
+		query: 'send email',
+		limit: 10,
+		userId: 'user-1',
+		registry,
+		optionalRows: emptyOptionalSearchRows,
+		domain: 'nope',
+	}).catch((error: unknown) => error)
+	expect(unknownDomain).toBeInstanceOf(McpCallerError)
+	expect(unknownDomain).toMatchObject({
+		message: expect.stringMatching(/Unknown domain "nope"/),
+	})
 
 	const browse = await searchUnified({
 		env: {} as Env,