From 40a5827db81e60c84f23d2475cd4db8172def9c9 Mon Sep 17 00:00:00 2001 From: yyhhyyyyyy Date: Wed, 15 Jul 2026 18:04:28 +0800 Subject: [PATCH 1/5] docs(tape): define capability layering --- .../tape-capability-layering/plan.md | 86 +++++++++++++++++++ .../tape-capability-layering/spec.md | 77 +++++++++++++++++ .../tape-capability-layering/tasks.md | 10 +++ 3 files changed, 173 insertions(+) create mode 100644 docs/architecture/tape-capability-layering/plan.md create mode 100644 docs/architecture/tape-capability-layering/spec.md create mode 100644 docs/architecture/tape-capability-layering/tasks.md diff --git a/docs/architecture/tape-capability-layering/plan.md b/docs/architecture/tape-capability-layering/plan.md new file mode 100644 index 000000000..161df3d9a --- /dev/null +++ b/docs/architecture/tape-capability-layering/plan.md @@ -0,0 +1,86 @@ +# Tape Capability Layering - Plan + +## Capability Policy + +- Define `AgentToolExposure` as `user-configurable`, `system-model`, `runtime-only`, or + `diagnostic` in a shared Agent tool policy module. +- Keep the five Tape names and their classifications in one registry. Treat unregistered existing + Agent tools as `user-configurable` to avoid unrelated behavior changes. +- Keep exposure metadata out of provider-bound `MCPToolDefinition` values. Main-process catalog + selection and execution checks consume the policy directly. + +## Catalog Separation + +- Keep `ToolPresenter.getAllToolDefinitions()` as the runtime catalog. Apply + `disabledAgentTools` only to `user-configurable` definitions and publish only this catalog into + the per-conversation `ToolMapper`. +- Add `getConfigurableAgentToolDefinitions()` for renderer configuration. Return only + `user-configurable` Agent definitions and do not resolve MCP definitions, publish mappings, or + remember conversation MCP access. +- Preserve the existing IPC route name and payload while routing it to the configurable catalog. + Rename the renderer client method so callers cannot confuse it with the runtime catalog. +- Update the new-thread tool popover and Agent settings to consume the configurable catalog without + hard-coded Tape-name filtering. + +### Renderer Behavior + +```text +BEFORE + +New-thread draft -> runtime catalog -> no agent-tape group (no persisted session) +Persisted DeepChat thread -> runtime catalog -> agent-tape group with user-facing switches + +AFTER + +New-thread draft -> configurable catalog -> user-configurable Agent tools only +Persisted DeepChat thread -> configurable catalog -> user-configurable Agent tools only +``` + +## Tape Model Boundary + +- Restrict `AgentTapeToolHandler` definitions and execution to the atomic `tape_search` and + `tape_context` recall pair. +- Require a DeepChat session plus both recall runtime ports before advertising either definition. +- Remove diagnostic and handoff descriptions from the tool system prompt. +- Narrow `AgentToolRuntimePort` to model-callable Tape operations. Retain Tape info, anchor listing, + and handoff on the lower runtime/application Tape boundary for future diagnostics and runtime use. +- Reject known non-model Tape names at the Agent tool execution boundary, including stale deferred + calls, while keeping their historical facts replayable. + +## Handoff and Persistence + +- Require a trimmed non-empty summary for runtime handoff. Validate before appending and continue to + derive cursor, range, and source message IDs from the effective Tape. +- Leave automatic compaction and context-overflow recovery on their existing compare-and-set path. +- Normalize all disabled-tool writes to drop non-configurable Tape names. +- Add a separately keyed, idempotent startup cleanup for existing session and Agent configuration + rows. Reuse the existing bounded-yield migration pattern and do not alter transcript or Tape data. + +## Compatibility + +- Keep the IPC route wire shape, provider tool shape, Tape schema, replay schema, and database schema + unchanged. +- Keep Tape names reserved so an MCP server cannot reinterpret a historical/internal name. +- A downgraded build may show Tape tools again after the cleanup removed obsolete disabled values; + no conversation or Tape data is lost, and the upgraded build remains deterministic. +- Leave pre-existing untracked documentation untouched. Update only maintained tracked contracts + that describe the old five-tool model surface. + +## Test Strategy + +- Cover runtime/configurable catalog membership and prove configurable reads have no mapper/access + side effects. +- Cover DeepChat, ACP, missing-session, missing-port, stale-disabled-list, same-name MCP, and deferred + call behavior. +- Cover pre-append handoff validation and derived provenance for valid handoffs. +- Cover v2 cleanup idempotence, preservation of ordinary disabled tools, Agent config cleanup, and + bounded session iteration. +- Run focused main and renderer tests before each implementation commit, then the full repository + validation sequence before final handoff. + +## Commit Slices + +1. `docs(tape): define capability layering` +2. `refactor(tools): split tool catalogs` +3. `fix(tape): limit model capabilities` +4. `fix(config): remove tape tool toggles` diff --git a/docs/architecture/tape-capability-layering/spec.md b/docs/architecture/tape-capability-layering/spec.md new file mode 100644 index 000000000..4bf720072 --- /dev/null +++ b/docs/architecture/tape-capability-layering/spec.md @@ -0,0 +1,77 @@ +# Tape Capability Layering - Spec + +> Status: **planned** + +## Problem + +DeepChat currently uses the runtime tool catalog as the source for both model execution and the +renderer's Agent tool configuration. Tape tools require a persisted DeepChat session, so the +configuration UI changes after the first message creates a session. The same catalog also applies +`disabledAgentTools` to every built-in Agent tool, which incorrectly presents Tape runtime +capabilities as optional user tools. + +This behavior is reported by +[GitHub issue #1975](https://github.com/ThinkInAIXYZ/deepchat/issues/1975). + +## Requirements + +1. Tape recording, reconstruction, compaction, replay, and memory lineage remain always-on runtime + infrastructure with no user-facing disable switch. +2. Agent tools have one authoritative exposure classification: + + | Tool | Exposure | + | --- | --- | + | `tape_search` | `system-model` | + | `tape_context` | `system-model` | + | `tape_info` | `diagnostic` | + | `tape_anchors` | `diagnostic` | + | `tape_handoff` | `runtime-only` | + +3. Existing Agent tools without an explicit classification remain `user-configurable`. +4. The runtime catalog includes available `system-model` tools regardless of + `disabledAgentTools`. The configurable catalog contains only `user-configurable` Agent tools. +5. Configurable catalog reads must not publish a `ToolMapper`, update conversation MCP access + context, or mutate runtime tool-profile caches. +6. The model-facing Tape contract exposes `tape_search` and `tape_context` as an atomic recall pair. + Neither tool is exposed if either runtime port is unavailable. +7. `tape_info`, `tape_anchors`, and `tape_handoff` remain reserved names but cannot be invoked as + model tools. +8. Runtime handoff requires a non-empty durable summary before advancing the reconstruction cursor. +9. Persisted Tape names are removed from Agent and session disabled-tool lists without modifying + messages, Tape entries, or replay manifests. + +## Acceptance Criteria + +- A new-thread draft and its persisted session show the same configurable Agent tool groups; no + `agent-tape` group appears in either state. +- A DeepChat runtime session exposes exactly `tape_search` and `tape_context` from the Tape tool + group, even when a legacy disabled list contains those names. +- ACP sessions, missing conversation IDs, and incomplete recall ports expose no Tape model tools. +- Direct or deferred model calls to diagnostic/runtime-only Tape names fail without appending an + anchor or otherwise mutating Tape state. +- Empty or whitespace-only runtime handoff summaries fail before an anchor is appended. +- The disabled-tool cleanup is idempotent, preserves ordinary disabled tools, and yields while + processing large session sets. +- Memory enablement, memory extraction/injection, subagents, skills, MCP tools, and ordinary Agent + tool switches retain their existing behavior. +- Focused tests, typecheck, full tests, formatting, i18n validation, and lint pass. + +## Constraints + +- Keep the `tools.listDefinitions` IPC route name and wire shape compatible. +- Keep all Tape names reserved against same-name MCP definitions. +- Preserve historical messages, tool-call facts, Tape entries, and replay behavior. +- Do not modify or stage pre-existing untracked documentation in the working tree. +- Do not push, create a pull request, tag, or mutate the linked GitHub issue. + +## Non-goals + +- Implementing a phase-aware or topic-aware Tape view. +- Implementing an anchor graph, cross-anchor traversal, or a second `TapeViewPolicy`. +- Adding a Tape inspector or special-purpose Tape trace UI. +- Reclassifying Memory, Subagent, Skill, Browser, or other non-Tape Agent tools. +- Making Bub's current tool exposure policy a compatibility target. + +## Open Questions + +None. diff --git a/docs/architecture/tape-capability-layering/tasks.md b/docs/architecture/tape-capability-layering/tasks.md new file mode 100644 index 000000000..0243bd392 --- /dev/null +++ b/docs/architecture/tape-capability-layering/tasks.md @@ -0,0 +1,10 @@ +# Tape Capability Layering - Tasks + +- [ ] T1: Record the capability matrix, catalog contracts, compatibility rules, and non-goals. +- [ ] T2: Add the exposure policy and separate runtime from configurable Agent tool catalogs. +- [ ] T3: Move renderer configuration callers to the side-effect-free configurable catalog. +- [ ] T4: Restrict the model-facing Tape surface to the search/context recall pair. +- [ ] T5: Make runtime handoff reject empty inherited summaries before appending an anchor. +- [ ] T6: Remove non-configurable Tape names from new and persisted disabled-tool state. +- [ ] T7: Update maintained tracked architecture contracts and focused regression tests. +- [ ] T8: Run format, i18n validation, lint, typecheck, and the full test suite. From 18aa7263265363915ed2bb38c069c98ee57097e8 Mon Sep 17 00:00:00 2001 From: yyhhyyyyyy Date: Wed, 15 Jul 2026 18:12:10 +0800 Subject: [PATCH 2/5] refactor(tools): split tool catalogs --- .../tape-capability-layering/tasks.md | 6 +- .../agentTools/agentTapeTools.ts | 10 +- .../agentTools/agentToolManager.ts | 47 +++++--- .../toolPresenter/agentTools/index.ts | 7 +- src/main/presenter/toolPresenter/index.ts | 60 ++++++++-- src/main/routes/index.ts | 2 +- src/renderer/api/ToolClient.ts | 4 +- .../components/DeepChatAgentsSettings.vue | 2 +- .../components/chat-input/McpIndicator.vue | 2 +- src/shared/agentTools.ts | 29 +++++ .../types/presenters/tool.presenter.d.ts | 5 + .../toolPresenter/toolPresenter.test.ts | 109 +++++++++++++++++- test/main/routes/dispatcher.test.ts | 41 +++++++ test/renderer/api/clients.test.ts | 2 +- .../components/DeepChatAgentsSettings.test.ts | 34 +++--- test/renderer/components/McpIndicator.test.ts | 12 +- 16 files changed, 301 insertions(+), 71 deletions(-) diff --git a/docs/architecture/tape-capability-layering/tasks.md b/docs/architecture/tape-capability-layering/tasks.md index 0243bd392..a3a2d06f5 100644 --- a/docs/architecture/tape-capability-layering/tasks.md +++ b/docs/architecture/tape-capability-layering/tasks.md @@ -1,8 +1,8 @@ # Tape Capability Layering - Tasks -- [ ] T1: Record the capability matrix, catalog contracts, compatibility rules, and non-goals. -- [ ] T2: Add the exposure policy and separate runtime from configurable Agent tool catalogs. -- [ ] T3: Move renderer configuration callers to the side-effect-free configurable catalog. +- [x] T1: Record the capability matrix, catalog contracts, compatibility rules, and non-goals. +- [x] T2: Add the exposure policy and separate runtime from configurable Agent tool catalogs. +- [x] T3: Move renderer configuration callers to the side-effect-free configurable catalog. - [ ] T4: Restrict the model-facing Tape surface to the search/context recall pair. - [ ] T5: Make runtime handoff reject empty inherited summaries before appending an anchor. - [ ] T6: Remove non-configurable Tape names from new and persisted disabled-tool state. diff --git a/src/main/presenter/toolPresenter/agentTools/agentTapeTools.ts b/src/main/presenter/toolPresenter/agentTools/agentTapeTools.ts index 413c9cbaa..3e982c063 100644 --- a/src/main/presenter/toolPresenter/agentTools/agentTapeTools.ts +++ b/src/main/presenter/toolPresenter/agentTools/agentTapeTools.ts @@ -2,17 +2,11 @@ import { z } from 'zod' import { toDeepChatJsonSchema } from '@shared/lib/zodJsonSchema' import type { MCPToolDefinition } from '@shared/presenter' import { createAgentToolSuccessResult } from '@shared/lib/agentToolResultEnvelope' +import { TAPE_TOOL_NAMES, type TapeToolName } from '@shared/agentTools' import type { AgentToolRuntimePort } from '../runtimePorts' import type { AgentToolCallResult } from './agentToolManager' export const AGENT_TAPE_TOOL_SERVER_NAME = 'agent-tape' -export const TAPE_TOOL_NAMES = { - info: 'tape_info', - search: 'tape_search', - context: 'tape_context', - anchors: 'tape_anchors', - handoff: 'tape_handoff' -} as const const tapeInfoSchema = z.object({}) @@ -132,8 +126,6 @@ const tapeToolSchemas = { [TAPE_TOOL_NAMES.handoff]: tapeHandoffSchema } -type TapeToolName = (typeof TAPE_TOOL_NAMES)[keyof typeof TAPE_TOOL_NAMES] - type TapeAnchorOverview = { name: string | null entryId: number diff --git a/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts b/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts index 30b4b1c02..5166014ba 100644 --- a/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts +++ b/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts @@ -41,7 +41,11 @@ import { AgentPlanTool, UPDATE_PLAN_TOOL_NAME } from './agentPlanTool' import { AgentTapeToolHandler } from './agentTapeTools' import { AgentMemoryToolHandler } from './agentMemoryTools' import { createAgentToolErrorResult } from '@shared/lib/agentToolResultEnvelope' -import { CRON_JOB_AGENT_TOOL_NAME } from '@shared/agentTools' +import { + CRON_JOB_AGENT_TOOL_NAME, + TAPE_TOOL_NAMES, + isUserConfigurableAgentTool +} from '@shared/agentTools' import { CRON_JOB_TOOL_SERVER_NAME, CronJobToolHandler, @@ -366,30 +370,41 @@ export class AgentToolManager { agentWorkspacePath: string | null conversationId?: string activeSkillNames?: string[] + catalogPurpose?: 'runtime' | 'configurable' }): Promise { const defs: MCPToolDefinition[] = [] const isAgentMode = context.chatMode === 'agent' - this.syncContext(context) + const isConfigurableCatalog = context.catalogPurpose === 'configurable' + const acceptsToolName = (toolName: string): boolean => + !isConfigurableCatalog || isUserConfigurableAgentTool(toolName) + const appendDefinitions = (definitions: MCPToolDefinition[]): void => { + defs.push(...definitions.filter((definition) => acceptsToolName(definition.function.name))) + } + + if (!isConfigurableCatalog) { + this.syncContext(context) + } // 1. FileSystem tools (agent mode only) - if (isAgentMode && this.fileSystemHandler) { + if (isAgentMode && (isConfigurableCatalog || this.fileSystemHandler)) { const fsDefs = this.getFileSystemToolDefinitions() - defs.push(...fsDefs) + appendDefinitions(fsDefs) } // 2. Built-in question tool (all modes) - defs.push(...this.getQuestionToolDefinitions()) + appendDefinitions(this.getQuestionToolDefinitions()) // 2.1. Progress checklist tool (deepchat regular sessions only) if (isAgentMode && this.planTool) { - defs.push(this.planTool.getToolDefinition()) + appendDefinitions([this.planTool.getToolDefinition()]) } // 2.15. Session tape tools (DeepChat sessions only) - if (isAgentMode && this.tapeToolHandler) { + const acceptsTapeTools = Object.values(TAPE_TOOL_NAMES).some(acceptsToolName) + if (isAgentMode && acceptsTapeTools && this.tapeToolHandler) { try { if (await this.tapeToolHandler.canUse(context.conversationId)) { - defs.push(...this.tapeToolHandler.getToolDefinitions()) + appendDefinitions(this.tapeToolHandler.getToolDefinitions()) } } catch (error) { logger.warn('[AgentToolManager] Failed to resolve tape tool availability', { error }) @@ -400,7 +415,7 @@ export class AgentToolManager { if (isAgentMode && this.memoryToolHandler) { try { if (await this.memoryToolHandler.canUse(context.conversationId)) { - defs.push(...this.memoryToolHandler.getToolDefinitions()) + appendDefinitions(this.memoryToolHandler.getToolDefinitions()) } } catch (error) { logger.warn('[AgentToolManager] Failed to resolve memory tool availability', { error }) @@ -411,7 +426,7 @@ export class AgentToolManager { if (isAgentMode && this.imageGenerationTool) { try { if (await this.imageGenerationTool.canUse(context.conversationId)) { - defs.push(this.imageGenerationTool.getToolDefinition()) + appendDefinitions([this.imageGenerationTool.getToolDefinition()]) } } catch (error) { logger.warn('[AgentToolManager] Failed to resolve image generation tool availability', { @@ -422,7 +437,7 @@ export class AgentToolManager { // 2.3. Scheduled task tool (disabled by default in DeepChat agent settings) if (isAgentMode && this.cronJobToolHandler?.canUse()) { - defs.push(this.cronJobToolHandler.getToolDefinition()) + appendDefinitions([this.cronJobToolHandler.getToolDefinition()]) } // 2.5. Subagent orchestration tool (deepchat regular sessions only) @@ -432,7 +447,7 @@ export class AgentToolManager { context.conversationId ) if (subagentToolDefinition) { - defs.push(subagentToolDefinition) + appendDefinitions([subagentToolDefinition]) } } catch (error) { logger.warn('[AgentToolManager] Failed to resolve subagent tool availability', { error }) @@ -442,13 +457,13 @@ export class AgentToolManager { // 3. Skill tools (agent mode only) if (isAgentMode && this.isSkillsEnabled()) { const skillDefs = this.getSkillToolDefinitions() - defs.push(...skillDefs) + appendDefinitions(skillDefs) if ( context.conversationId && (await this.hasRunnableSkillScripts(context.conversationId, context.activeSkillNames)) ) { - defs.push(this.getSkillRunToolDefinition()) + appendDefinitions([this.getSkillRunToolDefinition()]) } } @@ -475,7 +490,7 @@ export class AgentToolManager { : Array.from(new Set([...allowedTools, ...requiredSettingsTools])) const settingsDefs = buildChatSettingsToolDefinitions(effectiveAllowedTools) - defs.push(...settingsDefs) + appendDefinitions(settingsDefs) } } catch (error) { logger.warn('[AgentToolManager] Failed to load DeepChat settings tools', { error }) @@ -485,7 +500,7 @@ export class AgentToolManager { // 5. YoBrowser CDP tools (agent mode only) if (isAgentMode) { try { - defs.push(...this.getYoBrowserToolHandler().getToolDefinitions()) + appendDefinitions(this.getYoBrowserToolHandler().getToolDefinitions()) } catch (error) { logger.warn('[AgentToolManager] Failed to load YoBrowser tools', { error }) } diff --git a/src/main/presenter/toolPresenter/agentTools/index.ts b/src/main/presenter/toolPresenter/agentTools/index.ts index 760679537..c3d589722 100644 --- a/src/main/presenter/toolPresenter/agentTools/index.ts +++ b/src/main/presenter/toolPresenter/agentTools/index.ts @@ -13,11 +13,8 @@ export { CHAT_SETTINGS_TOOL_NAMES } from './chatSettingsTools' export { AGENT_CORE_TOOL_SERVER_NAME, UPDATE_PLAN_TOOL_NAME, AgentPlanTool } from './agentPlanTool' -export { - AGENT_TAPE_TOOL_SERVER_NAME, - TAPE_TOOL_NAMES, - AgentTapeToolHandler -} from './agentTapeTools' +export { AGENT_TAPE_TOOL_SERVER_NAME, AgentTapeToolHandler } from './agentTapeTools' +export { TAPE_TOOL_NAMES } from '@shared/agentTools' export { CRON_JOB_TOOL_SERVER_NAME, CronJobToolHandler, diff --git a/src/main/presenter/toolPresenter/index.ts b/src/main/presenter/toolPresenter/index.ts index 524df45eb..e002ae407 100644 --- a/src/main/presenter/toolPresenter/index.ts +++ b/src/main/presenter/toolPresenter/index.ts @@ -15,13 +15,16 @@ import type { PermissionMode } from '@shared/types/agent-interface' import { resolveToolOffloadTemplatePath } from '@/agent/shared/storage/sessionPaths' import { QUESTION_TOOL_NAME } from '@/presenter/toolPresenter/agentTools/questionTool' import { ToolMapper, type ToolSource } from './toolMapper' -import { CRON_JOB_AGENT_TOOL_NAME } from '@shared/agentTools' +import { + CRON_JOB_AGENT_TOOL_NAME, + TAPE_TOOL_NAMES, + isUserConfigurableAgentTool +} from '@shared/agentTools' import { AgentToolManager, IMAGE_GENERATE_TOOL_NAME, UPDATE_PLAN_TOOL_NAME, AGENT_TAPE_TOOL_SERVER_NAME, - TAPE_TOOL_NAMES, CRON_JOB_TOOL_SERVER_NAME, type AgentToolCallResult } from './agentTools' @@ -36,6 +39,7 @@ import { YO_BROWSER_TOOL_NAMES } from '../browser/YoBrowserToolDefinitions' export interface IToolPresenter { getAllToolDefinitions(context: ToolDefinitionContext): Promise + getConfigurableAgentToolDefinitions(context: ToolDefinitionContext): Promise syncAgentToolContext?(context: { chatMode?: 'agent' | 'acp agent' agentWorkspacePath?: string | null @@ -122,14 +126,18 @@ export class ToolPresenter implements IToolPresenter { this.conversationMappers = new Map() } + private createAgentToolManager(agentWorkspacePath: string | null): AgentToolManager { + return new AgentToolManager({ + agentWorkspacePath, + configPresenter: this.options.configPresenter, + commandPermissionHandler: this.options.commandPermissionHandler, + runtimePort: this.options.agentToolRuntime + }) + } + private ensureAgentToolManager(agentWorkspacePath: string | null): AgentToolManager { if (!this.agentToolManager) { - this.agentToolManager = new AgentToolManager({ - agentWorkspacePath, - configPresenter: this.options.configPresenter, - commandPermissionHandler: this.options.commandPermissionHandler, - runtimePort: this.options.agentToolRuntime - }) + this.agentToolManager = this.createAgentToolManager(agentWorkspacePath) } return this.agentToolManager @@ -189,7 +197,9 @@ export class ToolPresenter implements IToolPresenter { return false }) const filteredAgentDefs = dedupedAgentDefs.filter( - (tool) => !disabledAgentToolSet.has(tool.function.name) + (tool) => + !isUserConfigurableAgentTool(tool.function.name) || + !disabledAgentToolSet.has(tool.function.name) ) defs.push(...filteredAgentDefs) mapper.registerTools(filteredAgentDefs, 'agent') @@ -201,6 +211,38 @@ export class ToolPresenter implements IToolPresenter { return defs } + /** + * Get only user-configurable Agent tool definitions for renderer settings. + * This query intentionally does not touch runtime mappings or MCP access context. + */ + async getConfigurableAgentToolDefinitions( + context: ToolDefinitionContext + ): Promise { + const chatMode = context.chatMode || 'agent' + const supportsVision = context.supportsVision || false + const agentWorkspacePath = context.agentWorkspacePath || null + const agentToolManager = this.createAgentToolManager(null) + + try { + const agentDefs = withToolSource( + await agentToolManager.getAllToolDefinitions({ + chatMode, + supportsVision, + agentWorkspacePath, + conversationId: context.conversationId, + activeSkillNames: context.activeSkillNames, + catalogPurpose: 'configurable' + }), + 'agent' + ) + + return agentDefs.filter((tool) => isUserConfigurableAgentTool(tool.function.name)) + } catch (error) { + console.warn('[ToolPresenter] Failed to load configurable Agent tool definitions', error) + return [] + } + } + syncAgentToolContext(context: { chatMode?: 'agent' | 'acp agent' agentWorkspacePath?: string | null diff --git a/src/main/routes/index.ts b/src/main/routes/index.ts index 449f6a66f..fd1ae308e 100644 --- a/src/main/routes/index.ts +++ b/src/main/routes/index.ts @@ -4265,7 +4265,7 @@ export async function dispatchDeepchatRoute( case toolsListDefinitionsRoute.name: { const input = toolsListDefinitionsRoute.input.parse(rawInput) - const tools = await runtime.toolPresenter.getAllToolDefinitions(input) + const tools = await runtime.toolPresenter.getConfigurableAgentToolDefinitions(input) return toolsListDefinitionsRoute.output.parse({ tools }) } diff --git a/src/renderer/api/ToolClient.ts b/src/renderer/api/ToolClient.ts index a8dd23902..bc48f9951 100644 --- a/src/renderer/api/ToolClient.ts +++ b/src/renderer/api/ToolClient.ts @@ -3,7 +3,7 @@ import { toolsListDefinitionsRoute } from '@shared/contracts/routes' import { getDeepchatBridge } from './core' export function createToolClient(bridge: DeepchatBridge = getDeepchatBridge()) { - async function getAllToolDefinitions(context: { + async function getConfigurableAgentToolDefinitions(context: { enabledMcpTools?: string[] disabledAgentTools?: string[] chatMode?: 'agent' | 'acp agent' @@ -16,7 +16,7 @@ export function createToolClient(bridge: DeepchatBridge = getDeepchatBridge()) { } return { - getAllToolDefinitions + getConfigurableAgentToolDefinitions } } diff --git a/src/renderer/settings/components/DeepChatAgentsSettings.vue b/src/renderer/settings/components/DeepChatAgentsSettings.vue index b79957b4d..671ac3e89 100644 --- a/src/renderer/settings/components/DeepChatAgentsSettings.vue +++ b/src/renderer/settings/components/DeepChatAgentsSettings.vue @@ -1470,7 +1470,7 @@ const loadRecentProjects = async () => { } const loadTools = async () => { try { - const definitions = await toolClient.getAllToolDefinitions({ chatMode: 'agent' }) + const definitions = await toolClient.getConfigurableAgentToolDefinitions({ chatMode: 'agent' }) tools.value = Array.isArray(definitions) ? definitions .filter((tool) => tool.source === 'agent') diff --git a/src/renderer/src/components/chat-input/McpIndicator.vue b/src/renderer/src/components/chat-input/McpIndicator.vue index 368a699f5..f3f322bbd 100644 --- a/src/renderer/src/components/chat-input/McpIndicator.vue +++ b/src/renderer/src/components/chat-input/McpIndicator.vue @@ -574,7 +574,7 @@ const loadDeepchatTools = async () => { try { const [toolDefinitions, persistedDisabledTools] = await Promise.all([ - toolClient.getAllToolDefinitions({ + toolClient.getConfigurableAgentToolDefinitions({ chatMode: 'agent', conversationId: deepchatSessionId.value ?? undefined, agentWorkspacePath: workspacePath.value diff --git a/src/shared/agentTools.ts b/src/shared/agentTools.ts index 167707c89..6324ff2d2 100644 --- a/src/shared/agentTools.ts +++ b/src/shared/agentTools.ts @@ -1,2 +1,31 @@ export const CRON_JOB_AGENT_TOOL_NAME = 'cronjob' export const DEFAULT_DISABLED_AGENT_TOOLS = [CRON_JOB_AGENT_TOOL_NAME] as const + +export const TAPE_TOOL_NAMES = Object.freeze({ + info: 'tape_info', + search: 'tape_search', + context: 'tape_context', + anchors: 'tape_anchors', + handoff: 'tape_handoff' +} as const) + +export type TapeToolName = (typeof TAPE_TOOL_NAMES)[keyof typeof TAPE_TOOL_NAMES] + +export type AgentToolExposure = 'user-configurable' | 'system-model' | 'diagnostic' | 'runtime-only' + +const AGENT_TOOL_EXPOSURE_BY_NAME: Readonly> = + Object.freeze({ + [TAPE_TOOL_NAMES.search]: 'system-model', + [TAPE_TOOL_NAMES.context]: 'system-model', + [TAPE_TOOL_NAMES.info]: 'diagnostic', + [TAPE_TOOL_NAMES.anchors]: 'diagnostic', + [TAPE_TOOL_NAMES.handoff]: 'runtime-only' + }) + +export const getAgentToolExposure = (toolName: string): AgentToolExposure => + Object.prototype.hasOwnProperty.call(AGENT_TOOL_EXPOSURE_BY_NAME, toolName) + ? AGENT_TOOL_EXPOSURE_BY_NAME[toolName as TapeToolName] + : 'user-configurable' + +export const isUserConfigurableAgentTool = (toolName: string): boolean => + getAgentToolExposure(toolName) === 'user-configurable' diff --git a/src/shared/types/presenters/tool.presenter.d.ts b/src/shared/types/presenters/tool.presenter.d.ts index 5be98893c..d76d462ee 100644 --- a/src/shared/types/presenters/tool.presenter.d.ts +++ b/src/shared/types/presenters/tool.presenter.d.ts @@ -79,6 +79,11 @@ export interface IToolPresenter { */ getAllToolDefinitions(context: ToolDefinitionContext): Promise + /** + * Get only Agent tools that users may enable or disable. + */ + getConfigurableAgentToolDefinitions(context: ToolDefinitionContext): Promise + /** * Synchronize agent-tool runtime state without rebuilding tool schemas. */ diff --git a/test/main/presenter/toolPresenter/toolPresenter.test.ts b/test/main/presenter/toolPresenter/toolPresenter.test.ts index 23eb992f5..2320b0a19 100644 --- a/test/main/presenter/toolPresenter/toolPresenter.test.ts +++ b/test/main/presenter/toolPresenter/toolPresenter.test.ts @@ -9,7 +9,7 @@ import { } from '@/presenter/toolPresenter/agentTools' import { CommandPermissionService } from '@/presenter/permission' import { IMAGE_GENERATE_TOOL_NAME } from '@shared/agentImageGenerationTool' -import { CRON_JOB_AGENT_TOOL_NAME } from '@shared/agentTools' +import { CRON_JOB_AGENT_TOOL_NAME, getAgentToolExposure } from '@shared/agentTools' vi.mock('electron', () => ({ app: { @@ -444,6 +444,113 @@ describe('ToolPresenter', () => { expect(defs.some((tool) => tool.function.name === 'ls')).toBe(false) }) + it('uses one exposure policy for Tape tools and defaults existing tools to configurable', () => { + expect(getAgentToolExposure(TAPE_TOOL_NAMES.search)).toBe('system-model') + expect(getAgentToolExposure(TAPE_TOOL_NAMES.context)).toBe('system-model') + expect(getAgentToolExposure(TAPE_TOOL_NAMES.info)).toBe('diagnostic') + expect(getAgentToolExposure(TAPE_TOOL_NAMES.anchors)).toBe('diagnostic') + expect(getAgentToolExposure(TAPE_TOOL_NAMES.handoff)).toBe('runtime-only') + expect(getAgentToolExposure('read')).toBe('user-configurable') + expect(getAgentToolExposure('__proto__')).toBe('user-configurable') + }) + + it('keeps non-configurable Tape tools in the runtime catalog despite stale disabled values', async () => { + const toolPresenter = new ToolPresenter({ + mcpPresenter: { + getAllToolDefinitions: vi.fn().mockResolvedValue([]) + } as any, + configPresenter: { + getSkillsEnabled: vi.fn().mockReturnValue(false), + getSkillsPath: vi.fn().mockReturnValue('C:\\skills'), + getModelConfig: vi.fn() + } as any, + commandPermissionHandler: new CommandPermissionService(), + agentToolRuntime: buildAgentToolRuntimeMock({ + resolveConversationSessionInfo: vi.fn().mockResolvedValue({ agentType: 'deepchat' }), + getTapeInfo: vi.fn(), + searchTape: vi.fn(), + getTapeContext: vi.fn(), + listTapeAnchors: vi.fn(), + handoffTape: vi.fn() + }) + }) + + const defs = await toolPresenter.getAllToolDefinitions({ + disabledAgentTools: Object.values(TAPE_TOOL_NAMES), + chatMode: 'agent', + supportsVision: false, + agentWorkspacePath: 'C:\\runtime-workspace', + conversationId: 'conv-1' + }) + const names = defs.map((tool) => tool.function.name) + + expect(names).toEqual(expect.arrayContaining(Object.values(TAPE_TOOL_NAMES))) + }) + + it('reads configurable definitions without publishing mappings or mutating runtime context', async () => { + const mcpPresenter = { + getAllToolDefinitions: vi + .fn() + .mockResolvedValue([buildToolDefinition('mcp_only', 'mcp-server')]), + callTool: vi.fn().mockResolvedValue({ content: 'mcp-result' }) + } as any + const toolPresenter = new ToolPresenter({ + mcpPresenter, + configPresenter: { + getSkillsEnabled: vi.fn().mockReturnValue(false), + getSkillsPath: vi.fn().mockReturnValue('C:\\skills'), + getModelConfig: vi.fn() + } as any, + commandPermissionHandler: new CommandPermissionService(), + agentToolRuntime: buildAgentToolRuntimeMock({ + resolveConversationSessionInfo: vi.fn().mockResolvedValue({ agentType: 'deepchat' }), + getTapeInfo: vi.fn(), + searchTape: vi.fn(), + getTapeContext: vi.fn(), + listTapeAnchors: vi.fn(), + handoffTape: vi.fn() + }) + }) + + await toolPresenter.getAllToolDefinitions({ + chatMode: 'agent', + supportsVision: false, + agentWorkspacePath: 'C:\\runtime-workspace', + conversationId: 'conv-1', + agentId: 'agent-1', + enabledMcpServerIds: ['mcp-server'] + }) + const runtimeManager = (toolPresenter as any).agentToolManager + + const configurableDefs = await toolPresenter.getConfigurableAgentToolDefinitions({ + chatMode: 'agent', + supportsVision: false, + agentWorkspacePath: 'C:\\settings-workspace', + conversationId: 'conv-1', + disabledAgentTools: ['read'] + }) + const tapeToolNames = new Set(Object.values(TAPE_TOOL_NAMES)) + + expect(configurableDefs.some((tool) => tool.function.name === 'read')).toBe(true) + expect(configurableDefs.some((tool) => tapeToolNames.has(tool.function.name))).toBe(false) + expect(configurableDefs.every((tool) => tool.source === 'agent')).toBe(true) + expect(mcpPresenter.getAllToolDefinitions).toHaveBeenCalledTimes(1) + expect((toolPresenter as any).agentToolManager).toBe(runtimeManager) + expect(runtimeManager.agentWorkspacePath).toBe('C:\\runtime-workspace') + + await toolPresenter.callTool({ + id: 'tool-1', + type: 'function', + function: { name: 'mcp_only', arguments: '{}' }, + conversationId: 'conv-1' + }) + + expect(mcpPresenter.callTool).toHaveBeenCalledWith( + expect.objectContaining({ function: expect.objectContaining({ name: 'mcp_only' }) }), + expect.objectContaining({ agentId: 'agent-1', enabledServerIds: ['mcp-server'] }) + ) + }) + it('exposes cronjob only when runtime ports are available and the tool is enabled', async () => { const toolPresenter = new ToolPresenter({ mcpPresenter: { diff --git a/test/main/routes/dispatcher.test.ts b/test/main/routes/dispatcher.test.ts index fb9d2c4b8..a8e91f980 100644 --- a/test/main/routes/dispatcher.test.ts +++ b/test/main/routes/dispatcher.test.ts @@ -643,6 +643,25 @@ function createRuntime() { listInstalledServerIds: vi.fn().mockResolvedValue(['context7']), updateMcpRouterServersAuth: vi.fn().mockResolvedValue(undefined) } as unknown as IMCPPresenter + const toolPresenter = { + getAllToolDefinitions: vi.fn().mockResolvedValue([]), + getConfigurableAgentToolDefinitions: vi.fn().mockResolvedValue([ + { + type: 'function', + source: 'agent', + function: { + name: 'read', + description: 'Read a file', + parameters: { type: 'object', properties: {} } + }, + server: { + name: 'agent-filesystem', + icons: '', + description: 'Agent filesystem tools' + } + } + ]) + } const remoteControlPresenter = { listRemoteChannels: vi.fn().mockResolvedValue([ { @@ -1313,6 +1332,7 @@ function createRuntime() { exporter, oauthPresenter, mcpPresenter, + toolPresenter: toolPresenter as any, remoteControlPresenter, shortcutPresenter, sqlitePresenter, @@ -1344,6 +1364,7 @@ function createRuntime() { exporter, oauthPresenter, mcpPresenter, + toolPresenter, remoteControlPresenter, shortcutPresenter, sqlitePresenter, @@ -1365,6 +1386,26 @@ function createRuntime() { } describe('dispatchDeepchatRoute', () => { + it('routes tools.listDefinitions to the configurable Agent catalog', async () => { + const { runtime, toolPresenter } = createRuntime() + const input = { + chatMode: 'agent' as const, + conversationId: 'session-1', + disabledAgentTools: ['read'] + } + + const result = await dispatchDeepchatRoute(runtime, 'tools.listDefinitions', input, { + webContentsId: 42, + windowId: 7 + }) + + expect(result).toMatchObject({ + tools: [{ source: 'agent', function: { name: 'read' } }] + }) + expect(toolPresenter.getConfigurableAgentToolDefinitions).toHaveBeenCalledWith(input) + expect(toolPresenter.getAllToolDefinitions).not.toHaveBeenCalled() + }) + it('dispatches Cron Jobs routes through the runtime service', async () => { const { runtime, cronJobs } = createRuntime() const context = { diff --git a/test/renderer/api/clients.test.ts b/test/renderer/api/clients.test.ts index 2f1d24536..38cf524b3 100644 --- a/test/renderer/api/clients.test.ts +++ b/test/renderer/api/clients.test.ts @@ -2660,7 +2660,7 @@ describe('renderer api clients', () => { await projectClient.pathExists('/workspace') await projectClient.selectDirectory() const unsubscribe = projectClient.onEnvironmentsChanged(() => undefined) - await toolClient.getAllToolDefinitions({ chatMode: 'agent' }) + await toolClient.getConfigurableAgentToolDefinitions({ chatMode: 'agent' }) expect(bridge.invoke).toHaveBeenNthCalledWith(1, 'device.selectFiles', { filters: [{ name: 'ZIP Files', extensions: ['zip'] }] diff --git a/test/renderer/components/DeepChatAgentsSettings.test.ts b/test/renderer/components/DeepChatAgentsSettings.test.ts index 52b77affd..6f42c3ca4 100644 --- a/test/renderer/components/DeepChatAgentsSettings.test.ts +++ b/test/renderer/components/DeepChatAgentsSettings.test.ts @@ -84,7 +84,7 @@ const clientMocks = vi.hoisted(() => ({ selectDirectory: vi.fn() }, toolClient: { - getAllToolDefinitions: vi.fn() + getConfigurableAgentToolDefinitions: vi.fn() }, sessionClient: { getAgentTransferImpact: vi.fn(), @@ -99,7 +99,7 @@ type ProjectClientMockSource = { selectDirectory: () => Promise } type ToolClientMockSource = { - getAllToolDefinitions: (context: unknown) => Promise + getConfigurableAgentToolDefinitions: (context: unknown) => Promise } const bindClientMocks = ( @@ -112,8 +112,8 @@ const bindClientMocks = ( clientMocks.projectClient.selectDirectory.mockImplementation(() => projectPresenter.selectDirectory() ) - clientMocks.toolClient.getAllToolDefinitions.mockImplementation((context: unknown) => - toolPresenter.getAllToolDefinitions(context) + clientMocks.toolClient.getConfigurableAgentToolDefinitions.mockImplementation( + (context: unknown) => toolPresenter.getConfigurableAgentToolDefinitions(context) ) } @@ -152,7 +152,7 @@ describe('DeepChatAgentsSettings', () => { vi.clearAllMocks() clientMocks.projectClient.listRecent.mockReset() clientMocks.projectClient.selectDirectory.mockReset() - clientMocks.toolClient.getAllToolDefinitions.mockReset() + clientMocks.toolClient.getConfigurableAgentToolDefinitions.mockReset() clientMocks.sessionClient.getAgentTransferImpact.mockReset() clientMocks.sessionClient.deleteAgentSessions.mockReset() clientMocks.sessionClient.moveAgentSessions.mockReset() @@ -190,7 +190,7 @@ describe('DeepChatAgentsSettings', () => { ...options.configPresenter } const toolPresenter = { - getAllToolDefinitions: vi.fn().mockResolvedValue(options.toolDefinitions ?? []) + getConfigurableAgentToolDefinitions: vi.fn().mockResolvedValue(options.toolDefinitions ?? []) } const projectPresenter = options.projectPresenter ?? { getRecentProjects: vi.fn().mockResolvedValue([]), @@ -346,7 +346,7 @@ describe('DeepChatAgentsSettings', () => { .mockResolvedValue({ removed: true, cleanupPendingRestart: false }) } const toolPresenter = { - getAllToolDefinitions: vi.fn().mockResolvedValue([ + getConfigurableAgentToolDefinitions: vi.fn().mockResolvedValue([ { source: 'agent', function: { name: 'tool_alpha', description: 'Alpha tool' }, @@ -648,7 +648,7 @@ describe('DeepChatAgentsSettings', () => { .mockResolvedValue({ removed: true, cleanupPendingRestart: false }) } const toolPresenter = { - getAllToolDefinitions: vi.fn().mockResolvedValue([]) + getConfigurableAgentToolDefinitions: vi.fn().mockResolvedValue([]) } const projectPresenter = { getRecentProjects: vi.fn().mockResolvedValue([]), @@ -745,7 +745,7 @@ describe('DeepChatAgentsSettings', () => { .mockResolvedValue({ removed: true, cleanupPendingRestart: false }) } const toolPresenter = { - getAllToolDefinitions: vi.fn().mockResolvedValue([]) + getConfigurableAgentToolDefinitions: vi.fn().mockResolvedValue([]) } const projectPresenter = { getRecentProjects: vi.fn().mockResolvedValue([]), @@ -856,7 +856,7 @@ describe('DeepChatAgentsSettings', () => { .mockResolvedValue({ removed: true, cleanupPendingRestart: false }) } const toolPresenter = { - getAllToolDefinitions: vi.fn().mockResolvedValue([]) + getConfigurableAgentToolDefinitions: vi.fn().mockResolvedValue([]) } const projectPresenter = { getRecentProjects: vi.fn().mockResolvedValue([]), @@ -1028,7 +1028,9 @@ describe('DeepChatAgentsSettings', () => { .fn() .mockResolvedValue({ removed: true, cleanupPendingRestart: false }) } - const toolPresenter = { getAllToolDefinitions: vi.fn().mockResolvedValue([]) } + const toolPresenter = { + getConfigurableAgentToolDefinitions: vi.fn().mockResolvedValue([]) + } const projectPresenter = { getRecentProjects: vi.fn().mockResolvedValue([]), selectDirectory: vi.fn().mockResolvedValue(null) @@ -1186,7 +1188,7 @@ describe('DeepChatAgentsSettings', () => { .mockResolvedValue({ removed: true, cleanupPendingRestart: false }) } const toolPresenter = { - getAllToolDefinitions: vi.fn().mockResolvedValue([]) + getConfigurableAgentToolDefinitions: vi.fn().mockResolvedValue([]) } const projectPresenter = { getRecentProjects: vi.fn().mockResolvedValue([]), @@ -1304,7 +1306,7 @@ describe('DeepChatAgentsSettings', () => { .mockResolvedValue({ removed: true, cleanupPendingRestart: false }) } const toolPresenter = { - getAllToolDefinitions: vi.fn().mockResolvedValue([]) + getConfigurableAgentToolDefinitions: vi.fn().mockResolvedValue([]) } const projectPresenter = { getRecentProjects: vi.fn().mockResolvedValue([]), @@ -1444,7 +1446,7 @@ describe('DeepChatAgentsSettings', () => { .mockResolvedValue({ removed: true, cleanupPendingRestart: false }) } const toolPresenter = { - getAllToolDefinitions: vi.fn().mockResolvedValue([]) + getConfigurableAgentToolDefinitions: vi.fn().mockResolvedValue([]) } const projectPresenter = { getRecentProjects: vi.fn().mockResolvedValue([]), @@ -1581,7 +1583,7 @@ describe('DeepChatAgentsSettings', () => { .mockResolvedValue({ removed: true, cleanupPendingRestart: false }) } const toolPresenter = { - getAllToolDefinitions: vi.fn().mockResolvedValue([]) + getConfigurableAgentToolDefinitions: vi.fn().mockResolvedValue([]) } const projectPresenter = { getRecentProjects: vi.fn().mockResolvedValue([]), @@ -1756,7 +1758,7 @@ describe('DeepChatAgentsSettings', () => { .mockResolvedValue({ removed: true, cleanupPendingRestart: false }) } const toolPresenter = { - getAllToolDefinitions: vi.fn().mockResolvedValue([]) + getConfigurableAgentToolDefinitions: vi.fn().mockResolvedValue([]) } const projectPresenter = { getRecentProjects: vi.fn().mockResolvedValue([]), diff --git a/test/renderer/components/McpIndicator.test.ts b/test/renderer/components/McpIndicator.test.ts index 806990534..ee7e09e42 100644 --- a/test/renderer/components/McpIndicator.test.ts +++ b/test/renderer/components/McpIndicator.test.ts @@ -124,7 +124,7 @@ const setup = async (options?: { }) const toolPresenter = { - getAllToolDefinitions: vi + getConfigurableAgentToolDefinitions: vi .fn() .mockResolvedValue([ buildTool('read', 'agent-filesystem'), @@ -168,7 +168,7 @@ const setup = async (options?: { })) vi.doMock('@api/ToolClient', () => ({ createToolClient: vi.fn(() => ({ - getAllToolDefinitions: toolPresenter.getAllToolDefinitions + getConfigurableAgentToolDefinitions: toolPresenter.getConfigurableAgentToolDefinitions })) })) vi.doMock('@api/SessionClient', () => ({ @@ -371,7 +371,7 @@ describe('McpIndicator', () => { const buttons = wrapper.findAll('button') expect(buttons[0].text()).toContain('MCP 1') expect(wrapper.text()).not.toContain('Tools') - expect(toolPresenter.getAllToolDefinitions).not.toHaveBeenCalled() + expect(toolPresenter.getConfigurableAgentToolDefinitions).not.toHaveBeenCalled() }) it('renders plugin-owned MCP tools in a separate plugin section', async () => { @@ -447,7 +447,7 @@ describe('McpIndicator', () => { activeAgentId: 'deepchat' }) - toolPresenter.getAllToolDefinitions.mockClear() + toolPresenter.getConfigurableAgentToolDefinitions.mockClear() skillEvents.emitSessionChanged({ conversationId: 's1', skills: ['deepchat-settings'], @@ -455,8 +455,8 @@ describe('McpIndicator', () => { }) await flushPromises() - expect(toolPresenter.getAllToolDefinitions).toHaveBeenCalledTimes(1) - expect(toolPresenter.getAllToolDefinitions).toHaveBeenCalledWith({ + expect(toolPresenter.getConfigurableAgentToolDefinitions).toHaveBeenCalledTimes(1) + expect(toolPresenter.getConfigurableAgentToolDefinitions).toHaveBeenCalledWith({ chatMode: 'agent', conversationId: 's1', agentWorkspacePath: '/tmp/workspace' From 32e87d8e2abd241593c03c65abe40ac76eb1df7e Mon Sep 17 00:00:00 2001 From: yyhhyyyyyy Date: Wed, 15 Jul 2026 18:23:46 +0800 Subject: [PATCH 3/5] fix(tape): limit model capabilities --- .../tape-capability-layering/tasks.md | 4 +- src/main/agent/shared/agentSharedData.ts | 3 +- .../deferredToolExecutor.ts | 8 +- .../presenter/agentRuntimePresenter/index.ts | 8 +- .../agentRuntimePresenter/tapeService.ts | 24 ++- src/main/presenter/index.ts | 9 - .../projectionCoordinator.ts | 3 +- .../agentTools/agentTapeTools.ts | 178 ++++-------------- .../agentTools/agentToolManager.ts | 7 +- src/main/presenter/toolPresenter/index.ts | 22 +-- .../presenter/toolPresenter/runtimePorts.ts | 13 -- src/shared/agentTools.ts | 5 +- src/shared/types/agent-interface.d.ts | 4 + .../agentRuntimePresenter.test.ts | 37 ++++ .../agentRuntimePresenter/tapeService.test.ts | 46 ++--- .../projectionCoordinator.test.ts | 2 +- .../agentTools/agentTapeTools.test.ts | 129 ++++++------- .../toolPresenter/toolPresenter.test.ts | 51 ++++- 18 files changed, 245 insertions(+), 308 deletions(-) diff --git a/docs/architecture/tape-capability-layering/tasks.md b/docs/architecture/tape-capability-layering/tasks.md index a3a2d06f5..bfe403534 100644 --- a/docs/architecture/tape-capability-layering/tasks.md +++ b/docs/architecture/tape-capability-layering/tasks.md @@ -3,8 +3,8 @@ - [x] T1: Record the capability matrix, catalog contracts, compatibility rules, and non-goals. - [x] T2: Add the exposure policy and separate runtime from configurable Agent tool catalogs. - [x] T3: Move renderer configuration callers to the side-effect-free configurable catalog. -- [ ] T4: Restrict the model-facing Tape surface to the search/context recall pair. -- [ ] T5: Make runtime handoff reject empty inherited summaries before appending an anchor. +- [x] T4: Restrict the model-facing Tape surface to the search/context recall pair. +- [x] T5: Make runtime handoff reject empty inherited summaries before appending an anchor. - [ ] T6: Remove non-configurable Tape names from new and persisted disabled-tool state. - [ ] T7: Update maintained tracked architecture contracts and focused regression tests. - [ ] T8: Run format, i18n validation, lint, typecheck, and the full test suite. diff --git a/src/main/agent/shared/agentSharedData.ts b/src/main/agent/shared/agentSharedData.ts index 1be832293..58b421e34 100644 --- a/src/main/agent/shared/agentSharedData.ts +++ b/src/main/agent/shared/agentSharedData.ts @@ -3,6 +3,7 @@ import type { AgentTapeAnchorsOptions, AgentTapeContextOptions, AgentTapeContextResult, + AgentTapeHandoffState, AgentTapeInfo, AgentTapeSearchOptions, AgentTapeSearchResult, @@ -85,7 +86,7 @@ export interface AgentTapePort { handoffTape( sessionId: string, name: string, - state?: Record + state: AgentTapeHandoffState ): Promise listMessageViewManifests( sessionId: string, diff --git a/src/main/presenter/agentRuntimePresenter/deferredToolExecutor.ts b/src/main/presenter/agentRuntimePresenter/deferredToolExecutor.ts index 484b8d462..692865063 100644 --- a/src/main/presenter/agentRuntimePresenter/deferredToolExecutor.ts +++ b/src/main/presenter/agentRuntimePresenter/deferredToolExecutor.ts @@ -6,6 +6,7 @@ import { extractToolCallImagePreviews } from '@/lib/toolCallImagePreviews' import type { PendingToolInteraction } from './types' import type { DeepChatToolResolver } from './toolResolver' import { toolContentToText } from './toolAdapters' +import { isUserConfigurableAgentTool } from '@shared/agentTools' export type DeferredToolExecutionResult = { responseText: string @@ -107,9 +108,10 @@ export class DeferredToolExecutor { if (!toolDefinition) { const disabledAgentTools = this.dependencies.toolResolver.getDisabledAgentTools(sessionId) return { - responseText: disabledAgentTools.includes(toolName) - ? `Tool '${toolName}' is disabled for the current session.` - : `Tool '${toolName}' is no longer available in the current session.`, + responseText: + isUserConfigurableAgentTool(toolName) && disabledAgentTools.includes(toolName) + ? `Tool '${toolName}' is disabled for the current session.` + : `Tool '${toolName}' is no longer available in the current session.`, isError: true } } diff --git a/src/main/presenter/agentRuntimePresenter/index.ts b/src/main/presenter/agentRuntimePresenter/index.ts index 5ec427429..1e5703aa8 100644 --- a/src/main/presenter/agentRuntimePresenter/index.ts +++ b/src/main/presenter/agentRuntimePresenter/index.ts @@ -5,6 +5,7 @@ import type { AgentTapeAnchorsOptions, AgentTapeContextOptions, AgentTapeContextResult, + AgentTapeHandoffState, AgentTapeInfo, AgentTapeSearchOptions, AgentTapeSearchResult, @@ -79,7 +80,7 @@ import { } from './compactionService' import { reviewAutoApproveToolPermission } from './toolPermissionReviewer' import { buildTerminalErrorBlocks, DeepChatMessageStore } from './messageStore' -import { DeepChatTapeService } from './tapeService' +import { DeepChatTapeService, normalizeTapeHandoffState } from './tapeService' import { PendingInputCoordinator } from '@/agent/deepchat/pending/pendingInputCoordinator' import { DeepChatPendingInputStore } from '@/agent/deepchat/pending/pendingInputStore' import { DeepChatSessionStore, type SessionSummaryState } from './sessionStore' @@ -1602,10 +1603,11 @@ export class AgentRuntimePresenter { async handoffTape( sessionId: string, name: string, - state: Record = {} + state: AgentTapeHandoffState ): Promise { + const normalizedState = normalizeTapeHandoffState(state) this.tapeService.ensureSessionTapeReady(sessionId, this.messageStore) - const row = this.tapeService.handoff(sessionId, name, state) + const row = this.tapeService.handoff(sessionId, name, normalizedState) return this.toTapeAnchorResult(row) } diff --git a/src/main/presenter/agentRuntimePresenter/tapeService.ts b/src/main/presenter/agentRuntimePresenter/tapeService.ts index 6fc09f009..f43d652f0 100644 --- a/src/main/presenter/agentRuntimePresenter/tapeService.ts +++ b/src/main/presenter/agentRuntimePresenter/tapeService.ts @@ -8,6 +8,7 @@ import type { AgentTapeContextEntry, AgentTapeContextOptions, AgentTapeContextResult, + AgentTapeHandoffState, AgentTapeSearchOptions, ChatMessageRecord } from '@shared/types/agent-interface' @@ -654,6 +655,22 @@ function enrichHandoffState( return enrichedState } +export function normalizeTapeHandoffState(state: unknown): AgentTapeHandoffState { + if (!state || typeof state !== 'object' || Array.isArray(state)) { + throw new Error('Tape handoff requires a non-empty summary.') + } + + const summary = (state as Record).summary + if (typeof summary !== 'string' || !summary.trim()) { + throw new Error('Tape handoff requires a non-empty summary.') + } + + return { + ...(state as Record), + summary: summary.trim() + } +} + function forkSessionId(parentSessionId: string, forkId: string): string { return `${parentSessionId}::fork::${forkId}` } @@ -1465,16 +1482,17 @@ export class DeepChatTapeService implements Pick handoff( sessionId: string, name: string, - state: Record = {}, + state: AgentTapeHandoffState, meta: Record = {} ): DeepChatTapeEntryRow { + const normalizedState = normalizeTapeHandoffState(state) const table = this.table if (!table) { throw new Error('Tape table is not available.') } table.ensureBootstrapAnchor(sessionId) - const handoffState = enrichHandoffState(state, this.getMessageRecords(sessionId)) + const handoffState = enrichHandoffState(normalizedState, this.getMessageRecords(sessionId)) return table.appendAnchor({ sessionId, name: normalizeHandoffName(name), @@ -1494,7 +1512,7 @@ export class DeepChatTapeService implements Pick handoffResult( sessionId: string, name: string, - state: Record = {}, + state: AgentTapeHandoffState, meta: Record = {} ): TapeAnchorResult { return this.toAnchorResult(this.handoff(sessionId, name, state, meta)) diff --git a/src/main/presenter/index.ts b/src/main/presenter/index.ts index 93d2e9365..3cb68c9b0 100644 --- a/src/main/presenter/index.ts +++ b/src/main/presenter/index.ts @@ -355,9 +355,6 @@ export class Presenter implements IPresenter { availableSubagentSlots } }, - getTapeInfo: async (conversationId) => { - return await this.sessionProjectionCoordinator.getTapeInfo(conversationId) - }, searchTape: async (conversationId, query, options) => { return await this.sessionProjectionCoordinator.searchTape(conversationId, query, options) }, @@ -368,12 +365,6 @@ export class Presenter implements IPresenter { options ) }, - listTapeAnchors: async (conversationId, options) => { - return await this.sessionProjectionCoordinator.listTapeAnchors(conversationId, options) - }, - handoffTape: async (conversationId, name, state) => { - return await this.sessionProjectionCoordinator.handoffTape(conversationId, name, state) - }, isMemoryEnabled: (agentId) => this.memoryPresenter.isEnabled(agentId), rememberMemory: async (agentId, input, sourceSession, model) => this.memoryPresenter.rememberMemory( diff --git a/src/main/presenter/sessionApplication/projectionCoordinator.ts b/src/main/presenter/sessionApplication/projectionCoordinator.ts index 61faf4a5c..ad5a509d5 100644 --- a/src/main/presenter/sessionApplication/projectionCoordinator.ts +++ b/src/main/presenter/sessionApplication/projectionCoordinator.ts @@ -5,6 +5,7 @@ import type { AgentTapeAnchorsOptions, AgentTapeContextOptions, AgentTapeContextResult, + AgentTapeHandoffState, AgentTapeInfo, AgentTapeSearchOptions, AgentTapeSearchResult, @@ -179,7 +180,7 @@ export class SessionProjectionCoordinator async handoffTape( sessionId: string, name: string, - state: Record = {} + state: AgentTapeHandoffState ): Promise { this.requireSession(sessionId) return await this.dependencies.tape.handoffTape(sessionId, name, state) diff --git a/src/main/presenter/toolPresenter/agentTools/agentTapeTools.ts b/src/main/presenter/toolPresenter/agentTools/agentTapeTools.ts index 3e982c063..e3b443ed9 100644 --- a/src/main/presenter/toolPresenter/agentTools/agentTapeTools.ts +++ b/src/main/presenter/toolPresenter/agentTools/agentTapeTools.ts @@ -2,24 +2,12 @@ import { z } from 'zod' import { toDeepChatJsonSchema } from '@shared/lib/zodJsonSchema' import type { MCPToolDefinition } from '@shared/presenter' import { createAgentToolSuccessResult } from '@shared/lib/agentToolResultEnvelope' -import { TAPE_TOOL_NAMES, type TapeToolName } from '@shared/agentTools' +import { TAPE_TOOL_NAMES, getAgentToolExposure, isTapeToolName } from '@shared/agentTools' import type { AgentToolRuntimePort } from '../runtimePorts' import type { AgentToolCallResult } from './agentToolManager' export const AGENT_TAPE_TOOL_SERVER_NAME = 'agent-tape' -const tapeInfoSchema = z.object({}) - -const tapeAnchorsSchema = z.object({ - limit: z - .number() - .int() - .min(1) - .max(50) - .optional() - .describe('Maximum number of recent anchors to return. Defaults to 20.') -}) - const tapeEntryKindSchema = z.enum(['event', 'anchor', 'message', 'tool_call', 'tool_result']) function isTapeSearchBoundary(value: string): boolean { @@ -103,37 +91,19 @@ const tapeContextSchema = z.object({ .describe('Maximum evidence bytes across all returned entries. Defaults to 16384.') }) -const tapeHandoffSchema = z.strictObject({ - name: z - .string() - .trim() - .min(1) - .optional() - .describe('Handoff name. Values without a prefix are normalized to handoff/.'), - summary: z - .string() - .trim() - .optional() - .default('') - .describe('Compact durable summary for the handoff anchor.') -}) - const tapeToolSchemas = { - [TAPE_TOOL_NAMES.info]: tapeInfoSchema, [TAPE_TOOL_NAMES.search]: tapeSearchSchema, - [TAPE_TOOL_NAMES.context]: tapeContextSchema, - [TAPE_TOOL_NAMES.anchors]: tapeAnchorsSchema, - [TAPE_TOOL_NAMES.handoff]: tapeHandoffSchema + [TAPE_TOOL_NAMES.context]: tapeContextSchema } -type TapeAnchorOverview = { - name: string | null - entryId: number - createdAt: number +type RecallTapeToolName = typeof TAPE_TOOL_NAMES.search | typeof TAPE_TOOL_NAMES.context + +function hasRecallToolSchema(toolName: string): toolName is RecallTapeToolName { + return Object.prototype.hasOwnProperty.call(tapeToolSchemas, toolName) } function buildToolDefinition( - name: TapeToolName, + name: RecallTapeToolName, description: string, schema: z.ZodTypeAny ): MCPToolDefinition { @@ -157,7 +127,7 @@ function buildToolDefinition( } function createTapeResult( - toolName: TapeToolName, + toolName: RecallTapeToolName, result: unknown, summary: string ): AgentToolCallResult { @@ -175,18 +145,6 @@ function createTapeResult( } } -function toTapeAnchorOverview(anchor: { - name: string | null - entryId: number - createdAt: number -}): TapeAnchorOverview { - return { - name: anchor.name, - entryId: anchor.entryId, - createdAt: anchor.createdAt - } -} - function toTapeSearchOverview(result: { entryId: number kind: string @@ -215,71 +173,43 @@ function toTapeSearchOverview(result: { } } -function parseTapeHandoffArgs(rawArgs: Record): z.infer { - const parsed = tapeHandoffSchema.safeParse(rawArgs) - if (parsed.success) { - return parsed.data - } - - throw new Error( - `Invalid arguments for ${TAPE_TOOL_NAMES.handoff}. Use only {"name"?: string, "summary"?: string}; do not pass "state" or arbitrary fields. Validation details: ${parsed.error.message}` - ) -} - export class AgentTapeToolHandler { constructor(private readonly runtimePort: AgentToolRuntimePort) {} - isTapeTool(toolName: string): toolName is TapeToolName { - return Object.values(TAPE_TOOL_NAMES).includes(toolName as TapeToolName) + isModelTool(toolName: string): toolName is RecallTapeToolName { + return ( + hasRecallToolSchema(toolName) && + isTapeToolName(toolName) && + getAgentToolExposure(toolName) === 'system-model' + ) } async canUse(conversationId?: string): Promise { + const normalizedConversationId = conversationId?.trim() if ( - !conversationId || - !this.runtimePort.getTapeInfo || + !normalizedConversationId || !this.runtimePort.searchTape || - !this.runtimePort.listTapeAnchors || - !this.runtimePort.handoffTape + !this.runtimePort.getTapeContext ) { return false } - const session = await this.runtimePort.resolveConversationSessionInfo(conversationId) + const session = await this.runtimePort.resolveConversationSessionInfo(normalizedConversationId) return session?.agentType === 'deepchat' } getToolDefinitions(): MCPToolDefinition[] { - const definitions = [ - buildToolDefinition( - TAPE_TOOL_NAMES.info, - 'Inspect this DeepChat-scoped append-only tape subset inspired by bub tape.info. Returns entry counts, anchor state, token usage, and migration status for the current session.', - tapeInfoSchema - ), + return [ buildToolDefinition( TAPE_TOOL_NAMES.search, 'Search this DeepChat-scoped append-only tape subset inspired by bub tape.search. Supports text query plus optional kind and created-at filters for the current session.', tapeSearchSchema ), - buildToolDefinition( - TAPE_TOOL_NAMES.anchors, - 'List recent bub-style anchors for this DeepChat session tape. Use this before handoff when you need to inspect recent phase transitions or reconstruction checkpoints.', - tapeAnchorsSchema - ), - buildToolDefinition( - TAPE_TOOL_NAMES.handoff, - 'Write a bub-style phase-transition anchor to this DeepChat session tape. The anchor becomes the durable reconstruction marker for later context builds; include a compact summary when earlier history should be carried forward.', - tapeHandoffSchema - ) - ] - if (!this.runtimePort.getTapeContext) return definitions - return [ - ...definitions.slice(0, 2), buildToolDefinition( TAPE_TOOL_NAMES.context, 'Expand compact local evidence around selected tape entry IDs for the current session without returning unbounded raw payloads.', tapeContextSchema - ), - ...definitions.slice(2) + ) ] } @@ -288,28 +218,20 @@ export class AgentTapeToolHandler { rawArgs: Record, conversationId?: string ): Promise { - if (!this.isTapeTool(toolName)) { - throw new Error(`Unknown tape tool: ${toolName}`) + if (!this.isModelTool(toolName)) { + throw new Error(`Tape tool '${toolName}' is not available to the model.`) } - if (!conversationId) { + const normalizedConversationId = conversationId?.trim() + if (!normalizedConversationId) { throw new Error(`${toolName} requires a conversation ID.`) } - if (toolName === TAPE_TOOL_NAMES.info) { - if (!this.runtimePort.getTapeInfo) { - throw new Error('Tape info is not available.') - } - tapeToolSchemas[toolName].parse(rawArgs) - const info = await this.runtimePort.getTapeInfo(conversationId) - return createTapeResult(toolName, info, `Tape has ${info.entries} entries.`) - } - if (toolName === TAPE_TOOL_NAMES.search) { if (!this.runtimePort.searchTape) { throw new Error('Tape search is not available.') } const args = tapeToolSchemas[toolName].parse(rawArgs) - const results = await this.runtimePort.searchTape(conversationId, args.query, { + const results = await this.runtimePort.searchTape(normalizedConversationId, args.query, { limit: args.limit, kinds: args.kinds, start: args.start, @@ -319,49 +241,21 @@ export class AgentTapeToolHandler { return createTapeResult(toolName, overview, `Found ${overview.length} tape entries.`) } - if (toolName === TAPE_TOOL_NAMES.context) { - if (!this.runtimePort.getTapeContext) { - throw new Error('Tape context is not available.') - } - const args = tapeToolSchemas[toolName].parse(rawArgs) - const context = await this.runtimePort.getTapeContext(conversationId, args.entryIds, { - before: args.before, - after: args.after, - limit: args.limit, - maxBytesPerEntry: args.maxBytesPerEntry, - maxTotalBytes: args.maxTotalBytes - }) - return createTapeResult( - toolName, - context, - `Expanded ${context.entries.length} tape context entries.` - ) - } - - if (toolName === TAPE_TOOL_NAMES.anchors) { - if (!this.runtimePort.listTapeAnchors) { - throw new Error('Tape anchors are not available.') - } - const args = tapeToolSchemas[toolName].parse(rawArgs) - const anchors = await this.runtimePort.listTapeAnchors(conversationId, { - limit: args.limit - }) - const overview = anchors.map(toTapeAnchorOverview) - return createTapeResult(toolName, overview, `Found ${overview.length} tape anchors.`) - } - - if (!this.runtimePort.handoffTape) { - throw new Error('Tape handoff is not available.') + if (!this.runtimePort.getTapeContext) { + throw new Error('Tape context is not available.') } - const args = parseTapeHandoffArgs(rawArgs) - const handoff = await this.runtimePort.handoffTape(conversationId, args.name ?? 'manual', { - summary: args.summary + const args = tapeToolSchemas[toolName].parse(rawArgs) + const context = await this.runtimePort.getTapeContext(normalizedConversationId, args.entryIds, { + before: args.before, + after: args.after, + limit: args.limit, + maxBytesPerEntry: args.maxBytesPerEntry, + maxTotalBytes: args.maxTotalBytes }) - const overview = toTapeAnchorOverview(handoff) return createTapeResult( toolName, - overview, - `Wrote tape handoff anchor ${overview.name ?? 'unknown'}.` + context, + `Expanded ${context.entries.length} tape context entries.` ) } } diff --git a/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts b/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts index 5166014ba..75b6a2368 100644 --- a/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts +++ b/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts @@ -44,6 +44,7 @@ import { createAgentToolErrorResult } from '@shared/lib/agentToolResultEnvelope' import { CRON_JOB_AGENT_TOOL_NAME, TAPE_TOOL_NAMES, + isTapeToolName, isUserConfigurableAgentTool } from '@shared/agentTools' import { @@ -562,10 +563,14 @@ export class AgentToolManager { return await this.imageGenerationTool.call(args, conversationId, options) } - if (this.tapeToolHandler?.isTapeTool(toolName)) { + if (this.tapeToolHandler?.isModelTool(toolName)) { return await this.tapeToolHandler.call(toolName, args, conversationId) } + if (isTapeToolName(toolName)) { + throw new Error(`Tape tool '${toolName}' is not available to the model.`) + } + if (this.memoryToolHandler?.isMemoryTool(toolName)) { return await this.memoryToolHandler.call(toolName, args, conversationId) } diff --git a/src/main/presenter/toolPresenter/index.ts b/src/main/presenter/toolPresenter/index.ts index e002ae407..873559d50 100644 --- a/src/main/presenter/toolPresenter/index.ts +++ b/src/main/presenter/toolPresenter/index.ts @@ -18,6 +18,7 @@ import { ToolMapper, type ToolSource } from './toolMapper' import { CRON_JOB_AGENT_TOOL_NAME, TAPE_TOOL_NAMES, + getAgentToolExposure, isUserConfigurableAgentTool } from '@shared/agentTools' import { @@ -714,17 +715,17 @@ export class ToolPresenter implements IToolPresenter { } private buildTapePrompt(tools: MCPToolDefinition[]): string { - if (tools.length === 0) { + const modelTools = tools.filter( + (tool) => getAgentToolExposure(tool.function.name) === 'system-model' + ) + if (modelTools.length === 0) { return '' } - const toolNames = new Set(tools.map((tool) => tool.function.name)) - const names = tools.map((tool) => `\`${tool.function.name}\``).join(', ') + const toolNames = new Set(modelTools.map((tool) => tool.function.name)) + const names = modelTools.map((tool) => `\`${tool.function.name}\``).join(', ') const lines = ['## Tape Tools', `DeepChat tape tools are available in this session: ${names}.`] - if (toolNames.has(TAPE_TOOL_NAMES.info)) { - lines.push('`tape_info` inspects this DeepChat-scoped tape subset inspired by bub tape.info.') - } if (toolNames.has(TAPE_TOOL_NAMES.search)) { lines.push( '`tape_search` supports `query`, `limit`, `kinds`, `start`, and `end` for scoped canonical tape lookup.' @@ -735,15 +736,6 @@ export class ToolPresenter implements IToolPresenter { '`tape_context` expands selected `entryIds` from compact `tape_search` results into bounded evidence/context without dumping raw payloads.' ) } - if (toolNames.has(TAPE_TOOL_NAMES.anchors)) { - lines.push('`tape_anchors` lists recent bub-style phase-transition anchors.') - } - if (toolNames.has(TAPE_TOOL_NAMES.handoff)) { - lines.push( - '`tape_handoff` writes a bub-style phase-transition anchor. Include a compact `summary` when earlier history must be preserved.' - ) - } - return lines.join('\n') } diff --git a/src/main/presenter/toolPresenter/runtimePorts.ts b/src/main/presenter/toolPresenter/runtimePorts.ts index 856f1707d..66b36e541 100644 --- a/src/main/presenter/toolPresenter/runtimePorts.ts +++ b/src/main/presenter/toolPresenter/runtimePorts.ts @@ -7,11 +7,8 @@ import type { import type { DeepChatSubagentMeta, DeepChatSubagentSlot, - AgentTapeAnchorResult, - AgentTapeAnchorsOptions, AgentTapeContextOptions, AgentTapeContextResult, - AgentTapeInfo, AgentTapeSearchOptions, AgentTapeSearchResult, PermissionMode, @@ -72,7 +69,6 @@ export interface CreateSubagentSessionInput { export interface AgentToolRuntimePort { resolveConversationWorkdir(conversationId: string): Promise resolveConversationSessionInfo(conversationId: string): Promise - getTapeInfo?(conversationId: string): Promise searchTape?( conversationId: string, query: string, @@ -83,15 +79,6 @@ export interface AgentToolRuntimePort { entryIds: number[], options?: AgentTapeContextOptions ): Promise - listTapeAnchors?( - conversationId: string, - options?: AgentTapeAnchorsOptions - ): Promise - handoffTape?( - conversationId: string, - name: string, - state?: Record - ): Promise /** Returns whether long-term memory is enabled for the active agent. */ isMemoryEnabled?(agentId: string): boolean /** Writes a long-term memory through the shared semantic coordinator. */ diff --git a/src/shared/agentTools.ts b/src/shared/agentTools.ts index 6324ff2d2..b8c297bfb 100644 --- a/src/shared/agentTools.ts +++ b/src/shared/agentTools.ts @@ -22,8 +22,11 @@ const AGENT_TOOL_EXPOSURE_BY_NAME: Readonly +export const isTapeToolName = (toolName: string): toolName is TapeToolName => Object.prototype.hasOwnProperty.call(AGENT_TOOL_EXPOSURE_BY_NAME, toolName) + +export const getAgentToolExposure = (toolName: string): AgentToolExposure => + isTapeToolName(toolName) ? AGENT_TOOL_EXPOSURE_BY_NAME[toolName as TapeToolName] : 'user-configurable' diff --git a/src/shared/types/agent-interface.d.ts b/src/shared/types/agent-interface.d.ts index b3e4fb7d2..0e3c26fe2 100644 --- a/src/shared/types/agent-interface.d.ts +++ b/src/shared/types/agent-interface.d.ts @@ -74,6 +74,10 @@ export interface AgentTapeAnchorResult { createdAt: number } +export type AgentTapeHandoffState = Record & { + summary: string +} + export interface AgentTapeAnchorsOptions { limit?: number } diff --git a/test/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.ts b/test/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.ts index e78cf09c2..dd99742e5 100644 --- a/test/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.ts +++ b/test/main/presenter/agentRuntimePresenter/agentRuntimePresenter.test.ts @@ -10360,6 +10360,20 @@ describe('AgentRuntimePresenter', () => { }) }) + describe('tape handoff', () => { + it('rejects an empty summary before preparing or appending Tape state', async () => { + const ensureTapeReady = vi.spyOn((agent as any).tapeService, 'ensureSessionTapeReady') + const appendHandoff = vi.spyOn((agent as any).tapeService, 'handoff') + + await expect(agent.handoffTape('s1', 'manual', { summary: ' ' })).rejects.toThrow( + 'Tape handoff requires a non-empty summary.' + ) + + expect(ensureTapeReady).not.toHaveBeenCalled() + expect(appendHandoff).not.toHaveBeenCalled() + }) + }) + describe('disabled tools', () => { it('returns a disabled error when a deferred tool call is no longer enabled', async () => { sqlitePresenter.newSessionsTable.getDisabledAgentTools.mockReturnValue(['exec']) @@ -10381,6 +10395,29 @@ describe('AgentRuntimePresenter', () => { ) }) + it('does not re-execute deferred non-model Tape calls', async () => { + sqlitePresenter.newSessionsTable.getDisabledAgentTools.mockReturnValue(['tape_handoff']) + toolPresenter.getAllToolDefinitions.mockResolvedValueOnce([]) + toolPresenter.callTool.mockClear() + + await agent.initSession('s1', { providerId: 'openai', modelId: 'gpt-4' }) + + const result = await (agent as any).executeDeferredToolCall('s1', 'm1', { + id: 'tc-tape-handoff', + name: 'tape_handoff', + params: '{"name":"manual","summary":"done"}', + server_name: 'agent-tape' + }) + + expect(result).toEqual( + expect.objectContaining({ + isError: true, + responseText: "Tool 'tape_handoff' is no longer available in the current session." + }) + ) + expect(toolPresenter.callTool).not.toHaveBeenCalled() + }) + it('returns a deferred tool-local AbortError as a tool failure while the run is active', async () => { toolPresenter.getAllToolDefinitions.mockResolvedValueOnce([ { diff --git a/test/main/presenter/agentRuntimePresenter/tapeService.test.ts b/test/main/presenter/agentRuntimePresenter/tapeService.test.ts index ed999c6af..d38899dd5 100644 --- a/test/main/presenter/agentRuntimePresenter/tapeService.test.ts +++ b/test/main/presenter/agentRuntimePresenter/tapeService.test.ts @@ -541,7 +541,7 @@ describe('DeepChatTapeService', () => { } service.ensureSessionTapeReady('s1', messageStore as any) - service.handoff('s1', 'phase_done', { summary: 'done' }) + service.handoff('s1', 'phase_done', { summary: ' done ' }) const handoffAnchor = entries.find((entry) => entry.name === 'handoff/phase_done') expect(service.info('s1')).toMatchObject({ @@ -2199,47 +2199,23 @@ describe('DeepChatTapeService', () => { expect(tapeOnlyStore.getMessages).not.toHaveBeenCalled() }) - it('enriches handoff anchors without requiring a summary field', () => { + it('rejects handoff anchors without a non-empty summary before writing Tape state', () => { const { table, entries } = createTapeTableMock() const service = new DeepChatTapeService({ deepchatTapeEntriesTable: table, deepchatSessionsTable: { getSummaryState: vi.fn().mockReturnValue(null) } } as any) - const messageStore = { - getMessages: vi.fn().mockReturnValue([ - createRecord({ id: 'u1', orderSeq: 1 }), - createRecord({ - id: 'a1', - orderSeq: 2, - role: 'assistant', - content: JSON.stringify([ - { type: 'content', content: 'answer', status: 'success', timestamp: 101 } - ]), - createdAt: 101, - updatedAt: 101 - }) - ]) - } - service.ensureSessionTapeReady('s1', messageStore as any) - service.handoff('s1', 'phase_done', { - reason: 'phase complete', - nextSteps: ['verify parity'] - }) + expect(() => service.handoff('s1', 'phase_done', { summary: ' ' })).toThrow( + 'Tape handoff requires a non-empty summary.' + ) + expect(() => service.handoff('s1', 'phase_done', { reason: 'phase complete' } as any)).toThrow( + 'Tape handoff requires a non-empty summary.' + ) - const handoffAnchor = entries.find((entry) => entry.name === 'handoff/phase_done') - const state = JSON.parse(handoffAnchor.payload_json).state - expect(state).toMatchObject({ - reason: 'phase complete', - nextSteps: ['verify parity'], - cursorOrderSeq: 3, - range: { - fromOrderSeq: 1, - toOrderSeq: 2 - }, - sourceMessageIds: ['u1', 'a1'] - }) - expect(state.summary).toBeUndefined() + expect(table.ensureBootstrapAnchor).not.toHaveBeenCalled() + expect(table.appendAnchor).not.toHaveBeenCalled() + expect(entries).toEqual([]) }) it('migrates legacy session summary into a tape anchor during backfill', () => { diff --git a/test/main/presenter/sessionApplication/projectionCoordinator.test.ts b/test/main/presenter/sessionApplication/projectionCoordinator.test.ts index 512a21e92..cd6d6835b 100644 --- a/test/main/presenter/sessionApplication/projectionCoordinator.test.ts +++ b/test/main/presenter/sessionApplication/projectionCoordinator.test.ts @@ -257,7 +257,7 @@ describe('SessionProjectionCoordinator', () => { await harness.coordinator.searchTape('s1', 'needle') await harness.coordinator.getTapeContext('s1', [1]) await harness.coordinator.listTapeAnchors('s1') - await harness.coordinator.handoffTape('s1', 'handoff') + await harness.coordinator.handoffTape('s1', 'handoff', { summary: 'handoff summary' }) await expect(harness.coordinator.listMessageViewManifests(' m1 ')).resolves.toEqual([ { id: 'view-1' } ]) diff --git a/test/main/presenter/toolPresenter/agentTools/agentTapeTools.test.ts b/test/main/presenter/toolPresenter/agentTools/agentTapeTools.test.ts index e7d4ac228..394fee2a1 100644 --- a/test/main/presenter/toolPresenter/agentTools/agentTapeTools.test.ts +++ b/test/main/presenter/toolPresenter/agentTools/agentTapeTools.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it, vi } from 'vitest' import { AgentToolManager } from '@/presenter/toolPresenter/agentTools/agentToolManager' -import { TAPE_TOOL_NAMES } from '@/presenter/toolPresenter/agentTools' +import { AgentTapeToolHandler, TAPE_TOOL_NAMES } from '@/presenter/toolPresenter/agentTools' vi.mock('electron', () => ({ app: { @@ -142,7 +142,7 @@ const buildManager = (runtimePort = buildRuntimePort()) => }) describe('Agent tape tools', () => { - it('exposes tape tools for DeepChat sessions', async () => { + it('exposes only the atomic recall pair for DeepChat sessions', async () => { const manager = buildManager() const defs = await manager.getAllToolDefinitions({ @@ -152,25 +152,14 @@ describe('Agent tape tools', () => { conversationId: 'conv-1' }) - expect(defs.map((def) => def.function.name)).toEqual( - expect.arrayContaining([ - TAPE_TOOL_NAMES.info, - TAPE_TOOL_NAMES.search, - TAPE_TOOL_NAMES.context, - TAPE_TOOL_NAMES.anchors, - TAPE_TOOL_NAMES.handoff - ]) - ) - const handoffDef = defs.find((def) => def.function.name === TAPE_TOOL_NAMES.handoff) - const handoffParameters = handoffDef?.function.parameters as - | { additionalProperties?: unknown; properties?: Record } - | undefined - expect(handoffParameters?.properties).toHaveProperty('summary') - expect(handoffParameters?.properties).not.toHaveProperty('state') - expect(handoffParameters?.additionalProperties).toBe(false) + const tapeNames = defs + .filter((def) => def.server.name === 'agent-tape') + .map((def) => def.function.name) + + expect(tapeNames).toEqual([TAPE_TOOL_NAMES.search, TAPE_TOOL_NAMES.context]) }) - it('keeps base tape tools available when compact context is unsupported', async () => { + it('exposes neither recall tool when compact context is unsupported', async () => { const manager = buildManager(buildRuntimePort({ getTapeContext: undefined })) const defs = await manager.getAllToolDefinitions({ @@ -179,17 +168,20 @@ describe('Agent tape tools', () => { agentWorkspacePath: '/workspace', conversationId: 'conv-1' }) - const names = defs.map((def) => def.function.name) + expect(defs.some((def) => def.server.name === 'agent-tape')).toBe(false) + }) - expect(names).toEqual( - expect.arrayContaining([ - TAPE_TOOL_NAMES.info, - TAPE_TOOL_NAMES.search, - TAPE_TOOL_NAMES.anchors, - TAPE_TOOL_NAMES.handoff - ]) - ) - expect(names).not.toContain(TAPE_TOOL_NAMES.context) + it('exposes neither recall tool when search is unsupported', async () => { + const manager = buildManager(buildRuntimePort({ searchTape: undefined })) + + const defs = await manager.getAllToolDefinitions({ + chatMode: 'agent', + supportsVision: false, + agentWorkspacePath: '/workspace', + conversationId: 'conv-1' + }) + + expect(defs.some((def) => def.server.name === 'agent-tape')).toBe(false) }) it('does not expose tape tools outside DeepChat sessions', async () => { @@ -208,16 +200,35 @@ describe('Agent tape tools', () => { conversationId: 'conv-1' }) - expect(defs.some((def) => def.function.name === TAPE_TOOL_NAMES.info)).toBe(false) + expect(defs.some((def) => def.server.name === 'agent-tape')).toBe(false) }) - it('routes tape tool calls through the runtime port', async () => { + it('does not expose recall tools without a conversation ID', async () => { + const runtimePort = buildRuntimePort() + const handler = new AgentTapeToolHandler(runtimePort) + const manager = buildManager(runtimePort) + + await expect(handler.canUse(' ')).resolves.toBe(false) + expect(runtimePort.resolveConversationSessionInfo).not.toHaveBeenCalled() + + const defs = await manager.getAllToolDefinitions({ + chatMode: 'agent', + supportsVision: false, + agentWorkspacePath: '/workspace', + conversationId: ' ' + }) + + expect(defs.some((def) => def.server.name === 'agent-tape')).toBe(false) + await expect( + manager.callTool(TAPE_TOOL_NAMES.search, { query: 'needle' }, ' ') + ).rejects.toThrow(`${TAPE_TOOL_NAMES.search} requires a conversation ID.`) + expect(runtimePort.searchTape).not.toHaveBeenCalled() + }) + + it('routes only recall calls through the runtime port', async () => { const runtimePort = buildRuntimePort() const manager = buildManager(runtimePort) - const info = (await manager.callTool(TAPE_TOOL_NAMES.info, {}, 'conv-1')) as { - content: string - } const search = (await manager.callTool( TAPE_TOOL_NAMES.search, { @@ -231,13 +242,6 @@ describe('Agent tape tools', () => { )) as { content: string } - const handoff = (await manager.callTool( - TAPE_TOOL_NAMES.handoff, - { name: 'manual', summary: 'done' }, - 'conv-1' - )) as { - content: string - } const context = (await manager.callTool( TAPE_TOOL_NAMES.context, { entryIds: [2], before: 1, after: 1, limit: 10 }, @@ -245,11 +249,6 @@ describe('Agent tape tools', () => { )) as { content: string } - const anchors = (await manager.callTool(TAPE_TOOL_NAMES.anchors, { limit: 5 }, 'conv-1')) as { - content: string - } - - expect(JSON.parse(info.content)).toMatchObject({ entries: 3, migrationState: 'ready' }) expect(JSON.parse(search.content)).toHaveLength(1) expect(JSON.parse(search.content)[0]).not.toHaveProperty('payload') expect(JSON.parse(search.content)[0]).not.toHaveProperty('meta') @@ -264,16 +263,6 @@ describe('Agent tape tools', () => { ] }) expect(JSON.parse(context.content).entries[0]).not.toHaveProperty('payload') - expect(JSON.parse(handoff.content)).toEqual({ - name: 'handoff/manual', - entryId: 4, - createdAt: 20 - }) - expect(JSON.parse(anchors.content)).toEqual([ - { name: 'session/start', entryId: 1, createdAt: 1 } - ]) - expect(JSON.parse(anchors.content)[0]).not.toHaveProperty('payload') - expect(runtimePort.getTapeInfo).toHaveBeenCalledWith('conv-1') expect(runtimePort.searchTape).toHaveBeenCalledWith('conv-1', 'auth', { limit: 5, kinds: ['message'], @@ -287,22 +276,24 @@ describe('Agent tape tools', () => { maxBytesPerEntry: undefined, maxTotalBytes: undefined }) - expect(runtimePort.listTapeAnchors).toHaveBeenCalledWith('conv-1', { limit: 5 }) - expect(runtimePort.handoffTape).toHaveBeenCalledWith('conv-1', 'manual', { summary: 'done' }) + expect(runtimePort.getTapeInfo).not.toHaveBeenCalled() + expect(runtimePort.listTapeAnchors).not.toHaveBeenCalled() + expect(runtimePort.handoffTape).not.toHaveBeenCalled() }) - it('rejects legacy tape_handoff state without writing an empty anchor', async () => { - const runtimePort = buildRuntimePort() - const manager = buildManager(runtimePort) + it.each([TAPE_TOOL_NAMES.info, TAPE_TOOL_NAMES.anchors, TAPE_TOOL_NAMES.handoff])( + 'rejects non-model Tape call %s without runtime side effects', + async (toolName) => { + const runtimePort = buildRuntimePort() + const manager = buildManager(runtimePort) - await expect( - manager.callTool( - TAPE_TOOL_NAMES.handoff, - { name: 'manual', state: { summary: 'done' } }, - 'conv-1' + await expect(manager.callTool(toolName, {}, 'conv-1')).rejects.toThrow( + `Tape tool '${toolName}' is not available to the model.` ) - ).rejects.toThrow('do not pass "state"') - expect(runtimePort.handoffTape).not.toHaveBeenCalled() - }) + expect(runtimePort.getTapeInfo).not.toHaveBeenCalled() + expect(runtimePort.listTapeAnchors).not.toHaveBeenCalled() + expect(runtimePort.handoffTape).not.toHaveBeenCalled() + } + ) }) diff --git a/test/main/presenter/toolPresenter/toolPresenter.test.ts b/test/main/presenter/toolPresenter/toolPresenter.test.ts index 2320b0a19..a60a3c69e 100644 --- a/test/main/presenter/toolPresenter/toolPresenter.test.ts +++ b/test/main/presenter/toolPresenter/toolPresenter.test.ts @@ -188,6 +188,34 @@ describe('ToolPresenter', () => { expect(mcpPresenter.callTool).not.toHaveBeenCalled() }) + it('keeps every Tape capability name reserved against same-name MCP tools', async () => { + const tapeToolNames = new Set(Object.values(TAPE_TOOL_NAMES)) + const toolPresenter = new ToolPresenter({ + mcpPresenter: { + getAllToolDefinitions: vi + .fn() + .mockResolvedValue( + Object.values(TAPE_TOOL_NAMES).map((name) => buildToolDefinition(name, 'untrusted-mcp')) + ) + } as any, + configPresenter: { + getSkillsEnabled: vi.fn().mockReturnValue(false), + getSkillsPath: vi.fn().mockReturnValue('C:\\skills'), + getModelConfig: vi.fn() + } as any, + commandPermissionHandler: new CommandPermissionService(), + agentToolRuntime: buildAgentToolRuntimeMock() + }) + + const defs = await toolPresenter.getAllToolDefinitions({ + chatMode: 'agent', + supportsVision: false, + agentWorkspacePath: 'C:\\workspace' + }) + + expect(defs.some((definition) => tapeToolNames.has(definition.function.name))).toBe(false) + }) + it('keeps ToolPresenter collision resolution behind the DeepChat catalog port', async () => { const mcpDefs = [buildToolDefinition('shared', 'mcp')] const mcpPresenter = { @@ -454,7 +482,7 @@ describe('ToolPresenter', () => { expect(getAgentToolExposure('__proto__')).toBe('user-configurable') }) - it('keeps non-configurable Tape tools in the runtime catalog despite stale disabled values', async () => { + it('keeps the recall pair in the runtime catalog despite stale disabled values', async () => { const toolPresenter = new ToolPresenter({ mcpPresenter: { getAllToolDefinitions: vi.fn().mockResolvedValue([]) @@ -484,7 +512,10 @@ describe('ToolPresenter', () => { }) const names = defs.map((tool) => tool.function.name) - expect(names).toEqual(expect.arrayContaining(Object.values(TAPE_TOOL_NAMES))) + expect(names).toEqual(expect.arrayContaining([TAPE_TOOL_NAMES.search, TAPE_TOOL_NAMES.context])) + expect(names).not.toContain(TAPE_TOOL_NAMES.info) + expect(names).not.toContain(TAPE_TOOL_NAMES.anchors) + expect(names).not.toContain(TAPE_TOOL_NAMES.handoff) }) it('reads configurable definitions without publishing mappings or mutating runtime context', async () => { @@ -1073,7 +1104,7 @@ describe('ToolPresenter', () => { expect(withProgress).toContain('Before ending the turn, reconcile the checklist') }) - it('describes only enabled tape tools in the tape prompt', () => { + it('omits diagnostic and runtime-only Tape capabilities from the model prompt', () => { const mcpPresenter = { getAllToolDefinitions: vi.fn().mockResolvedValue([]), callTool: vi.fn() @@ -1101,16 +1132,18 @@ describe('ToolPresenter', () => { { ...buildToolDefinition(TAPE_TOOL_NAMES.anchors, 'agent-tape'), source: 'agent' + }, + { + ...buildToolDefinition(TAPE_TOOL_NAMES.handoff, 'agent-tape'), + source: 'agent' } ] }) - expect(prompt).toContain('## Tape Tools') - expect(prompt).toContain('`tape_info` inspects') - expect(prompt).toContain('`tape_anchors` lists') - expect(prompt).not.toContain('`tape_search` supports') - expect(prompt).not.toContain('`tape_context` expands') - expect(prompt).not.toContain('`tape_handoff` writes') + expect(prompt).not.toContain('## Tape Tools') + expect(prompt).not.toContain('tape_info') + expect(prompt).not.toContain('tape_anchors') + expect(prompt).not.toContain('tape_handoff') }) it('describes tape_context only when the context tool is enabled', () => { From c52071a59c26299aa49e3a040a1deb54447c888d Mon Sep 17 00:00:00 2001 From: yyhhyyyyyy Date: Wed, 15 Jul 2026 18:37:38 +0800 Subject: [PATCH 4/5] fix(config): remove tape tool toggles --- docs/architecture/agent-memory-system/spec.md | 19 +- .../deepchat-tape-baseline/spec.md | 3 +- .../session-boundary-cleanup/spec.md | 11 +- .../tape-capability-layering/spec.md | 2 +- .../tape-capability-layering/tasks.md | 6 +- docs/architecture/tool-system.md | 30 +++- scripts/architecture-guard.mjs | 2 +- .../agent/deepchat/deepChatAgentRepository.ts | 29 ++- .../agent/shared/agentSessionNormalization.ts | 17 +- ...ook.ts => disabledAgentToolCleanupHook.ts} | 16 +- .../lifecyclePresenter/hooks/index.ts | 2 +- .../sessionDataMigrations.ts | 170 +++++++++++------- .../deepchat/deepChatAgentRepository.test.ts | 65 +++++++ .../startupMaintenanceHooks.test.ts | 16 +- .../agentAssignmentPolicy.test.ts | 5 + .../sessionDataMigrations.test.ts | 152 ++++++++++++++-- test/main/scripts/architectureGuard.test.ts | 2 +- 17 files changed, 413 insertions(+), 134 deletions(-) rename src/main/presenter/lifecyclePresenter/hooks/after-start/{disabledSearchToolCleanupHook.ts => disabledAgentToolCleanupHook.ts} (60%) diff --git a/docs/architecture/agent-memory-system/spec.md b/docs/architecture/agent-memory-system/spec.md index 45a7b3153..f3f633fd8 100644 --- a/docs/architecture/agent-memory-system/spec.md +++ b/docs/architecture/agent-memory-system/spec.md @@ -159,7 +159,7 @@ flowchart TD | Runtime wiring | `agentRuntimePresenter/index.ts` | coordinator construction/dependencies and thin instance/lifecycle delegates; no duplicate orchestration maps | | Runtime | `agentRuntimePresenter/tapeService.ts` | `search()` / `getContext()` / `ensureSearchProjection()` | | Tools | `toolPresenter/agentTools/agentMemoryTools.ts` | `memory_remember` / `memory_recall` / `memory_forget` | -| Tools | `toolPresenter/agentTools/agentTapeTools.ts` | `tape_info` / `tape_search` / `tape_context` / `tape_anchors` / `tape_handoff` | +| Tools | `toolPresenter/agentTools/agentTapeTools.ts` | Atomic model recall pair: `tape_search` / `tape_context`; info, anchors, and handoff remain below the model-tool boundary | | Skills | `resources/skills/memory-management/SKILL.md` | Discoverable guidance for recall/remember discipline and Memory vs Skill vs Scheduled Task routing | | Contracts | `shared/contracts/routes/memory.routes.ts` | All `memory.*` IPC routes + DTO schemas | | Contracts | `shared/contracts/events/memory.events.ts` | `memory.updated` event plus the schema-derived authoritative update-reason type | @@ -717,9 +717,11 @@ flowchart TD BM25-ranked with a `LIKE` fallback/supplement. - **`tape_context`** expands specific entry ids with a bounded window (before/after default 2, clamp ≤ 20), an entry cap (default 50), and byte budgets (per-entry default 2048 / max 8192; total default 16384 / max - 65536), UTF-8-safe truncated. Tape tools are DeepChat-agent-only and `tape_context` is advertised only when - the tool-runtime port exposes `getTapeContext` (wired directly to `SessionProjectionCoordinator`, not the - memory kernel; in practice always present). + 65536), UTF-8-safe truncated. The recall pair is DeepChat-agent-only and is advertised atomically only when + both `searchTape` and `getTapeContext` runtime ports are present for a persisted DeepChat session. These + ports are wired directly to `SessionProjectionCoordinator`, not the memory kernel. +- Memory extraction and injection do not invoke either model tool. They consume the effective Tape and exact + lineage through internal runtime contracts, so model-facing Tape exposure does not gate Memory behavior. - The whole projection/search layer is fail-open: any error degrades to a coarser search over the effective tape rather than throwing (the one exception is an unparseable time boundary, which is reported). @@ -822,9 +824,12 @@ This is where the "stabilization" and "kernel hardening" work concentrates. | `agent-memory` | `memory_remember` | Persist a durable fact/event with optional category; routes through the decision ring (`coordinateWrite`). | | `agent-memory` | `memory_recall` | Recall relevant memories for a query (ranking/limit are kernel-side). | | `agent-memory` | `memory_forget` | **Archive** (soft delete) a memory by id so it is no longer recalled. | -| `agent-tape` | `tape_info` / `tape_anchors` / `tape_handoff` | Tape introspection and subagent handoff. | -| `agent-tape` | `tape_search` | Overview-only search of the tape projection (no payload). | -| `agent-tape` | `tape_context` | Bounded evidence expansion of specific entry ids. | +| `agent-tape` | `tape_search` | Overview-only search of the tape projection (no payload); exposed only as part of the recall pair. | +| `agent-tape` | `tape_context` | Bounded evidence expansion of specific entry ids; exposed only as part of the recall pair. | + +`tape_info` and `tape_anchors` remain lower-level diagnostic APIs. `tape_handoff` remains a runtime-only +transition API and requires a non-empty durable summary. All three names stay reserved but are absent from +model definitions, prompts, configuration UI, and Agent tool dispatch. Tool results are success-enveloped even for "soft" outcomes (memory disabled, `noop`, not-found) — callers inspect `result.ok`, not `isError`. Hard infra failures throw. diff --git a/docs/architecture/deepchat-tape-baseline/spec.md b/docs/architecture/deepchat-tape-baseline/spec.md index 0e0639e97..eb64b6878 100644 --- a/docs/architecture/deepchat-tape-baseline/spec.md +++ b/docs/architecture/deepchat-tape-baseline/spec.md @@ -37,7 +37,8 @@ DeepChat already has the main Tape primitives. | View policy | `tapeViewPolicy.ts` | Registry and selector boundary; `legacy_context_v1` delegates to the current selector. | | Context selection | `contextBuilder.ts` | Legacy token-budget selector used by `legacy_context_v1`. | | Request trace | `deepchat_message_traces` | Stores redacted provider request previews for the trace dialog. | -| Agent tools | `agentTapeTools.ts` | Exposes `tape_info`, `tape_search`, `tape_anchors`, `tape_handoff`. | +| Model recall tools | `agentTapeTools.ts` | Exposes the atomic `tape_search` / `tape_context` pair only when both runtime ports are available for a persisted DeepChat session. | +| Diagnostic/runtime APIs | `DeepChatTapeService` + `SessionProjectionCoordinator` | Retains info, anchor listing, and handoff below the model-tool boundary; these capabilities are not user-configurable Agent tools. | The first implementation step uses this baseline as the single runtime path. `DeepChatTapeService` remains the Tape service boundary. diff --git a/docs/architecture/session-boundary-cleanup/spec.md b/docs/architecture/session-boundary-cleanup/spec.md index d78a01a51..39e005a6c 100644 --- a/docs/architecture/session-boundary-cleanup/spec.md +++ b/docs/architecture/session-boundary-cleanup/spec.md @@ -152,14 +152,19 @@ AgentSessionPresenter ### Startup tasks - Existing task IDs, targets, phases, resource classes, label keys, priorities, and non-critical - failure behavior remain unchanged. + failure behavior remain unchanged unless a later migration explicitly supersedes that task. The + disabled-tool cleanup is superseded by `main:disabled-agent-tool-cleanup` with label key + `startup.main.disabledAgentToolCleanup`; its target, phase, resource class, priority, and + non-critical failure behavior remain unchanged. - Legacy import remains single-flight and preserves persisted status and retry behavior. - Usage backfill remains single-flight, stale-running detection remains unchanged, and dashboard reads continue to expose its normalized status. - Mainline normalization retains `sqlite-mainline-normalization-v1`, batch size, cursor ordering, progress writes, yielding, completion metadata, and failure metadata. -- Disabled-tool cleanup retains `agent-disabled-search-tool-cleanup-v1`, session and agent-config - cleanup behavior, progress yielding, and completion/failure metadata. +- Disabled-tool cleanup preserves the historical `agent-disabled-search-tool-cleanup-v1` record but + uses the independent `agent-disabled-tool-capability-cleanup-v2` completion gate so users who + completed v1 still receive Tape capability cleanup. Session and agent-config cleanup remains + idempotent, bounded, yielding, and retryable with completion/failure metadata. ### History search diff --git a/docs/architecture/tape-capability-layering/spec.md b/docs/architecture/tape-capability-layering/spec.md index 4bf720072..9a12a60a3 100644 --- a/docs/architecture/tape-capability-layering/spec.md +++ b/docs/architecture/tape-capability-layering/spec.md @@ -1,6 +1,6 @@ # Tape Capability Layering - Spec -> Status: **planned** +> Status: **implemented** ## Problem diff --git a/docs/architecture/tape-capability-layering/tasks.md b/docs/architecture/tape-capability-layering/tasks.md index bfe403534..f98ecd07b 100644 --- a/docs/architecture/tape-capability-layering/tasks.md +++ b/docs/architecture/tape-capability-layering/tasks.md @@ -5,6 +5,6 @@ - [x] T3: Move renderer configuration callers to the side-effect-free configurable catalog. - [x] T4: Restrict the model-facing Tape surface to the search/context recall pair. - [x] T5: Make runtime handoff reject empty inherited summaries before appending an anchor. -- [ ] T6: Remove non-configurable Tape names from new and persisted disabled-tool state. -- [ ] T7: Update maintained tracked architecture contracts and focused regression tests. -- [ ] T8: Run format, i18n validation, lint, typecheck, and the full test suite. +- [x] T6: Remove non-configurable Tape names from new and persisted disabled-tool state. +- [x] T7: Update maintained tracked architecture contracts and focused regression tests. +- [x] T8: Run format, i18n validation, lint, typecheck, and the full test suite. diff --git a/docs/architecture/tool-system.md b/docs/architecture/tool-system.md index 9eaf136d2..76f4f8ed3 100644 --- a/docs/architecture/tool-system.md +++ b/docs/architecture/tool-system.md @@ -11,6 +11,7 @@ | DeepChat tool adapters | `src/main/presenter/agentRuntimePresenter/` | 将 loop ports 接到现有 ToolPresenter、permission、normalization/output guard | | `ToolPresenter` | `src/main/presenter/toolPresenter/index.ts` | 聚合工具定义、建立映射、路由调用 | | `ToolMapper` | `src/main/presenter/toolPresenter/toolMapper.ts` | `toolName -> source` 映射 | +| Agent tool exposure policy | `src/shared/agentTools.ts` | Agent tool 的配置、模型、诊断与 runtime-only 暴露分类 | | `AgentToolManager` | `src/main/presenter/toolPresenter/agentTools/agentToolManager.ts` | 本地 agent tools 装配与执行 | | `AgentFileSystemHandler` | `src/main/presenter/toolPresenter/agentTools/agentFileSystemHandler.ts` | 文件系统类工具 | | `AgentBashHandler` | `src/main/presenter/toolPresenter/agentTools/agentBashHandler.ts` | 命令执行与后台 session | @@ -18,7 +19,7 @@ | `chatSettingsTools` | `src/main/presenter/toolPresenter/agentTools/chatSettingsTools.ts` | chat/session settings 工具 | | `SubagentOrchestratorTool` | `src/main/presenter/toolPresenter/agentTools/subagentOrchestratorTool.ts` | subagent orchestration | | `AgentPlanTool` | `src/main/presenter/toolPresenter/agentTools/agentPlanTool.ts` | `agent-core/update_plan` | -| `AgentTapeToolHandler` | `src/main/presenter/toolPresenter/agentTools/agentTapeTools.ts` | tape read/merge/discard tools | +| `AgentTapeToolHandler` | `src/main/presenter/toolPresenter/agentTools/agentTapeTools.ts` | 模型可调用的原子 Tape recall pair：`tape_search` / `tape_context` | | `AgentImageGenerationTool` | `src/main/presenter/toolPresenter/agentTools/agentImageGenerationTool.ts` | image generation tool | | `McpPresenter` | `src/main/presenter/mcpPresenter/` | 外部 MCP servers 与 tools | | `ACP helpers` | `src/main/agent/acp/` | ACP runtime、workdir、config、MCP 映射 | @@ -45,12 +46,33 @@ graph LR ## 获取工具定义 -`ToolPresenter.getAllToolDefinitions()` 会按顺序做三件事： +工具目录分为 runtime catalog 与 configurable catalog。 + +`ToolPresenter.getAllToolDefinitions()` 是 runtime catalog，会按顺序： 1. 从 `mcpPresenter` 拉取 MCP tools。 2. 从 `AgentToolManager` 拉取本地 agent tools。 -3. 用 `ToolMapper` 记录来源，并在重名时优先保留 MCP tool。 -4. 过滤 disabled agent tools，并为每个 conversation 维护独立映射。 +3. 保留 built-in reserved names，拒绝同名 MCP definition；其他重名仍按现有 collision policy 处理。 +4. 只对 `user-configurable` definitions 应用 `disabledAgentTools`。 +5. 用 `ToolMapper` 记录最终来源，并为每个 conversation 维护独立映射。 + +`ToolPresenter.getConfigurableAgentToolDefinitions()` 是 renderer settings 使用的 configurable +catalog。它只返回 `user-configurable` Agent tools，不解析 MCP catalog、不发布 `ToolMapper`，也不修改 +conversation MCP access context 或 runtime cache。`tools.listDefinitions` 保持原 IPC name/wire shape，内部 +读取该 configurable catalog。 + +Tape 的 exposure matrix 为： + +| Tool | Exposure | Model behavior | +| --- | --- | --- | +| `tape_search` | `system-model` | 与 `tape_context` 成对提供，不受 disabled list 影响 | +| `tape_context` | `system-model` | 与 `tape_search` 成对提供，不受 disabled list 影响 | +| `tape_info` | `diagnostic` | 保留底层诊断 API，不生成模型 definition | +| `tape_anchors` | `diagnostic` | 保留底层诊断 API，不生成模型 definition | +| `tape_handoff` | `runtime-only` | 仅 runtime handoff 路径可写，模型 dispatcher 拒绝 | + +五个 Tape 名称全部 reserved，防止同名 MCP 工具劫持。`disabledAgentTools` 的持久化正规化会永久移除 +这些非配置能力；v2 startup cleanup 幂等清理旧 Session 和 DeepChat Agent Config。 这意味着 `DeepChatLoopEngine` 不知道 tool 的真实来源，只接收 `MCPToolDefinition[]` snapshot 和窄 execution/result ports。`AgentRuntimePresenter` 保留 adapter wiring，但 tool mapping/collision/dispatch owner diff --git a/scripts/architecture-guard.mjs b/scripts/architecture-guard.mjs index c48bbae2e..51fb16fdb 100644 --- a/scripts/architecture-guard.mjs +++ b/scripts/architecture-guard.mjs @@ -134,7 +134,7 @@ const SESSION_PHASE_ONE_FOREIGN_IMPORT_PATTERN = /(?:^|[/_.-])(?:history|export(?:er)?|usage(?:[-_.]?stats?)?|rtk|legacy[-_.]?import(?:er|s)?|import(?:er|s)?|migrations?|translation|catalog)(?:$|[/_.-])/ const SESSION_BOUNDARY_STARTUP_HOOK_PATHS = new Set( [ - 'disabledSearchToolCleanupHook.ts', + 'disabledAgentToolCleanupHook.ts', 'legacyImportHook.ts', 'rtkHealthCheckHook.ts', 'sqliteMainlineNormalizationHook.ts', diff --git a/src/main/agent/deepchat/deepChatAgentRepository.ts b/src/main/agent/deepchat/deepChatAgentRepository.ts index 05c022f0c..99ae807cd 100644 --- a/src/main/agent/deepchat/deepChatAgentRepository.ts +++ b/src/main/agent/deepchat/deepChatAgentRepository.ts @@ -1,6 +1,7 @@ import { nanoid } from 'nanoid' import type { AgentRowStore } from '@/agent/shared/agentRowStore' import type { AppSessionId } from '@/agent/shared/agentSessionIds' +import { normalizeDisabledAgentTools } from '@/agent/shared/agentSessionNormalization' import type { AgentRow } from '@/presenter/sqlitePresenter/tables/agents' import { createDefaultDeepChatSubagentSlots, @@ -42,6 +43,17 @@ const sanitizeString = (value?: string | null): string | null => { const clone = (value: T): T => JSON.parse(JSON.stringify(value)) as T +const normalizeExplicitDisabledAgentTools = (config: DeepChatAgentConfig): DeepChatAgentConfig => { + if (!Object.prototype.hasOwnProperty.call(config, 'disabledAgentTools')) { + return config + } + + return { + ...config, + disabledAgentTools: normalizeDisabledAgentTools(config.disabledAgentTools) + } +} + const normalizeNullableStringList = ( value: string[] | null | undefined ): string[] | null | undefined => { @@ -69,7 +81,9 @@ const mergeDeepChatConfig = ( defaultProjectPath: overrideConfig.defaultProjectPath ?? baseConfig.defaultProjectPath ?? null, systemPrompt: overrideConfig.systemPrompt ?? baseConfig.systemPrompt ?? '', permissionMode: overrideConfig.permissionMode ?? baseConfig.permissionMode, - disabledAgentTools: overrideConfig.disabledAgentTools ?? baseConfig.disabledAgentTools ?? [], + disabledAgentTools: normalizeDisabledAgentTools( + overrideConfig.disabledAgentTools ?? baseConfig.disabledAgentTools + ), enabledSkillNames: mergeNullableStringList( baseConfig.enabledSkillNames, overrideConfig.enabledSkillNames @@ -123,7 +137,9 @@ export class DeepChatAgentRepository { protected: true, icon: sanitizeString(defaults?.icon), avatarJson: stringifyJson(defaults?.avatar ?? null), - configJson: stringifyJson(defaults?.config ?? null) + configJson: stringifyJson( + defaults?.config ? normalizeExplicitDisabledAgentTools(defaults.config) : null + ) }) } else { rows.update(BUILTIN_DEEPCHAT_AGENT_ID, { enabled: true, protected: true }) @@ -143,7 +159,9 @@ export class DeepChatAgentRepository { description: sanitizeString(input.description), icon: sanitizeString(input.icon), avatarJson: stringifyJson(input.avatar ?? null), - configJson: stringifyJson(input.config ?? null) + configJson: stringifyJson( + input.config ? normalizeExplicitDisabledAgentTools(input.config) : null + ) }) return this.dependencies.rows.get(id) as AgentRow } @@ -157,7 +175,10 @@ export class DeepChatAgentRepository { const nextConfig = updates.config === undefined ? currentConfig - : { ...currentConfig, ...clone(updates.config ?? {}) } + : normalizeExplicitDisabledAgentTools({ + ...currentConfig, + ...clone(updates.config ?? {}) + }) rows.update(agentId, { name: updates.name?.trim() || row.name, enabled: updates.enabled ?? row.enabled === 1, diff --git a/src/main/agent/shared/agentSessionNormalization.ts b/src/main/agent/shared/agentSessionNormalization.ts index b1ea4ab1b..4f641b84a 100644 --- a/src/main/agent/shared/agentSessionNormalization.ts +++ b/src/main/agent/shared/agentSessionNormalization.ts @@ -3,14 +3,15 @@ import type { MessageFile, SendMessageInput } from '@shared/types/agent-interface' +import { isUserConfigurableAgentTool } from '@shared/agentTools' const RETIRED_DEFAULT_AGENT_TOOLS = new Set(['find', 'ls']) const LEGACY_PERSISTED_DISABLED_AGENT_TOOLS = new Set(['find', 'grep', 'ls']) -const LEGACY_AGENT_TOOL_NAME_MAP: Record = { - yo_browser_cdp_send: 'cdp_send', - yo_browser_window_open: 'load_url', - yo_browser_window_list: 'get_browser_status' -} +const LEGACY_AGENT_TOOL_NAME_MAP = new Map([ + ['yo_browser_cdp_send', 'cdp_send'], + ['yo_browser_window_open', 'load_url'], + ['yo_browser_window_list', 'get_browser_status'] +]) export const normalizeDisabledAgentTools = ( disabledAgentTools?: string[], @@ -26,8 +27,10 @@ export const normalizeDisabledAgentTools = ( disabledAgentTools .filter((item): item is string => typeof item === 'string') .map((item) => item.trim()) - .map((item) => LEGACY_AGENT_TOOL_NAME_MAP[item] ?? item) - .filter((item) => Boolean(item) && !retiredTools.has(item)) + .map((item) => LEGACY_AGENT_TOOL_NAME_MAP.get(item) ?? item) + .filter( + (item) => Boolean(item) && !retiredTools.has(item) && isUserConfigurableAgentTool(item) + ) ) ).sort((left, right) => left.localeCompare(right)) } diff --git a/src/main/presenter/lifecyclePresenter/hooks/after-start/disabledSearchToolCleanupHook.ts b/src/main/presenter/lifecyclePresenter/hooks/after-start/disabledAgentToolCleanupHook.ts similarity index 60% rename from src/main/presenter/lifecyclePresenter/hooks/after-start/disabledSearchToolCleanupHook.ts rename to src/main/presenter/lifecyclePresenter/hooks/after-start/disabledAgentToolCleanupHook.ts index a8e5adcc9..1bdd1b977 100644 --- a/src/main/presenter/lifecyclePresenter/hooks/after-start/disabledSearchToolCleanupHook.ts +++ b/src/main/presenter/lifecyclePresenter/hooks/after-start/disabledAgentToolCleanupHook.ts @@ -1,27 +1,27 @@ import { LifecycleHook, LifecycleContext } from '@shared/presenter' import { LifecyclePhase } from '@shared/lifecycle' import { presenter } from '@/presenter' -import { runDisabledSearchToolCleanupMigration } from '@/presenter/startupMigrations/sessionDataMigrations' +import { runDisabledAgentToolCapabilityCleanupMigration } from '@/presenter/startupMigrations/sessionDataMigrations' -export const disabledSearchToolCleanupHook: LifecycleHook = { - name: 'disabled-search-tool-cleanup', +export const disabledAgentToolCleanupHook: LifecycleHook = { + name: 'disabled-agent-tool-cleanup', phase: LifecyclePhase.AFTER_START, priority: 23, critical: false, execute: async (_context: LifecycleContext) => { if (!presenter) { - throw new Error('disabledSearchToolCleanupHook: Presenter not initialized') + throw new Error('disabledAgentToolCleanupHook: Presenter not initialized') } void presenter.startupWorkloadCoordinator .scheduleTask({ - id: 'main:disabled-search-tool-cleanup', + id: 'main:disabled-agent-tool-cleanup', target: 'main', phase: 'background', resource: 'io', - labelKey: 'startup.main.disabledSearchToolCleanup', + labelKey: 'startup.main.disabledAgentToolCleanup', run: async (taskContext) => { - await runDisabledSearchToolCleanupMigration( + await runDisabledAgentToolCapabilityCleanupMigration( { sqlitePresenter: presenter.sessionDataMigrationSQLite, configPresenter: presenter.configPresenter, @@ -33,7 +33,7 @@ export const disabledSearchToolCleanupHook: LifecycleHook = { }) .catch((error) => { console.error( - 'disabledSearchToolCleanupHook: failed to start disabled search tool cleanup:', + 'disabledAgentToolCleanupHook: failed to start disabled Agent tool cleanup:', error ) }) diff --git a/src/main/presenter/lifecyclePresenter/hooks/index.ts b/src/main/presenter/lifecyclePresenter/hooks/index.ts index 9482e6596..9799ef466 100644 --- a/src/main/presenter/lifecyclePresenter/hooks/index.ts +++ b/src/main/presenter/lifecyclePresenter/hooks/index.ts @@ -15,7 +15,7 @@ export { legacyImportHook } from './after-start/legacyImportHook' export { rtkHealthCheckHook } from './after-start/rtkHealthCheckHook' export { usageStatsBackfillHook } from './after-start/usageStatsBackfillHook' export { sqliteMainlineNormalizationHook } from './after-start/sqliteMainlineNormalizationHook' -export { disabledSearchToolCleanupHook } from './after-start/disabledSearchToolCleanupHook' +export { disabledAgentToolCleanupHook } from './after-start/disabledAgentToolCleanupHook' export { cronJobsStartHook } from './after-start/cronJobsStartHook' export { memoryMaintenanceStartHook } from './after-start/memoryMaintenanceStartHook' export { mcpShutdownHook } from './beforeQuit/mcpShutdownHook' diff --git a/src/main/presenter/startupMigrations/sessionDataMigrations.ts b/src/main/presenter/startupMigrations/sessionDataMigrations.ts index 51f3a7ddf..0ead9266d 100644 --- a/src/main/presenter/startupMigrations/sessionDataMigrations.ts +++ b/src/main/presenter/startupMigrations/sessionDataMigrations.ts @@ -9,9 +9,12 @@ import type { AppSessionService } from '@/agent/shared/appSessionService' import type { SQLitePresenter } from '../sqlitePresenter' import type { DeepChatMessageRow } from '../sqlitePresenter/tables/deepchatMessages' import type { StartupWorkloadTaskContext } from '../startupWorkloadCoordinator' +import { normalizeDisabledAgentTools } from '@/agent/shared/agentSessionNormalization' export const SQLITE_MAINLINE_NORMALIZATION_KEY = 'sqlite-mainline-normalization-v1' -export const DISABLED_SEARCH_TOOL_CLEANUP_KEY = 'agent-disabled-search-tool-cleanup-v1' +export const DISABLED_SEARCH_TOOL_CLEANUP_V1_KEY = 'agent-disabled-search-tool-cleanup-v1' +export const DISABLED_AGENT_TOOL_CAPABILITY_CLEANUP_KEY = + 'agent-disabled-tool-capability-cleanup-v2' export type SessionDataMigrationSQLitePort = Pick< SQLitePresenter, @@ -33,13 +36,6 @@ type SessionDataMigrationDependencies = { appSessionService: AppSessionService } -const LEGACY_PERSISTED_DISABLED_AGENT_TOOLS = new Set(['find', 'grep', 'ls']) -const LEGACY_AGENT_TOOL_NAME_MAP: Record = { - yo_browser_cdp_send: 'cdp_send', - yo_browser_window_open: 'load_url', - yo_browser_window_list: 'get_browser_status' -} - const yieldToEventLoop = async (): Promise => { await new Promise((resolve) => setTimeout(resolve, 0)) } @@ -320,95 +316,135 @@ export async function runMainlineNormalizationMigration( } } -const normalizeDisabledAgentTools = (disabledAgentTools?: string[]): string[] => { - if (!Array.isArray(disabledAgentTools)) return [] - return Array.from( - new Set( - disabledAgentTools - .filter((item): item is string => typeof item === 'string') - .map((item) => item.trim()) - .map((item) => LEGACY_AGENT_TOOL_NAME_MAP[item] ?? item) - .filter((item) => Boolean(item) && !LEGACY_PERSISTED_DISABLED_AGENT_TOOLS.has(item)) - ) - ).sort((left, right) => left.localeCompare(right)) -} - const areStringArraysEqual = (left: string[], right: string[]): boolean => left.length === right.length && left.every((item, index) => item === right[index]) -async function cleanupDeepChatAgentConfigDisabledTools( - configPresenter: IConfigPresenter -): Promise { - const agents = await configPresenter.listAgents() - let updatedCount = 0 - for (const agent of agents) { - if (agent.type !== 'deepchat') continue - const config = await configPresenter.getDeepChatAgentConfig(agent.id) - if (!Array.isArray(config?.disabledAgentTools)) continue - const normalized = normalizeDisabledAgentTools(config.disabledAgentTools) - if (areStringArraysEqual(config.disabledAgentTools, normalized)) continue - await configPresenter.updateDeepChatAgent(agent.id, { - config: { disabledAgentTools: normalized } - }) - updatedCount += 1 - } - return updatedCount -} - -export async function runDisabledSearchToolCleanupMigration( +export async function runDisabledAgentToolCapabilityCleanupMigration( { sqlitePresenter, configPresenter, appSessionService }: SessionDataMigrationDependencies, taskContext?: StartupWorkloadTaskContext ): Promise { const current = sqlitePresenter.configTables.getAgentSetting<{ status?: 'running' | 'completed' | 'failed' }>( - DISABLED_SEARCH_TOOL_CLEANUP_KEY + DISABLED_AGENT_TOOL_CAPABILITY_CLEANUP_KEY ) ?? null if (current?.status === 'completed') return const startedAt = Date.now() - sqlitePresenter.configTables.setAgentSetting(DISABLED_SEARCH_TOOL_CLEANUP_KEY, { + const batchSize = 50 + let processedCount = 0 + let updatedCount = 0 + let configProcessedCount = 0 + let configUpdatedCount = 0 + const writeRunningState = (): void => { + sqlitePresenter.configTables.setAgentSetting(DISABLED_AGENT_TOOL_CAPABILITY_CLEANUP_KEY, { + status: 'running', + startedAt, + finishedAt: null, + updatedAt: Date.now(), + processedCount, + updatedCount, + configProcessedCount, + configUpdatedCount + }) + } + const yieldForBatch = async (): Promise => { + writeRunningState() + await (taskContext?.yield() ?? yieldToEventLoop()) + } + + sqlitePresenter.configTables.setAgentSetting(DISABLED_AGENT_TOOL_CAPABILITY_CLEANUP_KEY, { status: 'running', startedAt, finishedAt: null, - updatedAt: startedAt + updatedAt: startedAt, + processedCount, + updatedCount, + configProcessedCount, + configUpdatedCount }) try { - const sessionRows = sqlitePresenter - .getDatabase() - .prepare<[], { id: string }>('SELECT id FROM new_sessions ORDER BY updated_at ASC') - .all() - let processedCount = 0 - let updatedCount = 0 - for (const sessionRow of sessionRows) { - const disabledAgentTools = sqlitePresenter.newSessionsTable.getDisabledAgentTools( - sessionRow.id - ) - const normalized = normalizeDisabledAgentTools(disabledAgentTools) - if (!areStringArraysEqual(disabledAgentTools, normalized)) { - appSessionService.updateDisabledAgentTools(sessionRow.id, normalized) - updatedCount += 1 + const db = sqlitePresenter.getDatabase() + const firstSessionBatch = db.prepare<[number], { id: string }>( + `SELECT id + FROM new_sessions + ORDER BY id ASC + LIMIT ?` + ) + const nextSessionBatch = db.prepare<[string, number], { id: string }>( + `SELECT id + FROM new_sessions + WHERE id > ? + ORDER BY id ASC + LIMIT ?` + ) + let sessionCursor: string | null = null + while (true) { + const sessionRows = + sessionCursor === null + ? firstSessionBatch.all(batchSize) + : nextSessionBatch.all(sessionCursor, batchSize) + if (sessionRows.length === 0) break + + for (const sessionRow of sessionRows) { + const disabledAgentTools = sqlitePresenter.newSessionsTable.getDisabledAgentTools( + sessionRow.id + ) + const normalized = normalizeDisabledAgentTools(disabledAgentTools, { + dropLegacySearchTools: true + }) + if (!areStringArraysEqual(disabledAgentTools, normalized)) { + appSessionService.updateDisabledAgentTools(sessionRow.id, normalized) + updatedCount += 1 + } + sessionCursor = sessionRow.id + processedCount += 1 } - processedCount += 1 - if (processedCount % 50 === 0) await (taskContext?.yield() ?? yieldToEventLoop()) + + await yieldForBatch() } - const configUpdatedCount = await cleanupDeepChatAgentConfigDisabledTools(configPresenter) - sqlitePresenter.configTables.setAgentSetting(DISABLED_SEARCH_TOOL_CLEANUP_KEY, { + const agents = await configPresenter.listAgents() + for (const agent of agents) { + if (agent.type !== 'deepchat') continue + const config = await configPresenter.getDeepChatAgentConfig(agent.id) + if (Array.isArray(config?.disabledAgentTools)) { + const normalized = normalizeDisabledAgentTools(config.disabledAgentTools, { + dropLegacySearchTools: true + }) + if (!areStringArraysEqual(config.disabledAgentTools, normalized)) { + await configPresenter.updateDeepChatAgent(agent.id, { + config: { disabledAgentTools: normalized } + }) + configUpdatedCount += 1 + } + } + configProcessedCount += 1 + if (configProcessedCount % batchSize === 0) await yieldForBatch() + } + + const finishedAt = Date.now() + sqlitePresenter.configTables.setAgentSetting(DISABLED_AGENT_TOOL_CAPABILITY_CLEANUP_KEY, { status: 'completed', startedAt, - finishedAt: Date.now(), - updatedAt: Date.now(), + finishedAt, + updatedAt: finishedAt, processedCount, updatedCount, + configProcessedCount, configUpdatedCount }) } catch (error) { - sqlitePresenter.configTables.setAgentSetting(DISABLED_SEARCH_TOOL_CLEANUP_KEY, { + const finishedAt = Date.now() + sqlitePresenter.configTables.setAgentSetting(DISABLED_AGENT_TOOL_CAPABILITY_CLEANUP_KEY, { status: 'failed', startedAt, - finishedAt: Date.now(), - updatedAt: Date.now(), + finishedAt, + updatedAt: finishedAt, + processedCount, + updatedCount, + configProcessedCount, + configUpdatedCount, error: error instanceof Error ? error.message : String(error) }) throw error diff --git a/test/main/agent/deepchat/deepChatAgentRepository.test.ts b/test/main/agent/deepchat/deepChatAgentRepository.test.ts index e643791dc..8354bcbf5 100644 --- a/test/main/agent/deepchat/deepChatAgentRepository.test.ts +++ b/test/main/agent/deepchat/deepChatAgentRepository.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest' import { DeepChatAgentRepository } from '@/agent/deepchat/deepChatAgentRepository' +import { TAPE_TOOL_NAMES } from '@shared/agentTools' function createRepository(sqlitePresenter: any): DeepChatAgentRepository { return new DeepChatAgentRepository({ @@ -327,6 +328,70 @@ describe('DeepChatAgentRepository', () => { expect(config.subagents?.every((slot) => slot.targetType === 'self')).toBe(true) }) + it('keeps non-configurable Tape names out of persisted and resolved Agent configs', () => { + const rows = new Map() + const agentsTable = { + get: (id: string) => rows.get(id), + create: (input: any) => { + const now = Date.now() + rows.set(input.id, { + id: input.id, + agent_type: input.agentType, + source: input.source, + name: input.name, + enabled: input.enabled ? 1 : 0, + protected: input.protected ? 1 : 0, + description: input.description ?? null, + icon: input.icon ?? null, + avatar_json: input.avatarJson, + config_json: input.configJson, + state_json: null, + created_at: now, + updated_at: now + }) + }, + update: (id: string, input: any) => { + const current = rows.get(id) + rows.set(id, { + ...current, + ...input, + config_json: input.configJson ?? current.config_json + }) + } + } + const repository = createRepository({ agentsTable }) + + repository.ensureBuiltin({ + config: { disabledAgentTools: [TAPE_TOOL_NAMES.search, 'read'] } + }) + const created = repository.create({ + name: 'Writer', + config: { disabledAgentTools: [TAPE_TOOL_NAMES.handoff, 'exec'] } + }) + + expect(repository.getConfig('deepchat')?.disabledAgentTools).toEqual(['read']) + expect(repository.getConfig(created.id)?.disabledAgentTools).toEqual(['exec']) + + repository.update(created.id, { + config: { disabledAgentTools: [TAPE_TOOL_NAMES.info, 'write'] } + }) + expect(repository.getConfig(created.id)?.disabledAgentTools).toEqual(['write']) + + rows.set('legacy-agent', { + ...rows.get(created.id), + id: 'legacy-agent', + config_json: JSON.stringify({ + disabledAgentTools: [TAPE_TOOL_NAMES.anchors, TAPE_TOOL_NAMES.context, 'exec'] + }) + }) + expect(repository.getConfig('legacy-agent')?.disabledAgentTools).toEqual([ + TAPE_TOOL_NAMES.anchors, + TAPE_TOOL_NAMES.context, + 'exec' + ]) + expect(repository.resolveConfig('legacy-agent').disabledAgentTools).toEqual(['exec']) + }) + it('inherits DeepChat image generation model from the builtin agent', () => { const now = Date.now() const rows = new Map([ diff --git a/test/main/presenter/lifecyclePresenter/startupMaintenanceHooks.test.ts b/test/main/presenter/lifecyclePresenter/startupMaintenanceHooks.test.ts index 3f21ed530..c46369067 100644 --- a/test/main/presenter/lifecyclePresenter/startupMaintenanceHooks.test.ts +++ b/test/main/presenter/lifecyclePresenter/startupMaintenanceHooks.test.ts @@ -17,7 +17,7 @@ const mocks = vi.hoisted(() => { legacyStart: vi.fn(async () => undefined), usageStart: vi.fn(async () => undefined), runMainline: vi.fn(async () => undefined), - runDisabledCleanup: vi.fn(async () => undefined), + runDisabledAgentToolCleanup: vi.fn(async () => undefined), rtkStart: vi.fn(async () => undefined), sessionDataMigrationSQLite: {}, configPresenter: {}, @@ -38,7 +38,7 @@ vi.mock('@/presenter', () => ({ vi.mock('@/presenter/startupMigrations/sessionDataMigrations', () => ({ runMainlineNormalizationMigration: mocks.runMainline, - runDisabledSearchToolCleanupMigration: mocks.runDisabledCleanup + runDisabledAgentToolCapabilityCleanupMigration: mocks.runDisabledAgentToolCleanup })) vi.mock('@/agent/shared/process/rtkRuntimeService', () => ({ @@ -48,7 +48,7 @@ vi.mock('@/agent/shared/process/rtkRuntimeService', () => ({ import { legacyImportHook } from '@/presenter/lifecyclePresenter/hooks/after-start/legacyImportHook' import { usageStatsBackfillHook } from '@/presenter/lifecyclePresenter/hooks/after-start/usageStatsBackfillHook' import { sqliteMainlineNormalizationHook } from '@/presenter/lifecyclePresenter/hooks/after-start/sqliteMainlineNormalizationHook' -import { disabledSearchToolCleanupHook } from '@/presenter/lifecyclePresenter/hooks/after-start/disabledSearchToolCleanupHook' +import { disabledAgentToolCleanupHook } from '@/presenter/lifecyclePresenter/hooks/after-start/disabledAgentToolCleanupHook' import { rtkHealthCheckHook } from '@/presenter/lifecyclePresenter/hooks/after-start/rtkHealthCheckHook' describe('startup maintenance hooks', () => { @@ -80,11 +80,11 @@ describe('startup maintenance hooks', () => { labelKey: 'startup.main.sqliteMainlineNormalization' }, { - hook: disabledSearchToolCleanupHook, - name: 'disabled-search-tool-cleanup', + hook: disabledAgentToolCleanupHook, + name: 'disabled-agent-tool-cleanup', priority: 23, - id: 'main:disabled-search-tool-cleanup', - labelKey: 'startup.main.disabledSearchToolCleanup' + id: 'main:disabled-agent-tool-cleanup', + labelKey: 'startup.main.disabledAgentToolCleanup' }, { hook: rtkHealthCheckHook, @@ -132,7 +132,7 @@ describe('startup maintenance hooks', () => { }, mocks.taskContext ) - expect(mocks.runDisabledCleanup).toHaveBeenCalledWith( + expect(mocks.runDisabledAgentToolCleanup).toHaveBeenCalledWith( { sqlitePresenter: mocks.sessionDataMigrationSQLite, configPresenter: mocks.configPresenter, diff --git a/test/main/presenter/sessionApplication/agentAssignmentPolicy.test.ts b/test/main/presenter/sessionApplication/agentAssignmentPolicy.test.ts index cc539d821..878730a86 100644 --- a/test/main/presenter/sessionApplication/agentAssignmentPolicy.test.ts +++ b/test/main/presenter/sessionApplication/agentAssignmentPolicy.test.ts @@ -4,6 +4,7 @@ import { normalizeActiveSkills, normalizeDisabledAgentTools } from '@/agent/shared/agentSessionNormalization' +import { TAPE_TOOL_NAMES } from '@shared/agentTools' function createHarness() { const agents = new Map([ @@ -245,6 +246,10 @@ describe('SessionAgentAssignmentPolicy', () => { dropLegacySearchTools: true }) ).toEqual(['cdp_send']) + expect( + normalizeDisabledAgentTools([...Object.values(TAPE_TOOL_NAMES), 'read', 'tape_search']) + ).toEqual(['read']) + expect(normalizeDisabledAgentTools(['__proto__'])).toEqual(['__proto__']) expect(normalizeActiveSkills([' review ', '', 'review', 'test'])).toEqual(['review', 'test']) }) }) diff --git a/test/main/presenter/startupMigrations/sessionDataMigrations.test.ts b/test/main/presenter/startupMigrations/sessionDataMigrations.test.ts index 6cf948081..7c1a9db18 100644 --- a/test/main/presenter/startupMigrations/sessionDataMigrations.test.ts +++ b/test/main/presenter/startupMigrations/sessionDataMigrations.test.ts @@ -1,15 +1,18 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import { - DISABLED_SEARCH_TOOL_CLEANUP_KEY, + DISABLED_AGENT_TOOL_CAPABILITY_CLEANUP_KEY, + DISABLED_SEARCH_TOOL_CLEANUP_V1_KEY, SQLITE_MAINLINE_NORMALIZATION_KEY, - runDisabledSearchToolCleanupMigration, + runDisabledAgentToolCapabilityCleanupMigration, runMainlineNormalizationMigration, type SessionDataMigrationSQLitePort } from '@/presenter/startupMigrations/sessionDataMigrations' +import { TAPE_TOOL_NAMES } from '@shared/agentTools' function createFixture() { const settings = new Map() const sessionRows: Array<{ id: string }> = [] + const sessionDisabledTools = new Map() const statements: string[] = [] const sqlitePresenter = { configTables: { @@ -20,16 +23,22 @@ function createFixture() { prepare: vi.fn((sql: string) => { statements.push(sql) return { - all: vi.fn(() => - sql.includes('SELECT id FROM new_sessions ORDER BY updated_at ASC') ? sessionRows : [] - ) + all: vi.fn((...params: unknown[]) => { + if (!sql.includes('FROM new_sessions') || !sql.includes('ORDER BY id ASC')) return [] + const cursor = sql.includes('WHERE id > ?') ? String(params[0]) : null + const limit = Number(params.at(-1)) + return sessionRows + .filter((row) => cursor === null || row.id > cursor) + .sort((left, right) => left.id.localeCompare(right.id)) + .slice(0, limit) + }) } }) })), newSessionsTable: { get: vi.fn(() => null), getActiveSkills: vi.fn(() => []), - getDisabledAgentTools: vi.fn(() => []) + getDisabledAgentTools: vi.fn((sessionId: string) => sessionDisabledTools.get(sessionId) ?? []) }, newSessionActiveSkillsTable: { replaceForSession: vi.fn() }, newSessionDisabledAgentToolsTable: { replaceForSession: vi.fn() }, @@ -44,7 +53,11 @@ function createFixture() { getDeepChatAgentConfig: vi.fn(), updateDeepChatAgent: vi.fn() } - const appSessionService = { updateDisabledAgentTools: vi.fn() } + const appSessionService = { + updateDisabledAgentTools: vi.fn((sessionId: string, disabledAgentTools: string[]) => { + sessionDisabledTools.set(sessionId, disabledAgentTools) + }) + } const taskContext = { reportProgress: vi.fn(), yield: vi.fn(async () => undefined) @@ -53,6 +66,7 @@ function createFixture() { return { settings, sessionRows, + sessionDisabledTools, statements, sqlitePresenter, configPresenter, @@ -112,53 +126,155 @@ describe('session data migrations', () => { }) }) - it('normalizes session and agent-config disabled tools and records completion', async () => { + it('runs v2 after v1 completion and removes Tape names from sessions and Agent configs', async () => { const fixture = createFixture() + fixture.settings.set(DISABLED_SEARCH_TOOL_CLEANUP_V1_KEY, { status: 'completed' }) fixture.sessionRows.push({ id: 'session-1' }) - fixture.sqlitePresenter.newSessionsTable.getDisabledAgentTools.mockReturnValue([ + fixture.sessionDisabledTools.set('session-1', [ + ...Object.values(TAPE_TOOL_NAMES), 'grep', 'find', 'exec', - 'yo_browser_cdp_send' + 'yo_browser_cdp_send', + 'custom_tool' ]) fixture.configPresenter.listAgents.mockResolvedValue([ { id: 'deepchat', type: 'deepchat', name: 'DeepChat', enabled: true } ]) fixture.configPresenter.getDeepChatAgentConfig.mockResolvedValue({ - disabledAgentTools: ['ls', 'exec'] + disabledAgentTools: ['ls', 'exec', ...Object.values(TAPE_TOOL_NAMES)] }) - await runDisabledSearchToolCleanupMigration(fixture as never, fixture.taskContext as never) + await runDisabledAgentToolCapabilityCleanupMigration( + fixture as never, + fixture.taskContext as never + ) expect(fixture.appSessionService.updateDisabledAgentTools).toHaveBeenCalledWith('session-1', [ 'cdp_send', + 'custom_tool', 'exec' ]) expect(fixture.configPresenter.updateDeepChatAgent).toHaveBeenCalledWith('deepchat', { config: { disabledAgentTools: ['exec'] } }) - expect(fixture.settings.get(DISABLED_SEARCH_TOOL_CLEANUP_KEY)).toMatchObject({ + expect(fixture.settings.get(DISABLED_SEARCH_TOOL_CLEANUP_V1_KEY)).toEqual({ + status: 'completed' + }) + expect(fixture.settings.get(DISABLED_AGENT_TOOL_CAPABILITY_CLEANUP_KEY)).toMatchObject({ status: 'completed', processedCount: 1, updatedCount: 1, + configProcessedCount: 1, configUpdatedCount: 1 }) + expect(fixture.statements).toEqual( + expect.arrayContaining([ + expect.stringMatching(/ORDER BY id ASC\s+LIMIT \?/), + expect.stringMatching(/WHERE id > \?\s+ORDER BY id ASC\s+LIMIT \?/) + ]) + ) + }) + + it('uses bounded session batches and yields between them', async () => { + const fixture = createFixture() + for (let index = 0; index < 125; index += 1) { + const id = `session-${String(index).padStart(3, '0')}` + fixture.sessionRows.push({ id }) + fixture.sessionDisabledTools.set(id, [TAPE_TOOL_NAMES.search, 'read']) + } + + await runDisabledAgentToolCapabilityCleanupMigration( + fixture as never, + fixture.taskContext as never + ) + + expect(fixture.appSessionService.updateDisabledAgentTools).toHaveBeenCalledTimes(125) + expect(fixture.taskContext.yield).toHaveBeenCalledTimes(3) + expect(fixture.settings.get(DISABLED_AGENT_TOOL_CAPABILITY_CLEANUP_KEY)).toMatchObject({ + status: 'completed', + processedCount: 125, + updatedCount: 125 + }) + expect([...fixture.sessionDisabledTools.values()]).toEqual(expect.arrayContaining([['read']])) + expect( + [...fixture.sessionDisabledTools.values()].every( + (disabledAgentTools) => disabledAgentTools.length === 1 && disabledAgentTools[0] === 'read' + ) + ).toBe(true) + }) + + it('restarts safely after a partially applied session batch', async () => { + const fixture = createFixture() + fixture.sessionRows.push({ id: 'session-1' }, { id: 'session-2' }) + fixture.sessionDisabledTools.set('session-1', [TAPE_TOOL_NAMES.search, 'read']) + fixture.sessionDisabledTools.set('session-2', [TAPE_TOOL_NAMES.handoff, 'exec']) + let updateCount = 0 + fixture.appSessionService.updateDisabledAgentTools.mockImplementation( + (sessionId: string, disabledAgentTools: string[]) => { + updateCount += 1 + if (updateCount === 2) throw new Error('write interrupted') + fixture.sessionDisabledTools.set(sessionId, disabledAgentTools) + } + ) + + await expect( + runDisabledAgentToolCapabilityCleanupMigration(fixture as never, fixture.taskContext as never) + ).rejects.toThrow('write interrupted') + expect(fixture.sessionDisabledTools.get('session-1')).toEqual(['read']) + expect(fixture.sessionDisabledTools.get('session-2')).toEqual([TAPE_TOOL_NAMES.handoff, 'exec']) + expect(fixture.settings.get(DISABLED_AGENT_TOOL_CAPABILITY_CLEANUP_KEY)).toMatchObject({ + status: 'failed', + processedCount: 1, + updatedCount: 1, + error: 'write interrupted' + }) + + fixture.appSessionService.updateDisabledAgentTools.mockImplementation( + (sessionId: string, disabledAgentTools: string[]) => { + fixture.sessionDisabledTools.set(sessionId, disabledAgentTools) + } + ) + await runDisabledAgentToolCapabilityCleanupMigration( + fixture as never, + fixture.taskContext as never + ) + + expect(fixture.sessionDisabledTools.get('session-1')).toEqual(['read']) + expect(fixture.sessionDisabledTools.get('session-2')).toEqual(['exec']) + expect(fixture.settings.get(DISABLED_AGENT_TOOL_CAPABILITY_CLEANUP_KEY)).toMatchObject({ + status: 'completed', + processedCount: 2, + updatedCount: 1 + }) }) - it('skips completed cleanup and records cleanup failures', async () => { + it('skips completed v2 cleanup and records retryable failures', async () => { const completed = createFixture() - completed.settings.set(DISABLED_SEARCH_TOOL_CLEANUP_KEY, { status: 'completed' }) - await runDisabledSearchToolCleanupMigration(completed as never, completed.taskContext as never) + completed.settings.set(DISABLED_AGENT_TOOL_CAPABILITY_CLEANUP_KEY, { status: 'completed' }) + await runDisabledAgentToolCapabilityCleanupMigration( + completed as never, + completed.taskContext as never + ) expect(completed.sqlitePresenter.getDatabase).not.toHaveBeenCalled() const failed = createFixture() failed.configPresenter.listAgents.mockRejectedValue(new Error('config unavailable')) await expect( - runDisabledSearchToolCleanupMigration(failed as never, failed.taskContext as never) + runDisabledAgentToolCapabilityCleanupMigration(failed as never, failed.taskContext as never) ).rejects.toThrow('config unavailable') - expect(failed.settings.get(DISABLED_SEARCH_TOOL_CLEANUP_KEY)).toMatchObject({ + expect(failed.settings.get(DISABLED_AGENT_TOOL_CAPABILITY_CLEANUP_KEY)).toMatchObject({ status: 'failed', error: 'config unavailable' }) + + failed.configPresenter.listAgents.mockResolvedValue([]) + await runDisabledAgentToolCapabilityCleanupMigration( + failed as never, + failed.taskContext as never + ) + expect(failed.settings.get(DISABLED_AGENT_TOOL_CAPABILITY_CLEANUP_KEY)).toMatchObject({ + status: 'completed' + }) }) }) diff --git a/test/main/scripts/architectureGuard.test.ts b/test/main/scripts/architectureGuard.test.ts index 26ba44179..07ea03dc5 100644 --- a/test/main/scripts/architectureGuard.test.ts +++ b/test/main/scripts/architectureGuard.test.ts @@ -520,7 +520,7 @@ const SESSION_BOUNDARY_HOOK_ROOT = path.join( ) const SESSION_BOUNDARY_HOOK_FIXTURES = [ { - filePath: path.join(SESSION_BOUNDARY_HOOK_ROOT, 'disabledSearchToolCleanupHook.ts'), + filePath: path.join(SESSION_BOUNDARY_HOOK_ROOT, 'disabledAgentToolCleanupHook.ts'), rules: ['presenter'], source: ` declare const presenter: { agentSessionPresenter: unknown } From 1a8b4fa67286eead49d0d2dc051681506cf7fd57 Mon Sep 17 00:00:00 2001 From: yyhhyyyyyy Date: Wed, 15 Jul 2026 19:10:37 +0800 Subject: [PATCH 5/5] fix(tools): harden tape capability isolation --- .../session-boundary-cleanup/spec.md | 4 +- .../tape-capability-layering/plan.md | 17 +- .../tape-capability-layering/spec.md | 21 ++- .../tape-capability-layering/tasks.md | 4 + docs/architecture/tool-system.md | 9 +- .../disabledAgentToolCleanupHook.ts | 3 +- .../sqliteMainlineNormalizationHook.ts | 3 +- .../sessionDataMigrations.ts | 25 ++- .../agentTools/agentTapeTools.ts | 3 + .../agentTools/agentToolManager.ts | 47 +++-- src/main/presenter/toolPresenter/index.ts | 10 +- src/shared/agentTools.ts | 12 ++ .../startupMaintenanceHooks.test.ts | 12 +- .../sessionDataMigrations.sqlite.test.ts | 171 ++++++++++++++++++ .../sessionDataMigrations.test.ts | 77 +++++--- .../agentTools/agentTapeTools.test.ts | 33 +++- .../toolPresenter/toolPresenter.test.ts | 118 +++++++++++- 17 files changed, 488 insertions(+), 81 deletions(-) create mode 100644 test/main/presenter/startupMigrations/sessionDataMigrations.sqlite.test.ts diff --git a/docs/architecture/session-boundary-cleanup/spec.md b/docs/architecture/session-boundary-cleanup/spec.md index 39e005a6c..4890b97fb 100644 --- a/docs/architecture/session-boundary-cleanup/spec.md +++ b/docs/architecture/session-boundary-cleanup/spec.md @@ -164,7 +164,9 @@ AgentSessionPresenter - Disabled-tool cleanup preserves the historical `agent-disabled-search-tool-cleanup-v1` record but uses the independent `agent-disabled-tool-capability-cleanup-v2` completion gate so users who completed v1 still receive Tape capability cleanup. Session and agent-config cleanup remains - idempotent, bounded, yielding, and retryable with completion/failure metadata. + idempotent, bounded, yielding, and retryable with completion/failure metadata. Session cleanup + updates the legacy JSON and normalized disabled-tool rows transactionally without changing + `updated_at` or environment recency. ### History search diff --git a/docs/architecture/tape-capability-layering/plan.md b/docs/architecture/tape-capability-layering/plan.md index 161df3d9a..958066582 100644 --- a/docs/architecture/tape-capability-layering/plan.md +++ b/docs/architecture/tape-capability-layering/plan.md @@ -6,6 +6,9 @@ `diagnostic` in a shared Agent tool policy module. - Keep the five Tape names and their classifications in one registry. Treat unregistered existing Agent tools as `user-configurable` to avoid unrelated behavior changes. +- Require every Agent tool assembly site to state its expected exposure and assert that it matches + the shared registry. This preserves the compatibility fallback while preventing a capability + registered as internal from silently entering the configurable catalog. - Keep exposure metadata out of provider-bound `MCPToolDefinition` values. Main-process catalog selection and execution checks consume the policy directly. @@ -14,6 +17,10 @@ - Keep `ToolPresenter.getAllToolDefinitions()` as the runtime catalog. Apply `disabledAgentTools` only to `user-configurable` definitions and publish only this catalog into the per-conversation `ToolMapper`. +- Track the conversation provenance of the compatibility mapper. A published per-conversation + mapper is authoritative; an absent tool, or a missing/cleared mapper while the compatibility + snapshot belongs to another conversation, is unavailable. Preserve only the no-ID draft snapshot + as the bridge into the first persisted turn and for legacy no-ID calls. - Add `getConfigurableAgentToolDefinitions()` for renderer configuration. Return only `user-configurable` Agent definitions and do not resolve MCP definitions, publish mappings, or remember conversation MCP access. @@ -46,6 +53,8 @@ Persisted DeepChat thread -> configurable catalog -> user-configurable Agent too and handoff on the lower runtime/application Tape boundary for future diagnostics and runtime use. - Reject known non-model Tape names at the Agent tool execution boundary, including stale deferred calls, while keeping their historical facts replayable. +- Revalidate the DeepChat session and complete recall pair at execution time before calling either + runtime port. ## Handoff and Persistence @@ -55,6 +64,8 @@ Persisted DeepChat thread -> configurable catalog -> user-configurable Agent too - Normalize all disabled-tool writes to drop non-configurable Tape names. - Add a separately keyed, idempotent startup cleanup for existing session and Agent configuration rows. Reuse the existing bounded-yield migration pattern and do not alter transcript or Tape data. +- Persist session cleanup through a migration-owned transaction that updates both disabled-tool + stores without touching `updated_at` or synchronizing environment activity. ## Compatibility @@ -70,11 +81,15 @@ Persisted DeepChat thread -> configurable catalog -> user-configurable Agent too - Cover runtime/configurable catalog membership and prove configurable reads have no mapper/access side effects. +- Cover concurrent conversation mappings and prove a missing, cleared, or incomplete conversation + catalog cannot resolve through another conversation's latest mapping, while draft-origin mapping + compatibility remains intact. - Cover DeepChat, ACP, missing-session, missing-port, stale-disabled-list, same-name MCP, and deferred call behavior. - Cover pre-append handoff validation and derived provenance for valid handoffs. - Cover v2 cleanup idempotence, preservation of ordinary disabled tools, Agent config cleanup, and - bounded session iteration. + bounded session iteration. Use a real SQLite test to preserve session ordering and environment + recency. - Run focused main and renderer tests before each implementation commit, then the full repository validation sequence before final handoff. diff --git a/docs/architecture/tape-capability-layering/spec.md b/docs/architecture/tape-capability-layering/spec.md index 9a12a60a3..860d4a568 100644 --- a/docs/architecture/tape-capability-layering/spec.md +++ b/docs/architecture/tape-capability-layering/spec.md @@ -39,11 +39,22 @@ This behavior is reported by 8. Runtime handoff requires a non-empty durable summary before advancing the reconstruction cursor. 9. Persisted Tape names are removed from Agent and session disabled-tool lists without modifying messages, Tape entries, or replay manifests. +10. A published per-conversation runtime catalog is authoritative for execution. A missing tool in + that catalog, or a missing/cleared mapper while the global snapshot belongs to another + conversation, cannot fall back across conversations. A global snapshot published without a + conversation ID remains available as the draft-to-persisted-session compatibility bridge. +11. Tape recall execution revalidates the persisted DeepChat session and complete runtime pair so + stale, direct, or fabricated calls cannot bypass catalog availability. +12. Capability cleanup changes only disabled-tool state; it preserves session activity timestamps, + conversation ordering, and environment recency. +13. Every Agent tool assembly site declares its expected exposure and verifies it against the + shared exposure policy before adding definitions to a catalog. ## Acceptance Criteria -- A new-thread draft and its persisted session show the same configurable Agent tool groups; no - `agent-tape` group appears in either state. +- No `agent-tape` group appears in either a new-thread draft or a persisted session. Other + configurable Agent tool groups may continue to vary with session-scoped capabilities such as + Memory, image generation, subagents, and active Skills. - A DeepChat runtime session exposes exactly `tape_search` and `tape_context` from the Tape tool group, even when a legacy disabled list contains those names. - ACP sessions, missing conversation IDs, and incomplete recall ports expose no Tape model tools. @@ -51,7 +62,10 @@ This behavior is reported by anchor or otherwise mutating Tape state. - Empty or whitespace-only runtime handoff summaries fail before an anchor is appended. - The disabled-tool cleanup is idempotent, preserves ordinary disabled tools, and yields while - processing large session sets. + processing large session sets without changing session or environment recency. +- A request carrying a conversation ID cannot execute through another conversation's latest + mapping when its own catalog is missing, cleared, or does not contain the tool, while a draft + catalog published without an ID remains usable for the first persisted turn. - Memory enablement, memory extraction/injection, subagents, skills, MCP tools, and ordinary Agent tool switches retain their existing behavior. - Focused tests, typecheck, full tests, formatting, i18n validation, and lint pass. @@ -70,6 +84,7 @@ This behavior is reported by - Implementing an anchor graph, cross-anchor traversal, or a second `TapeViewPolicy`. - Adding a Tape inspector or special-purpose Tape trace UI. - Reclassifying Memory, Subagent, Skill, Browser, or other non-Tape Agent tools. +- Making every configurable Agent tool group identical between draft and persisted sessions. - Making Bub's current tool exposure policy a compatibility target. ## Open Questions diff --git a/docs/architecture/tape-capability-layering/tasks.md b/docs/architecture/tape-capability-layering/tasks.md index f98ecd07b..b8ab8eac9 100644 --- a/docs/architecture/tape-capability-layering/tasks.md +++ b/docs/architecture/tape-capability-layering/tasks.md @@ -8,3 +8,7 @@ - [x] T6: Remove non-configurable Tape names from new and persisted disabled-tool state. - [x] T7: Update maintained tracked architecture contracts and focused regression tests. - [x] T8: Run format, i18n validation, lint, typecheck, and the full test suite. +- [x] T9: Make per-conversation mappings authoritative and revalidate Tape recall on execution. +- [x] T10: Preserve session and environment recency during disabled-tool capability cleanup. +- [x] T11: Require explicit exposure expectations at Agent tool assembly sites. +- [x] T12: Align acceptance criteria and add regression coverage for the hardened boundaries. diff --git a/docs/architecture/tool-system.md b/docs/architecture/tool-system.md index 76f4f8ed3..f3543e9dc 100644 --- a/docs/architecture/tool-system.md +++ b/docs/architecture/tool-system.md @@ -54,7 +54,10 @@ graph LR 2. 从 `AgentToolManager` 拉取本地 agent tools。 3. 保留 built-in reserved names，拒绝同名 MCP definition；其他重名仍按现有 collision policy 处理。 4. 只对 `user-configurable` definitions 应用 `disabledAgentTools`。 -5. 用 `ToolMapper` 记录最终来源，并为每个 conversation 维护独立映射。 +5. 用 `ToolMapper` 记录最终来源，并为每个 conversation 维护独立映射，同时记录兼容 mapping 的 + conversation 来源。已发布的 conversation mapping 是执行边界；mapping 不含目标工具，或 mapping + 缺失/已清理且兼容 snapshot 属于另一 conversation 时，不允许跨 conversation 回退。仅无 ID 的 draft + snapshot 可作为首轮持久化执行的兼容桥，并继续兼容不带 conversation ID 的旧调用。 `ToolPresenter.getConfigurableAgentToolDefinitions()` 是 renderer settings 使用的 configurable catalog。它只返回 `user-configurable` Agent tools，不解析 MCP catalog、不发布 `ToolMapper`，也不修改 @@ -72,7 +75,9 @@ Tape 的 exposure matrix 为： | `tape_handoff` | `runtime-only` | 仅 runtime handoff 路径可写，模型 dispatcher 拒绝 | 五个 Tape 名称全部 reserved，防止同名 MCP 工具劫持。`disabledAgentTools` 的持久化正规化会永久移除 -这些非配置能力；v2 startup cleanup 幂等清理旧 Session 和 DeepChat Agent Config。 +这些非配置能力；v2 startup cleanup 幂等清理旧 Session 和 DeepChat Agent Config，并且不更新 +Session activity timestamp 或 environment recency。Agent tool 装配点必须显式声明预期 exposure，且与 +共享 policy 不一致时拒绝发布 definition。 这意味着 `DeepChatLoopEngine` 不知道 tool 的真实来源，只接收 `MCPToolDefinition[]` snapshot 和窄 execution/result ports。`AgentRuntimePresenter` 保留 adapter wiring，但 tool mapping/collision/dispatch owner diff --git a/src/main/presenter/lifecyclePresenter/hooks/after-start/disabledAgentToolCleanupHook.ts b/src/main/presenter/lifecyclePresenter/hooks/after-start/disabledAgentToolCleanupHook.ts index 1bdd1b977..05b4cd7f8 100644 --- a/src/main/presenter/lifecyclePresenter/hooks/after-start/disabledAgentToolCleanupHook.ts +++ b/src/main/presenter/lifecyclePresenter/hooks/after-start/disabledAgentToolCleanupHook.ts @@ -24,8 +24,7 @@ export const disabledAgentToolCleanupHook: LifecycleHook = { await runDisabledAgentToolCapabilityCleanupMigration( { sqlitePresenter: presenter.sessionDataMigrationSQLite, - configPresenter: presenter.configPresenter, - appSessionService: presenter.appSessionService + configPresenter: presenter.configPresenter }, taskContext ) diff --git a/src/main/presenter/lifecyclePresenter/hooks/after-start/sqliteMainlineNormalizationHook.ts b/src/main/presenter/lifecyclePresenter/hooks/after-start/sqliteMainlineNormalizationHook.ts index 0baede9bb..06a2ee867 100644 --- a/src/main/presenter/lifecyclePresenter/hooks/after-start/sqliteMainlineNormalizationHook.ts +++ b/src/main/presenter/lifecyclePresenter/hooks/after-start/sqliteMainlineNormalizationHook.ts @@ -24,8 +24,7 @@ export const sqliteMainlineNormalizationHook: LifecycleHook = { await runMainlineNormalizationMigration( { sqlitePresenter: presenter.sessionDataMigrationSQLite, - configPresenter: presenter.configPresenter, - appSessionService: presenter.appSessionService + configPresenter: presenter.configPresenter }, taskContext ) diff --git a/src/main/presenter/startupMigrations/sessionDataMigrations.ts b/src/main/presenter/startupMigrations/sessionDataMigrations.ts index 0ead9266d..97e0b7c29 100644 --- a/src/main/presenter/startupMigrations/sessionDataMigrations.ts +++ b/src/main/presenter/startupMigrations/sessionDataMigrations.ts @@ -5,7 +5,6 @@ import type { UserMessageContent } from '@shared/types/agent-interface' import type { IConfigPresenter } from '@shared/presenter' -import type { AppSessionService } from '@/agent/shared/appSessionService' import type { SQLitePresenter } from '../sqlitePresenter' import type { DeepChatMessageRow } from '../sqlitePresenter/tables/deepchatMessages' import type { StartupWorkloadTaskContext } from '../startupWorkloadCoordinator' @@ -33,7 +32,6 @@ export type SessionDataMigrationSQLitePort = Pick< type SessionDataMigrationDependencies = { sqlitePresenter: SessionDataMigrationSQLitePort configPresenter: IConfigPresenter - appSessionService: AppSessionService } const yieldToEventLoop = async (): Promise => { @@ -320,7 +318,7 @@ const areStringArraysEqual = (left: string[], right: string[]): boolean => left.length === right.length && left.every((item, index) => item === right[index]) export async function runDisabledAgentToolCapabilityCleanupMigration( - { sqlitePresenter, configPresenter, appSessionService }: SessionDataMigrationDependencies, + { sqlitePresenter, configPresenter }: SessionDataMigrationDependencies, taskContext?: StartupWorkloadTaskContext ): Promise { const current = @@ -378,6 +376,22 @@ export async function runDisabledAgentToolCapabilityCleanupMigration( ORDER BY id ASC LIMIT ?` ) + const updateSessionDisabledTools = db.prepare<[string, string]>( + 'UPDATE new_sessions SET disabled_agent_tools = ? WHERE id = ?' + ) + const persistSessionDisabledTools = db.transaction( + (sessionId: string, disabledAgentTools: string[]): boolean => { + const result = updateSessionDisabledTools.run(JSON.stringify(disabledAgentTools), sessionId) + if (result.changes === 0) { + return false + } + sqlitePresenter.newSessionDisabledAgentToolsTable.replaceForSession( + sessionId, + disabledAgentTools + ) + return true + } + ) let sessionCursor: string | null = null while (true) { const sessionRows = @@ -394,8 +408,9 @@ export async function runDisabledAgentToolCapabilityCleanupMigration( dropLegacySearchTools: true }) if (!areStringArraysEqual(disabledAgentTools, normalized)) { - appSessionService.updateDisabledAgentTools(sessionRow.id, normalized) - updatedCount += 1 + if (persistSessionDisabledTools(sessionRow.id, normalized)) { + updatedCount += 1 + } } sessionCursor = sessionRow.id processedCount += 1 diff --git a/src/main/presenter/toolPresenter/agentTools/agentTapeTools.ts b/src/main/presenter/toolPresenter/agentTools/agentTapeTools.ts index e3b443ed9..9f8d54c1d 100644 --- a/src/main/presenter/toolPresenter/agentTools/agentTapeTools.ts +++ b/src/main/presenter/toolPresenter/agentTools/agentTapeTools.ts @@ -225,6 +225,9 @@ export class AgentTapeToolHandler { if (!normalizedConversationId) { throw new Error(`${toolName} requires a conversation ID.`) } + if (!(await this.canUse(normalizedConversationId))) { + throw new Error('Tape recall tools are not available for this conversation.') + } if (toolName === TAPE_TOOL_NAMES.search) { if (!this.runtimePort.searchTape) { diff --git a/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts b/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts index 75b6a2368..0dabe6a3c 100644 --- a/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts +++ b/src/main/presenter/toolPresenter/agentTools/agentToolManager.ts @@ -43,9 +43,9 @@ import { AgentMemoryToolHandler } from './agentMemoryTools' import { createAgentToolErrorResult } from '@shared/lib/agentToolResultEnvelope' import { CRON_JOB_AGENT_TOOL_NAME, - TAPE_TOOL_NAMES, + assertAgentToolExposure, isTapeToolName, - isUserConfigurableAgentTool + type AgentToolExposure } from '@shared/agentTools' import { CRON_JOB_TOOL_SERVER_NAME, @@ -376,10 +376,18 @@ export class AgentToolManager { const defs: MCPToolDefinition[] = [] const isAgentMode = context.chatMode === 'agent' const isConfigurableCatalog = context.catalogPurpose === 'configurable' - const acceptsToolName = (toolName: string): boolean => - !isConfigurableCatalog || isUserConfigurableAgentTool(toolName) - const appendDefinitions = (definitions: MCPToolDefinition[]): void => { - defs.push(...definitions.filter((definition) => acceptsToolName(definition.function.name))) + const acceptsExposure = (exposure: AgentToolExposure): boolean => + !isConfigurableCatalog || exposure === 'user-configurable' + const appendDefinitions = ( + definitions: MCPToolDefinition[], + expectedExposure: AgentToolExposure + ): void => { + for (const definition of definitions) { + assertAgentToolExposure(definition.function.name, expectedExposure) + } + if (acceptsExposure(expectedExposure)) { + defs.push(...definitions) + } } if (!isConfigurableCatalog) { @@ -389,23 +397,22 @@ export class AgentToolManager { // 1. FileSystem tools (agent mode only) if (isAgentMode && (isConfigurableCatalog || this.fileSystemHandler)) { const fsDefs = this.getFileSystemToolDefinitions() - appendDefinitions(fsDefs) + appendDefinitions(fsDefs, 'user-configurable') } // 2. Built-in question tool (all modes) - appendDefinitions(this.getQuestionToolDefinitions()) + appendDefinitions(this.getQuestionToolDefinitions(), 'user-configurable') // 2.1. Progress checklist tool (deepchat regular sessions only) if (isAgentMode && this.planTool) { - appendDefinitions([this.planTool.getToolDefinition()]) + appendDefinitions([this.planTool.getToolDefinition()], 'user-configurable') } // 2.15. Session tape tools (DeepChat sessions only) - const acceptsTapeTools = Object.values(TAPE_TOOL_NAMES).some(acceptsToolName) - if (isAgentMode && acceptsTapeTools && this.tapeToolHandler) { + if (isAgentMode && acceptsExposure('system-model') && this.tapeToolHandler) { try { if (await this.tapeToolHandler.canUse(context.conversationId)) { - appendDefinitions(this.tapeToolHandler.getToolDefinitions()) + appendDefinitions(this.tapeToolHandler.getToolDefinitions(), 'system-model') } } catch (error) { logger.warn('[AgentToolManager] Failed to resolve tape tool availability', { error }) @@ -416,7 +423,7 @@ export class AgentToolManager { if (isAgentMode && this.memoryToolHandler) { try { if (await this.memoryToolHandler.canUse(context.conversationId)) { - appendDefinitions(this.memoryToolHandler.getToolDefinitions()) + appendDefinitions(this.memoryToolHandler.getToolDefinitions(), 'user-configurable') } } catch (error) { logger.warn('[AgentToolManager] Failed to resolve memory tool availability', { error }) @@ -427,7 +434,7 @@ export class AgentToolManager { if (isAgentMode && this.imageGenerationTool) { try { if (await this.imageGenerationTool.canUse(context.conversationId)) { - appendDefinitions([this.imageGenerationTool.getToolDefinition()]) + appendDefinitions([this.imageGenerationTool.getToolDefinition()], 'user-configurable') } } catch (error) { logger.warn('[AgentToolManager] Failed to resolve image generation tool availability', { @@ -438,7 +445,7 @@ export class AgentToolManager { // 2.3. Scheduled task tool (disabled by default in DeepChat agent settings) if (isAgentMode && this.cronJobToolHandler?.canUse()) { - appendDefinitions([this.cronJobToolHandler.getToolDefinition()]) + appendDefinitions([this.cronJobToolHandler.getToolDefinition()], 'user-configurable') } // 2.5. Subagent orchestration tool (deepchat regular sessions only) @@ -448,7 +455,7 @@ export class AgentToolManager { context.conversationId ) if (subagentToolDefinition) { - appendDefinitions([subagentToolDefinition]) + appendDefinitions([subagentToolDefinition], 'user-configurable') } } catch (error) { logger.warn('[AgentToolManager] Failed to resolve subagent tool availability', { error }) @@ -458,13 +465,13 @@ export class AgentToolManager { // 3. Skill tools (agent mode only) if (isAgentMode && this.isSkillsEnabled()) { const skillDefs = this.getSkillToolDefinitions() - appendDefinitions(skillDefs) + appendDefinitions(skillDefs, 'user-configurable') if ( context.conversationId && (await this.hasRunnableSkillScripts(context.conversationId, context.activeSkillNames)) ) { - appendDefinitions([this.getSkillRunToolDefinition()]) + appendDefinitions([this.getSkillRunToolDefinition()], 'user-configurable') } } @@ -491,7 +498,7 @@ export class AgentToolManager { : Array.from(new Set([...allowedTools, ...requiredSettingsTools])) const settingsDefs = buildChatSettingsToolDefinitions(effectiveAllowedTools) - appendDefinitions(settingsDefs) + appendDefinitions(settingsDefs, 'user-configurable') } } catch (error) { logger.warn('[AgentToolManager] Failed to load DeepChat settings tools', { error }) @@ -501,7 +508,7 @@ export class AgentToolManager { // 5. YoBrowser CDP tools (agent mode only) if (isAgentMode) { try { - appendDefinitions(this.getYoBrowserToolHandler().getToolDefinitions()) + appendDefinitions(this.getYoBrowserToolHandler().getToolDefinitions(), 'user-configurable') } catch (error) { logger.warn('[AgentToolManager] Failed to load YoBrowser tools', { error }) } diff --git a/src/main/presenter/toolPresenter/index.ts b/src/main/presenter/toolPresenter/index.ts index 873559d50..5e53c1d81 100644 --- a/src/main/presenter/toolPresenter/index.ts +++ b/src/main/presenter/toolPresenter/index.ts @@ -117,6 +117,7 @@ type StoredMcpAccessContext = { export class ToolPresenter implements IToolPresenter { private readonly mapper: ToolMapper private readonly conversationMappers: Map + private globalMapperConversationId: string | null = null private readonly conversationMcpAccessContexts = new Map() private readonly options: ToolPresenterOptions private agentToolManager: AgentToolManager | null = null @@ -478,15 +479,18 @@ export class ToolPresenter implements IToolPresenter { for (const mapping of mapper.getAllMappings()) { this.mapper.registerTool(mapping.toolName, mapping.source, mapping.originalName) } + this.globalMapperConversationId = normalizedConversationId || null } private getToolSource(toolName: string, conversationId?: string): ToolSource | undefined { const normalizedConversationId = conversationId?.trim() if (normalizedConversationId) { const mapper = this.conversationMappers.get(normalizedConversationId) - const mappedSource = mapper?.getToolSource(toolName) - if (mappedSource) { - return mappedSource + if (mapper) { + return mapper.getToolSource(toolName) + } + if (this.globalMapperConversationId !== null) { + return undefined } } diff --git a/src/shared/agentTools.ts b/src/shared/agentTools.ts index b8c297bfb..0de94fb6b 100644 --- a/src/shared/agentTools.ts +++ b/src/shared/agentTools.ts @@ -30,5 +30,17 @@ export const getAgentToolExposure = (toolName: string): AgentToolExposure => ? AGENT_TOOL_EXPOSURE_BY_NAME[toolName as TapeToolName] : 'user-configurable' +export const assertAgentToolExposure = ( + toolName: string, + expectedExposure: AgentToolExposure +): void => { + const registeredExposure = getAgentToolExposure(toolName) + if (registeredExposure !== expectedExposure) { + throw new Error( + `Agent tool exposure mismatch for '${toolName}': expected '${expectedExposure}', registered '${registeredExposure}'.` + ) + } +} + export const isUserConfigurableAgentTool = (toolName: string): boolean => getAgentToolExposure(toolName) === 'user-configurable' diff --git a/test/main/presenter/lifecyclePresenter/startupMaintenanceHooks.test.ts b/test/main/presenter/lifecyclePresenter/startupMaintenanceHooks.test.ts index c46369067..622dffa9f 100644 --- a/test/main/presenter/lifecyclePresenter/startupMaintenanceHooks.test.ts +++ b/test/main/presenter/lifecyclePresenter/startupMaintenanceHooks.test.ts @@ -20,8 +20,7 @@ const mocks = vi.hoisted(() => { runDisabledAgentToolCleanup: vi.fn(async () => undefined), rtkStart: vi.fn(async () => undefined), sessionDataMigrationSQLite: {}, - configPresenter: {}, - appSessionService: {} + configPresenter: {} } }) @@ -31,8 +30,7 @@ vi.mock('@/presenter', () => ({ legacyChatImportService: { start: mocks.legacyStart }, usageStatsService: { startBackfill: mocks.usageStart }, sessionDataMigrationSQLite: mocks.sessionDataMigrationSQLite, - configPresenter: mocks.configPresenter, - appSessionService: mocks.appSessionService + configPresenter: mocks.configPresenter } })) @@ -127,16 +125,14 @@ describe('startup maintenance hooks', () => { expect(mocks.runMainline).toHaveBeenCalledWith( { sqlitePresenter: mocks.sessionDataMigrationSQLite, - configPresenter: mocks.configPresenter, - appSessionService: mocks.appSessionService + configPresenter: mocks.configPresenter }, mocks.taskContext ) expect(mocks.runDisabledAgentToolCleanup).toHaveBeenCalledWith( { sqlitePresenter: mocks.sessionDataMigrationSQLite, - configPresenter: mocks.configPresenter, - appSessionService: mocks.appSessionService + configPresenter: mocks.configPresenter }, mocks.taskContext ) diff --git a/test/main/presenter/startupMigrations/sessionDataMigrations.sqlite.test.ts b/test/main/presenter/startupMigrations/sessionDataMigrations.sqlite.test.ts new file mode 100644 index 000000000..8fa90a98e --- /dev/null +++ b/test/main/presenter/startupMigrations/sessionDataMigrations.sqlite.test.ts @@ -0,0 +1,171 @@ +import { describe, expect, it, vi } from 'vitest' +import { + runDisabledAgentToolCapabilityCleanupMigration, + type SessionDataMigrationSQLitePort +} from '@/presenter/startupMigrations/sessionDataMigrations' +import { TAPE_TOOL_NAMES } from '@shared/agentTools' + +const sqliteModule = await import('better-sqlite3-multiple-ciphers').catch(() => null) +const sessionsModule = sqliteModule + ? await import('@/presenter/sqlitePresenter/tables/newSessions').catch(() => null) + : null +const disabledToolsModule = sqliteModule + ? await import('@/presenter/sqlitePresenter/tables/newSessionDisabledAgentTools').catch( + () => null + ) + : null +const environmentsModule = sqliteModule + ? await import('@/presenter/sqlitePresenter/tables/newEnvironments').catch(() => null) + : null + +const Database = sqliteModule?.default +const NewSessionsTable = sessionsModule?.NewSessionsTable +const NewSessionDisabledAgentToolsTable = disabledToolsModule?.NewSessionDisabledAgentToolsTable +const NewEnvironmentsTable = environmentsModule?.NewEnvironmentsTable +const DatabaseCtor = Database! +const NewSessionsTableCtor = NewSessionsTable! +const NewSessionDisabledAgentToolsTableCtor = NewSessionDisabledAgentToolsTable! +const NewEnvironmentsTableCtor = NewEnvironmentsTable! + +let sqliteAvailable = false +if (Database) { + try { + const smokeDb = new Database(':memory:') + smokeDb.close() + sqliteAvailable = true + } catch { + sqliteAvailable = false + } +} + +const describeIfSqlite = + sqliteAvailable && NewSessionsTable && NewSessionDisabledAgentToolsTable && NewEnvironmentsTable + ? describe + : describe.skip + +describeIfSqlite('disabled Agent tool capability cleanup SQLite integration', () => { + it('preserves session and environment recency while replacing both disabled-tool stores', async () => { + const db = new DatabaseCtor(':memory:') + try { + db.exec(` + CREATE TABLE acp_sessions ( + conversation_id TEXT NOT NULL, + workdir TEXT, + updated_at INTEGER NOT NULL + ); + `) + + const sessions = new NewSessionsTableCtor(db) + const disabledTools = new NewSessionDisabledAgentToolsTableCtor(db) + const environments = new NewEnvironmentsTableCtor(db) + sessions.createTable() + disabledTools.createTable() + environments.createTable() + + sessions.create('older', 'deepchat', 'Older', '/work/app', { + disabledAgentTools: [TAPE_TOOL_NAMES.search, 'read'], + createdAt: 50, + updatedAt: 100 + }) + sessions.create('newer', 'deepchat', 'Newer', '/work/app', { + disabledAgentTools: ['exec'], + createdAt: 150, + updatedAt: 200 + }) + environments.rebuildFromSessions() + const environmentBefore = environments.list() + + const settings = new Map() + const sqlitePresenter = { + configTables: { + getAgentSetting: vi.fn((key: string) => settings.get(key)), + setAgentSetting: vi.fn((key: string, value: unknown) => settings.set(key, value)) + }, + getDatabase: vi.fn(() => db), + newSessionsTable: sessions, + newSessionDisabledAgentToolsTable: disabledTools + } as unknown as SessionDataMigrationSQLitePort + + await runDisabledAgentToolCapabilityCleanupMigration( + { + sqlitePresenter, + configPresenter: { + listAgents: vi.fn(async () => []) + } + } as never, + { + reportProgress: vi.fn(), + yield: vi.fn(async () => undefined) + } as never + ) + + expect(sessions.list().map((row) => row.id)).toEqual(['newer', 'older']) + expect(sessions.get('older')).toMatchObject({ + disabled_agent_tools: JSON.stringify(['read']), + updated_at: 100 + }) + expect(disabledTools.listBySession('older')).toEqual([ + { session_id: 'older', ordinal: 0, tool_name: 'read' } + ]) + expect(environments.list()).toEqual(environmentBefore) + expect(environmentBefore).toEqual([ + { path: '/work/app', session_count: 2, last_used_at: 200 } + ]) + } finally { + db.close() + } + }) + + it('rolls back both disabled-tool stores when normalized persistence fails', async () => { + const db = new DatabaseCtor(':memory:') + try { + const sessions = new NewSessionsTableCtor(db) + const disabledTools = new NewSessionDisabledAgentToolsTableCtor(db) + sessions.createTable() + disabledTools.createTable() + + const originalDisabledTools = [TAPE_TOOL_NAMES.search, 'read'] + sessions.create('session-1', 'deepchat', 'Session', '/work/app', { + disabledAgentTools: originalDisabledTools, + createdAt: 50, + updatedAt: 100 + }) + + const settings = new Map() + const sqlitePresenter = { + configTables: { + getAgentSetting: vi.fn((key: string) => settings.get(key)), + setAgentSetting: vi.fn((key: string, value: unknown) => settings.set(key, value)) + }, + getDatabase: vi.fn(() => db), + newSessionsTable: sessions, + newSessionDisabledAgentToolsTable: { + replaceForSession: (sessionId: string, toolNames: string[]) => { + disabledTools.replaceForSession(sessionId, toolNames) + throw new Error('normalized persistence failed') + } + } + } as unknown as SessionDataMigrationSQLitePort + + await expect( + runDisabledAgentToolCapabilityCleanupMigration({ + sqlitePresenter, + configPresenter: { + listAgents: vi.fn(async () => []) + } + } as never) + ).rejects.toThrow('normalized persistence failed') + + expect(sessions.get('session-1')).toMatchObject({ + disabled_agent_tools: JSON.stringify(originalDisabledTools), + updated_at: 100 + }) + expect(disabledTools.listBySession('session-1')).toEqual([ + { session_id: 'session-1', ordinal: 0, tool_name: TAPE_TOOL_NAMES.search }, + { session_id: 'session-1', ordinal: 1, tool_name: 'read' } + ]) + } finally { + db.close() + } + }) +}) diff --git a/test/main/presenter/startupMigrations/sessionDataMigrations.test.ts b/test/main/presenter/startupMigrations/sessionDataMigrations.test.ts index 7c1a9db18..a9470621f 100644 --- a/test/main/presenter/startupMigrations/sessionDataMigrations.test.ts +++ b/test/main/presenter/startupMigrations/sessionDataMigrations.test.ts @@ -14,34 +14,52 @@ function createFixture() { const sessionRows: Array<{ id: string }> = [] const sessionDisabledTools = new Map() const statements: string[] = [] + const updateSessionDisabledTools = vi.fn((serialized: string, sessionId: string) => ({ + changes: sessionRows.some((row) => row.id === sessionId) ? 1 : 0, + serialized + })) + const replaceSessionDisabledTools = vi.fn((sessionId: string, disabledAgentTools: string[]) => { + sessionDisabledTools.set(sessionId, disabledAgentTools) + }) + const database = { + prepare: vi.fn((sql: string) => { + statements.push(sql) + return { + all: vi.fn((...params: unknown[]) => { + if (!sql.includes('FROM new_sessions') || !sql.includes('ORDER BY id ASC')) return [] + const cursor = sql.includes('WHERE id > ?') ? String(params[0]) : null + const limit = Number(params.at(-1)) + return sessionRows + .filter((row) => cursor === null || row.id > cursor) + .sort((left, right) => left.id.localeCompare(right.id)) + .slice(0, limit) + }), + run: sql.startsWith('UPDATE new_sessions SET disabled_agent_tools') + ? updateSessionDisabledTools + : vi.fn() + } + }), + transaction: vi.fn( + (callback: (...args: any[]) => unknown) => + (...args: any[]) => + callback(...args) + ) + } const sqlitePresenter = { configTables: { getAgentSetting: vi.fn((key: string) => settings.get(key)), setAgentSetting: vi.fn((key: string, value: unknown) => settings.set(key, value)) }, - getDatabase: vi.fn(() => ({ - prepare: vi.fn((sql: string) => { - statements.push(sql) - return { - all: vi.fn((...params: unknown[]) => { - if (!sql.includes('FROM new_sessions') || !sql.includes('ORDER BY id ASC')) return [] - const cursor = sql.includes('WHERE id > ?') ? String(params[0]) : null - const limit = Number(params.at(-1)) - return sessionRows - .filter((row) => cursor === null || row.id > cursor) - .sort((left, right) => left.id.localeCompare(right.id)) - .slice(0, limit) - }) - } - }) - })), + getDatabase: vi.fn(() => database), newSessionsTable: { get: vi.fn(() => null), getActiveSkills: vi.fn(() => []), getDisabledAgentTools: vi.fn((sessionId: string) => sessionDisabledTools.get(sessionId) ?? []) }, newSessionActiveSkillsTable: { replaceForSession: vi.fn() }, - newSessionDisabledAgentToolsTable: { replaceForSession: vi.fn() }, + newSessionDisabledAgentToolsTable: { + replaceForSession: replaceSessionDisabledTools + }, deepchatSearchDocumentsTable: { upsert: vi.fn() }, deepchatUserMessagesTable: { upsert: vi.fn() }, deepchatUserMessageFilesTable: { replaceForMessage: vi.fn() }, @@ -53,11 +71,6 @@ function createFixture() { getDeepChatAgentConfig: vi.fn(), updateDeepChatAgent: vi.fn() } - const appSessionService = { - updateDisabledAgentTools: vi.fn((sessionId: string, disabledAgentTools: string[]) => { - sessionDisabledTools.set(sessionId, disabledAgentTools) - }) - } const taskContext = { reportProgress: vi.fn(), yield: vi.fn(async () => undefined) @@ -70,7 +83,8 @@ function createFixture() { statements, sqlitePresenter, configPresenter, - appSessionService, + updateSessionDisabledTools, + replaceSessionDisabledTools, taskContext } } @@ -150,11 +164,15 @@ describe('session data migrations', () => { fixture.taskContext as never ) - expect(fixture.appSessionService.updateDisabledAgentTools).toHaveBeenCalledWith('session-1', [ + expect(fixture.replaceSessionDisabledTools).toHaveBeenCalledWith('session-1', [ 'cdp_send', 'custom_tool', 'exec' ]) + expect(fixture.updateSessionDisabledTools).toHaveBeenCalledWith( + JSON.stringify(['cdp_send', 'custom_tool', 'exec']), + 'session-1' + ) expect(fixture.configPresenter.updateDeepChatAgent).toHaveBeenCalledWith('deepchat', { config: { disabledAgentTools: ['exec'] } }) @@ -174,6 +192,11 @@ describe('session data migrations', () => { expect.stringMatching(/WHERE id > \?\s+ORDER BY id ASC\s+LIMIT \?/) ]) ) + const sessionUpdate = fixture.statements.find((sql) => + sql.startsWith('UPDATE new_sessions SET disabled_agent_tools') + ) + expect(sessionUpdate).toBe('UPDATE new_sessions SET disabled_agent_tools = ? WHERE id = ?') + expect(sessionUpdate).not.toContain('updated_at') }) it('uses bounded session batches and yields between them', async () => { @@ -189,7 +212,7 @@ describe('session data migrations', () => { fixture.taskContext as never ) - expect(fixture.appSessionService.updateDisabledAgentTools).toHaveBeenCalledTimes(125) + expect(fixture.replaceSessionDisabledTools).toHaveBeenCalledTimes(125) expect(fixture.taskContext.yield).toHaveBeenCalledTimes(3) expect(fixture.settings.get(DISABLED_AGENT_TOOL_CAPABILITY_CLEANUP_KEY)).toMatchObject({ status: 'completed', @@ -210,7 +233,7 @@ describe('session data migrations', () => { fixture.sessionDisabledTools.set('session-1', [TAPE_TOOL_NAMES.search, 'read']) fixture.sessionDisabledTools.set('session-2', [TAPE_TOOL_NAMES.handoff, 'exec']) let updateCount = 0 - fixture.appSessionService.updateDisabledAgentTools.mockImplementation( + fixture.replaceSessionDisabledTools.mockImplementation( (sessionId: string, disabledAgentTools: string[]) => { updateCount += 1 if (updateCount === 2) throw new Error('write interrupted') @@ -230,7 +253,7 @@ describe('session data migrations', () => { error: 'write interrupted' }) - fixture.appSessionService.updateDisabledAgentTools.mockImplementation( + fixture.replaceSessionDisabledTools.mockImplementation( (sessionId: string, disabledAgentTools: string[]) => { fixture.sessionDisabledTools.set(sessionId, disabledAgentTools) } diff --git a/test/main/presenter/toolPresenter/agentTools/agentTapeTools.test.ts b/test/main/presenter/toolPresenter/agentTools/agentTapeTools.test.ts index 394fee2a1..b44a74a46 100644 --- a/test/main/presenter/toolPresenter/agentTools/agentTapeTools.test.ts +++ b/test/main/presenter/toolPresenter/agentTools/agentTapeTools.test.ts @@ -185,13 +185,12 @@ describe('Agent tape tools', () => { }) it('does not expose tape tools outside DeepChat sessions', async () => { - const manager = buildManager( - buildRuntimePort({ - resolveConversationSessionInfo: vi.fn().mockResolvedValue({ - agentType: 'acp' - }) + const runtimePort = buildRuntimePort({ + resolveConversationSessionInfo: vi.fn().mockResolvedValue({ + agentType: 'acp' }) - ) + }) + const manager = buildManager(runtimePort) const defs = await manager.getAllToolDefinitions({ chatMode: 'agent', @@ -201,8 +200,30 @@ describe('Agent tape tools', () => { }) expect(defs.some((def) => def.server.name === 'agent-tape')).toBe(false) + await expect( + manager.callTool(TAPE_TOOL_NAMES.search, { query: 'needle' }, 'conv-1') + ).rejects.toThrow('Tape recall tools are not available for this conversation.') + expect(runtimePort.searchTape).not.toHaveBeenCalled() }) + it.each([ + [TAPE_TOOL_NAMES.search, { query: 'needle' }, 'getTapeContext'], + [TAPE_TOOL_NAMES.context, { entryIds: [1] }, 'searchTape'] + ] as const)( + 'rejects direct %s execution when the recall pair is incomplete', + async (toolName, args, missingPort) => { + const runtimePort = buildRuntimePort({ [missingPort]: undefined }) + const manager = buildManager(runtimePort) + + await expect(manager.callTool(toolName, args, 'conv-1')).rejects.toThrow( + 'Tape recall tools are not available for this conversation.' + ) + const targetPort = + toolName === TAPE_TOOL_NAMES.search ? runtimePort.searchTape : runtimePort.getTapeContext + expect(targetPort).not.toHaveBeenCalled() + } + ) + it('does not expose recall tools without a conversation ID', async () => { const runtimePort = buildRuntimePort() const handler = new AgentTapeToolHandler(runtimePort) diff --git a/test/main/presenter/toolPresenter/toolPresenter.test.ts b/test/main/presenter/toolPresenter/toolPresenter.test.ts index a60a3c69e..4ee8057ff 100644 --- a/test/main/presenter/toolPresenter/toolPresenter.test.ts +++ b/test/main/presenter/toolPresenter/toolPresenter.test.ts @@ -8,8 +8,13 @@ import { UPDATE_PLAN_TOOL_NAME } from '@/presenter/toolPresenter/agentTools' import { CommandPermissionService } from '@/presenter/permission' +import { QUESTION_TOOL_NAME } from '@/presenter/toolPresenter/agentTools/questionTool' import { IMAGE_GENERATE_TOOL_NAME } from '@shared/agentImageGenerationTool' -import { CRON_JOB_AGENT_TOOL_NAME, getAgentToolExposure } from '@shared/agentTools' +import { + CRON_JOB_AGENT_TOOL_NAME, + assertAgentToolExposure, + getAgentToolExposure +} from '@shared/agentTools' vi.mock('electron', () => ({ app: { @@ -480,6 +485,9 @@ describe('ToolPresenter', () => { expect(getAgentToolExposure(TAPE_TOOL_NAMES.handoff)).toBe('runtime-only') expect(getAgentToolExposure('read')).toBe('user-configurable') expect(getAgentToolExposure('__proto__')).toBe('user-configurable') + expect(() => assertAgentToolExposure(TAPE_TOOL_NAMES.handoff, 'user-configurable')).toThrow( + "Agent tool exposure mismatch for 'tape_handoff': expected 'user-configurable', registered 'runtime-only'." + ) }) it('keeps the recall pair in the runtime catalog despite stale disabled values', async () => { @@ -582,6 +590,114 @@ describe('ToolPresenter', () => { ) }) + it('does not fall back to another conversation mapping when a tool is unavailable', async () => { + const toolPresenter = new ToolPresenter({ + mcpPresenter: { + getAllToolDefinitions: vi.fn().mockResolvedValue([]) + } as any, + configPresenter: { + getSkillsEnabled: vi.fn().mockReturnValue(false), + getSkillsPath: vi.fn().mockReturnValue('C:\\skills'), + getModelConfig: vi.fn() + } as any, + commandPermissionHandler: new CommandPermissionService(), + agentToolRuntime: buildAgentToolRuntimeMock() + }) + + await toolPresenter.getAllToolDefinitions({ + chatMode: 'agent', + supportsVision: false, + agentWorkspacePath: 'C:\\workspace-b', + conversationId: 'conv-b', + disabledAgentTools: [QUESTION_TOOL_NAME] + }) + await toolPresenter.getAllToolDefinitions({ + chatMode: 'agent', + supportsVision: false, + agentWorkspacePath: 'C:\\workspace-a', + conversationId: 'conv-a' + }) + + await expect( + toolPresenter.callTool({ + id: 'tool-b', + type: 'function', + function: { + name: QUESTION_TOOL_NAME, + arguments: JSON.stringify({ question: 'Should not execute', options: [] }) + }, + conversationId: 'conv-b' + }) + ).rejects.toThrow(`Tool ${QUESTION_TOOL_NAME} not found in any source`) + + await expect( + toolPresenter.callTool({ + id: 'tool-unknown', + type: 'function', + function: { + name: QUESTION_TOOL_NAME, + arguments: JSON.stringify({ question: 'Unknown conversation', options: [] }) + }, + conversationId: 'conv-unknown' + }) + ).rejects.toThrow(`Tool ${QUESTION_TOOL_NAME} not found in any source`) + + toolPresenter.clearConversationToolMapping('conv-b') + await expect( + toolPresenter.callTool({ + id: 'tool-b-cleared', + type: 'function', + function: { + name: QUESTION_TOOL_NAME, + arguments: JSON.stringify({ question: 'Still should not execute', options: [] }) + }, + conversationId: 'conv-b' + }) + ).rejects.toThrow(`Tool ${QUESTION_TOOL_NAME} not found in any source`) + }) + + it('keeps a draft-origin mapping available for the first persisted turn', async () => { + const toolPresenter = new ToolPresenter({ + mcpPresenter: { + getAllToolDefinitions: vi.fn().mockResolvedValue([]) + } as any, + configPresenter: { + getSkillsEnabled: vi.fn().mockReturnValue(false), + getSkillsPath: vi.fn().mockReturnValue('C:\\skills'), + getModelConfig: vi.fn() + } as any, + commandPermissionHandler: new CommandPermissionService(), + agentToolRuntime: buildAgentToolRuntimeMock() + }) + + await toolPresenter.getAllToolDefinitions({ + chatMode: 'agent', + supportsVision: false, + agentWorkspacePath: 'C:\\workspace' + }) + const callTool = vi.fn().mockResolvedValue('draft-result') + const agentToolManager = (toolPresenter as any).agentToolManager + agentToolManager.callTool = callTool + + await expect( + toolPresenter.callTool({ + id: 'draft-tool', + type: 'function', + function: { + name: QUESTION_TOOL_NAME, + arguments: JSON.stringify({ question: 'First turn', options: [] }) + }, + conversationId: 'persisted-session' + }) + ).resolves.toMatchObject({ content: 'draft-result' }) + expect(callTool).toHaveBeenCalledWith( + QUESTION_TOOL_NAME, + { question: 'First turn', options: [] }, + 'persisted-session', + expect.objectContaining({ toolCallId: 'draft-tool' }) + ) + }) + it('exposes cronjob only when runtime ports are available and the tool is enabled', async () => { const toolPresenter = new ToolPresenter({ mcpPresenter: {