From fc5ca178a25d508185d531720f992ab4471c24b6 Mon Sep 17 00:00:00 2001 From: Behruz Nassre Esfahani Date: Tue, 23 Jun 2026 16:34:08 -0700 Subject: [PATCH] test(#1178): consolidate duplicated agent-roster helper into tests/helpers (#1420) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(#1178): consolidate duplicated agent-roster helper into tests/helpers The "list gsd-*.md agent files, strip .md, sort" derivation was hand-duplicated across the suite (two listAgentFiles(), an identical agentFilesOnDisk(), and inline readdir blocks). Add tests/helpers/agent-roster.cjs exporting listAgentFiles(agentsDir?) and route the genuinely-identical source-roster sites through it. Semantically-different sites (installed-dest dirs, absolute-path returns, .toml-inclusive Codex rosters, full-.md-filename readers, the uniform multi-family inventory table) are left intact, each with a one-line comment. Test-only; no production code touched. Co-Authored-By: Claude Opus 4.8 (1M context) * test(#1178): note AGENTS_DIR export is for future call sites Review nit: clarify that the currently-unused AGENTS_DIR export is intentional — available for future tests needing the canonical source agents path. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) Co-authored-by: Tom Boucher --- tests/agent-classification-parity.test.cjs | 15 ++----- tests/agent-frontmatter.test.cjs | 6 +-- ...gent-required-reading-consistency.test.cjs | 8 ++-- tests/bug-3245-codex-toml-floats.test.cjs | 2 + ...-research-insert-phase-agent-refs.test.cjs | 2 + ...g-3677-agent-colon-namespace-leak.test.cjs | 2 + tests/bug-570-codex-leak-scanner.test.cjs | 2 + tests/codex-config.test.cjs | 6 ++- tests/copilot-install.test.cjs | 10 +++-- tests/helpers/agent-roster.cjs | 42 +++++++++++++++++++ tests/install-minimal-hooks.test.cjs | 2 + tests/install.test.cjs | 2 + tests/inventory-manifest-sync.test.cjs | 3 ++ tests/model-profiles.test.cjs | 11 ++--- 14 files changed, 82 insertions(+), 31 deletions(-) create mode 100644 tests/helpers/agent-roster.cjs diff --git a/tests/agent-classification-parity.test.cjs b/tests/agent-classification-parity.test.cjs index 3b3820a90..2d4877f5b 100644 --- a/tests/agent-classification-parity.test.cjs +++ b/tests/agent-classification-parity.test.cjs @@ -18,6 +18,7 @@ const { describe, test } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('node:fs'); const path = require('node:path'); +const { listAgentFiles } = require('./helpers/agent-roster.cjs'); const ROOT = path.resolve(__dirname, '..'); const AGENTS_MD = path.join(ROOT, 'docs', 'AGENTS.md'); @@ -156,17 +157,6 @@ function parseInventoryMd(raw) { return result; } -/** - * List all agents/gsd-*.md basenames (without .md extension). - */ -function listAgentFiles() { - return fs - .readdirSync(AGENTS_DIR) - .filter((f) => /^gsd-.*\.md$/.test(f)) - .map((f) => f.replace(/\.md$/, '')) - .sort(); -} - // --------------------------------------------------------------------------- // Load and parse // --------------------------------------------------------------------------- @@ -176,7 +166,8 @@ const rawInventoryMd = fs.readFileSync(INVENTORY_MD, 'utf8'); const { primaryHeadings, advancedHeadings } = parseAgentsMd(rawAgentsMd); const inventoryMap = parseInventoryMd(rawInventoryMd); -const agentFiles = listAgentFiles(); +// Canonical source roster (sorted gsd-* basenames without .md) — shared helper. +const agentFiles = listAgentFiles(AGENTS_DIR); // --------------------------------------------------------------------------- // Robustness guards — must pass before any assertion block runs diff --git a/tests/agent-frontmatter.test.cjs b/tests/agent-frontmatter.test.cjs index 56af97659..97ecd6754 100644 --- a/tests/agent-frontmatter.test.cjs +++ b/tests/agent-frontmatter.test.cjs @@ -16,14 +16,14 @@ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); +const { listAgentFiles } = require('./helpers/agent-roster.cjs'); const AGENTS_DIR = path.join(__dirname, '..', 'agents'); const WORKFLOWS_DIR = path.join(__dirname, '..', 'gsd-core', 'workflows'); const COMMANDS_DIR = path.join(__dirname, '..', 'commands', 'gsd'); -const ALL_AGENTS = fs.readdirSync(AGENTS_DIR) - .filter(f => f.startsWith('gsd-') && f.endsWith('.md')) - .map(f => f.replace('.md', '')); +// Sorted basenames (without `.md`); reads below re-add `.md` via `name + '.md'`. +const ALL_AGENTS = listAgentFiles(AGENTS_DIR); const FILE_WRITING_AGENTS = ALL_AGENTS.filter(name => { const content = fs.readFileSync(path.join(AGENTS_DIR, name + '.md'), 'utf-8'); diff --git a/tests/agent-required-reading-consistency.test.cjs b/tests/agent-required-reading-consistency.test.cjs index 1d5ecd59a..bf93ebd72 100644 --- a/tests/agent-required-reading-consistency.test.cjs +++ b/tests/agent-required-reading-consistency.test.cjs @@ -15,12 +15,14 @@ const { test, describe } = require('node:test'); const assert = require('node:assert/strict'); const fs = require('fs'); const path = require('path'); +const { listAgentFiles } = require('./helpers/agent-roster.cjs'); const AGENTS_DIR = path.join(__dirname, '..', 'agents'); -const ALL_AGENTS = fs.readdirSync(AGENTS_DIR) - .filter(f => f.startsWith('gsd-') && f.endsWith('.md')) - .map(f => f.replace('.md', '')); +// Sorted basenames (without `.md`). Every use below generates an independent +// per-agent test and reads each file via `agent + '.md'`; nothing here depends +// on registration order, so the sorted helper roster is behaviorally identical. +const ALL_AGENTS = listAgentFiles(AGENTS_DIR); // ─── No Legacy files_to_read Blocks ──────────────────────────────────────── diff --git a/tests/bug-3245-codex-toml-floats.test.cjs b/tests/bug-3245-codex-toml-floats.test.cjs index 94167d2b7..725144a93 100644 --- a/tests/bug-3245-codex-toml-floats.test.cjs +++ b/tests/bug-3245-codex-toml-floats.test.cjs @@ -391,6 +391,8 @@ describe('#3245 — idempotent rollback reverts skills/, agents/, and VERSION', } // agents/ — GSD writes gsd-*.md and gsd-*.toml here. All must be absent. + // Not the shared listAgentFiles() helper: reads the INSTALLED Codex dest + // dir and is .toml-inclusive, so its semantics differ from the source roster. const agentsDir = path.join(codexHome, 'agents'); if (fs.existsSync(agentsDir)) { const gsdAgents = fs.readdirSync(agentsDir) diff --git a/tests/bug-3605-stale-research-insert-phase-agent-refs.test.cjs b/tests/bug-3605-stale-research-insert-phase-agent-refs.test.cjs index 4508c6f0a..6f3c46929 100644 --- a/tests/bug-3605-stale-research-insert-phase-agent-refs.test.cjs +++ b/tests/bug-3605-stale-research-insert-phase-agent-refs.test.cjs @@ -33,6 +33,8 @@ const RETIRED_COMMANDS = [ '/gsd-analyze-dependencies', ]; +// Not the shared listAgentFiles() helper: this returns ABSOLUTE paths (consumed +// by scanForRetired below as readFileSync targets), not stripped basenames. function listAgentFiles() { return fs .readdirSync(AGENTS_DIR) diff --git a/tests/bug-3677-agent-colon-namespace-leak.test.cjs b/tests/bug-3677-agent-colon-namespace-leak.test.cjs index b76044f7f..0355db8c8 100644 --- a/tests/bug-3677-agent-colon-namespace-leak.test.cjs +++ b/tests/bug-3677-agent-colon-namespace-leak.test.cjs @@ -202,6 +202,8 @@ describe('bug #3677 — agent body colon-namespace leak (Claude / Qwen / Hermes) test('E1: every agents/gsd-*.md transforms clean — no roster colon refs survive', () => { const agentsDir = path.join(REPO_ROOT, 'agents'); const offenders = []; + // Not the shared listAgentFiles() helper: this needs full `.md` filenames + // (not stripped basenames) to readFileSync + transform each agent body. for (const f of fs.readdirSync(agentsDir)) { if (!f.startsWith('gsd-') || !f.endsWith('.md')) continue; const src = fs.readFileSync(path.join(agentsDir, f), 'utf-8'); diff --git a/tests/bug-570-codex-leak-scanner.test.cjs b/tests/bug-570-codex-leak-scanner.test.cjs index e0ad8b186..74a38b76a 100644 --- a/tests/bug-570-codex-leak-scanner.test.cjs +++ b/tests/bug-570-codex-leak-scanner.test.cjs @@ -83,6 +83,8 @@ describe('#570 — Codex leak scanner sub-bugs', { concurrency: false }, () => { withCodexHome(codexHome, () => install(true, 'codex')); const agentsDir = path.join(codexHome, 'agents'); + // Not the shared listAgentFiles() helper: this reads the INSTALLED Codex + // dest dir and filters .toml (not source .md), so its semantics differ. // Confirm that Codex actually wrote .toml agent files — if none exist the // test is vacuous and we should fail loudly. const tomlFiles = fs.existsSync(agentsDir) diff --git a/tests/codex-config.test.cjs b/tests/codex-config.test.cjs index eb6f1908f..b08b3a1c2 100644 --- a/tests/codex-config.test.cjs +++ b/tests/codex-config.test.cjs @@ -1644,7 +1644,9 @@ describe('installCodexConfig (integration)', () => { const { installCodexConfig } = require('../bin/install.js'); installCodexConfig(tmpTarget, agentsSrc); - // Collect all .toml files: per-agent files in agents/ plus top-level config.toml + // Collect all .toml files: per-agent files in agents/ plus top-level config.toml. + // Not the shared listAgentFiles() helper: reads the INSTALLED target dir and + // collects generated .toml (absolute paths), not the source .md roster. const agentsDir = path.join(tmpTarget, 'agents'); const tomlFiles = fs.readdirSync(agentsDir) .filter(f => f.endsWith('.toml')) @@ -1669,6 +1671,8 @@ describe('installCodexConfig (integration)', () => { const { installCodexConfig } = require('../bin/install.js'); installCodexConfig(tmpTarget, agentsSrc); + // Not the shared listAgentFiles() helper: reads the INSTALLED target dir and + // filters generated gsd-*.toml output, not the source .md roster. const agentsDir = path.join(tmpTarget, 'agents'); const tomlFiles = fs.readdirSync(agentsDir) .filter((file) => file.startsWith('gsd-') && file.endsWith('.toml')); diff --git a/tests/copilot-install.test.cjs b/tests/copilot-install.test.cjs index de7cfe06b..eaa6b8eca 100644 --- a/tests/copilot-install.test.cjs +++ b/tests/copilot-install.test.cjs @@ -25,6 +25,7 @@ const path = require('path'); const os = require('os'); const fs = require('fs'); const { parseFrontmatter, createTempDir, cleanup } = require('./helpers.cjs'); +const { listAgentFiles } = require('./helpers/agent-roster.cjs'); const { getDirName, @@ -857,10 +858,11 @@ describe('Copilot agent conversion - real files', () => { }); test('all 18 agents convert without error', () => { + // Not the shared listAgentFiles() helper: this needs full `.md` filenames + // (not stripped basenames) to readFileSync each agent below. const agents = fs.readdirSync(agentsSrc) .filter(f => f.startsWith('gsd-') && f.endsWith('.md')); - const expectedAgentCount = fs.readdirSync(agentsSrc) - .filter(f => f.startsWith('gsd-') && f.endsWith('.md')).length; + const expectedAgentCount = listAgentFiles(agentsSrc).length; assert.strictEqual(agents.length, expectedAgentCount, `expected ${expectedAgentCount} agents, got ${agents.length}`); for (const agentFile of agents) { @@ -1350,8 +1352,8 @@ const crypto = require('crypto'); const INSTALL_PATH = path.join(__dirname, '..', 'bin', 'install.js'); const EXPECTED_SKILLS = fs.readdirSync(path.join(__dirname, '..', 'commands', 'gsd')) .filter(f => f.endsWith('.md')).length; -const EXPECTED_AGENTS = fs.readdirSync(path.join(__dirname, '..', 'agents')) - .filter(f => f.startsWith('gsd-') && f.endsWith('.md')).length; +// Source-roster count (gsd-*.md basenames) — shared helper. +const EXPECTED_AGENTS = listAgentFiles().length; function runCopilotInstall(cwd) { const env = { ...process.env }; diff --git a/tests/helpers/agent-roster.cjs b/tests/helpers/agent-roster.cjs new file mode 100644 index 000000000..4b5003481 --- /dev/null +++ b/tests/helpers/agent-roster.cjs @@ -0,0 +1,42 @@ +'use strict'; + +/** + * Shared helper for the canonical shipped-agent roster. + * + * Several tests derive "the set of agents we ship" from the source `agents/` + * directory via `fs.readdirSync(...).filter(/^gsd-.*\.md$/)`. This consolidates + * that hand-duplicated logic into one canonical SOURCE-roster derivation. + * + * NOTE: This returns the SOURCE roster (basenames without `.md`, sorted). Sites + * with different semantics — installed-destination dirs, absolute-path returns, + * or `.toml`-inclusive Codex rosters — must NOT use this helper. + */ + +const fs = require('node:fs'); +const path = require('node:path'); + +// Canonical source agents directory: /agents, relative to this +// helper at tests/helpers/. Matches the path the consolidated call sites used. +const AGENTS_DIR = path.join(__dirname, '..', '..', 'agents'); + +/** + * List shipped agent basenames (without the `.md` extension), sorted. + * + * @param {string} [agentsDir] Override for the source agents directory. + * Defaults to the canonical `/agents`. + * @returns {string[]} Sorted `gsd-*` basenames with `.md` stripped. + */ +function listAgentFiles(agentsDir = AGENTS_DIR) { + return fs + .readdirSync(agentsDir) + .filter((f) => /^gsd-.*\.md$/.test(f)) + .map((f) => f.replace(/\.md$/, '')) + .sort(); +} + +module.exports = { + // AGENTS_DIR is exported (not yet consumed by a call site) so future tests that + // need the canonical source agents path can reuse it instead of rediscovering it. + AGENTS_DIR, + listAgentFiles, +}; diff --git a/tests/install-minimal-hooks.test.cjs b/tests/install-minimal-hooks.test.cjs index 2da12e539..94dd61bab 100644 --- a/tests/install-minimal-hooks.test.cjs +++ b/tests/install-minimal-hooks.test.cjs @@ -393,6 +393,8 @@ describe('install: on-disk skill files match manifest for --minimal', () => { const onDisk = collectSkillBasenamesOnDisk(configDir); const inManifest = manifestSkillSet(manifest); assert.deepStrictEqual([...onDisk].sort(), [...inManifest].sort()); + // Not the shared listAgentFiles() helper: asserts on the INSTALLED + // dest dir (must be empty in --minimal mode), not the source roster. const agentsDir = path.join(configDir, 'agents'); if (fs.existsSync(agentsDir)) { const gsdAgents = fs.readdirSync(agentsDir) diff --git a/tests/install.test.cjs b/tests/install.test.cjs index 53a4c38eb..2e87d5833 100644 --- a/tests/install.test.cjs +++ b/tests/install.test.cjs @@ -606,6 +606,8 @@ for (const runtime of ['hermes', 'qwen']) { }); test('agents contain no CLAUDE.md or Claude Code references', () => { + // Not the shared listAgentFiles() helper: walks the INSTALLED dest dir + // and returns absolute paths (for leak scanning), not the source roster. const agentsDir = path.join(tmpDir, getDirName(runtime), 'agents'); assert.ok(fs.existsSync(agentsDir)); diff --git a/tests/inventory-manifest-sync.test.cjs b/tests/inventory-manifest-sync.test.cjs index 68f4a7405..d59b77f37 100644 --- a/tests/inventory-manifest-sync.test.cjs +++ b/tests/inventory-manifest-sync.test.cjs @@ -15,6 +15,9 @@ const path = require('node:path'); const ROOT = path.resolve(__dirname, '..'); const MANIFEST_PATH = path.join(ROOT, 'docs', 'INVENTORY-MANIFEST.json'); +// The `agents` row is NOT swapped to the shared listAgentFiles() helper: it is one +// row in a uniform multi-family table (each with its own filter/toName + an isFile +// guard); folding only agents in would break that uniformity. const FAMILIES = [ { name: 'agents', dir: path.join(ROOT, 'agents'), filter: (f) => /^gsd-.*\.md$/.test(f), toName: (f) => f.replace(/\.md$/, '') }, { name: 'commands', dir: path.join(ROOT, 'commands', 'gsd'), filter: (f) => f.endsWith('.md'), toName: (f) => '/gsd-' + f.replace(/\.md$/, '') }, diff --git a/tests/model-profiles.test.cjs b/tests/model-profiles.test.cjs index 9d19706b5..9094f9bc8 100644 --- a/tests/model-profiles.test.cjs +++ b/tests/model-profiles.test.cjs @@ -21,6 +21,7 @@ const { const { resolveModelInternal } = require('../gsd-core/bin/lib/model-resolver.cjs'); const { createTempProject, cleanup } = require('./helpers.cjs'); +const { listAgentFiles } = require('./helpers/agent-roster.cjs'); // ─── temp-project helpers ────────────────────────────────────────────────────── @@ -32,18 +33,12 @@ function writeConfig(tmpDir, obj) { ); } -function agentFilesOnDisk() { - return fs.readdirSync(path.join(__dirname, '..', 'agents')) - .filter((f) => /^gsd-.*\.md$/.test(f)) - .map((f) => f.replace(/\.md$/, '')) - .sort(); -} - // ─── MODEL_PROFILES data integrity ──────────────────────────────────────────── describe('MODEL_PROFILES', () => { test('contains every shipped gsd agent file on disk (#3229)', () => { - const expectedAgents = agentFilesOnDisk(); + // Canonical source roster (sorted gsd-* basenames without .md) — shared helper. + const expectedAgents = listAgentFiles(); const actualAgents = Object.keys(MODEL_PROFILES).sort(); assert.deepStrictEqual(actualAgents, expectedAgents); });