mcp: advertise the registry's projection on all three servers (#9655) (#9812)

One contract entry produced three differently-advertised tools, and nothing could see it:
`diffToolSets` compares name sets, and `checkAdvertisedShape` only asks whether a description is
non-empty.

- The REMOTE server wrote a description literal at every `register(...)` call site -- 35 had drifted
  from the contract's, which is the text `listToolDefinitions()` serves to the agent-spec builders
  and the `.well-known` catalogs, so one tool was described two different ways depending on which
  LoopOver surface you asked. It advertised no tool `title` and no `annotations` at all, so a client
  that gates confirmation on `destructiveHint` saw nothing for the server that performs the writes.
  The `register` wrapper now fills all three from `getToolDefinition(name)`, spread after the call
  site's config so a stray literal cannot win, and throws on a name with no contract entry.

- The STDIO server passed `contract.annotations` raw. That field is a `Partial` stating only what
  differs from the default posture, so the five tools declaring one field advertised it without a
  `destructiveHint`, and the ~95 declaring none advertised no annotations -- both disagreeing with
  what the registry publishes for the same tool.

- The MINER server passed neither, and passed every schema as `SomeSchema.shape`. The SDK re-wraps a
  raw shape in a plain `z.object` that DISCARDS the catchall, and every miner output is a
  `looseObject`: all 21 were advertised and enforced as `additionalProperties: false`, so any field a
  payload carried beyond the modelled set came back as a -32602 the caller could do nothing about.
  #9762 fixed this class elsewhere and added a guard, but its roots omitted
  `packages/loopover-miner/bin` -- the guard reported none while the server it did not look at had 21.
  A `registerMinerTool` helper replaces all of them, and the guard now proves its own roots cover
  every file that registers a tool.

Also closes #9656 and #9657, which are the same registration path: the remote's descriptions come
from the contract now (its `MCP_TOOL_CATEGORIES` was already derived), and
`loopover_admin_rotate_secret` -- the one tool still registering from schemas declared in
`src/mcp/server.ts` -- reads its four siblings' contract entry instead. The reason it survived is
fixed too: every case booted a server without `LOOPOVER_MCP_ADMIN_ENABLED`, so the admin category was
never diffed, compiled, smoke-called or output-validated, and the validator's own "nothing is
registered without a contract entry" assertion was structurally unable to see the tool that violated
it. A second remote surface with the flag set now runs the full pass over all five.

`ToolContract` becomes generic in its input/output schemas so a server registering from an entry gets
that tool's real argument type in its handler rather than the erased `z.ZodObject`.

New invariant `checkAdvertisedMetadata` compares advertised title/description/posture against the
projection, and runs for all three surfaces. No tool's declared posture changed.

Author: JSONbored

packages/loopover-contract/src/index.ts

