feat: strict agent/command validators to prevent Issue #34-class regr…

.gemini/styleguide.md

@@ -53,6 +53,10 @@ The codebase consists of:
 - Do NOT include Claude-style names (`search_files`, `replace_in_file`,
   `Read`, `Edit`) — those are not valid in Gemini CLI.
 - No `model` or `color` field (Gemini CLI schema does not support them).
+- The authoritative allowlist lives in `scripts/lib/gemini-tools.js` and is
+  enforced by `scripts/ci/validate-agents.js` (run on every PR). Update the
+  lib first if the Gemini CLI tool surface changes; this file is the
+  human-facing mirror.
 
 ## Documentation
 

scripts/ci/validate-agents.js

@@ -1,12 +1,19 @@
 #!/usr/bin/env node
 /**
- * Validate agent markdown files have required frontmatter
+ * Validate agent markdown files against the Gemini CLI agent loader rules.
+ *
+ * Catches Issue #34-class regressions: invalid tool names, MCP tools listed
+ * in frontmatter, Claude-style names, and unsupported keys (color/model).
+ *
+ * The validation rules live in scripts/lib/gemini-tools.js so they stay in
+ * sync with .gemini/styleguide.md.
  */
 
+'use strict';
+
 const path = require('path');
 const { extractFrontmatter, validateFiles } = require('../lib/validator');
