diff --git a/.changeset/archive-early-synced-removed.md b/.changeset/archive-early-synced-removed.md new file mode 100644 index 0000000000..25ed4dba06 --- /dev/null +++ b/.changeset/archive-early-synced-removed.md @@ -0,0 +1,5 @@ +--- +'@fission-ai/openspec': patch +--- + +`openspec archive` no longer aborts when a REMOVED delta's requirement is already gone from the main spec (the early-sync pattern the sync skill teaches): it warns, treats the removal as already applied, and reports applied-only totals. In `--json` mode those warnings are carried in a new optional `warnings` array on the archive result. When every operation for a spec was already synced, archive skips rewriting that file instead of churning normalization differences into it. A delta that both RENAMEs and REMOVEs the same requirement is now rejected explicitly, by both `validate` and `archive` — the two spellings are compared case- and whitespace-insensitively — and a REMOVED header that differs only in case or whitespace from an existing requirement still aborts (that is a typo, not an early sync). Also fixed: the archive delta gate matches section headers case-insensitively like the parser; symlinked `specs//spec.md` files are discovered instead of silently dropped; `openspec show ` no longer prints a spurious "scenarios" flag warning; files generated for qwen and bob reference commands by their real hyphenated names (`/opsx-`), and init's getting-started hint follows suit; apply/update/onboard guidance names the CLI fallback for profiles that don't install `/opsx:continue` or `/opsx:new`. diff --git a/docs/agent-contract.md b/docs/agent-contract.md index c2429d10eb..e88d3d4795 100644 --- a/docs/agent-contract.md +++ b/docs/agent-contract.md @@ -69,7 +69,7 @@ Change: `{ "id", "title", "deltaCount", "deltas": [...], "root" }`. Spec: `{ "id Success: `{ "change": { "id", "path", "metadataPath", "schema" }, "root" }`. Failure: `{ "change": null, "status": [d] }`, exit 1. ### 4.8 `archive --json` -Success: `{ "archive": { "change", "archivedAs": "YYYY-MM-DD-name", "path", "specsUpdated", "totals"? }, "root" }`. Failure: `{ "archive": null, "root"?, "status": [d] }`, exit 1. JSON mode is strictly non-interactive: every prompt point becomes an `archive_*` code. +Success: `{ "archive": { "change", "archivedAs": "YYYY-MM-DD-name", "path", "specsUpdated", "totals"?, "warnings"? }, "root" }`. Failure: `{ "archive": null, "root"?, "status": [d] }`, exit 1. `specsUpdated` is true only when at least one spec file was written; an already-synced change archives with all-zero totals and the skips listed in `warnings`. JSON mode is strictly non-interactive: every prompt point becomes an `archive_*` code. ### 4.9 `doctor --json` `{ "root": { "path", "source", "store_id"?, "healthy", "status": [] }, "store": { "id", "metadata": {present,valid,remote?}, "origin_url"?, "drift"?: {ahead,behind}, "status": [] } | null, "references": [...], "status": [] }`. `drift` (present only for a git-backed store checkout that has an upstream tracking ref) is ahead/behind counts against the last-fetched upstream, not the live remote. Health findings of any severity exit 0. Failure payload: `{ "root": null, "store": null, "references": [], "status": [d] }`, exit 1. diff --git a/openspec/specs/openspec-conventions/spec.md b/openspec/specs/openspec-conventions/spec.md index b47a98eb3e..85ad36d619 100644 --- a/openspec/specs/openspec-conventions/spec.md +++ b/openspec/specs/openspec-conventions/spec.md @@ -183,8 +183,10 @@ The archive process SHALL programmatically apply delta changes to current specif 2. Parse REMOVED sections and remove by normalized header match 3. Parse MODIFIED sections and replace by normalized header match (using new names if renamed) 4. Parse ADDED sections and append new requirements -- **AND** validate that all MODIFIED/REMOVED headers exist in current spec -- **AND** validate that ADDED headers don't already exist +- **AND** validate that all MODIFIED headers exist in current spec +- **AND** treat a REMOVED header that is already absent as already removed (warn and continue; a REMOVED header that names the FROM side of a RENAMED in the same delta — compared case- and whitespace-insensitively — or that differs only in case or whitespace from an existing requirement, is a conflict) +- **AND** treat an ADDED header that already exists with identical content as already synced (differing content is a conflict) +- **AND** treat a RENAMED whose source is gone but target present as already synced - **AND** generate the updated spec in the main specs/ directory #### Scenario: Handling conflicts during archive diff --git a/skills/openspec-apply-change/SKILL.md b/skills/openspec-apply-change/SKILL.md index 49f3b4bd2c..3ea1e37814 100644 --- a/skills/openspec-apply-change/SKILL.md +++ b/skills/openspec-apply-change/SKILL.md @@ -48,7 +48,7 @@ Implement tasks from an OpenSpec change. - Dynamic instruction based on current state **Handle states:** - - If `state: "blocked"` (missing artifacts): show message, suggest using openspec-continue-change + - If `state: "blocked"` (missing artifacts): show message, suggest using openspec-continue-change (if it is not installed, run `openspec status --change "" --json` to see the next artifact and `openspec instructions --change "" --json` for how to create it) - If `state: "all_done"`: congratulate, suggest archive - Otherwise: proceed to implementation diff --git a/skills/openspec-onboard/SKILL.md b/skills/openspec-onboard/SKILL.md index a9b1a7049a..9a4a3887f0 100644 --- a/skills/openspec-onboard/SKILL.md +++ b/skills/openspec-onboard/SKILL.md @@ -475,7 +475,7 @@ This same rhythm works for any size change—a small fix or a major feature. | `/openspec-apply-change` | Implement tasks from a change | | `/openspec-archive-change` | Archive a completed change | -**Additional commands:** +**Additional commands** (only if installed - availability depends on your profile): | Command | What it does | |--------------------|----------------------------------------------------------| @@ -503,7 +503,7 @@ If the user says they need to stop, want to pause, or seem disengaged: No problem! Your change is saved at the `changeRoot` reported by `openspec status --change "" --json`. To pick up where we left off later: -- `/openspec-continue-change ` - Resume artifact creation +- `/openspec-continue-change ` - Resume artifact creation (if installed; otherwise `openspec status --change "" --json` shows the next artifact) - `/openspec-apply-change ` - Jump to implementation (if tasks exist) The work won't be lost. Come back whenever you're ready. @@ -527,7 +527,7 @@ If the user says they just want to see the commands or skip the tutorial: | `/openspec-apply-change ` | Implement tasks | | `/openspec-archive-change ` | Archive when done | -**Additional commands:** +**Additional commands** (only if installed - availability depends on your profile): | Command | What it does | |---------------------------|-------------------------------------| diff --git a/skills/openspec-update-change/SKILL.md b/skills/openspec-update-change/SKILL.md index d708818204..b39170da77 100644 --- a/skills/openspec-update-change/SKILL.md +++ b/skills/openspec-update-change/SKILL.md @@ -83,3 +83,4 @@ After each invocation, show: - Do not advance the build frontier: no new artifacts, no new files under glob artifacts - that is `/openspec-continue-change`'s job. - Confirm every edit with the user before writing. - If the request changes the change's *intent* rather than refining it, recommend starting fresh with `/openspec-new-change` (the "Update vs. Start Fresh" heuristic). +- `/openspec-continue-change` and `/openspec-new-change` may not be installed (core profile). When suggesting one that is unavailable, point to the CLI instead: `openspec status --change "" --json` shows the next artifact and `openspec instructions --change "" --json` explains how to create it. diff --git a/src/commands/show.ts b/src/commands/show.ts index 408f11a7ca..bfdb2958dc 100644 --- a/src/commands/show.ts +++ b/src/commands/show.ts @@ -203,10 +203,14 @@ export class ShowCommand { private warnIrrelevantFlags(type: ItemType, options: { [k: string]: any }): boolean { const irrelevant: string[] = []; + // --no-scenarios makes commander default `scenarios` to true, so its + // presence alone does not mean the user passed it — only false does. + const isUserProvided = (k: string) => + k === 'scenarios' ? options[k] === false : k in options; if (type === 'change') { - for (const k of SPEC_FLAG_KEYS) if (k in options) irrelevant.push(k); + for (const k of SPEC_FLAG_KEYS) if (isUserProvided(k)) irrelevant.push(k); } else { - for (const k of CHANGE_FLAG_KEYS) if (k in options) irrelevant.push(k); + for (const k of CHANGE_FLAG_KEYS) if (isUserProvided(k)) irrelevant.push(k); } if (irrelevant.length > 0) { console.error(`Warning: Ignoring flags not applicable to ${type}: ${irrelevant.join(', ')}`); diff --git a/src/core/archive.ts b/src/core/archive.ts index 6c35868c6c..f0f6013b3e 100644 --- a/src/core/archive.ts +++ b/src/core/archive.ts @@ -74,6 +74,8 @@ interface ArchiveResult { path: string; specsUpdated: boolean; totals?: { added: number; modified: number; removed: number; renamed: number }; + /** Non-blocking spec-merge warnings (e.g. a REMOVED requirement that was already gone). */ + warnings?: string[]; } /** @@ -323,7 +325,9 @@ export class ArchiveCommand { for (const { specFile } of hasDeltaSpecs ? [] : await discoverSpecFiles(changeSpecsDir)) { try { const content = await fs.readFile(specFile, 'utf-8'); - if (/^##\s+(ADDED|MODIFIED|REMOVED|RENAMED)\s+Requirements/m.test(content)) { + // Case-insensitive to match the delta parser, so a lowercase header + // routes through the same delta validation that validate runs. + if (/^##\s+(ADDED|MODIFIED|REMOVED|RENAMED)\s+Requirements/im.test(content)) { hasDeltaSpecs = true; break; } @@ -424,6 +428,7 @@ export class ArchiveCommand { // Handle spec updates unless skipSpecs flag is set let specsUpdated = false; let totals: ArchiveResult['totals']; + const specWarnings: string[] = []; if (options.skipSpecs) { if (!json) { console.log('Skipping spec updates (--skip-specs flag provided).'); @@ -468,6 +473,9 @@ export class ArchiveCommand { for (const update of specUpdates) { const built = await buildUpdatedSpec(update, changeName!, { silent: json }); prepared.push({ update, rebuilt: built.rebuilt, counts: built.counts }); + // Carried into the result so JSON mode (where nothing was + // printed) still surfaces them; human mode discards the result. + specWarnings.push(...built.warnings); } } catch (err: any) { if (json) { @@ -511,24 +519,36 @@ export class ArchiveCommand { // All validations passed; write files and display counts const writeTotals = { added: 0, modified: 0, removed: 0, renamed: 0 }; + let wroteAny = false; for (const p of prepared) { + const { added, modified, removed, renamed } = p.counts; + if (added + modified + removed + renamed === 0) { + // Every operation was already synced: rewriting the file would + // only churn normalization differences into it. + continue; + } await writeUpdatedSpec(p.update, p.rebuilt, p.counts, { silent: json, // Cross-root paths must be absolute when a store is selected. ...(isStoreSelectedRoot(root) ? { displayPath: p.update.target } : {}), }); - writeTotals.added += p.counts.added; - writeTotals.modified += p.counts.modified; - writeTotals.removed += p.counts.removed; - writeTotals.renamed += p.counts.renamed; + wroteAny = true; + writeTotals.added += added; + writeTotals.modified += modified; + writeTotals.removed += removed; + writeTotals.renamed += renamed; } - specsUpdated = true; + specsUpdated = wroteAny; totals = writeTotals; if (!json) { console.log( `Totals: + ${writeTotals.added}, ~ ${writeTotals.modified}, - ${writeTotals.removed}, → ${writeTotals.renamed}` ); - console.log('Specs updated successfully.'); + console.log( + wroteAny + ? 'Specs updated successfully.' + : 'Specs already in sync; no files changed.' + ); } } } @@ -573,6 +593,7 @@ export class ArchiveCommand { path: archivePath, specsUpdated, ...(totals ? { totals } : {}), + ...(specWarnings.length > 0 ? { warnings: specWarnings } : {}), }; } diff --git a/src/core/command-generation/adapters/qwen.ts b/src/core/command-generation/adapters/qwen.ts index 9d31a07719..a22726ad57 100644 --- a/src/core/command-generation/adapters/qwen.ts +++ b/src/core/command-generation/adapters/qwen.ts @@ -10,6 +10,7 @@ import path from 'path'; import type { CommandContent, ToolCommandAdapter } from '../types.js'; +import { transformToHyphenCommands } from '../../../utils/command-references.js'; /** * Escapes a string value for safe YAML output. @@ -39,11 +40,15 @@ export const qwenAdapter: ToolCommandAdapter = { }, formatFile(content: CommandContent): string { + // Qwen commands are invoked by filename (/opsx-), so cross-references + // must use the hyphen form too. + const transformedBody = transformToHyphenCommands(content.body); + return `--- description: ${escapeYamlValue(content.description)} --- -${content.body} +${transformedBody} `; }, }; diff --git a/src/core/init.ts b/src/core/init.ts index 48774602d7..faf83f658c 100644 --- a/src/core/init.ts +++ b/src/core/init.ts @@ -897,7 +897,14 @@ export class InitCommand { for (const tool of successfulTools) { let hint: string; if (shouldGenerateCommandsForTool(tool.value, activeDelivery)) { - hint = `Start your first change: ${command} "your idea"`; + // Tools that invoke commands by filename (bob, qwen, ...) need the + // hyphen form here too, not just inside generated bodies. + const transformer = getTransformerForTool( + tool.value, + activeDelivery, + resolveCommandSurfaceCapability(tool.value) + ); + hint = `Start your first change: ${transformer ? transformer(command) : command} "your idea"`; } else if (shouldGenerateSkillsForTool(tool.value, activeDelivery)) { hint = resolveCommandSurfaceCapability(tool.value) === 'skills-invocable' diff --git a/src/core/parsers/requirement-blocks.ts b/src/core/parsers/requirement-blocks.ts index 6bc4e15109..b47b9ecd45 100644 --- a/src/core/parsers/requirement-blocks.ts +++ b/src/core/parsers/requirement-blocks.ts @@ -18,6 +18,17 @@ export function normalizeRequirementName(name: string): string { return name.trim(); } +/** + * Case- and whitespace-insensitive fold of a requirement name. Requirement + * matching itself is case-sensitive (normalizeRequirementName); this fold + * exists only for typo detection - near-miss REMOVED headers and the + * RENAMED+REMOVED cross-section conflict - where two spellings that differ + * only in case or interior whitespace mean a mistake, never two requirements. + */ +export function foldRequirementName(name: string): string { + return normalizeRequirementName(name).toLowerCase().replace(/\s+/g, ' '); +} + /** The canonical requirement header the delta reader recognizes. */ const REQUIREMENT_HEADER_REGEX = /^###\s*Requirement:\s*(.+)\s*$/i; diff --git a/src/core/parsers/requirement-text.ts b/src/core/parsers/requirement-text.ts index 8aa0e89567..9841e3ddcf 100644 --- a/src/core/parsers/requirement-text.ts +++ b/src/core/parsers/requirement-text.ts @@ -9,60 +9,10 @@ * `validate `, `validate `, and `archive`. */ -/** - * Build a per-line mask marking lines that fall inside a fenced code block - * (``` ``` ``` or ``` ~~~ ```), including the fence lines themselves. Mirrors the - * fence rules markdown uses: a fence opens on the first ```` ```/~~~ ```` of - * length >= 3 and closes on a line of the same marker whose length is >= the - * opening length, with nothing but whitespace after it. - */ -export function buildCodeFenceMask(lines: string[]): boolean[] { - const mask = new Array(lines.length).fill(false); - let activeFence: { marker: '`' | '~'; length: number } | null = null; - - for (let i = 0; i < lines.length; i++) { - const fence = getFenceMarker(lines[i]); - - if (!activeFence) { - if (fence) { - activeFence = fence; - mask[i] = true; - } - continue; - } - - mask[i] = true; - if (isClosingFence(lines[i], activeFence)) { - activeFence = null; - } - } - - return mask; -} - -function getFenceMarker(line: string): { marker: '`' | '~'; length: number } | null { - const fenceMatch = line.match(/^\s*(`{3,}|~{3,})/); - if (!fenceMatch) { - return null; - } - - return { - marker: fenceMatch[1][0] as '`' | '~', - length: fenceMatch[1].length, - }; -} - -function isClosingFence( - line: string, - activeFence: { marker: '`' | '~'; length: number } -): boolean { - const fenceMatch = line.match(/^\s*(`{3,}|~{3,})\s*$/); - return Boolean( - fenceMatch && - fenceMatch[1][0] === activeFence.marker && - fenceMatch[1].length >= activeFence.length - ); -} +// Re-exported so existing importers keep working; the single implementation +// lives in code-fence.ts. +export { buildCodeFenceMask } from './code-fence.js'; +import { buildCodeFenceMask } from './code-fence.js'; /** Lines that look like `**ID**: ...` / `**Priority**: ...` metadata. */ const METADATA_LINE = /^\*\*[^*]+\*\*:/; diff --git a/src/core/specs-apply.ts b/src/core/specs-apply.ts index 563769b63a..db80ae8bf5 100644 --- a/src/core/specs-apply.ts +++ b/src/core/specs-apply.ts @@ -10,6 +10,7 @@ import path from 'path'; import chalk from 'chalk'; import { extractRequirementsSection, + foldRequirementName, parseDeltaSpec, normalizeRequirementName, type RequirementBlock, @@ -17,7 +18,6 @@ import { import { findMainSpecStructureIssues } from './parsers/spec-structure.js'; import { buildCodeFenceMask } from './parsers/code-fence.js'; import { MarkdownParser } from './parsers/markdown-parser.js'; -import { Validator } from './validation/validator.js'; import { MIN_PURPOSE_LENGTH } from './validation/constants.js'; import { discoverSpecFiles } from '../utils/spec-discovery.js'; @@ -33,26 +33,6 @@ export interface SpecUpdate { exists: boolean; } -export interface ApplyResult { - capability: string; - added: number; - modified: number; - removed: number; - renamed: number; -} - -export interface SpecsApplyOutput { - changeName: string; - capabilities: ApplyResult[]; - totals: { - added: number; - modified: number; - removed: number; - renamed: number; - }; - noChanges: boolean; -} - interface ScenarioBlock { name: string; raw: string; @@ -105,7 +85,20 @@ export async function buildUpdatedSpec( update: SpecUpdate, changeName: string, options: { silent?: boolean } = {} -): Promise<{ rebuilt: string; counts: { added: number; modified: number; removed: number; renamed: number } }> { +): Promise<{ + rebuilt: string; + counts: { added: number; modified: number; removed: number; renamed: number }; + warnings: string[]; +}> { + // Collected so silent (JSON) callers can surface them; printed live for + // human callers at the point they occur. + const warnings: string[] = []; + const warn = (message: string): void => { + warnings.push(message); + if (!options.silent) { + console.log(chalk.yellow(`⚠️ Warning: ${message}`)); + } + }; // Read change spec content (delta-format expected) const changeContent = await fs.readFile(update.source, 'utf-8'); @@ -176,6 +169,21 @@ export async function buildUpdatedSpec( for (const { from, to } of plan.renamed) { const fromNorm = normalizeRequirementName(from); const toNorm = normalizeRequirementName(to); + // A REMOVED naming the FROM side contradicts the rename. This used to + // fail incidentally at apply time (the rename consumed the old header, + // so REMOVED hit "not found"); now that a missing REMOVED target is a + // no-op, the conflict must be rejected explicitly. Compared folded, so + // a case/whitespace variant cannot slip past the guard and degrade + // into a warned no-op. + const removedFoldMatch = [...removedNamesSet].find( + (r) => foldRequirementName(r) === foldRequirementName(fromNorm) + ); + if (removedFoldMatch !== undefined) { + throw new Error( + `${specName} validation failed - requirement present in multiple sections (RENAMED and REMOVED) for header "### Requirement: ${from}"` + + (removedFoldMatch === fromNorm ? '' : ` (REMOVED spells it "${removedFoldMatch}")`) + ); + } if (modifiedNames.has(fromNorm)) { throw new Error( `${specName} validation failed - when a rename exists, MODIFIED must reference the NEW header "### Requirement: ${to}"` @@ -214,14 +222,12 @@ export async function buildUpdatedSpec( // Only when the spec really does have a different Purpose: claiming it // "already has one" would be false when it has none, and saying anything at // all is noise when the two bodies match. - if (deltaPurpose && !options.silent) { + if (deltaPurpose) { const existingPurpose = extractPurposeSection(targetContent); if (existingPurpose && existingPurpose !== deltaPurpose) { - console.log( - chalk.yellow( - `⚠️ Warning: ${specName} - delta Purpose ignored; ${specName} already has one. ` + - `Edit ${update.target} directly to change it.` - ) + warn( + `${specName} - delta Purpose ignored; ${specName} already has one. ` + + `Edit ${update.target} directly to change it.` ); } } @@ -234,11 +240,9 @@ export async function buildUpdatedSpec( ); } // Warn about REMOVED requirements being ignored for new specs - if (plan.removed.length > 0 && !options.silent) { - console.log( - chalk.yellow( - `⚠️ Warning: ${specName} - ${plan.removed.length} REMOVED requirement(s) ignored for new spec (nothing to remove).` - ) + if (plan.removed.length > 0) { + warn( + `${specName} - ${plan.removed.length} REMOVED requirement(s) ignored for new spec (nothing to remove).` ); } isNewSpec = true; @@ -248,22 +252,16 @@ export async function buildUpdatedSpec( // Keep the placeholder rather than turning this into a failure: these // deltas archived cleanly before the Purpose carry-over existed. targetContent = buildSpecSkeleton(specName, changeName); - if (!options.silent) { - console.log( - chalk.yellow( - `⚠️ Warning: ${specName} - delta Purpose ignored (it would leave the new spec unreadable); wrote the placeholder Purpose instead.` - ) - ); - } - } else if (overview && overview.length < MIN_PURPOSE_LENGTH && !options.silent) { + warn( + `${specName} - delta Purpose ignored (it would leave the new spec unreadable); wrote the placeholder Purpose instead.` + ); + } else if (overview && overview.length < MIN_PURPOSE_LENGTH) { // The placeholder always cleared this threshold, so a carried Purpose is // the first way archive can leave a spec that `validate --strict` fails. // Measured on the parsed overview, which is what the validator reads. - console.log( - chalk.yellow( - `⚠️ Warning: ${specName} - carried Purpose is under ${MIN_PURPOSE_LENGTH} characters; ` + - `openspec validate --strict reports it as too brief.` - ) + warn( + `${specName} - carried Purpose is under ${MIN_PURPOSE_LENGTH} characters; ` + + `openspec validate --strict reports it as too brief.` ); } } @@ -318,18 +316,31 @@ export async function buildUpdatedSpec( } // REMOVED + let removedApplied = 0; for (const name of plan.removed) { const key = normalizeRequirementName(name); if (!nameToBlock.has(key)) { - // For new specs, REMOVED requirements are already warned about and ignored - // For existing specs, missing requirements are an error + // Requirement gone from the baseline means the removal was already + // synced (early-sync pattern) — re-applying it is a no-op, not a + // failure. One signal does separate that from a mistyped header: a + // requirement that differs only in case or interior whitespace still + // being present. That is a typo, and stays a hard abort. + // For new specs the skip was already warned about above. if (!isNewSpec) { - throw new Error(`${specName} REMOVED failed for header "### Requirement: ${name}" - not found`); + const nearMiss = [...nameToBlock.keys()].find((k) => foldRequirementName(k) === foldRequirementName(key)); + if (nearMiss !== undefined) { + throw new Error( + `${specName} REMOVED failed for header "### Requirement: ${name}" - not found, but "### Requirement: ${nameToBlock.get(nearMiss)!.name}" exists; fix the header to match it exactly` + ); + } + warn( + `${specName} - REMOVED requirement "${name}" is not in the current spec; treating it as already removed.` + ); } - // Skip removal for new specs (already warned above) continue; } nameToBlock.delete(key); + removedApplied++; } // MODIFIED @@ -409,9 +420,10 @@ export async function buildUpdatedSpec( counts: { added: addedApplied, modified: plan.modified.length, - removed: plan.removed.length, + removed: removedApplied, renamed: renamedApplied, }, + warnings, }; } @@ -593,119 +605,3 @@ function parseScenarioBlocks(requirementRaw: string): ScenarioBlock[] { return scenarios; } -/** - * Apply all delta specs from a change to main specs. - * - * @param projectRoot - The project root directory - * @param changeName - The name of the change to apply - * @param options - Options for the operation - * @returns Result of the operation with counts - */ -export async function applySpecs( - projectRoot: string, - changeName: string, - options: { - dryRun?: boolean; - skipValidation?: boolean; - silent?: boolean; - } = {} -): Promise { - const changeDir = path.join(projectRoot, 'openspec', 'changes', changeName); - const mainSpecsDir = path.join(projectRoot, 'openspec', 'specs'); - - // Verify change exists - try { - const stat = await fs.stat(changeDir); - if (!stat.isDirectory()) { - throw new Error(`Change '${changeName}' not found.`); - } - } catch { - throw new Error(`Change '${changeName}' not found.`); - } - - // Find specs to update - const specUpdates = await findSpecUpdates(changeDir, mainSpecsDir); - - if (specUpdates.length === 0) { - return { - changeName, - capabilities: [], - totals: { added: 0, modified: 0, removed: 0, renamed: 0 }, - noChanges: true, - }; - } - - // Prepare all updates first (validation pass, no writes) - const prepared: Array<{ - update: SpecUpdate; - rebuilt: string; - counts: { added: number; modified: number; removed: number; renamed: number }; - }> = []; - - for (const update of specUpdates) { - const built = await buildUpdatedSpec(update, changeName); - prepared.push({ update, rebuilt: built.rebuilt, counts: built.counts }); - } - - // Validate rebuilt specs unless validation is skipped - if (!options.skipValidation) { - const validator = new Validator(); - for (const p of prepared) { - const specName = p.update.id; - const report = await validator.validateSpecContent(specName, p.rebuilt); - if (!report.valid) { - const errors = report.issues - .filter((i) => i.level === 'ERROR') - .map((i) => ` ✗ ${i.message}`) - .join('\n'); - throw new Error(`Validation errors in rebuilt spec for ${specName}:\n${errors}`); - } - } - } - - // Build results - const capabilities: ApplyResult[] = []; - const totals = { added: 0, modified: 0, removed: 0, renamed: 0 }; - - for (const p of prepared) { - const capability = p.update.id; - - if (!options.dryRun) { - // Write the updated spec - const targetDir = path.dirname(p.update.target); - await fs.mkdir(targetDir, { recursive: true }); - await fs.writeFile(p.update.target, p.rebuilt); - - if (!options.silent) { - console.log(`Applying changes to openspec/specs/${capability}/spec.md:`); - if (p.counts.added) console.log(` + ${p.counts.added} added`); - if (p.counts.modified) console.log(` ~ ${p.counts.modified} modified`); - if (p.counts.removed) console.log(` - ${p.counts.removed} removed`); - if (p.counts.renamed) console.log(` → ${p.counts.renamed} renamed`); - } - } else if (!options.silent) { - console.log(`Would apply changes to openspec/specs/${capability}/spec.md:`); - if (p.counts.added) console.log(` + ${p.counts.added} added`); - if (p.counts.modified) console.log(` ~ ${p.counts.modified} modified`); - if (p.counts.removed) console.log(` - ${p.counts.removed} removed`); - if (p.counts.renamed) console.log(` → ${p.counts.renamed} renamed`); - } - - capabilities.push({ - capability, - ...p.counts, - }); - - totals.added += p.counts.added; - totals.modified += p.counts.modified; - totals.removed += p.counts.removed; - totals.renamed += p.counts.renamed; - } - - return { - changeName, - capabilities, - totals, - noChanges: false, - }; -} diff --git a/src/core/templates/workflows/apply-change.ts b/src/core/templates/workflows/apply-change.ts index a08b24ddd0..e7c5b6c6a4 100644 --- a/src/core/templates/workflows/apply-change.ts +++ b/src/core/templates/workflows/apply-change.ts @@ -50,7 +50,7 @@ ${STORE_SELECTION_GUIDANCE} - Dynamic instruction based on current state **Handle states:** - - If \`state: "blocked"\` (missing artifacts): show message, suggest using openspec-continue-change + - If \`state: "blocked"\` (missing artifacts): show message, suggest using openspec-continue-change (if it is not installed, run \`openspec status --change "" --json\` to see the next artifact and \`openspec instructions --change "" --json\` for how to create it) - If \`state: "all_done"\`: congratulate, suggest archive - Otherwise: proceed to implementation @@ -210,7 +210,7 @@ ${STORE_SELECTION_GUIDANCE} - Dynamic instruction based on current state **Handle states:** - - If \`state: "blocked"\` (missing artifacts): show message, suggest using \`/opsx:continue\` + - If \`state: "blocked"\` (missing artifacts): show message, suggest using \`/opsx:continue\` (if it is not installed, run \`openspec status --change "" --json\` to see the next artifact and \`openspec instructions --change "" --json\` for how to create it) - If \`state: "all_done"\`: congratulate, suggest archive - Otherwise: proceed to implementation diff --git a/src/core/templates/workflows/onboard.ts b/src/core/templates/workflows/onboard.ts index d175b08322..82a54395c0 100644 --- a/src/core/templates/workflows/onboard.ts +++ b/src/core/templates/workflows/onboard.ts @@ -485,7 +485,7 @@ This same rhythm works for any size change—a small fix or a major feature. | \`/opsx:apply\` | Implement tasks from a change | | \`/opsx:archive\` | Archive a completed change | -**Additional commands:** +**Additional commands** (only if installed - availability depends on your profile): | Command | What it does | |--------------------|----------------------------------------------------------| @@ -513,7 +513,7 @@ If the user says they need to stop, want to pause, or seem disengaged: No problem! Your change is saved at the \`changeRoot\` reported by \`openspec status --change "" --json\`. To pick up where we left off later: -- \`/opsx:continue \` - Resume artifact creation +- \`/opsx:continue \` - Resume artifact creation (if installed; otherwise \`openspec status --change "" --json\` shows the next artifact) - \`/opsx:apply \` - Jump to implementation (if tasks exist) The work won't be lost. Come back whenever you're ready. @@ -537,7 +537,7 @@ If the user says they just want to see the commands or skip the tutorial: | \`/opsx:apply \` | Implement tasks | | \`/opsx:archive \` | Archive when done | -**Additional commands:** +**Additional commands** (only if installed - availability depends on your profile): | Command | What it does | |---------------------------|-------------------------------------| diff --git a/src/core/templates/workflows/update-change.ts b/src/core/templates/workflows/update-change.ts index a5780c4872..551633deb7 100644 --- a/src/core/templates/workflows/update-change.ts +++ b/src/core/templates/workflows/update-change.ts @@ -84,7 +84,8 @@ After each invocation, show: - Edit only the concrete files in \`existingOutputPaths\`; never write to a glob \`resolvedOutputPath\`. - Do not advance the build frontier: no new artifacts, no new files under glob artifacts - that is \`/opsx:continue\`'s job. - Confirm every edit with the user before writing. -- If the request changes the change's *intent* rather than refining it, recommend starting fresh with \`/opsx:new\` (the "Update vs. Start Fresh" heuristic).`, +- If the request changes the change's *intent* rather than refining it, recommend starting fresh with \`/opsx:new\` (the "Update vs. Start Fresh" heuristic). +- \`/opsx:continue\` and \`/opsx:new\` may not be installed (core profile). When suggesting one that is unavailable, point to the CLI instead: \`openspec status --change "" --json\` shows the next artifact and \`openspec instructions --change "" --json\` explains how to create it.`, license: 'MIT', compatibility: 'Requires openspec CLI.', metadata: { author: 'openspec', version: '1.0' }, @@ -170,6 +171,7 @@ After each invocation, show: - Edit only the concrete files in \`existingOutputPaths\`; never write to a glob \`resolvedOutputPath\`. - Do not advance the build frontier: no new artifacts, no new files under glob artifacts - that is \`/opsx:continue\`'s job. - Confirm every edit with the user before writing. -- If the request changes the change's *intent* rather than refining it, recommend starting fresh with \`/opsx:new\` (the "Update vs. Start Fresh" heuristic).` +- If the request changes the change's *intent* rather than refining it, recommend starting fresh with \`/opsx:new\` (the "Update vs. Start Fresh" heuristic). +- \`/opsx:continue\` and \`/opsx:new\` may not be installed (core profile). When suggesting one that is unavailable, point to the CLI instead: \`openspec status --change "" --json\` shows the next artifact and \`openspec instructions --change "" --json\` explains how to create it.` }; } diff --git a/src/core/validation/validator.ts b/src/core/validation/validator.ts index 4b59ed6cb1..25989f86e2 100644 --- a/src/core/validation/validator.ts +++ b/src/core/validation/validator.ts @@ -10,7 +10,7 @@ import { MAX_REQUIREMENT_TEXT_LENGTH, VALIDATION_MESSAGES } from './constants.js'; -import { parseDeltaSpec, normalizeRequirementName, extractRequirementsSection } from '../parsers/requirement-blocks.js'; +import { parseDeltaSpec, foldRequirementName, normalizeRequirementName, extractRequirementsSection } from '../parsers/requirement-blocks.js'; import { extractRequirementBody as extractRequirementBodyShared, containsShallOrMust as containsShallOrMustShared, @@ -318,6 +318,20 @@ export class Validator { if (addedNames.has(toKey)) { issues.push({ level: 'ERROR', path: entryPath, message: `RENAMED TO collides with ADDED for "${to}"` }); } + // Folded comparison: a case/whitespace variant of the FROM header + // in REMOVED is the same contradiction, not a different name. + const removedFoldMatch = [...removedNames].find( + (r) => foldRequirementName(r) === foldRequirementName(fromKey) + ); + if (removedFoldMatch !== undefined) { + issues.push({ + level: 'ERROR', + path: entryPath, + message: + `Requirement present in both RENAMED and REMOVED: "${from}"` + + (removedFoldMatch === fromKey ? '' : ` (REMOVED spells it "${removedFoldMatch}")`), + }); + } } } } catch { diff --git a/src/utils/command-references.ts b/src/utils/command-references.ts index b3cadf766a..987f7f8634 100644 --- a/src/utils/command-references.ts +++ b/src/utils/command-references.ts @@ -104,8 +104,8 @@ export function getSkillReferenceTransformer(toolId: string): (text: string) => * because the tool has no command surface at all (capability 'none', e.g. * Kimi Code or Mistral Vibe) — so those skills never point at commands * that were not generated. When commands are generated, tools where the - * command filename doubles as the command name (oh-my-pi, opencode, pi) use - * hyphen-based command references. All other cases keep the default + * command filename doubles as the command name (bob, oh-my-pi, opencode, + * pi, qwen) use hyphen-based command references. All other cases keep the default * `/opsx:*` references; notably skills-invocable tools (codex) are * deliberately left untouched here to keep codex output stable while its * reference rewriting is reworked separately. @@ -123,7 +123,13 @@ export function getTransformerForTool( if (delivery === 'skills' || capability === 'none') { return getSkillReferenceTransformer(toolId); } - if (toolId === 'opencode' || toolId === 'pi' || toolId === 'oh-my-pi') { + if ( + toolId === 'bob' || + toolId === 'oh-my-pi' || + toolId === 'opencode' || + toolId === 'pi' || + toolId === 'qwen' + ) { return transformToHyphenCommands; } return undefined; diff --git a/src/utils/spec-discovery.ts b/src/utils/spec-discovery.ts index 7282498041..509f259143 100644 --- a/src/utils/spec-discovery.ts +++ b/src/utils/spec-discovery.ts @@ -13,8 +13,11 @@ export interface DiscoveredSpec { * `specs//spec.md` layout and nested `specs///spec.md` layouts * are found (#1353). A `spec.md` sitting directly in the root is ignored, * matching the historical requirement that specs live in a capability folder. - * Dot-directories are skipped and symlinks are not followed. Results are - * sorted by id for deterministic output. + * Dot-directories are skipped and symlinked directories are not followed. + * A symlinked `spec.md` IS resolved: `hasAnyFileUnder` and the artifact + * graph's globs both count it as content, so dropping it here would silently + * lose the delta on archive; a dangling link is skipped. Results are sorted + * by id for deterministic output. * * A missing root (ENOENT) yields an empty list, but any other read failure * (EACCES, EIO, ...) is thrown rather than swallowed: since this feeds the @@ -35,8 +38,20 @@ export async function discoverSpecFiles(specsRoot: string): Promise 0) { - results.push({ id: segments.join('/'), specFile: path.join(dir, entry.name) }); + } else if (entry.name === 'spec.md' && segments.length > 0) { + if (entry.isFile()) { + results.push({ id: segments.join('/'), specFile: path.join(dir, entry.name) }); + } else if (entry.isSymbolicLink()) { + const specFile = path.join(dir, entry.name); + try { + if ((await fs.stat(specFile)).isFile()) { + results.push({ id: segments.join('/'), specFile }); + } + } catch (err: any) { + // A dangling link is not content; anything else fails loudly. + if (err?.code !== 'ENOENT') throw err; + } + } } } }; diff --git a/test/commands/show.test.ts b/test/commands/show.test.ts index a606b1fe53..19ec6b2820 100644 --- a/test/commands/show.test.ts +++ b/test/commands/show.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { promises as fs } from 'fs'; import path from 'path'; -import { execFileSync } from 'child_process'; +import { execFileSync, spawnSync } from 'child_process'; describe('top-level show command', () => { const projectRoot = process.cwd(); @@ -64,6 +64,27 @@ describe('top-level show command', () => { } }); + it('does not warn about spec-only flags that were never passed', () => { + // commander defaults `scenarios` to true for --no-scenarios, so a plain + // `show ` must not warn about a flag the user never typed. + const res = spawnSync('node', [openspecBin, 'show', 'demo', '--json'], { + encoding: 'utf-8', + cwd: testDir, + }); + expect(res.status).toBe(0); + expect(res.stderr).not.toContain('not applicable'); + }); + + it('still warns when --no-scenarios is explicitly passed for a change', () => { + const res = spawnSync( + 'node', + [openspecBin, 'show', 'demo', '--json', '--no-scenarios'], + { encoding: 'utf-8', cwd: testDir } + ); + expect(res.status).toBe(0); + expect(res.stderr).toContain('Ignoring flags not applicable to change: scenarios'); + }); + it('auto-detects spec id and supports spec-only flags', () => { const originalCwd = process.cwd(); try { diff --git a/test/core/archive.test.ts b/test/core/archive.test.ts index 0a2b3f4ff3..fd5a5d3132 100644 --- a/test/core/archive.test.ts +++ b/test/core/archive.test.ts @@ -392,6 +392,167 @@ Then expected result happens`; expect(untouched).toBe(mainSpecContent); }); + it('should abort when REMOVED names the FROM side of a RENAMED in the same delta', async () => { + // Contradictory delta: you cannot both rename and remove the same + // requirement. This used to fail incidentally at apply time (the rename + // consumed the old header, so REMOVED hit "not found"); now that a + // missing REMOVED target is treated as already synced, the conflict has + // to be rejected explicitly. + const changeName = 'rename-and-remove'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const changeSpecDir = path.join(changeDir, 'specs', 'core-layer'); + await fs.mkdir(changeSpecDir, { recursive: true }); + + await fs.writeFile( + path.join(changeSpecDir, 'spec.md'), + `# Core Layer - Changes\n\n## RENAMED Requirements\n\n- FROM: \`### Requirement: Old name\`\n- TO: \`### Requirement: New name\`\n\n## REMOVED Requirements\n\n### Requirement: Old name\n` + ); + + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'core-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const mainSpecContent = `# core-layer Specification\n\n## Purpose\nCore abstraction layer.\n\n## Requirements\n\n### Requirement: Old name\n\n#### Scenario: Works\n- **WHEN** it runs\n- **THEN** it works\n`; + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), mainSpecContent); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('requirement present in multiple sections (RENAMED and REMOVED) for header "### Requirement: Old name"') + ); + expect(process.exitCode).toBe(1); + const untouched = await fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8'); + expect(untouched).toBe(mainSpecContent); + }); + + it('should abort when REMOVED spells the renamed FROM header with different case', async () => { + const changeName = 'rename-and-remove-case'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const changeSpecDir = path.join(changeDir, 'specs', 'core-layer'); + await fs.mkdir(changeSpecDir, { recursive: true }); + + await fs.writeFile( + path.join(changeSpecDir, 'spec.md'), + `# Core Layer - Changes\n\n## RENAMED Requirements\n\n- FROM: \`### Requirement: Old Name\`\n- TO: \`### Requirement: New Name\`\n\n## REMOVED Requirements\n\n### Requirement: old name\n` + ); + + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'core-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const mainSpecContent = `# core-layer Specification\n\n## Purpose\nCore abstraction layer.\n\n## Requirements\n\n### Requirement: Old Name\n\n#### Scenario: Works\n- **WHEN** it runs\n- **THEN** it works\n`; + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), mainSpecContent); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('requirement present in multiple sections (RENAMED and REMOVED) for header "### Requirement: Old Name" (REMOVED spells it "old name")') + ); + expect(process.exitCode).toBe(1); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + const untouched = await fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8'); + expect(untouched).toBe(mainSpecContent); + }); + + it('should archive when REMOVED requirements were already synced to the baseline', async () => { + const changeName = 'early-synced-removal'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const changeSpecDir = path.join(changeDir, 'specs', 'core-layer'); + await fs.mkdir(changeSpecDir, { recursive: true }); + + await fs.writeFile( + path.join(changeSpecDir, 'spec.md'), + `# Core Layer - Changes\n\n## REMOVED Requirements\n\n### Requirement: The system SHALL provide a legacy layer\n**Reason**: Replaced by the core abstraction layer.\n` + ); + + // Early-sync pattern: the requirement was already removed from the main spec. + const keptBlock = `### Requirement: The system SHALL provide a core abstraction layer\n\n#### Scenario: Layer is available\n- **WHEN** a consumer imports the layer\n- **THEN** the abstraction is available`; + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'core-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const mainSpecContent = `# core-layer Specification\n\n## Purpose\nCore abstraction layer.\n\n## Requirements\n\n${keptBlock}\n`; + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), mainSpecContent); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + // Archive succeeds with a warning instead of aborting + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('REMOVED requirement "The system SHALL provide a legacy layer" is not in the current spec') + ); + // The skipped removal is not reported as applied + expect(console.log).not.toHaveBeenCalledWith(expect.stringContaining('- 1 removed')); + // A no-op update must not churn the file with normalization differences + const updatedContent = await fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8'); + expect(updatedContent).toBe(mainSpecContent); + // ...and must not claim an update happened + expect(console.log).toHaveBeenCalledWith('Specs already in sync; no files changed.'); + expect(console.log).not.toHaveBeenCalledWith('Specs updated successfully.'); + + const archives = await fs.readdir(path.join(tempDir, 'openspec', 'changes', 'archive')); + expect(archives.some(a => a.includes(changeName))).toBe(true); + expect(process.exitCode).toBeUndefined(); + }); + + it('should abort when a REMOVED header near-misses an existing requirement (case/whitespace typo)', async () => { + // A fold-insensitive match in the current spec means the header is a + // typo, not an early-synced removal - that case must stay a hard abort. + const changeName = 'typo-removal'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const changeSpecDir = path.join(changeDir, 'specs', 'core-layer'); + await fs.mkdir(changeSpecDir, { recursive: true }); + + await fs.writeFile( + path.join(changeSpecDir, 'spec.md'), + `# Core Layer - Changes\n\n## REMOVED Requirements\n\n### Requirement: legacy layer\n**Reason**: Replaced.\n` + ); + + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'core-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + const mainSpecContent = `# core-layer Specification\n\n## Purpose\nCore abstraction layer.\n\n## Requirements\n\n### Requirement: Legacy Layer\n\n#### Scenario: Works\n- **WHEN** it runs\n- **THEN** it works\n`; + await fs.writeFile(path.join(mainSpecDir, 'spec.md'), mainSpecContent); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true }); + + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('REMOVED failed for header "### Requirement: legacy layer" - not found, but "### Requirement: Legacy Layer" exists') + ); + expect(process.exitCode).toBe(1); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + const untouched = await fs.readFile(path.join(mainSpecDir, 'spec.md'), 'utf-8'); + expect(untouched).toBe(mainSpecContent); + }); + + it('should surface the skipped REMOVED as a warning in --json output', async () => { + const changeName = 'early-synced-removal-json'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const changeSpecDir = path.join(changeDir, 'specs', 'core-layer'); + await fs.mkdir(changeSpecDir, { recursive: true }); + + await fs.writeFile( + path.join(changeSpecDir, 'spec.md'), + `# Core Layer - Changes\n\n## REMOVED Requirements\n\n### Requirement: The system SHALL provide a legacy layer\n**Reason**: Replaced.\n` + ); + + const keptBlock = `### Requirement: The system SHALL provide a core abstraction layer\n\n#### Scenario: Layer is available\n- **WHEN** a consumer imports the layer\n- **THEN** the abstraction is available`; + const mainSpecDir = path.join(tempDir, 'openspec', 'specs', 'core-layer'); + await fs.mkdir(mainSpecDir, { recursive: true }); + await fs.writeFile( + path.join(mainSpecDir, 'spec.md'), + `# core-layer Specification\n\n## Purpose\nCore abstraction layer.\n\n## Requirements\n\n${keptBlock}\n` + ); + + await archiveCommand.execute(changeName, { yes: true, noValidate: true, json: true }); + + expect(process.exitCode).toBeUndefined(); + const logCalls = (console.log as unknown as { mock: { calls: unknown[][] } }).mock.calls.flat().map(String); + const jsonLine = logCalls.find((entry) => entry.trimStart().startsWith('{')); + expect(jsonLine).toBeDefined(); + const parsed = JSON.parse(jsonLine!); + expect(parsed.archive.totals.removed).toBe(0); + // No file was written, so the result must not claim an update + expect(parsed.archive.specsUpdated).toBe(false); + // The silent path must not swallow the skip: agents reading JSON get + // the same signal humans get on stdout. + expect(parsed.archive.warnings).toEqual([ + expect.stringContaining('REMOVED requirement "The system SHALL provide a legacy layer" is not in the current spec'), + ]); + }); + it('should merge nested delta specs into the same relative path (#1353)', async () => { const changeName = 'nested-spec-feature'; const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); @@ -472,6 +633,9 @@ The system SHALL support logo and backgroundColor fields for gift cards. expect(console.log).toHaveBeenCalledWith( expect.stringContaining('Warning: gift-card - 2 REMOVED requirement(s) ignored for new spec (nothing to remove).') ); + + // The ignored removals are not reported as applied + expect(console.log).not.toHaveBeenCalledWith(expect.stringContaining('- 2 removed')); // Verify spec was created with only ADDED requirements const mainSpecPath = path.join(tempDir, 'openspec', 'specs', 'gift-card', 'spec.md'); @@ -1679,7 +1843,7 @@ content D`; expect(updated).not.toContain('### Requirement: B'); }); - it('should abort with error when MODIFIED/REMOVED reference non-existent requirements', async () => { + it('should abort with error when MODIFIED references non-existent requirements', async () => { const changeName = 'validate-missing'; const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); const changeSpecDir = path.join(changeDir, 'specs', 'gamma'); @@ -1696,15 +1860,12 @@ Gamma purpose. ## Requirements`; await fs.writeFile(path.join(mainSpecDir, 'spec.md'), mainContent); - // Delta tries to modify and remove non-existent requirement + // Delta tries to modify a non-existent requirement const deltaContent = `# Gamma - Changes ## MODIFIED Requirements ### Requirement: Missing -new text - -## REMOVED Requirements -### Requirement: Another Missing`; +new text`; await fs.writeFile(path.join(changeSpecDir, 'spec.md'), deltaContent); await archiveCommand.execute(changeName, { yes: true, noValidate: true }); @@ -2016,7 +2177,7 @@ Zeta purpose. ### Requirement: Z1 z1`); - // Delta: epsilon is valid modification; zeta tries to remove non-existent -> should abort both + // Delta: epsilon is valid modification; zeta tries to modify non-existent -> should abort both await fs.writeFile(path.join(spec1Dir, 'spec.md'), `# Epsilon - Changes ## MODIFIED Requirements @@ -2025,8 +2186,9 @@ E1 updated`); await fs.writeFile(path.join(spec2Dir, 'spec.md'), `# Zeta - Changes -## REMOVED Requirements -### Requirement: Missing`); +## MODIFIED Requirements +### Requirement: Missing +missing body`); await archiveCommand.execute(changeName, { yes: true, noValidate: true }); @@ -2073,6 +2235,33 @@ E1 updated`); // Regression for the silent-exit-0 bug: when archive is blocked in // human mode it must set a non-zero exit code so scripts/CI can detect // the failure, mirroring the JSON-mode behavior. + it('runs delta spec validation for lowercase delta headers (parity with validate)', async () => { + const changeName = 'exit-lowercase-delta'; + const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); + const changeSpecDir = path.join(changeDir, 'specs', 'lower-capability'); + await fs.mkdir(changeSpecDir, { recursive: true }); + + // Lowercase section header: the parser reads it case-insensitively, so + // the archive gate must route it into delta validation the same way + // validate does instead of falling through to the rebuilt-spec check. + const specContent = `# Lower Capability - Changes + +## added requirements + +### Requirement: Logging Feature +The system SHALL log all events.`; + await fs.writeFile(path.join(changeSpecDir, 'spec.md'), specContent); + await fs.writeFile(path.join(changeDir, 'tasks.md'), '- [x] Task 1\n'); + + await archiveCommand.execute(changeName, { yes: true }); + + expect(process.exitCode).toBe(1); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('must include at least one scenario') + ); + await expect(fs.access(changeDir)).resolves.not.toThrow(); + }); + it('sets exit code 1 when delta spec validation fails', async () => { const changeName = 'exit-delta-fail'; const changeDir = path.join(tempDir, 'openspec', 'changes', changeName); diff --git a/test/core/command-generation/adapters.test.ts b/test/core/command-generation/adapters.test.ts index f758305704..75d63bc6b8 100644 --- a/test/core/command-generation/adapters.test.ts +++ b/test/core/command-generation/adapters.test.ts @@ -544,6 +544,19 @@ describe('command-generation/adapters', () => { }); expect(output).toContain('description: "Review: plan & apply \\"changes\\""'); }); + + it('should transform colon command references to hyphen format', () => { + // Qwen commands are invoked by filename (/opsx-), like bob/opencode. + const contentWithRefs: CommandContent = { + ...sampleContent, + body: 'Run /opsx:apply to implement. Then use /opsx:archive.', + }; + const output = qwenAdapter.formatFile(contentWithRefs); + expect(output).toContain('/opsx-apply'); + expect(output).toContain('/opsx-archive'); + expect(output).not.toContain('/opsx:apply'); + expect(output).not.toContain('/opsx:archive'); + }); }); describe('piAdapter', () => { diff --git a/test/core/init.test.ts b/test/core/init.test.ts index 492a23c6a4..3b4b5570a9 100644 --- a/test/core/init.test.ts +++ b/test/core/init.test.ts @@ -1064,6 +1064,22 @@ describe('InitCommand - profile and detection features', () => { } }); + it('should print the hyphen command hint for filename-invoked tools (claude+qwen)', async () => { + const initCommand = new InitCommand({ tools: 'claude,qwen', force: true }); + await initCommand.execute(testDir); + + const logCalls = (console.log as unknown as { mock: { calls: unknown[][] } }).mock.calls.flat().map(String); + const startHints = logCalls.filter((entry) => entry.includes('Start your first change')); + // Qwen invokes commands by filename (/opsx-propose), so it must not share + // Claude's /opsx:propose line + expect(startHints).toHaveLength(2); + const claudeHint = startHints.find((entry) => entry.includes('Claude Code')); + const qwenHint = startHints.find((entry) => entry.includes('Qwen Code')); + expect(claudeHint).toContain('/opsx:propose'); + expect(qwenHint).toContain('/opsx-propose'); + expect(qwenHint).not.toContain('/opsx:propose'); + }); + it('should not advertise an instruction for a tool that got no skills (delivery=commands, codex+kimi)', async () => { saveGlobalConfig({ featureFlags: {}, diff --git a/test/core/templates/skill-templates-parity.test.ts b/test/core/templates/skill-templates-parity.test.ts index ab84ea92dc..a9b2e6fdb2 100644 --- a/test/core/templates/skill-templates-parity.test.ts +++ b/test/core/templates/skill-templates-parity.test.ts @@ -40,43 +40,43 @@ const EXPECTED_FUNCTION_HASHES: Record = { getExploreSkillTemplate: 'a7eb6fabdc05a5b90a4773ba93320a60edffea88e9b27985668a2959dcec2e3d', getNewChangeSkillTemplate: '39663a6d2037e6697020393a66f6327506e3e3bc573b7a3556dcb7f9457dc51d', getContinueChangeSkillTemplate: '5cc6cf74c055ae67b08373421d934ece65dacbccafbc7452ab5636df3eb9e862', - getApplyChangeSkillTemplate: '0f5a15fc7fb9ad6059a5643d0e01365d27642637a4aaebf182f9eabb45348197', + getApplyChangeSkillTemplate: '3d52b852f3c5f87c3c88aeb4915c78604d97cc75d33aaac8f7e174d365b49971', getFfChangeSkillTemplate: '097a9ff9533900f227cac0523289eae4e19f06a081e5f355a8374dbecf3ff55d', getSyncSpecsSkillTemplate: '8a0e6a41250d9e5f893dd016c375ffb5773823693cb4e481ca74775bbfb9bfb9', - getOnboardSkillTemplate: 'bc2216b72724b01c3a733e63b8bf4aff457f561c0e9ff7288bdacc39780a37a7', + getOnboardSkillTemplate: 'f9988a9ef9ab7c09a16f64847902b2a082499f8a2d5c0533856cefb1d68f2318', getOpsxExploreCommandTemplate: 'eef1f8b4fd90ade6d70be46f0f8c3e6722f221fed175a6f9cf626287ef504a94', getOpsxNewCommandTemplate: '57c600cce318d16b9b4308a18d0d983ea3c0673034e606a7cceec07b4c705e87', getOpsxContinueCommandTemplate: '5c3968174001c20737ba39d2473ecec0f3b76591a80f7e2fc3974904d3da9dcd', - getOpsxApplyCommandTemplate: 'daeb507206707169de73c828e199648dde5732cbc17791ef2a027adffd028574', + getOpsxApplyCommandTemplate: '147408d7085b468981a400cc725804252c3fd84e519c57c5f6f83562e32606ee', getOpsxFfCommandTemplate: '264b514cc4849f91fb4414f639484c4181f1e5850d0d788ef276c851efa92859', getArchiveChangeSkillTemplate: '206a22b6778e97c30da9145ef51fdad449b8c995538f6fc25752ef551a37b675', getBulkArchiveChangeSkillTemplate: '2b74b1f73380ff32e35f580734780d843c6161a2748c39edb07f1e00453771b4', getOpsxSyncCommandTemplate: 'df0240a79f7b4943a54c7413ab088ee48f5bf5fe19f9347c170d695c8ec777a4', getVerifyChangeSkillTemplate: 'cab4db01b5d2b1243d63d90c53747d8b39e488c60f76eba3fe8b994467f69267', getOpsxArchiveCommandTemplate: '7dea65d0e2e17db366bb666ba6ae5e205ea02707b8c5c7707565200875c78916', - getOpsxOnboardCommandTemplate: '9430a0fb6530791ab720e068f4b172bc3dfc4e96a1ae29102bee0b92c2afe7b5', + getOpsxOnboardCommandTemplate: '16a68b8c9819e2a7bab013c3b49a3e49ea258b68c4e7f47f0d598e30815e0a80', getOpsxBulkArchiveCommandTemplate: 'da7be1a7318f15b915f5aae8eb638797a8a24a31e5fc7fc0a2bad01bba137686', getOpsxVerifyCommandTemplate: 'f01c0c0cef53be0956de52363d955d4ace131b1b2d77adf902f35fead9a1486d', getOpsxProposeSkillTemplate: '57fb556a060e2eb246b500922837af7573a6e100a6ed7dfaa7bd4ce0f5daffd3', getOpsxProposeCommandTemplate: '434cae3ee20835725bb1d2ccb9698310a850c5b95ed669ea15fc7a0125371c59', getFeedbackSkillTemplate: 'd7d83c5f7fc2b92fe8f4588a5bf2d9cb315e4c73ec19bcd5ef28270906319a0d', - getUpdateChangeSkillTemplate: 'a30e5bc2ce1e6ba97db22fd7773797ef1760309ee9f4fc28ca46e63486b5e9dd', - getOpsxUpdateCommandTemplate: 'd4eafd808ad614b7d3f188cbe8d8c5fff36504fd63f9b2903dc7aa6fc0f1201d', + getUpdateChangeSkillTemplate: 'd885847ea1af48a2ef41a08f6319888d058d50b81cf5511bda768cd4b59359ee', + getOpsxUpdateCommandTemplate: 'cf43a6bdcdc549180970ddde40893223493a55e171a39290731e0339df530975', }; const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record = { 'openspec-explore': 'c8de6033b2c78009647647c65a504e4ada1a3bdcee31aed38a4bf7d629513f6e', 'openspec-new-change': 'd5b8909bea70a33b7a312b38ce204a91f40b6bb2bff12c4c06b3e11641b6a689', 'openspec-continue-change': '02ec4de061ad6277866b877497a1e66142ba364e12b83dd7dedb838579ea88db', - 'openspec-apply-change': '09c0e1cdf5ccc82416d0969d6bd715cc70616bdbc3531358a5c36057f78be55a', + 'openspec-apply-change': '2f7a8e7a7528d9f8d89b508a8cbc909ba47bdff473db19317008d156b9ba5893', 'openspec-ff-change': 'ff3bd3eac427a1e50071ad7c70f73b556cffa3db43e90da2726e96849c3fc886', 'openspec-sync-specs': '74de778dd8a8fd4987a09621147358cc32505bb58110492ab2b4ffe7f35aa48f', 'openspec-archive-change': '64b1611dd7aee04ca268820d1b193e8bf0a39ff3672ec6ba21fb0a1bcb1786c2', 'openspec-bulk-archive-change': '49d410bda408c0411decd584be9c2355335e3b3db760fc6a0adcd82c172a280f', 'openspec-verify-change': '57693d22940f06080c6cf8d590ac2f48240d4a5e9ce7074dacd0f8d3c9945afa', - 'openspec-onboard': '76225d10352454a304e56566997811d16f91de1b37653816f2bc5d8ec976febc', + 'openspec-onboard': '1d581c12d4928d751eb79de099e275dabe9c99fc15dc1f502abebd99ad7cb7d2', 'openspec-propose': '4638400113946f4f1ee9f0bd0e965aafb200bd89b64ec7f5406ef5e948e8e218', - 'openspec-update-change': '6b37268bca94856d5533515762821274664b8dc9f2644b6c081ea6cc0205eda7', + 'openspec-update-change': '4e6669540bc5332b72db7dd432625cc4b45234ae7674f9b46fcd1309b9697b0d', }; // Intentionally excludes getFeedbackSkillTemplate: this list only models templates diff --git a/test/core/validation.test.ts b/test/core/validation.test.ts index a443e4ab0d..04c63d943e 100644 --- a/test/core/validation.test.ts +++ b/test/core/validation.test.ts @@ -446,6 +446,63 @@ Then result`; }); describe('validateChangeDeltaSpecs with metadata', () => { + it('rejects a delta that both renames and removes the same requirement', async () => { + // Parity with archive: apply-time rejects this contradiction, so + // validate must flag it too instead of reporting the change as valid. + const changeDir = path.join(testDir, 'rename-remove-conflict'); + const specsDir = path.join(changeDir, 'specs', 'test-spec'); + await fs.mkdir(specsDir, { recursive: true }); + + const deltaSpec = `# Test Spec + +## RENAMED Requirements + +- FROM: \`### Requirement: Old name\` +- TO: \`### Requirement: New name\` + +## REMOVED Requirements + +### Requirement: Old name`; + + await fs.writeFile(path.join(specsDir, 'spec.md'), deltaSpec); + + const validator = new Validator(true); + const report = await validator.validateChangeDeltaSpecs(changeDir); + + expect(report.valid).toBe(false); + const msg = report.issues.map((i) => i.message).join('\n'); + expect(msg).toContain('Requirement present in both RENAMED and REMOVED: "Old name"'); + }); + + it('rejects a case/whitespace variant of the renamed FROM header in REMOVED', async () => { + // The contradiction is the same when REMOVED spells the FROM header + // with different case or spacing - the folded identity must catch it. + const changeDir = path.join(testDir, 'rename-remove-case-conflict'); + const specsDir = path.join(changeDir, 'specs', 'test-spec'); + await fs.mkdir(specsDir, { recursive: true }); + + const deltaSpec = `# Test Spec + +## RENAMED Requirements + +- FROM: \`### Requirement: Old Name\` +- TO: \`### Requirement: New Name\` + +## REMOVED Requirements + +### Requirement: old name`; + + await fs.writeFile(path.join(specsDir, 'spec.md'), deltaSpec); + + const validator = new Validator(true); + const report = await validator.validateChangeDeltaSpecs(changeDir); + + expect(report.valid).toBe(false); + const msg = report.issues.map((i) => i.message).join('\n'); + expect(msg).toContain('Requirement present in both RENAMED and REMOVED: "Old Name"'); + expect(msg).toContain('(REMOVED spells it "old name")'); + }); + it('should validate requirement with metadata before SHALL/MUST text', async () => { const changeDir = path.join(testDir, 'test-change'); const specsDir = path.join(changeDir, 'specs', 'test-spec'); diff --git a/test/utils/command-references.test.ts b/test/utils/command-references.test.ts index 1f2367e517..8a9d7dced1 100644 --- a/test/utils/command-references.test.ts +++ b/test/utils/command-references.test.ts @@ -206,13 +206,15 @@ describe('getTransformerForTool', () => { } }); - it('selects hyphen commands for opencode, pi, and oh-my-pi when commands are generated', () => { - expect(getTransformerForTool('opencode', 'both', 'adapter-backed')).toBe(transformToHyphenCommands); - expect(getTransformerForTool('opencode', 'commands', 'adapter-backed')).toBe(transformToHyphenCommands); - expect(getTransformerForTool('pi', 'both', 'adapter-backed')).toBe(transformToHyphenCommands); - expect(getTransformerForTool('pi', 'commands', 'adapter-backed')).toBe(transformToHyphenCommands); - expect(getTransformerForTool('oh-my-pi', 'both', 'adapter-backed')).toBe(transformToHyphenCommands); - expect(getTransformerForTool('oh-my-pi', 'commands', 'adapter-backed')).toBe(transformToHyphenCommands); + it('selects hyphen commands for bob, oh-my-pi, opencode, pi, and qwen when commands are generated', () => { + // These tools invoke commands by filename (/opsx-), so skills must + // reference the hyphen form their command files actually answer to. + for (const toolId of ['bob', 'oh-my-pi', 'opencode', 'pi', 'qwen'] as const) { + expect(getTransformerForTool(toolId, 'both', 'adapter-backed')).toBe(transformToHyphenCommands); + expect(getTransformerForTool(toolId, 'commands', 'adapter-backed')).toBe(transformToHyphenCommands); + // ...but must not fall back to hyphen commands when no commands are generated + expect(getTransformerForTool(toolId, 'skills', 'adapter-backed')).toBe(transformToSkillReferences); + } }); it('selects no transformer for adapter-backed and skills-invocable tools when commands are generated', () => { diff --git a/test/utils/spec-discovery.test.ts b/test/utils/spec-discovery.test.ts index e8c18e75a5..1fb6713a77 100644 --- a/test/utils/spec-discovery.test.ts +++ b/test/utils/spec-discovery.test.ts @@ -108,6 +108,45 @@ describe('discoverSpecFiles', () => { }); }); + it('discovers a symlinked spec.md file', async () => { + await withTempDir(async (dir) => { + // hasAnyFileUnder and the artifact graph's globs both count a symlinked + // spec.md as content, so discovery must not silently drop it. + const target = path.join(dir, 'shared-delta.md'); + await fs.writeFile(target, '# Spec\n', 'utf8'); + await fs.mkdir(path.join(dir, 'auth'), { recursive: true }); + try { + await fs.symlink(target, path.join(dir, 'auth', 'spec.md'), 'file'); + } catch { + // Symlink creation can be unavailable (e.g. Windows without dev mode). + return; + } + + const found = await discoverSpecFiles(dir); + expect(found.map((s) => s.id)).toEqual(['auth']); + expect(found[0].specFile).toBe(path.join(dir, 'auth', 'spec.md')); + }); + }); + + it('skips a dangling spec.md symlink', async () => { + await withTempDir(async (dir) => { + await writeSpec(dir, 'real'); + await fs.mkdir(path.join(dir, 'ghost'), { recursive: true }); + try { + await fs.symlink( + path.join(dir, 'missing-target.md'), + path.join(dir, 'ghost', 'spec.md'), + 'file' + ); + } catch { + return; + } + + const found = await discoverSpecFiles(dir); + expect(found.map((s) => s.id)).toEqual(['real']); + }); + }); + it('does not follow symlinked directories', async () => { await withTempDir(async (dir) => { await writeSpec(dir, 'real');