feat(validate): check what each host sends against the published API spec

.github/workflows/api-changed.yml

@@ -0,0 +1,121 @@
+name: API changed
+
+# Runs when the docs repo publishes a new API spec (it sends a repository_dispatch
+# with the file's sha256), or by hand. Waits for www.zenrows.com/openapi.json to
+# carry that spec, then runs npm run validate, which checks every host's api:
+# against it. A failure opens (or updates) the issue "API change: hosts to
+# update" and posts to #proj-integrations; a clean run closes the issue.
+
+on:
+  repository_dispatch:
+    types: [api-spec-changed]
+  workflow_dispatch:
+    inputs:
+      sha256:
+        description: "sha256 of the spec to wait for (empty: don't wait)"
+        type: string
+        required: false
+
+permissions: {}
+
+# Two pings minutes apart (a push and a fix-up) run one after the other, so
+# they can't both open the issue.
+concurrency:
+  group: api-changed
+  cancel-in-progress: false
+
+env:
+  SPEC_URL: https://www.zenrows.com/openapi.json
+  ISSUE_TITLE: "API change: hosts to update"
+
+jobs:
+  check:
+    name: Check the hosts against the API spec
+    runs-on: ubuntu-latest
+    # The release environment holds the Slack webhook and deploys only from main.
+    environment: release
+    permissions:
+      contents: read
+      issues: write
+    env:
+      EXPECTED_SHA256: ${{ github.event.client_payload.sha256 || inputs.sha256 }}
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+        with:
+          persist-credentials: false
+      - uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0
+        with:
+          node-version: "22"
+          cache: npm
+      - run: npm ci
+
+      # The site deploys a few minutes after the push that sent the ping, so
+      # the published file can still be the old one for a while.
+      - name: Wait for the published spec
+        if: env.EXPECTED_SHA256 != ''
+        run: |
+          set -euo pipefail
+          for i in $(seq 1 30); do
+            actual=$(curl -fsS --max-time 20 "$SPEC_URL" | sha256sum | cut -d' ' -f1 || true)
+            if [ "$actual" = "$EXPECTED_SHA256" ]; then
+              echo "The published spec matches ($actual)."
+              exit 0
+            fi
+            echo "Published spec is $actual, waiting for $EXPECTED_SHA256 ($i/30)…"
+            sleep 60
+          done
+          echo "::error::The published spec didn't reach $EXPECTED_SHA256 within 30 minutes; checking against what's published now."
+
+      - name: Validate every host against it
+        id: validate
+        continue-on-error: true
+        run: |
+          set -o pipefail
+          rm -f .cache/openapi.json
+          npm run -s validate 2>&1 | tee validate.log
+
+      - name: Report
+        env:
+          GH_TOKEN: ${{ github.token }}
+          WEBHOOK: ${{ secrets.SLACK_WEBHOOK_INTEGRATIONS }}
+          MENTION: "<!subteam^S098279QJNA>"
+          FAILED: ${{ steps.validate.outcome == 'failure' }}
+          RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
+        run: |
+          set -euo pipefail
+          existing=$(gh issue list --search "\"$ISSUE_TITLE\" in:title" --state open --json number,title --jq ".[] | select(.title == \"$ISSUE_TITLE\") | .number" | head -1)
+          # Only a host out of step with the spec is the hosts' problem; anything
+          # else (the spec didn't download, a broken render) is this run's.
+          problems=$(grep '^❌ .*vars\.yaml api:' validate.log || true)
+
+          if [ "$FAILED" = "true" ] && [ -z "$problems" ]; then
+            echo "::error::validate failed for a reason other than the API spec; see the log above."
+            exit 1
+          fi
+          if [ "$FAILED" != "true" ]; then
+            echo "Every host matches the published spec."
+            if [ -n "$existing" ]; then
+              gh issue close "$existing" --comment "The hosts match the published API spec again ($RUN_URL)."
+            fi
+            exit 0
+          fi
+
+          fence='```'
+          body="The API spec changed, and npm run validate says a host sends something the API no longer takes:"$'\n\n'"$fence"$'\n'"$problems"$'\n'"$fence"$'\n\n'"From $RUN_URL. Fix the host, and this issue closes on the next clean run."
+          if [ -n "$existing" ]; then
+            gh issue edit "$existing" --body "$body"
+            issue_url=$(gh issue view "$existing" --json url --jq .url)
+          else
+            issue_url=$(gh issue create --title "$ISSUE_TITLE" --body "$body")
+          fi
+          echo "::error::$problems"
+
+          payload=$(jq -n --arg mention "$MENTION" --arg problems "$problems" --arg issue "$issue_url" '{
+            text: ("\($mention) :warning: The API spec changed and a host is out of date.\n" + $problems + "\n<\($issue)|Issue>")
+          }')
+          if [ -z "$WEBHOOK" ]; then
+            echo "::warning::No SLACK_WEBHOOK_INTEGRATIONS secret, so nothing was posted to Slack; the issue is $issue_url."
+          else
+            curl -fsS -X POST -H 'Content-type: application/json' --data "$payload" "$WEBHOOK" >/dev/null
+          fi
+          exit 1

