From 1848b636c4d19c0ee5899a2388ca443345a27a8e Mon Sep 17 00:00:00 2001 From: MarioCadenas Date: Wed, 5 Aug 2026 14:53:06 +0200 Subject: [PATCH] feat(shared): lint rule flags aiSearch indexes missing columns Add ai-search-index-requires-columns to `appkit lint`. In production the plugin does not auto-discover an index's columns (dev-only), so a missing or empty `columns` fails at query time. This is the pre-deploy static catch. Uses a structural traversal via a new optional `find` escape hatch on Rule, since "every index in indexes:{...} has non-empty columns" isn't a single ast-grep pattern. Flags bare aiSearch(), aiSearch({}), empty indexes, and any index missing columns or with columns:[]; passes dynamic/const/spread configs to avoid false positives. Extracts lintSource() so rules can be tested against in-memory source. Signed-off-by: MarioCadenas --- packages/shared/src/cli/commands/lint.test.ts | 88 ++++++++++++ packages/shared/src/cli/commands/lint.ts | 135 ++++++++++++++++-- 2 files changed, 209 insertions(+), 14 deletions(-) create mode 100644 packages/shared/src/cli/commands/lint.test.ts diff --git a/packages/shared/src/cli/commands/lint.test.ts b/packages/shared/src/cli/commands/lint.test.ts new file mode 100644 index 000000000..958ab65e1 --- /dev/null +++ b/packages/shared/src/cli/commands/lint.test.ts @@ -0,0 +1,88 @@ +import { describe, expect, test } from "vitest"; +import { lintSource } from "./lint"; + +const RULE = "ai-search-index-requires-columns"; + +/** Lint TS source, keeping only this rule's violations. */ +function columnsViolations(code: string) { + return lintSource(code, "server.ts").filter((v) => v.rule === RULE); +} + +describe("ai-search-index-requires-columns", () => { + test("passes an index with columns", () => { + const code = `aiSearch({ indexes: { demo: { columns: ["id", "text"] } } });`; + expect(columnsViolations(code)).toHaveLength(0); + }); + + test("flags an index missing columns", () => { + const code = `aiSearch({ indexes: { demo: { queryType: "hybrid" } } });`; + const v = columnsViolations(code); + expect(v).toHaveLength(1); + expect(v[0].code).toContain("demo"); + }); + + test("flags an index with an empty columns array", () => { + const code = `aiSearch({ indexes: { demo: { columns: [] } } });`; + expect(columnsViolations(code)).toHaveLength(1); + }); + + test("flags an empty array with whitespace", () => { + const code = `aiSearch({ indexes: { demo: { columns: [ ] } } });`; + expect(columnsViolations(code)).toHaveLength(1); + }); + + test("flags bare aiSearch() — relies on the columnless env default index", () => { + expect(columnsViolations("aiSearch();")).toHaveLength(1); + }); + + test("flags aiSearch({}) with no indexes key", () => { + expect(columnsViolations("aiSearch({});")).toHaveLength(1); + }); + + test("flags aiSearch({ indexes: {} }) with no configured index", () => { + expect(columnsViolations("aiSearch({ indexes: {} });")).toHaveLength(1); + }); + + test("flags only the index missing columns among several", () => { + const code = `aiSearch({ + indexes: { + ok: { columns: ["id"] }, + bad: { queryType: "hybrid" }, + alsoOk: { columns: ["title"] }, + }, + });`; + const v = columnsViolations(code); + expect(v).toHaveLength(1); + expect(v[0].code).toContain("bad"); + }); + + test("passes a columns reference to a constant (can't prove empty)", () => { + const code = `aiSearch({ indexes: { demo: { columns: DEFAULT_COLUMNS } } });`; + expect(columnsViolations(code)).toHaveLength(0); + }); + + test("passes a dynamically-built indexes object", () => { + const code = `aiSearch({ indexes: buildIndexes() });`; + expect(columnsViolations(code)).toHaveLength(0); + }); + + test("passes a spread index config (can't inspect)", () => { + const code = `aiSearch({ indexes: { demo: { ...base } } });`; + expect(columnsViolations(code)).toHaveLength(0); + }); + + test("passes a dynamic config argument", () => { + expect(columnsViolations("aiSearch(myConfig);")).toHaveLength(0); + }); + + test("ignores a non-aiSearch call with the same shape", () => { + const code = `genie({ indexes: { demo: { queryType: "hybrid" } } });`; + expect(columnsViolations(code)).toHaveLength(0); + }); + + test("skipped on test files (includeTests: false)", () => { + const code = `aiSearch();`; + const testFileViolations = lintSource(code, "server.ts", undefined, true); + expect(testFileViolations.some((v) => v.rule === RULE)).toBe(false); + }); +}); diff --git a/packages/shared/src/cli/commands/lint.ts b/packages/shared/src/cli/commands/lint.ts index 7284e277d..6fa9c4b19 100644 --- a/packages/shared/src/cli/commands/lint.ts +++ b/packages/shared/src/cli/commands/lint.ts @@ -1,16 +1,101 @@ import fs from "node:fs"; import path from "node:path"; -import { Lang, parse } from "@ast-grep/napi"; +import { Lang, parse, type SgNode } from "@ast-grep/napi"; import { Command } from "commander"; -interface Rule { +interface BaseRule { id: string; - pattern: string; message: string; includeTests?: boolean; filter?: (code: string) => boolean; } +/** + * A rule matches via exactly one of `pattern` (an ast-grep pattern for + * `root.findAll`) or `find` (an escape hatch for checks a single pattern can't + * express). Discriminated union so a rule with neither or both fails to compile. + */ +type Rule = + | (BaseRule & { pattern: string; find?: never }) + | (BaseRule & { find: (root: SgNode) => SgNode[]; pattern?: never }); + +/** Value node of an object literal's `key: value` pair, or undefined. */ +function objectPropertyValue(obj: SgNode, key: string): SgNode | undefined { + for (const pair of obj.children()) { + if (pair.kind() !== "pair") continue; + const k = pair.field("key"); + // Strip quotes so `columns` and `"columns"` are treated the same. + if (k && k.text().replace(/^["']|["']$/g, "") === key) { + return pair.field("value") ?? undefined; + } + } + return undefined; +} + +/** True for `[]` / `[ ]` — an array literal with no elements. */ +function isEmptyArrayLiteral(node: SgNode): boolean { + return node.kind() === "array" && node.children().every((c) => !c.isNamed()); +} + +/** + * Flags `aiSearch(...)` configs whose indexes lack usable `columns`. Production + * does not auto-discover columns (dev-only), so a missing or empty `columns` + * fails at query time. Passes anything it can't statically prove bad (constant + * or dynamically-built columns/indexes) to avoid false positives. + */ +function findAiSearchIndexesMissingColumns(root: SgNode): SgNode[] { + const flagged: SgNode[] = []; + + for (const call of root.findAll("aiSearch($$$ARGS)")) { + const argNodes = + call + .field("arguments") + ?.children() + .filter((c) => c.isNamed()) ?? []; + + // Bare aiSearch(): relies on the env default index (no columns). + if (argNodes.length === 0) { + flagged.push(call); + continue; + } + + // aiSearch(dynamicConfig) — not an object literal, can't inspect. Pass. + const configObj = argNodes[0].kind() === "object" ? argNodes[0] : undefined; + if (!configObj) continue; + + const indexes = objectPropertyValue(configObj, "indexes"); + + // No `indexes` key => falls back to the env default index (no columns). + if (!indexes) { + flagged.push(call); + continue; + } + if (indexes.kind() !== "object") continue; // dynamic indexes: pass. + + const indexPairs = indexes.children().filter((c) => c.kind() === "pair"); + + // Empty `indexes: {}` => no configured index, same as bare. + if (indexPairs.length === 0) { + flagged.push(call); + continue; + } + + for (const pair of indexPairs) { + const idxObj = pair.field("value"); + if (!idxObj || idxObj.kind() !== "object") continue; // dynamic: pass. + // A spread (`{ ...base }`) may carry `columns` we can't see: pass. + if (idxObj.children().some((c) => c.kind() === "spread_element")) + continue; + const columns = objectPropertyValue(idxObj, "columns"); + if (!columns || isEmptyArrayLiteral(columns)) { + flagged.push(pair); // points at the offending index alias. + } + } + } + + return flagged; +} + const rules: Rule[] = [ { id: "no-double-type-assertion", @@ -48,6 +133,13 @@ const rules: Rule[] = [ " is a development-only variant picker and must not be shipped. Finalize the chosen before deploying.", includeTests: false, }, + { + id: "ai-search-index-requires-columns", + message: + "AI Search index has no `columns`; it will fail in production (columns are auto-discovered only in dev). Set `columns` explicitly on each index.", + includeTests: false, + find: findAiSearchIndexesMissingColumns, + }, ]; function isTestFile(filePath: string, rootDir: string): boolean { @@ -85,24 +177,25 @@ interface Violation { code: string; } -function lintFile( +/** + * Runs all applicable rules against a file's source. Exported so tests can lint + * an in-memory string without touching disk. + */ +export function lintSource( + content: string, filePath: string, - rules: Rule[], - rootDir: string, + activeRules: Rule[] = rules, + isTest = false, ): Violation[] { const violations: Violation[] = []; - const content = fs.readFileSync(filePath, "utf-8"); const lang = filePath.endsWith(".tsx") ? Lang.Tsx : Lang.TypeScript; - const testFile = isTestFile(filePath, rootDir); - - const ast = parse(lang, content); - const root = ast.root(); + const root = parse(lang, content).root(); - for (const rule of rules) { + for (const rule of activeRules) { // skip rules that don't apply to test files - if (testFile && rule.includeTests === false) continue; + if (isTest && rule.includeTests === false) continue; - const matches = root.findAll(rule.pattern); + const matches = rule.find ? rule.find(root) : root.findAll(rule.pattern); for (const match of matches) { const code = match.text(); @@ -124,6 +217,20 @@ function lintFile( return violations; } +function lintFile( + filePath: string, + activeRules: Rule[], + rootDir: string, +): Violation[] { + const content = fs.readFileSync(filePath, "utf-8"); + return lintSource( + content, + filePath, + activeRules, + isTestFile(filePath, rootDir), + ); +} + /** * Lint command implementation */