fix: leave the container limit to the API, and release a spec sync only when the CLI changes

.github/workflows/spec-sync.yml

@@ -72,8 +72,10 @@ jobs:
           CHECKED: ${{ steps.check.outcome }}
           RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
         run: |
-          # A change to an operation the CLI calls is a feat and names it, so the
-          # changelog says what moved; anything else is a chore and releases nothing.
+          # A change to what the CLI checks or offers — the parameters of an
+          # operation it calls, a list of values a flag reads — is a feat naming
+          # it, so the changelog says what moved. A body the CLI sends as given,
+          # an operation it does not call, or prose is a chore: nothing to release.
           title="$(node scripts/spec-diff.mjs --title "${RUNNER_TEMP}/openapi.before.yaml" spec/openapi.yaml)" ||
             title='feat: follow openapi.yaml'
           delimiter="EOF_$(openssl rand -hex 8)"

ARCHITECTURE.md

@@ -174,9 +174,14 @@ sends that is no longer taken fails a test.
 `.github/workflows/spec-sync.yml` fetches the published spec every day. When it
 differs, it regenerates, runs `check` and `test` against it, and opens a pull
 request on `chore/sync-openapi` listing the operations and schemas that moved
-and which of them the CLI calls. Its title comes from the same comparison: a
-change to an operation the CLI calls is a `feat` naming those operations, and
-releases; anything else is a `chore`, and does not. The pull request is opened even when the CLI no
+and which of them the CLI calls. Its title comes from the same comparison, and
+follows what the CLI does with the spec at run time: a change to the method,
+path or parameters of an operation it calls, or to a list of values a flag
+reads, is a `feat` naming it, and releases. A request body changing is a
+`chore`, since the CLI sends bodies as given and only its tests read their
+schemas; so is an operation it does not call, and so is prose. `spec-diff`
+reads the lookups off the source, which is why each names its operation and
+field in literals. The pull request is opened even when the CLI no
 longer passes, and says so: that is the case that needs a person.
 
 ## Errors and exit codes

scripts/spec-diff.mjs

@@ -1,19 +1,21 @@
 /**
  * What changed between two copies of openapi.yaml, for the spec-sync pull
- * request: which operations were added, removed or changed in what a request
- * may carry, which of them the CLI calls, and a conventional-commit title that
- * says so.
+ * request, and whether the change reaches what the CLI does.
  *
- * Only the request side counts, as it does for src/generated: an operation
- * changed when its method, path, parameters or body changed, or any schema
- * they refer to, however deep. Prose and responses do not count.
+ * The CLI follows the spec in two places at run time: the method, path and
+ * parameters of each operation it calls, which `call` checks and spells, and
+ * the lists of values its flags read with `choices` and `bodyChoices`. A
+ * change to either is a feat, and releases. A request body goes as given, so a
+ * change to one moves only what the tests hold the CLI's own bodies to: a
+ * chore, like a change to an operation the CLI does not call, or to prose.
+ * Responses never count.
  *
  * Usage:
  *   node scripts/spec-diff.mjs <old openapi.yaml> <new openapi.yaml>          markdown
  *   node scripts/spec-diff.mjs --title <old openapi.yaml> <new openapi.yaml>  the title
  */
 import { readFileSync, readdirSync } from 'node:fs';
-import { join } from 'node:path';
+import { join, relative } from 'node:path';
 import { parse } from 'yaml';
 
 const args = process.argv.slice(2);
@@ -45,7 +47,7 @@ function lookup(spec, ref) {
     .reduce((node, part) => node?.[part.replace(/~1/g, '/').replace(/~0/g, '~')], spec);
 }
 
