feat(advisor): Gate tools with read-only metadata

packages/junior/src/chat/respond.ts

@@ -39,10 +39,9 @@ import type { ThreadArtifactsState } from "@/chat/state/artifacts";
 import type { ConversationPendingAuthState } from "@/chat/state/conversation";
 import { createTools } from "@/chat/tools";
 import { resolveChannelCapabilities } from "@/chat/tools/channel-capabilities";
-import type { ToolDefinition } from "@/chat/tools/definition";
 import { toActiveMcpCatalogSummaries } from "@/chat/tools/skill/mcp-tool-summary";
 import type { ImageGenerateToolDeps } from "@/chat/tools/types";
-import { isAdvisorToolAllowed } from "@/chat/tools/advisor/tool";
+import { createAdvisorToolDefinitions } from "@/chat/tools/advisor/tool";
 import {
   GEN_AI_PROVIDER_NAME,
   completeObject,
@@ -898,7 +897,20 @@ export async function generateAssistantReply(
       }
     };
     const agentTools = createAgentTools(
-      tools as Record<string, ToolDefinition<any>>,
+      tools,
+      skillSandbox,
+      spanContext,
+      context.onStatus,
+      sandboxExecutor,
+      capabilityRuntime,
+      pluginAuth,
+      onToolCall,
+    );
+    advisorTools = createAgentTools(
+      createAdvisorToolDefinitions(tools, {
+        getActiveSkills: () => activeSkills,
+        mcpToolManager: turnMcpToolManager,
+      }),
       skillSandbox,
       spanContext,
       context.onStatus,
@@ -907,7 +919,6 @@ export async function generateAssistantReply(
       pluginAuth,
       onToolCall,
     );
-    advisorTools = agentTools.filter((tool) => isAdvisorToolAllowed(tool.name));
     // Keep Pi's native tool schema static for the whole turn. Ideally this
     // would use provider-native tool loading/search APIs, but Pi's generic
     // AgentTool surface cannot yet express OpenAI/Anthropic deferred MCP tools.

packages/junior/src/chat/tools/advisor/tool.ts

@@ -5,6 +5,7 @@ import {
 } from "@mariozechner/pi-agent-core";
 import { Type } from "@sinclair/typebox";
 import type { AdvisorConfig } from "@/chat/config";
+import type { McpToolManager } from "@/chat/mcp/tool-manager";
 import {
   extractGenAiUsageAttributes,
   setSpanAttributes,
@@ -21,12 +22,19 @@ import {
   extractAssistantText,
   isAssistantMessage,
 } from "@/chat/respond-helpers";
+import type { Skill } from "@/chat/skills";
 import {
   createStateAdvisorSessionStore,
   getAdvisorSessionKey,
   type AdvisorSessionStore,
 } from "@/chat/tools/advisor/session-store";
-import { tool } from "@/chat/tools/definition";
+import {
+  hasReadOnlyToolAnnotations,
+  tool,
+  type ToolDefinition,
+} from "@/chat/tools/definition";
+import { createCallMcpToolTool } from "@/chat/tools/skill/call-mcp-tool";
+import { createSearchMcpToolsTool } from "@/chat/tools/skill/search-mcp-tools";
 import { escapeXml } from "@/chat/xml";
 
 export type AdvisorErrorCode =
@@ -57,26 +65,15 @@ export interface AdvisorToolRuntimeContext {
   streamFn?: StreamFn;
 }
 
-const ADVISOR_ALLOWED_TOOL_NAMES = new Set([
-  "bash",
-  "readFile",
-  "searchMcpTools",
-  "slackCanvasRead",
-  "slackChannelListMessages",
-  "slackListGetItems",
-  "systemTime",
-  "webFetch",
-  "webSearch",
-]);
-
 const ADVISOR_TOOL_DESCRIPTION =
-  "Ask a stronger advisor for deep technical guidance. Call this when the task has a hard reasoning core: algorithm design, architecture, concurrency, security-sensitive logic, data modeling, unclear requirements, repeated failures, difficult debugging, broad refactors, or final review of nontrivial work. Pass a focused question plus curated context containing the exact evidence, constraints, current plan, alternatives, command output, code snippets, or diffs the advisor should start from. The advisor does not automatically receive the parent transcript, keeps its own advisor history for this parent conversation, can use inspection tools to verify evidence, can reason deeply, and returns guidance for you to apply and verify. Follow-up calls can build on prior advisor guidance but must include any new evidence or changed constraints. Use it after initial orientation reads when repository context matters, before committing to a non-obvious implementation plan, when changing approach, when stuck, and before declaring complex work complete. Do not use it for greetings, simple deterministic edits, routine formatting, or tasks where the next action is already obvious from fresh tool output.";
+  "Ask a stronger advisor for deep technical guidance. Call this when the task has a hard reasoning core: algorithm design, architecture, concurrency, security-sensitive logic, data modeling, unclear requirements, repeated failures, difficult debugging, broad refactors, or final review of nontrivial work. Pass a focused question plus curated context containing the exact evidence, constraints, current plan, alternatives, command output, code snippets, or diffs the advisor should start from. The advisor does not automatically receive the parent transcript, keeps its own advisor history for this parent conversation, can use read-only inspection tools to verify evidence, can reason deeply, and returns guidance for you to apply and verify. Follow-up calls can build on prior advisor guidance but must include any new evidence or changed constraints. Use it after initial orientation reads when repository context matters, before committing to a non-obvious implementation plan, when changing approach, when stuck, and before declaring complex work complete. Do not use it for greetings, simple deterministic edits, routine formatting, or tasks where the next action is already obvious from fresh tool output.";
 
 const ADVISOR_SYSTEM_PROMPT = [
   "You are a senior technical advisor for the executor.",
-  "Analyze the executor-supplied context deeply. Use inspection tools when direct inspection or verification would materially improve the advice.",
+  "Analyze the executor-supplied context deeply. Use read-only tools when direct inspection or verification would materially improve the advice.",
   "Distinguish evidence from inference. Treat the advisor task as the focus for this call and the executor context as the starting evidence packet.",
   "Do not assume access to parent transcript or tool output that was not included or gathered in this advisor call.",
+  "Use only read-only tools. For MCP, use searchMcpTools and callMcpTool only for tools explicitly annotated readOnlyHint: true without destructiveHint: true.",
   "Do not make user-visible side effects, post Slack messages, or mutate files. If a mutating action is needed, recommend it to the executor instead.",
   "Identify the hard part, recommend a concrete plan or correction, call out blocking risks, and propose focused verification.",
   "If the supplied context is insufficient, say exactly what additional evidence the executor needs to gather before acting.",
@@ -116,9 +113,41 @@ function success(memo: string): AdvisorToolResult {
   };
 }
 
-/** Return whether a normal executor tool is safe to expose to the advisor. */
-export function isAdvisorToolAllowed(toolName: string): boolean {
-  return ADVISOR_ALLOWED_TOOL_NAMES.has(toolName);
+/** Build the advisor's read-only tool definition subset. */
+export function createAdvisorToolDefinitions(
+  definitions: Record<string, ToolDefinition<any>>,
+  options: {
+    getActiveSkills?: () => Skill[];
+    mcpToolManager?: McpToolManager;
+  } = {},
+): Record<string, ToolDefinition<any>> {
+  const advisorDefinitions = Object.fromEntries(
+    Object.entries(definitions).filter(
+      ([name, definition]) =>
+        name !== "callMcpTool" &&
+        name !== "searchMcpTools" &&
+        hasReadOnlyToolAnnotations(definition.annotations),
+    ),
+  );
+
+  if (options.mcpToolManager && options.getActiveSkills) {
+    if (definitions.searchMcpTools) {
+      advisorDefinitions.searchMcpTools = createSearchMcpToolsTool(
+        options.mcpToolManager,
+        options.getActiveSkills,
+        { readOnlyToolsOnly: true },
+      );
+    }
+    if (definitions.callMcpTool) {
+      advisorDefinitions.callMcpTool = createCallMcpToolTool(
+        options.mcpToolManager,
+        options.getActiveSkills,
+        { readOnlyToolsOnly: true },
+      );
+    }
+  }
+
+  return advisorDefinitions;
 }
 
 /** Create the advisor tool backed by conversation-scoped message history. */

packages/junior/src/chat/tools/definition.ts

@@ -1,8 +1,10 @@
+import type { ToolAnnotations } from "@modelcontextprotocol/sdk/types.js";
 import type { Static, TSchema } from "@sinclair/typebox";
 
 export interface ToolDefinition<TInputSchema extends TSchema = TSchema> {
   description: string;
   inputSchema: TInputSchema;
+  annotations?: ToolAnnotations;
   execute?: (
     input: Static<TInputSchema>,
     options: { experimental_context?: unknown },
@@ -15,3 +17,12 @@ export function tool<TInputSchema extends TSchema>(
 ): ToolDefinition<TInputSchema> {
   return definition;
 }
+
+/** Return whether tool annotations opt into read-only exposure. */
+export function hasReadOnlyToolAnnotations(
+  annotations: ToolAnnotations | Record<string, unknown> | undefined,
+): boolean {
+  return (
+    annotations?.readOnlyHint === true && annotations.destructiveHint !== true
+  );
+}

packages/junior/src/chat/tools/index.ts

@@ -23,6 +23,7 @@ import {
 } from "@/chat/tools/slack/list-tools";
 import { createSystemTimeTool } from "@/chat/tools/system-time";
 import { createAdvisorTool } from "@/chat/tools/advisor/tool";
+import type { ToolDefinition } from "@/chat/tools/definition";
 import type {
   ToolHooks,
   ToolRuntimeContext,
@@ -82,7 +83,7 @@ export function createTools(
   context: ToolRuntimeContext,
 ) {
   const state = createToolState(hooks, context);
-  const tools: Record<string, unknown> = {
+  const tools: Record<string, ToolDefinition<any>> = {
     loadSkill: createLoadSkillTool(availableSkills, {
       onSkillLoaded: hooks.onSkillLoaded,
     }),

packages/junior/src/chat/tools/sandbox/read-file.ts

@@ -5,6 +5,7 @@ export function createReadFileTool() {
   return tool({
     description:
       "Read a file from the sandbox workspace. Use when you need exact file contents to verify facts or make edits safely. Do not use for broad discovery when search tools are better.",
+    annotations: { readOnlyHint: true, destructiveHint: false },
     inputSchema: Type.Object(
       {
         path: Type.String({

packages/junior/src/chat/tools/skill/call-mcp-tool.ts

@@ -1,7 +1,11 @@
 import { Type } from "@sinclair/typebox";
-import type { McpToolManager } from "@/chat/mcp/tool-manager";
+import type { ManagedMcpTool, McpToolManager } from "@/chat/mcp/tool-manager";
 import type { Skill } from "@/chat/skills";
-import { tool } from "@/chat/tools/definition";
+import { hasReadOnlyToolAnnotations, tool } from "@/chat/tools/definition";
+
+interface CallMcpToolOptions {
+  readOnlyToolsOnly?: boolean;
+}
 
 function resolveMcpArguments(
   input: Record<string, unknown>,
@@ -29,14 +33,38 @@ function resolveMcpArguments(
   return {};
 }
 
+function isCallableMcpTool(
+  toolDef: ManagedMcpTool,
+  options: CallMcpToolOptions,
+): boolean {
+  return (
+    !options.readOnlyToolsOnly ||
+    hasReadOnlyToolAnnotations(toolDef.annotations)
+  );
+}
+
+function inactiveToolMessage(
+  toolName: string,
+  options: CallMcpToolOptions,
+): string {
+  return options.readOnlyToolsOnly
+    ? `MCP tool is not active or read-only for this turn: ${toolName}`
+    : `MCP tool is not active for this turn: ${toolName}`;
+}
+
 /** Create the stable dispatcher for active MCP provider tools. */
 export function createCallMcpToolTool(
   mcpToolManager: McpToolManager,
   getActiveSkills: () => Skill[],
+  options: CallMcpToolOptions = {},
 ) {
   return tool({
-    description:
-      "Call an active MCP tool by exact tool_name. Use loadSkill to activate the provider, then searchMcpTools to discover tool names and schemas; copy required provider fields into arguments. Do not call with only tool_name unless the discovered tool has no arguments. Authorization is handled by the runtime when required.",
+    description: options.readOnlyToolsOnly
+      ? "Call an active read-only MCP tool by exact tool_name. Use searchMcpTools to discover read-only tool names and schemas; copy required provider fields into arguments. The runtime rejects MCP tools that are not explicitly annotated readOnlyHint: true or are annotated destructiveHint: true."
+      : "Call an active MCP tool by exact tool_name. Use loadSkill to activate the provider, then searchMcpTools to discover tool names and schemas; copy required provider fields into arguments. Do not call with only tool_name unless the discovered tool has no arguments. Authorization is handled by the runtime when required.",
+    ...(options.readOnlyToolsOnly
+      ? { annotations: { readOnlyHint: true, destructiveHint: false } }
+      : {}),
     inputSchema: Type.Object(
       {
         tool_name: Type.String({
@@ -56,9 +84,10 @@ export function createCallMcpToolTool(
       const { tool_name } = input;
       const mcpTool = mcpToolManager
         .getResolvedActiveTools(getActiveSkills())
+        .filter((candidate) => isCallableMcpTool(candidate, options))
         .find((candidate) => candidate.name === tool_name);
       if (!mcpTool) {
-        throw new Error(`MCP tool is not active for this turn: ${tool_name}`);
+        throw new Error(inactiveToolMessage(tool_name, options));
       }
       return await mcpTool.execute(
         resolveMcpArguments(input as Record<string, unknown>),

packages/junior/src/chat/tools/skill/search-mcp-tools.ts

@@ -4,7 +4,7 @@ import type {
   McpToolManager,
 } from "@/chat/mcp/tool-manager";
 import type { Skill } from "@/chat/skills";
-import { tool } from "@/chat/tools/definition";
+import { hasReadOnlyToolAnnotations, tool } from "@/chat/tools/definition";
 import { toExposedToolSummary } from "@/chat/tools/skill/mcp-tool-summary";
 
 const DEFAULT_MAX_RESULTS = 5;
@@ -15,6 +15,10 @@ interface RankedTool {
   score: number;
 }
 
+interface SearchMcpToolsOptions {
+  readOnlyToolsOnly?: boolean;
+}
+
 function normalize(value: string): string {
   return value
     .toLowerCase()
@@ -108,14 +112,28 @@ function searchMcpCatalog(
     .map((ranked) => ranked.tool);
 }
 
+function filterSearchableTools(
+  tools: ManagedMcpToolDescriptor[],
+  options: SearchMcpToolsOptions,
+): ManagedMcpToolDescriptor[] {
+  if (!options.readOnlyToolsOnly) {
+    return tools;
+  }
+  return tools.filter((toolDef) =>
+    hasReadOnlyToolAnnotations(toolDef.annotations),
+  );
+}
+
 /** Create the progressive MCP catalog search tool used before callMcpTool. */
 export function createSearchMcpToolsTool(
   mcpToolManager: McpToolManager,
   getActiveSkills: () => Skill[],
+  options: SearchMcpToolsOptions = {},
 ) {
   return tool({
     description:
       "List or search active MCP tools and return full descriptors, including input/output schemas and annotations. Use after loadSkill when choosing a provider tool or when callMcpTool arguments are unclear.",
+    annotations: { readOnlyHint: true, destructiveHint: false },
     inputSchema: Type.Object(
       {
         query: Type.Optional(
@@ -146,15 +164,16 @@ export function createSearchMcpToolsTool(
         getActiveSkills(),
         provider ? { provider } : {},
       );
+      const searchableCatalog = filterSearchableTools(catalog, options);
       const maxResults = max_results ?? DEFAULT_MAX_RESULTS;
-      const matches = searchMcpCatalog(catalog, query ?? "").slice(
+      const matches = searchMcpCatalog(searchableCatalog, query ?? "").slice(
         0,
         maxResults,
       );
       return {
         query: query ?? null,
         provider: provider ?? null,
-        total_active_tools: catalog.length,
+        total_active_tools: searchableCatalog.length,
         returned_tools: matches.length,
         tools: matches.map(toExposedToolSummary),
       };

packages/junior/src/chat/tools/slack/canvas-tools.ts

@@ -237,6 +237,7 @@ export function createSlackCanvasReadTool() {
   return tool({
     description:
       "Read a Slack canvas the bot has access to (including canvases the bot created) by canvas ID or Slack canvas/docs URL. Use when the user shares a Slack canvas link (https://*.slack.com/docs/... or /canvas/...) or references a canvas ID and you need its contents. Do not use for generic web pages — use webFetch for those.",
+    annotations: { readOnlyHint: true, destructiveHint: false },
     inputSchema: Type.Object({
       canvas: Type.String({
         minLength: 1,

packages/junior/src/chat/tools/slack/channel-list-messages.ts

@@ -10,6 +10,7 @@ export function createSlackChannelListMessagesTool(
   return tool({
     description:
       "List channel messages from Slack history in the active channel context. Use when the user asks for recent or historical channel context outside this thread. Do not use for live monitoring or when current thread context already answers the question.",
+    annotations: { readOnlyHint: true, destructiveHint: false },
     inputSchema: Type.Object({
       limit: Type.Optional(
         Type.Integer({

packages/junior/src/chat/tools/slack/list-tools.ts

@@ -133,6 +133,7 @@ export function createSlackListGetItemsTool(state: ToolState) {
   return tool({
     description:
       "Read items from the active Slack list tracked in artifact context. Use when the user asks for task status, open items, or list contents. Do not use when list state is already known from the immediately prior result.",
+    annotations: { readOnlyHint: true, destructiveHint: false },
     inputSchema: Type.Object({
       limit: Type.Optional(
         Type.Integer({

packages/junior/src/chat/tools/system-time.ts

@@ -5,16 +5,19 @@ export function createSystemTimeTool() {
   return tool({
     description:
       "Return current system time in UTC and local ISO formats. Use when the user asks for current time/date context. Do not use as a substitute for historical or timezone-conversion research.",
+    annotations: { readOnlyHint: true, destructiveHint: false },
     inputSchema: Type.Object({}),
     execute: async () => {
       const now = new Date();
       return {
         ok: true,
         unix_ms: now.getTime(),
         iso_utc: now.toISOString(),
-        iso_local: new Date(now.getTime() - now.getTimezoneOffset() * 60000).toISOString().replace("Z", ""),
-        timezone_offset_minutes: now.getTimezoneOffset()
+        iso_local: new Date(now.getTime() - now.getTimezoneOffset() * 60000)
+          .toISOString()
+          .replace("Z", ""),
+        timezone_offset_minutes: now.getTimezoneOffset(),
       };
-    }
+    },
   });
 }

packages/junior/src/chat/tools/web/fetch-tool.ts

@@ -41,6 +41,11 @@ export function createWebFetchTool(hooks: ToolHooks) {
   return tool({
     description:
       "Fetch and extract readable content from a specific URL. Use when you need details from a known page or document. Do not use for discovery when search is the first step.",
+    annotations: {
+      readOnlyHint: true,
+      destructiveHint: false,
+      openWorldHint: true,
+    },
     inputSchema: Type.Object({
       url: Type.String({
         minLength: 1,