diff --git a/package-lock.json b/package-lock.json index e1daf932f..9340fe4bf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@fission-ai/openspec", - "version": "1.1.1", + "version": "1.2.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@fission-ai/openspec", - "version": "1.1.1", + "version": "1.2.0", "hasInstallScript": true, "license": "MIT", "dependencies": { diff --git a/src/cli/index.ts b/src/cli/index.ts index 8947736f7..b776ffbf7 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -20,12 +20,14 @@ import { statusCommand, instructionsCommand, applyInstructionsCommand, + verifyInstructionsCommand, templatesCommand, schemasCommand, newChangeCommand, DEFAULT_SCHEMA, type StatusOptions, type InstructionsOptions, + type VerifyInstructionsOptions, type TemplatesOptions, type SchemasOptions, type NewChangeOptions, @@ -445,9 +447,11 @@ program .option('--json', 'Output as JSON') .action(async (artifactId: string | undefined, options: InstructionsOptions) => { try { - // Special case: "apply" is not an artifact, but a command to get apply instructions + // Special case: "apply" and "verify" are not artifacts, but phase commands if (artifactId === 'apply') { await applyInstructionsCommand(options); + } else if (artifactId === 'verify') { + await verifyInstructionsCommand(options as VerifyInstructionsOptions); } else { await instructionsCommand(artifactId, options); } diff --git a/src/commands/workflow/index.ts b/src/commands/workflow/index.ts index 232b2dbe3..28c89db6a 100644 --- a/src/commands/workflow/index.ts +++ b/src/commands/workflow/index.ts @@ -7,8 +7,8 @@ export { statusCommand } from './status.js'; export type { StatusOptions } from './status.js'; -export { instructionsCommand, applyInstructionsCommand } from './instructions.js'; -export type { InstructionsOptions } from './instructions.js'; +export { instructionsCommand, applyInstructionsCommand, verifyInstructionsCommand } from './instructions.js'; +export type { InstructionsOptions, VerifyInstructionsOptions } from './instructions.js'; export { templatesCommand } from './templates.js'; export type { TemplatesOptions } from './templates.js'; diff --git a/src/commands/workflow/instructions.ts b/src/commands/workflow/instructions.ts index 0d501afec..340f137b6 100644 --- a/src/commands/workflow/instructions.ts +++ b/src/commands/workflow/instructions.ts @@ -17,8 +17,10 @@ import { import { validateChangeExists, validateSchemaExists, + readPhaseConfig, type TaskItem, type ApplyInstructions, + type VerifyInstructions, } from './shared.js'; // ----------------------------------------------------------------------------- @@ -37,6 +39,12 @@ export interface ApplyInstructionsOptions { json?: boolean; } +export interface VerifyInstructionsOptions { + change?: string; + schema?: string; + json?: boolean; +} + // ----------------------------------------------------------------------------- // Artifact Instructions Command // ----------------------------------------------------------------------------- @@ -386,6 +394,9 @@ export async function generateApplyInstructions( instruction = schemaInstruction?.trim() ?? 'Read context files, work through pending tasks, mark complete as you go.\nPause if you hit blockers or need clarification.'; } + // Read project config for context and rules injection + const phaseConfig = readPhaseConfig(projectRoot, 'apply'); + return { changeName, changeDir, @@ -396,6 +407,8 @@ export async function generateApplyInstructions( state, missingArtifacts: missingArtifacts.length > 0 ? missingArtifacts : undefined, instruction, + context: phaseConfig.context, + rules: phaseConfig.rules, }; } @@ -429,7 +442,7 @@ export async function applyInstructionsCommand(options: ApplyInstructionsOptions } export function printApplyInstructionsText(instructions: ApplyInstructions): void { - const { changeName, schemaName, contextFiles, progress, tasks, state, missingArtifacts, instruction } = instructions; + const { changeName, schemaName, contextFiles, progress, tasks, state, missingArtifacts, instruction, context, rules } = instructions; console.log(`## Apply: ${changeName}`); console.log(`Schema: ${schemaName}`); @@ -475,7 +488,160 @@ export function printApplyInstructionsText(instructions: ApplyInstructions): voi console.log(); } + // Project context (if present) + if (context) { + console.log('### Project Context'); + console.log(context); + console.log(); + } + + // Rules (if present) + if (rules && rules.length > 0) { + console.log('### Rules'); + for (const rule of rules) { + console.log(`- ${rule}`); + } + console.log(); + } + // Instruction console.log('### Instruction'); console.log(instruction); } + +// ----------------------------------------------------------------------------- +// Verify Instructions Command +// ----------------------------------------------------------------------------- + +/** + * Generates verify instructions for validating implementation against artifacts. + * Reads config.yaml context and rules.verify for custom verification strategies. + */ +export async function generateVerifyInstructions( + projectRoot: string, + changeName: string, + schemaName?: string +): Promise { + const context = loadChangeContext(projectRoot, changeName, schemaName); + const changeDir = path.join(projectRoot, 'openspec', 'changes', changeName); + + const schema = resolveSchema(context.schemaName, projectRoot); + + // Build context files from all existing artifacts in schema + const contextFiles: Record = {}; + for (const artifact of schema.artifacts) { + if (artifactOutputExists(changeDir, artifact.generates)) { + contextFiles[artifact.id] = path.join(changeDir, artifact.generates); + } + } + + // Parse tasks if tracking file exists (reuse apply logic) + const applyConfig = schema.apply; + const tracksFile = applyConfig?.tracks ?? null; + let tasks: TaskItem[] = []; + if (tracksFile) { + const tracksPath = path.join(changeDir, tracksFile); + if (fs.existsSync(tracksPath)) { + const tasksContent = await fs.promises.readFile(tracksPath, 'utf-8'); + tasks = parseTasksFile(tasksContent); + } + } + + const total = tasks.length; + const complete = tasks.filter((t) => t.done).length; + const remaining = total - complete; + + // Read project config for context and rules injection + const phaseConfig = readPhaseConfig(projectRoot, 'verify'); + + return { + changeName, + changeDir, + schemaName: context.schemaName, + contextFiles, + progress: { total, complete, remaining }, + tasks, + context: phaseConfig.context, + rules: phaseConfig.rules, + }; +} + +export async function verifyInstructionsCommand(options: VerifyInstructionsOptions): Promise { + const spinner = ora('Generating verify instructions...').start(); + + try { + const projectRoot = process.cwd(); + const changeName = await validateChangeExists(options.change, projectRoot); + + if (options.schema) { + validateSchemaExists(options.schema, projectRoot); + } + + const instructions = await generateVerifyInstructions(projectRoot, changeName, options.schema); + + spinner.stop(); + + if (options.json) { + console.log(JSON.stringify(instructions, null, 2)); + return; + } + + printVerifyInstructionsText(instructions); + } catch (error) { + spinner.stop(); + throw error; + } +} + +export function printVerifyInstructionsText(instructions: VerifyInstructions): void { + const { changeName, schemaName, contextFiles, progress, tasks, context, rules } = instructions; + + console.log(`## Verify: ${changeName}`); + console.log(`Schema: ${schemaName}`); + console.log(); + + // Context files + const contextFileEntries = Object.entries(contextFiles); + if (contextFileEntries.length > 0) { + console.log('### Context Files'); + for (const [artifactId, filePath] of contextFileEntries) { + console.log(`- ${artifactId}: ${filePath}`); + } + console.log(); + } + + // Progress + if (progress.total > 0) { + console.log('### Progress'); + console.log(`${progress.complete}/${progress.total} tasks complete`); + console.log(); + } + + // Project context (if present) + if (context) { + console.log('### Project Context'); + console.log(context); + console.log(); + } + + // Rules (if present) + if (rules && rules.length > 0) { + console.log('### Rules'); + for (const rule of rules) { + console.log(`- ${rule}`); + } + console.log(); + } + + // Incomplete tasks + if (tasks.length > 0) { + const incomplete = tasks.filter(t => !t.done); + if (incomplete.length > 0) { + console.log('### Incomplete Tasks'); + for (const task of incomplete) { + console.log(`- [ ] ${task.description}`); + } + console.log(); + } + } +} diff --git a/src/commands/workflow/shared.ts b/src/commands/workflow/shared.ts index a85fcd585..75f198735 100644 --- a/src/commands/workflow/shared.ts +++ b/src/commands/workflow/shared.ts @@ -10,6 +10,7 @@ import path from 'path'; import * as fs from 'fs'; import { getSchemaDir, listSchemas } from '../../core/artifact-graph/index.js'; import { validateChangeName } from '../../utils/change-utils.js'; +import { readProjectConfig, RESERVED_PHASE_IDS } from '../../core/project-config.js'; // ----------------------------------------------------------------------------- // Types @@ -35,6 +36,56 @@ export interface ApplyInstructions { state: 'blocked' | 'all_done' | 'ready'; missingArtifacts?: string[]; instruction: string; + context?: string; + rules?: string[]; +} + +export interface VerifyInstructions { + changeName: string; + changeDir: string; + schemaName: string; + contextFiles: Record; + progress: { + total: number; + complete: number; + remaining: number; + }; + tasks: TaskItem[]; + context?: string; + rules?: string[]; +} + +// Re-export RESERVED_PHASE_IDS for convenience +export { RESERVED_PHASE_IDS } from '../../core/project-config.js'; + +/** + * Reads project config and extracts context + phase-specific rules. + * Centralizes the config-reading logic used by both apply and verify phases. + * + * @param projectRoot - The root directory of the project + * @param phase - The phase to read rules for ('apply' | 'verify') + * @returns Object with optional context and rules, or empty object on failure + */ +export function readPhaseConfig( + projectRoot: string, + phase: 'apply' | 'verify' +): { context?: string; rules?: string[] } { + try { + const projectConfig = readProjectConfig(projectRoot); + const result: { context?: string; rules?: string[] } = {}; + + if (projectConfig?.context?.trim()) { + result.context = projectConfig.context.trim(); + } + if (projectConfig?.rules?.[phase] && projectConfig.rules[phase].length > 0) { + result.rules = projectConfig.rules[phase]; + } + + return result; + } catch { + // If config read fails, continue without config + return {}; + } } // ----------------------------------------------------------------------------- diff --git a/src/core/artifact-graph/schema.ts b/src/core/artifact-graph/schema.ts index 6371745e2..7cd0dece0 100644 --- a/src/core/artifact-graph/schema.ts +++ b/src/core/artifact-graph/schema.ts @@ -1,6 +1,7 @@ import * as fs from 'node:fs'; import { parse as parseYaml } from 'yaml'; import { SchemaYamlSchema, type SchemaYaml, type Artifact } from './types.js'; +import { RESERVED_PHASE_IDS } from '../project-config.js'; export class SchemaValidationError extends Error { constructor(message: string) { @@ -32,6 +33,9 @@ export function parseSchema(yamlContent: string): SchemaYaml { const schema = result.data; + // Check for reserved phase IDs used as artifact IDs + validateNoReservedIds(schema.artifacts); + // Check for duplicate artifact IDs validateNoDuplicateIds(schema.artifacts); @@ -44,6 +48,21 @@ export function parseSchema(yamlContent: string): SchemaYaml { return schema; } +/** + * Validates that no artifact uses a reserved phase ID ('apply', 'verify'). + * These names are reserved for phase-level commands in the CLI. + */ +function validateNoReservedIds(artifacts: Artifact[]): void { + for (const artifact of artifacts) { + if (RESERVED_PHASE_IDS.has(artifact.id.toLowerCase())) { + throw new SchemaValidationError( + `Artifact ID '${artifact.id}' is reserved for phase commands. ` + + `Reserved IDs: ${Array.from(RESERVED_PHASE_IDS).join(', ')}` + ); + } + } +} + /** * Validates that there are no duplicate artifact IDs. */ diff --git a/src/core/project-config.ts b/src/core/project-config.ts index 6c1ea04a5..ea28f5d09 100644 --- a/src/core/project-config.ts +++ b/src/core/project-config.ts @@ -3,6 +3,12 @@ import path from 'path'; import { parse as parseYaml } from 'yaml'; import { z } from 'zod'; +/** + * Phase IDs that are valid rule targets but not schema artifacts. + * These names are reserved and cannot be used as artifact IDs. + */ +export const RESERVED_PHASE_IDS = new Set(['apply', 'verify']); + /** * Zod schema for project configuration. * @@ -123,7 +129,22 @@ export function readProjectConfig(projectRoot: string): ProjectConfig | null { let hasValidRules = false; for (const [artifactId, rules] of Object.entries(raw.rules)) { - const rulesArrayResult = z.array(z.string()).safeParse(rules); + // Normalize rules array: YAML may parse strings containing colons + // (e.g., "subagent_type: flutter-agent") as objects instead of strings. + // Convert such objects back to "key: value" strings before validation. + const normalizedRules = Array.isArray(rules) + ? rules.map((item: unknown) => { + if (typeof item === 'string') return item; + if (typeof item === 'object' && item !== null && !Array.isArray(item)) { + return Object.entries(item) + .map(([k, v]) => `${k}: ${v}`) + .join(', '); + } + return String(item); + }) + : rules; + + const rulesArrayResult = z.array(z.string()).safeParse(normalizedRules); if (rulesArrayResult.success) { // Filter out empty strings @@ -178,11 +199,11 @@ export function validateConfigRules( const warnings: string[] = []; for (const artifactId of Object.keys(rules)) { - if (!validArtifactIds.has(artifactId)) { + if (!validArtifactIds.has(artifactId) && !RESERVED_PHASE_IDS.has(artifactId)) { const validIds = Array.from(validArtifactIds).sort().join(', '); warnings.push( `Unknown artifact ID in rules: "${artifactId}". ` + - `Valid IDs for schema "${schemaName}": ${validIds}` + `Valid IDs for schema "${schemaName}": ${validIds}, apply, verify` ); } } diff --git a/src/core/templates/workflows/apply-change.ts b/src/core/templates/workflows/apply-change.ts index c5044698a..e4f06d125 100644 --- a/src/core/templates/workflows/apply-change.ts +++ b/src/core/templates/workflows/apply-change.ts @@ -44,6 +44,8 @@ export function getApplyChangeSkillTemplate(): SkillTemplate { - Progress (total, complete, remaining) - Task list with status - Dynamic instruction based on current state + - \`context\`: Project context from config.yaml (if present) + - \`rules\`: Apply-phase rules from config.yaml (if present) — **these are mandatory constraints, you MUST follow them** **Handle states:** - If \`state: "blocked"\` (missing artifacts): show message, suggest using openspec-continue-change @@ -64,9 +66,13 @@ export function getApplyChangeSkillTemplate(): SkillTemplate { - Progress: "N/M tasks complete" - Remaining tasks overview - Dynamic instruction from CLI + - If rules are present, display them as "Apply Rules" section 6. **Implement tasks (loop until done or blocked)** + **IMPORTANT**: If \`rules\` are present in the apply instructions output, you MUST follow them as mandatory execution constraints. Rules may specify execution strategies (e.g., using subagents, specific testing requirements, retry policies). Follow them exactly. + + If no rules are present, use the default behavior: For each pending task: - Show which task is being worked on - Make the code changes required @@ -201,6 +207,8 @@ export function getOpsxApplyCommandTemplate(): CommandTemplate { - Progress (total, complete, remaining) - Task list with status - Dynamic instruction based on current state + - \`context\`: Project context from config.yaml (if present) + - \`rules\`: Apply-phase rules from config.yaml (if present) — **these are mandatory constraints, you MUST follow them** **Handle states:** - If \`state: "blocked"\` (missing artifacts): show message, suggest using \`/opsx:continue\` @@ -221,9 +229,13 @@ export function getOpsxApplyCommandTemplate(): CommandTemplate { - Progress: "N/M tasks complete" - Remaining tasks overview - Dynamic instruction from CLI + - If rules are present, display them as "Apply Rules" section 6. **Implement tasks (loop until done or blocked)** + **IMPORTANT**: If \`rules\` are present in the apply instructions output, you MUST follow them as mandatory execution constraints. Rules may specify execution strategies (e.g., using subagents, specific testing requirements, retry policies). Follow them exactly. + + If no rules are present, use the default behavior: For each pending task: - Show which task is being worked on - Make the code changes required diff --git a/src/core/templates/workflows/verify-change.ts b/src/core/templates/workflows/verify-change.ts index 825f7ecca..545b540c2 100644 --- a/src/core/templates/workflows/verify-change.ts +++ b/src/core/templates/workflows/verify-change.ts @@ -34,13 +34,19 @@ export function getVerifyChangeSkillTemplate(): SkillTemplate { - \`schemaName\`: The workflow being used (e.g., "spec-driven") - Which artifacts exist for this change -3. **Get the change directory and load artifacts** +3. **Get verify instructions and load artifacts** \`\`\`bash - openspec instructions apply --change "" --json + openspec instructions verify --change "" --json \`\`\` - This returns the change directory and context files. Read all available artifacts from \`contextFiles\`. + This returns: + - Change directory and context files — read all available artifacts from \`contextFiles\` + - Task progress (total, complete, remaining) + - \`context\`: Project context from config.yaml (if present) + - \`rules\`: Verify-phase rules from config.yaml (if present) — **these are mandatory constraints, you MUST follow them** + + **IMPORTANT**: If \`rules\` are present, they define the verification strategy (e.g., parallel subagent scans, issue classification, fix loops). You MUST follow the rules instead of the default verification steps below. The default steps (4-8) are only used when no custom rules are configured. 4. **Initialize verification report structure** @@ -203,13 +209,19 @@ export function getOpsxVerifyCommandTemplate(): CommandTemplate { - \`schemaName\`: The workflow being used (e.g., "spec-driven") - Which artifacts exist for this change -3. **Get the change directory and load artifacts** +3. **Get verify instructions and load artifacts** \`\`\`bash - openspec instructions apply --change "" --json + openspec instructions verify --change "" --json \`\`\` - This returns the change directory and context files. Read all available artifacts from \`contextFiles\`. + This returns: + - Change directory and context files — read all available artifacts from \`contextFiles\` + - Task progress (total, complete, remaining) + - \`context\`: Project context from config.yaml (if present) + - \`rules\`: Verify-phase rules from config.yaml (if present) — **these are mandatory constraints, you MUST follow them** + + **IMPORTANT**: If \`rules\` are present, they define the verification strategy (e.g., parallel subagent scans, issue classification, fix loops). You MUST follow the rules instead of the default verification steps below. The default steps (4-8) are only used when no custom rules are configured. 4. **Initialize verification report structure** diff --git a/test/core/templates/skill-templates-parity.test.ts b/test/core/templates/skill-templates-parity.test.ts index f8fb1307b..49161a29f 100644 --- a/test/core/templates/skill-templates-parity.test.ts +++ b/test/core/templates/skill-templates-parity.test.ts @@ -33,23 +33,23 @@ const EXPECTED_FUNCTION_HASHES: Record = { getExploreSkillTemplate: '55a2a1afcba0af88c638e77e4e3870f65ed82c030b4a2056d39812ae13a616be', getNewChangeSkillTemplate: '5989672758eccf54e3bb554ab97f2c129a192b12bbb7688cc1ffcf6bccb1ae9d', getContinueChangeSkillTemplate: 'f2e413f0333dfd6641cc2bd1a189273fdea5c399eecdde98ef528b5216f097b3', - getApplyChangeSkillTemplate: '26e52e67693e93fbcdd40dcd3e20949c07ce019183d55a8149d0260c791cd7f4', + getApplyChangeSkillTemplate: '594e98ec4f0b05c97ad28cf660d451ac96b658511665b4e507e14a5a8686d2cd', getFfChangeSkillTemplate: 'a7332fb14c8dc3f9dec71f5d332790b4a8488191e7db4ab6132ccbefecf9ded9', getSyncSpecsSkillTemplate: 'bded184e4c345619148de2c0ad80a5b527d4ffe45c87cc785889b9329e0f465b', getOnboardSkillTemplate: '819a2d117ad1386187975686839cb0584b41484013d0ca6a6691f7a439a11a4a', getOpsxExploreCommandTemplate: '91353d9e8633a3a9ce7339e796f1283478fca279153f3807c92f4f8ece246b19', getOpsxNewCommandTemplate: '62eee32d6d81a376e7be845d0891e28e6262ad07482f9bfe6af12a9f0366c364', getOpsxContinueCommandTemplate: '8bbaedcc95287f9e822572608137df4f49ad54cedfb08d3342d0d1c4e9716caa', - getOpsxApplyCommandTemplate: 'a9d631a07fcd832b67d263ff3800b98604ab8d378baf1b0d545907ef3affa3b5', + getOpsxApplyCommandTemplate: '8fa15858acd7123538f748f0bfa66edbae7e1109fcc85c84a10064446fb8d950', getOpsxFfCommandTemplate: 'cdebe872cc8e0fcc25c8864b98ffd66a93484c0657db94bd1285b8113092702a', getArchiveChangeSkillTemplate: '6f8ca383fdb5a4eb9872aca81e07bf0ba7f25e4de8617d7a047ca914ca7f14b9', getBulkArchiveChangeSkillTemplate: 'b40fc44ea4e420bdc9c803985b10e5c091fc472cdfc69153b962be6be303bddd', getOpsxSyncCommandTemplate: '378d035fe7cc30be3e027b66dcc4b8afc78ef1c8369c39479c9b05a582fb5ccf', - getVerifyChangeSkillTemplate: '63a213ba3b42af54a1cd56f5072234a03b265c3fe4a1da12cd6fbbef5ee46c4b', + getVerifyChangeSkillTemplate: '790bde37183e0043d3f907880cc448561d7e93f769d00ff19fa202e7a1a90738', getOpsxArchiveCommandTemplate: 'b44cc9748109f61687f9f596604b037bc3ea803abc143b22f09a76aebd98b493', getOpsxOnboardCommandTemplate: '10052d05a4e2cdade7fdfa549b3444f7a92f55a39bf81ddd6af7e0e9e83a7302', getOpsxBulkArchiveCommandTemplate: 'eaaba253a950b9e681d8427a5cbc6b50c4e91137fb37fd2360859e08f63a0c14', - getOpsxVerifyCommandTemplate: '9b4d3ca422553b7534764eb3a009da87a051612c5238e9baab294c7b1233e9a2', + getOpsxVerifyCommandTemplate: 'dc19d83b22d47a1c70674a9147ccb5170b3434dccf1fb3cf13107fdda9c1ca2e', getOpsxProposeSkillTemplate: 'd67f937d44650e9c61d2158c865309fbab23cb3f50a3d4868a640a97776e3999', getOpsxProposeCommandTemplate: '41ad59b37eafd7a161bab5c6e41997a37368f9c90b194451295ede5cd42e4d46', getFeedbackSkillTemplate: 'd7d83c5f7fc2b92fe8f4588a5bf2d9cb315e4c73ec19bcd5ef28270906319a0d', @@ -59,12 +59,12 @@ const EXPECTED_GENERATED_SKILL_CONTENT_HASHES: Record = { 'openspec-explore': '90463d00761417dfbca5cb09361adcf8bbdbbb24000b86dd03647869a4104479', 'openspec-new-change': 'c324a7ace1f244aa3f534ac8e3370a2c11190d6d1b85a315f26a211398310f0f', 'openspec-continue-change': '463cf0b980ec9c3c24774414ef2a3e48e9faa8577bc8748990f45ab3d5efe960', - 'openspec-apply-change': 'a0084442b59be9d7e22a0382a279d470501e1ecf74bdd5347e169951c9be191c', + 'openspec-apply-change': '97317c6eda77a5a83425eb10300bf417bd6d93062f12fa1807a99b4470cb3a0d', 'openspec-ff-change': '672c3a5b8df152d959b15bd7ae2be7a75ab7b8eaa2ec1e0daa15c02479b27937', 'openspec-sync-specs': 'b8859cf454379a19ca35dbf59eedca67306607f44a355327f9dc851114e50bde', 'openspec-archive-change': 'f83c85452bd47de0dee6b8efbcea6a62534f8a175480e9044f3043f887cebf0f', 'openspec-bulk-archive-change': 'a235a539f7729ab7669e45256905808789240ecd02820e044f4d0eef67b0c2ab', - 'openspec-verify-change': '30d07c6f7051965f624f5964db51844ec17c7dfd05f0da95281fe0ca73616326', + 'openspec-verify-change': 'cf3df6f0548db07686993cc91efa7d856b855a6dcc314e5fceb6ed2d2f6a9fe2', 'openspec-onboard': 'dbce376cf895f3fe4f63b4bce66d258c35b7b8884ac746670e5e35fabcefd255', 'openspec-propose': '20e36dabefb90e232bad0667292bd5007ec280f8fc4fc995dbc4282bf45a22e7', };