-/** A parameter or body with the references to other parameters and bodies followed; schemas stay named. */
+/** A node with references to parameters and bodies followed; schemas stay named. */
 function expanded(spec, node) {
   if (Array.isArray(node)) return node.map((item) => expanded(spec, item));
   if (!node || typeof node !== 'object') return node;
@@ -70,19 +72,27 @@ function reach(spec, node, found = new Set()) {
   return found;
 }
 
+/** Each operation's request, as what `call` follows and as what goes through untouched. */
 function operations(spec) {
   const out = new Map();
   for (const [path, item] of Object.entries(spec.paths ?? {})) {
     for (const method of METHODS) {
       const op = item[method];
       if (!op?.operationId) continue;
-      const shape = expanded(spec, {
+      const parameters = expanded(spec, {
         method,
         path,
-        parameters: [...(item.parameters ?? []), ...(op.parameters ?? [])],
-        body: op.requestBody
+        parameters: [...(item.parameters ?? []), ...(op.parameters ?? [])]
+      });
+      const body = expanded(spec, op.requestBody ?? null);
+      out.set(op.operationId, {
+        op,
+        item,
+        parameters: JSON.stringify(withoutProse(parameters)),
+        parameterSchemas: reach(spec, parameters),
+        body: JSON.stringify(withoutProse(body)),
+        bodySchemas: reach(spec, body)
       });
-      out.set(op.operationId, { own: JSON.stringify(withoutProse(shape)), schemas: reach(spec, shape) });
     }
   }
   return out;
@@ -94,74 +104,173 @@ function schemas(spec) {
   );
 }
 
-const oldOps = operations(before);
-const newOps = operations(after);
-const oldSchemas = schemas(before);
-const newSchemas = schemas(after);
-const schemaChanged = (name) => oldSchemas.get(name) !== newSchemas.get(name);
+// --- what the CLI's source says it uses -----------------------------------------
 
-const sorted = (items) => [...items].sort();
-const added = sorted([...newOps.keys()].filter((id) => !oldOps.has(id)));
-const removed = sorted([...oldOps.keys()].filter((id) => !newOps.has(id)));
-const changed = sorted(
-  [...newOps.keys()].filter((id) => {
-    const old = oldOps.get(id);
-    const now = newOps.get(id);
-    return old && (old.own !== now.own || [...now.schemas].some(schemaChanged));
-  })
-);
-
-/** The operation ids the CLI's own code names, read off its source. */
-function usedByCli(ids) {
+/**
+ * The operation ids the CLI names, and the value lists its flags read, off its
+ * source. A lookup has to name its operation and its parameter or property in
+ * literals to be read here; one that does not stops the script rather than
+ * leaving a flag's list unwatched.
+ */
+function source(ids) {
   const used = new Set();
+  const lookups = [];
+  const unreadable = [];
+  const root = new URL('..', import.meta.url).pathname;
   const walk = (dir) => {
     for (const entry of readdirSync(dir, { withFileTypes: true })) {
       const file = join(dir, entry.name);
       if (entry.isDirectory()) {
-        if (entry.name !== 'generated') walk(file);
-      } else if (file.endsWith('.ts') && !file.endsWith('.test.ts')) {
-        for (const [, id] of readFileSync(file, 'utf8').matchAll(/'([A-Za-z]+)'/g)) if (ids.has(id)) used.add(id);
+        if (entry.name !== 'generated' && entry.name !== 'testing') walk(file);
+        continue;
+      }
+      if (!file.endsWith('.ts') || file.endsWith('.test.ts')) continue;
+      const text = readFileSync(file, 'utf8');
+      for (const [, id] of text.matchAll(/'([A-Za-z]+)'/g)) if (ids.has(id)) used.add(id);
+      for (const call of text.matchAll(/(?<!function )\b(choices|bodyChoices)\(([^)]*)\)/g)) {
+        const literal = /^\s*'([A-Za-z]+)'\s*,\s*'([^']+)'\s*$/.exec(call[2]);
+        if (literal) lookups.push({ kind: call[1], id: literal[1], path: literal[2] });
+        else unreadable.push(`${relative(root, file)}: ${call[0]}`);
       }
     }
   };
-  walk(new URL('../src', import.meta.url).pathname);
-  return used;
+  walk(join(root, 'src'));
+  if (unreadable.length > 0) {
+    console.error(
+      `spec-diff: these lookups do not name their operation and field in literals:\n  ${unreadable.join('\n  ')}`
+    );
+    process.exit(1);
+  }
+  return { used, lookups };
 }
 
-const used = usedByCli(new Set([...oldOps.keys(), ...newOps.keys()]));
-const affected = [...removed, ...changed].filter((id) => used.has(id));
+/** What a lookup reads in one copy of the spec, as `choices` and `bodyChoices` read it. */
+function valuesOf(spec, { kind, id, path }) {
+  const resolved = (node) => {
+    let current = node;
+    for (let hops = 0; current && typeof current.$ref === 'string' && hops < 32; hops += 1) {
+      current = lookup(spec, current.$ref);
+    }
+    return current;
+  };
+  const items = (schema) => (resolved(schema)?.type === 'array' ? resolved(resolved(schema).items) : resolved(schema));
+  const property = (schema, name) => {
+    const here = resolved(schema);
+    if (!here) return undefined;
+    if (here.properties?.[name]) return here.properties[name];
+    for (const part of here.allOf ?? []) {
+      const found = property(part, name);
+      if (found) return found;
+    }
+    return undefined;
+  };
+
+  let found;
+  for (const [route, item] of Object.entries(spec.paths ?? {})) {
+    for (const method of METHODS) if (item[method]?.operationId === id) found = { route, item, op: item[method] };
+  }
+  if (!found) return undefined;
+  let schema;
+  if (kind === 'choices') {
+    const parameters = [...(found.item.parameters ?? []), ...(found.op.parameters ?? [])].map(resolved);
+    schema = items(parameters.find((parameter) => parameter?.name === path)?.schema);
+  } else {
+    schema = resolved(found.op.requestBody)?.content?.['application/json']?.schema;
+    for (const step of path.split('.')) {
+      const many = step.endsWith('[]');
+      const next = property(schema, many ? step.slice(0, -2) : step);
+      schema = many ? items(next) : next;
+      if (!schema) break;
+    }
+  }
+  const values = (resolved(schema)?.enum ?? []).filter((value) => typeof value === 'string');
+  return values.length > 0 ? values : undefined;
+}
+
+// --- the comparison ----------------------------------------------------------
+
+const oldOps = operations(before);
+const newOps = operations(after);
+const oldSchemas = schemas(before);
+const newSchemas = schemas(after);
+const schemaChanged = (name) => oldSchemas.get(name) !== newSchemas.get(name);
+const { used, lookups } = source(new Set([...oldOps.keys(), ...newOps.keys()]));
+
+const sorted = (items) => [...items].sort();
+const added = sorted([...newOps.keys()].filter((id) => !oldOps.has(id)));
+const removed = sorted([...oldOps.keys()].filter((id) => !newOps.has(id)));
+const kept = sorted([...newOps.keys()].filter((id) => oldOps.has(id)));
+const parametersChanged = kept.filter((id) => {
+  const [old, now] = [oldOps.get(id), newOps.get(id)];
+  return old.parameters !== now.parameters || [...now.parameterSchemas].some(schemaChanged);
+});
+const bodyChanged = kept.filter((id) => {
+  const [old, now] = [oldOps.get(id), newOps.get(id)];
+  return old.body !== now.body || [...now.bodySchemas].some(schemaChanged);
+});
+
+/** The value lists a flag reads that came out different, each with what it gained and lost. */
+const movedLookups = lookups.flatMap((lookup) => {
+  const [old, now] = [valuesOf(before, lookup), valuesOf(after, lookup)];
+  if (JSON.stringify(old) === JSON.stringify(now)) return [];
+  const gained = (now ?? []).filter((value) => !(old ?? []).includes(value));
+  const lost = (old ?? []).filter((value) => !(now ?? []).includes(value));
+  return [{ name: `${lookup.id} ${lookup.path}`, gained, lost }];
+});
+
+const reachesCli = [
+  ...removed.filter((id) => used.has(id)).map((id) => ({ name: id, why: 'removed' })),
+  ...parametersChanged.filter((id) => used.has(id)).map((id) => ({ name: id, why: 'method, path or parameters' })),
+  ...movedLookups.map(({ name, gained, lost }) => ({
+    name,
+    why: [...gained.map((value) => `+${value}`), ...lost.map((value) => `−${value}`)].join(' ') || 'values'
+  }))
+];
+const bodiesOnly = bodyChanged.filter((id) => used.has(id) && !parametersChanged.includes(id));
+const elsewhere = {
+  added: added.filter((id) => !used.has(id)),
+  removed: removed.filter((id) => !used.has(id)),
+  changed: [...new Set([...parametersChanged, ...bodyChanged])].filter((id) => !used.has(id)).sort()
+};
 
-/**
- * A change to an operation the CLI calls is a feat, and releases: what the CLI
- * may send moved. Anything else is a chore, so a sync that touches nothing the
- * CLI calls neither releases nor adds a line to the changelog.
- */
 function title() {
-  if (affected.length > 0) {
-    const named = affected.slice(0, 3).join(', ');
-    const more = affected.length > 3 ? ` and ${affected.length - 3} more` : '';
-    return `feat: follow openapi.yaml changes to ${named}${more}`;
+  if (reachesCli.length > 0) {
+    const names = [...new Set(reachesCli.map((change) => change.name))];
+    const more = names.length > 3 ? ` and ${names.length - 3} more` : '';
+    return `feat: follow openapi.yaml changes to ${names.slice(0, 3).join(', ')}${more}`;
   }
-  if (added.length + removed.length + changed.length > 0) {
-    return 'chore: sync openapi.yaml, no operation the CLI calls changed';
+  if (added.length + removed.length + parametersChanged.length + bodyChanged.length > 0) {
+    return 'chore: sync openapi.yaml, nothing the CLI checks or offers changed';
   }
   return 'chore: sync openapi.yaml, prose only';
 }
 
 if (titleOnly) {
   console.log(title());
 } else {
-  const mark = (id) => (used.has(id) ? `**\`${id}\`** (called by the CLI)` : `\`${id}\``);
+  const code = (item) => `\`${item}\``;
   const lines = [];
-  const section = (heading, items, format = (item) => `\`${item}\``) => {
-    if (items.length > 0) lines.push(`- ${heading}: ${items.map(format).join(', ')}`);
-  };
-  section('Operations added', added, mark);
-  section('Operations removed', removed, mark);
-  section('Operations changed', changed, mark);
-  section(
-    'Schemas changed',
-    sorted([...newSchemas.keys()].filter((name) => oldSchemas.has(name) && schemaChanged(name)))
-  );
+  if (reachesCli.length > 0) {
+    lines.push(
+      `- **What the CLI checks or offers changed**: ${reachesCli.map(({ name, why }) => `${code(name)} (${why})`).join(', ')}`
+    );
+  }
+  if (bodiesOnly.length > 0) {
+    lines.push(
+      `- Request bodies the CLI sends as given changed, for the API to judge; the tests hold the CLI's own bodies to them: ${bodiesOnly.map(code).join(', ')}`
+    );
+  }
+  const others = [
+    ['added', elsewhere.added],
+    ['removed', elsewhere.removed],
+    ['changed', elsewhere.changed]
+  ].filter(([, ids]) => ids.length > 0);
+  if (others.length > 0) {
+    lines.push(
+      `- Operations the CLI does not call: ${others.map(([what, ids]) => `${what} ${ids.map(code).join(', ')}`).join('; ')}`
+    );
+  }
+  const changedSchemas = sorted([...newSchemas.keys()].filter((name) => oldSchemas.has(name) && schemaChanged(name)));
+  if (changedSchemas.length > 0) lines.push(`- Schemas changed: ${changedSchemas.map(code).join(', ')}`);
   console.log(lines.length > 0 ? lines.join('\n') : '- Only prose changed: descriptions, summaries or examples.');
 }

src/commands.test.ts

@@ -680,15 +680,22 @@ test('entry count takes one container, or several through the batch endpoint', a
   assert.equal(url(many.requests[0]), '/api/v1/entries/count?form_tokens=Kp7mQ2%2CaB3dE9');
 });
 
-test('entry count refuses more containers than the endpoint accepts', async () => {
+test('entry count leaves how many containers it may name to the API', async () => {
+  // The API says 10 in its prose, not its schema, and answers more with a 400
+  // that says so; a CLI refusing an eleventh would go on refusing after the
+  // API took more.
+  const mock = createMockClient();
   const tokens = Array.from({ length: 11 }, (_, index) => ['--form', `Kp7mQ${index}`]).flat();
   const result = await cli(['entry', 'count', ...tokens], {
     env: { JINSHUJU_ACCESS_TOKEN: 'tok' },
-    client: createMockClient().client
+    client: mock.client
   });
 
-  assert.equal(result.exitCode, 2);
-  assert.match(result.stderr, /at most 10 containers/);
+  assert.equal(result.exitCode, 0);
+  assert.equal(
+    mock.requests[0]?.query?.form_tokens,
+    Array.from({ length: 11 }, (_, index) => `Kp7mQ${index}`).join(',')
+  );
 });
 
 test('entry count, aggregate and summary read through a view when given one', async () => {

src/commands/entry.ts

@@ -39,9 +39,6 @@ import { isRecord } from '../values.js';
 import { basename } from 'node:path';
 import { upload } from './upload.js';
 
-/** What the batch count endpoint accepts, and what the design document states. */
-const MAX_COUNTED_CONTAINERS = 10;
-
 function list(value: unknown): string | undefined {
   const values = value as string[] | undefined;
   return values && values.length > 0 ? values.join(',') : undefined;
@@ -479,7 +476,7 @@ export const ENTRY: readonly Command[] = [
       FILTERS_OPTION
     ],
     request: (input) => {
-      const { tokens } = resolveContainers(input.options, MAX_COUNTED_CONTAINERS);
+      const { tokens } = resolveContainers(input.options);
       const keyword = input.options.keyword as string | undefined;
       if (tokens.length === 1) return analysis(input, COUNT, { filters: filters(input), keyword });
       if (input.options.view !== undefined) {