From 149a79ca18c6cf56e20a7d889514b2191a898391 Mon Sep 17 00:00:00 2001 From: liugddx Date: Fri, 23 Oct 2026 15:00:15 +0800 Subject: [PATCH] fix(assistant): keep the discovery Session to read and report tools The proactive discovery Session could create Matters that the conversation cannot see, adopt unassigned Matters, mutate conversation memory indexes, and control initiative settings. Its catalog and execution now share one visibility policy, and a discovery-only exclusion list removes these tools. Read tools, DiscoveryReport and DiscoveryContext remain available. The conversation, the Bot and index workers keep their existing tools. The exclusion list is intentionally scoped to verified behavior; whether MemorySourceQuery and MemoryExtract belong in discovery is still open. Refs #6247, fixes #6249, refs #6250 Generated-by: Claude Code Co-Authored-By: Claude Sonnet 5.5 --- .../assistant-initiative-boundary.test.ts | 4 +- .../discovery-capabilities.ts | 70 +++++++ .../src/assistant/index-initiative/host.ts | 9 + .../runtime-host/src/assistant/service.ts | 20 +- .../test/discovery-boundary.test.ts | 193 ++++++++++++++++++ .../index-initiative/test/fixture.ts | 1 + 6 files changed, 289 insertions(+), 8 deletions(-) create mode 100644 packages/runtime-host/src/assistant/index-initiative/discovery-capabilities.ts create mode 100644 packages/runtime-host/test/assistant/index-initiative/test/discovery-boundary.test.ts diff --git a/packages/runtime-host/src/__tests__/assistant-initiative-boundary.test.ts b/packages/runtime-host/src/__tests__/assistant-initiative-boundary.test.ts index 0fe0de02bd6..8bb2facda89 100644 --- a/packages/runtime-host/src/__tests__/assistant-initiative-boundary.test.ts +++ b/packages/runtime-host/src/__tests__/assistant-initiative-boundary.test.ts @@ -113,7 +113,7 @@ test('DiscoveryReport reports real mailbox rejection, success and storage failur bot, systemPrompt: { section() {}, context() {} }, turns: { beforeFinish() {} }, - tools: { register: (tool: any) => tools.set(tool.name, tool) }, + tools: { register: (tool: any) => tools.set(tool.name, tool), filter() {} }, api: { register() {} }, onClose: (fn: any) => closers.push(fn), onStart: (fn: any) => starters.push(fn), @@ -283,7 +283,7 @@ test('assistant.bind recovers from startup lookup failure without a Host restart bot: new AssistantBotBridge(), systemPrompt: { section() {}, context() {} }, turns: { beforeFinish() {} }, - tools: { register() {} }, + tools: { register() {}, filter() {} }, api: { register: (name: string, handler: any) => apis.set(name, handler) }, onClose: (fn: any) => closers.push(fn), onStart: (fn: any) => starters.push(fn), diff --git a/packages/runtime-host/src/assistant/index-initiative/discovery-capabilities.ts b/packages/runtime-host/src/assistant/index-initiative/discovery-capabilities.ts new file mode 100644 index 00000000000..728c65aeeae --- /dev/null +++ b/packages/runtime-host/src/assistant/index-initiative/discovery-capabilities.ts @@ -0,0 +1,70 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * Tools the proactive discovery Session must not see or run (#6249, #6250). + * + * Discovery investigates and reports through DiscoveryReport; it does not create, + * adopt or mutate conversation-owned Matters or memory indexes, control initiative + * configuration, or deliver to the user directly. Read tools (MatterOverview, + * MemoryIndexRead, DiscoveryContext, ...) and source queries stay available. + * Whether MemorySourceQuery and MemoryExtract belong here is still an open + * design question tracked in the RFC. + */ +export const DISCOVERY_EXCLUDED_TOOLS: ReadonlySet = new Set([ + // Matter creation, adoption and conversation-scoped control. + 'MatterDelegate', + 'MatterStart', + 'MatterAdopt', + 'MatterControl', + 'MatterMessage', + 'MatterTaskControl', + 'MatterTaskMessage', + 'MatterReply', + 'MatterDelegationRecover', + 'MatterFeedbackRetry', + // Matter worker lifecycle tools; valid only inside an activation the Session owns. + 'MatterRead', + 'MatterReadFile', + 'MatterWriteFile', + 'MatterCheckpoint', + 'MatterSettle', + 'MatterConditionTest', + 'MatterAskBot', + // Memory index mutation. Writes belong to index workers and the conversation. + 'MemoryIndexCreate', + 'MemoryIndexCreateGroup', + 'MemoryIndexWrite', + 'MemoryIndexEdit', + 'MemoryIndexCheckpoint', + 'MemoryIndexControl', + 'MemoryIndexAddSources', + 'MemoryIndexMaintain', + // Initiative configuration is owned by the conversation. + 'InitiativeEnable', + 'InitiativeConfigure', + 'InitiativeControl', + // Bot-only coordination tools. + 'SendUser', + 'InboxCheck', + 'InboxList', + 'InboxRead', + 'AssistantSetAvatar', + 'Sleep', +]); diff --git a/packages/runtime-host/src/assistant/index-initiative/host.ts b/packages/runtime-host/src/assistant/index-initiative/host.ts index 257ea7c03ea..6af39d7160a 100644 --- a/packages/runtime-host/src/assistant/index-initiative/host.ts +++ b/packages/runtime-host/src/assistant/index-initiative/host.ts @@ -24,6 +24,7 @@ import { InitiativeStore } from './store.js'; import { InitiativeController } from './controller.js'; import { initializeBot } from '../bot/host.js'; import { registerAssistant } from './assistant.js'; +import { DISCOVERY_EXCLUDED_TOOLS } from './discovery-capabilities.js'; export async function initializeInitiative( ctx: AssistantModuleServices, directory: string, @@ -47,6 +48,14 @@ export async function initializeInitiative( ctx.backgroundSessions.register(() => store.get()?.discoverySession ? [store.get().discoverySession] : [], ); + // Discovery investigates and reports; it must not hold conversation-mutating tools. + if (!ctx.tools.filter) + throw Error('Assistant tool filtering is required to bound discovery Sessions'); + ctx.tools.filter((sessionId, tools) => + store.get()?.discoverySession === sessionId + ? tools.filter((tool) => !DISCOVERY_EXCLUDED_TOOLS.has(tool.name)) + : tools, + ); ctx.tools.register({ name: 'DiscoveryReport', description: diff --git a/packages/runtime-host/src/assistant/service.ts b/packages/runtime-host/src/assistant/service.ts index 742fe75362c..f9e79e72ca7 100644 --- a/packages/runtime-host/src/assistant/service.ts +++ b/packages/runtime-host/src/assistant/service.ts @@ -109,6 +109,17 @@ class AssistantRuntime { this.sessionAccess.some((allows) => allows(sessionId)) ); } + /** + * The only visibility policy. Catalogs and execution both use it, so a tool hidden + * from a Session cannot run in that Session either. + */ + visibleTools(sessionId: string, tools: readonly MakaTool[]): readonly MakaTool[] { + const nativeNames = new Set(this.tools.map((tool) => tool.name)); + const allowed = this.hasSessionAccess(sessionId) + ? tools + : tools.filter((tool) => !nativeNames.has(tool.name)); + return this.toolFilters.reduce((current, filter) => filter(sessionId, current), allowed); + } private readonly sections: PluginSystemPromptSection[] = []; private readonly contexts: PluginSystemPromptSection[] = []; private readonly checks: TurnFinishCheck[] = []; @@ -172,6 +183,8 @@ class AssistantRuntime { if (this.closing) throw Error('Personal assistant is shutting down'); if (!this.hasSessionAccess(call.sessionId)) throw Error('Personal assistant tools are unavailable in this session'); + if (!this.visibleTools(call.sessionId, [tool]).length) + throw Error('Personal assistant tool is unavailable in this session'); return internalTool.impl(input, call); }, }); @@ -503,12 +516,7 @@ export class PersonalAssistantService { } filterTools(sessionId: string, tools: readonly MakaTool[]): readonly MakaTool[] { if (!this.enabled) return tools; - const runtime = this.runtime!; - const nativeNames = new Set(runtime.tools.map((tool) => tool.name)); - const allowed = runtime.hasSessionAccess(sessionId) - ? tools - : tools.filter((tool) => !nativeNames.has(tool.name)); - return runtime.toolFilters.reduce((current, filter) => filter(sessionId, current), allowed); + return this.runtime!.visibleTools(sessionId, tools); } toolCeiling(sessionId: string): readonly MakaTool[] | undefined { return this.enabled && this.runtime!.bot.isWorker(sessionId) diff --git a/packages/runtime-host/test/assistant/index-initiative/test/discovery-boundary.test.ts b/packages/runtime-host/test/assistant/index-initiative/test/discovery-boundary.test.ts new file mode 100644 index 00000000000..b1ef10f04c9 --- /dev/null +++ b/packages/runtime-host/test/assistant/index-initiative/test/discovery-boundary.test.ts @@ -0,0 +1,193 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// Regression tests for #6249 and #6250. The proactive discovery Session investigates +// and reports; it must not create, adopt, or mutate conversation-owned Matters or +// memory indexes, and must not appear to hold those tools in its catalog. +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { fixture, until } from './fixture.js'; + +async function setup() { + const f = await fixture({ matters: true }); + await f.remote('assistant.bind', { sessionId: 'owner' }); + f.setIndexRunner(async () => {}); + f.setRunner(async () => {}); + await f.invoke('InitiativeEnable', {}); + await f.invoke('InitiativeControl', { action: 'check' }); + await until(async () => !!(await f.invoke('InitiativeStatus')).discoverySession); + const discovery = (await f.invoke('InitiativeStatus')).discoverySession as string; + return { f, discovery }; +} + +test('discovery Session cannot delegate a Matter the conversation cannot see (#6249)', async () => { + const { f, discovery } = await setup(); + try { + await assert.rejects( + f.invokeAs(discovery, 'MatterDelegate', { + taskKey: 'probe', + title: 'probe', + request: 'probe', + }), + ); + const tasks = await f.invokeAs('owner', 'MatterTasks', {}); + assert.equal((tasks.items ?? []).length, 0); + } finally { + await f.close(); + } +}); + +test('discovery catalog omits conversation-mutating tools and keeps read and report tools (#6250)', async () => { + const { f, discovery } = await setup(); + try { + const visible = f + .filterTools(discovery, f.tools.resolve(discovery, []).tools) + .map((tool: any) => tool.name); + for (const name of [ + 'MatterDelegate', + 'MatterAdopt', + 'MatterSettle', + 'MemoryIndexWrite', + 'MemoryIndexControl', + 'SendUser', + ]) + assert.ok(!visible.includes(name), `catalog still exposes ${name}`); + for (const name of ['DiscoveryReport', 'DiscoveryContext', 'MemoryIndexRead', 'MatterOverview']) + assert.ok(visible.includes(name), `catalog lost ${name}`); + } finally { + await f.close(); + } +}); + +test('discovery Session cannot write, edit, checkpoint, control or create conversation memory indexes (#6250)', async () => { + const { f, discovery } = await setup(); + try { + const range = await f.invoke('MemoryRange', {}); + const index = await f.invoke('MemoryIndexCreate', { + name: 'Owner index', + instructions: 'owner', + cursor: range.to, + background: true, + }); + const indexId = index.index.id; + const seededRead = await f.invoke('MemoryIndexRead', { indexId }); + await f.invoke('MemoryIndexWrite', { + indexId, + key: 'owner-key', + expectedRevision: seededRead.index.revision, + text: 'OWNER TEXT', + }); + const seeded = await f.invoke('MemoryIndexRead', { indexId }); + await assert.rejects( + f.invokeAs(discovery, 'MemoryIndexWrite', { + indexId, + key: 'owner-key', + expectedRevision: seeded.index.revision, + text: 'INJECTED', + }), + ); + await assert.rejects( + f.invokeAs(discovery, 'MemoryIndexEdit', { + indexId, + key: 'owner-key', + oldText: 'OWNER', + newText: 'INJECTED', + expectedRevision: seeded.index.revision, + }), + ); + await assert.rejects( + f.invokeAs(discovery, 'MemoryIndexCheckpoint', { + indexId, + rangeId: seeded.range.rangeId, + expectedRevision: seeded.index.revision, + notes: 'injected', + complete: false, + }), + ); + await assert.rejects(f.invokeAs(discovery, 'MemoryIndexControl', { indexId, action: 'pause' })); + await assert.rejects( + f.invokeAs(discovery, 'MemoryIndexCreate', { + name: 'Injected', + instructions: 'injected', + cursor: range.to, + background: false, + }), + ); + const content = await f.invoke('MemoryIndexContent', { indexId, key: 'owner-key' }); + assert.equal(content.text, 'OWNER TEXT'); + assert.equal( + (await f.invoke('MemoryIndexRead', { indexId })).index.revision, + seeded.index.revision, + ); + } finally { + await f.close(); + } +}); + +test('discovery Session cannot adopt an unassigned Matter from the conversation (#6250)', async () => { + const { f, discovery } = await setup(); + try { + await f.invokeAs('owner', 'MatterStart', { title: 'legacy', request: 'legacy request' }); + const before = (await f.invokeAs('owner', 'MatterOverview', {})).legacy ?? []; + assert.equal(before.length, 1); + await assert.rejects(f.invokeAs(discovery, 'MatterAdopt', { id: before[0].id })); + const after = (await f.invokeAs('owner', 'MatterOverview', {})).legacy ?? []; + assert.equal(after.length, 1); + } finally { + await f.close(); + } +}); + +test('discovery Session cannot control or amend a conversation-delegated task (#6250)', async () => { + const { f, discovery } = await setup(); + try { + const task = await f.invoke('MatterDelegate', { + taskKey: 'owner-task', + title: 'owner task', + request: 'owner request', + }); + await assert.rejects( + f.invokeAs(discovery, 'MatterTaskControl', { id: task.id, action: 'cancel' }), + ); + await assert.rejects( + f.invokeAs(discovery, 'MatterTaskMessage', { id: task.id, text: 'injected requirement' }), + ); + assert.notEqual((await f.invoke('MatterTasks', { id: task.id })).status, 'cancelled'); + } finally { + await f.close(); + } +}); + +test('discovery Session keeps read access to conversation memory (regression guard)', async () => { + const { f, discovery } = await setup(); + try { + const range = await f.invoke('MemoryRange', {}); + const index = await f.invoke('MemoryIndexCreate', { + name: 'Readable index', + instructions: 'readable', + cursor: range.to, + background: true, + }); + const read = await f.invokeAs(discovery, 'MemoryIndexRead', { indexId: index.index.id }); + assert.equal(read.index.id, index.index.id); + await f.invokeAs(discovery, 'MatterOverview', {}); + } finally { + await f.close(); + } +}); diff --git a/packages/runtime-host/test/assistant/index-initiative/test/fixture.ts b/packages/runtime-host/test/assistant/index-initiative/test/fixture.ts index b23f04f324d..8231644c61e 100644 --- a/packages/runtime-host/test/assistant/index-initiative/test/fixture.ts +++ b/packages/runtime-host/test/assistant/index-initiative/test/fixture.ts @@ -248,6 +248,7 @@ export async function fixture(options: any = {}) { workers, sessions, invokeAs, + filterTools: native.filterTools, remote: native.remote, send: (id: string, text: string) => native.send('owner', id, { text }), cancels: () => cancels,