feat(vnext): port capability policy and Cursor HITL updates

shared/glean/mcp/build.mjs

@@ -2,54 +2,89 @@
 //
 // Why bundle: Cowork's plugin-install validator rejects zip entries whose
 // paths contain `@`, which appears in every scoped npm package's directory
-// name (`node_modules/@modelcontextprotocol/...`). Inlining every dep into
-// one `dist/index.js` means the shipped tree has no scoped-package paths.
-//
-// Bundle shape:
-//   - platform=node, format=esm so Node can load it with `node dist/index.js`
-//     and no `--experimental-*` flags, matching our package.json type:module
-//   - bundle=true with packages='bundled' so every import except Node
-//     builtins gets inlined
-//   - external: the `node:*` builtins (explicit for clarity; esbuild on
-//     platform=node treats bare `node:*` as external by default but we pin
-//     it so this doesn't regress silently)
-//   - no sourcemap or minification — the bundle is checked into git and
-//     should stay readable for debugging
+// name (`node_modules/@modelcontextprotocol/...`). Inlining every dependency
+// into one dist/index.js means the shipped tree has no scoped-package paths.
 
 import { build } from "esbuild";
 import { builtinModules } from "node:module";
+import { readFileSync } from "node:fs";
 
 const nodeBuiltins = [
   ...builtinModules,
   ...builtinModules.map((m) => `node:${m}`),
 ];
 
+const VERSION_FILES = [
+  "package.json",
+  "shared/glean/mcp/package.json",
+];
+const SEMVER_RE = /^(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)$/;
+
+function pluginVersionFromPackages() {
+  const found = VERSION_FILES.map((file) => {
+    let raw;
+    try {
+      raw = JSON.parse(readFileSync(file, "utf-8"));
+    } catch (err) {
+      throw new Error(`build: cannot read ${file}: ${err.message}`);
+    }
+    const version = raw.version;
+    if (typeof version !== "string" || !SEMVER_RE.test(version)) {
+      throw new Error(
+        `build: ${file} must declare a plain x.y.z version, got ${JSON.stringify(version)}`,
+      );
+    }
+    return { file, version };
+  });
+
+  const versions = [...new Set(found.map((entry) => entry.version))];
+  if (versions.length !== 1) {
+    throw new Error(
+      `build: package versions disagree, so there is no single version to bake in:\n` +
+        found.map((entry) => `  ${entry.version}  ${entry.file}`).join("\n"),
+    );
+  }
+  return versions[0];
+}
+
+const pluginVersion = pluginVersionFromPackages();
+const OUTFILE = "shared/glean/mcp/dist/index.js";
+
 await build({
   entryPoints: ["shared/glean/mcp/src/index.ts"],
-  outfile: "shared/glean/mcp/dist/index.js",
+  outfile: OUTFILE,
   bundle: true,
   platform: "node",
   format: "esm",
   target: "node20",
-  // Not setting `packages` — esbuild only accepts `"external"` here, which
-  // would ship every dep as a runtime lookup (defeating the purpose). The
-  // default when `bundle:true` is to inline every import whose specifier
-  // isn't in `external`, which is exactly what we want.
+  define: {
+    __GLEAN_PLUGIN_VERSION__: JSON.stringify(pluginVersion),
+  },
+  // Not setting `packages` — the default with bundle:true inlines every
+  // non-external import, which is exactly what the shipped single-file server
+  // needs.
   external: nodeBuiltins,
-  // Some transitive deps (e.g. `yaml`) ship CJS that does `require("node:*")`
-  // at module-eval time. esbuild inlines that CJS under an ESM shim that
-  // does NOT provide a `require`, so imports blow up with "Dynamic require
-  // of X is not supported". Prepending a `createRequire`-based shim gives
-  // the inlined CJS a working `require` for Node builtins.
+  // Some transitive deps ship CJS that requires node:* at module-eval time.
+  // Provide a working require shim inside the ESM bundle for those builtins.
   banner: {
     js: `import { createRequire as __glean_createRequire } from "node:module";\nconst require = __glean_createRequire(import.meta.url);`,
   },
   minify: false,
   legalComments: "linked",
   logLevel: "info",
-  // The SDK and some transitive deps still ship CJS under their "require"
-  // export condition. We're emitting ESM and asking esbuild to resolve
-  // through each package's "import" condition first.
   conditions: ["import", "node", "default"],
   mainFields: ["module", "main"],
 });