CONTRIBUTING.md

@@ -49,6 +49,18 @@ A fact that more than one place states (what an error code means, what a request
 
 `kind: code` in `vars.yaml` makes a host whose output isn't a plugin but files for somewhere else: code for another project (a piece as it sits in that project's repo), or a listing a marketplace reads. A code host ships only its own files, with `{{…}}` filled in source files too: no shared skills, `LICENSE` or assets, and no `plugin.json`. Its `README.md` documents it here and isn't shipped or rendered; `npm run validate` requires one, and the render fails if the host has a `skills/` folder. It needs no release-please entry until it's released from this repo.
 
+## Keeping hosts current with the API
+
+A code host that calls the API lists what it sends in `vars.yaml`, operation by operation:
+
+```yaml
+api:
+  GET /: [url, mode, js_instructions]
+  GET /subscriptions/self/details: []
+```
+
+`npm run validate` checks it against the published spec (`https://www.zenrows.com/openapi.json`, the last download kept in `.cache/openapi.json` for when the network is down; `ZENROWS_OPENAPI=<file>` uses a file instead): the operation exists, each parameter is one it takes, and each name appears in the host's rendered files, so the list can't go stale and a mistyped name in the code shows up as not sent. When the spec changes, the docs repo pings this one and `.github/workflows/api-changed.yml` runs the same check: on a failure it opens (or updates) the issue "API change: hosts to update" and posts to #proj-integrations; a clean run closes the issue.
+
 ## Tests
 
 `npm test` (CI runs it too) checks that render and validate catch the mistakes they're meant to: each case in `scripts/test.mjs` makes one change in a temporary copy of the repo (a throwaway code host, a snippet, …) and expects a given error, or a pass. When you add a rule, add a case that breaks it.

scripts/lib.mjs

@@ -1,8 +1,8 @@
 // Shared by render.mjs, validate.mjs and override.mjs: where things live, how a
 // host's plugin is rendered, and how override hashes are computed.
 import { createHash } from "node:crypto";
-import { existsSync, readdirSync, readFileSync, statSync } from "node:fs";
-import { join, relative } from "node:path";
+import { existsSync, mkdirSync, readdirSync, readFileSync, statSync, writeFileSync } from "node:fs";
+import { dirname, join, relative } from "node:path";
 import { fileURLToPath } from "node:url";
 import { parse as parseYaml } from "yaml";
 
@@ -71,6 +71,48 @@ export function vars(host) {
   return version ? { ...v, version } : v;
 }
 
