ci: block removing a parameter from a model

.github/workflows/param-guard.yml

@@ -0,0 +1,47 @@
+name: Param guard
+
+# Blocks pull requests that remove a parameter from a model that still exists.
+# Removing a parameter is breaking: every Manifest user who already configured it
+# loses the ability to see or change that setting.
+#
+# Override: a maintainer applies the `allow-param-removal` label to the PR. The
+# label change re-triggers this workflow (see `types` below) and the guard step
+# is skipped, leaving an auditable warning in the checks output.
+
+on:
+  pull_request:
+    branches: [main]
+    types: [opened, synchronize, reopened, labeled, unlabeled]
+
+jobs:
+  no-param-removals:
+    name: No parameter removals
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check out PR
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      - name: Fetch base branch
+        run: git fetch --no-tags origin "${{ github.base_ref }}"
+
+      - name: Set up Node
+        uses: actions/setup-node@v4
+        with:
+          node-version: "20"
+          cache: "npm"
+
+      - name: Install dependencies
+        run: npm ci
+
+      - name: Guard against parameter removals
+        if: ${{ !contains(github.event.pull_request.labels.*.name, 'allow-param-removal') }}
+        run: npm run guard:params
+        env:
+          BASE_REF: "origin/${{ github.base_ref }}"
+
+      - name: Removal override acknowledged
+        if: ${{ contains(github.event.pull_request.labels.*.name, 'allow-param-removal') }}
+        run: |
+          echo "::warning::'allow-param-removal' label present — parameter-removal guard intentionally skipped for this PR."

CONTRIBUTING.md

