diff --git a/CONTEXT.md b/CONTEXT.md index 2f909f29a..4b4203420 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -160,8 +160,8 @@ A named, stable site on a host loop step (per-step `pre`/`post` plus per-wave in ### Capability State Resolver ADR-857 phase 4b unified resolver that composes the three toggle systems (install profile, runtime surface, config activation) into one per-capability view. ADDITIVE — install/surface/workflows untouched; currently consumed by nothing (phase-6 wiring out of scope). Source of truth: `gsd-core/bin/lib/capability-state.cjs` (generated from `src/capability-state.cts`). Interface: `resolveCapabilityState({ registry, installedSkills, surfacedSkills, config, cwd? }) → { capabilities: CapabilityStateEntry[] }` (pure, no I/O); `cmdCapabilityState(cwd, runtimeConfigDir, raw, opts)` (I/O entry point). CLI surface: `gsd-tools capability state [--config-dir ]` — emits `{ runtimeConfigDir, capabilities[] }`. Per-capability output: `{ id, tier, skills[], installed, surfaced, hooks[] }` where `installed` = every owned skill ∈ installedSkills (or `installedSkills==='*'`; vacuously true for empty-skills caps), `surfaced` = every owned skill ∈ surfacedSkills (vacuously true for empty-skills caps), `hooks` = `[{ point, kind: 'step'|'gate'|'contribution', when, active }]` derived from the cap's `steps`, `gates`, `contributions` arrays (no `when` → active=true; `when` resolved via `_resolveActivationValue` from loop-resolver). Capabilities sorted by `id` for determinism. Defensive: malformed registry → `{ capabilities: [] }`, never throws; inline literal `__proto__`/`constructor`/`prototype` prototype-pollution guard on capability id keys. `runtimeConfigDir` auto-detection falls back to `getGlobalConfigDir` based on env-var presence (CODEX_HOME → codex, CURSOR_CONFIG_DIR → cursor, GEMINI_CONFIG_DIR → gemini, CLAUDE_CONFIG_DIR → claude, default → claude/`~/.claude`). -### Capability Command Family [Planned] -ADR-959 (phase 4d) — a CLI command family (a top-level `gsd-tools` command and its subcommands) owned by a Capability via a new optional `commands: [{ family, module, router }]` field on the `feature` role. The Capability declares the `family` name, a first-party in-tree `module` (under `gsd-core/bin/lib/`), and the exported `router` — a standard `route*Command({ args, cwd, raw, error })` function identical in shape to the 12 existing host routers (so it routes through the stateless CommandRoutingHub via `routeCjsCommandFamily`, owning its own subcommand list and arg parsing). The registry materializes a `commandFamilies` index (`family → { capId, module, router }`); the formerly-dead `_dispatchNonFamily` shim becomes a real `dispatchCapabilityCommand` consulted in `runCommand`'s **`default` case** — an unmigrated command hits its hardcoded `case`; a migrated command's `case` is removed so it reaches `default` → registry → router, making collision structurally impossible. The registry *discovers* a router (it does not rebuild a handler table). First-party only; third-party command loading deferred. `graphify` is the first real cutover (bundling its command + skill + `isGraphifyEnabled` gate + `tier: full`), proven equivalent old-path vs new-path and serving as the phase-6 cutover template. +### Capability Command Family [Planned — mechanism built, unconsumed] +ADR-959 (phase 4d) — a CLI command family (a top-level `gsd-tools` command and its subcommands) owned by a Capability via a new optional `commands: [{ family, module, router }]` field on the `feature` role. The Capability declares the `family` name, a first-party in-tree `module` (under `gsd-core/bin/lib/`), and the exported `router` — a standard `route*Command({ args, cwd, raw, error })` function identical in shape to the 12 existing host routers (so it routes through the stateless CommandRoutingHub via `routeCjsCommandFamily`, owning its own subcommand list and arg parsing). The registry materializes a `commandFamilies` index (`family → { capId, module, router }`); the formerly-dead `_dispatchNonFamily` shim is replaced by a real `dispatchCapabilityCommand` (exported from `gsd-core/bin/gsd-tools.cjs`) consulted in `runCommand`'s **`default` case** — an unmigrated command hits its hardcoded `case`; a migrated command's `case` is removed so it reaches `default` → registry → router, making collision structurally impossible. The registry *discovers* a router (it does not rebuild a handler table). First-party only; third-party command loading deferred. **Mechanism built (4d-impl-1):** `commands` schema + validator + single-family-ownership cross-check in `gen-capability-registry.cjs`; `commandFamilies` index emitted in the generated `capability-registry.cjs` (currently `{}` — no capability declares commands yet); `dispatchCapabilityCommand` wired into `runCommand`'s `default` case (behavior-preserving today). **Pending next step (4d-impl-2 / pilot):** cut over `graphify` as the first real capability command family (bundling its command + skill + `isGraphifyEnabled` gate + `tier: full`), proven equivalent old-path vs new-path and serving as the phase-6 cutover template. ### Runtime Capability [Planned] A `role: runtime` variant of a Capability (a Capability carries `role: feature | runtime`) that projects GSD's produced artifacts (skills/agents/hooks/commands) onto one host CLI's conventions — config-surface format, artifact-layout kinds, command template, hooks manifest, sandbox tier. It is a declarative descriptor over a fixed first-party primitive vocabulary (not a code adapter); install composes active Feature Capabilities × the chosen Runtime Capability at the InstallPlan seam (ADR-0058). First-party runtimes are authored through the same descriptor a third party would write (dogfooding the interface); tier-1 (Claude Code, Codex, Antigravity) is fully tested, the other existing runtimes ship lower-tier, none dropped. Third-party runtime loading is deferred to a purely additive external loader + trust gate. diff --git a/gsd-core/bin/gsd-tools.cjs b/gsd-core/bin/gsd-tools.cjs index a8f5b9ff2..1d92d91f5 100755 --- a/gsd-core/bin/gsd-tools.cjs +++ b/gsd-core/bin/gsd-tools.cjs @@ -256,6 +256,122 @@ function _dispatchNonFamily({ registryCommand, registryArgs, legacyCommand, lega return false; } +// ─── ADR-959: Capability Command Dispatch ───────────────────────────────────── + +/** + * Dispatch a command via the capability registry's commandFamilies index. + * + * Consulted in the `default` case of `runCommand` BEFORE the unknown-command + * error is emitted. Returns: + * true — command was "consumed" (found in registry, or a dispatch error was + * emitted); "Unknown command" is suppressed in all consumed cases. + * false — command not found in the registry (including prototype-pollution + * guard hits and missing/empty commandFamilies); caller falls through + * to the existing unknown-command error path. + * Behavior-preserving when commandFamilies is empty ({}). + * + * Injectable for tests: + * - `registry` defaults to require('./lib/capability-registry.cjs') + * - `requireModule` defaults to a confinement-checked loader that resolves the + * module path relative to bin/lib/ and asserts it stays within that directory + * before requiring — defense-in-depth against corrupted/hand-edited registry entries. + * + * @param {object} opts + * @param {string} opts.command The command name (top-level gsd-tools command) + * @param {string[]} opts.args Remaining args passed to the router + * @param {string} opts.cwd Project working directory + * @param {boolean} opts.raw Raw output mode flag + * @param {Function} opts.error Error reporter (core.error) + * @param {object} [opts.registry] Injectable registry (for tests) + * @param {Function} [opts.requireModule] Injectable module loader (for tests) + * @returns {boolean} true if the command was dispatched, false otherwise + */ +function dispatchCapabilityCommand({ command, args, cwd, raw, error, registry, requireModule }) { + // Prototype-pollution guard: reject reserved property names as command keys + if (command === '__proto__' || command === 'constructor' || command === 'prototype') { + return false; + } + + // Resolve defaults (injectable for tests) + const reg = registry !== undefined ? registry : require('./lib/capability-registry.cjs'); + + // Default requireModule: confined to bin/lib/ — validate the module name is a + // safe bare .cjs basename (no path separators, no directory traversal), then + // resolve and assert confinement, then require the RESOLVED absolute path so + // the checked representation and the required representation are identical. + const libDir = path.join(__dirname, 'lib'); + const defaultRequireModule = function (m) { + // Step 1: validate m is a bare .cjs basename — same conservative pattern the + // generator uses. Rejects any value with path separators (/, \, ..) or + // missing the .cjs extension before we even touch the filesystem. + if (typeof m !== 'string' || !/^[A-Za-z0-9._-]+\.cjs$/.test(m)) { + throw new Error('capability module must be a bare .cjs basename: ' + JSON.stringify(m)); + } + // Step 2: confinement check — belt-and-suspenders even after the basename + // validation above. Resolved path must be inside libDir (not equal to it, + // and must start with libDir + sep so "libDir-suffix" can't sneak through). + const resolved = path.resolve(libDir, m); + if (resolved === libDir || !resolved.startsWith(libDir + path.sep)) { + throw new Error('capability module path escapes bin/lib/: ' + JSON.stringify(m)); + } + // Step 3: require the resolved absolute path — the SAME representation that + // was checked above, not the concatenated './lib/' + m string. + return require(resolved); + }; + const loadModule = requireModule !== undefined ? requireModule : defaultRequireModule; + + // Look up the command family in the registry + const families = reg && reg.commandFamilies; + if (!families || typeof families !== 'object') return false; + + const entry = families[command]; + if (!entry || typeof entry !== 'object') return false; + + // Resolve and call the router + let mod; + try { + mod = loadModule(entry.module); + } catch (_) { + // Module not found, load error, or confinement violation — surface a + // diagnostic and return true (consumed) so "Unknown command" is suppressed. + error('capability command "' + command + '" module "' + entry.module + '" failed to load'); + return true; // consumed — don't emit "Unknown command" + } + + // Own-property guard: prevent invoking inherited prototype methods + // (constructor, toString, hasOwnProperty, etc.) as a router when the registry + // entry names one of those. Must come before the typeof check. + if (!mod || !Object.prototype.hasOwnProperty.call(mod, entry.router)) { + error('capability command "' + command + '" router "' + entry.router + '" is not an own export of module "' + entry.module + '"'); + return true; // consumed — don't emit "Unknown command" + } + const fn = mod[entry.router]; + if (typeof fn !== 'function') { + // Router export not found — surface a diagnostic and return true (consumed) + // so "Unknown command" is suppressed. + error('capability command "' + command + '" router "' + entry.router + '" is not a function in module "' + entry.module + '"'); + return true; // consumed — don't emit "Unknown command" + } + + let _result; + try { + _result = fn({ args, cwd, raw, error }); + } catch (e) { + if (e instanceof ExitError) throw e; // intentional structured error from the router (honors --json-errors) — propagate untouched + error( + 'capability command "' + command + '" router "' + entry.router + '" in module "' + entry.module + '" threw: ' + (e && e.message ? e.message : String(e)), + ERROR_REASON.SDK_FAIL_FAST, + ); + } + if (_result && typeof _result.then === 'function') { + error( + 'capability command "' + command + '" router "' + entry.router + '" in module "' + entry.module + '" must be synchronous (returned a Promise); async capability routers are not supported.', + ERROR_REASON.SDK_FAIL_FAST, + ); + } + return true; +} + // ─── Arg parsing helpers ────────────────────────────────────────────────────── // ─── CLI Router ─────────────────────────────────────────────────────────────── @@ -1966,6 +2082,13 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand } default: { + // ADR-959: try capability-registry dispatch before emitting the unknown-command error. + // An unmigrated command still hits its hardcoded `case` above — untouched. + // A migrated command's `case` is removed at cutover, so it reaches here and + // dispatchCapabilityCommand routes it to the capability's registered router. + // With commandFamilies={} today, this always returns false and is a no-op. + if (dispatchCapabilityCommand({ command, args, cwd, raw, error })) break; + // #3243: if the caller passed a dotted form (e.g. "foo.bar"), the shim // above split it so `command` here is the head ("foo"). Use // originalCommand to reconstruct the original dotted form and suggest @@ -1987,4 +2110,12 @@ async function runCommand(command, args, cwd, raw, defaultValue, originalCommand } } -runMain(main); +// ─── CLI entry point ────────────────────────────────────────────────────────── +if (require.main === module) { + runMain(main); +} + +// ─── Exports (for tests) ────────────────────────────────────────────────────── +// ADR-959: export dispatchCapabilityCommand so tests can exercise it with +// synthetic registry + requireModule injections. +module.exports = { dispatchCapabilityCommand }; diff --git a/gsd-core/bin/lib/capability-registry.cjs b/gsd-core/bin/lib/capability-registry.cjs index ec7582a47..710076578 100644 --- a/gsd-core/bin/lib/capability-registry.cjs +++ b/gsd-core/bin/lib/capability-registry.cjs @@ -230,6 +230,8 @@ const configSchema = { const runtimes = {}; +const commandFamilies = {}; + const capabilityClusters = { "ui": [ "ui-phase", @@ -275,6 +277,7 @@ module.exports = { configKeys, configSchema, runtimes, + commandFamilies, capabilityClusters, profileMembership, requiresClosure, diff --git a/scripts/gen-capability-registry.cjs b/scripts/gen-capability-registry.cjs index 24ab9cf72..17ee3adbc 100644 --- a/scripts/gen-capability-registry.cjs +++ b/scripts/gen-capability-registry.cjs @@ -266,6 +266,77 @@ function validateCapability(cap, folderId) { return errors; } +/** + * ADR-959: Validate a single commands[] entry on a feature-role capability. + * { family: string, module: string, router: string, subcommands?: string[] } + * + * - family: non-empty string, no reserved names + * - module: non-empty string, no path traversal, no absolute paths, no "/" + * segments other than a bare basename (expected form: "foo.cjs") + * - router: non-empty string + * - subcommands: optional array of strings (doc/introspection only) + * + * @param {string} capId Capability id (for error messages) + * @param {*} entry The entry to validate + * @param {string} prefix Path prefix (e.g. "commands[0]") + * @returns {string[]} Array of error strings; empty = valid. + */ +function validateCommandEntry(capId, entry, prefix) { + const errors = []; + const ctx = 'capability "' + capId + '" ' + prefix; + + if (typeof entry !== 'object' || entry === null || Array.isArray(entry)) { + errors.push(ctx + ' must be an object with family, module, and router'); + return errors; + } + + // family: non-empty string, no reserved names + if (typeof entry.family !== 'string' || entry.family.length === 0) { + errors.push(ctx + '.family must be a non-empty string'); + } else if (entry.family === '__proto__' || entry.family === 'constructor' || entry.family === 'prototype') { + // S2a: inline literal reserved-name guard (CodeQL barrier) + errors.push(ctx + '.family "' + entry.family + '" is a reserved name'); + } + + // module: must be a safe bare basename matching /^[A-Za-z0-9._-]+\.cjs$/ — + // no path separators, no "..", no NUL bytes, no absolute paths, ends in .cjs. + // This conservative pattern subsumes all earlier traversal/absolute/separator checks. + if (typeof entry.module !== 'string' || entry.module.length === 0) { + errors.push(ctx + '.module must be a non-empty string'); + } else { + const mod = entry.module; + const SAFE_BASENAME = /^[A-Za-z0-9._-]+\.cjs$/; + if (!SAFE_BASENAME.test(mod)) { + errors.push( + ctx + '.module must be a safe bare basename (pattern: /^[A-Za-z0-9._-]+\\.cjs$/, no path separators, no "..", no NUL bytes, must end in ".cjs"); got: ' + + JSON.stringify(mod), + ); + } + } + + // router: non-empty string + if (typeof entry.router !== 'string' || entry.router.length === 0) { + errors.push(ctx + '.router must be a non-empty string'); + } + + // subcommands: optional array of non-empty strings (doc/introspection only) + if (entry.subcommands !== undefined) { + if (!Array.isArray(entry.subcommands)) { + errors.push(ctx + '.subcommands must be an array of strings if present'); + } else { + for (let i = 0; i < entry.subcommands.length; i++) { + if (typeof entry.subcommands[i] !== 'string') { + errors.push(ctx + '.subcommands[' + i + '] must be a string'); + } else if (entry.subcommands[i].length === 0) { + errors.push(ctx + '.subcommands[' + i + '] must be a non-empty string'); + } + } + } + } + + return errors; +} + function validateFeatureBody(cap) { const errors = []; @@ -282,6 +353,17 @@ function validateFeatureBody(cap) { } } + // ADR-959: optional commands array + if (cap.commands !== undefined) { + if (!Array.isArray(cap.commands)) { + errors.push('commands must be an array of {family, module, router} objects'); + } else { + for (let i = 0; i < cap.commands.length; i++) { + errors.push(...validateCommandEntry(cap.id || cap.role, cap.commands[i], 'commands[' + i + ']')); + } + } + } + if (!Array.isArray(cap.agents)) { errors.push('agents must be an array of strings'); } else { @@ -755,6 +837,7 @@ function validateCrossCapability(capMap, centralKeys) { // Ownership: one owner per skill stem + agent name const skillOwner = new Map(); // skill → capId const agentOwner = new Map(); // agent → capId + const familyOwner = new Map(); // command family → capId (ADR-959) for (const [capId, cap] of capMap) { if (cap.role !== 'feature') continue; for (const skill of cap.skills) { @@ -775,6 +858,20 @@ function validateCrossCapability(capMap, centralKeys) { agentOwner.set(agent, capId); } } + // ADR-959: single family ownership across the whole registry + if (Array.isArray(cap.commands)) { + for (const cmd of cap.commands) { + if (typeof cmd.family !== 'string' || cmd.family.length === 0) continue; // already reported + if (cmd.family === '__proto__' || cmd.family === 'constructor' || cmd.family === 'prototype') continue; + if (familyOwner.has(cmd.family)) { + errors.push( + 'command family "' + cmd.family + '" is owned by both "' + familyOwner.get(cmd.family) + '" and "' + capId + '"', + ); + } else { + familyOwner.set(cmd.family, capId); + } + } + } } // Config key ownership: exclusive AND absent from central schema @@ -1357,6 +1454,24 @@ function buildRegistry(capMap) { })); } + // ── ADR-959: commandFamilies index ───────────────────────────────────────── + // family → { capId, module, router } + // Built from all feature capabilities' commands arrays. + const commandFamilies = Object.create(null); + for (const [capId, cap] of capMap) { + // S2b: inline literal guard at each write site (CodeQL barrier) + if (capId === '__proto__' || capId === 'constructor' || capId === 'prototype') continue; + if (cap.role !== 'feature' || !Array.isArray(cap.commands)) continue; + for (const cmd of cap.commands) { + if (typeof cmd.family !== 'string' || cmd.family.length === 0) continue; + // S2b: inline literal guard at family key write site (CodeQL barrier) + if (cmd.family === '__proto__' || cmd.family === 'constructor' || cmd.family === 'prototype') continue; + if (typeof cmd.module !== 'string' || cmd.module.length === 0) continue; + if (typeof cmd.router !== 'string' || cmd.router.length === 0) continue; + commandFamilies[cmd.family] = { capId, module: cmd.module, router: cmd.router }; + } + } + // ── ADR-857 phase 4a: derived views ──────────────────────────────────────── const capabilityClusters = deriveCapabilityClusters(capMap); const profileMembership = deriveProfileMembership(capMap); @@ -1374,6 +1489,7 @@ function buildRegistry(capMap) { configKeys, configSchema, runtimes, + commandFamilies, capabilityClusters, profileMembership, // warnings are NOT serialized — returned only for caller consumption via stderr @@ -1417,6 +1533,17 @@ function serializeRegistry(registry, capMap) { lines.push('const runtimes = ' + JSON.stringify(registry.runtimes, null, 2) + ';'); lines.push(''); + // ADR-959: commandFamilies index — sort family keys for determinism. + const sortedCommandFamilies = Object.create(null); + const commandFamilyKeys = Object.keys(registry.commandFamilies || {}).sort(); + for (const family of commandFamilyKeys) { + // S2b: inline literal guard at write site (CodeQL barrier) + if (family === '__proto__' || family === 'constructor' || family === 'prototype') continue; + sortedCommandFamilies[family] = registry.commandFamilies[family]; + } + lines.push('const commandFamilies = ' + JSON.stringify(sortedCommandFamilies, null, 2) + ';'); + lines.push(''); + // ADR-857 phase 4a: derived views — globally sorted capIds for determinism. // FIX 2: collect ALL capIds across both views and sort globally so feature + runtime // capIds interleave correctly when both are present (phase 5 readiness). @@ -1486,6 +1613,7 @@ function serializeRegistry(registry, capMap) { lines.push(' configKeys,'); lines.push(' configSchema,'); lines.push(' runtimes,'); + lines.push(' commandFamilies,'); lines.push(' capabilityClusters,'); lines.push(' profileMembership,'); lines.push(' requiresClosure,'); @@ -1665,6 +1793,8 @@ module.exports = { deriveCapabilityClusters, deriveProfileMembership, runConsistencyGate, + // ADR-959: command entry validation + validateCommandEntry, // FIX 5 (lazy): PROFILE_RANK and CLUSTERS are loaded on first access via getters // so importing the generator on a fresh/unbuilt worktree doesn't fail at module load. get PROFILE_RANK() { return getInstallProfiles().PROFILE_RANK; }, diff --git a/tests/capability-command-dispatch.test.cjs b/tests/capability-command-dispatch.test.cjs new file mode 100644 index 000000000..6f57feadc --- /dev/null +++ b/tests/capability-command-dispatch.test.cjs @@ -0,0 +1,772 @@ +'use strict'; + +/** + * capability-command-dispatch.test.cjs — unit tests for dispatchCapabilityCommand. + * + * ADR-959 phase 4d-impl-1. + * Tests use synthetic registry + requireModule injections — no real bin/lib/ modules loaded. + * Covers: happy path dispatch, unknown command fallback, empty/missing registry, prototype + * pollution guard, router-not-a-function handling, module-load failure handling. + */ + +const { describe, test } = require('node:test'); +const assert = require('node:assert/strict'); + +const { dispatchCapabilityCommand } = require('../gsd-core/bin/gsd-tools.cjs'); + +// ─── Helpers ────────────────────────────────────────────────────────────────── + +/** + * Build a synthetic registry with a single commandFamilies entry. + */ +function makeRegistry(families) { + return { commandFamilies: families }; +} + +/** + * Build a requireModule that returns a module with a named router function. + * The router records its call ctx into `calls` array. + */ +function makeRequireModule(moduleName, routerName, calls) { + return function requireModule(m) { + if (m !== moduleName) throw new Error('unexpected module: ' + m); + const mod = {}; + mod[routerName] = function (ctx) { calls.push(ctx); }; + return mod; + }; +} + +// ─── 1. Happy path dispatch ─────────────────────────────────────────────────── + +describe('dispatchCapabilityCommand — happy path', () => { + test('dispatches to the registered router and returns true', () => { + const calls = []; + const registry = makeRegistry({ + foo: { capId: 'x', module: 'fake.cjs', router: 'routeFoo' }, + }); + const requireModule = makeRequireModule('fake.cjs', 'routeFoo', calls); + + const result = dispatchCapabilityCommand({ + command: 'foo', + args: ['bar', '--baz'], + cwd: '/some/path', + raw: false, + error: () => {}, + registry, + requireModule, + }); + + assert.strictEqual(result, true, 'dispatch should return true'); + assert.strictEqual(calls.length, 1, 'router should have been called once'); + assert.deepEqual(calls[0].args, ['bar', '--baz'], 'args forwarded'); + assert.strictEqual(calls[0].cwd, '/some/path', 'cwd forwarded'); + assert.strictEqual(calls[0].raw, false, 'raw forwarded'); + assert.strictEqual(typeof calls[0].error, 'function', 'error function forwarded'); + }); + + test('returns true and dispatches when raw=true', () => { + const calls = []; + const registry = makeRegistry({ + myCmd: { capId: 'c1', module: 'mycmd.cjs', router: 'routeMyCmd' }, + }); + const requireModule = makeRequireModule('mycmd.cjs', 'routeMyCmd', calls); + + const result = dispatchCapabilityCommand({ + command: 'myCmd', + args: [], + cwd: '/proj', + raw: true, + error: () => {}, + registry, + requireModule, + }); + + assert.strictEqual(result, true); + assert.strictEqual(calls.length, 1); + assert.strictEqual(calls[0].raw, true); + }); +}); + +// ─── 2. Unknown command → returns false ────────────────────────────────────── + +describe('dispatchCapabilityCommand — unknown command', () => { + test('returns false when command not in registry', () => { + const registry = makeRegistry({ + foo: { capId: 'x', module: 'fake.cjs', router: 'routeFoo' }, + }); + const requireModule = () => { throw new Error('should not load'); }; + + const result = dispatchCapabilityCommand({ + command: 'nonexistent', + args: [], + cwd: '/p', + raw: false, + error: () => {}, + registry, + requireModule, + }); + + assert.strictEqual(result, false); + }); + + test('returns false when commandFamilies is empty ({})', () => { + const registry = makeRegistry({}); + const requireModule = () => { throw new Error('should not load'); }; + + const result = dispatchCapabilityCommand({ + command: 'anything', + args: [], + cwd: '/p', + raw: false, + error: () => {}, + registry, + requireModule, + }); + + assert.strictEqual(result, false); + }); +}); + +// ─── 3. Missing/empty registry → false, no throw ───────────────────────────── + +describe('dispatchCapabilityCommand — missing/empty registry', () => { + test('registry=null → false, no throw', () => { + assert.doesNotThrow(() => { + const result = dispatchCapabilityCommand({ + command: 'foo', + args: [], + cwd: '/p', + raw: false, + error: () => {}, + registry: null, + requireModule: () => {}, + }); + assert.strictEqual(result, false); + }); + }); + + test('registry with no commandFamilies property → false, no throw', () => { + assert.doesNotThrow(() => { + const result = dispatchCapabilityCommand({ + command: 'foo', + args: [], + cwd: '/p', + raw: false, + error: () => {}, + registry: { version: '1' }, + requireModule: () => {}, + }); + assert.strictEqual(result, false); + }); + }); + + test('registry.commandFamilies=null → false, no throw', () => { + assert.doesNotThrow(() => { + const result = dispatchCapabilityCommand({ + command: 'foo', + args: [], + cwd: '/p', + raw: false, + error: () => {}, + registry: { commandFamilies: null }, + requireModule: () => {}, + }); + assert.strictEqual(result, false); + }); + }); +}); + +// ─── 4. Prototype pollution guard ──────────────────────────────────────────── + +describe('dispatchCapabilityCommand — prototype pollution guard', () => { + test('__proto__ command → false, no pollution', () => { + const result = dispatchCapabilityCommand({ + command: '__proto__', + args: [], + cwd: '/p', + raw: false, + error: () => {}, + registry: makeRegistry({}), + requireModule: () => {}, + }); + assert.strictEqual(result, false, '__proto__ must return false'); + }); + + test('constructor command → false, no pollution', () => { + const result = dispatchCapabilityCommand({ + command: 'constructor', + args: [], + cwd: '/p', + raw: false, + error: () => {}, + registry: makeRegistry({}), + requireModule: () => {}, + }); + assert.strictEqual(result, false, 'constructor must return false'); + }); + + test('prototype command → false, no pollution', () => { + const result = dispatchCapabilityCommand({ + command: 'prototype', + args: [], + cwd: '/p', + raw: false, + error: () => {}, + registry: makeRegistry({}), + requireModule: () => {}, + }); + assert.strictEqual(result, false, 'prototype must return false'); + }); +}); + +// ─── 5. Router not a function → handled, no throw ──────────────────────────── + +describe('dispatchCapabilityCommand — router not a function', () => { + test('router export is missing → does not throw, returns true (consumed)', () => { + const errors = []; + const registry = makeRegistry({ + foo: { capId: 'x', module: 'fake.cjs', router: 'routeNotPresent' }, + }); + const requireModule = () => ({ somethingElse: 'not-a-function' }); + + let result; + assert.doesNotThrow(() => { + result = dispatchCapabilityCommand({ + command: 'foo', + args: [], + cwd: '/p', + raw: false, + error: (msg) => errors.push(msg), + registry, + requireModule, + }); + }); + // Per ADR-959 design: router-not-a-function is a consumed dispatch (returns true) + // with a diagnostic error, so we don't fall through to "Unknown command" + assert.strictEqual(result, true, 'consumed dispatch even when router is not a function'); + assert.ok(errors.length > 0, 'should emit a diagnostic error'); + }); + + test('router export is null → does not throw', () => { + const registry = makeRegistry({ + foo: { capId: 'x', module: 'fake.cjs', router: 'routeFoo' }, + }); + const requireModule = () => ({ routeFoo: null }); + + assert.doesNotThrow(() => { + dispatchCapabilityCommand({ + command: 'foo', + args: [], + cwd: '/p', + raw: false, + error: () => {}, + registry, + requireModule, + }); + }); + }); +}); + +// ─── 6. Module load failure → handled, no uncaught throw ───────────────────── + +describe('dispatchCapabilityCommand — module load failure', () => { + test('requireModule throws → does not propagate, returns true (consumed)', () => { + const errors = []; + const registry = makeRegistry({ + foo: { capId: 'x', module: 'nonexistent.cjs', router: 'routeFoo' }, + }); + const requireModule = () => { throw new Error('MODULE_NOT_FOUND'); }; + + let result; + assert.doesNotThrow(() => { + result = dispatchCapabilityCommand({ + command: 'foo', + args: [], + cwd: '/p', + raw: false, + error: (msg) => errors.push(msg), + registry, + requireModule, + }); + }); + assert.strictEqual(result, true, 'module-load failure returns true (consumed)'); + assert.ok(errors.length > 0, 'should emit a diagnostic error'); + }); +}); + +// ─── 7. Module confinement — default requireModule refuses out-of-lib paths ─── + +describe('dispatchCapabilityCommand — module confinement (default requireModule)', () => { + test('entry.module "../../evil.cjs" does not escape bin/lib/ — returns true (consumed), no require', () => { + // Bypasses the generator's validation by hand-crafting a registry entry. + // The default requireModule must refuse the path and treat it as a load failure + // (returns true = consumed, emits a diagnostic error) rather than require()ing it. + const errors = []; + const registry = makeRegistry({ + foo: { capId: 'x', module: '../../evil.cjs', router: 'routeFoo' }, + }); + + // Use NO injected requireModule so the real default loader (with confinement check) runs. + // But we cannot actually hit the real require() since the path wouldn't exist; + // we verify the confinement check fires before any require by checking the error message. + let result; + assert.doesNotThrow(() => { + result = dispatchCapabilityCommand({ + command: 'foo', + args: [], + cwd: '/p', + raw: false, + error: (msg) => errors.push(msg), + registry, + // requireModule NOT injected → real default loader runs + }); + }); + + // Confinement violation: treated as a load failure → consumed (true), diagnostic emitted + assert.strictEqual(result, true, 'confinement violation must return true (consumed)'); + assert.ok(errors.length > 0, 'confinement violation must emit a diagnostic error'); + // The diagnostic must mention the module (not a generic node MODULE_NOT_FOUND) + assert.ok( + errors[0].includes('../../evil.cjs') || errors[0].includes('evil.cjs'), + 'diagnostic should reference the offending module; got: ' + errors[0], + ); + }); + + test('entry.module "../sibling.cjs" also refused by confinement check', () => { + const errors = []; + const registry = makeRegistry({ + bar: { capId: 'y', module: '../sibling.cjs', router: 'routeBar' }, + }); + + let result; + assert.doesNotThrow(() => { + result = dispatchCapabilityCommand({ + command: 'bar', + args: [], + cwd: '/p', + raw: false, + error: (msg) => errors.push(msg), + registry, + // requireModule NOT injected → real default loader runs + }); + }); + + assert.strictEqual(result, true, 'confinement violation must return true (consumed)'); + assert.ok(errors.length > 0, 'confinement violation must emit a diagnostic error'); + }); +}); + +// ─── 7b. FIX 1: bare .cjs basename validation (no extension → refused) ───────── + +describe('dispatchCapabilityCommand — FIX 1: bare .cjs basename enforcement (default requireModule)', () => { + test('entry.module "foo" (no .cjs) is refused by default requireModule — returns true (consumed), no require of foo.js', () => { + // A hand-edited registry entry with module: "foo" (no extension) must be + // rejected by the basename pattern check BEFORE any filesystem access. + // The confinement test in section 7 verifies path-traversal; this verifies + // the extension/basename invariant that prevents node resolving foo.js or + // foo/index.js from inside bin/lib/. + const errors = []; + const registry = makeRegistry({ + foo: { capId: 'x', module: 'foo', router: 'routeFoo' }, + }); + + let result; + assert.doesNotThrow(() => { + result = dispatchCapabilityCommand({ + command: 'foo', + args: [], + cwd: '/p', + raw: false, + error: (msg) => errors.push(msg), + registry, + // requireModule NOT injected → real default loader runs + }); + }); + + // Refused → consumed (true), diagnostic emitted + assert.strictEqual(result, true, 'bare-module-no-cjs must return true (consumed)'); + assert.ok(errors.length > 0, 'must emit a diagnostic error'); + // Diagnostic must mention the offending module name + assert.ok( + errors[0].includes('foo'), + 'diagnostic should reference the offending module name; got: ' + errors[0], + ); + }); + + test('entry.module "foo.js" (wrong extension) is also refused', () => { + const errors = []; + const registry = makeRegistry({ + bar: { capId: 'y', module: 'foo.js', router: 'routeBar' }, + }); + + let result; + assert.doesNotThrow(() => { + result = dispatchCapabilityCommand({ + command: 'bar', + args: [], + cwd: '/p', + raw: false, + error: (msg) => errors.push(msg), + registry, + }); + }); + + assert.strictEqual(result, true, 'wrong-extension must return true (consumed)'); + assert.ok(errors.length > 0, 'must emit a diagnostic error'); + }); +}); + +// ─── 7c. FIX 2: own-property guard on router export ─────────────────────────── + +describe('dispatchCapabilityCommand — FIX 2: own-property guard on router export', () => { + test('entry.router "constructor" (inherited prototype property) is not invoked — treated as miss, returns true (consumed)', () => { + // A registry entry with router: "constructor" must be refused by the own- + // property guard. The module's own exports do NOT include "constructor" as + // an own property, but Object.prototype does via the prototype chain. + // Without the guard, mod["constructor"] would return Function (the Object + // constructor) — typeof Function === 'function' — and it would be invoked. + const errors = []; + const registry = makeRegistry({ + foo: { capId: 'x', module: 'fake.cjs', router: 'constructor' }, + }); + // Injected module whose OWN exports do NOT include 'constructor' + const requireModule = () => ({ someOwnProp: () => {} }); + + let result; + assert.doesNotThrow(() => { + result = dispatchCapabilityCommand({ + command: 'foo', + args: [], + cwd: '/p', + raw: false, + error: (msg) => errors.push(msg), + registry, + requireModule, + }); + }); + + assert.strictEqual(result, true, 'inherited-constructor router must return true (consumed)'); + assert.ok(errors.length > 0, 'must emit a diagnostic error'); + }); + + test('entry.router "toString" (inherited prototype method) is not invoked', () => { + const errors = []; + const registry = makeRegistry({ + bar: { capId: 'y', module: 'fake.cjs', router: 'toString' }, + }); + const requireModule = () => ({ realRouter: () => {} }); + + let result; + assert.doesNotThrow(() => { + result = dispatchCapabilityCommand({ + command: 'bar', + args: [], + cwd: '/p', + raw: false, + error: (msg) => errors.push(msg), + registry, + requireModule, + }); + }); + + assert.strictEqual(result, true, 'inherited-toString router must return true (consumed)'); + assert.ok(errors.length > 0, 'must emit a diagnostic error'); + }); + + test('entry.router that IS an own property is still dispatched normally', () => { + // Regression: ensure the own-property guard does not break the happy path. + const calls = []; + const registry = makeRegistry({ + foo: { capId: 'x', module: 'fake.cjs', router: 'routeFoo' }, + }); + const requireModule = makeRequireModule('fake.cjs', 'routeFoo', calls); + + const result = dispatchCapabilityCommand({ + command: 'foo', + args: [], + cwd: '/p', + raw: false, + error: () => {}, + registry, + requireModule, + }); + + assert.strictEqual(result, true, 'own-property router must still dispatch'); + assert.strictEqual(calls.length, 1, 'router must have been called'); + }); +}); + +// ─── 8. Router throws — structured error handling ──────────────────────────── + +const { ExitError } = require('../gsd-core/bin/lib/cli-exit.cjs'); + +describe('dispatchCapabilityCommand — non-ExitError from router → structured error via error()', () => { + test('router throws TypeError → injected error() called with attributed message, raw error does NOT propagate', () => { + // A capability plug-in command's unexpected failure must surface as a + // structured, attributed error (honoring --json-errors / SDK consumers), + // not a raw stack trace bypassing the error formatter. + const errorCalls = []; + const registry = makeRegistry({ + foo: { capId: 'x', module: 'fake.cjs', router: 'routeFoo' }, + }); + const requireModule = () => ({ + routeFoo: () => { throw new TypeError('boom'); }, + }); + + // Must NOT throw — the raw TypeError must be caught and routed through error() + assert.doesNotThrow(() => { + dispatchCapabilityCommand({ + command: 'foo', + args: [], + cwd: '/p', + raw: false, + error: (msg, reason) => { errorCalls.push({ msg, reason }); }, + registry, + requireModule, + }); + }); + + assert.strictEqual(errorCalls.length, 1, 'error() should be called exactly once'); + const { msg, reason } = errorCalls[0]; + // Message must name the command, router, module, and original error message + assert.ok(msg.includes('foo'), 'message must name the command; got: ' + msg); + assert.ok(msg.includes('routeFoo'), 'message must name the router; got: ' + msg); + assert.ok(msg.includes('fake.cjs'), 'message must name the module; got: ' + msg); + assert.ok(msg.includes('boom'), 'message must include original error message; got: ' + msg); + // Reason must be SDK_FAIL_FAST + const { ERROR_REASON } = require('../gsd-core/bin/lib/core.cjs'); + assert.strictEqual(reason, ERROR_REASON.SDK_FAIL_FAST, 'reason must be SDK_FAIL_FAST'); + }); + + test('router throws a generic Error → same structured attribution, does not propagate', () => { + const errorCalls = []; + const registry = makeRegistry({ + bar: { capId: 'y', module: 'bar.cjs', router: 'routeBar' }, + }); + const requireModule = () => ({ + routeBar: () => { throw new Error('unexpected failure'); }, + }); + + assert.doesNotThrow(() => { + dispatchCapabilityCommand({ + command: 'bar', + args: [], + cwd: '/p', + raw: false, + error: (msg, reason) => { errorCalls.push({ msg, reason }); }, + registry, + requireModule, + }); + }); + + assert.strictEqual(errorCalls.length, 1, 'error() should be called exactly once'); + assert.ok(errorCalls[0].msg.includes('bar'), 'message must name the command'); + assert.ok(errorCalls[0].msg.includes('unexpected failure'), 'message must include original error'); + }); + + test('router throws an ExitError → propagates unchanged, error() is NOT called', () => { + // An ExitError comes from the router calling its own error() (intentional + // structured exit). It must propagate untouched so message/code/json-mode + // are preserved. + const errorCalls = []; + const thrown = new ExitError(1, 'intentional-exit'); + const registry = makeRegistry({ + foo: { capId: 'x', module: 'fake.cjs', router: 'routeFoo' }, + }); + const requireModule = () => ({ + routeFoo: () => { throw thrown; }, + }); + + let caught; + try { + dispatchCapabilityCommand({ + command: 'foo', + args: [], + cwd: '/p', + raw: false, + error: (msg, reason) => { errorCalls.push({ msg, reason }); }, + registry, + requireModule, + }); + } catch (e) { + caught = e; + } + + // The exact ExitError must have been rethrown + assert.strictEqual(caught, thrown, 'the original ExitError must propagate unchanged'); + // error() must NOT have been called + assert.strictEqual(errorCalls.length, 0, 'error() must not be called when an ExitError propagates'); + }); + + test('router returns normally → returns true, error() not called', () => { + const errorCalls = []; + const calls = []; + const registry = makeRegistry({ + foo: { capId: 'x', module: 'fake.cjs', router: 'routeFoo' }, + }); + const requireModule = makeRequireModule('fake.cjs', 'routeFoo', calls); + + const result = dispatchCapabilityCommand({ + command: 'foo', + args: ['a'], + cwd: '/p', + raw: false, + error: (msg, reason) => { errorCalls.push({ msg, reason }); }, + registry, + requireModule, + }); + + assert.strictEqual(result, true, 'successful dispatch must return true'); + assert.strictEqual(errorCalls.length, 0, 'error() must not be called on success'); + assert.strictEqual(calls.length, 1, 'router must have been called once'); + }); +}); + +// ─── 10. Async router (thenable) → structured error ───────────────────────── + +describe('dispatchCapabilityCommand — async router returns a Promise → structured error', () => { + const { ERROR_REASON } = require('../gsd-core/bin/lib/core.cjs'); + + test('router returns Promise.resolve() → error() called with "must be synchronous" + SDK_FAIL_FAST', () => { + const errorCalls = []; + const registry = makeRegistry({ + foo: { capId: 'x', module: 'fake.cjs', router: 'routeFoo' }, + }); + const requireModule = () => ({ + routeFoo: () => Promise.resolve(), + }); + + // Must NOT throw — the thenable check surfaces via error(), not an exception + assert.doesNotThrow(() => { + dispatchCapabilityCommand({ + command: 'foo', + args: [], + cwd: '/p', + raw: false, + error: (msg, reason) => { errorCalls.push({ msg, reason }); }, + registry, + requireModule, + }); + }); + + assert.strictEqual(errorCalls.length, 1, 'error() should be called exactly once'); + const { msg, reason } = errorCalls[0]; + assert.ok(msg.includes('must be synchronous'), 'message must say "must be synchronous"; got: ' + msg); + assert.ok(msg.includes('foo'), 'message must name the command; got: ' + msg); + assert.ok(msg.includes('routeFoo'), 'message must name the router; got: ' + msg); + assert.ok(msg.includes('fake.cjs'), 'message must name the module; got: ' + msg); + assert.strictEqual(reason, ERROR_REASON.SDK_FAIL_FAST, 'reason must be SDK_FAIL_FAST'); + }); + + test('router returns Promise.reject() → error() called with "must be synchronous", async rejection does NOT escape', () => { + const errorCalls = []; + const registry = makeRegistry({ + bar: { capId: 'y', module: 'bar.cjs', router: 'routeBar' }, + }); + // Attach .catch(()=>{}) immediately so the test process does not log an + // unhandled-rejection warning for the returned (un-awaited) rejected Promise. + const rejectedPromise = Promise.reject(new Error('async failure')); + rejectedPromise.catch(() => {}); + const requireModule = () => ({ + routeBar: () => rejectedPromise, + }); + + assert.doesNotThrow(() => { + dispatchCapabilityCommand({ + command: 'bar', + args: [], + cwd: '/p', + raw: false, + error: (msg, reason) => { errorCalls.push({ msg, reason }); }, + registry, + requireModule, + }); + }); + + assert.strictEqual(errorCalls.length, 1, 'error() should be called exactly once'); + const { msg, reason } = errorCalls[0]; + assert.ok(msg.includes('must be synchronous'), 'message must say "must be synchronous"; got: ' + msg); + assert.ok(msg.includes('bar'), 'message must name the command; got: ' + msg); + assert.ok(msg.includes('routeBar'), 'message must name the router; got: ' + msg); + assert.ok(msg.includes('bar.cjs'), 'message must name the module; got: ' + msg); + assert.strictEqual(reason, ERROR_REASON.SDK_FAIL_FAST, 'reason must be SDK_FAIL_FAST'); + }); + + test('sync router that returns undefined (normal) still dispatches without error', () => { + // Regression: ensure the thenable guard does not fire on undefined return + const errorCalls = []; + const calls = []; + const registry = makeRegistry({ + foo: { capId: 'x', module: 'fake.cjs', router: 'routeFoo' }, + }); + const requireModule = makeRequireModule('fake.cjs', 'routeFoo', calls); + + const result = dispatchCapabilityCommand({ + command: 'foo', + args: [], + cwd: '/p', + raw: false, + error: (msg, reason) => { errorCalls.push({ msg, reason }); }, + registry, + requireModule, + }); + + assert.strictEqual(result, true, 'sync router must return true'); + assert.strictEqual(errorCalls.length, 0, 'error() must not be called for sync router'); + assert.strictEqual(calls.length, 1, 'router must have been called'); + }); + + test('sync router that returns a non-thenable object does not trigger thenable guard', () => { + // A router that returns a plain object (not a Promise) must not be rejected. + const errorCalls = []; + const registry = makeRegistry({ + foo: { capId: 'x', module: 'fake.cjs', router: 'routeFoo' }, + }); + const requireModule = () => ({ + routeFoo: () => ({ status: 'ok' }), // plain object, not thenable + }); + + const result = dispatchCapabilityCommand({ + command: 'foo', + args: [], + cwd: '/p', + raw: false, + error: (msg, reason) => { errorCalls.push({ msg, reason }); }, + registry, + requireModule, + }); + + assert.strictEqual(result, true, 'sync router returning plain object must return true'); + assert.strictEqual(errorCalls.length, 0, 'error() must not be called'); + }); +}); + +// ─── 9. Behavior-preservation: real registry has empty commandFamilies ──────── + +describe('dispatchCapabilityCommand — real registry behavior-preservation', () => { + test('real capability-registry.cjs commandFamilies is {} (no capability declares commands)', () => { + const realRegistry = require('../gsd-core/bin/lib/capability-registry.cjs'); + assert.ok(realRegistry.commandFamilies, 'commandFamilies must be exported'); + assert.deepEqual( + Object.keys(realRegistry.commandFamilies), + [], + 'real registry commandFamilies must be empty today', + ); + }); + + test('unknown command against real registry returns false (behavior-preserving)', () => { + const realRegistry = require('../gsd-core/bin/lib/capability-registry.cjs'); + + const result = dispatchCapabilityCommand({ + command: 'some-unknown-command-xyz', + args: [], + cwd: process.cwd(), + raw: false, + error: () => {}, + registry: realRegistry, + requireModule: () => { throw new Error('should not load'); }, + }); + + assert.strictEqual(result, false, 'unknown command against real registry must return false'); + }); +}); diff --git a/tests/capability-registry.test.cjs b/tests/capability-registry.test.cjs index 7b981dfb9..4f391be4d 100644 --- a/tests/capability-registry.test.cjs +++ b/tests/capability-registry.test.cjs @@ -37,6 +37,8 @@ const { deriveProfileMembership, runConsistencyGate, PROFILE_RANK, + // ADR-959 + validateCommandEntry, } = require('../scripts/gen-capability-registry.cjs'); const ROOT = path.resolve(__dirname, '..'); @@ -2136,3 +2138,186 @@ describe('FIX 6: runConsistencyGate does NOT throw for real UI capability (true- ); }); }); + +// ─── 23. ADR-959: commands field + commandFamilies index ────────────────────── + +/** + * Build a minimal feature capability for ADR-959 command tests. + * skills/agents/etc. kept minimal-valid so validateCapability passes. + */ +function makeCommandCap(id, commands) { + return { + id, + role: 'feature', + title: 'Test cap ' + id, + description: 'Synthetic capability for ADR-959 command tests.', + tier: 'full', + requires: [], + skills: [], + agents: [], + hooks: [], + config: {}, + steps: [], + contributions: [], + gates: [], + commands, + }; +} + +describe('ADR-959: validateCommandEntry — valid entry', () => { + test('valid minimal entry (no subcommands) passes', () => { + const errors = validateCommandEntry('my-cap', { family: 'foo', module: 'foo.cjs', router: 'routeFoo' }, 'commands[0]'); + assert.deepEqual(errors, []); + }); + + test('valid entry with subcommands passes', () => { + const errors = validateCommandEntry('my-cap', { + family: 'bar', + module: 'bar-router.cjs', + router: 'routeBar', + subcommands: ['query', 'status'], + }, 'commands[0]'); + assert.deepEqual(errors, []); + }); +}); + +describe('ADR-959: validateCommandEntry — adversarial rejects', () => { + test('missing family → error', () => { + const errors = validateCommandEntry('my-cap', { module: 'foo.cjs', router: 'routeFoo' }, 'commands[0]'); + assert.ok(errors.some((e) => e.includes('family')), 'Expected family error, got: ' + JSON.stringify(errors)); + }); + + test('empty family → error', () => { + const errors = validateCommandEntry('my-cap', { family: '', module: 'foo.cjs', router: 'routeFoo' }, 'commands[0]'); + assert.ok(errors.some((e) => e.includes('family')), 'Expected family error, got: ' + JSON.stringify(errors)); + }); + + test('missing module → error', () => { + const errors = validateCommandEntry('my-cap', { family: 'foo', router: 'routeFoo' }, 'commands[0]'); + assert.ok(errors.some((e) => e.includes('module')), 'Expected module error, got: ' + JSON.stringify(errors)); + }); + + test('missing router → error', () => { + const errors = validateCommandEntry('my-cap', { family: 'foo', module: 'foo.cjs' }, 'commands[0]'); + assert.ok(errors.some((e) => e.includes('router')), 'Expected router error, got: ' + JSON.stringify(errors)); + }); + + test('non-string router → error', () => { + const errors = validateCommandEntry('my-cap', { family: 'foo', module: 'foo.cjs', router: 42 }, 'commands[0]'); + assert.ok(errors.some((e) => e.includes('router')), 'Expected router error, got: ' + JSON.stringify(errors)); + }); + + test('traversal module "../evil.cjs" → error', () => { + const errors = validateCommandEntry('my-cap', { family: 'foo', module: '../evil.cjs', router: 'r' }, 'commands[0]'); + assert.ok(errors.some((e) => e.includes('module')), 'Expected module traversal error, got: ' + JSON.stringify(errors)); + }); + + test('absolute module "/abs/path.cjs" → error', () => { + const errors = validateCommandEntry('my-cap', { family: 'foo', module: '/abs/path.cjs', router: 'r' }, 'commands[0]'); + assert.ok(errors.some((e) => e.includes('module')), 'Expected module absolute error, got: ' + JSON.stringify(errors)); + }); + + test('module with "/" separator "lib/foo.cjs" → error', () => { + const errors = validateCommandEntry('my-cap', { family: 'foo', module: 'lib/foo.cjs', router: 'r' }, 'commands[0]'); + assert.ok(errors.some((e) => e.includes('module')), 'Expected module separator error, got: ' + JSON.stringify(errors)); + }); + + test('subcommands non-array → error', () => { + const errors = validateCommandEntry('my-cap', { + family: 'foo', module: 'foo.cjs', router: 'r', subcommands: 'not-array', + }, 'commands[0]'); + assert.ok(errors.some((e) => e.includes('subcommands')), 'Expected subcommands error, got: ' + JSON.stringify(errors)); + }); + + test('subcommands with non-string entry → error', () => { + const errors = validateCommandEntry('my-cap', { + family: 'foo', module: 'foo.cjs', router: 'r', subcommands: ['ok', 42], + }, 'commands[0]'); + assert.ok(errors.some((e) => e.includes('subcommands')), 'Expected subcommands[1] error, got: ' + JSON.stringify(errors)); + }); +}); + +describe('ADR-959: validateCrossCapability — duplicate family ownership', () => { + test('duplicate family across two capabilities → error', () => { + const capA = makeCommandCap('cap-a', [{ family: 'shared', module: 'a.cjs', router: 'rA' }]); + const capB = makeCommandCap('cap-b', [{ family: 'shared', module: 'b.cjs', router: 'rB' }]); + const capMap = new Map([['cap-a', capA], ['cap-b', capB]]); + const errors = validateCrossCapability(capMap, new Set()); + assert.ok( + errors.some((e) => e.includes('shared') && e.includes('cap-a') && e.includes('cap-b')), + 'Expected duplicate family error mentioning both caps, got: ' + JSON.stringify(errors), + ); + }); + + test('unique families in two capabilities → no error', () => { + const capA = makeCommandCap('cap-a', [{ family: 'alpha', module: 'alpha.cjs', router: 'rA' }]); + const capB = makeCommandCap('cap-b', [{ family: 'beta', module: 'beta.cjs', router: 'rB' }]); + const capMap = new Map([['cap-a', capA], ['cap-b', capB]]); + const errors = validateCrossCapability(capMap, new Set()); + assert.ok( + !errors.some((e) => e.includes('owned by both')), + 'Expected no duplicate-ownership error, got: ' + JSON.stringify(errors), + ); + }); +}); + +describe('ADR-959: buildRegistry — commandFamilies index shape', () => { + test('cap with valid commands entry produces commandFamilies entry', () => { + const cap = makeCommandCap('test-cmd', [ + { family: 'myfamily', module: 'myfamily.cjs', router: 'routeMyFamily' }, + ]); + const capMap = new Map([['test-cmd', cap]]); + const registry = buildRegistry(capMap); + assert.ok(registry.commandFamilies, 'commandFamilies must be present'); + const entry = registry.commandFamilies['myfamily']; + assert.ok(entry, 'commandFamilies["myfamily"] must exist'); + assert.strictEqual(entry.capId, 'test-cmd'); + assert.strictEqual(entry.module, 'myfamily.cjs'); + assert.strictEqual(entry.router, 'routeMyFamily'); + }); + + test('cap without commands → commandFamilies is empty', () => { + const capDir = makeTempCapDir({ ui: UI_CAP }); + const { capMap } = loadAndValidate(new Set(), capDir); + const registry = buildRegistry(capMap); + assert.ok(registry.commandFamilies, 'commandFamilies must be present'); + assert.deepEqual(Object.keys(registry.commandFamilies), [], 'commandFamilies must be empty for real registry'); + }); + + test('commandFamilies keys are sorted in serialized output (determinism)', () => { + // Two caps with commands in z→a order; expect a→z in the commandFamilies section + const capA = makeCommandCap('cap-a', [{ family: 'zebra', module: 'z.cjs', router: 'rZ' }]); + const capB = makeCommandCap('cap-b', [{ family: 'alpha', module: 'a.cjs', router: 'rA' }]); + const capMap = new Map([['cap-a', capA], ['cap-b', capB]]); + const registry = buildRegistry(capMap); + const serialized = serializeRegistry(registry, capMap); + + // Find the commandFamilies section specifically (not the full capabilities JSON) + const cfStart = serialized.indexOf('const commandFamilies = '); + assert.ok(cfStart >= 0, 'commandFamilies section must be present'); + const cfEnd = serialized.indexOf('\n};', cfStart) + 3; // closing }; of const assignment + const cfSection = serialized.slice(cfStart, cfEnd); + + const alphaIdx = cfSection.indexOf('"alpha"'); + const zebraIdx = cfSection.indexOf('"zebra"'); + assert.ok(alphaIdx >= 0, '"alpha" must appear in commandFamilies section'); + assert.ok(zebraIdx >= 0, '"zebra" must appear in commandFamilies section'); + assert.ok(alphaIdx < zebraIdx, 'commandFamilies section must list "alpha" before "zebra" (sorted)'); + }); +}); + +describe('ADR-959: validateCapability — commands field on feature role', () => { + test('valid commands entry on feature cap passes validateCapability', () => { + const cap = makeCommandCap('cmd-cap', [ + { family: 'testfamily', module: 'testfamily.cjs', router: 'routeTestFamily' }, + ]); + const errors = validateCapability(cap, 'cmd-cap'); + assert.deepEqual(errors, [], 'Expected no errors: ' + JSON.stringify(errors)); + }); + + test('commands: null on feature cap → error', () => { + const cap = makeCommandCap('cmd-cap', null); + const errors = validateCapability(cap, 'cmd-cap'); + assert.ok(errors.some((e) => e.includes('commands')), 'Expected commands error, got: ' + JSON.stringify(errors)); + }); +});