feat(mcp): read discovered skill files remotely

shared/glean/mcp/skills/glean_run/SKILL.md

@@ -2,8 +2,6 @@
 name: glean_run
 description: Discover and run Glean skills for enterprise app tasks
 argument-hint: <task description>
-allowed-tools:
-  - Read(path="//**/glean-skills-cache/**")
 ---
 
 # Glean Run
@@ -64,24 +62,26 @@ find_skills_and_tools({
 })
 ```
 
-The response is an XML index of discovered skills with file paths.
+The response is an XML index of discovered skills and their available file paths.
 
 You can call `find_skills_and_tools` multiple times — e.g. to discover skills for
 individual sub-tasks as you work through a broad request.
 
 ## Step 2: Read Skill Instructions
 
 Browse the returned skills and select the one most relevant to the user's
-request. Read its `SKILL.md` file for detailed instructions. Skills typically
-contain guidance on how to use their tools, but the tools can also be called
-as independent units.
+request. Call `read_skill_files` with the selected skill name and `SKILL.md`
+to retrieve its detailed instructions. Skills typically contain guidance on
+how to use their tools, but the tools can also be called as independent units.
 
 ## Step 3: Read Tool Schemas
 
-Read each tool's JSON file (e.g. `tools/TOOL_NAME.json`) to get the exact
-`server_id`, `name`, and `inputSchema` with parameter names and types.
+Call `read_skill_files` for each tool's JSON file (e.g.
+`tools/TOOL_NAME.json`) to get the exact `server_id`, `name`, and `inputSchema`
+with parameter names and types.
 
-**Never guess parameter names** - always read the tool JSON file first.
+**Never guess parameter names** - always call `read_skill_files` for the tool
+JSON before calling `run_tool`.
 
 ## Step 4: Execute Tools
 

shared/glean/mcp/src/index.ts

@@ -8,7 +8,6 @@ import {
 } from "@modelcontextprotocol/sdk/types.js";
 import path from "node:path";
 import fs from "node:fs";
-import { homedir, tmpdir } from "node:os";
 import {
   AuthRequiredError,
   createRemoteClient,
@@ -20,8 +19,9 @@ import {
   closeCallbackServer,
 } from "./auth-callback-server.js";
 import { handleFindSkills } from "./tools/find-skills.js";
+import { handleReadSkillFiles } from "./tools/read-skill-files.js";
 import { handleRunTool, runToolAnnotations } from "./tools/run-tool.js";
-import { evictStaleSkills } from "./skill-writer.js";
+import { SkillFileCache } from "./skill-files.js";
 import {
   loadServerUrl,
   saveServerUrl,
@@ -127,13 +127,6 @@ function logLine(label: string, detail?: Record<string, unknown>): void {
   console.error(line.trimEnd());
 }
 
-function resolveSkillsBaseDir(): string {
-  if (process.env.SKILLS_BASE_DIR) {
-    return process.env.SKILLS_BASE_DIR;
-  }
-  return path.join(tmpdir(), "glean-skills-cache");
-}
-
 const server = new Server(
   { name: "glean", version: pluginVersionString() },
   { capabilities: { tools: { listChanged: true } } },
@@ -156,6 +149,11 @@ let oauthProvider: GleanOAuthClientProvider | undefined;
 // successful tool fetch.
 let cachedRemoteTools: Tool[] = loadRemoteTools(resolveServerUrl() ?? "");
 
+// Skill contents are owned by the remote server. The plugin retains only the
+// tool metadata needed by local HITL and file_args handling, plus an in-memory
+// compatibility cache for older servers that still return full JSON skills.
+const skillFiles = new SkillFileCache();
+
 function getOAuthProvider(): GleanOAuthClientProvider {
   if (!oauthProvider) {
     oauthProvider = new GleanOAuthClientProvider();
@@ -193,18 +191,16 @@ const FIND_SKILLS_TOOL: Tool = {
     "timing, reasons, constraints) — and pass each as a separate entry in the " +
     "'queries' array. For example, for \"Send an email to X for tomorrow's demo " +
     "meeting as leadership will be visiting\", the single query is \"send an email\". " +
-    "Discovered skills are written to local files and an XML skill " +
-    "index with usage instructions is returned. " +
+    "An XML skill index with the available file paths is returned. " +
     "If a returned skill lists no tools and its playbook does not let you " +
     "complete the task, first check whether tools already in scope can do it — " +
     "tools from other skills in this response, tools from earlier find_skills_and_tools " +
     "calls, or tools you can already call directly. If none fit, call find_skills_and_tools " +
     "again with reworded or additional queries. " +
-    "If a previously-cached skill file referenced from memory or instructions " +
-    "is missing on disk, call find_skills_and_tools again to re-fetch it before failing. " +
     "To use a returned skill: (1) pick the most relevant from the returned " +
-    "skills; (2) read its SKILL.md for instructions; (3) read each tool's JSON " +
-    "file (tools/TOOL_NAME.json) for the exact server_id, name, and inputSchema " +
+    "skills; (2) call read_skill_files for SKILL.md and read it for instructions; " +
+    "(3) call read_skill_files for each tool JSON file " +
+    "(tools/TOOL_NAME.json) to get the exact server_id, name, and inputSchema " +
     "(exact parameter names and types); (4) call run_tool with the server_id, " +
     "tool_name (from the name field), and arguments matching the inputSchema. " +
     "Never guess parameter names — read the tool JSON file first.",
@@ -225,11 +221,38 @@ const FIND_SKILLS_TOOL: Tool = {
   },
 };
 
+const READ_SKILL_FILES_TOOL: Tool = {
+  name: "read_skill_files",
+  annotations: { readOnlyHint: true },
+  description:
+    "Read one or more files from a Glean skill returned by a previous " +
+    "find_skills_and_tools call. Use this to read SKILL.md and tools/*.json " +
+    "before using run_tool. Pass the exact skill name and file paths from the " +
+    "find_skills_and_tools response.",
+  inputSchema: {
+    type: "object" as const,
+    properties: {
+      skill_name: {
+        type: "string",
+        description: "The skill name from a previous find_skills_and_tools response.",
+      },
+      file_paths: {
+        type: "array",
+        items: { type: "string" },
+        minItems: 1,
+        description:
+          "Paths within the skill to read, such as SKILL.md or tools/TOOL_NAME.json.",
+      },
+    },
+    required: ["skill_name", "file_paths"],
+  },
+};
+
 const RUN_TOOL_TOOL: Tool = {
   name: "run_tool",
   description:
     "Execute a tool on a downstream MCP server. Before calling this tool, " +
-    "you MUST read the tool's JSON file from the find_skills_and_tools output to get " +
+    "you MUST call read_skill_files for the tool's JSON file to get " +
     "the exact server_id, tool_name, and input_schema. Pass arguments that match " +
     "the input_schema exactly — do not guess parameter names.",
   inputSchema: {
@@ -322,6 +345,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => {
       decision,
       setupTool: SETUP_TOOL,
       findSkillsTool: FIND_SKILLS_TOOL,
+      readSkillFilesTool: READ_SKILL_FILES_TOOL,
       runTool,
       promoted: dynamic,
     });
@@ -663,8 +687,6 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => {
 
       const sessionId = resolveSessionId();
 
-      const skillsBaseDir = resolveSkillsBaseDir();
-
       let remoteClient;
       try {
         remoteClient = await createRemoteClient(
@@ -691,11 +713,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => {
         };
       }
       try {
-        const text = await handleFindSkills(
-          remoteClient,
-          skillsBaseDir,
-          args,
-        );
+        const text = await handleFindSkills(remoteClient, skillFiles, args);
         return { content: [{ type: "text", text }] };
       } catch (err) {
         const msg = err instanceof Error ? err.message : String(err);
@@ -709,6 +727,58 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => {
       }
     }
 
+    case "read_skill_files": {
+      const serverUrl = resolveServerUrl();
+      if (!serverUrl) {
+        return {
+          content: [{ type: "text", text: SETUP_NEEDED_ERROR }],
+          isError: true,
+        };
+      }
+
+      if (!getOAuthProvider().tokens()) {
+        return {
+          content: [{ type: "text", text: AUTH_REDIRECT_TO_SETUP_TEXT }],
+        };
+      }
+
+      let remoteClient;
+      try {
+        remoteClient = await createRemoteClient(
+          serverUrl,
+          getRemoteClientOpts(),
+          resolveSessionId(),
+        );
+      } catch (err) {
+        if (err instanceof AuthRequiredError) {
+          return {
+            content: [{ type: "text", text: AUTH_REDIRECT_TO_SETUP_TEXT }],
+          };
+        }
+        const msg = err instanceof Error ? err.message : String(err);
+        logLine("connect.backend-error", { label: "read_skill_files", msg });
+        return {
+          content: [
+            { type: "text", text: `Failed to connect to Glean backend: ${msg}` },
+          ],
+          isError: true,
+        };
+      }
+      try {
+        const text = await handleReadSkillFiles(remoteClient, skillFiles, args);
+        return { content: [{ type: "text", text }] };
+      } catch (err) {
+        const msg = err instanceof Error ? err.message : String(err);
+        console.error(`read_skill_files: execution failed: ${msg}`);
+        return {
+          content: [{ type: "text", text: `read_skill_files failed: ${msg}` }],
+          isError: true,
+        };
+      } finally {
+        await remoteClient.close();
+      }
+    }
+
     case "run_tool": {
       const serverUrl = resolveServerUrl();
       if (!serverUrl) {
@@ -755,8 +825,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => {
         };
       }
       try {
-        const skillsBaseDir = resolveSkillsBaseDir();
-        return await handleRunTool(remoteClient, server, skillsBaseDir, args, {
+        return await handleRunTool(remoteClient, server, skillFiles.metadata, args, {
           fileArgs: decision.features.fileArgs,
         });
       } catch (err) {
@@ -780,6 +849,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => {
         clearServerUrl();
         clearCredentials();
         clearRemoteTools();
+        skillFiles.clear();
         oauthProvider = undefined;
         cachedRemoteTools = [];
         // Policy survives a user reset: only a new valid remote policy may
@@ -862,6 +932,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => {
         // instant); we just rehydrate from whatever cache exists for the
         // newly configured URL — empty for a first-time server.
         clearCredentials();
+        skillFiles.clear();
         oauthProvider = undefined;
         cachedRemoteTools = loadRemoteTools(normalized);
         setPolicyServerUrl(normalized);
@@ -882,15 +953,6 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => {
 });
 
 async function main() {
-  // Run once per session at MCP server startup.
-  const ONE_WEEK_MS = 7 * 24 * 60 * 60 * 1000;
-  try {
-    await evictStaleSkills(resolveSkillsBaseDir(), ONE_WEEK_MS, logLine);
-  } catch (err) {
-    const msg = err instanceof Error ? err.message : String(err);
-    logLine("evict-stale-skills.failed", { msg });
-  }
-
   // Observe the negotiated MCP protocol revision from the initialize response.
   const transport = protocolVersion.wrap(new StdioServerTransport());
   await server.connect(transport);

shared/glean/mcp/src/policy/enforce.ts

@@ -5,10 +5,11 @@ import type { Decision } from "./types.js";
 // because it is how a user restores the connection that would lift the deactivation.
 export const SETUP_TOOL_NAME = "setup";
 
-// Gated together by `metaTools`, and gated as a pair on purpose: a surface with
-// find_skills_and_tools but no run_tool can discover work it cannot perform.
+// Gated together by `metaTools`: discovery, file reads, and execution form one
+// surface and must be withdrawn together.
 export const META_TOOL_NAMES: ReadonlySet<string> = new Set([
   "find_skills_and_tools",
+  "read_skill_files",
   "run_tool",
 ]);
 
@@ -47,16 +48,25 @@ export function advertisedTools(input: {
   decision: Decision;
   setupTool: Tool;
   findSkillsTool: Tool;
+  readSkillFilesTool: Tool;
   runTool: Tool;
   promoted: Tool[];
 }): Advertisement {
-  const { decision, setupTool, findSkillsTool, runTool, promoted } = input;
+  const {
+    decision,
+    setupTool,
+    findSkillsTool,
+    readSkillFilesTool,
+    runTool,
+    promoted,
+  } = input;
 
   if (decision.deactivated) {
     return {
       tools: [setupTool],
       withheld: [
         findSkillsTool.name,
+        readSkillFilesTool.name,
         runTool.name,
         ...promoted.map((t) => t.name),
       ],
@@ -69,10 +79,11 @@ export function advertisedTools(input: {
   if (decision.features.metaTools) {
     tools.push(
       findSkillsTool,
+      readSkillFilesTool,
       decision.features.fileArgs ? runTool : withoutFileArgs(runTool),
     );
   } else {
-    withheld.push(findSkillsTool.name, runTool.name);
+    withheld.push(findSkillsTool.name, readSkillFilesTool.name, runTool.name);
   }
 
   tools.push(setupTool);