+// The published API spec, which validate checks each host's api: against.
+// ZENROWS_OPENAPI names a file to use instead (tests, a spec not published
+// yet). The last download is kept in .cache/openapi.json for when the network
+// is down.
+export const openapiUrl = "https://www.zenrows.com/openapi.json";
+const openapiCache = join(root, ".cache", "openapi.json");
+export async function openapi() {
+  if (process.env.ZENROWS_OPENAPI) return JSON.parse(readFileSync(process.env.ZENROWS_OPENAPI, "utf8"));
+  try {
+    const res = await fetch(openapiUrl, { signal: AbortSignal.timeout(20000) });
+    if (!res.ok) throw new Error(`HTTP ${res.status}`);
+    const text = await res.text();
+    const spec = JSON.parse(text);
+    mkdirSync(dirname(openapiCache), { recursive: true });
+    writeFileSync(openapiCache, text);
+    return spec;
+  } catch (e) {
+    if (!existsSync(openapiCache)) {
+      throw new Error(`couldn't download the API spec from ${openapiUrl} (${e.message}), and there's no copy in .cache/openapi.json yet`);
+    }
+    console.warn(`⚠️ couldn't download the API spec from ${openapiUrl} (${e.message}); using the copy in .cache/openapi.json`);
+    return JSON.parse(readFileSync(openapiCache, "utf8"));
+  }
+}
+
+// The query parameters an operation ("GET /") takes: the path's and the
+// operation's own, with $refs resolved. undefined if the spec has no such operation.
+export function apiParameters(spec, operation) {
+  const [method, path] = operation.trim().split(/\s+/);
+  const item = spec.paths?.[path];
+  const op = method && item?.[method.toLowerCase()];
+  if (!op) return undefined;
+  const resolve = (p) => {
+    while (p?.$ref) p = p.$ref.replace(/^#\//, "").split("/").reduce((o, k) => o?.[k], spec);
+    return p;
+  };
+  return [...(item.parameters ?? []), ...(op.parameters ?? [])]
+    .map(resolve)
+    .filter((p) => p?.in === "query")
+    .map((p) => p.name);
+}
+
 // overrides.lock: "<shared path>: <sha256>" per line; # starts a comment.
 export function readLock(host) {
   const file = join(platformsDir, host, "overrides.lock");

scripts/test.mjs

@@ -26,7 +26,10 @@ const piece = {
 // Runs render then validate in a copy of the repo with `files` written over it
 // (null deletes). Returns { ok, out, dist }: out is the failing step's output,
 // dist(path) reads a rendered file.
-function run(files) {
+// The API spec validate reads: the last download if there is one, so the tests
+// don't need the network; a case can name its own file in env.ZENROWS_OPENAPI.
+const cachedSpec = join(root, ".cache", "openapi.json");
+function run(files, env = {}) {
   const dir = mkdtempSync(join(tmpdir(), "zenrows-integrations-test-"));
   try {
     for (const path of copied) {
@@ -44,7 +47,11 @@ function run(files) {
       }
     }
     for (const script of ["render", "validate"]) {
-      const r = spawnSync("node", [join("scripts", `${script}.mjs`)], { cwd: dir, encoding: "utf8" });
+      const r = spawnSync("node", [join("scripts", `${script}.mjs`)], {
+        cwd: dir,
+        encoding: "utf8",
+        env: { ...process.env, ...(existsSync(cachedSpec) ? { ZENROWS_OPENAPI: cachedSpec } : {}), ...env },
+      });
       if (r.status !== 0) return { ok: false, out: `${r.stdout}${r.stderr}` };
     }
     const rendered = new Map(Object.keys(files).filter((p) => p.startsWith("platforms/")).map((p) => {
@@ -57,12 +64,12 @@ function run(files) {
   }
 }
 
-const passes = (files) => () => {
-  const r = run(files);
+const passes = (files, env) => () => {
+  const r = run(files, env);
   assert.ok(r.ok, r.out);
 };
-const fails = (files, message) => () => {
-  const r = run(files);
+const fails = (files, message, env) => () => {
+  const r = run(files, env);
   assert.ok(!r.ok, "expected a failure, but render and validate passed");
   assert.ok(r.out.includes(message), `expected "${message}" in:\n${r.out}`);
 };
@@ -120,6 +127,23 @@ test("an unknown kind fails", () => {
 });
 test("a vars.yaml that isn't name: value lines fails", fails({ ...piece, "platforms/piece/vars.yaml": "- host_full: Piece\n" }, "platforms/piece/vars.yaml must be a list of name: value lines"));
 
+// api: in vars.yaml, checked against a small spec stub instead of the published one.
+const stubSpec = {
+  "openapi-stub.json": JSON.stringify({
+    openapi: "3.1.0",
+    paths: { "/": { parameters: [{ $ref: "#/components/parameters/mode" }, { name: "url", in: "query" }], get: {} } },
+    components: { parameters: { mode: { name: "mode", in: "query" } } },
+  }),
+};
+const withSpec = { ZENROWS_OPENAPI: "openapi-stub.json" };
+const sends = (api, code) => ({ ...piece, ...stubSpec, "platforms/piece/vars.yaml": `host_full: Piece\nkind: code\napi:\n  ${api}\n`, "platforms/piece/src/index.ts": `${code}\n` });
+test("a host's api: passes when the API takes each parameter and the host sends it", passes(sends('GET /: [url, mode]', 'export const q = { url: "https://example.com", mode: "auto" };'), withSpec));
+test("an api: operation the API doesn't have fails", fails(sends('GET /nope: []', 'export const q = {};'), 'the API has no operation "GET /nope"', withSpec));
+test("an api: parameter the operation doesn't take fails", fails(sends('GET /: [foo]', 'export const q = { foo: 1 };'), 'GET / doesn\'t take "foo"', withSpec));
+test("an api: parameter sent as a JavaScript shorthand property passes", passes(sends('GET /: [url, mode]', 'export const q = ({ url, mode }) => ({ url, mode });'), withSpec));
+test("an api: operation whose value isn't a list fails", fails(sends('GET /: url', 'export const q = { url: "x" };'), 'GET / must list parameter names', withSpec));
+test("an api: parameter the host never sends fails", fails(sends('GET /: [url, mode]', 'export const q = { mode: "auto" };'), 'declares "url" for GET /, but no file in dist/piece sends it', withSpec));
+
 // {{…}} that isn't one of this tool's tokens ships as written.
 const theirs = {
   // JSX, with a string, a number and a variable value, spaced or not.

scripts/validate.mjs

@@ -5,9 +5,20 @@ import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
 import { basename, dirname, join } from "node:path";
 import Ajv2020 from "ajv/dist/2020.js";
 import { parse as parseYaml } from "yaml";
-import { catalogs, hostSkillName, hostSkills, hosts, isCodeHost, listFiles, platformsDir, readLock, renderHost, root, sha256, sharedTextDir, skillNames, textFile, vars } from "./lib.mjs";
+import { apiParameters, catalogs, hostSkillName, hostSkills, hosts, isCodeHost, listFiles, openapi, platformsDir, readLock, renderHost, root, sha256, sharedTextDir, skillNames, textFile, vars } from "./lib.mjs";
 
 const errors = [];
+
+// The API spec, downloaded once, and only if a host declares api:.
+let spec;
+const apiSpec = async () => (spec ??= await openapi());
+
+// Whether a host's files send a parameter: as a key (name: …, name = …), a
+// quoted string, or a JavaScript shorthand property ({ url, mode }).
+function sendsParameter(text, name) {
+  const n = name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
+  return new RegExp(`(?<![\\w-])${n}(?![\\w-])\\s*[:=]|["'\`]${n}["'\`]|[{,]\\s*${n}\\s*[,}]`).test(text);
+}
 const report = [];
 
 // Every name a skill has anywhere: shared names, each host's own skills, and
@@ -236,6 +247,33 @@ for (const host of hosts()) {
     if (text.includes("TODO(new-host)")) errors.push(`${host}: platforms/${host}/${file} still has a TODO(new-host)`);
   }
 
+  // api: in vars.yaml lists the API operations and parameters the host sends.
+  // Each operation must exist in the published spec, each parameter must be one
+  // it takes, and each name must appear in the host's own files, so the list
+  // can't go stale and a mistyped name in the code shows up as "unused".
+  if (values.api !== undefined) {
+    if (typeof values.api !== "object" || Array.isArray(values.api)) {
+      errors.push(`${host}: vars.yaml api: must map "METHOD /path" to a list of parameter names`);
+    } else {
+      const sent = [...files].filter(([f]) => textFile.test(f)).map(([, buf]) => buf.toString("utf8")).join("\n");
+      for (const [operation, names] of Object.entries(values.api)) {
+        const takes = apiParameters(await apiSpec(), operation);
+        if (!takes) {
+          errors.push(`${host}: vars.yaml api: the API has no operation "${operation}"`);
+          continue;
+        }
+        if (names != null && !Array.isArray(names)) {
+          errors.push(`${host}: vars.yaml api: ${operation} must list parameter names, for example [url, mode]`);
+          continue;
+        }
+        for (const name of names ?? []) {
+          if (!takes.includes(name)) errors.push(`${host}: vars.yaml api: ${operation} doesn't take "${name}"`);
+          else if (!sendsParameter(sent, name)) errors.push(`${host}: vars.yaml api: declares "${name}" for ${operation}, but no file in dist/${host} sends it`);
+        }
+      }
+    }
+  }
+
   // A code host's README stays here (it isn't shipped) and the repo's hosts
   // list links to it.
   if (isCodeHost(values)) {