diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index a7982de..7565cbe 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -8,7 +8,7 @@ { "name": "clify", "description": "Generate A+ Node.js CLIs from API documentation. Copies a hand-crafted exemplar (structurally inspired by google/agents-cli), mechanically substitutes API-specific content, and verifies via a deterministic validation gate.", - "version": "0.5.0", + "version": "0.6.0", "source": "./" } ] diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index b6e7f04..a836432 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "clify", - "version": "0.5.0", + "version": "0.6.0", "description": "Generate A+ Node.js CLIs from API documentation. Copies a hand-crafted exemplar (structurally inspired by google/agents-cli), mechanically substitutes API-specific content, and verifies via a deterministic validation gate.", "author": { "name": "codeyogi911", diff --git a/examples/exemplar-cli/.clify.json b/examples/exemplar-cli/.clify.json index 40907b7..66e6177 100644 --- a/examples/exemplar-cli/.clify.json +++ b/examples/exemplar-cli/.clify.json @@ -26,5 +26,8 @@ "totalParsed": 11, "totalIncluded": 11, "totalDropped": 0 - } + }, + "filterProbes": [ + { "resource": "orders", "filter": "status", "baselineCount": 5, "filteredCount": 1, "status": "verified", "note": "verified against the bundled mock server in test/integration.test.mjs" } + ] } diff --git a/examples/exemplar-cli/bin/exemplar-cli.mjs b/examples/exemplar-cli/bin/exemplar-cli.mjs index 124840b..66ef5f9 100755 --- a/examples/exemplar-cli/bin/exemplar-cli.mjs +++ b/examples/exemplar-cli/bin/exemplar-cli.mjs @@ -11,7 +11,7 @@ import { dirname, join, resolve } from "node:path"; import { fileURLToPath } from "node:url"; import { loadEnv } from "../lib/env.mjs"; -import { splitGlobal, hasHelp, toParseArgs, checkRequired } from "../lib/args.mjs"; +import { splitGlobal, hasHelp, toParseArgs, checkRequired, checkEnum } from "../lib/args.mjs"; import { output, errorOut } from "../lib/output.mjs"; import { apiRequest, paginate } from "../lib/api.mjs"; import { showRootHelp, showResourceHelp, showActionHelp } from "../lib/help.mjs"; @@ -73,6 +73,12 @@ async function runResourceAction(resourceArg, actionArg, remaining, global, rest const missing = checkRequired(parsed.values, def.flags); if (missing.length) errorOut("validation_error", `Missing required flag(s): ${missing.map((m) => `--${m}`).join(", ")}`); + const enumViolations = checkEnum(parsed.values, def.flags); + if (enumViolations.length) { + const v = enumViolations[0]; + errorOut("validation_error", `--${v.flag}=${v.value} is not allowed; expected one of: ${v.allowed.join(", ")}`); + } + const path = interpolatePath(def.path, parsed.values); const buildPayload = PAYLOAD_BUILDERS[resourceArg]; diff --git a/examples/exemplar-cli/knowledge/query-flags-and-broken-list-filters.md b/examples/exemplar-cli/knowledge/query-flags-and-broken-list-filters.md index ae99334..47e33ad 100644 --- a/examples/exemplar-cli/knowledge/query-flags-and-broken-list-filters.md +++ b/examples/exemplar-cli/knowledge/query-flags-and-broken-list-filters.md @@ -1,7 +1,7 @@ --- type: contract -source: clify v0.5 -applies-to: ["bin/-cli.mjs", "commands/*.mjs", "lib/quirks.mjs"] +source: clify v0.6 +applies-to: ["bin/-cli.mjs", "commands/*.mjs", "lib/quirks.mjs", "lib/help.mjs", "lib/args.mjs"] --- # `queryFlags` and `brokenListFilters` action annotations @@ -45,40 +45,99 @@ The runtime in `bin/-cli.mjs`: Some `GET …/list` endpoints accept filter query parameters that the server silently ignores: `200 OK`, no error header, full unfiltered list returned. -Detection requires comparing baseline (no filter) and `FAKE-NONEXISTENT` -(impossible value) row counts — equal counts means the filter is broken. +**Detection requires per-flag probing** — running one probe with the +wrong name and blanket-marking the rest is the v0.5 anti-pattern this +contract exists to prevent. + +### Discovery checklist (per list endpoint) + +When generating or auditing a CLI: + +1. **Read the docs page properly.** Every documented query-parameter + becomes its own flag, **using the documented name verbatim**: + `--customer_name_startswith`, `--customer_name_contains`, + `--reference_number_startswith`, `--filter_by`, `--date_start`, + `--date_end`. Do NOT collapse variants — the suffix (`_startswith`, + `_contains`) is the server-side match mode and is not optional. +2. **Surface enum values.** When the docs say a param accepts an enum + (e.g. `filter_by: All|NotShipped|Shipped|Delivered`), set + `flags..enum: [...]` AND mention the allowed values in + `description`. The exemplar's help generator renders inline: + `--filter_by `. The bin runtime + rejects out-of-enum values with a `validation_error`. +3. **Auto-emit `sort_column` + `sort_order`** for any list endpoint + whose docs mention sorting. Most paginated APIs (Zoho, Shopify, + Stripe-style) accept these even when not tabled. +4. **Per-flag probe** (when the API can be reached at generation time): + - Make one unfiltered call → record `baselineCount` (response row count). + - For each filter, make one call with a value sampled from the + unfiltered response (or the first enum value). + - Record `filteredCount`. + - Status: + - `filteredCount < baselineCount` → `verified` (server honored it). + - `filteredCount === baselineCount` → `broken` (server ignored it). + - probe failed (no creds, network blocked, rate limit) → `untested`. + Leave the flag working; do NOT add to `brokenListFilters`. +5. **Write the probe log to `.clify.json.filterProbes`** — one entry per + filter probed (or skipped). The validator's `filter-coverage` check + reads this: + - List action declares filter-shaped flags but `filterProbes` has + zero entries for that resource → HARD-FAIL. + - Every filter on a list action is in `brokenListFilters` AND + `filterProbes` shows no individual probes → HARD-FAIL (the + blanket-mark anti-pattern). + - Filter is `untested` → WARN, not fail. + +For each filter the probe marked `broken`, declare it in +`brokenListFilters` on the action def. The v0.6+ form takes +per-filter `match` modes so the client-side fallback mirrors the +documented match semantics: ```js list: { method: "GET", path: "/orders", - flags: { customerId: { type: "string", description: "Filter (BROKEN upstream — client-side fallback)" } }, - brokenListFilters: ["customerId"], + flags: { + customer_name_startswith: { type: "string", description: "Filter by customer name prefix (BROKEN upstream — client-side fallback)" }, + filter_by: { type: "string", enum: ["All", "Shipped", "NotShipped"], description: "Status filter (BROKEN upstream)" }, + }, + brokenListFilters: [ + { name: "customer_name_startswith", match: "startswith" }, + { name: "filter_by", match: "equals" }, + ], } ``` +(The legacy v0.5 string-list form `brokenListFilters: ["customer_name"]` is +still accepted and defaults to `match: "equals"` — but new generations +should use the object form so the fallback semantics match the docs.) + Runtime behavior when the user passes a broken filter: 1. Strip the filter from the wire query. 2. Pull the full list via cursor pagination. -3. Filter client-side (case-insensitive equals OR substring) on the - row's same-named field. +3. Filter client-side with the per-filter `match` mode (`equals`, + `startswith`, `contains`). 4. Write a one-line `note: …` to stderr. Cost goes up to the full-list response size. For small datasets this is trivial; for larger ones, callers should pull once and filter themselves. -## Discovery checklist - -When generating or auditing a CLI: - -1. For every documented `POST ` create — does the API doc list a - foreign-key parameter under "Query parameters" (not "Body parameters")? - If yes, add it to `queryFlags`. (Common APIs that do this: - Zoho Inventory/Books, Zoho CRM, Xero, Razorpay X.) -2. For every list endpoint — pick one declared filter and compare the - row count with `-- FAKE-NONEXISTENT-XYZ` against an - unfiltered call. Equal counts → broken; declare in `brokenListFilters` - AND clarify the help text. +### Worked example: Zoho Inventory `packages list` + +Docs declare nine query parameters: `filter_by` (enum), `customer_name_startswith`, +`customer_name_contains`, `reference_number_startswith`, `reference_number_contains`, +`date_start`, `date_end`, `sort_column`, `sort_order`. The pre-v0.6 generator +collapsed these into a bare `--status`, probed once, saw the API ignore `status`, +and marked all nine as broken. + +Correct generation: +- Emit each flag verbatim. +- Probe `--filter_by Shipped` → `filteredCount=83 vs baselineCount=319` → `verified`. +- Probe `--customer_name_startswith Acme` → `filteredCount=12 vs 319` → `verified`. +- Probe `--status Shipped` (if mistakenly emitted) → `filteredCount=319` → `broken`. +- `.clify.json.filterProbes` records all nine — verified or broken or untested. +- Only the genuinely-broken ones land in `brokenListFilters` with the right + `match` mode. A new clify version should re-run these probes whenever the upstream docs change (`clify sync-check` flags doc drift; the manual probe is the diff --git a/examples/exemplar-cli/lib/args.mjs b/examples/exemplar-cli/lib/args.mjs index 6e4de76..9ff493e 100644 --- a/examples/exemplar-cli/lib/args.mjs +++ b/examples/exemplar-cli/lib/args.mjs @@ -37,3 +37,17 @@ export function checkRequired(values, flagSpec) { } return missing; } + +// For flags that declare `enum: [...]`, reject values that aren't in the +// allowed set. Returns a list of `{flag, value, allowed}` for each +// violator (empty list = no violations). Skips flags that weren't passed. +export function checkEnum(values, flagSpec) { + const violations = []; + for (const [name, spec] of Object.entries(flagSpec)) { + if (!Array.isArray(spec.enum) || spec.enum.length === 0) continue; + const v = values[name]; + if (v === undefined || v === "") continue; + if (!spec.enum.includes(v)) violations.push({ flag: name, value: v, allowed: spec.enum }); + } + return violations; +} diff --git a/examples/exemplar-cli/lib/help.mjs b/examples/exemplar-cli/lib/help.mjs index edd306a..2ec9a6a 100644 --- a/examples/exemplar-cli/lib/help.mjs +++ b/examples/exemplar-cli/lib/help.mjs @@ -30,8 +30,13 @@ export function showActionHelp(resource, action, registry) { if (entries.length === 0) out += ` (none)\n`; for (const [name, spec] of entries) { const req = spec.required ? "required" : "optional"; + // Render enum values inline so users see allowed values without + // bouncing to the docs: `--filter_by `. + const flagLabel = Array.isArray(spec.enum) && spec.enum.length + ? `--${name} <${spec.enum.join("|")}>` + : `--${name}`; const desc = spec.description || ""; - out += ` --${name.padEnd(20)} ${spec.type.padEnd(8)} ${req.padEnd(8)} ${desc}\n`; + out += ` ${flagLabel.padEnd(22)} ${spec.type.padEnd(8)} ${req.padEnd(8)} ${desc}\n`; } return out; } diff --git a/examples/exemplar-cli/lib/quirks.mjs b/examples/exemplar-cli/lib/quirks.mjs index 08bb897..9e1f8f5 100644 --- a/examples/exemplar-cli/lib/quirks.mjs +++ b/examples/exemplar-cli/lib/quirks.mjs @@ -26,34 +26,64 @@ export function stripQueryFlags(values, def) { return out; } +// `brokenListFilters` may be either a flat list of flag names (legacy v0.5 +// shape) OR a list of `{ name, match }` objects (v0.6+, per-flag match mode). +// Normalize both into `{ name, match }`. Default match is "equals". +function normalizeBrokenFilters(def) { + if (!def?.brokenListFilters?.length) return []; + return def.brokenListFilters.map((entry) => + typeof entry === "string" + ? { name: entry, match: "equals" } + : { name: entry.name, match: entry.match || "equals" }, + ); +} + // Detect which of `def.brokenListFilters` the user populated on this call. -// Returns a {flag: value} map (empty when none). Callers should drop these -// from the wire query, fetch the full list, and run `clientFilter` on the -// result. +// Returns a `{ flag: { value, match } }` map (empty when none). Callers +// should drop these from the wire query, fetch the full list, and run +// `clientFilter` on the result. export function pickBrokenFilters(values, def) { - if (!def?.brokenListFilters?.length) return {}; const out = {}; - for (const k of def.brokenListFilters) { - const v = values[k]; - if (v !== undefined && v !== "") out[k] = String(v); + for (const { name, match } of normalizeBrokenFilters(def)) { + const v = values[name]; + if (v !== undefined && v !== "") out[name] = { value: String(v), match }; } return out; } // Client-side fallback for list endpoints whose upstream-API filter is -// silently ignored. Compares each requested value against the row's -// same-named field with case-insensitive equality OR substring match (the -// substring covers prefix-style number lookups users typically want). +// silently ignored. Per-filter match modes: +// - "equals" — case-insensitive exact equality. +// - "startswith" — case-insensitive prefix match. +// - "contains" — case-insensitive substring match. +// `filters` is the `{ name: { value, match } }` map from pickBrokenFilters. +// Backwards-compatible: callers passing the legacy `{ name: value }` shape +// are coerced to `{ value, match: "equals-or-contains" }` for the v0.5 +// hybrid behavior (case-insensitive equals OR substring). export function clientFilter(items, filters) { const checks = Object.entries(filters); if (!checks.length) return items; + const normalized = checks.map(([k, raw]) => { + if (raw && typeof raw === "object" && "value" in raw) return [k, raw]; + return [k, { value: raw, match: "equals-or-contains" }]; + }); return items.filter((row) => - checks.every(([k, target]) => { + normalized.every(([k, { value, match }]) => { const v = row?.[k]; if (v === undefined || v === null) return false; const a = String(v).toLowerCase(); - const b = String(target).toLowerCase(); - return a === b || a.includes(b); + const b = String(value).toLowerCase(); + switch (match) { + case "equals": + return a === b; + case "startswith": + return a.startsWith(b); + case "contains": + return a.includes(b); + case "equals-or-contains": + default: + return a === b || a.includes(b); + } }), ); } diff --git a/examples/exemplar-cli/test/integration.test.mjs b/examples/exemplar-cli/test/integration.test.mjs index d0e3a2f..1b4c3f4 100644 --- a/examples/exemplar-cli/test/integration.test.mjs +++ b/examples/exemplar-cli/test/integration.test.mjs @@ -139,6 +139,46 @@ test("orders list filters by status query", async () => { }); }); +// ---------- filter-probe pattern (v0.6+) ---------- +// +// This is the canonical test pattern any generated CLI inherits — it +// proves a declared list-filter flag actually filters server-side. +// Catches the Zoho `filter_by` miss the v0.6 generator-side fix targets: +// pre-v0.6, generated CLIs would declare `--status` on a list endpoint, +// then mark the WHOLE filter set as broken when one probe returned the +// unfiltered count. With per-flag probing, the verified-server-side +// filter (this test) and the broken-fallback (unit-tested in quirks.test.mjs) +// stay distinct, and `.clify.json.filterProbes` records the truth. +test("filter-probe pattern: declared --status flag filters server-side (verified)", async () => { + // Mock returns 5 rows unfiltered, 2 rows when ?status=shipped. + const allRows = [ + { id: "o1", status: "pending" }, + { id: "o2", status: "shipped" }, + { id: "o3", status: "shipped" }, + { id: "o4", status: "cancelled" }, + { id: "o5", status: "pending" }, + ]; + await withMock({ + "GET /orders": (req) => { + const filter = req.query.status; + const rows = filter ? allRows.filter((r) => r.status === filter) : allRows; + return { status: 200, body: { items: rows, nextCursor: null } }; + }, + }, async (server) => { + // Baseline (unfiltered). + const baseline = await runJson(["orders", "list"], { env: { ...ENV, EXEMPLAR_BASE_URL: server.url } }); + assert.equal(baseline.exitCode, 0, baseline.stderr); + assert.equal(baseline.json.items.length, 5, "baseline returns the full list"); + + // Filtered call — must (a) put status on the wire, (b) return fewer rows. + const filtered = await runJson(["orders", "list", "--status", "shipped"], { env: { ...ENV, EXEMPLAR_BASE_URL: server.url } }); + assert.equal(filtered.exitCode, 0, filtered.stderr); + assert.equal(filtered.json.items.length, 2, "server-side filter took effect"); + const filteredReq = server.requests[server.requests.length - 1]; + assert.equal(filteredReq.query.status, "shipped", "wire URL contained ?status=shipped (server-side filter, not client-side fallback)"); + }); +}); + test("orders upload posts multipart/form-data with --file", async () => { const tmp = mkdtempSync(join(tmpdir(), "exemplar-mp-")); const filePath = join(tmp, "receipt.txt"); diff --git a/examples/exemplar-cli/test/quirks.test.mjs b/examples/exemplar-cli/test/quirks.test.mjs index 5a73846..c74bfb1 100644 --- a/examples/exemplar-cli/test/quirks.test.mjs +++ b/examples/exemplar-cli/test/quirks.test.mjs @@ -34,30 +34,79 @@ test("stripQueryFlags is a no-op when no queryFlags declared", () => { assert.deepEqual(stripQueryFlags({ a: 1, b: 2 }, {}), { a: 1, b: 2 }); }); -test("pickBrokenFilters returns the declared filters that were populated", () => { +test("pickBrokenFilters returns the declared filters that were populated (legacy string form)", () => { const def = { brokenListFilters: ["customerId", "tag"] }; assert.deepEqual( pickBrokenFilters({ customerId: "C-9", tag: "", other: "x" }, def), - { customerId: "C-9" }, + { customerId: { value: "C-9", match: "equals" } }, ); assert.deepEqual(pickBrokenFilters({}, def), {}); assert.deepEqual(pickBrokenFilters({ a: 1 }, {}), {}); }); -test("clientFilter returns rows that match every filter (case-insensitive equals OR substring)", () => { +test("pickBrokenFilters honors per-filter match mode (v0.6+ object form)", () => { + const def = { + brokenListFilters: [ + { name: "customer_name_startswith", match: "startswith" }, + { name: "reference_number_contains", match: "contains" }, + { name: "filter_by", match: "equals" }, + ], + }; + assert.deepEqual( + pickBrokenFilters( + { customer_name_startswith: "Ac", reference_number_contains: "INV", filter_by: "Shipped" }, + def, + ), + { + customer_name_startswith: { value: "Ac", match: "startswith" }, + reference_number_contains: { value: "INV", match: "contains" }, + filter_by: { value: "Shipped", match: "equals" }, + }, + ); +}); + +test("clientFilter (legacy string-value shape): case-insensitive equals OR substring", () => { const items = [ { id: "1", customerId: "ACME-CO", region: "us-west" }, { id: "2", customerId: "acme-co", region: "us-east" }, { id: "3", customerId: "OTHER", region: "us-west" }, ]; - // Equals (case-insensitive) assert.deepEqual(clientFilter(items, { customerId: "acme-co" }).map((r) => r.id), ["1", "2"]); - // Substring (e.g. user passes a prefix or contained fragment) assert.deepEqual(clientFilter(items, { customerId: "ACME" }).map((r) => r.id), ["1", "2"]); - // Multi-filter is AND assert.deepEqual(clientFilter(items, { customerId: "ACME", region: "us-west" }).map((r) => r.id), ["1"]); }); +test("clientFilter (v0.6+ object shape) honors per-filter match modes", () => { + const items = [ + { id: "1", customer_name: "Acme Industries", reference_number: "INV-001", filter_by: "Shipped" }, + { id: "2", customer_name: "Acme Goods", reference_number: "INV-002", filter_by: "NotShipped" }, + { id: "3", customer_name: "Other Co", reference_number: "PO-077", filter_by: "Shipped" }, + ]; + // startswith: "Ac" matches both Acme rows but not Other Co. + assert.deepEqual( + clientFilter(items, { customer_name: { value: "Ac", match: "startswith" } }).map((r) => r.id), + ["1", "2"], + ); + // contains: "INV" matches the two with INV reference numbers. + assert.deepEqual( + clientFilter(items, { reference_number: { value: "INV", match: "contains" } }).map((r) => r.id), + ["1", "2"], + ); + // equals: only an exact case-insensitive match — "Shipped" must NOT match "NotShipped". + assert.deepEqual( + clientFilter(items, { filter_by: { value: "Shipped", match: "equals" } }).map((r) => r.id), + ["1", "3"], + ); + // Multi-filter is AND across mixed match modes. + assert.deepEqual( + clientFilter(items, { + customer_name: { value: "Acme", match: "startswith" }, + filter_by: { value: "Shipped", match: "equals" }, + }).map((r) => r.id), + ["1"], + ); +}); + test("clientFilter is a no-op when no filters provided", () => { const items = [{ id: "1" }, { id: "2" }]; assert.equal(clientFilter(items, {}), items); diff --git a/lib/validate.mjs b/lib/validate.mjs index 8dac166..f57b7dc 100644 --- a/lib/validate.mjs +++ b/lib/validate.mjs @@ -54,6 +54,7 @@ export async function validate(repoDir, options = {}) { await checkCoverage(ctx); await checkFamilyConsistency(ctx); await checkStatusVerbCanonical(ctx); + await checkFilterCoverage(ctx); await checkStructural(ctx); await checkReadme(ctx); await checkNuances(ctx); @@ -411,6 +412,239 @@ async function checkFamilyConsistency(ctx) { } } +// ---------- 5d. filter-coverage (per-flag probe enforcement) ---------- + +// Names that are pagination/sort/identity, not user-facing filters. +// Filter-shaped is "everything else" on a list action. +const NON_FILTER_FLAGS = new Set([ + "id", + "page", + "cursor", + "limit", + "offset", + "per_page", + "perPage", + "page_size", + "pageSize", + "sort", + "sort_column", + "sort_order", + "sortColumn", + "sortOrder", + "body", + "file", + "idempotency-key", + "if-match", +]); + +// Catch the v0.5 anti-pattern this repo's #5 issue is about: +// 1. List action with filter-shaped flags but ZERO filterProbes for the +// resource → HARD-FAIL (Phase 5 skipped the probe step). +// 2. Every filter on a list action declared in brokenListFilters AND no +// individual probes for that resource in filterProbes → HARD-FAIL +// (the blanket-mark anti-pattern). +// 3. A filter with status:"untested" → WARN. +// +// Scans commands/*.mjs textually for the shape — the LLM-emitted source +// is always plain JS literal exports, so a regex pass is enough and we +// don't have to dynamically import a foreign repo's source. +async function checkFilterCoverage(ctx) { + const cfg = readJson(join(ctx.dir, ".clify.json")); + if (cfg.__err) return; + const probes = Array.isArray(cfg.filterProbes) ? cfg.filterProbes : []; + const probesByResource = new Map(); + for (const p of probes) { + if (!p?.resource) continue; + if (!probesByResource.has(p.resource)) probesByResource.set(p.resource, []); + probesByResource.get(p.resource).push(p); + } + + const commandsDir = join(ctx.dir, "commands"); + if (!existsSync(commandsDir)) return; + + let sawAnyListAction = false; + const failures = []; + + for (const entry of readdirSync(commandsDir, { withFileTypes: true })) { + if (!entry.isFile() || !entry.name.endsWith(".mjs")) continue; + const src = readText(join(commandsDir, entry.name)) || ""; + const resourceMatch = src.match(/name:\s*["']([^"']+)["']/); + if (!resourceMatch) continue; + const resource = resourceMatch[1]; + + // Match each `: { ... }` block where method:"GET" appears at + // the top-level. We extract the list action's flags and brokenListFilters. + // The block is balanced-brace-delimited; we scan with a small parser. + for (const action of extractListActions(src)) { + sawAnyListAction = true; + // Filter-shaped = anything not in NON_FILTER_FLAGS and not a path + // param (e.g. `itemId` in `/items/:itemId/variants` — that's a + // parent-resource id, not a filter). + const pathParamSet = new Set(action.pathParams || []); + const filterFlags = action.flags.filter((f) => !NON_FILTER_FLAGS.has(f) && !pathParamSet.has(f)); + if (filterFlags.length === 0) continue; + + const resourceProbes = probesByResource.get(resource) || []; + // Failure mode 1: filter-shaped flags exist but no probes for resource. + if (resourceProbes.length === 0) { + failures.push({ + resource, + action: action.name, + reason: "list action declares filter-shaped flags but .clify.json.filterProbes has zero entries for this resource (Phase 5 skipped the probe step)", + filterFlags, + }); + continue; + } + // Failure mode 2: every filter is in brokenListFilters AND the + // probes for this resource are all 'broken' OR have no per-filter + // detail — i.e. the LLM blanket-marked everything from one bad probe. + const brokenSet = new Set(action.brokenListFilters); + const everyFilterBroken = filterFlags.length > 0 && filterFlags.every((f) => brokenSet.has(f)); + if (everyFilterBroken) { + const verifiedCount = resourceProbes.filter((p) => p.status === "verified").length; + if (verifiedCount === 0) { + failures.push({ + resource, + action: action.name, + reason: "every filter on this list action is in brokenListFilters and no probe in .clify.json.filterProbes verified ANY filter — looks like a blanket-mark from a single failed probe", + filterFlags, + brokenListFilters: action.brokenListFilters, + }); + continue; + } + } + + // Warning mode: any filter listed as 'untested' for this resource. + for (const p of resourceProbes) { + if (p.status === "untested") { + ctx.warnings.push(`filter-coverage: ${resource}.${p.filter} is untested at generation time${p.note ? ` (${p.note})` : ""}`); + } + } + } + } + + if (!sawAnyListAction) return; // nothing to enforce on this CLI + if (failures.length === 0) { + ctx.results.push(pass("coverage", "filter-coverage: every list action's filters are individually probed")); + } else { + ctx.results.push(fail("coverage", "filter-coverage: list actions blanket-marked filters or skipped probe step", { failures })); + } +} + +// Walk a commands/.mjs source and pull out list actions with +// their flag names and brokenListFilters. +// +// Strategy: find the top-level `actions: { ... }` block, then scan only +// its top-level keys (each is one action). For every action whose body +// has `method: "GET"`, treat it as a candidate list action — but skip +// single-resource GETs (action name "get", or path ends in `/:id`). +function extractListActions(src) { + const actionsMatch = src.match(/actions\s*:\s*\{/); + if (!actionsMatch) return []; + const blockStart = src.indexOf("{", actionsMatch.index + actionsMatch[0].length - 1); + const blockEnd = findMatchingBrace(src, blockStart); + if (blockEnd < 0) return []; + const block = src.slice(blockStart + 1, blockEnd); + + const actions = []; + let depth = 0; + let i = 0; + while (i < block.length) { + const ch = block[i]; + if (ch === "{") { depth++; i++; continue; } + if (ch === "}") { depth--; i++; continue; } + if (depth !== 0) { i++; continue; } + const tail = block.slice(i); + const km = tail.match(/^\s*(?:"([^"]+)"|'([^']+)'|([A-Za-z_][\w-]*))\s*:\s*\{/); + if (!km) { i++; continue; } + const actionName = km[1] || km[2] || km[3]; + const bodyStart = i + km[0].lastIndexOf("{"); + const bodyEnd = findMatchingBrace(block, bodyStart); + if (bodyEnd < 0) { i += km[0].length; continue; } + const body = block.slice(bodyStart + 1, bodyEnd); + i = bodyEnd + 1; + + if (!/method\s*:\s*["']GET["']/.test(body)) continue; + if (actionName === "get") continue; + const pathMatch = body.match(/path\s*:\s*["']([^"']+)["']/); + if (!pathMatch) continue; + if (/\/:[^/]+\/?$/.test(pathMatch[1]) && actionName !== "list") continue; + const pathParams = [...pathMatch[1].matchAll(/:([A-Za-z_][\w]*)/g)].map((m) => m[1]); + + actions.push({ + name: actionName, + flags: extractFlagNames(body), + pathParams, + brokenListFilters: extractBrokenListFilters(body), + }); + } + return actions; +} + +function findMatchingBrace(src, openAt) { + let depth = 0; + for (let i = openAt; i < src.length; i++) { + const ch = src[i]; + if (ch === "{") depth++; + else if (ch === "}") { depth--; if (depth === 0) return i; } + } + return -1; +} + +function extractFlagNames(body) { + const flagsMatch = body.match(/flags\s*:\s*\{/); + if (!flagsMatch) return []; + const start = body.indexOf("{", flagsMatch.index + flagsMatch[0].length - 1); + const end = findMatchingBrace(body, start); + if (end < 0) return []; + const flagsBody = body.slice(start + 1, end); + // Each top-level key in flagsBody is a flag. Match `"name": {` or `name: {`. + const names = []; + // Track nesting so we only catch top-level keys. + let depth = 0; + let i = 0; + while (i < flagsBody.length) { + const ch = flagsBody[i]; + if (ch === "{") { depth++; i++; continue; } + if (ch === "}") { depth--; i++; continue; } + if (depth !== 0) { i++; continue; } + // At depth 0: try to match an identifier/quoted-string followed by ':' + const tail = flagsBody.slice(i); + const km = tail.match(/^\s*(?:"([^"]+)"|'([^']+)'|([A-Za-z_][\w-]*))\s*:/); + if (km) { + names.push(km[1] || km[2] || km[3]); + i += km[0].length; + // skip whitespace and `{` to enter the value + while (i < flagsBody.length && /\s/.test(flagsBody[i])) i++; + if (flagsBody[i] === "{") { + const valEnd = findMatchingBrace(flagsBody, i); + if (valEnd > 0) i = valEnd + 1; + else i++; + } + continue; + } + i++; + } + return names; +} + +function extractBrokenListFilters(body) { + const m = body.match(/brokenListFilters\s*:\s*\[([^\]]*)\]/); + if (!m) return []; + const inside = m[1]; + // Either a flat list of strings, or objects like `{ name: "...", match: "..." }`. + const out = []; + for (const tok of inside.split(",")) { + const t = tok.trim(); + if (!t) continue; + const strMatch = t.match(/^["']([^"']+)["']$/); + if (strMatch) { out.push(strMatch[1]); continue; } + const objMatch = t.match(/name\s*:\s*["']([^"']+)["']/); + if (objMatch) { out.push(objMatch[1]); continue; } + } + return out; +} + // ---------- 5c. status-verb canonicalisation ---------- // Every endpoint matching POST //:id/status/ must map to action diff --git a/package.json b/package.json index 7185163..b94ebd7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "clify", - "version": "0.5.0", + "version": "0.6.0", "description": "Generate A+ Node.js CLIs from API documentation. Copies a hand-crafted exemplar (structurally inspired by google/agents-cli), mechanically substitutes API-specific content, and verifies via a deterministic validation gate.", "type": "module", "bin": { diff --git a/references/scaffold-pipeline.md b/references/scaffold-pipeline.md index b598c68..2954ce2 100644 --- a/references/scaffold-pipeline.md +++ b/references/scaffold-pipeline.md @@ -65,6 +65,73 @@ After grouping endpoints by resource, group AGAIN by sub-path tail. Common share Why hard-fail: in the Fix Coffee Zoho rollout, `/comments` was generated for 5 of 7 sibling resources and silently skipped on the rest. Users had to hand-add the missing actions. The family-consistency check catches this at gate time. +### List-filter extraction (canonical contract) + +Generated CLIs systematically lose information at this step — collapsing +documented variant names into bare flags, then blanket-marking the +result as BROKEN when one probe with the wrong name fails. The +`filter-coverage` validator check (v0.6+) enforces the rules below. + +1. **Every documented query-parameter on a `GET …/list` endpoint becomes + its own flag, named verbatim.** When the docs list + `customer_name_startswith`, `customer_name_contains`, + `reference_number_startswith`, `date_start`, `date_end`, `filter_by`, + emit ALL of them — six flags, not one collapsed `--customer_name`. + The suffix encodes the server-side match mode and is not optional. +2. **Enum values go in the flag spec.** When the docs say `filter_by` + accepts `All|NotShipped|Shipped|Delivered`, set + `flags.filter_by.enum: ["All", "NotShipped", "Shipped", "Delivered"]` + AND include them in `flags.filter_by.description`. The exemplar's + help generator reads `enum` and renders inline allowed-values. +3. **Sort flags are auto-emit.** For any list endpoint, add + `--sort_column` (string) and `--sort_order` (`asc|desc`, enum) even if + not explicitly tabled. Almost every paginated list API supports them + and the absence in docs is almost always an omission. +4. **Per-flag probe.** When the API can be reached at generation time, + probe each filter individually: + - Make one unfiltered request → record `baselineCount` (response row count). + - For each filter, make one request with a value that should match ≥1 + record (use a real value sampled from the unfiltered response, or + the first enum value for `filter_by`-style flags). + - Record `filteredCount`. + - Status: + - `filteredCount < baselineCount` (and ≥0) → `verified`. + - `filteredCount === baselineCount` → `broken` (server ignored it). + - couldn't probe (no creds, network blocked, rate-limited) → + `untested`. Leave the flag working, do NOT add to + `brokenListFilters`. +5. **Write the probe log to `.clify.json.filterProbes`.** Schema: + ```json + { + "filterProbes": [ + { "resource": "packages", "filter": "filter_by", "baselineCount": 319, "filteredCount": 83, "status": "verified" }, + { "resource": "packages", "filter": "status", "baselineCount": 319, "filteredCount": 319, "status": "broken" }, + { "resource": "packages", "filter": "date_start", "baselineCount": 319, "filteredCount": 319, "status": "untested", "note": "no upstream test data in date range" } + ] + } + ``` + The validator's `filter-coverage` check reads this in Phase 6: + - If a list action declares filter-shaped flags (anything not in + `{id, page, cursor, limit, offset, per_page, sort_column, + sort_order}`) and `filterProbes` has zero entries for that + resource → HARD-FAIL ("Phase 5 skipped probe step"). + - If every filter on a list action is in `brokenListFilters` AND + `filterProbes` shows no individual probes → HARD-FAIL (the + blanket-mark anti-pattern). + - If a filter is `untested` → WARN, not fail. Untested-at-generation + is fine; silent blanket-marking is not. + +**Worked example — Zoho Inventory `packages list`:** Docs declare +`filter_by` (enum: All|NotShipped|Shipped|Delivered), `customer_name_startswith`, +`reference_number_startswith`, `date_start`, `date_end`, `sort_column`, `sort_order`. +Pre-v0.6 generation collapsed these into a bare `--status`, probed it, +saw the API ignore `status`, and marked all nine filters as broken. +Correct generation: emit each flag verbatim, probe `--filter_by Shipped` +(returns 83 vs unfiltered 319 → `verified`), probe the rest, only mark +the genuinely-ignored ones as broken. The `customer_name_startswith` and +`reference_number_startswith` are usually verified; `filter_by` is +verified; `status` (if mistakenly emitted) is broken. + ### Nuance detection cheat sheet | Signal | Nuance | Artifact | @@ -198,10 +265,18 @@ Generally unchanged. Edit the `BASE_URL` default if the API doesn't have a singl "clifyVersion": "", "auth": { "envVar": "_API_KEY", "scheme": "", "validationCommand": " " }, "nuances": { /* every detected nuance */ }, - "coverage": { "totalParsed": N, "totalIncluded": M, "totalDropped": K } + "coverage": { "totalParsed": N, "totalIncluded": M, "totalDropped": K }, + "filterProbes": [ + { "resource": "", "filter": "", "baselineCount": , "filteredCount": , "status": "verified" | "broken" | "untested", "note": "" } + ] } ``` +`filterProbes` is read by the `filter-coverage` validator check (v0.6+). +One entry per probed-or-skipped filter. Skipping a filter entirely (no +entry) is forbidden when the resource declares filter-shaped flags — +the validator hard-fails that. + ### `.env.example` For `scheme !== none`, include: diff --git a/skills/clify/SKILL.md b/skills/clify/SKILL.md index 05e7799..51ed95e 100644 --- a/skills/clify/SKILL.md +++ b/skills/clify/SKILL.md @@ -66,13 +66,21 @@ Then `git init` the new repo and commit the unmodified scaffold so the next phas Edit only what changes per API. Preserve helper signatures (`apiRequest`, `output`, `errorOut`, `splitGlobal`, `toParseArgs`, `checkRequired`, help generators) verbatim from the exemplar. Per-file substitutions: -> **Two opt-in action-def annotations available this phase** (substrate: `examples/exemplar-cli/lib/quirks.mjs` + bin runtime; contract: [`knowledge/query-flags-and-broken-list-filters.md`](../../examples/exemplar-cli/knowledge/query-flags-and-broken-list-filters.md)). The exemplar itself doesn't pre-populate either — they're dormant until a generated `commands/.mjs` opts in. +> **List-filter handling — the most-drifted area in generated CLIs.** Read [`knowledge/query-flags-and-broken-list-filters.md`](../../examples/exemplar-cli/knowledge/query-flags-and-broken-list-filters.md) before emitting any `list` action. The five rules below are the contract; the validator's `filter-coverage` check (v0.6+) hard-fails generations that skip them. +> +> 1. **Verbatim flag extraction.** Emit each documented query-parameter as its own flag, **using the documented name verbatim**: `--customer_name_startswith`, `--customer_name_contains`, `--filter_by`, `--date_start`, `--date_end`, `--reference_number_startswith`. Do NOT collapse variants (`*_startswith`, `*_contains`) into a bare `--customer_name` — the suffix IS the filter mode and is what the server keys on. +> 2. **Enum surfacing.** When the docs say a param accepts an enum (e.g. `filter_by: All|NotShipped|Shipped|Delivered`), set `flags..enum: [...]` AND include the allowed values in `flags..description`. The exemplar's help generator renders these inline as `--filter_by `. +> 3. **Auto-emit `sort_column` + `sort_order`** for any list endpoint where the docs mention sorting (Zoho, Shopify, Stripe-style APIs all expose these). Their absence is almost always a docs omission, not a server-side restriction. +> 4. **Per-flag probe protocol.** For each filter you declare, probe individually with a value that should match ≥1 record. Compare the result count against an unfiltered baseline. Mark BROKEN ONLY the filters whose count equals the baseline (server ignored them). Untested flags stay as working flags but get a `# untested-at-generation` note in `.clify.json.filterProbes`. NEVER blanket-mark unprobed flags; that's the anti-pattern this issue exists to fix. +> 5. **Probe log.** Phase 5 must write a `.clify.json.filterProbes` array — one entry per filter probed (or skipped): `{ resource, filter, baselineCount, filteredCount, status: "verified" | "broken" | "untested", note?: "..." }`. The validator reads this to enforce that `brokenListFilters: [...]` is justified by a real probe, not guessed. +> +> **Two opt-in action-def annotations** drive this in the runtime (substrate: `examples/exemplar-cli/lib/quirks.mjs` + bin runtime). The exemplar itself doesn't pre-populate either — they're dormant until a generated `commands/.mjs` opts in. > > - **`queryFlags: [...]`** — opt in for any `POST` create whose docs list a foreign-key as a **URL query parameter** instead of a body field (e.g. Zoho's `POST /creditnotes ?invoice_id=`, `POST /salesreturns ?salesorder_id=`, `POST /vendorcredits ?bill_id=`). The body equivalent is silently dropped on these APIs and the resulting record has no FK to the source. Detect these in Phase 2 by reading the docs' "Query Parameters" table for every `POST` — anything that names a sibling-resource id goes here. > -> - **`brokenListFilters: [...]`** — opt in for any `GET …/list` filter the upstream API silently ignores (HTTP 200, full unfiltered list). Detect via probe: compare row count from `-- FAKE-NONEXISTENT-XYZ` against unfiltered baseline. Equal counts → broken. The runtime drops the filter from the wire, fetches the full list, filters client-side, and prints a stderr note. +> - **`brokenListFilters: [...]`** — opt in ONLY for filters the per-flag probe above proved are silently ignored. The runtime drops the filter from the wire, fetches the full list, filters client-side (with the documented `match` mode — `equals`, `startswith`, `contains` — see Phase 3 substrate), and prints a stderr note. > -> Do NOT declare a filter in `flags` without checking it works against a live endpoint or against the docs. Pre-v0.5 generations guessed `listFilters` from naming convention; that produced CLIs that lied to users (filter passed → CLI shows help → API ignores it → user gets unfiltered results with no warning). The Fix Coffee Zoho rollout's `refund_cleanup_audit.py` mis-classified every SO as having returns until this was caught. +> Pre-v0.5 generations guessed `listFilters` from naming convention; that produced CLIs that lied to users (filter passed → CLI shows help → API ignores it → user gets unfiltered results with no warning). The Fix Coffee Zoho rollout's `refund_cleanup_audit.py` mis-classified every SO as having returns until this was caught. - `commands/.mjs` — replace the exemplar's items/orders/item-variants with the parsed resources. One file per resource. Each default-exports `{ name, actions, buildPayload? }`. - `bin/-cli.mjs` — update the imports and `COMMANDS` array to reflect the new resource set; everything else stays. @@ -120,7 +128,8 @@ Never declare done while either the gate or the verification subagent has unreso - Don't delete the `redactHeaders` call in `lib/api.mjs` — dry-run output must not leak credentials. The gate scans dry-run output and will hard-fail on a leak. - Don't write nuance prose into `knowledge/` if `.clify.json.nuances.*` isn't set; the gate cross-references them. - Don't put auth tokens in source. The gate scans for them and will flag real-shaped tokens. -- Don't declare list-filter flags you haven't verified work upstream. Either probe them (FAKE-value vs baseline row counts) or omit them. If they're documented but silently ignored, declare them in `brokenListFilters` so the runtime falls back to a client-side filter; never silently expose a flag that lies. +- Don't declare list-filter flags you haven't verified work upstream. Probe each one individually (`-- ` vs unfiltered baseline) and log the result in `.clify.json.filterProbes`. If documented but silently ignored, declare in `brokenListFilters` so the runtime falls back to client-side filtering; never silently expose a flag that lies. Don't blanket-mark every filter on a resource as broken from one failed probe — the validator's `filter-coverage` check hard-fails on that pattern. +- Don't normalize away the suffix on `*_startswith` / `*_contains` / `*_start` / `*_end` filter variants. The suffix IS the filter mode. Emit each documented variant as its own flag with the documented name verbatim. - Don't route a foreign-key into the body when the upstream docs list it as a query parameter on a `POST` create. Use `queryFlags` so the runtime puts it on the URL — most "convert from X" modes work ONLY via the query string and silently no-op via the body. ## Edge cases diff --git a/test/clify.test.mjs b/test/clify.test.mjs index 5e023a1..885bfde 100644 --- a/test/clify.test.mjs +++ b/test/clify.test.mjs @@ -175,6 +175,83 @@ test("validate: missing BASE_URL override → manifest fail", async () => { } finally { rmSync(root, { recursive: true, force: true }); } }); +// ---------- filter-coverage (v0.6+) ---------- + +test("validate: filter-coverage passes when filterProbes covers list actions", async () => { + const r = await validate(EXEMPLAR, { skipTests: true }); + // exemplar declares one filter (orders.status) and one matching probe + assert.ok( + r.results.some((x) => x.ok && /filter-coverage:.*individually probed/.test(x.name)), + "expected filter-coverage pass", + ); +}); + +test("validate: list action with filter flags but no probes → coverage fail", async () => { + const { root, repo } = freshCopy(); + try { + // Strip the existing filterProbes from .clify.json so orders.status + // (which is filter-shaped) becomes an unprobed declared filter. + const cpath = join(repo, ".clify.json"); + const cfg = JSON.parse(readFileSync(cpath, "utf8")); + delete cfg.filterProbes; + writeFileSync(cpath, JSON.stringify(cfg, null, 2)); + const r = await validate(repo, { skipTests: true }); + assert.ok( + r.results.some((x) => !x.ok && x.category === "coverage" && /filter-coverage/.test(x.name)), + `expected filter-coverage failure; got results=${JSON.stringify(r.results.filter((x) => !x.ok), null, 2)}`, + ); + } finally { rmSync(root, { recursive: true, force: true }); } +}); + +test("validate: blanket-marked filters with no verified probes → coverage fail", async () => { + const { root, repo } = freshCopy(); + try { + // Mutate orders.list to declare every filter-shaped flag as broken + // AND set the filterProbes to all 'broken' status — the v0.5 + // blanket-mark anti-pattern. + const ordersPath = join(repo, "commands/orders.mjs"); + let src = readFileSync(ordersPath, "utf8"); + // Inject brokenListFilters: ["status"] at the ACTION level (after + // the entire flags block, before the action's own closing brace). + // The flags block ends with `\n },\n` at indent 6. + src = src.replace( + /(\n flags:\s*\{[\s\S]*?\n \},)/, + `$1\n brokenListFilters: ["status"],`, + ); + writeFileSync(ordersPath, src); + + const cpath = join(repo, ".clify.json"); + const cfg = JSON.parse(readFileSync(cpath, "utf8")); + cfg.filterProbes = [ + { resource: "orders", filter: "status", baselineCount: 5, filteredCount: 5, status: "broken" }, + ]; + writeFileSync(cpath, JSON.stringify(cfg, null, 2)); + + const r = await validate(repo, { skipTests: true }); + assert.ok( + r.results.some((x) => !x.ok && x.category === "coverage" && /filter-coverage/.test(x.name) && /blanket-marked|every filter/.test(JSON.stringify(x))), + `expected blanket-mark failure; got results=${JSON.stringify(r.results.filter((x) => !x.ok), null, 2)}`, + ); + } finally { rmSync(root, { recursive: true, force: true }); } +}); + +test("validate: untested filterProbe surfaces as a warning, not a failure", async () => { + const { root, repo } = freshCopy(); + try { + const cpath = join(repo, ".clify.json"); + const cfg = JSON.parse(readFileSync(cpath, "utf8")); + cfg.filterProbes = [ + { resource: "orders", filter: "status", baselineCount: 5, filteredCount: 5, status: "untested", note: "no test creds" }, + ]; + writeFileSync(cpath, JSON.stringify(cfg, null, 2)); + const r = await validate(repo, { skipTests: true }); + // No filter-coverage hard-fail. + assert.ok(!r.results.some((x) => !x.ok && /filter-coverage/.test(x.name)), + `untested probes should warn, not fail; got ${JSON.stringify(r.results.filter((x) => !x.ok), null, 2)}`); + assert.ok(r.warnings.some((w) => /untested/.test(w)), `expected untested warning; got ${JSON.stringify(r.warnings)}`); + } finally { rmSync(root, { recursive: true, force: true }); } +}); + // ---------- syncCheck ---------- test("syncCheck: detects identical content as unchanged", async () => {