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 0ea88c7..eb7b205 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 @@ -4,6 +4,7 @@ import { buildCompositionFromCli, } from "../composition/artifact-cli.mjs"; import { computePipelineFastPath } from "../composition/pipeline-fast-path.mjs"; +import { findLayerByLookupId } from "../ui/lookup-id.mjs"; import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; @@ -193,6 +194,11 @@ describe("buildCompositionFromCli", () => { assert.equal(winner.hidden, false); assert.equal(winner.manifestPath, ".specify/presets/compliance/preset.yml"); assert.equal(winner.lookupId, "preset:compliance:command:speckit.plan"); + // Behavioral coverage: the CLI-shaped composition artifact + // round-trips through findLayerByLookupId back to this winner + // (parseLookupId's own shape/edge-case behavior is covered by + // test/lookup-id.test.mjs — no need to re-assert it here). + assert.equal(findLayerByLookupId(cmd, winner.lookupId), winner); // Hidden built-in layer. const built = cmd.stack[1]; diff --git a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/compute-provider-contributions.test.mjs b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/compute-provider-contributions.test.mjs new file mode 100644 index 0000000..6912b58 --- /dev/null +++ b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/compute-provider-contributions.test.mjs @@ -0,0 +1,75 @@ +import assert from "node:assert/strict"; +import { describe, test } from "node:test"; +import { computeProviderContributions } from "../ui/composition.js"; + +// computeProviderContributions buckets stack layers by provider id, tallying +// "customized" (core-inventory overrides) vs "added" (new) contributions. +// Bucket key resolution: prefer parseLookupId(layer.lookupId)?.providerId, +// falling back to layer.presetId ?? layer.extensionId for wizard-synthesized +// hook layers (which carry lookupId: null). + +describe("computeProviderContributions", () => { + test("buckets a preset winner by lookupId providerId", () => { + const artifacts = [ + { + id: "commands/speckit.plan", + kind: "command", + stack: [ + { layer: "preset", presetId: "compliance", lookupId: "preset:compliance:command:speckit.plan" }, + ], + }, + ]; + const contributions = computeProviderContributions(artifacts); + assert.ok(contributions.has("compliance")); + }); + + test("buckets an extension layer by lookupId providerId", () => { + const artifacts = [ + { + id: "templates/spec.md", + kind: "template", + stack: [ + { layer: "extension", extensionId: "foo", lookupId: "extension:foo:template:spec.md" }, + ], + }, + ]; + const contributions = computeProviderContributions(artifacts); + assert.ok(contributions.has("foo")); + }); + + test("falls back to extensionId for hook-synthetic layers with lookupId: null", () => { + const artifacts = [ + { + id: "commands/some-hook-command", + kind: "hook", + hookBindings: [{ phase: "after_specify" }], + stack: [ + { layer: "extension", extensionId: "hooks-ext", lookupId: null }, + ], + }, + ]; + const contributions = computeProviderContributions(artifacts); + assert.ok(contributions.has("hooks-ext")); + }); + + test("lookupId wins when both lookupId and legacy presetId/extensionId are present", () => { + const artifacts = [ + { + id: "commands/speckit.plan", + kind: "command", + stack: [ + { + layer: "preset", + presetId: "legacy-id", + extensionId: "legacy-ext-id", + lookupId: "preset:compliance:command:speckit.plan", + }, + ], + }, + ]; + const contributions = computeProviderContributions(artifacts); + assert.ok(contributions.has("compliance")); + assert.ok(!contributions.has("legacy-id")); + assert.ok(!contributions.has("legacy-ext-id")); + }); +}); diff --git a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/lookup-id.test.mjs b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/lookup-id.test.mjs new file mode 100644 index 0000000..3af2bd8 --- /dev/null +++ b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/lookup-id.test.mjs @@ -0,0 +1,62 @@ +import assert from "node:assert/strict"; +import { describe, test } from "node:test"; +import { parseLookupId, findLayerByLookupId } from "../ui/lookup-id.mjs"; + +describe("parseLookupId", () => { + test("parses a preset command lookupId", () => { + assert.deepEqual( + parseLookupId("preset:compliance:command:speckit.plan"), + { providerKind: "preset", providerId: "compliance", kind: "command", name: "speckit.plan" }, + ); + }); + + test("parses an extension template lookupId", () => { + assert.deepEqual( + parseLookupId("extension:foo:template:spec.md"), + { providerKind: "extension", providerId: "foo", kind: "template", name: "spec.md" }, + ); + }); + + test("treats colons in as opaque tail", () => { + const parsed = parseLookupId("preset:x:command:has:colons:in:name"); + assert.equal(parsed.name, "has:colons:in:name"); + assert.equal(parsed.providerId, "x"); + assert.equal(parsed.kind, "command"); + }); + + test("returns null for null/empty/garbage/unknown-provider-kind input", () => { + assert.equal(parseLookupId(null), null); + assert.equal(parseLookupId(""), null); + assert.equal(parseLookupId("garbage"), null); + assert.equal(parseLookupId("core:x:y:z"), null); + assert.equal(parseLookupId(undefined), null); + assert.equal(parseLookupId("preset:x:y"), null); + }); +}); + +describe("findLayerByLookupId", () => { + const artifact = { + id: "commands/speckit.plan", + stack: [ + { lookupId: "preset:compliance:command:speckit.plan", active: true }, + { lookupId: null, active: false }, + ], + }; + + test("returns the matching layer", () => { + const layer = findLayerByLookupId(artifact, "preset:compliance:command:speckit.plan"); + assert.equal(layer, artifact.stack[0]); + }); + + test("returns null when lookupId is null", () => { + assert.equal(findLayerByLookupId(artifact, null), null); + }); + + test("returns null when no layer matches", () => { + assert.equal(findLayerByLookupId(artifact, "preset:other:command:x"), null); + }); + + test("returns null when compArtifact is null", () => { + assert.equal(findLayerByLookupId(null, "preset:compliance:command:speckit.plan"), null); + }); +}); diff --git a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/phase-runtime-command-source-path.test.mjs b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/phase-runtime-command-source-path.test.mjs new file mode 100644 index 0000000..69d0a05 --- /dev/null +++ b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/phase-runtime-command-source-path.test.mjs @@ -0,0 +1,91 @@ +import assert from "node:assert/strict"; +import { describe, test, beforeEach } from "node:test"; +import { commandSourcePath } from "../ui/phase-runtime.js"; +import { state } from "../ui/state.js"; + +// commandSourcePath resolves the on-disk markdown path for a command tile. +// Priority: (1) composition activeLayer.sourcePath, (2) derived preset path +// from the winning layer's `lookupId` provider id + `p.commandName`, +// (3) legacy `p.source: "preset:"` string fallback — this is the ONLY +// provenance the real snapshot-builder.mjs::buildCommands producer attaches +// to preset-only command objects today (no `lookupId` yet), so this +// fallback must stay reachable until that producer is migrated. + +describe("commandSourcePath", () => { + beforeEach(() => { + state.snapshot = null; + }); + + test("returns null for falsy input", () => { + assert.equal(commandSourcePath(null), null); + }); + + test("prefers composition activeLayer.sourcePath when present", () => { + state.snapshot = { + composition: { + artifacts: [ + { + id: "commands/speckit.plan", + stack: [ + { + active: true, + sourcePath: ".specify/presets/compliance/commands/speckit.plan.md", + lookupId: "preset:compliance:command:speckit.plan", + }, + ], + }, + ], + }, + }; + const p = { id: "plan", commandName: "speckit.plan", lookupId: "preset:compliance:command:speckit.plan" }; + assert.equal(commandSourcePath(p), ".specify/presets/compliance/commands/speckit.plan.md"); + }); + + test("falls back to deriving path from active composition layer's lookupId when sourcePath is absent", () => { + state.snapshot = { + composition: { + artifacts: [ + { + id: "commands/speckit.plan", + stack: [ + { + active: true, + sourcePath: null, + lookupId: "preset:compliance:command:speckit.plan", + }, + ], + }, + ], + }, + }; + const p = { id: "plan", commandName: "speckit.plan", lookupId: null }; + assert.equal(commandSourcePath(p), ".specify/presets/compliance/commands/speckit.plan.md"); + }); + + test("falls back to the phase's own lookupId when there is no composition entry", () => { + state.snapshot = { composition: { artifacts: [] } }; + const p = { id: "plan", commandName: "speckit.plan", lookupId: "preset:game-narrative:command:speckit.plan" }; + assert.equal(commandSourcePath(p), ".specify/presets/game-narrative/commands/speckit.plan.md"); + }); + + test("returns null for an extension-provided lookupId (not a preset path)", () => { + state.snapshot = { composition: { artifacts: [] } }; + const p = { id: "foo", commandName: "speckit.foo.bar", lookupId: "extension:foo:command:speckit.foo.bar" }; + assert.equal(commandSourcePath(p), null); + }); + + test("falls back to legacy p.source string when there is no lookupId at all (real preset-only command shape)", () => { + // Mirrors what snapshot-builder.mjs::buildCommands actually emits for + // a preset-only command: `source: "preset:"`, no + // `lookupId`, and no composition artifact entry. + state.snapshot = { composition: { artifacts: [] } }; + const p = { id: "plan", commandName: "speckit.plan", source: "preset:compliance" }; + assert.equal(commandSourcePath(p), ".specify/presets/compliance/commands/speckit.plan.md"); + }); + + test("returns null when there is no lookupId, no legacy source, and no composition entry", () => { + state.snapshot = { composition: { artifacts: [] } }; + const p = { id: "specify", commandName: "speckit.specify", lookupId: null }; + assert.equal(commandSourcePath(p), null); + }); +}); diff --git a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/server-integration.test.mjs b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/server-integration.test.mjs index 7b60297..79523fb 100644 --- a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/server-integration.test.mjs +++ b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/test/server-integration.test.mjs @@ -739,6 +739,56 @@ test("S6: .specify/presets/.registry order flows through preset-loader into scan } }); +// -------- S6b: scanner → buildStateSnapshot → commandSourcePath ----------- + +test("S6b: a real scanned preset command's snapshot object resolves via commandSourcePath's legacy source fallback", async () => { + // Regression test: buildCommands forwards `cmd.source` ("preset:") + // onto snapshot.commands but does not attach a `lookupId` — that field + // only exists on composition-artifact stack layers, a separate producer. + // commandSourcePath must still resolve a path for these real command + // objects, not silently return null. + const ws = tmpWs(); + try { + mkdirSync(join(ws, ".specify", "presets", "alpha", "commands"), { recursive: true }); + writeFileSync( + join(ws, ".specify", "presets", ".registry"), + JSON.stringify([{ id: "alpha", priority: 100, enabled: true }]), + ); + writeFileSync( + join(ws, ".specify", "presets", "alpha", "preset.yml"), + [ + "preset:", + " name: Alpha", + " version: 1.0.0", + "provides:", + " templates:", + " - type: command", + " name: speckit.a1", + " file: commands/a1.md", + "", + ].join("\n"), + ); + writeFileSync( + join(ws, ".specify", "presets", "alpha", "commands", "a1.md"), + "---\nhandoffs: []\n---\n# A1\n", + ); + + const scan = await scanWorkspace(ws, await realFsDeps()); + const snap = buildStateSnapshot(scan); + const cmd = (snap.commands ?? []).find((c) => c.commandName === "speckit.a1" || c.name === "speckit.a1"); + assert.ok(cmd, `snapshot.commands missing speckit.a1: ${JSON.stringify(snap.commands)}`); + assert.equal(cmd.source, "preset:alpha"); + assert.equal(cmd.lookupId, undefined, "buildCommands does not attach lookupId today"); + + const { commandSourcePath } = await import("../ui/phase-runtime.js"); + const { state } = await import("../ui/state.js"); + state.snapshot = { composition: { artifacts: [] } }; + assert.equal(commandSourcePath(cmd), ".specify/presets/alpha/commands/speckit.a1.md"); + } finally { + rmSync(ws, { recursive: true, force: true }); + } +}); + // -------- S7: scanner → buildStateSnapshot lock/gate ---------------------- test("S7: buildStateSnapshot derives per-phase locked from durable setup completion", async () => { 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 09564ed..7c3bdce 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 @@ -13,6 +13,7 @@ import { renderCompositionArtifacts, setArtifactRowsDeps, } from "./composition-artifacts.js"; +import { parseLookupId } from "./lookup-id.mjs"; // -------- Section: composition/layers.mjs -------- // Single source of truth for the composition layer-stack order. @@ -169,10 +170,12 @@ export function computeProviderContributions(artifacts) { const seen = new Set(); for (const layer of a.stack ?? []) { if (layer.layer !== "preset" && layer.layer !== "extension") continue; - const id = layer.presetId - || layer.extensionId - || layer.presetName - || layer.extensionName; + // Prefer the deterministic lookupId's providerId; fall back to + // legacy presetId/extensionId for wizard-synthesized hook layers + // (applyHookAttributions writes lookupId: null). + const id = parseLookupId(layer.lookupId)?.providerId + ?? layer.presetId + ?? layer.extensionId; if (!id || seen.has(id)) continue; seen.add(id); let bucket = out.get(id); diff --git a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/lookup-id.mjs b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/lookup-id.mjs new file mode 100644 index 0000000..f84aad2 --- /dev/null +++ b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/lookup-id.mjs @@ -0,0 +1,34 @@ +// Pure helpers for the deterministic `lookupId` field carried on composition +// stack layers. +// +// Format: `preset:::` or `extension:::`, +// `null` for core (built-in) layers. Stable across reinstalls; NOT a CLI +// round-trip key (do not send it back to `specify` commands). + +const KNOWN_PROVIDER_KINDS = new Set(["preset", "extension"]); + +// Parse a `lookupId` string into its constituent parts. Returns `null` for +// anything that isn't a recognized `preset:`/`extension:` lookupId (including +// `null`, empty string, garbage, or an unknown provider kind like `core:...`). +// +// Colons inside `` are legal — everything after the third colon is +// treated as the opaque `name` tail (locked decision: do not split further). +export function parseLookupId(lookupId) { + if (typeof lookupId !== "string" || lookupId.length === 0) return null; + const parts = lookupId.split(":"); + if (parts.length < 4) return null; + const [providerKind, providerId, kind, ...nameParts] = parts; + if (!KNOWN_PROVIDER_KINDS.has(providerKind)) return null; + if (!providerId || !kind) return null; + const name = nameParts.join(":"); + if (!name) return null; + return { providerKind, providerId, kind, name }; +} + +// Find the stack layer within a composition artifact whose `lookupId` +// matches. Returns `null` when `lookupId` is falsy or no layer matches. +export function findLayerByLookupId(compArtifact, lookupId) { + if (!lookupId || !compArtifact) return null; + const stack = compArtifact.stack ?? []; + return stack.find((layer) => layer?.lookupId === lookupId) ?? null; +} diff --git a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/phase-card.js b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/phase-card.js index 144528e..a44ad04 100644 --- a/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/phase-card.js +++ b/plugins/spec-kit-copilot-wizard/extensions/speckit-wizard-canvas/ui/phase-card.js @@ -329,6 +329,7 @@ export function synthesizeCanonicalPhase(id) { optional: isCanonicalOptional(id), locked: false, source: "core", + lookupId: null, artifact: null, artifactPath: null, }; 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 715501b..d624244 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 @@ -22,6 +22,7 @@ import { import { CANONICAL_BY_FULL } from "../pipeline/effective-phases.mjs"; import { resolveHooksForCommand } from "../pipeline/active-artifacts.mjs"; import { effectivePipelinePhases } from "../pipeline/effective-phases.mjs"; +import { findLayerByLookupId, parseLookupId } from "./lookup-id.mjs"; // -------- Section: phase/clarifications.js -------- @@ -198,6 +199,7 @@ export function resolvePipelineEntry(id, snapshot) { // (state.json has status:"done", artifactPath set). Mirrors the // extension branch below. const scanned = snapshot?.phases?.[id] ?? null; + const active = lookupActiveLayerForCommand({ id, commandName: `speckit.${id}` }); return { kind: "core", id, @@ -217,6 +219,7 @@ export function resolvePipelineEntry(id, snapshot) { commandName: `speckit.${id}`, artifactPath: scanned?.artifactPath ?? null, lastRunAt: scanned?.lastRunAt ?? null, + lookupId: active?.lookupId ?? null, ...(scanned?.folderPath ? { folderPath: scanned.folderPath } : {}), }, }; @@ -234,6 +237,7 @@ export function resolvePipelineEntry(id, snapshot) { // there so the phase card renders a live "Writes to" link the same // way core phases do. const scanned = snapshot?.phases?.[id] ?? null; + const active = lookupActiveLayerForCommand({ id, commandName: extResolved.commandName }); return { kind: "extension", id, @@ -250,6 +254,7 @@ export function resolvePipelineEntry(id, snapshot) { source: `extension:${extResolved.ext.id}`, artifactPath: scanned?.artifactPath ?? null, lastRunAt: scanned?.lastRunAt ?? null, + lookupId: active?.lookupId ?? null, // LLM-inferred metadata from artifact-targets.json cache // (via extension.inferArtifactTargets prompt). The phase // card reads these to render the tagline under the header @@ -890,14 +895,30 @@ export function renderMoreCommandsPanel() { // Resolve the on-disk markdown path for a command tile, when known. // Priority: // 1. composition activeLayer.sourcePath (accurate — includes preset overrides). -// 2. derived preset path from `p.source` + `p.commandName`. +// 2. derived preset path from the `lookupId` provider id + `p.commandName`. +// 3. derived preset path from the legacy `p.source` string ("preset:") — +// still the ONLY provenance snapshot-builder.mjs::buildCommands attaches +// to preset-only command objects (it forwards `cmd.source` but not a +// `lookupId`; the composition-artifact lookupId pipeline is a separate +// producer). Keep this until that producer starts propagating lookupId. // Returns null when the file isn't on disk (e.g. synthesized core-only commands). export function commandSourcePath(p) { if (!p) return null; - const activeLayer = lookupActiveLayer(p.id, p.commandName); + const activeLayer = lookupActiveLayerForCommand(p); if (activeLayer?.sourcePath) return activeLayer.sourcePath; - // Derive from `source: "preset:"` for preset-only commands - // that don't have composition entries (game-narrative extras). + // Derive from the preset provider id for preset-only commands that + // don't have composition entries (game-narrative extras). Prefer the + // active composition layer's lookupId, falling back to the phase's own. + const parsedActive = parseLookupId(activeLayer?.lookupId); + const parsedPhase = parseLookupId(p.lookupId); + const parsed = parsedActive?.providerKind === "preset" ? parsedActive + : parsedPhase?.providerKind === "preset" ? parsedPhase + : null; + if (parsed && p.commandName) { + return `.specify/presets/${parsed.providerId}/commands/${p.commandName}.md`; + } + // Legacy fallback: derive from `source: "preset:"` for + // preset-only commands that don't yet carry a `lookupId` at all. if (typeof p.source === "string" && p.source.startsWith("preset:") && p.commandName) { const presetId = p.source.slice("preset:".length).split(":")[0]; return `.specify/presets/${presetId}/commands/${p.commandName}.md`; @@ -905,8 +926,12 @@ export function commandSourcePath(p) { return null; } -// Look up the winning composition layer for a command id (either "commands/" or a phase id). -export function lookupActiveLayer(id, commandName) { +// Look up the winning composition layer for a phase, using phase-discovery +// semantics: prefer the "commands/" artifact, falling back to +// the phase `id` (either "commands/" or a bare phase id). +export function lookupActiveLayerForCommand(p) { + const id = p?.id; + const commandName = p?.commandName; const compArtifacts = state.snapshot?.composition?.artifacts ?? []; const cmdLookupId = commandName ? `commands/${commandName}` : null; const compArtifact = @@ -915,3 +940,16 @@ export function lookupActiveLayer(id, commandName) { return (compArtifact?.stack ?? []).find((l) => l.active) || null; } +// Look up a composition stack layer by its deterministic `lookupId` +// (see ui/lookup-id.mjs). Returns `null` when no artifact/layer matches. +export function lookupLayerByLookupId(lookupId) { + if (!lookupId) return null; + const compArtifacts = state.snapshot?.composition?.artifacts ?? []; + for (const compArtifact of compArtifacts) { + const layer = findLayerByLookupId(compArtifact, lookupId); + if (layer) return layer; + } + return null; +} + +