diff --git a/skills/rig/SKILL.md b/skills/rig/SKILL.md index e895d80..75a4200 100644 --- a/skills/rig/SKILL.md +++ b/skills/rig/SKILL.md @@ -53,7 +53,7 @@ Defaults: `name: "agent"`, `model: "small"`, `maxTurns: 4`, string input/output, | Numeric schema choice | `s.int` for counts/line numbers; `s.number` for measurements and ratios | | Optional versus nullable | `s.optional(shape)` for omission; `s.nullable(shape)` for explicit `null` | | Custom model-callable operation | `defineTool(name, { description, parameters, handler })` | -| Structured-output retries | `maxTurns` on the agent plus `addons: repair()` | +| Structured-output retries | `maxTurns` on the agent plus `addons: [repair()]` | | Retry with final-turn warning | `addons: [steering(), repair()]` in that order | Prompt intents are declarative instructions, not in-process operations. Prefer file intents over `cat` and workspace paths over large in-memory strings. @@ -65,6 +65,8 @@ Prompt intents are declarative instructions, not in-process operations. Prefer f - `defineTool` uses the two-argument config form. Use `s.object({ ... })` for object-shaped parameters — plain `{ key: s.string }` loses handler arg type inference. Arrow callbacks in handlers must have explicit type annotations: `.map((line: string) => ...)`. - `repair()` takes no arguments. Turn budgets belong on the agent spec or invocation. - Stable settings belong in `agent({ ... })`; per-run `model`, `maxTurns`, `timeout`, and `signal` belong on invocation; `agent.use()` accepts only addons. +- Valid `agent()` fields: `name`, `instructions`, `input`, `output`, `model`, `maxTurns`, `addons`, `agents`, `systemMessage`, `tools`. Misspelled keys (e.g. `instructions2`) are silently dropped; the linter flags them. +- Handler functions that return string literals must use `as const` to preserve the literal type for enum schema comparison. ## Runnable output diff --git a/skills/rig/eslint/index.js b/skills/rig/eslint/index.js index 15765d4..dd410e0 100644 --- a/skills/rig/eslint/index.js +++ b/skills/rig/eslint/index.js @@ -4,6 +4,8 @@ import noObjectLiteralRecord from "./rules/no-object-literal-record.js"; import repairNoArgs from "./rules/repair-no-args.js"; import noImplicitAnyInToolHandler from "./rules/no-implicit-any-in-tool-handler.js"; import preferPGlobOverBashFind from "./rules/prefer-p-glob-over-bash-find.js"; +import noInvalidAgentFields from "./rules/no-invalid-agent-fields.js"; +import enumReturnNeedsAsConst from "./rules/enum-return-needs-as-const.js"; export default { meta: { @@ -16,5 +18,7 @@ export default { "repair-no-args": repairNoArgs, "no-implicit-any-in-tool-handler": noImplicitAnyInToolHandler, "prefer-p-glob-over-bash-find": preferPGlobOverBashFind, + "no-invalid-agent-fields": noInvalidAgentFields, + "enum-return-needs-as-const": enumReturnNeedsAsConst, }, }; diff --git a/skills/rig/eslint/lint.js b/skills/rig/eslint/lint.js index 38fe633..356839e 100644 --- a/skills/rig/eslint/lint.js +++ b/skills/rig/eslint/lint.js @@ -9,9 +9,10 @@ import { scanTokens as scanNoObjectLiteralRecord } from "./rules/no-object-liter import { scanTokens as scanRepairNoArgs } from "./rules/repair-no-args.js"; import { scanTokens as scanNoImplicitAnyInToolHandler } from "./rules/no-implicit-any-in-tool-handler.js"; import { scanTokens as scanPreferPGlobOverBashFind } from "./rules/prefer-p-glob-over-bash-find.js"; +import { scanTokens as scanNoInvalidAgentFields } from "./rules/no-invalid-agent-fields.js"; const ignoredDirectories = new Set([".git", "node_modules"]); -const tokenRules = [scanDefineToolArgCount, scanAgentsMustBeObject, scanNoObjectLiteralRecord, scanRepairNoArgs, scanNoImplicitAnyInToolHandler, scanPreferPGlobOverBashFind]; +const tokenRules = [scanDefineToolArgCount, scanAgentsMustBeObject, scanNoObjectLiteralRecord, scanRepairNoArgs, scanNoImplicitAnyInToolHandler, scanPreferPGlobOverBashFind, scanNoInvalidAgentFields]; function tokenize(source) { const tokens = []; diff --git a/skills/rig/eslint/rules/enum-return-needs-as-const.js b/skills/rig/eslint/rules/enum-return-needs-as-const.js new file mode 100644 index 0000000..af7a6d0 --- /dev/null +++ b/skills/rig/eslint/rules/enum-return-needs-as-const.js @@ -0,0 +1,69 @@ +export default { + meta: { + type: "suggestion", + docs: { + description: + "Require 'as const' on string literal returns inside defineTool handlers to prevent type widening", + }, + fixable: "code", + schema: [], + messages: { + needsAsConst: + "String literal returned without 'as const'. TypeScript widens the type to string; add 'as const' to preserve the literal type.", + }, + }, + create(context) { + /** + * Walk up the ancestor chain to determine whether this return statement is + * directly inside a `handler:` property function body (arrow or regular). + */ + function insideHandlerProp(node) { + let current = node.parent; + while (current) { + const t = current.type; + if ( + t === "ArrowFunctionExpression" + || t === "FunctionExpression" + || t === "FunctionDeclaration" + ) { + const parent = current.parent; + if ( + parent?.type === "Property" + && !parent.computed + && parent.key?.type === "Identifier" + && parent.key.name === "handler" + ) { + return true; + } + // Stop at any other function boundary + return false; + } + current = current.parent; + } + return false; + } + + return { + ReturnStatement(node) { + const { argument } = node; + if ( + !argument + || argument.type !== "Literal" + || typeof argument.value !== "string" + ) { + return; + } + + if (!insideHandlerProp(node)) return; + + context.report({ + node: argument, + messageId: "needsAsConst", + fix(fixer) { + return fixer.replaceText(argument, `${argument.raw} as const`); + }, + }); + }, + }; + }, +}; diff --git a/skills/rig/eslint/rules/no-invalid-agent-fields.js b/skills/rig/eslint/rules/no-invalid-agent-fields.js new file mode 100644 index 0000000..5b9a4be --- /dev/null +++ b/skills/rig/eslint/rules/no-invalid-agent-fields.js @@ -0,0 +1,138 @@ +const validAgentFields = new Set([ + "name", + "instructions", + "input", + "output", + "model", + "maxTurns", + "addons", + "agents", + "systemMessage", + "tools", +]); + +function findClosing(tokens, openingIndex) { + const pairs = { "(": ")", "{": "}", "[": "]" }; + const open = tokens[openingIndex]?.value; + const close = pairs[open]; + if (!close) return undefined; + let depth = 0; + for (let i = openingIndex; i < tokens.length; i += 1) { + if (tokens[i].value === open) depth += 1; + if (tokens[i].value === close) depth -= 1; + if (depth === 0) return i; + } + return undefined; +} + +export function scanTokens(tokens) { + const problems = []; + + for (let index = 0; index <= tokens.length - 2; index += 1) { + const [fn, openParen] = tokens.slice(index, index + 2); + if ( + tokens[index - 1]?.value === "." + || fn.value !== "agent" + || openParen.value !== "(" + ) { + continue; + } + + const closeParenIndex = findClosing(tokens, index + 1); + if (closeParenIndex === undefined) continue; + + // Find the opening brace of the first argument + let braceIndex = index + 2; + while (braceIndex < closeParenIndex && tokens[braceIndex]?.value !== "{") { + braceIndex += 1; + } + if (braceIndex >= closeParenIndex) continue; + + const closeBraceIndex = findClosing(tokens, braceIndex); + if (closeBraceIndex === undefined || closeBraceIndex > closeParenIndex) continue; + + // Walk the top-level entries of the object literal + let depth = 0; + for (let i = braceIndex + 1; i < closeBraceIndex; i += 1) { + const t = tokens[i]; + if (t.value === "(" || t.value === "{" || t.value === "[") { + depth += 1; + continue; + } + if (t.value === ")" || t.value === "}" || t.value === "]") { + depth -= 1; + continue; + } + if (depth !== 0) continue; + + // A bare identifier followed by ":" is a property key + const next = tokens[i + 1]; + if ( + /^[A-Za-z_$][\w$]*$/.test(t.value) + && next?.value === ":" + && !validAgentFields.has(t.value) + ) { + problems.push({ + start: t.start, + end: t.end, + message: `"${t.value}" is not a valid agent spec field. Valid fields: ${[...validAgentFields].join(", ")}.`, + kind: "no-invalid-agent-fields", + edits: [], + }); + } + } + } + + return problems; +} + +export default { + meta: { + type: "problem", + docs: { + description: "Disallow unknown fields on the agent() spec object", + }, + fixable: null, + schema: [], + messages: { + invalidField: + "\"{{field}}\" is not a valid agent spec field. Valid fields: {{valid}}.", + }, + }, + create(context) { + return { + CallExpression(node) { + const { callee, arguments: args } = node; + if ( + callee.type !== "Identifier" + || callee.name !== "agent" + || args.length === 0 + || args[0].type !== "ObjectExpression" + ) { + return; + } + + for (const prop of args[0].properties) { + if (prop.type !== "Property") continue; // skip spread + if (prop.computed) continue; // skip computed keys + const keyName = + prop.key.type === "Identifier" + ? prop.key.name + : prop.key.type === "Literal" + ? String(prop.key.value) + : null; + if (keyName === null || validAgentFields.has(keyName)) continue; + + context.report({ + node: prop.key, + messageId: "invalidField", + data: { + field: keyName, + valid: [...validAgentFields].join(", "), + }, + }); + } + }, + }; + }, +}; diff --git a/skills/rig/references/linting.md b/skills/rig/references/linting.md index c819315..69ff435 100644 --- a/skills/rig/references/linting.md +++ b/skills/rig/references/linting.md @@ -106,6 +106,40 @@ The rule autofixes simple `find DIR -name PATTERN` commands (no other predicates Use `p.bash` when you need shell command output (counts, content, sorting, etc.); use `p.glob` when you want the model to iterate over a list of matching paths. +### `rig/no-invalid-agent-fields` + +Unknown or misspelled fields on the `agent()` spec object are silently ignored at runtime. TypeScript does not always catch them due to excess property checking gaps, so this rule flags them explicitly. + +Valid fields: `name`, `instructions`, `input`, `output`, `model`, `maxTurns`, `addons`, `agents`, `systemMessage`, `tools`. + +```ts +// Invalid — "instructions2" is silently dropped at runtime +agent({ instructions2: p`Review the diff.` }) + +// Valid +agent({ instructions: p`Review the diff.` }) +``` + +No autofix (the correct field name must be supplied by the author). + +### `rig/enum-return-needs-as-const` + +A `defineTool` handler that returns a bare string literal widens the return type to `string`, which breaks the enum schema comparison. TypeScript reports the error at the `output` schema, not at the `return` site. + +```ts +// Invalid — TypeScript widens "stable" to string +handler: ({ risk }) => { + return "stable"; +} + +// Valid — literal type preserved +handler: ({ risk }) => { + return "stable" as const; +} +``` + +The rule autofixes by inserting `as const` after the string literal. + ## Adding rules Put rule implementations in `skills/rig/eslint/rules/`, export them from `skills/rig/eslint/index.js`, add the equivalent skill-local check to `skills/rig/eslint/lint.js`, and cover both in `src/eslint-rules.test.js`. diff --git a/src/eslint-rules.test.js b/src/eslint-rules.test.js index 00f7438..4acff99 100644 --- a/src/eslint-rules.test.js +++ b/src/eslint-rules.test.js @@ -6,6 +6,8 @@ import rule from "../skills/rig/eslint/rules/no-object-literal-record.js"; import repairNoArgsRule from "../skills/rig/eslint/rules/repair-no-args.js"; import noImplicitAnyRule from "../skills/rig/eslint/rules/no-implicit-any-in-tool-handler.js"; import preferPGlobRule from "../skills/rig/eslint/rules/prefer-p-glob-over-bash-find.js"; +import noInvalidAgentFieldsRule from "../skills/rig/eslint/rules/no-invalid-agent-fields.js"; +import enumReturnNeedsAsConstRule from "../skills/rig/eslint/rules/enum-return-needs-as-const.js"; describe("define-tool-arg-count", () => { it.each([ @@ -558,3 +560,297 @@ describe("prefer-p-glob-over-bash-find", () => { expect(reports).toHaveLength(0); }); }); + +describe("no-invalid-agent-fields", () => { + it.each([ + // All valid fields + "const a = agent({ name: \"a\", instructions: p`x`, input: s.string, output: s.string, model: \"small\", maxTurns: 4, addons: repair(), agents: { sub }, systemMessage: \"\", tools: [] });", + // Subset of valid fields + "const a = agent({ model: \"small\", output: s.string });", + // agent as a key (not a call) — must not be flagged + "const x = { agent: { instructions2: 1 } };", + // Method call obj.agent() — must not be flagged + "const x = obj.agent({ instructions2: \"x\" });", + // Inside string + "const text = 'agent({ instructions2: 1 })';", + // Computed key — not flagged + "const a = agent({ [key]: value });", + ])("accepts %s", (source) => { + const problems = lintSource(source).filter((p) => p.kind === "no-invalid-agent-fields"); + expect(problems).toEqual([]); + }); + + it.each([ + "const a = agent({ instructions2: p`x` });", + "const a = agent({ model: \"small\", instruction: p`x` });", + "const a = agent({ desciption: \"typo\" });", + ])("flags %s", (source) => { + const problems = lintSource(source).filter((p) => p.kind === "no-invalid-agent-fields"); + expect(problems).toHaveLength(1); + }); + + it("flags multiple invalid fields in one agent call", () => { + const source = "const a = agent({ instructions2: p`x`, modell: \"small\" });"; + const problems = lintSource(source).filter((p) => p.kind === "no-invalid-agent-fields"); + expect(problems).toHaveLength(2); + }); + + it("does not autofix (no edits)", () => { + const source = "const a = agent({ instructions2: p`x` });"; + const problems = lintSource(source).filter((p) => p.kind === "no-invalid-agent-fields"); + expect(problems[0].edits).toEqual([]); + expect(fixSource(source, problems)).toBe(source); + }); + + it("keeps the ESLint rule aligned", () => { + const reports = []; + const visitor = noInvalidAgentFieldsRule.create({ + sourceCode: {}, + report: (problem) => reports.push(problem), + }); + + visitor.CallExpression({ + type: "CallExpression", + callee: { type: "Identifier", name: "agent" }, + arguments: [ + { + type: "ObjectExpression", + properties: [ + { + type: "Property", + computed: false, + key: { type: "Identifier", name: "instructions2" }, + value: { type: "Literal", value: "x" }, + }, + { + type: "Property", + computed: false, + key: { type: "Identifier", name: "model" }, + value: { type: "Literal", value: "small" }, + }, + ], + }, + ], + }); + + expect(reports).toHaveLength(1); + expect(reports[0].messageId).toBe("invalidField"); + expect(reports[0].data.field).toBe("instructions2"); + }); + + it("does not flag valid agent fields via ESLint rule", () => { + const reports = []; + const visitor = noInvalidAgentFieldsRule.create({ + sourceCode: {}, + report: (problem) => reports.push(problem), + }); + + visitor.CallExpression({ + type: "CallExpression", + callee: { type: "Identifier", name: "agent" }, + arguments: [ + { + type: "ObjectExpression", + properties: [ + { + type: "Property", + computed: false, + key: { type: "Identifier", name: "instructions" }, + value: { type: "Literal", value: "x" }, + }, + ], + }, + ], + }); + + expect(reports).toHaveLength(0); + }); + + it("does not flag member-expression agent calls via ESLint rule", () => { + const reports = []; + const visitor = noInvalidAgentFieldsRule.create({ + sourceCode: {}, + report: (problem) => reports.push(problem), + }); + + visitor.CallExpression({ + type: "CallExpression", + callee: { + type: "MemberExpression", + object: { type: "Identifier", name: "obj" }, + property: { type: "Identifier", name: "agent" }, + }, + arguments: [ + { + type: "ObjectExpression", + properties: [ + { + type: "Property", + computed: false, + key: { type: "Identifier", name: "instructions2" }, + value: { type: "Literal", value: "x" }, + }, + ], + }, + ], + }); + + expect(reports).toHaveLength(0); + }); +}); + +describe("enum-return-needs-as-const", () => { + // This rule is ESLint-only: the tokenizer cannot track scope to limit + // the check to handler: property bodies. The ESLint rule walks ancestors + // to verify the return is inside a handler function. + + function makeHandlerReturn(raw, value = "stable") { + const retNode = { + type: "ReturnStatement", + argument: { type: "Literal", value, raw }, + }; + // Build ancestor chain: BlockStatement → ArrowFunctionExpression → Property(handler) + retNode.parent = { + type: "BlockStatement", + parent: { + type: "ArrowFunctionExpression", + parent: { + type: "Property", + computed: false, + key: { type: "Identifier", name: "handler" }, + }, + }, + }; + return retNode; + } + + it("flags a bare string literal return inside a handler", () => { + const reports = []; + const visitor = enumReturnNeedsAsConstRule.create({ + sourceCode: {}, + report: (problem) => reports.push(problem), + }); + + visitor.ReturnStatement(makeHandlerReturn("\"stable\"")); + + expect(reports).toHaveLength(1); + expect(reports[0].messageId).toBe("needsAsConst"); + expect(reports[0].fix({ replaceText: (_node, text) => text })) + .toBe("\"stable\" as const"); + }); + + it("flags single-quoted string literals inside a handler", () => { + const reports = []; + const visitor = enumReturnNeedsAsConstRule.create({ + sourceCode: {}, + report: (problem) => reports.push(problem), + }); + + visitor.ReturnStatement(makeHandlerReturn("'missing'", "missing")); + + expect(reports).toHaveLength(1); + expect(reports[0].fix({ replaceText: (_node, text) => text })) + .toBe("'missing' as const"); + }); + + it("does not flag a non-string return inside a handler", () => { + const reports = []; + const visitor = enumReturnNeedsAsConstRule.create({ + sourceCode: {}, + report: (problem) => reports.push(problem), + }); + + const retNode = { type: "ReturnStatement", argument: { type: "Literal", value: 42, raw: "42" } }; + retNode.parent = { + type: "BlockStatement", + parent: { + type: "ArrowFunctionExpression", + parent: { + type: "Property", + computed: false, + key: { type: "Identifier", name: "handler" }, + }, + }, + }; + visitor.ReturnStatement(retNode); + + expect(reports).toHaveLength(0); + }); + + it("does not flag return outside a handler (top-level function)", () => { + const reports = []; + const visitor = enumReturnNeedsAsConstRule.create({ + sourceCode: {}, + report: (problem) => reports.push(problem), + }); + + const retNode = { + type: "ReturnStatement", + argument: { type: "Literal", value: "stable", raw: "\"stable\"" }, + parent: { + type: "BlockStatement", + parent: { + type: "FunctionDeclaration", + parent: null, + }, + }, + }; + visitor.ReturnStatement(retNode); + + expect(reports).toHaveLength(0); + }); + + it("does not flag return inside a non-handler property function", () => { + const reports = []; + const visitor = enumReturnNeedsAsConstRule.create({ + sourceCode: {}, + report: (problem) => reports.push(problem), + }); + + const retNode = { + type: "ReturnStatement", + argument: { type: "Literal", value: "stable", raw: "\"stable\"" }, + parent: { + type: "BlockStatement", + parent: { + type: "ArrowFunctionExpression", + parent: { + type: "Property", + computed: false, + key: { type: "Identifier", name: "transform" }, + }, + }, + }, + }; + visitor.ReturnStatement(retNode); + + expect(reports).toHaveLength(0); + }); + + it("does not flag return when argument is not a literal (e.g. identifier)", () => { + const reports = []; + const visitor = enumReturnNeedsAsConstRule.create({ + sourceCode: {}, + report: (problem) => reports.push(problem), + }); + + const retNode = { + type: "ReturnStatement", + argument: { type: "Identifier", name: "value" }, + parent: { + type: "BlockStatement", + parent: { + type: "ArrowFunctionExpression", + parent: { + type: "Property", + computed: false, + key: { type: "Identifier", name: "handler" }, + }, + }, + }, + }; + visitor.ReturnStatement(retNode); + + expect(reports).toHaveLength(0); + }); +});