From 4d6889d8106f2618dcc38254bea8eb2c55010835 Mon Sep 17 00:00:00 2001 From: Natalia Venditto Date: Thu, 20 Aug 2026 09:33:22 +0200 Subject: [PATCH] fix(tools): unregister enter_plan_mode/exit_plan_mode (complete #73) PR #73 removed the plan-mode *prompt* guidance, but left both tools registered with self-instructing descriptions ('Call this before ... 2+ steps'), so the model still entered plan mode from the schema alone and exit_plan_mode surfaced a generic approval card the client can't render as a plan. Stop registering both tools while plan mode is disabled. Both execute() were no-ops and nothing else references them, so it's self-contained. Restore with the prompt block when da-nx#658 lands. tsc clean; full suite green. Co-Authored-By: Claude Opus 4.8 --- src/tools/tools.ts | 41 +++++++----------------------------- test/eds-tools.test.ts | 48 ++++++++++++++++++++++++++++++------------ 2 files changed, 42 insertions(+), 47 deletions(-) diff --git a/src/tools/tools.ts b/src/tools/tools.ts index 7d481e2..b51e068 100644 --- a/src/tools/tools.ts +++ b/src/tools/tools.ts @@ -574,39 +574,14 @@ export function createDATools( }); // Planning bracket — mirrors AO's enter_plan_mode / exit_plan_mode built-in tools. - // enter_plan_mode: signals start of planning phase; no approval, no side effects. - tools.enter_plan_mode = tool({ - description: - 'Signal the start of a planning phase. Call this before reasoning about what steps to take ' + - 'for any operation involving 2 or more distinct steps or tool calls. ' + - 'No action is taken — this is a signal only. Follow it by calling exit_plan_mode with the full plan.', - inputSchema: z.object({}), - needsApproval: async () => false, - execute: async () => ({ planning: true }), - }); - - // exit_plan_mode: submits the plan for user review; requires approval before execution proceeds. - tools.exit_plan_mode = tool({ - description: - 'Submit the completed plan for the user to review before any actions are taken. ' + - 'Call this after enter_plan_mode, once you have determined all the steps. ' + - 'The user will see the plan card and click Run to approve execution. ' + - 'Use the same task labels later in :::task-item directives to report progress.', - inputSchema: z.object({ - title: z.string().describe('Short plan title (≤ 8 words)'), - description: z.string().optional().describe('One-line summary of what you are about to do'), - tasks: z - .array( - z.object({ - id: z.string().describe('Unique step identifier, e.g. "1", "2"'), - label: z.string().describe('Human-readable step description'), - }), - ) - .describe('Ordered list of steps to execute'), - }), - needsApproval: async () => true, - execute: async () => ({ approved: true }), - }); + // + // HOTFIX(da-nx#658): plan mode is disabled until the da-nx client can render + // plan/tasks. PR #73 removed the prompt guidance, but the model still called + // these tools from their schema descriptions alone — entering plan mode and then + // showing a generic approval card for exit_plan_mode (which the client can't render + // as a plan). So do NOT register enter_plan_mode / exit_plan_mode at all while + // disabled. Both execute() were no-ops and nothing else references them, so this is + // self-contained. Restore this block (see git history / PR #73) once #658 lands. // Memory tools write to internal agent metadata paths — no user approval needed. tools.write_project_memory = tool({ diff --git a/test/eds-tools.test.ts b/test/eds-tools.test.ts index 83c4739..397b9e5 100644 --- a/test/eds-tools.test.ts +++ b/test/eds-tools.test.ts @@ -5,21 +5,17 @@ import { createDATools, createEDSTools } from '../src/tools/tools'; // Minimal mock for EDSAdminClient function makeEdsClient(overrides: Partial = {}): EDSAdminClient { return { - preview: vi - .fn() - .mockResolvedValue({ - status: 200, - path: '/docs/index', - url: 'https://main--repo--org.hlx.page/docs/index', - }), + preview: vi.fn().mockResolvedValue({ + status: 200, + path: '/docs/index', + url: 'https://main--repo--org.hlx.page/docs/index', + }), unpreview: vi.fn().mockResolvedValue({ status: 200, path: '/docs/index' }), - publishLive: vi - .fn() - .mockResolvedValue({ - status: 200, - path: '/docs/index', - url: 'https://main--repo--org.hlx.live/docs/index', - }), + publishLive: vi.fn().mockResolvedValue({ + status: 200, + path: '/docs/index', + url: 'https://main--repo--org.hlx.live/docs/index', + }), unpublishLive: vi.fn().mockResolvedValue({ status: 200, path: '/docs/index' }), ...overrides, } as unknown as EDSAdminClient; @@ -204,6 +200,30 @@ describe('DA tools still registered when client provided', () => { const tools = createDATools(null, {}); expect(tools).not.toHaveProperty('content_list'); }); + + // HOTFIX(da-nx#658): plan mode is disabled until the client renders plan/tasks. + // PR #73 removed the prompt guidance; this ensures the tools themselves are not + // registered, so the model can't enter plan mode from the schema alone. Restore + // both assertions to toHaveProperty once #658 lands. + it('does not register enter_plan_mode / exit_plan_mode while plan mode is disabled', () => { + const daClient = { + listSources: vi.fn(), + getSource: vi.fn(), + createSource: vi.fn(), + updateSource: vi.fn(), + deleteSource: vi.fn(), + copyContent: vi.fn(), + moveContent: vi.fn(), + createVersion: vi.fn(), + getVersions: vi.fn(), + lookupMedia: vi.fn(), + lookupFragment: vi.fn(), + uploadMedia: vi.fn(), + } as any; + const tools = createDATools(daClient, {}); + expect(tools).not.toHaveProperty('enter_plan_mode'); + expect(tools).not.toHaveProperty('exit_plan_mode'); + }); }); describe('content_read missing-file handling', () => {