-
-const REQUIRED_FIELDS = ['tools'];
+const { validateAgentFrontmatter } = require('../lib/gemini-tools');
 
 validateFiles({
   dir: path.join(__dirname, '../../agents'),
@@ -15,13 +22,7 @@ validateFiles({
   validate(content) {
     const frontmatter = extractFrontmatter(content);
     if (!frontmatter) return ['Missing frontmatter'];
-
-    const errors = [];
-    for (const field of REQUIRED_FIELDS) {
-      if (!frontmatter[field]) {
-        errors.push(`Missing required field: ${field}`);
-      }
-    }
+    const errors = validateAgentFrontmatter(frontmatter);
     return errors.length > 0 ? errors : null;
   }
 });

scripts/ci/validate-commands.js

@@ -1,17 +1,75 @@
 #!/usr/bin/env node
 /**
- * Validate command files are non-empty and readable
+ * Validate command TOML files.
+ *
+ * Beyond the original "non-empty file" check this enforces:
+ *   - filename uses the `egc-` prefix (PR #38 — avoids collisions with
+ *     Gemini CLI built-in commands like `/plan` and `/docs`)
+ *   - a non-empty `description = "..."` field exists
+ *
+ * `EGC_PREFIX_EXEMPT` lists filenames that are intentionally allowed to
+ * skip the prefix (none today; kept as an explicit allowlist so future
+ * exceptions are reviewed instead of slipping through).
  */
 
+'use strict';
+
 const path = require('path');
 const { validateFiles } = require('../lib/validator');
 
-validateFiles({
-  dir: path.join(__dirname, '../../commands'),
-  label: 'command',
-  extension: '.toml',
-  validate(content) {
-    if (content.trim().length === 0) return ['Empty command file'];
-    return null;
+const EGC_PREFIX_EXEMPT = new Set([]);
+
+// TOML allows four string forms for `description`:
+//   description = "basic"      description = 'literal'
+//   description = """basic"""  description = '''literal'''
+// Try the triple-quoted form first; otherwise the non-greedy single-quote
+// regex matches the empty span between the first two quotes of a `"""`
+// opener and produces a false "Empty description field" error.
+const DESCRIPTION_TRIPLE_RE = /^\s*description\s*=\s*("""|''')([\s\S]*?)\1/m;
+const DESCRIPTION_SINGLE_RE = /^\s*description\s*=\s*(['"])([^]*?)\1/m;
+
+function extractDescriptionValue(content) {
+  const triple = DESCRIPTION_TRIPLE_RE.exec(content);
+  if (triple) return triple[2];
+  const single = DESCRIPTION_SINGLE_RE.exec(content);
+  return single ? single[2] : null;
+}
+
+function validateCommandContent(content, filename) {
+  const errors = [];
+
+  if (content.trim().length === 0) {
+    errors.push('Empty command file');
+    return errors;
+  }
+
+  if (!EGC_PREFIX_EXEMPT.has(filename) && !filename.startsWith('egc-')) {
+    errors.push(
+      `Filename must start with "egc-" prefix (collides with Gemini CLI ` +
+      `built-ins otherwise — see PR #38).`
+    );
+  }
+
+  const description = extractDescriptionValue(content);
+  if (description === null) {
+    errors.push('Missing required field: description');
+  } else if (description.trim().length === 0) {
+    errors.push('Empty description field');
   }
-});
+
+  return errors;
+}
+
+if (require.main === module) {
+  validateFiles({
+    dir: path.join(__dirname, '../../commands'),
+    label: 'command',
+    extension: '.toml',
+    validate(content, filename) {
+      const errors = validateCommandContent(content, filename);
+      return errors.length > 0 ? errors : null;
+    }
+  });
+}
+
+module.exports = { validateCommandContent };

scripts/lib/gemini-tools.js

@@ -0,0 +1,209 @@
+/**
+ * Gemini CLI tool allowlist + frontmatter validation helpers.
+ *
+ * Single source of truth for what `tools:` and other frontmatter keys are
+ * accepted by the Gemini CLI agent loader. `.gemini/styleguide.md` mirrors
+ * this list for human-facing review guidance — keep both in sync.
+ *
+ * Issue #34 history: Gemini CLI rejects agents whose frontmatter references
+ * Claude-style names (Read, Edit), legacy names (search_files), MCP tools
+ * (mcp__*), or unsupported keys (color, model). The validators below catch
+ * those before they hit the loader.
+ */
+
+'use strict';
+
+const VALID_TOOLS = Object.freeze([
+  'read_file',
+  'read_many_files',
+  'write_file',
+  'replace',
+  'glob',
+  'search_file_content',
+  'list_directory',
+  'run_shell_command',
+  'save_memory',
+  'web_fetch',
+  'google_web_search'
+]);
+
+const VALID_TOOL_SET = new Set(VALID_TOOLS);
+
+// Forbidden top-level frontmatter keys. The Gemini CLI agent schema only
+// supports `name`, `description`, `tools`. Anything else (notably `color`
+// and `model`) causes the loader to reject the file.
+const FORBIDDEN_FRONTMATTER_KEYS = Object.freeze(['color', 'model']);
+
+const REQUIRED_FRONTMATTER_KEYS = Object.freeze(['name', 'description', 'tools']);
+
+// Claude Code → Gemini CLI tool name migration table. Used to produce
+// actionable error messages when contributors copy frontmatter from a
+// Claude-style agent.
+const CLAUDE_TO_GEMINI = Object.freeze({
+  Read: 'read_file',
+  Write: 'write_file',
+  Edit: 'replace',
+  Bash: 'run_shell_command',
+  Glob: 'glob',
+  Grep: 'search_file_content',
+  LS: 'list_directory',
+  WebFetch: 'web_fetch',
+  WebSearch: 'google_web_search',
+  search_files: 'search_file_content',
+  replace_in_file: 'replace'
+});
+
+/**
+ * Parse the value of a `tools:` frontmatter line into a list of tool names.
+ *
+ * Accepts both quoted and unquoted YAML inline arrays:
+ *   tools: ["read_file", "write_file"]
+ *   tools: [read_file, write_file]
+ *
+ * Returns null if the value is malformed (missing brackets, etc.) so callers
+ * can surface a frontmatter parse error instead of silently passing.
+ *
+ * @param {string} rawValue
+ * @returns {string[]|null}
+ */
+function parseToolsField(rawValue) {
+  if (typeof rawValue !== 'string') return null;
+  const trimmed = rawValue.trim();
+  if (!trimmed.startsWith('[') || !trimmed.endsWith(']')) return null;
+
+  const inner = trimmed.slice(1, -1).trim();
+  if (inner.length === 0) return [];
+
+  return inner
+    .split(',')
+    .map(part => part.trim().replace(/^["']|["']$/g, ''))
+    .filter(part => part.length > 0);
+}
+
+/**
+ * Compute Levenshtein distance for "did you mean" suggestions.
+ * Bounded loop, no allocations beyond the working row.
+ */
+function levenshtein(a, b) {
+  if (a === b) return 0;
+  if (a.length === 0) return b.length;
+  if (b.length === 0) return a.length;
+
+  let prev = new Array(b.length + 1);
+  let curr = new Array(b.length + 1);
+  for (let j = 0; j <= b.length; j++) prev[j] = j;
+
+  for (let i = 1; i <= a.length; i++) {
+    curr[0] = i;
+    for (let j = 1; j <= b.length; j++) {
+      const cost = a[i - 1] === b[j - 1] ? 0 : 1;
+      curr[j] = Math.min(curr[j - 1] + 1, prev[j] + 1, prev[j - 1] + cost);
+    }
+    [prev, curr] = [curr, prev];
+  }
+  return prev[b.length];
+}
+
+/**
+ * Suggest the nearest valid tool name for a typo. Returns null if no
+ * candidate is close enough (distance > 4) to be useful.
+ */
+function suggestTool(badName) {
+  if (CLAUDE_TO_GEMINI[badName]) return CLAUDE_TO_GEMINI[badName];
+
+  let best = null;
+  let bestDist = Infinity;
+  for (const candidate of VALID_TOOLS) {
+    const d = levenshtein(badName.toLowerCase(), candidate);
+    if (d < bestDist) {
+      bestDist = d;
+      best = candidate;
+    }
+  }
+  return bestDist <= 4 ? best : null;
+}
+
+/**
+ * Validate a list of tool names against the Gemini CLI allowlist.
+ * Returns an array of human-readable error strings (empty on success).
+ *
+ * @param {string[]} tools
+ * @returns {string[]}
+ */
+function validateToolList(tools) {
+  const errors = [];
+  for (const tool of tools) {
+    if (VALID_TOOL_SET.has(tool)) continue;
+
+    if (tool.startsWith('mcp__')) {
+      errors.push(
+        `Invalid tool "${tool}": MCP tools must not be listed in agent ` +
+        `frontmatter — they are auto-discovered from configured MCP servers.`
+      );
+      continue;
+    }
+
+    const suggestion = suggestTool(tool);
+    const hint = suggestion ? ` Did you mean "${suggestion}"?` : '';
+    errors.push(`Invalid tool "${tool}".${hint}`);
+  }
+  return errors;
+}
+
+/**
+ * Validate the full frontmatter object of an agent file.
+ * @param {object} frontmatter - parsed key/value pairs
+ * @returns {string[]} list of error messages (empty on success)
+ */
+function validateAgentFrontmatter(frontmatter) {
+  const errors = [];
+  const has = key => Object.prototype.hasOwnProperty.call(frontmatter, key);
+  const isEmpty = value => value === null || value === undefined || value === '';
+
+  for (const key of REQUIRED_FRONTMATTER_KEYS) {
+    if (!has(key)) {
+      errors.push(`Missing required field: ${key}`);
+    } else if (isEmpty(frontmatter[key])) {
+      errors.push(`Empty required field: ${key}`);
+    }
+  }
+
+  for (const key of FORBIDDEN_FRONTMATTER_KEYS) {
+    if (has(key)) {
+      errors.push(
+        `Forbidden frontmatter key: "${key}" — Gemini CLI agent schema only ` +
+        `accepts ${REQUIRED_FRONTMATTER_KEYS.join(', ')}.`
+      );
+    }
+  }
+
+  // Run the tool-list parse only if the field has a non-empty value. The
+  // empty / missing cases are already covered above; running parseToolsField
+  // on `''` would just produce a redundant "Malformed tools field" message.
+  if (has('tools') && !isEmpty(frontmatter.tools)) {
+    const tools = parseToolsField(frontmatter.tools);
+    if (tools === null) {
+      errors.push(
+        `Malformed tools field: expected an inline YAML array like ` +
+        `[read_file, write_file].`
+      );
+    } else if (tools.length === 0) {
+      errors.push('Empty tools array — agents must declare at least one tool.');
+    } else {
+      errors.push(...validateToolList(tools));
+    }
+  }
+
+  return errors;
+}
+
+module.exports = {
+  VALID_TOOLS,
+  FORBIDDEN_FRONTMATTER_KEYS,
+  REQUIRED_FRONTMATTER_KEYS,
+  CLAUDE_TO_GEMINI,
+  parseToolsField,
+  suggestTool,
+  validateToolList,
+  validateAgentFrontmatter
+};