diff --git a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/canvas-runtime/composition-apply.mjs b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/canvas-runtime/composition-apply.mjs index 932eb71..8ab84d7 100644 --- a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/canvas-runtime/composition-apply.mjs +++ b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/canvas-runtime/composition-apply.mjs @@ -87,7 +87,15 @@ export function normalizeHookArtifactsInComposition(composition) { } const artifacts = composition.artifacts.map((artifact) => { if (artifact?.kind !== "hook") return artifact; - const ownCommand = String(artifact.id).replace(/^commands\//, ""); + // Native hook artifact ids are shaped as `hooks/:` + // (see cliIdToWizardId), not `commands/`, so stripping a + // `commands/` prefix off `artifact.id` never actually recovers the + // target command. Prefer the artifact's own authoritative + // `targetCommand` field (set at shape time from the CLI) and only + // fall back to the id-derived guess when it's missing. + const ownCommand = typeof artifact.targetCommand === "string" + ? artifact.targetCommand.replace(/^commands\//, "") + : String(artifact.id).replace(/^commands\//, ""); const existingBindings = Array.isArray(artifact.hookBindings) && artifact.hookBindings.length ? artifact.hookBindings : [artifact.hookBinding].filter(Boolean); @@ -140,7 +148,12 @@ export async function applyComposition(inst, input) { // provide commands. If an extension truly declares templates or // scripts (rare), the prompt still emits them and this scrub // leaves them alone. - const VALID_STRATEGIES = new Set(["replace", "wrap", "prepend", "append"]); + // Must stay in sync with artifact-cli.mjs's VALID_STRATEGIES — that's + // where the CLI's hook layers are shaped with "additive" (hooks stack + // alongside a command rather than replace/wrap/prepend/append it). + // Omitting it here would silently coerce every native hook layer to + // "replace", contradicting what the CLI actually reported. + const VALID_STRATEGIES = new Set(["replace", "wrap", "prepend", "append", "additive"]); const normalizeArtifact = (a) => { if (!a || typeof a !== "object") return a; const stack = Array.isArray(a.stack) ? a.stack : []; diff --git a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/composition/artifact-cli.mjs b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/composition/artifact-cli.mjs index 9cec36c..431ebd7 100644 --- a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/composition/artifact-cli.mjs +++ b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/composition/artifact-cli.mjs @@ -1,34 +1,26 @@ // speckit-wizard — CLI-backed composition source. // -// Uses `specify artifact list --json` as the sole source of truth for the -// command, template, and script composition slices. +// Uses `specify artifact list --json` as the sole source of truth for all +// artifact kinds (commands, templates, scripts, and hooks). // // Shape mapping (CLI → wizard): // • CLI id `command:` → wizard id `commands/` // • CLI id `template:` → wizard id `` (bare) // • CLI id `script:` → wizard id `` (bare) +// • CLI id `hook:` → wizard id `hooks/` // • CLI `layer: null` (built-in) → wizard `layer: "core"` // • CLI `active` (index-0 winner) → passed through verbatim // • Everything else — presetId, presetName, strategy, hidden, sourceId, // manifestPath, lookupId — passed through unchanged. // -// Hook enrichment (`kind: "hook"`, `hookBindings`) is layered on top by -// reading extension manifests — the CLI doesn't distinguish hook artifacts -// from ordinary command artifacts. +// Native CLI hook artifacts are consumed directly without manual extension +// manifest parsing. import { execFile } from "node:child_process"; import { promisify } from "node:util"; import { buildAugmentedPath } from "../env/resolve-path.mjs"; -import { readExtensionManifest, readHooksMap } from "./hooks.mjs"; const execFileP = promisify(execFile); -// Default runner. Async so it doesn't block the Node event loop while a -// shell-out is in flight. Returns a string (stdout). Tests inject a -// synchronous runner that returns a Buffer/string — we `await` its -// return, which unwraps both sync and Promise values transparently. -// Keep this as a bounded one-shot read for the wizard's current inventory -// scope. An oversized payload should fail the refresh rather than introducing -// streaming complexity or allowing partial JSON to be treated as complete. const defaultAsyncRunner = async (cmd, args, opts) => { const augmentedPath = await buildAugmentedPath(); const { stdout } = await execFileP(cmd, args, { ...opts, env: { ...process.env, PATH: augmentedPath } }); @@ -37,9 +29,6 @@ const defaultAsyncRunner = async (cmd, args, opts) => { const CLI_COMMAND_TIMEOUT_MS = 15_000; -// Windows may ship `specify` as `.cmd`/`.bat` (uv tool / pipx layouts). -// Node ≥ 20.12.2 refuses to spawn those without a shell (CVE-2024-27980), -// so route through cmd.exe on Windows only. POSIX stays direct-exec. function specifyExecOpts(cwd) { return { cwd, @@ -66,36 +55,17 @@ export async function specifyArtifactList(root, { runner = defaultAsyncRunner } // --------------------------------------------------------------------------- // Shape mapping helpers // --------------------------------------------------------------------------- -// -// Guardrails — keep the CLI's contract intact when translating stack layers: -// -// 1. `layer: null` on the CLI means the built-in tier. We display it as -// "core" for the UI, but that's cosmetic ONLY. `sourceId`, `presetId`, -// `presetName`, `manifestPath`, and `lookupId` stay null on that layer. -// Never synthesize provenance fields to match the display label. When -// code needs to ask "does this layer have provenance?", check -// `sourceId != null` / `presetId != null` — not `layer !== "core"`. -// -// 2. The CLI round-trip key is the top-level `id` -// (`command:X`, `template:X`, `script:X`) — never `lookupId`. -// `lookupId` describes layer provenance and may be null for built-in -// or legacy filesystem-derived layers. -// -// 3. Prefer exclusion filters over positive `layer === "core"` predicates. -// "User customized this" is `stack.some(l => l.layer === "project")`; -// "not project-owned" is `l.layer !== "project"`. Treat the null-layer -// state as the semantic truth, `"core"` as its display alias. -const VALID_STRATEGIES = new Set(["replace", "wrap", "prepend", "append"]); +const VALID_STRATEGIES = new Set(["replace", "wrap", "prepend", "append", "additive"]); function cliIdToWizardId(cliId, kind) { if (typeof cliId !== "string") return null; - // CLI ids are `:`; strip prefix (defensive: also accept - // already-bare names in case the CLI ever grows a --bare mode). const sep = cliId.indexOf(":"); const name = sep >= 0 ? cliId.slice(sep + 1) : cliId; if (!name) return null; - return kind === "command" ? `commands/${name}` : name; + if (kind === "command") return `commands/${name}`; + if (kind === "hook") return `hooks/${name}`; + return name; } function normalizeCliStackLayer(layer) { @@ -104,41 +74,66 @@ function normalizeCliStackLayer(layer) { ? layer.strategy : "replace"; return { - // CLI `null` layer = built-in; wizard code expects "core". layer: layer.layer == null ? "core" : layer.layer, presetId: layer.presetId ?? null, presetName: layer.presetName ?? null, - // Extension layers intentionally retain the CLI's sourceId-only - // identity. Stack labels may fall back to that ID because display-name - // enrichment is outside the artifact stack contract. sourceId: layer.sourceId ?? null, strategy, active: !!layer.active, hidden: !!layer.hidden, manifestPath: layer.manifestPath ?? null, lookupId: layer.lookupId ?? null, - // Preserve project layers for CLI contract fidelity, but the wizard - // does not currently support project-override workflows or source - // navigation. Their sourcePath may therefore intentionally be null. sourcePath: layer.sourcePath ?? null, + priority: typeof layer.priority === "number" ? layer.priority : null, + optional: !!layer.optional, }; } function shapeArtifact(cliArtifact) { if (!cliArtifact || typeof cliArtifact !== "object") return null; const kind = cliArtifact.kind; - if (kind !== "command" && kind !== "template" && kind !== "script") return null; + if (kind !== "command" && kind !== "template" && kind !== "script" && kind !== "hook") return null; const wizardId = cliIdToWizardId(cliArtifact.id, kind); if (!wizardId) return null; const stack = Array.isArray(cliArtifact.stack) ? cliArtifact.stack.map(normalizeCliStackLayer).filter(Boolean) : []; - return { + const shaped = { id: wizardId, kind, description: cliArtifact.description ?? "", stack, }; + if (kind === "hook") { + const event = cliArtifact.event ?? cliArtifact.eventName ?? ( + typeof cliArtifact.id === "string" && cliArtifact.id.startsWith("hook:") + ? cliArtifact.id.split(":")[1] + : null + ); + const targetCommand = cliArtifact.targetCommand ?? ( + typeof cliArtifact.id === "string" && cliArtifact.id.startsWith("hook:") + ? cliArtifact.id.split(":").slice(2).join(":") + : null + ); + shaped.event = event; + shaped.targetCommand = targetCommand; + shaped.registered = cliArtifact.registered ?? true; + const hookBindings = []; + for (const layer of stack) { + const extId = layer.sourceId ?? layer.presetId; + hookBindings.push({ + phase: event, + targetCommand: targetCommand ?? (typeof wizardId === "string" ? wizardId.replace(/^hooks\//, "") : null), + optional: !!layer.optional, + priority: layer.priority ?? null, + extensionId: extId, + manifestPath: layer.manifestPath ?? null, + }); + } + shaped.hookBindings = hookBindings; + shaped.hookBinding = hookBindings[0] ?? null; + } + return shaped; } // --------------------------------------------------------------------------- @@ -152,9 +147,6 @@ function providerIdForLayer(layer) { } function accumulateProvidesCounts(artifacts) { - // Map - // Presets use their installed presetId; extensions use the sourceId from - // their contribution lookupId because extension rows have presetId: null. const counts = new Map(); for (const artifact of artifacts) { for (const layer of artifact.stack) { @@ -171,29 +163,26 @@ function accumulateProvidesCounts(artifacts) { commands: 0, templates: 0, scripts: 0, + hooks: 0, }; counts.set(key, entry); } if (artifact.kind === "command") entry.commands++; else if (artifact.kind === "template") entry.templates++; else if (artifact.kind === "script") entry.scripts++; + else if (artifact.kind === "hook") entry.hooks++; } } return counts; } -function summarizeInstalled(kind, artifacts, cachedItems, extraExtensionData) { +function summarizeInstalled(kind, artifacts, cachedItems) { const counts = accumulateProvidesCounts(artifacts); const cachedById = new Map( (cachedItems ?? []) .filter((it) => it && it.active) .map((it) => [it.installedId || it.id, it]), ); - // The wizard supports providers from its built-in and community catalog - // caches, so preserve that existing order. Providers observed only in - // artifact stacks are appended for best-effort visibility; the wizard - // does not install or manage them and must not infer global precedence - // from artifact enumeration. const ids = new Set(); for (const [, item] of cachedById) ids.add(item.installedId || item.id); for (const [key, entry] of counts) { @@ -205,12 +194,11 @@ function summarizeInstalled(kind, artifacts, cachedItems, extraExtensionData) { const key = `${kind}:${id}`; const c = counts.get(key); const cached = cachedById.get(id); - const extra = extraExtensionData?.get(id); if (!c && !cached) continue; const item = { id, - name: extra?.name ?? cached?.name ?? c?.providerName ?? id, - version: extra?.version ?? cached?.version ?? undefined, + name: cached?.name ?? c?.providerName ?? id, + version: cached?.version ?? undefined, priority: typeof cached?.priority === "number" ? cached.priority : 10, enabled: true, description: cached?.description ?? "", @@ -221,251 +209,91 @@ function summarizeInstalled(kind, artifacts, cachedItems, extraExtensionData) { }, }; if (kind === "extension") { - if (extra) { - if (extra.category) item.category = extra.category; - if (extra.effect) item.effect = extra.effect; - item.provides.hooks = extra.hookCount ?? 0; - } else if (cached?.category !== undefined || cached?.effect !== undefined) { + if (cached?.category !== undefined || cached?.effect !== undefined) { if (cached.category) item.category = cached.category; if (cached.effect) item.effect = cached.effect; - item.provides.hooks = 0; - } else { - item.provides.hooks = 0; } + // Hooks are always extension-provided auto-run wiring — presets + // cannot declare them (see artifactOrigin() in ui/composition.js, + // which hardcodes "extension" for kind === "hook"). So + // provides.hooks is intentionally only surfaced for extension + // summaries, even though accumulateProvidesCounts() tallies hook + // counts generically for any layer kind. + item.provides.hooks = c?.hooks ?? 0; } out.push(item); } return out; } -// --------------------------------------------------------------------------- -// Hook attribution — layered on top of CLI-derived artifacts -// --------------------------------------------------------------------------- - -/** - * Walk installed extensions on disk. Returns: - * • extensionHookInfo: Map - * • hooksMap: .specify/extensions.yml hook bindings, or null - * - * Walks installed extensions on disk to collect hook metadata — the CLI's - * artifact command doesn't emit hook bindings, so we still parse extension.yml. - */ -async function collectHookMetadata(workspaceRoot, activeExtensions) { - const extensionHookInfo = new Map(); - for (const { sourceId, manifestPath } of activeExtensions.values()) { - const manifest = await readExtensionManifest(workspaceRoot, sourceId, manifestPath); - if (!manifest || manifest.error) continue; - extensionHookInfo.set(sourceId, { - hooks: manifest.hooks ?? [], - category: manifest.category ?? null, - effect: manifest.effect ?? null, - hookCount: (manifest.hooks ?? []).length, - manifestPath: manifest.manifestPath ?? null, - name: manifest.name ?? sourceId, - version: manifest.version ?? null, - }); - } - const hooksMap = await readHooksMap(workspaceRoot); - return { extensionHookInfo, hooksMap }; -} - -/** - * Layer hook attributions onto the CLI-derived artifacts array in place: - * (a) inline `hooks[]` on the parent phase command artifact - * (b) standalone `kind: "hook"` artifact with `hookBindings`. - * - * Extension-provided commands whose name matches a declared hook command are - * removed as `kind: "command"` artifacts (they only exist as hook artifacts). - */ -function applyHookAttributions(artifacts, extensionHookInfo, hooksMap) { - // Fast id → artifact lookup. +function applyNativeHookAttributions(artifacts) { const byId = new Map(artifacts.map((a) => [a.id, a])); - - // Track hook artifacts as we build them. - const hookArtifactsById = new Map(); - - // Collect the set of hook command names per extension so we can remove - // the corresponding "command" artifact rows. - const extensionHookCommandNames = new Map(); // extensionId -> Set - - for (const [extensionId, info] of extensionHookInfo) { - for (const hook of info.hooks) { - const phase = hook.phase; - const hookCommand = hook.command; - if (!phase || !hookCommand) continue; - - // Track for command-artifact suppression. - let set = extensionHookCommandNames.get(extensionId); - if (!set) { - set = new Set(); - extensionHookCommandNames.set(extensionId, set); - } - set.add(hookCommand); - - const registeredBindings = hooksMap?.[phase] ?? []; - const registered = registeredBindings.some( - (b) => b?.extension === extensionId && (b?.command == null || b.command === hookCommand), - ); - - // (a) Inline attribution on the parent phase command artifact. - const targetPhaseName = phase.replace(/^(before_|after_)/, ""); - const parentCommandId = `commands/speckit.${targetPhaseName}`; + for (const artifact of artifacts) { + if (artifact.kind !== "hook") continue; + const event = artifact.event; + const targetCommand = artifact.targetCommand; + if (!event) continue; + // Known limitation: only the first active stack layer is attributed + // here. A hook artifact's id is keyed on (event, targetCommand), so + // 2+ active layers only occur when a *different* extension declares + // a hook for the exact same event/target — e.g. it intentionally + // piggybacks on another extension's existing command rather than + // colliding on a command name (which is already deduped/rejected + // elsewhere). That's legitimate per the data model but uncommon and + // not exercised by any extension in this repo today, so this is + // deliberately left as a single-attribution readout for now rather + // than iterating every active layer. See PR #28 discussion. + const activeLayer = artifact.stack.find((l) => l.active) ?? artifact.stack[0]; + const providerId = activeLayer?.sourceId ?? activeLayer?.presetId; + const providerName = activeLayer?.presetName ?? providerId; + + const match = event.match(/^(?:before|after)_(.+)$/); + const phaseName = match ? match[1] : null; + if (phaseName) { + const parentCommandId = `commands/speckit.${phaseName}`; const parent = byId.get(parentCommandId); if (parent) { (parent.hooks ??= []).push({ - phase, - extensionId, - extensionName: info.name, - targetCommand: hookCommand, + phase: event, + extensionId: providerId, + extensionName: providerName, + targetCommand, + // Mirror the active layer's optionality — omitting this + // makes resolveHooksForCommand()'s de-duped inline entry + // (which wins over the standalone hook artifact's + // hookBindings[].optional) render as Required even when + // the hook is declared optional. + optional: !!activeLayer?.optional, declared: true, - registered, - }); - } - - // (b) Standalone hook artifact. - const hookArtifactId = `commands/${hookCommand}`; - let hookArtifact = hookArtifactsById.get(hookArtifactId); - if (!hookArtifact) { - const commandArtifact = byId.get(hookArtifactId); - hookArtifact = commandArtifact - ? { ...commandArtifact, kind: "hook", hookBindings: [] } - : { - id: hookArtifactId, - kind: "hook", - description: "", - stack: [], - hookBindings: [], - }; - hookArtifactsById.set(hookArtifactId, hookArtifact); - } - const binding = { - phase, - targetCommand: hookCommand, - optional: !!hook.optional, - extensionId, - manifestPath: info.manifestPath, - }; - const bindingKey = `${binding.phase}|${binding.extensionId}`; - if (!hookArtifact.hookBindings.some((b) => `${b.phase}|${b.extensionId}` === bindingKey)) { - hookArtifact.hookBindings.push(binding); - } - hookArtifact.hookBinding = hookArtifact.hookBindings[0]; - if (!hookArtifact.stack.some((l) => l.sourceId === extensionId)) { - hookArtifact.stack.push({ - layer: "extension", - presetId: null, - presetName: null, - sourceId: extensionId, - extensionName: info.name, - strategy: "replace", - active: hookArtifact.stack.length === 0, - hidden: false, - manifestPath: info.manifestPath, - lookupId: null, + registered: artifact.registered ?? true, }); } } } - - // Strip extension-provided command artifacts whose name matches a - // declared hook command from the same extension. The hook artifact - // above replaces them. - const filtered = artifacts.filter((artifact) => { - if (artifact.kind !== "command") return true; - const name = artifact.id.replace(/^commands\//, ""); - const active = artifact.stack.find((layer) => layer.active); - if (active?.layer !== "extension") return true; - return !extensionHookCommandNames.get(active.sourceId)?.has(name); - }); - - // Append hook artifacts. - filtered.push(...hookArtifactsById.values()); - return filtered; + return artifacts; } // --------------------------------------------------------------------------- // Public: build the wizard composition payload from the CLI // --------------------------------------------------------------------------- -/** - * Build the wizard's `{ presets, extensions, artifacts }` composition payload - * from a SINGLE `specify artifact list --json` call. Layers hook enrichment - * on top of the CLI-derived artifacts. - * - * ## Upstream contract - * - * `specify artifact list --json` returns one row per artifact carrying the - * FULL composition stack (i.e. list rows include `stack: [...]`). - * - * If a CLI ships where `list --json` omits `stack`, this function still - * returns a well-formed payload — artifacts get empty stacks and the - * composition summary folds to `[]`. Not desirable, but not a crash. - * - * @param {object} opts - * @param {string} opts.workspaceRoot Absolute path to the workspace root. - * @param {Array} opts.presetItems Cached preset catalog (inst.cachedPresetItems). - * @param {Array} opts.extensionItems Cached extension catalog (inst.cachedExtensionItems). - * @param {Function} [opts.runner] Injectable runner — for tests. Returns - * stdout as a string/Buffer, sync or async. - */ export async function buildCompositionFromCli({ workspaceRoot, presetItems, extensionItems, runner = defaultAsyncRunner, } = {}) { - // 1. Single list call — each row carries `stack`. const list = await specifyArtifactList(workspaceRoot, { runner }); - // 2. Shape each row directly. shapeArtifact reads `stack` off its input. const artifactsRaw = []; for (const row of list) { const shaped = shapeArtifact(row); if (shaped) artifactsRaw.push(shaped); } - // 3. Enrich with hook metadata (extension.yml manifests). - const activeExtensions = new Map(); - for (const layer of artifactsRaw.flatMap((artifact) => artifact.stack)) { - if (layer.layer !== "extension" || !layer.sourceId) continue; - const existing = activeExtensions.get(layer.sourceId); - if (!existing || (!existing.manifestPath && layer.manifestPath)) { - activeExtensions.set(layer.sourceId, { - sourceId: layer.sourceId, - manifestPath: layer.manifestPath, - }); - } - } - // Also include any active extensions from the cached catalog that - // didn't contribute an artifact (pure hook-only extensions). - for (const ext of extensionItems ?? []) { - if (ext?.active) { - const id = ext.installedId || ext.id; - if (id && !activeExtensions.has(id)) { - activeExtensions.set(id, { sourceId: id, manifestPath: null }); - } - } - } - const { extensionHookInfo, hooksMap } = await collectHookMetadata( - workspaceRoot, - activeExtensions, - ); - const artifacts = applyHookAttributions(artifactsRaw, extensionHookInfo, hooksMap); - - // 4. Summarize installed presets / extensions via a fold over the - // artifact stacks — no separate CLI query needed. Known edge case: - // a preset that contributes zero currently-active artifacts (every - // contribution shadowed, or the preset is empty) won't appear here. - // Living with that in exchange for a single-shell-out boot; if - // upstream ever ships `preset list --json` / `extension list --json` - // with active detail, switch the summary to a direct query. - const presetsOut = summarizeInstalled("preset", artifactsRaw, presetItems); - const extensionsOut = summarizeInstalled( - "extension", - artifactsRaw, - extensionItems, - extensionHookInfo, - ); + const artifacts = applyNativeHookAttributions(artifactsRaw); + const presetsOut = summarizeInstalled("preset", artifacts, presetItems); + const extensionsOut = summarizeInstalled("extension", artifacts, extensionItems); return { presets: presetsOut, diff --git a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/composition/hooks.mjs b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/composition/hooks.mjs deleted file mode 100644 index a810cc1..0000000 --- a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/composition/hooks.mjs +++ /dev/null @@ -1,162 +0,0 @@ -// speckit-wizard — hook-metadata extraction. -// -// The `specify artifact` CLI doesn't yet emit hook metadata. Until it does, -// this temporary wizard-owned enrichment reads extension manifests and -// `.specify/extensions.yml` directly. It intentionally preserves the -// wizard's pre-existing hook extraction behavior while command, template, -// and script composition moves to the CLI. Expanding support for the full -// hook manifest contract belongs with the later migration to native CLI hook -// artifacts, which will replace this compatibility bridge. - -import { readFileSync, existsSync, readdirSync } from "node:fs"; -import { - isAbsolute, - join, - relative as pathRelative, - sep as pathSep, - resolve as pathResolve, -} from "node:path"; -import { platform } from "node:os"; - -const IS_CASE_INSENSITIVE_FS = platform() === "win32" || platform() === "darwin"; - -let _yamlPromise = null; -async function getYaml() { - if (!_yamlPromise) { - _yamlPromise = import("js-yaml").then( - (m) => { - const mod = m.default ?? m; - const schema = mod.JSON_SCHEMA ?? mod.FAILSAFE_SCHEMA; - return { - ...mod, - load: (raw, opts = {}) => mod.load(raw, { schema, ...opts }), - }; - }, - (err) => { - _yamlPromise = null; - throw err; - }, - ); - } - return _yamlPromise; -} - -function safeReadFile(path) { - try { return readFileSync(path, "utf8"); } catch { return null; } -} - -function safeReadDir(path) { - try { return readdirSync(path, { withFileTypes: true }); } catch { return []; } -} - -function repoRelative(root, absPath) { - if (!absPath) return absPath; - const rel = absPath.startsWith(root) ? absPath.slice(root.length) : absPath; - return rel.replace(/^[\\/]+/, "").split(pathSep).join("/"); -} - -/** - * Read one extension manifest from its CLI-reported path, falling back to - * .specify/extensions//extension.yml when the path is unavailable. - * Returns null on missing file, `{ id, error }` on parse failure, else the - * parsed manifest with `hooks` normalized. - */ -export async function readExtensionManifest(root, id, manifestPathHint = null) { - const yaml = await getYaml(); - const rootPath = pathResolve(root); - let manifestPath; - if (typeof manifestPathHint === "string" && manifestPathHint.length) { - manifestPath = pathResolve(rootPath, manifestPathHint); - const relativePath = pathRelative(rootPath, manifestPath); - if (relativePath === ".." - || relativePath.startsWith(`..${pathSep}`) - || isAbsolute(relativePath)) { - return null; - } - } else { - manifestPath = join(rootPath, ".specify", "extensions", id, "extension.yml"); - } - const raw = safeReadFile(manifestPath); - if (!raw) return null; - let doc; - try { doc = yaml.load(raw); } catch { return { id, error: "yaml-parse" }; } - if (!doc || typeof doc !== "object") return { id, error: "empty" }; - const metadata = doc.extension && typeof doc.extension === "object" ? doc.extension : {}; - return { - id, - manifestPath: repoRelative(root, manifestPath), - name: metadata.name ?? doc.name ?? id, - description: metadata.description ?? doc.description ?? "", - version: metadata.version ?? doc.version ?? null, - priority: typeof doc.priority === "number" ? doc.priority : null, - category: doc.category ?? null, - effect: doc.effect ?? null, - hooks: parseHookDeclarations(doc.hooks), - raw: doc, - }; -} - -/** - * Read `.specify/extensions.yml` and return the flattened per-phase hook - * bindings — used to compute the `registered` flag on inline hook chips. - */ -export async function readHooksMap(root) { - const yaml = await getYaml(); - const path = join(root, ".specify", "extensions.yml"); - const raw = safeReadFile(path); - if (!raw) return null; - let doc; - try { doc = yaml.load(raw); } catch { return null; } - if (!doc || typeof doc !== "object" || !doc.hooks || typeof doc.hooks !== "object") return null; - const out = {}; - for (const [phase, bindings] of Object.entries(doc.hooks)) { - if (!Array.isArray(bindings)) continue; - out[phase] = bindings.map((b) => { - if (typeof b === "string") return { extension: b, command: null, optional: false, description: "" }; - if (b && typeof b === "object") { - return { - extension: b.extension ?? null, - command: b.command ?? null, - optional: !!b.optional, - description: b.description ?? "", - }; - } - return null; - }).filter(Boolean); - } - return out; -} - -/** - * Normalize an extension manifest's `hooks` field. Accepts either the - * array form (`[{ phase, command, ... }]`) or the object form - * (`{ before_specify: { command: … } }`). - * - * Compatibility scope: this is the legacy wizard normalizer moved out of - * collect.mjs, not a new implementation of the evolving hook contract. - */ -export function parseHookDeclarations(hooks) { - if (hooks && typeof hooks === "object" && !Array.isArray(hooks)) { - hooks = Object.entries(hooks).map(([phase, cfg]) => ({ - phase, - ...(cfg && typeof cfg === "object" ? cfg : {}), - })); - } - if (!Array.isArray(hooks)) return []; - return hooks - .map((h) => { - if (!h || typeof h !== "object") return null; - return { - phase: h.phase ?? h.trigger ?? null, - command: h.command ?? h.targetCommand ?? null, - optional: !!h.optional, - priority: typeof h.priority === "number" ? h.priority : null, - description: h.description ?? "", - raw: h, - }; - }) - .filter((h) => h && h.phase && h.command); -} - -// Filesystem helpers re-exported for other composition modules. -export { safeReadFile, safeReadDir, repoRelative, IS_CASE_INSENSITIVE_FS, pathResolve, existsSync }; diff --git a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/artifact-cli.test.mjs b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/artifact-cli.test.mjs index 5b96a5a..bba5fcf 100644 --- a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/artifact-cli.test.mjs +++ b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/artifact-cli.test.mjs @@ -298,139 +298,138 @@ describe("buildCompositionFromCli", () => { } }); - test("enriches an extension command with its registered hook bindings", async () => { - const root = mkdtempSync(join(tmpdir(), "speckit-cli-test-")); - try { - const extensionDir = join(root, ".specify", "extensions", "audit-installed"); - mkdirSync(extensionDir, { recursive: true }); - writeFileSync( - join(extensionDir, "extension.yml"), - [ - "extension:", - " id: audit", - " name: Audit Extension", - " version: 1.0.0", - "category: process", - "effect: read-only", - "hooks:", - " after_specify:", - " command: speckit.audit.capture", - " after_plan:", - " command: speckit.audit.capture", - "", - ].join("\n"), - ); - writeFileSync( - join(root, ".specify", "extensions.yml"), - [ - "hooks:", - " after_specify:", - " - extension: audit", - " command: speckit.audit.capture", - " after_plan:", - " - extension: audit", - " command: speckit.audit.capture", - "", - ].join("\n"), - ); + test("processes native CLI hook artifacts and enriches parent phase commands", async () => { + const root = tmpdir(); + const hookArtifact1 = { + id: "hook:after_specify:speckit.audit.capture", + name: "after_specify:speckit.audit.capture", + kind: "hook", + event: "after_specify", + targetCommand: "speckit.audit.capture", + registered: true, + description: "Capture an audit record after specify.", + stack: [ + { + id: "hook:after_specify:speckit.audit.capture", + layer: "extension", + sourceId: "audit", + presetId: null, + presetName: "Audit Extension", + strategy: "additive", + active: true, + priority: 10, + optional: false, + hidden: false, + manifestPath: ".specify/extensions/audit-installed/extension.yml", + lookupId: "extension:audit:hook:after_specify:speckit.audit.capture", + }, + ], + }; + const hookArtifact2 = { + id: "hook:after_plan:speckit.audit.capture", + name: "after_plan:speckit.audit.capture", + kind: "hook", + event: "after_plan", + targetCommand: "speckit.audit.capture", + registered: true, + description: "Capture an audit record after plan.", + stack: [ + { + id: "hook:after_plan:speckit.audit.capture", + layer: "extension", + sourceId: "audit", + presetId: null, + presetName: "Audit Extension", + strategy: "additive", + active: true, + priority: 10, + optional: false, + hidden: false, + manifestPath: ".specify/extensions/audit-installed/extension.yml", + lookupId: "extension:audit:hook:after_plan:speckit.audit.capture", + }, + ], + }; - const extensionCommand = { - id: "command:speckit.audit.capture", - name: "speckit.audit.capture", - kind: "command", - description: "Capture an audit record.", - stack: [ - { - id: "command:speckit.audit.capture", - layer: "extension", - sourceId: "audit", - presetId: null, - presetName: null, - strategy: "replace", - active: true, - hidden: false, - manifestPath: ".specify/extensions/audit-installed/extension.yml", - lookupId: "extension:audit:command:speckit.audit.capture", - sourcePath: ".specify/extensions/audit-installed/commands/capture.md", - }, - ], - }; - const comp = await buildCompositionFromCli({ - workspaceRoot: root, - presetItems: [], - extensionItems: [], - runner: fakeRunner([ - ...canonicalCommandRows(), - extensionCommand, - ]), - }); + const comp = await buildCompositionFromCli({ + workspaceRoot: root, + presetItems: [], + extensionItems: [], + runner: fakeRunner([ + ...canonicalCommandRows(), + hookArtifact1, + hookArtifact2, + ]), + }); - assert.equal( - comp.artifacts.some( - (artifact) => artifact.kind === "command" - && artifact.id === "commands/speckit.audit.capture", - ), - false, - ); + const hooks = comp.artifacts.filter((a) => a.kind === "hook"); + assert.equal(hooks.length, 2); - const hook = comp.artifacts.find( - (artifact) => artifact.kind === "hook" - && artifact.id === "commands/speckit.audit.capture", + for (const phase of ["specify", "plan"]) { + const parent = comp.artifacts.find( + (artifact) => artifact.id === `commands/speckit.${phase}`, ); - assert.ok(hook); assert.deepEqual( - hook.hookBindings.map(({ phase, extensionId, targetCommand }) => ({ - phase, + parent.hooks.map(({ phase: hookPhase, extensionId, registered }) => ({ + phase: hookPhase, extensionId, - targetCommand, + registered, })), [ { - phase: "after_specify", - extensionId: "audit", - targetCommand: "speckit.audit.capture", - }, - { - phase: "after_plan", + phase: `after_${phase}`, extensionId: "audit", - targetCommand: "speckit.audit.capture", + registered: true, }, ], ); - assert.equal(hook.stack[0].sourceId, "audit"); - assert.equal(hook.stack[0].presetId, null); - assert.equal( - hook.stack[0].sourcePath, - ".specify/extensions/audit-installed/commands/capture.md", - ); + } - for (const phase of ["specify", "plan"]) { - const parent = comp.artifacts.find( - (artifact) => artifact.id === `commands/speckit.${phase}`, - ); - assert.deepEqual( - parent.hooks.map(({ phase: hookPhase, extensionId, registered }) => ({ - phase: hookPhase, - extensionId, - registered, - })), - [ - { - phase: `after_${phase}`, - extensionId: "audit", - registered: true, - }, - ], - ); - } + assert.equal(comp.extensions[0].id, "audit"); + assert.equal(comp.extensions[0].provides.hooks, 2); + }); - assert.equal(comp.extensions[0].name, "Audit Extension"); - assert.equal(comp.extensions[0].version, "1.0.0"); - assert.equal(comp.extensions[0].provides.commands, 1); - assert.equal(comp.extensions[0].provides.hooks, 2); - } finally { - rmSync(root, { recursive: true, force: true }); - } + test("carries the active layer's optional flag into the parent command's inline hooks[] entry", async () => { + const root = tmpdir(); + const hookArtifact = { + id: "hook:after_specify:speckit.agent-context.update", + name: "after_specify:speckit.agent-context.update", + kind: "hook", + event: "after_specify", + targetCommand: "speckit.agent-context.update", + registered: true, + description: "Refresh agent context after specification.", + stack: [ + { + id: "hook:after_specify:speckit.agent-context.update", + layer: "extension", + sourceId: "agent-context", + presetId: null, + presetName: "Coding Agent Context", + strategy: "additive", + active: true, + priority: 10, + optional: true, + hidden: false, + manifestPath: ".specify/extensions/agent-context/extension.yml", + lookupId: "extension:agent-context:hook:after_specify:speckit.agent-context.update", + }, + ], + }; + + const comp = await buildCompositionFromCli({ + workspaceRoot: root, + presetItems: [], + extensionItems: [], + runner: fakeRunner([ + ...canonicalCommandRows(), + hookArtifact, + ]), + }); + + const parent = comp.artifacts.find((artifact) => artifact.id === "commands/speckit.specify"); + assert.equal(parent.hooks.length, 1); + assert.equal(parent.hooks[0].optional, true); }); test("synthesizes a canonical pipeline when no inference is needed", async () => { diff --git a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/composition-apply.test.mjs b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/composition-apply.test.mjs index 1e32ac1..5004451 100644 --- a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/composition-apply.test.mjs +++ b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/composition-apply.test.mjs @@ -2,6 +2,7 @@ import assert from "node:assert/strict"; import { test } from "node:test"; import { normalizeHookArtifactsInComposition, + applyComposition, } from "../canvas-runtime/composition-apply.mjs"; test("preserves command and template artifacts with the same name", () => { @@ -86,3 +87,130 @@ test("preserves a hook whose binding already identifies its command", () => { "speckit.audit.capture", ); }); + +test("does not misattribute a native hook to another command from the same provider", () => { + // Native hook ids are shaped as `hooks/:` (never + // `commands/`), and the artifact's own `targetCommand` field + // is the authoritative source of truth set at shape time. A provider + // contributing multiple commands (e.g. an extension with several + // command artifacts) must not cause the hook to be rewritten to + // whichever of those commands happens to be indexed first. + const hook = { + id: "hooks/after_plan:speckit.audit.capture", + kind: "hook", + targetCommand: "speckit.audit.capture", + stack: [ + { + layer: "extension", + sourceId: "audit", + presetId: null, + active: true, + }, + ], + hookBindings: [ + { + phase: "after_plan", + extensionId: "audit", + targetCommand: "speckit.audit.capture", + }, + ], + hookBinding: { + phase: "after_plan", + extensionId: "audit", + targetCommand: "speckit.audit.capture", + }, + }; + const composition = { + artifacts: [ + { + id: "commands/speckit.audit.scan", + kind: "command", + stack: [ + { + layer: "extension", + sourceId: "audit", + presetId: null, + active: true, + }, + ], + }, + hook, + { + id: "commands/speckit.audit.capture", + kind: "command", + stack: [ + { + layer: "extension", + sourceId: "audit", + presetId: null, + active: true, + }, + ], + }, + ], + }; + + const normalized = normalizeHookArtifactsInComposition(composition); + const normalizedHook = normalized.artifacts.find( + (artifact) => artifact.kind === "hook", + ); + + assert.equal(normalizedHook.id, "hooks/after_plan:speckit.audit.capture"); + assert.equal( + normalizedHook.hookBindings[0].targetCommand, + "speckit.audit.capture", + ); + assert.equal( + normalizedHook.hookBinding.targetCommand, + "speckit.audit.capture", + ); +}); + +test("applyComposition preserves the additive strategy on native hook layers", async () => { + // artifact-cli.mjs shapes native hook layers with strategy "additive" + // (hooks stack alongside a command rather than replace/wrap/prepend/ + // append it). applyComposition's own strategy allowlist must accept + // it too, or every hook layer gets silently coerced to "replace" on + // the way into the cached/UI composition. + const inst = { + cachedComposition: undefined, + cachedPresetItems: [], + cachedExtensionItems: [], + broadcast: () => {}, + }; + const input = { + artifacts: [ + { + id: "hooks/after_plan:speckit.audit.capture", + kind: "hook", + targetCommand: "speckit.audit.capture", + stack: [ + { + layer: "extension", + sourceId: "audit", + presetId: null, + strategy: "additive", + active: true, + }, + ], + hookBindings: [ + { + phase: "after_plan", + extensionId: "audit", + targetCommand: "speckit.audit.capture", + }, + ], + hookBinding: { + phase: "after_plan", + extensionId: "audit", + targetCommand: "speckit.audit.capture", + }, + }, + ], + }; + + const result = await applyComposition(inst, input); + const hook = result.artifacts.find((artifact) => artifact.kind === "hook"); + + assert.equal(hook.stack[0].strategy, "additive"); +}); diff --git a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/composition-artifacts.js b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/composition-artifacts.js index 46b8281..b1b7327 100644 --- a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/composition-artifacts.js +++ b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/composition-artifacts.js @@ -151,22 +151,29 @@ export function renderCompositionArtifacts() { // "Dispatched after /X" subline) even when we're viewing the Commands // subtab. The Hooks subtab is unaffected — hook artifacts still render // as their own rows there. - // Hook-target lookup: every kind:"hook" artifact carries the id of the - // command it dispatches. A command can be the target of MULTIPLE hooks - // (e.g. `speckit.agent-context.update` fires from BOTH `after_specify` - // AND `after_plan`) so we store an ARRAY of hook artifacts per target - // id — collapsing to a single value here would silently drop trigger - // rows on the target command's "Dispatched after /X" subline. + // Hook-target lookup: every kind:"hook" artifact carries its + // `targetCommand` (the command it dispatches) — NOT its own id. + // Native hook ids are shaped `hooks/:` (see + // cliIdToWizardId), distinct from the target command's own + // `commands/` id, so keying this map by the hook's `a.id` + // never matches the command row it's meant to annotate. Key by the + // normalized target command id instead. A command can be the target + // of MULTIPLE hooks (e.g. `speckit.agent-context.update` fires from + // BOTH `after_specify` AND `after_plan`) so we store an ARRAY of hook + // artifacts per target id — collapsing to a single value here would + // silently drop trigger rows on the target command's "Dispatched + // after /X" subline. const hookByTargetId = new Map(); const sourceByCommandId = new Map(); for (const a of visible) { if (a.kind === "hook") { const hasBindings = (Array.isArray(a.hookBindings) && a.hookBindings.length) || !!a.hookBinding; - if (hasBindings) { - const list = hookByTargetId.get(a.id) || []; + if (hasBindings && a.targetCommand) { + const targetId = `commands/${a.targetCommand}`; + const list = hookByTargetId.get(targetId) || []; list.push(a); - hookByTargetId.set(a.id, list); + hookByTargetId.set(targetId, list); } } if (a.kind === "command") { @@ -381,18 +388,22 @@ export function renderArtifactRow(artifact, opts = {}) { // opening the `extension.yml` wiring declaration. let sourcePath = artifactSourcePath(artifact, activeLayer); const _isHookHere = artifact.kind === "hook"; - // For hook artifacts, the artifact.id is already the target command - // id (e.g. "commands/speckit.companion.capture"). Prefer opening that - // command's .md rather than the extension.yml wiring declaration. + // For hook artifacts, `artifact.id` is shaped `hooks/:` + // (see cliIdToWizardId) — it is NOT the target command's id, despite + // that having been true pre-migration. Use the hook's authoritative + // `targetCommand` field to resolve the command it dispatches, so we + // can open that command's .md rather than the extension.yml wiring + // declaration. let _hookTargetLabel = ""; if (_isHookHere) { const sourceByCommandId = opts?.sourceByCommandId; - const targetSrc = sourceByCommandId - ? (sourceByCommandId.get(artifact.id) || sourceByCommandId.get(bareCommandId(artifact.id))) + const targetCommandId = artifact.targetCommand ? `commands/${artifact.targetCommand}` : null; + const targetSrc = sourceByCommandId && targetCommandId + ? (sourceByCommandId.get(targetCommandId) || sourceByCommandId.get(artifact.targetCommand)) : ""; if (targetSrc) { sourcePath = targetSrc; - _hookTargetLabel = `/${bareCommandId(artifact.id)}`; + _hookTargetLabel = `/${artifact.targetCommand}`; } } const idTitle = _hookTargetLabel diff --git a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/composition.js b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/composition.js index 21ba34f..4f38d74 100644 --- a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/composition.js +++ b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/composition.js @@ -164,6 +164,15 @@ export function computeProviderContributions(artifacts) { // fired from both `after_specify` AND `after_plan`) into a single // artifact. Count each binding as its own contribution so the // per-extension totals match the Hooks subtab. + // NOTE: this weight is applied per stack layer below, so a hook + // artifact with bindings owned by more than one provider currently + // over-counts (every provider in the stack gets the full weight, + // not just its own share). This derivation is a stand-in for + // provider-scoped hook counts the CLI should eventually expose + // directly (e.g. via a JSON-capable extension/preset list command); + // once that lands, this local aggregation should be replaced with + // reading the provider's own reported hook count instead of + // recomputing it from the artifact stack here. const weight = kind === "hook" ? Math.max(1, Array.isArray(a.hookBindings) ? a.hookBindings.length : 0) : 1; @@ -223,6 +232,10 @@ export function computeCompositionKindCounts(artifacts) { // fired from both `after_specify` AND `after_plan`) into a single // artifact. Count each binding as its own row so the header // summary matches the per-binding rows the Hooks subtab renders. + // NOTE: like the per-provider tally above, this is a local + // derivation from the artifact stack and should eventually be + // replaced with hook counts reported directly by the CLI once a + // JSON-capable extension/preset list command exposes them. const weight = kind === "hook" ? Math.max(1, Array.isArray(a.hookBindings) ? a.hookBindings.length : 0) : 1; diff --git a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/phase-runtime.js b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/phase-runtime.js index f901a39..46e03a5 100644 --- a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/phase-runtime.js +++ b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/phase-runtime.js @@ -769,10 +769,15 @@ export function renderMoreCommandsPanel() { const compArtifactsAll = state.snapshot?.composition?.artifacts ?? []; const extensionSectionHtmlParts = compExtensions.map((ext) => { // Extension items in the More-Commands panel: only user-invokable - // commands render as cards. Hook artifacts share the same id as - // the command they dispatch — we merge their `hookBinding` onto - // the matching command card (auto-run pill + Triggered-by footer) - // instead of surfacing them as a second card. + // commands render as cards. Hook artifacts are a distinct kind + // whose id is shaped `hooks/:` — NOT the same id + // as the command they dispatch (see cliIdToWizardId). We merge + // their `hookBinding` onto the matching command card (auto-run + // pill + Triggered-by footer) by joining on `targetCommand`, and + // NEVER surface a hook artifact as its own card: every hook's + // `targetCommand` is guaranteed to have a corresponding `command` + // artifact (confirmed against the CLI's artifact list), so a + // hook's own id must never leak into a rendered card. const rawItems = compArtifactsAll.filter((a) => { if (a.kind !== "command" && a.kind !== "hook") return false; const active = (a.stack ?? []).find((l) => l.active); @@ -782,32 +787,25 @@ export function renderMoreCommandsPanel() { // A single extension command can be the target of MULTIPLE hook // bindings (e.g. `speckit.agent-context.update` fires from both // `after_specify` AND `after_plan`). Collect every binding per - // target command id so the card can render every parent phase in - // its "Triggered by" footer — not just the last one seen. + // target command id (normalized to the command's own `commands/` + // shape) so the card can render every parent phase in its + // "Triggered by" footer — not just the last one seen. const hookBindingsByCommandId = new Map(); for (const a of rawItems) { - if (a.kind !== "hook") continue; + if (a.kind !== "hook" || !a.targetCommand) continue; const bindings = Array.isArray(a.hookBindings) && a.hookBindings.length ? a.hookBindings : (a.hookBinding ? [a.hookBinding] : []); if (!bindings.length) continue; - const list = hookBindingsByCommandId.get(a.id) || []; + const targetId = `commands/${a.targetCommand}`; + const list = hookBindingsByCommandId.get(targetId) || []; for (const b of bindings) list.push(b); - hookBindingsByCommandId.set(a.id, list); + hookBindingsByCommandId.set(targetId, list); } - const items = rawItems.filter((a) => { - if (a.kind === "command") return true; - // Include hook artifacts too, so extensions whose only - // user-visible entry point is a hook target (e.g. - // `agent-context.update`, which the assembler excludes from - // the command kind because it's declared under `hooks:`) still - // get a card — rendered as a passive hook tile with an - // "Auto-runs" indicator and no + Add affordance. Skip any - // hook whose id is already covered by a command in rawItems - // (defensive; the assembler prevents this collision today). - if (a.kind !== "hook") return false; - return !rawItems.some((c) => c.kind === "command" && c.id === a.id); - }); + // Hook artifacts never render as their own card — only commands + // do. Their bindings are looked up via `hookBindingsByCommandId` + // (keyed by target command id) when rendering the command card. + const items = rawItems.filter((a) => a.kind === "command"); items.sort((a, b) => collator.compare(a.id, b.id)); const cards = items.map((art) => renderExtensionCommandCard(art, ext, hookBindingsByCommandId.get(art.id) || null)).join(""); const openAttr = isSectionOpen(`extension:${ext.id}`) ? " open" : "";