@@ -9,6 +9,7 @@ export {
   TOOL_LOCALITIES,
   TOOL_AVAILABILITIES,
   defineTool,
+  projectToolDefinition,
   projectToolDefinitions,
   toJsonSchema,
   type ToolCategory,

packages/loopover-contract/src/tool-definition.ts

@@ -62,7 +62,7 @@ export type ToolAnnotations = {
  * the wire-compatibility constraint metagraphed hit head-on when it tried to reuse its strict REST
  * schemas for MCP output and found the tighter contract was a regression.
  */
-export type ToolContract = {
+export type ToolContract<TInput extends z.ZodObject = z.ZodObject, TOutput extends z.ZodObject = z.ZodObject> = {
   name: string;
   title: string;
   description: string;
@@ -71,8 +71,11 @@ export type ToolContract = {
   locality: ToolLocality;
   availability: ToolAvailability;
   annotations?: Partial<ToolAnnotations>;
-  input: z.ZodObject;
-  output: z.ZodObject;
+  /** Generic so a server registering from a contract gets that tool's REAL argument type in its
+   *  handler, rather than the erased `z.ZodObject`. `TOOL_CONTRACTS` erases them again on the way
+   *  into the registry array, where only the common shape matters. */
+  input: TInput;
+  output: TOutput;
 };
 
 /** JSON Schema as emitted by `z.toJSONSchema` -- structurally open because draft-2020-12 allows
@@ -133,7 +136,18 @@ function matchesFilter(contract: ToolContract, filter: ToolFilter): boolean {
  * derives from this and never from the raw contract array.
  */
 export function projectToolDefinitions(contracts: readonly ToolContract[], filter: ToolFilter = {}): McpToolDefinition[] {
-  return contracts.filter((contract) => matchesFilter(contract, filter)).map((contract) => ({
+  return contracts.filter((contract) => matchesFilter(contract, filter)).map(projectToolDefinition);
+}
+
+/**
+ * The same projection for ONE contract, as a total function (#9655).
+ *
+ * A server registering a tool it has already resolved needs the defaulted `annotations` without a second
+ * lookup that can fail -- and the defaulting must not be re-implemented at the registration site, which is
+ * how the three servers came to advertise three different postures for one entry.
+ */
+export function projectToolDefinition(contract: ToolContract): McpToolDefinition {
+  return {
     name: contract.name,
     title: contract.title,
     description: contract.description,
@@ -144,7 +158,7 @@ export function projectToolDefinitions(contracts: readonly ToolContract[], filte
     annotations: { ...DEFAULT_ANNOTATIONS, ...contract.annotations },
     inputSchema: toJsonSchema(contract.input),
     outputSchema: toJsonSchema(contract.output),
-  }));
+  };
 }
 
 /**
@@ -155,6 +169,8 @@ export function projectToolDefinitions(contracts: readonly ToolContract[], filte
  * of those heterogeneous types collapses to an unsatisfiable intersection the moment anything
  * maps over it. metagraphed documents the same lesson on its own registry array.
  */
-export function defineTool(contract: ToolContract): ToolContract {
+export function defineTool<TInput extends z.ZodObject, TOutput extends z.ZodObject>(
+  contract: ToolContract<TInput, TOutput>,
+): ToolContract<TInput, TOutput> {
   return contract;
 }

packages/loopover-contract/src/tools/index.ts

@@ -3,7 +3,7 @@
 // Every MCP tool LoopOver serves, from any of its three servers, has exactly one entry here. A
 // runtime registers the slice it can actually serve by filtering on locality/availability -- it
 // does not keep its own list.
-import { projectToolDefinitions, type McpToolDefinition, type ToolContract, type ToolFilter } from "../tool-definition.js";
+import { projectToolDefinition, projectToolDefinitions, type McpToolDefinition, type ToolContract, type ToolFilter } from "../tool-definition.js";
 import { getRepoContextTool } from "./repo-context.js";
 import { getPrReviewabilityTool } from "./pr-reviewability.js";
 import { predictGateTool } from "./predict-gate.js";
@@ -303,6 +303,24 @@ export function getToolContract(name: string): ToolContract | undefined {
   return CONTRACTS_BY_NAME.get(name);
 }
 
+/**
+ * One tool's PROJECTED definition -- what a server must advertise for it (#9655).
+ *
+ * The raw contract is not what goes on the wire: `annotations` there is a `Partial` stating only what
+ * differs from the default posture, so a server reading it directly advertises `{ readOnlyHint: false }`
+ * with no `destructiveHint` for a tool that declares one field, and nothing at all for a tool that
+ * declares none. Three servers each doing that produced three different advertised tools from one entry.
+ * This is `listToolDefinitions()` for a single name, so the defaults are applied in exactly one place.
+ */
+export function getToolDefinition(name: string): McpToolDefinition | undefined {
+  const contract = CONTRACTS_BY_NAME.get(name);
+  return contract ? projectToolDefinition(contract) : undefined;
+}
+
+// The projection helpers, re-exported so a server registering from this entry point does not have to
+// import the root one as well just to apply the annotation defaults.
+export { projectToolDefinition, projectToolDefinitions, type McpToolDefinition, type ToolContract } from "../tool-definition.js";
+
 // Re-export each family wholesale so consumers can reach the individual input/output schemas (and
 // the shared sub-shapes like laneAdviceSchema) without importing deep paths.
 export * from "./repo-context.js";

packages/loopover-mcp/bin/loopover-mcp.ts

@@ -165,6 +165,7 @@ import {
   ValidateLinkedIssueInput,
   WatchIssuesInput,
   getToolContract,
+  projectToolDefinition,
   ListPendingActionsStdioInput,
 } from "@loopover/contract/tools";
 import { AUTONOMY_LEVELS as MAINTAIN_AUTONOMY_LEVELS, MAINTAIN_ACTION_CLASSES, PROPOSE_ACTION_CLASSES, type ToolContract } from "@loopover/contract";
@@ -897,20 +898,25 @@ function registerStdioTool<TInput>(
 ): void {
   const contract = getToolContract(name);
   if (!contract) throw new Error(`No @loopover/contract entry for stdio tool: ${name}`);
+  // The PROJECTED definition for everything advertised, so the defaults land once (#9655). Reading
+  // `contract.annotations` directly advertised the raw `Partial`: a tool declaring only
+  // `readOnlyHint: false` shipped without a `destructiveHint`, and a tool declaring none shipped with
+  // no annotations at all -- both disagreeing with what `listToolDefinitions()` publishes for it.
+  const advertised = projectToolDefinition(contract);
   locallyRegisteredToolNames.add(name);
   server.registerTool(
     name,
     {
-      title: contract.title,
-      description: contract.description,
+      title: advertised.title,
+      description: advertised.description,
       // The SCHEMA OBJECTS, not their `.shape`. The SDK accepts either, but a raw shape is
       // re-wrapped in a plain `z.object`, which silently discards the catchall -- so every output
       // modelled as a `looseObject` would be advertised and enforced as `additionalProperties:
       // false`, and any field the payload carries beyond the modelled set becomes a -32602 the
       // caller cannot do anything about. Passing the object preserves what the contract declared.
       inputSchema: overrides?.input ?? contract.input,
       outputSchema: contract.output,
-      ...(contract.annotations ? { annotations: contract.annotations } : {}),
+      annotations: advertised.annotations,
     },
     wrapStdioToolHandler(name, () => telemetryState().enabled, handler as (...args: unknown[]) => Promise<unknown>),
   );