@@ -103,6 +103,32 @@ params:
         thinking.type: enabled
 ```
 
+## Removing parameters is blocked
+
+Once a parameter is published for a model, **it cannot be removed**. People using
+that model in [Manifest](https://manifest.build/) may already have the parameter
+configured; dropping it from the catalog takes away their ability to see or change
+that setting and breaks their setup.
+
+CI enforces this. The `Param guard` workflow (`npm run guard:params`) compares your
+PR against `main` and **fails if any parameter `path` that exists on a model is gone**
+— this includes renaming a `path` (the old name counts as removed). You can run the
+same check locally before opening a PR:
+
+```bash
+npm run guard:params            # compares against origin/main
+npm run guard:params -- --base <ref>   # compare against a specific ref
+```
+
+What is _not_ blocked: adding new parameters, editing a parameter's metadata
+(label, description, default, range, values, applicability), and removing a whole
+model file. Only the disappearance of a `path` from a model that still exists is
+treated as a breaking removal.
+
+If a removal is genuinely necessary (e.g. a parameter was added by mistake), a
+maintainer must add the **`allow-param-removal`** label to the PR. The label
+re-runs the check and skips the guard, leaving a visible warning on the run.
+
 ## Site changes
 
 The website code lives under `src/`:

package.json

@@ -20,6 +20,7 @@
     "build": "tsx src/build/build.ts",
     "dev": "tsx watch src/server/dev.ts",
     "validate": "tsx src/data/validate.ts",
+    "guard:params": "tsx src/data/check-removals.ts",
     "typecheck": "tsc --noEmit",
     "lint": "eslint . --ext .ts,.tsx",
     "format": "prettier --write .",

src/data/check-removals.ts

@@ -0,0 +1,116 @@
+import { execFileSync } from "node:child_process";
+import fs from "node:fs/promises";
+import os from "node:os";
+import path from "node:path";
+import type { Model } from "../schema/model.js";
+import { loadAllModels } from "./load.js";
+import { REPO_ROOT } from "./paths.js";
+import { findRemovedParams, type ParamRemoval } from "./removals.js";
+
+const OVERRIDE_LABEL = "allow-param-removal";
+
+function git(args: string[]): string {
+  return execFileSync("git", args, {
+    cwd: REPO_ROOT,
+    encoding: "utf8",
+    maxBuffer: 64 * 1024 * 1024,
+  });
+}
+
+function refExists(ref: string): boolean {
+  try {
+    git(["rev-parse", "--verify", "--quiet", `${ref}^{commit}`]);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
+function argBase(): string | undefined {
+  const eq = process.argv.find((a) => a.startsWith("--base="));
+  if (eq) return eq.slice("--base=".length);
+  const i = process.argv.indexOf("--base");
+  if (i >= 0 && process.argv[i + 1]) return process.argv[i + 1];
+  return undefined;
+}
+
+/** Resolve the base ref to diff against, trying the most specific source first. */
+function resolveBaseRef(): string | null {
+  const githubBase = process.env.GITHUB_BASE_REF
+    ? `origin/${process.env.GITHUB_BASE_REF}`
+    : undefined;
+  const candidates = [argBase(), process.env.BASE_REF, githubBase, "origin/main", "main"];
+  for (const ref of candidates) {
+    if (ref && refExists(ref)) return ref;
+  }
+  return null;
+}
+
+/** Materialize the `models/` tree at `ref` into a temp dir and load it. */
+async function loadModelsAtRef(ref: string): Promise<Model[]> {
+  const listing = git(["ls-tree", "-r", "--name-only", ref, "--", "models"]).trim();
+  const files = listing ? listing.split("\n").filter((f) => /\.ya?ml$/i.test(f)) : [];
+  if (files.length === 0) return [];
+
+  const tmp = await fs.mkdtemp(path.join(os.tmpdir(), "mp-baseline-"));
+  try {
+    for (const file of files) {
+      const content = git(["show", `${ref}:${file}`]);
+      const dest = path.join(tmp, ...file.split("/"));
+      await fs.mkdir(path.dirname(dest), { recursive: true });
+      await fs.writeFile(dest, content, "utf8");
+    }
+    const { models } = await loadAllModels(path.join(tmp, "models"));
+    return models;
+  } finally {
+    await fs.rm(tmp, { recursive: true, force: true });
+  }
+}
+
+function reportRemovals(removals: ParamRemoval[], baseRef: string): void {
+  console.error(`\n✖ Parameter removal detected vs ${baseRef} — blocked.\n`);
+  console.error(
+    "Removing a parameter breaks every Manifest user who already has it\n" +
+      "configured: they lose the ability to see or change that setting.\n",
+  );
+  console.error("Parameters present on the base branch but missing in this change:\n");
+  let lastModel = "";
+  for (const removal of removals) {
+    if (removal.modelId !== lastModel) {
+      console.error(`  ${removal.modelId}`);
+      lastModel = removal.modelId;
+    }
+    console.error(`    - ${removal.path}`);
+  }
+  console.error(
+    `\nIf this removal is genuinely intended, a maintainer must add the` +
+      `\n\`${OVERRIDE_LABEL}\` label to the PR, then re-run this check.\n`,
+  );
+}
+
+async function main(): Promise<void> {
+  const baseRef = resolveBaseRef();
+  if (!baseRef) {
+    console.log("No base ref available to compare against — skipping removal guard.");
+    return;
+  }
+
+  const [{ models: current }, base] = await Promise.all([
+    loadAllModels(),
+    loadModelsAtRef(baseRef),
+  ]);
+
+  const removals = findRemovedParams(base, current);
+  if (removals.length === 0) {
+    console.log(`OK — no parameters removed vs ${baseRef}.`);
+    return;
+  }
+
+  reportRemovals(removals, baseRef);
+  process.exit(1);
+}
+
+main().catch((err) => {
+  console.error("Removal guard crashed:", err);
+  process.exit(2);
+});

src/data/removals.ts

@@ -0,0 +1,51 @@
+import { modelId, type Model } from "../schema/model.js";
+
+export interface ParamRemoval {
+  modelId: string;
+  path: string;
+}
+
+/**
+ * Build an index of `modelId -> set of parameter paths` declared for that model.
+ * Paths from duplicate model ids (should not happen in a valid catalog) are
+ * unioned so we never under-count what a model exposes.
+ */
+export function buildParamIndex(models: Model[]): Map<string, Set<string>> {
+  const index = new Map<string, Set<string>>();
+  for (const model of models) {
+    const id = modelId(model);
+    const paths = index.get(id) ?? new Set<string>();
+    for (const param of model.params) {
+      paths.add(param.path);
+    }
+    index.set(id, paths);
+  }
+  return index;
+}
+
+/**
+ * Find parameters that exist on `base` but are gone on `current`.
+ *
+ * Scope (intentional): only models that STILL EXIST in `current` are checked.
+ * Dropping a whole model (its id no longer appears) is not reported here — that
+ * is governed by a separate policy decision. The harm we guard against is a live
+ * model silently losing a knob that consumers already have configured.
+ */
+export function findRemovedParams(base: Model[], current: Model[]): ParamRemoval[] {
+  const baseIndex = buildParamIndex(base);
+  const currentIndex = buildParamIndex(current);
+  const removals: ParamRemoval[] = [];
+
+  for (const [id, basePaths] of baseIndex) {
+    const currentPaths = currentIndex.get(id);
+    if (!currentPaths) continue; // whole model gone — out of scope by policy
+    for (const path of basePaths) {
+      if (!currentPaths.has(path)) {
+        removals.push({ modelId: id, path });
+      }
+    }
+  }
+
+  removals.sort((a, b) => a.modelId.localeCompare(b.modelId) || a.path.localeCompare(b.path));
+  return removals;
+}