+
+const bundled = readFileSync(OUTFILE, "utf-8");
+if (bundled.includes("__GLEAN_PLUGIN_VERSION__")) {
+  throw new Error(
+    `build: ${OUTFILE} still contains __GLEAN_PLUGIN_VERSION__; esbuild did not substitute it`,
+  );
+}
+if (!bundled.includes(JSON.stringify(pluginVersion))) {
+  throw new Error(
+    `build: ${OUTFILE} does not contain the version literal ${JSON.stringify(pluginVersion)}`,
+  );
+}
+console.log(`Baked plugin version ${pluginVersion} into ${OUTFILE}`);

shared/glean/mcp/src/atomic-write.ts

@@ -0,0 +1,52 @@
+import fs from "node:fs";
+import path from "node:path";
+
+/**
+ * Write a file so no reader can observe a partial one.
+ *
+ * `fs.writeFileSync` truncates and then writes, so a process killed mid-write leaves a
+ * truncated file behind. Every store here parses JSON and treats a parse failure as "no
+ * data", so a torn write does not surface as an error — it silently discards whatever was
+ * stored. For the policy cache that is the one outcome we have gone out of our way to
+ * prevent: nothing is allowed to clear it, precisely because a cached policy may carry a
+ * deactivation or a version block, and a crash mid-write would clear it anyway.
+ *
+ * Writing to a sibling temp file and renaming makes the swap atomic — a reader sees either
+ * the old contents or the new ones, never a mixture. The temp file must live in the same
+ * directory, since rename is only atomic within a filesystem, and it carries the pid
+ * because each host session runs its own plugin process and they share these files.
+ *
+ * This is NOT mutual exclusion. Two processes doing read-modify-write can still lose one
+ * update, last writer winning; they simply cannot corrupt the file. A lost update
+ * self-heals on the next negotiation, whereas a corrupt file discards every entry for
+ * every URL until something rewrites it.
+ *
+ * On failure the temp file is removed and the error rethrown, leaving whatever was
+ * already on disk intact. Windows can fail the rename with EPERM/EBUSY when another
+ * process holds the target open; callers already tolerate a failed write, so that
+ * degrades to "this write was skipped" rather than to corruption.
+ */
+export function writeFileAtomicSync(
+  filePath: string,
+  data: string,
+  mode: number,
+): void {
+  const tmpPath = path.join(
+    path.dirname(filePath),
+    `.${path.basename(filePath)}.${process.pid}.tmp`,
+  );
+  try {
+    fs.writeFileSync(tmpPath, data, { encoding: "utf-8", mode });
+    // writeFileSync only applies `mode` when it creates the file, so a leftover temp
+    // from a previous crash could otherwise keep looser permissions.
+    fs.chmodSync(tmpPath, mode);
+    fs.renameSync(tmpPath, filePath);
+  } catch (err) {
+    try {
+      fs.rmSync(tmpPath, { force: true });
+    } catch {
+      // Nothing further to do; the target is untouched either way.
+    }
+    throw err;
+  }
+}

shared/glean/mcp/src/index.ts

@@ -20,11 +20,7 @@ import {
   closeCallbackServer,
 } from "./auth-callback-server.js";
 import { handleFindSkills } from "./tools/find-skills.js";
