Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,10 +1,24 @@
import type { ProviderOptions } from '@ai-sdk/provider-utils'
import type { StopCondition, ToolSet } from 'ai'
import { describe, expect, it } from 'vitest'
import type { StopCondition, Tool, ToolSet } from 'ai'
import { afterEach, describe, expect, it, vi } from 'vitest'

import { makeModel } from '../../../../__tests__/fixtures'
import { makeAssistant, makeModel } from '../../../../__tests__/fixtures'
import { registry } from '../../../../tools/adapters/aiSdk/registry'
import type { ToolEntry } from '../../../../tools/adapters/aiSdk/types'
import type { CallOverrides } from '../../../../types/requests'
import { applyCallOverrides, composeStopWhen } from '../buildAgentParams'

vi.mock('@application', () => ({
application: {
get: (name: string) => {
if (name === 'KnowledgeService') return { hasAnyBase: () => true }
throw new Error(`unexpected service: ${name}`)
}
}
}))

const { applyCallOverrides, composeStopWhen, resolveKnowledgeBaseIds, resolveTools } = await import(
'../buildAgentParams'
)

/**
* Covers the first-class per-request override merge that replaced the old
Expand Down Expand Up @@ -99,3 +113,62 @@ describe('composeStopWhen', () => {
expect(await conditions[0]({ steps: new Array(19) } as never)).toBe(false)
})
})

describe('resolveKnowledgeBaseIds', () => {
it('falls back to the assistant-bound bases when the request selects none', () => {
expect(resolveKnowledgeBaseIds(makeAssistant({ knowledgeBaseIds: ['kb-1'] }), undefined)).toEqual(['kb-1'])
})

it('trusts the request-selected bases when the assistant has no static binding', () => {
expect(resolveKnowledgeBaseIds(makeAssistant({ knowledgeBaseIds: [] }), ['kb-2'])).toEqual(['kb-2'])
expect(resolveKnowledgeBaseIds(undefined, ['kb-2'])).toEqual(['kb-2'])
})

it('drops request-selected bases outside the assistant scope instead of expanding it', () => {
// An assistant statically bound to `kb-public` must not become searchable for `kb-private`
// just because the renderer/IPC request asked for it — the assistant's own binding is the
// trust boundary, not whatever the composer UI happened to let the user pick.
expect(resolveKnowledgeBaseIds(makeAssistant({ knowledgeBaseIds: ['kb-public'] }), ['kb-private'])).toEqual([
'kb-public'
])
expect(resolveKnowledgeBaseIds(makeAssistant({ knowledgeBaseIds: ['kb-1'] }), ['kb-1', 'kb-2'])).toEqual(['kb-1'])
})

it('returns an empty array when neither source selects a base', () => {
expect(resolveKnowledgeBaseIds(undefined, undefined)).toEqual([])
expect(resolveKnowledgeBaseIds(makeAssistant({ knowledgeBaseIds: [] }), undefined)).toEqual([])
})
})

describe('resolveTools knowledge-base wiring', () => {
const KB_GATED_TOOL_NAME = 'test-kb-gated-tool'

const kbGatedEntry: ToolEntry = {
name: KB_GATED_TOOL_NAME,
namespace: 'test',
description: 'test-only tool gated on knowledgeBaseIds',
defer: 'never',
tool: {} as Tool,
applies: (scope) => (scope.knowledgeBaseIds?.length ?? 0) > 0
}

afterEach(() => {
registry.deregister(KB_GATED_TOOL_NAME)
})

it('exposes a kb-gated tool when the effective knowledgeBaseIds is non-empty', async () => {
registry.register(kbGatedEntry)

const { tools } = await resolveTools({}, undefined, makeModel(), false, ['kb-1'])

expect(tools?.[KB_GATED_TOOL_NAME]).toBeDefined()
})

it('hides a kb-gated tool when the effective knowledgeBaseIds is empty', async () => {
registry.register(kbGatedEntry)

const { tools } = await resolveTools({}, undefined, makeModel(), false, [])

expect(tools?.[KB_GATED_TOOL_NAME]).toBeUndefined()
})
})
36 changes: 30 additions & 6 deletions src/main/ai/runtime/aiSdk/params/buildAgentParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,9 @@ export async function buildAgentParams(input: BuildAgentParamsInput): Promise<Bu
applyHttpTrace(sdkConfig, request.chatId, model)
const fileAttachments = collectFileAttachments(request.messages)
const hasFileAttachments = fileAttachments.length > 0
const knowledgeBaseIds = resolveKnowledgeBaseIds(assistant, request.knowledgeBaseIds)

@AtomsH4 AtomsH4 Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This review comment was translated automatically.

[Blocker][A1] The knowledgeBaseIds calculated here is not passed into resolveTools() / registry.selectActive(), but the applies for the four kb_* tools have already been changed to check scope.knowledgeBaseIds. The actual path is: composer passes in request.knowledgeBaseIds, here we get a non-empty scope, but line 80 still calls resolveTools(request, assistant, model, hasFileAttachments); resolveTools() line 190's selectActive({ assistant, mcpToolIds, hasFileAttachments, hasAnyKnowledgeBase }) doesn't include knowledgeBaseIds, so KnowledgeSearchTool etc.'s applies all return false. The result is that not only the composer-selected KB that this PR is supposed to fix won't expose tools, but the originally assistant statically-bound KB tools will also be hidden. Minimal fix: pass the effective knowledgeBaseIds into resolveTools, and pass it to registry.selectActive({ ..., knowledgeBaseIds }); also add a buildAgentParams level test asserting that both assistant-bound and request-selected scenarios will select kb_* tools.


Original Content

[Blocker][A1] 这里算出的 knowledgeBaseIds 没有进入 resolveTools() / registry.selectActive(),但四个 kb_* tool 的 applies 已经改成检查 scope.knowledgeBaseIds。实际路径是:composer 传入 request.knowledgeBaseIds,这里得到非空 scope,但第 80 行仍调用 resolveTools(request, assistant, model, hasFileAttachments)resolveTools() 第 190 行的 selectActive({ assistant, mcpToolIds, hasFileAttachments, hasAnyKnowledgeBase }) 没带 knowledgeBaseIds,所以 KnowledgeSearchTool 等的 applies 都返回 false。结果不仅本 PR 要修的 composer-selected KB 不会暴露工具,原本 assistant 静态绑定的 KB 工具也会被隐藏。最小修复:把 effective knowledgeBaseIds 传进 resolveTools,并传给 registry.selectActive({ ..., knowledgeBaseIds });同时加一个 buildAgentParams 级别测试,断言 assistant-bound 和 request-selected 两种场景都会选中 kb_* 工具。

const { tools, deferredEntries, mcpToolIds } = canModelConsumeTools(model)
? await resolveTools(request, assistant, model, hasFileAttachments)
? await resolveTools(request, assistant, model, hasFileAttachments, knowledgeBaseIds)
: { tools: undefined, deferredEntries: [] as ToolEntry[], mcpToolIds: new Set<string>() }
const capabilities = assistant ? resolveCapabilities(model, provider, assistant) : undefined

Expand All @@ -89,7 +90,8 @@ export async function buildAgentParams(input: BuildAgentParamsInput): Promise<Bu
topicId: request.chatId,
assistant,
abortSignal: signal,
fileAttachments
fileAttachments,
knowledgeBaseIds
}

const scope: RequestScope = {
Expand All @@ -105,7 +107,8 @@ export async function buildAgentParams(input: BuildAgentParamsInput): Promise<Bu
aiSdkProviderId,
requestContext,
mcpToolIds,
hasFileAttachments
hasFileAttachments,
knowledgeBaseIds
}

const features = extraFeatures?.length ? [...INTERNAL_FEATURES, ...extraFeatures] : INTERNAL_FEATURES
Expand Down Expand Up @@ -161,11 +164,12 @@ function canModelConsumeTools(model: Model): boolean {
* sync the MCP entries into the registry, then materialise the active
* `ToolSet` via `applies` predicates and defer exposition.
*/
async function resolveTools(
export async function resolveTools(
request: BuildAgentParamsInput['request'],
assistant: Assistant | undefined,
model: Model,
hasFileAttachments: boolean
hasFileAttachments: boolean,
knowledgeBaseIds: readonly string[]
): Promise<{
tools: ToolSet | undefined
deferredEntries: ToolEntry[]
Expand All @@ -184,7 +188,13 @@ async function resolveTools(
}

const hasAnyKnowledgeBase = resolveHasAnyKnowledgeBase()
const activeEntries = registry.selectActive({ assistant, mcpToolIds, hasFileAttachments, hasAnyKnowledgeBase })
const activeEntries = registry.selectActive({
assistant,
mcpToolIds,
hasFileAttachments,
hasAnyKnowledgeBase,
knowledgeBaseIds
})
let tools: ToolSet | undefined
if (activeEntries.length > 0) {
tools = {}
Expand Down Expand Up @@ -218,6 +228,20 @@ function resolveHasAnyKnowledgeBase(): boolean {
}
}

/**
* Effective knowledge base scope for this request. When the assistant has its own static binding,
* that binding IS the scope — the composer's per-turn selection can never expand it, since main
* cannot trust a renderer/IPC-supplied id list to stay within the assistant's configured bases (the
* composer UI happens to restrict picks to that set today, but that's a UI nicety, not a security
* boundary). Only an assistant with no static binding lets the per-turn selection define the scope —
* which is the actual gap this resolves: composer-only, ad-hoc knowledge base use.
*/
export function resolveKnowledgeBaseIds(assistant: Assistant | undefined, requestIds: string[] | undefined): string[] {
const assistantIds = assistant?.knowledgeBaseIds ?? []
if (assistantIds.length > 0) return assistantIds
return Array.from(new Set(requestIds ?? []))
}

/**
* Assemble `AgentOptions`: capability-driven providerOptions overlaid with
* the user's customParameters (split into AI-SDK standard params vs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,14 @@ export class PersistentChatContextProvider implements ChatContextProvider {
const history = this.buildHistory(userMessage.id)
const models_ = assistantPlaceholders.map(({ model, placeholder, rootSpan }) => ({
modelId: model.id,
request: this.buildStreamRequest(req.topicId, assistantId, model.id, history, placeholder.id),
request: this.buildStreamRequest(
req.topicId,
assistantId,
model.id,
history,
placeholder.id,
req.knowledgeBaseIds
),
rootSpan
}))
// Author the turn span's input attributes here, where the built request payload is available.
Expand Down Expand Up @@ -368,7 +375,7 @@ export class PersistentChatContextProvider implements ChatContextProvider {
models: [
{
modelId: model.id,
request: this.buildStreamRequest(req.topicId, assistantId, model.id, history, anchor.id),
request: this.buildStreamRequest(req.topicId, assistantId, model.id, history, anchor.id, undefined),
rootSpan
}
],
Expand Down Expand Up @@ -438,7 +445,7 @@ export class PersistentChatContextProvider implements ChatContextProvider {
models: [
{
modelId: model.id,
request: this.buildStreamRequest(req.topicId, assistantId, model.id, history, placeholder.id),
request: this.buildStreamRequest(req.topicId, assistantId, model.id, history, placeholder.id, undefined),
rootSpan
}
],
Expand Down Expand Up @@ -471,15 +478,17 @@ export class PersistentChatContextProvider implements ChatContextProvider {
assistantId: string | undefined,
uniqueModelId: UniqueModelId,
history: CherryUIMessage[],
messageId: string
messageId: string,
knowledgeBaseIds: string[] | undefined
): AiStreamRequest {
return {
chatId: topicId,
trigger: 'submit-message',
assistantId,
uniqueModelId,
messages: history,
messageId
messageId,
knowledgeBaseIds
}
}
}
Expand Down
15 changes: 8 additions & 7 deletions src/main/ai/tools/adapters/aiSdk/builtin/KnowledgeListTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@
* Both modes live in the shared `knowledgeLookup` core so the Claude Code MCP bridge runs identical
* logic; this file is just the AI-SDK `tool()` wrapper.
*
* Scope: when `assistant.knowledgeBaseIds` is non-empty, only those bases are reachable; when empty,
* all user bases are.
* Scope: when the effective scope (the assistant's static binding when non-empty, else the composer's
* per-turn selection — see `resolveKnowledgeBaseIds`) is non-empty, only those bases are reachable;
* when empty, all user bases are.
*/

import {
Expand Down Expand Up @@ -45,7 +46,7 @@ const kbListTool = tool({
strict: true,
execute: async (input, options) => {
const { request } = getToolCallContext(options)
return listOrOutlineKnowledge(input, request.assistant?.knowledgeBaseIds ?? [])
return listOrOutlineKnowledge(input, request.knowledgeBaseIds ?? [])
},
toModelOutput: ({ input, output }) => knowledgeListModelOutput(output, input)
})
Expand All @@ -58,9 +59,9 @@ export function createKbListToolEntry(): ToolEntry {
defer: 'never',
tool: kbListTool,
// Discovery entry point, always inlined (defer: 'never') — but gated identically to kb_search /
// kb_read / kb_manage: a base must exist AND be bound to this assistant. Listing every base while
// none are bound (no kb_read / kb_search to act on them) is a discovery dead-end and widens the
// per-assistant scope, so kb_list shares the siblings' gate.
applies: (scope) => scope.hasAnyKnowledgeBase === true && (scope.assistant?.knowledgeBaseIds?.length ?? 0) > 0
// kb_read / kb_manage: a base must exist AND be in scope (bound to this assistant, or selected
// this turn). Listing every base while none are in scope (no kb_read / kb_search to act on them)
// is a discovery dead-end and widens the scope, so kb_list shares the siblings' gate.
applies: (scope) => scope.hasAnyKnowledgeBase === true && (scope.knowledgeBaseIds?.length ?? 0) > 0
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
* Knowledge base write tool — the destructive companion to the read tools.
*
* One tool with an `action`: add a new source (file / url / note), or delete /
* re-index existing documents addressed by their Concept ID. Per-request
* `assistant.knowledgeBaseIds` flows in via RequestContext and scopes which bases
* are reachable. Every action mutates the base, so the tool is approval-gated
* re-index existing documents addressed by their Concept ID. The effective knowledge
* base scope (the assistant's static binding when non-empty, else the composer's
* per-turn selection — see `resolveKnowledgeBaseIds`) flows in via
* `RequestContext.knowledgeBaseIds` and scopes which bases are reachable. Every
* action mutates the base, so the tool is approval-gated
* (`needsApproval: true`) — Cherry surfaces the approval card before it runs. The
* mutation itself lives in the shared `knowledgeLookup` core so the Claude Code
* MCP bridge runs identical logic (gated there by Claude Code's own permission
Expand Down Expand Up @@ -43,7 +45,7 @@ const kbManageTool = tool({
needsApproval: true,
execute: async (input, options) => {
const { request } = getToolCallContext(options)
return manageKnowledge(input, request.assistant?.knowledgeBaseIds ?? [])
return manageKnowledge(input, request.knowledgeBaseIds ?? [])
},
toModelOutput: ({ output }) => knowledgeManageModelOutput(output)
})
Expand All @@ -55,7 +57,7 @@ export function createKbManageToolEntry(): ToolEntry {
description: 'Add, delete, or re-index documents in a knowledge base (requires approval)',
defer: 'never',
tool: kbManageTool,
applies: (scope) => scope.hasAnyKnowledgeBase === true && (scope.assistant?.knowledgeBaseIds?.length ?? 0) > 0
applies: (scope) => scope.hasAnyKnowledgeBase === true && (scope.knowledgeBaseIds?.length ?? 0) > 0
}
}

Expand Down
10 changes: 6 additions & 4 deletions src/main/ai/tools/adapters/aiSdk/builtin/KnowledgeReadTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@
* match's line, offsets, and snippet — for a precise lookup when semantic search is too fuzzy.
*
* The model passes a `conceptId` + `baseId` from a `kb_search` hit (or a `kb_list` outline).
* Per-request `assistant.knowledgeBaseIds` flows in via RequestContext and scopes which bases are
* reachable. Both modes live in the shared `knowledgeLookup` core so the Claude Code MCP bridge runs
* The effective knowledge base scope (the assistant's static binding when non-empty, else the
* composer's per-turn selection — see `resolveKnowledgeBaseIds`) flows in via
* `RequestContext.knowledgeBaseIds` and scopes which bases are reachable. Both modes live in the
* shared `knowledgeLookup` core so the Claude Code MCP bridge runs
* identical logic; this file is just the AI-SDK `tool()` wrapper.
*/

Expand Down Expand Up @@ -39,7 +41,7 @@ const kbReadTool = tool({
strict: true,
execute: async (input, options) => {
const { request } = getToolCallContext(options)
return readOrGrepConcept(input, request.assistant?.knowledgeBaseIds ?? [])
return readOrGrepConcept(input, request.knowledgeBaseIds ?? [])
},
toModelOutput: ({ output }) => knowledgeReadModelOutput(output)
})
Expand All @@ -51,7 +53,7 @@ export function createKbReadToolEntry(): ToolEntry {
description: 'Read a knowledge base document by its Concept ID, or grep within it',
defer: 'always',
tool: kbReadTool,
applies: (scope) => scope.hasAnyKnowledgeBase === true && (scope.assistant?.knowledgeBaseIds?.length ?? 0) > 0
applies: (scope) => scope.hasAnyKnowledgeBase === true && (scope.knowledgeBaseIds?.length ?? 0) > 0
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@
* Knowledge base search tool — agentic.
*
* The model picks the query and target `baseIds` (typically after `kb_list`).
* Per-request `assistant.knowledgeBaseIds` flows in via RequestContext and
* scopes which base IDs are accepted. The search itself lives in the shared
* `knowledgeLookup` core so the Claude Code MCP bridge runs identical logic;
* this file is just the AI-SDK `tool()` wrapper.
* The effective knowledge base scope (the assistant's static binding when non-empty, else the
* composer's per-turn selection — see `resolveKnowledgeBaseIds`) flows in via
* `RequestContext.knowledgeBaseIds` and scopes which base IDs are accepted. The search itself lives
* in the shared `knowledgeLookup` core so the Claude Code MCP
* bridge runs identical logic; this file is just the AI-SDK `tool()` wrapper.
*/

import { KB_SEARCH_TOOL_NAME, kbSearchInputSchema, kbSearchOutputSchema } from '@shared/ai/builtinTools'
Expand Down Expand Up @@ -33,7 +34,7 @@ const kbSearchTool = tool({
strict: true,
execute: async ({ query, baseIds }, options) => {
const { request } = getToolCallContext(options)
return searchKnowledge(query, baseIds, request.assistant?.knowledgeBaseIds ?? [])
return searchKnowledge(query, baseIds, request.knowledgeBaseIds ?? [])
},
toModelOutput: ({ output }) => knowledgeSearchModelOutput(output)
})
Expand All @@ -45,7 +46,7 @@ export function createKbSearchToolEntry(): ToolEntry {
description: "Search the user's private knowledge base",
defer: 'always',
tool: kbSearchTool,
applies: (scope) => scope.hasAnyKnowledgeBase === true && (scope.assistant?.knowledgeBaseIds?.length ?? 0) > 0
applies: (scope) => scope.hasAnyKnowledgeBase === true && (scope.knowledgeBaseIds?.length ?? 0) > 0
}
}

Expand Down
Loading
Loading