tests/removals.test.ts

@@ -0,0 +1,103 @@
+import { describe, expect, it } from "vitest";
+import { buildParamIndex, findRemovedParams } from "../src/data/removals.js";
+import type { AuthType, Model, Parameter } from "../src/schema/model.js";
+
+function param(path: string): Parameter {
+  return { path, type: "number", label: "L", description: "d", group: "sampling" };
+}
+
+function model(provider: string, authType: AuthType, name: string, paths: string[]): Model {
+  return { provider, authType, model: name, params: paths.map(param) };
+}
+
+describe("findRemovedParams", () => {
+  it("flags a parameter removed from a model that still exists", () => {
+    const base = [model("anthropic", "api_key", "opus", ["max_tokens", "temperature"])];
+    const current = [model("anthropic", "api_key", "opus", ["max_tokens"])];
+
+    expect(findRemovedParams(base, current)).toEqual([
+      { modelId: "anthropic/opus", path: "temperature" },
+    ]);
+  });
+
+  it("treats a renamed parameter path as a removal", () => {
+    const base = [model("openai", "api_key", "gpt", ["max_tokens"])];
+    const current = [model("openai", "api_key", "gpt", ["maxTokens"])];
+
+    expect(findRemovedParams(base, current)).toEqual([
+      { modelId: "openai/gpt", path: "max_tokens" },
+    ]);
+  });
+
+  it("does not flag newly added parameters", () => {
+    const base = [model("openai", "api_key", "gpt", ["max_tokens"])];
+    const current = [model("openai", "api_key", "gpt", ["max_tokens", "temperature"])];
+
+    expect(findRemovedParams(base, current)).toEqual([]);
+  });
+
+  it("does not flag a whole model that was removed (out of scope by policy)", () => {
+    const base = [
+      model("anthropic", "api_key", "opus", ["max_tokens"]),
+      model("anthropic", "api_key", "haiku", ["temperature"]),
+    ];
+    const current = [model("anthropic", "api_key", "opus", ["max_tokens"])];
+
+    expect(findRemovedParams(base, current)).toEqual([]);
+  });
+
+  it("does not flag a brand-new model", () => {
+    const base = [model("openai", "api_key", "gpt", ["max_tokens"])];
+    const current = [
+      model("openai", "api_key", "gpt", ["max_tokens"]),
+      model("openai", "api_key", "gpt-mini", ["temperature"]),
+    ];
+
+    expect(findRemovedParams(base, current)).toEqual([]);
+  });
+
+  it("does not care about parameter ordering", () => {
+    const base = [model("openai", "api_key", "gpt", ["a", "b", "c"])];
+    const current = [model("openai", "api_key", "gpt", ["c", "a", "b"])];
+
+    expect(findRemovedParams(base, current)).toEqual([]);
+  });
+
+  it("treats api_key and subscription as separate models", () => {
+    const base = [
+      model("anthropic", "api_key", "opus", ["temperature"]),
+      model("anthropic", "subscription", "opus", ["temperature"]),
+    ];
+    const current = [
+      model("anthropic", "api_key", "opus", ["temperature"]),
+      model("anthropic", "subscription", "opus", []),
+    ];
+
+    expect(findRemovedParams(base, current)).toEqual([
+      { modelId: "anthropic/opus-subscription", path: "temperature" },
+    ]);
+  });
+
+  it("reports multiple removals sorted by model id then path", () => {
+    const base = [
+      model("openai", "api_key", "gpt", ["a", "z"]),
+      model("anthropic", "api_key", "opus", ["m", "n"]),
+    ];
+    const current = [
+      model("openai", "api_key", "gpt", ["a"]),
+      model("anthropic", "api_key", "opus", ["m"]),
+    ];
+
+    expect(findRemovedParams(base, current)).toEqual([
+      { modelId: "anthropic/opus", path: "n" },
+      { modelId: "openai/gpt", path: "z" },
+    ]);
+  });
+});
+
+describe("buildParamIndex", () => {
+  it("collects parameter paths per model id", () => {
+    const index = buildParamIndex([model("openai", "api_key", "gpt", ["a", "b"])]);
+    expect(index.get("openai/gpt")).toEqual(new Set(["a", "b"]));
+  });
+});