-import {
-  handleRunTool,
-  isCursorClient,
-  runToolAnnotations,
-} from "./tools/run-tool.js";
+import { handleRunTool, runToolAnnotations } from "./tools/run-tool.js";
 import { evictStaleSkills } from "./skill-writer.js";
 import {
   loadServerUrl,
@@ -45,7 +41,15 @@ import {
 } from "./tools/remote-passthrough.js";
 import { resolveSessionId } from "./session-id.js";
 import { resolveServerUrlFromEmail } from "./config-search.js";
-import { PLUGIN_VERSION } from "./version.js";
+import { pluginVersionString } from "./version.js";
+import {
+  decisionInForce,
+  initPolicySession,
+  policySummary,
+  protocolVersion,
+  setPolicyServerUrl,
+} from "./policy/session.js";
+import { advertisedTools, policyRefusal } from "./policy/enforce.js";
 
 function readEnv(...keys: string[]): string | undefined {
   for (const key of keys) {
@@ -127,10 +131,15 @@ function resolveSkillsBaseDir(): string {
 }
 
 const server = new Server(
-  { name: "glean", version: PLUGIN_VERSION },
+  { name: "glean", version: pluginVersionString() },
   { capabilities: { tools: { listChanged: true } } },
 );
 
+// Report the negotiated host/plugin context to the remote and enforce the
+// capability policy returned for this Glean instance.
+initPolicySession(server, logLine);
+setPolicyServerUrl(resolveServerUrl());
+
 let oauthProvider: GleanOAuthClientProvider | undefined;
 
 // Cache of the last successful remote tools/list fetch. Persists for the
@@ -293,31 +302,37 @@ const SETUP_TOOL: Tool = {
 };
 
 server.setRequestHandler(ListToolsRequestSchema, async () => {
-  const runTool: Tool = {
-    ...RUN_TOOL_TOOL,
-    annotations: runToolAnnotations(
-      process.env.ENABLE_HITL === "true",
-      !!server.getClientCapabilities()?.elicitation,
-      isCursorClient(server),
-    ),
-  };
-  const staticTools: Tool[] = [FIND_SKILLS_TOOL, runTool, SETUP_TOOL];
-
-  // One structured line on every return path, so "why don't my tools appear?"
-  // is answerable from the log alone: `static` is constant, `names` lists the
-  // dynamic tools we actually surfaced (freshly fetched or served from cache),
-  // and `state` names the path we took. The allow-list only ever drops tools
-  // outside our fixed set, so a missing allow-listed name (e.g. `chat`) means
-  // the backend never returned it. Only tool *names*, counts and the state
-  // tag are logged — never argument values, which can carry PII/secrets.
+  // Read the policy after any remote fetch: fetchAllowedRemoteTools records a
+  // policy returned by tools/list, so reading it earlier would be one request
+  // stale against the catalog we are about to advertise.
   const serve = (state: string, dynamic: Tool[]): { tools: Tool[] } => {
+    const decision = decisionInForce();
+    const runTool: Tool = {
+      ...RUN_TOOL_TOOL,
+      annotations: runToolAnnotations(
+        process.env.ENABLE_HITL === "true",
+        !!server.getClientCapabilities()?.elicitation,
+      ),
+    };
+    const { tools, withheld } = advertisedTools({
+      decision,
+      setupTool: SETUP_TOOL,
+      findSkillsTool: FIND_SKILLS_TOOL,
+      runTool,
+      promoted: dynamic,
+    });
+    const fromCatalog = new Set(dynamic.map((tool) => tool.name));
     logLine("tools-list.served", {
-      static: staticTools.length,
-      dynamic: dynamic.length,
-      names: dynamic.map((t) => t.name),
+      static: tools.filter((tool) => !fromCatalog.has(tool.name)).length,
+      dynamic: tools.filter((tool) => fromCatalog.has(tool.name)).length,
+      names: dynamic.map((tool) => tool.name),
+      withheld,
+      deactivated: decision.deactivated,
+      versionState: decision.versionState,
+      features: decision.features,
       state,
     });
-    return { tools: [...staticTools, ...dynamic] };
+    return { tools };
   };
 
   // Pre-auth gate: tokens() is sync. When unauthenticated (or unconfigured)
@@ -531,6 +546,20 @@ async function advanceSetup(): Promise<CallToolResult> {
     cachedRemoteTools = remoteTools;
     saveRemoteTools(serverUrl, remoteTools);
     const toolNames = remoteTools.map((t) => t.name).join(", ") || "(none)";
+    const decision = decisionInForce();
+    const closing = decision.deactivated
+      ? `This plugin version is not supported by your Glean instance, so only ` +
+        `\`setup\` is available. Upgrade the Glean plugin to restore the rest.`
+      : `You can now use ` +
+        [
+          ...(decision.features.metaTools
+            ? ["find_skills_and_tools", "run_tool"]
+            : []),
+          ...(decision.features.toolPromotion && remoteTools.length > 0
+            ? ["any of the listed remote tools"]
+            : []),
+        ].join(", ") +
+        `.`;
     return {
       content: [
         {
@@ -539,9 +568,9 @@ async function advanceSetup(): Promise<CallToolResult> {
             `Glean setup is complete.\n` +
             `Server URL: ${serverUrl}\n` +
             `Authenticated: yes\n` +
-            `Remote tools: ${toolNames}\n\n` +
-            `You can now use find_skills_and_tools, run_tool, and any of the listed ` +
-            `remote tools.`,
+            `Remote tools: ${toolNames}\n` +
+            `${policySummary().join("\n")}\n\n` +
+            closing,
         },
       ],
     };
@@ -569,6 +598,25 @@ async function advanceSetup(): Promise<CallToolResult> {
 server.setRequestHandler(CallToolRequestSchema, async (request) => {
   const { name, arguments: args = {} } = request.params;
 
+  // Advertisement is advisory: a host may retain a stale tool list, so every
+  // policy withdrawal is also enforced at call time. Setup remains the
+  // recovery path and is always exempt from refusal.
+  const decision = decisionInForce();
+  const refusal = policyRefusal({
+    name,
+    decision,
+    promoted: REMOTE_TOOLS_ALLOWLIST,
+  });
+  if (refusal) {
+    logLine("policy.refused", {
+      tool: name,
+      deactivated: decision.deactivated,
+      versionState: decision.versionState,
+      features: decision.features,
+    });
+    return refusal;
+  }
+
   // Allow-listed remote tools (chat/search/read_document) — only valid once
   // setup has provided a server URL. Auth is handled by dispatchRemoteTool
   // via the standard [AUTHENTICATION_REQUIRED] flow.
@@ -712,7 +760,9 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => {
       }
       try {
         const skillsBaseDir = resolveSkillsBaseDir();
-        return await handleRunTool(remoteClient, server, skillsBaseDir, args);
+        return await handleRunTool(remoteClient, server, skillsBaseDir, args, {
+          fileArgs: decision.features.fileArgs,
+        });
       } catch (err) {
         const msg = err instanceof Error ? err.message : String(err);
         console.error(`run_tool: execution failed: ${msg}`);
@@ -736,6 +786,9 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => {
         clearRemoteTools();
         oauthProvider = undefined;
         cachedRemoteTools = [];
+        // Policy survives a user reset: only a new valid remote policy may
+        // replace a cached deactivation or feature restriction.
+        setPolicyServerUrl(undefined);
         logLine("setup.reset");
         // Fire-and-forget — tools list is shorter without the dynamic
         // surface; the host should re-fetch on its next idle cycle.
@@ -815,6 +868,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => {
         clearCredentials();
         oauthProvider = undefined;
         cachedRemoteTools = loadRemoteTools(normalized);
+        setPolicyServerUrl(normalized);
         logLine("setup.configured", { serverUrl: normalized });
         // Fall through to advanceSetup, which will now find URL ✓ and try
         // to drive auth + tool fetch in the same call.
@@ -841,7 +895,8 @@ async function main() {
     logLine("evict-stale-skills.failed", { msg });
   }
 
-  const transport = new StdioServerTransport();
+  // Observe the negotiated MCP protocol revision from the initialize response.
+  const transport = protocolVersion.wrap(new StdioServerTransport());
   await server.connect(transport);
 }