diff --git a/apps/server/src/features/mcode-mcp/__tests__/browser-toolkit.test.ts b/apps/server/src/features/mcode-mcp/__tests__/browser-toolkit.test.ts index 01cfb536c..0402af287 100644 --- a/apps/server/src/features/mcode-mcp/__tests__/browser-toolkit.test.ts +++ b/apps/server/src/features/mcode-mcp/__tests__/browser-toolkit.test.ts @@ -288,6 +288,14 @@ describe("BrowserToolkit", () => { expect(tools.map((tool) => tool.name).join(" ")).not.toMatch(/browser_(status|navigate|snapshot|click|type)/); }); + // The runtime Browser section carries only routing rules, and lazy tool loading may never load browser_tabs. + it("tells the model to finalize tabs from the tools every Browser flow loads", () => { + const tools = setup().toolkit.bind(session)?.tools ?? []; + for (const name of ["browser_open", "browser_inspect"]) { + expect(tools.find((tool) => tool.name === name)?.description).toMatch(/call browser_tabs finalize before replying/i); + } + }); + it("never asks the model for observation, control, idempotency, deadline, or batch fields", () => { const tools = setup().toolkit.bind(session)?.tools ?? []; diff --git a/apps/server/src/features/mcode-mcp/toolkits/browser-tool-schemas.ts b/apps/server/src/features/mcode-mcp/toolkits/browser-tool-schemas.ts index bf6d8e6cf..3e29b8f68 100644 --- a/apps/server/src/features/mcode-mcp/toolkits/browser-tool-schemas.ts +++ b/apps/server/src/features/mcode-mcp/toolkits/browser-tool-schemas.ts @@ -110,10 +110,10 @@ export function browserInputSchema(operation: BrowserAutomationPublicOperation): const descriptionByOperation: Record = { open: [ - "Use only the mcode MCP server for Mcode's shared Preview. Never initialize or use bundled generic Browser/Chrome/Node-REPL, or launch another browser or profile.", + "Prefer the mcode MCP server for Mcode's shared Preview. After one failure, follow its recovery instruction here instead of using bundled generic Browser, Chrome, Playwright, Node-REPL, or another browser or profile. Switch only after two failures on the same step or when the user asks, and quote the raw error.", "For Browser-only tasks, do not run shell/terminal commands or read local skills/files; keep narration minimal.", "Create a sticky, agent-owned background tab without revealing or focusing the Browser panel.", - "Returns tab metadata, not a page snapshot: call browser_inspect before targeting elements. Use browser_tabs to select, claim, release, close, or finalize tabs.", + "Returns tab metadata, not a page snapshot: call browser_inspect before targeting elements. Use browser_tabs to select, claim, release, or close tabs, and call browser_tabs finalize before replying.", ].join(" "), inspect: [ "Authoritative for Browser readiness, tabs, capabilities, constraints, diagnostics, and recovery guidance.", @@ -122,7 +122,7 @@ const descriptionByOperation: Record = "Request includeScreenshot only for visual evidence or when the user asks. Verify every success claim against current page evidence.", "tabs lists every Browser tab in this thread, including tabs the user opened, with who holds each. Pass a tabId to read that tab, even while the user holds it; reading never takes control.", "browser_act targets the current tab, not one read by tabId, and refuses refs from another tab. To act on a tab read by tabId, browser_tabs select it while nobody holds the Browser, then inspect again.", - "Use only mcode MCP browser tools; Browser-only tasks need no shell commands or local skills/files.", + "Browser-only tasks need no shell commands or local skills/files. Call browser_tabs finalize before replying.", ].join(" "), act: [ "Perform one Browser action per call. Target elements only with a ref from the latest browser_inspect snapshot; never guess an ambiguous target.", @@ -144,7 +144,7 @@ const descriptionByOperation: Record = "Use browser_evaluate for privileged open-world page evaluation only when the live capability advertises it.", "All browser_act and browser_inspect rules apply, including readiness, interruption, receipts, effects, recovery, and current evidence.", "An evaluation ends the snapshot: call browser_inspect before the next browser_act.", - "Use only mcode MCP browser tools, never another browser or profile. Browser-only tasks need no shell commands or local skills/files.", + "Browser-only tasks need no shell commands or local skills/files.", ].join(" "), }; diff --git a/apps/server/src/features/providers/composition/__tests__/mcode-instruction-boundaries.test.ts b/apps/server/src/features/providers/composition/__tests__/mcode-instruction-boundaries.test.ts index 8a35847eb..c5d86bcbc 100644 --- a/apps/server/src/features/providers/composition/__tests__/mcode-instruction-boundaries.test.ts +++ b/apps/server/src/features/providers/composition/__tests__/mcode-instruction-boundaries.test.ts @@ -17,7 +17,7 @@ describe("provider-native Mcode instruction boundaries", () => { })); const first = appendCursorMcodeInstructions("user rules", runtime, false); expect(first).toEqual({ instructionMarkdown: `user rules\n\n${runtime}`, included: true }); - expect(first.instructionMarkdown).toContain("use only the mcode MCP browser_* tools"); + expect(first.instructionMarkdown).toContain("Prefer the mcode MCP browser_* tools"); const unacceptedRetry = appendCursorMcodeInstructions(first.instructionMarkdown, runtime, false); expect(unacceptedRetry).toEqual(first); diff --git a/docs/internals/providers/mcode-agent-instructions.md b/docs/internals/providers/mcode-agent-instructions.md index 00d3e4d69..4b494170d 100644 --- a/docs/internals/providers/mcode-agent-instructions.md +++ b/docs/internals/providers/mcode-agent-instructions.md @@ -18,9 +18,17 @@ Claude joins sections in its `systemPrompt` append, Copilot in `systemMessage`, and Cursor in the first normal prompt of its logical ACP session. Ephemeral Codex completions receive no runtime sections. -The thread-control and Browser sections carry their full operating rules, and -the descriptions of the tools they govern repeat them. The repetition is -deliberate. A harness that loads tools lazily shows only tool names until the -model searches, so a rule that lives only in a description arrives after the +A harness that loads tools lazily shows only tool names until the model +searches, so a rule that lives only in a tool description arrives after the model has chosen a tool. MCP initialization carries no instructions, and there are no guide tools. + +The thread-control section therefore carries its full operating rules, and the +tool descriptions repeat them. The Browser section carries only routing rules: +which browser to choose, how to confirm it is unavailable, and when to switch +after failures. Those decisions happen before any Browser tool loads. The +Browser tool descriptions own the operating rules, and the rules every Browser +flow needs, such as finalizing tabs before replying, appear on `browser_open` +and `browser_inspect` because a flow may never load `browser_tabs`. +`browser_open` also repeats the failure and switch rule, so edit both places +when that rule changes. diff --git a/packages/providers/src/__tests__/claude-factory.test.ts b/packages/providers/src/__tests__/claude-factory.test.ts index 8144b848c..d447b05a1 100644 --- a/packages/providers/src/__tests__/claude-factory.test.ts +++ b/packages/providers/src/__tests__/claude-factory.test.ts @@ -649,7 +649,7 @@ describe("Claude public factory core and capabilities", () => { assert(options); expect(options.mcpServers).toEqual({ mcode: { type: "http", url: "http://127.0.0.1:19400/mcp/mcode", headers: { Authorization: "Bearer fixture-token" } } }); expect(options.settings).toMatchObject({ ultracode: true }); - expect(options.systemPrompt).toMatchObject({ append: expect.stringContaining("browser_tabs") }); + expect(options.systemPrompt).toMatchObject({ append: expect.stringContaining("browser_inspect") }); assert(typeof options.systemPrompt === "object" && !Array.isArray(options.systemPrompt)); const append = options.systemPrompt.append ?? ""; expect(append).toContain("thread_target_list"); @@ -695,7 +695,7 @@ describe("Claude public factory core and capabilities", () => { assert(typeof prompt === "object" && !Array.isArray(prompt)); const append = prompt.append ?? ""; expect(append.includes("thread_target_list")).toBe(expected.thread); - expect(append.includes("browser_tabs")).toBe(expected.browser); + expect(append.includes("browser_inspect")).toBe(expected.browser); }); it("emits no Mcode server and grants nothing when the server declines to attach", async () => { @@ -707,7 +707,7 @@ describe("Claude public factory core and capabilities", () => { expect(options?.mcpServers).toEqual({}); assert(typeof options?.systemPrompt === "object" && !Array.isArray(options.systemPrompt)); expect(options.systemPrompt.append ?? "").not.toContain("thread_target_list"); - expect(options.systemPrompt.append ?? "").not.toContain("browser_tabs"); + expect(options.systemPrompt.append ?? "").not.toContain("browser_inspect"); }); it("attaches under the session id the Browser lease is staged for, not the resume or thread id", async () => { diff --git a/packages/providers/src/__tests__/codex/codex-provider-first-turn.test.ts b/packages/providers/src/__tests__/codex/codex-provider-first-turn.test.ts index 4cdcf5298..e712f2340 100644 --- a/packages/providers/src/__tests__/codex/codex-provider-first-turn.test.ts +++ b/packages/providers/src/__tests__/codex/codex-provider-first-turn.test.ts @@ -463,7 +463,7 @@ describe("CodexProvider first turn on new session", () => { "mcode_identity", "mcode_provider_subagents", "mcode_thread_control", "mcode_browser", "mcode_repository_search", ]); expect(server.options.instructionSections?.find((section) => section.key === "mcode_browser")?.text) - .toContain("use only the mcode MCP browser_* tools"); + .toContain("Prefer the mcode MCP browser_* tools"); expect(server.options.instructionSections?.find((section) => section.key === "mcode_thread_control")?.text) .toContain("thread_target_list"); expect(server.spawnedEnv?.MCODE_MCP_TOKEN).toBe("session-token-fixture"); diff --git a/packages/providers/src/private/claude/__tests__/claude-browser-lease.test.ts b/packages/providers/src/private/claude/__tests__/claude-browser-lease.test.ts index bdac04db1..256f7681c 100644 --- a/packages/providers/src/private/claude/__tests__/claude-browser-lease.test.ts +++ b/packages/providers/src/private/claude/__tests__/claude-browser-lease.test.ts @@ -128,8 +128,7 @@ describe("ClaudeProvider browser session lease lifecycle", () => { await (harness.provider as any).sendTurn(request()); const append = mockQuery.mock.calls[0][0].options.systemPrompt.append; - expect(append).toContain("use only the mcode MCP browser_* tools"); - expect(append).toContain("call browser_tabs finalize before replying"); + expect(append).toContain("Prefer the mcode MCP browser_* tools"); await (harness.provider as any).close(harness.spawnedState); }); diff --git a/packages/providers/src/private/copilot/__tests__/copilot-factory.test.ts b/packages/providers/src/private/copilot/__tests__/copilot-factory.test.ts index 2d0c1aac1..cfc1b26d7 100644 --- a/packages/providers/src/private/copilot/__tests__/copilot-factory.test.ts +++ b/packages/providers/src/private/copilot/__tests__/copilot-factory.test.ts @@ -916,7 +916,7 @@ describe("Copilot public factory", () => { await provider.sendTurn(request()); await finish(); const content: string = sdk.create.mock.calls[0]?.[0].systemMessage.content; expect(content.includes("thread_target_list")).toBe(expected.thread); - expect(content.includes("use only the mcode MCP browser_* tools")).toBe(expected.browser); + expect(content.includes("Prefer the mcode MCP browser_* tools")).toBe(expected.browser); await provider.stopSession("mcode-thread-1"); }); @@ -926,7 +926,7 @@ describe("Copilot public factory", () => { const content = options.systemMessage && "content" in options.systemMessage ? options.systemMessage.content ?? "" : ""; expect(options.mcpServers).toEqual({}); expect(content).not.toContain("thread_target_list"); - expect(content).not.toContain("use only the mcode MCP browser_* tools"); + expect(content).not.toContain("Prefer the mcode MCP browser_* tools"); }); it("attaches under the session id the Browser lease is staged for", async () => { diff --git a/packages/providers/src/private/cursor/__tests__/cursor-mcp.test.ts b/packages/providers/src/private/cursor/__tests__/cursor-mcp.test.ts index 07e814958..a4b1d8685 100644 --- a/packages/providers/src/private/cursor/__tests__/cursor-mcp.test.ts +++ b/packages/providers/src/private/cursor/__tests__/cursor-mcp.test.ts @@ -8,7 +8,7 @@ import { AcpSessionRuntime } from "../../protocols/acp/acp-session-runtime.js"; import { CursorProvider } from "../cursor-provider.js"; const THREAD_TOOL = "thread_target_list"; -const BROWSER_GUIDANCE = "use only the mcode MCP browser_* tools"; +const BROWSER_GUIDANCE = "Prefer the mcode MCP browser_* tools"; function attachment(toolkits: ProviderMcodeAttachment["toolkits"] = { browser: true, threadControl: true }): ProviderMcodeAttachment { return { serverName: "mcode", url: "http://127.0.0.1:19400/mcp/mcode", token: "mcode-token", toolkits }; diff --git a/packages/thread-orchestration/src/__tests__/mcode-instructions.test.ts b/packages/thread-orchestration/src/__tests__/mcode-instructions.test.ts index cdf12eb11..d3f4e2eab 100644 --- a/packages/thread-orchestration/src/__tests__/mcode-instructions.test.ts +++ b/packages/thread-orchestration/src/__tests__/mcode-instructions.test.ts @@ -53,6 +53,15 @@ describe("Mcode runtime instruction sections", () => { ); } + it("keeps the Browser section to routing rules without call shapes", () => { + const browser = buildMcodeInstructionSections({ toolkits: { threadControl: false, browser: true } }) + .find((section) => section.key === "mcode_browser")?.text ?? ""; + expect(browser.split(/\s+/).filter(Boolean).length).toBeLessThan(200); + expect(browser).not.toMatch(/[{}`]|\bs\d+e\d+\b/); + expect(browser).toContain("Prefer the mcode MCP browser_* tools"); + expect(browser).toContain("Switch only after two failures on the same step"); + }); + it("trims nested models and omits a blank model", () => { const toolkits = { threadControl: false, browser: false }; const sections = buildMcodeInstructionSections({ toolkits, nestedDelegationModel: " gpt-5.6-sol " }); diff --git a/packages/thread-orchestration/src/mcode-instructions.ts b/packages/thread-orchestration/src/mcode-instructions.ts index b0f8c5803..57d38ae8a 100644 --- a/packages/thread-orchestration/src/mcode-instructions.ts +++ b/packages/thread-orchestration/src/mcode-instructions.ts @@ -36,7 +36,7 @@ const IDENTITY_BLOCK = [ "Mcode runtime guidance is advisory; MCP authorization and lease state remain authoritative.", ].join(" "); -// The thread-control and Browser tool descriptions repeat these rules on purpose. With deferred tool loading, a description arrives only after the model has chosen a tool. +// The thread-control tool descriptions repeat these rules on purpose. With deferred tool loading, a description arrives only after the model has chosen a tool. const THREAD_CONTROL_BLOCK = [ "Mcode thread control", "- The mcode MCP tools manage Mcode threads: persistent, user-visible conversations. Use delegate_task when the user asks to hand work to another provider or model, even if they never say \"Mcode thread\".", @@ -71,20 +71,15 @@ function nestedDelegationBlock(model: string | undefined): string | undefined { ].join(" "); } +// Routing rules only: they decide which browser the model picks, which happens before any Browser tool description is loaded. Operating rules live in the tool descriptions. const BROWSER_BLOCK = [ "Mcode Browser", - "- Boundary: for visible Browser or Mcode Preview requests, use only the mcode MCP browser_* tools. Never use bundled generic Browser, Chrome, Playwright or Node-REPL, and never launch another browser or profile.", - "- Browser-only tasks need no shell commands or local skill files. Keep narration minimal.", - "- Open: browser_open creates a sticky, agent-owned background tab and returns tab metadata, not a page observation.", - "- Inspect: browser_inspect is authoritative for readiness, tabs, capabilities, constraints, diagnostics and element references. Inspect before the first action. Tool discovery does not prove readiness.", - "- Target: browser_act performs one action per call on an element reference such as s3e12 from the latest inspection. Each inspection replaces earlier references. Never guess an ambiguous target.", - "- Verify: stop on failure, interruption or invalidation. Verify non-navigation effects with an assert action or a fresh inspection. Navigation and reload end the snapshot, so inspect again before another action.", - "- Interruptions: user input invalidates the observation. Stop or Take control interrupts execution, and Take control holds the Browser until Hand back. Release held input, keep completed effects and yield to the user.", - "- Recover: inspect the receipt and effect before retrying. Effects do not roll back, so never replay mutations automatically. Follow exactly one recovery instruction: inspect, reopen, wait, yield_to_user or do_not_retry.", - "- Evidence: request includeScreenshot only for visual evidence or when the user asks. Verify every success claim against current page evidence.", - "- Finish: call browser_tabs finalize before replying. Release claimed user tabs and close agent tabs by default. For a tab the user wants open, set disposition handoff and verify success.", - "- Evaluate: use browser_evaluate only when the live capability advertises it. All rules above apply.", - "- If these tools are missing from an initial tool listing, they may load lazily. Make one direct attempt before reporting them unavailable.", + "- Prefer the mcode MCP browser_* tools for any browser work. They drive the Browser the user sees.", + "- Before concluding the Browser is unavailable, check its status with browser_inspect and open a tab with browser_open. If the tools are missing from an initial tool listing, they may load lazily; make one direct attempt.", + "- When the user means the page they have open, read their tab instead of opening a new one.", + "- After a first failure, follow the error's recovery instruction in Mcode's Browser. Do not switch to another browser, Chrome, Playwright or Node-REPL.", + "- Switch only after two failures on the same step, or when the user asks. Quote the raw error when you switch.", + "- A user hold, Stop or do_not_retry is not a failure to work around: yield to the user instead of switching.", ].join("\n"); const SECTION_REGISTRY: readonly {