types(mcp): narrow the CLI's closed-set guards, and discover the contract's modules (#9773)

Two things the merge with #9762 exposed, now that the action-class and autonomy-level lists are
readonly literal tuples rather than `string[]`:

- The CLI validated `<action>` and `<level>` with `LIST.includes(value)` and then passed the still-
  `string` value to a typed request. `includes` returns a boolean and narrows nothing, so the check
  ran and the type system learned nothing from it. `isOneOf` is the same check written as a type
  predicate, so a validated value arrives at the API as the union it was just proved to be.

- The generator resolved a copied schema's constants against a hardcoded pair of contract modules.
  That is a hand-maintained list by another name, and it fails in the quietest way available: a
  constant that moves between modules yields a generated file referencing a name it never imported.
  It now reads the contract's source directory, so a constant can move -- or a module can appear --
  without this script knowing anything about it.

Regression test pins the discovery against wherever PUBLIC_SURFACE_SKIP_REASONS lives, rather than
against the module it happens to live in today.

Author: JSONbored

packages/loopover-contract/src/api-schemas.ts

@@ -9,6 +9,7 @@
 import { z } from "zod";
 
 import { checkBeforeStartSchema, slopRiskSchema, validateFocusManifestSchema, validateLinkedIssueSchema } from "./api-requests.js";
+import { AGENT_ACTION_CLASSES, AUTONOMY_LEVELS } from "./enums.js";
 import { MAX_CONTRIBUTOR_OPEN_ITEM_CAP, MAX_REVIEW_NAG_COOLDOWN_DAYS } from "./limits.js";
 
 export const FindingSchema = z
@@ -581,8 +582,8 @@ export const RepositorySettingsSchema = z
     // exact drift class #9517's enum notes warned about, republished here. The compile-time parity
     // assertion in src/openapi/schema-type-parity.ts is what finally caught it.
     autonomy: z.partialRecord(
-      z.enum(["review", "request_changes", "approve", "merge", "close", "label", "review_state_label", "update_branch", "assign"]),
-      z.enum(["observe", "auto_with_approval", "auto"]),
+      z.enum(AGENT_ACTION_CLASSES),
+      z.enum(AUTONOMY_LEVELS),
     ),
     autoMaintain: z.object({ requireApprovals: z.number().int(), mergeMethod: z.enum(["merge", "squash", "rebase"]) }).optional(),
     agentPaused: z.boolean().optional(),

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

@@ -2801,7 +2801,7 @@ export async function maintainCli(args: readonly string[]) {
     if (!actionClass || !pullArg) {
       throw new Error("Usage: loopover-mcp maintain propose <action-class> <pull-number> --repo owner/repo [--reason ...] [--label ...] [--review-body ...] [--merge-method merge|squash|rebase] [--close-comment ...].");
     }
-    if (!PROPOSE_ACTION_CLASSES.includes(actionClass)) throw new Error(`Unknown action class: ${actionClass}. Use ${PROPOSE_ACTION_CLASSES.join(", ")}.`);
+    if (!isOneOf(PROPOSE_ACTION_CLASSES, actionClass)) throw new Error(`Unknown action class: ${actionClass}. Use ${PROPOSE_ACTION_CLASSES.join(", ")}.`);
     const pullNumber = Number(pullArg);
     if (!Number.isInteger(pullNumber) || pullNumber <= 0) throw new Error(`Invalid pull number: ${pullArg}. Pass a positive integer.`);
     const payload = await apiPost(
@@ -2824,8 +2824,8 @@ export async function maintainCli(args: readonly string[]) {
     const action = args[1] && !args[1].startsWith("--") ? args[1] : undefined;
     const level = args[2] && !args[2].startsWith("--") ? args[2] : undefined;
     if (!action || !level) throw new Error("Usage: loopover-mcp maintain set-level <action> <level> --repo owner/repo.");
-    if (!MAINTAIN_ACTION_CLASSES.includes(action)) throw new Error(`Unknown action: ${action}. Use ${MAINTAIN_ACTION_CLASSES.join(", ")}.`);
-    if (!MAINTAIN_AUTONOMY_LEVELS.includes(level)) throw new Error(`Unknown level: ${level}. Use ${MAINTAIN_AUTONOMY_LEVELS.join(", ")}.`);
+    if (!isOneOf(MAINTAIN_ACTION_CLASSES, action)) throw new Error(`Unknown action: ${action}. Use ${MAINTAIN_ACTION_CLASSES.join(", ")}.`);
+    if (!isOneOf(MAINTAIN_AUTONOMY_LEVELS, level)) throw new Error(`Unknown level: ${level}. Use ${MAINTAIN_AUTONOMY_LEVELS.join(", ")}.`);
     // Read-merge-write so one class is updated without clearing the others.
     const current = await apiGet(`${repoBase}/settings`);
     const autonomy = { ...(current.autonomy ?? {}), [action]: level };
@@ -4368,6 +4368,17 @@ function printProfileHelp() {
   process.stdout.write(printableUsage("profile"));
 }
 
+/**
+ * Whether a user-supplied string is one of a closed set (#9773).
+ *
+ * A TYPE PREDICATE, so the value narrows for whatever it is passed to next. `list.includes(value)` returns
+ * a boolean and narrows nothing, which is why an action class or autonomy level still arrived at the API
+ * as a plain `string` -- the check ran, and the type system learned nothing from it.
+ */
+function isOneOf<const T extends readonly string[]>(list: T, value: string): value is T[number] {
+  return (list as readonly string[]).includes(value);
+}
+
 /**
  * An option's value as text (#9773).
  *

scripts/gen-contract-api-schemas.ts

@@ -17,7 +17,7 @@
 // by construction -- and the contract copy is generated with the names stripped, which the CLI does not
 // need: it only parses and infers. `--check` in test:ci is what makes the copy safe, exactly like
 // gen-selfhost-env-reference.ts and gen-command-reference.ts.
-import { readFileSync, writeFileSync } from "node:fs";
+import { readdirSync, readFileSync, writeFileSync } from "node:fs";
 import { fileURLToPath } from "node:url";
 
 const SOURCE = "src/openapi/schemas.ts";
@@ -310,11 +310,7 @@ export function renderApiSchemas(sourceText: string, documentText: string, binSo
   const staticCalls = cliApiCalls(binSource);
   const byRequest = new Map([...requestSchemaByCall(document, staticCalls), ...requestSchemaByCall(document, parameterisedCalls)]);
   const blocks = closure(parseSchemaBlocks(sourceText), [...new Set([...byPath.values(), ...byPattern.values(), ...byRequest.values()])]);
-  const exportsByModule = new Map<string, ReadonlySet<string>>([
-    ["./limits.js", new Set(exportedNames(readModule("packages/loopover-contract/src/limits.ts")))],
-    ["./api-requests.js", new Set(exportedNames(readModule("packages/loopover-contract/src/api-requests.ts")))],
-  ]);
-  const externals = referencedExternals(blocks, exportsByModule);
+  const externals = referencedExternals(blocks, contractModuleExports());
   const body = blocks
     .map((block) =>
       block.source
@@ -426,16 +422,39 @@ export type ParameterisedApiResponse<Method extends string, Path extends string>
 >;
 `;
 
+const CONTRACT_SRC = "packages/loopover-contract/src";
+
+/**
+ * What every sibling contract module exports, DISCOVERED rather than listed (#9773).
+ *
+ * A hardcoded module list is a hand-maintained list by another name, and it fails in the quietest way
+ * there is: `PUBLIC_SURFACE_SKIP_REASONS` moved between two contract modules, and a generator that only
+ * knew about the module it left would have emitted a file referencing a name it never imported -- valid
+ * output, broken build. Reading the directory means a constant can move, or a new module can appear,
+ * without this script knowing anything about it.
+ *
+ * `index.ts` is excluded because it re-exports the generated file (importing it back would be a cycle),
+ * and the generated file itself because a schema cannot import its own copy.
+ */
+export function contractModuleExports(): Map<string, ReadonlySet<string>> {
+  const modules = listModules(CONTRACT_SRC)
+    .filter((file) => file.endsWith(".ts") && file !== "index.ts" && file !== "api-schemas.ts")
+    .sort();
+  return new Map(modules.map((file) => [`./${file.replace(/\.ts$/, ".js")}`, new Set(exportedNames(readModule(`${CONTRACT_SRC}/${file}`)))]));
+}
+
 /** The names a module exports, for resolving what a copied schema references. */
 export function exportedNames(source: string): string[] {
   return [...source.matchAll(/^export (?:const|function|type) ([A-Za-z_][A-Za-z0-9_]*)/gm)].map((match) => match[1]!);
 }
 
 let readModule: (path: string) => string = (path) => readFileSync(path, "utf8");
+let listModules: (dir: string) => string[] = (dir) => readdirSync(dir);
 
-export function generate(deps: { readFile?: (path: string) => string } = {}): string {
+export function generate(deps: { readFile?: (path: string) => string; listDir?: (dir: string) => string[] } = {}): string {
   const readFile = deps.readFile ?? ((path: string) => readFileSync(path, "utf8"));
   readModule = readFile;
+  listModules = deps.listDir ?? ((dir: string) => readdirSync(dir));
   return renderApiSchemas(readFile(SOURCE), readFile(OPENAPI_DOCUMENT), readFile(CLI_BIN));
 }
 

test/unit/mcp-api-client.test.ts

@@ -8,6 +8,8 @@ import {
   closure,
   declaredPathShapes,
   parseSchemaBlocks,
+  contractModuleExports,
+  referencedExternals,
   referencedLimits,
   responseSchemaByCall,
   responseSchemaByPath,
@@ -149,6 +151,21 @@ describe("parameterised response schemas (#9773)", () => {
     expect(referencedLimits(blocks), "one declared elsewhere is imported").toContain("MAX_REVIEW_NAG_COOLDOWN_DAYS");
   });
 
+  it("discovers which contract module a referenced constant lives in, so it may move", () => {
+    // A hardcoded module list fails silently when a constant relocates: PUBLIC_SURFACE_SKIP_REASONS moved
+    // between two contract modules, and a generator that still only knew the old one would have emitted a
+    // file referencing a name it never imported -- valid TypeScript, broken build.
+    const modules = contractModuleExports();
+    expect([...modules.keys()], "index.js would import this file back").not.toContain("./index.js");
+    expect([...modules.keys()], "the generated file cannot import its own copy").not.toContain("./api-schemas.js");
+
+    const owner = [...modules].find(([, names]) => names.has("PUBLIC_SURFACE_SKIP_REASONS"));
+    expect(owner?.[0], "wherever it lives today, it is found there").toMatch(/^\.\/[a-z-]+\.js$/);
+    expect(referencedExternals([{ name: "XSchema", exported: true, source: "const XSchema = z.enum(PUBLIC_SURFACE_SKIP_REASONS);" }], modules).get(owner![0])).toEqual([
+      "PUBLIC_SURFACE_SKIP_REASONS",
+    ]);
+  });
+
   it("does not mistake a capitalised word in prose for a constant", () => {
     // The first cut scanned comments too and emitted an import for DELETE, REQUIRED, REST and friends.
     expect(referencedLimits([{ name: "XSchema", exported: true, source: '// DELETE and REQUIRED and REST\nconst XSchema = z.string();' }])).toEqual([]);