diff --git a/openspec/changes/make-apply-instructions-schema-aware/tasks.md b/openspec/changes/make-apply-instructions-schema-aware/tasks.md index d9c173ce..29408bd9 100644 --- a/openspec/changes/make-apply-instructions-schema-aware/tasks.md +++ b/openspec/changes/make-apply-instructions-schema-aware/tasks.md @@ -1,35 +1,35 @@ ## Prerequisites -- [ ] 0.1 Implement `add-per-change-schema-metadata` first (to auto-detect schema) +- [x] 0.1 Implement `add-per-change-schema-metadata` first (to auto-detect schema) ## 1. Schema Format -- [ ] 1.1 Add `ApplyPhaseSchema` Zod schema to `src/core/artifact-graph/types.ts` -- [ ] 1.2 Update `SchemaYamlSchema` to include optional `apply` field -- [ ] 1.3 Export `ApplyPhase` type +- [x] 1.1 Add `ApplyPhaseSchema` Zod schema to `src/core/artifact-graph/types.ts` +- [x] 1.2 Update `SchemaYamlSchema` to include optional `apply` field +- [x] 1.3 Export `ApplyPhase` type ## 2. Update Existing Schemas -- [ ] 2.1 Add `apply` block to `schemas/spec-driven/schema.yaml` -- [ ] 2.2 Add `apply` block to `schemas/tdd/schema.yaml` +- [x] 2.1 Add `apply` block to `schemas/spec-driven/schema.yaml` +- [x] 2.2 Add `apply` block to `schemas/tdd/schema.yaml` ## 3. Refactor generateApplyInstructions -- [ ] 3.1 Load schema via `resolveSchema(schemaName)` -- [ ] 3.2 Read `apply.requires` to determine required artifacts -- [ ] 3.3 Check artifact existence dynamically (not hardcoded paths) -- [ ] 3.4 Use `apply.tracks` for progress tracking (or skip if null) -- [ ] 3.5 Use `apply.instruction` for the instruction text -- [ ] 3.6 Build `contextFiles` from all existing artifacts in schema +- [x] 3.1 Load schema via `resolveSchema(schemaName)` +- [x] 3.2 Read `apply.requires` to determine required artifacts +- [x] 3.3 Check artifact existence dynamically (not hardcoded paths) +- [x] 3.4 Use `apply.tracks` for progress tracking (or skip if null) +- [x] 3.5 Use `apply.instruction` for the instruction text +- [x] 3.6 Build `contextFiles` from all existing artifacts in schema ## 4. Handle Fallback -- [ ] 4.1 If schema has no `apply` block, require all artifacts to exist -- [ ] 4.2 Default instruction: "All artifacts complete. Proceed with implementation." +- [x] 4.1 If schema has no `apply` block, require all artifacts to exist +- [x] 4.2 Default instruction: "All artifacts complete. Proceed with implementation." ## 5. Tests -- [ ] 5.1 Test apply instructions with spec-driven schema -- [ ] 5.2 Test apply instructions with tdd schema -- [ ] 5.3 Test fallback when schema has no apply block -- [ ] 5.4 Test blocked state when required artifacts missing +- [x] 5.1 Test apply instructions with spec-driven schema +- [x] 5.2 Test apply instructions with tdd schema +- [x] 5.3 Test fallback when schema has no apply block +- [x] 5.4 Test blocked state when required artifacts missing diff --git a/schemas/spec-driven/schema.yaml b/schemas/spec-driven/schema.yaml index 0990566e..d4a68134 100644 --- a/schemas/spec-driven/schema.yaml +++ b/schemas/spec-driven/schema.yaml @@ -139,3 +139,10 @@ artifacts: requires: - specs - design + +apply: + requires: [tasks] + tracks: tasks.md + instruction: | + Read context files, work through pending tasks, mark complete as you go. + Pause if you hit blockers or need clarification. diff --git a/schemas/tdd/schema.yaml b/schemas/tdd/schema.yaml index c36d9c1c..dd9b21d7 100644 --- a/schemas/tdd/schema.yaml +++ b/schemas/tdd/schema.yaml @@ -204,3 +204,10 @@ artifacts: Reference the spec for requirements, implementation for details. requires: - implementation + +apply: + requires: [tests] + tracks: null + instruction: | + Run tests to see failures. Implement minimal code to pass each test. + Refactor while keeping tests green. diff --git a/src/commands/artifact-workflow.ts b/src/commands/artifact-workflow.ts index bb0eea20..1debc5ea 100644 --- a/src/commands/artifact-workflow.ts +++ b/src/commands/artifact-workflow.ts @@ -44,12 +44,8 @@ interface TaskItem { interface ApplyInstructions { changeName: string; changeDir: string; - contextFiles: { - proposal?: string; - specs: string; - design?: string; - tasks: string; - }; + schemaName: string; + contextFiles: Record; progress: { total: number; complete: number; @@ -429,8 +425,72 @@ function parseTasksFile(content: string): TaskItem[] { return tasks; } +/** + * Checks if an artifact output exists in the change directory. + * Supports glob patterns (e.g., "specs/*.md") by verifying at least one matching file exists. + */ +function artifactOutputExists(changeDir: string, generates: string): boolean { + // Normalize the generates path to use platform-specific separators + const normalizedGenerates = generates.split('/').join(path.sep); + const fullPath = path.join(changeDir, normalizedGenerates); + + // If it's a glob pattern (contains ** or *), check for matching files + if (generates.includes('*')) { + // Extract the directory part before the glob pattern + const parts = normalizedGenerates.split(path.sep); + const dirParts: string[] = []; + let patternPart = ''; + for (const part of parts) { + if (part.includes('*')) { + patternPart = part; + break; + } + dirParts.push(part); + } + const dirPath = path.join(changeDir, ...dirParts); + + // Check if directory exists + if (!fs.existsSync(dirPath) || !fs.statSync(dirPath).isDirectory()) { + return false; + } + + // Extract expected extension from pattern (e.g., "*.md" -> ".md") + const extMatch = patternPart.match(/\*(\.[a-zA-Z0-9]+)$/); + const expectedExt = extMatch ? extMatch[1] : null; + + // Recursively check for matching files + const hasMatchingFiles = (dir: string): boolean => { + try { + const entries = fs.readdirSync(dir, { withFileTypes: true }); + for (const entry of entries) { + if (entry.isDirectory()) { + // For ** patterns, recurse into subdirectories + if (generates.includes('**') && hasMatchingFiles(path.join(dir, entry.name))) { + return true; + } + } else if (entry.isFile()) { + // Check if file matches expected extension (or any file if no extension specified) + if (!expectedExt || entry.name.endsWith(expectedExt)) { + return true; + } + } + } + } catch { + return false; + } + return false; + }; + + return hasMatchingFiles(dirPath); + } + + return fs.existsSync(fullPath); +} + /** * Generates apply instructions for implementing tasks from a change. + * Schema-aware: reads apply phase configuration from schema to determine + * required artifacts, tracking file, and instruction. */ async function generateApplyInstructions( projectRoot: string, @@ -441,39 +501,43 @@ async function generateApplyInstructions( const context = loadChangeContext(projectRoot, changeName, schemaName); const changeDir = path.join(projectRoot, 'openspec', 'changes', changeName); - // Check if required artifacts exist (tasks.md is the minimum requirement) - const tasksPath = path.join(changeDir, 'tasks.md'); - const proposalPath = path.join(changeDir, 'proposal.md'); - const designPath = path.join(changeDir, 'design.md'); - const specsPath = path.join(changeDir, 'specs'); + // Get the full schema to access the apply phase configuration + const schema = resolveSchema(context.schemaName); + const applyConfig = schema.apply; - const hasProposal = fs.existsSync(proposalPath); - const hasDesign = fs.existsSync(designPath); - const hasTasks = fs.existsSync(tasksPath); - const hasSpecs = fs.existsSync(specsPath); + // Determine required artifacts and tracking file from schema + // Fallback: if no apply block, require all artifacts + const requiredArtifactIds = applyConfig?.requires ?? schema.artifacts.map((a) => a.id); + const tracksFile = applyConfig?.tracks ?? null; + const schemaInstruction = applyConfig?.instruction ?? null; - // Determine state and missing artifacts + // Check which required artifacts are missing const missingArtifacts: string[] = []; - if (!hasTasks) { - // Check what's missing to create tasks (design is optional) - if (!hasProposal) missingArtifacts.push('proposal'); - if (!hasSpecs) missingArtifacts.push('specs'); - if (missingArtifacts.length === 0) missingArtifacts.push('tasks'); + for (const artifactId of requiredArtifactIds) { + const artifact = schema.artifacts.find((a) => a.id === artifactId); + if (artifact && !artifactOutputExists(changeDir, artifact.generates)) { + missingArtifacts.push(artifactId); + } } - // Build context files object - const contextFiles: ApplyInstructions['contextFiles'] = { - specs: path.join(changeDir, 'specs/**/*.md'), - tasks: tasksPath, - }; - if (hasProposal) contextFiles.proposal = proposalPath; - if (hasDesign) contextFiles.design = designPath; + // 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 file exists + // Parse tasks if tracking file exists let tasks: TaskItem[] = []; - if (hasTasks) { - const tasksContent = await fs.promises.readFile(tasksPath, 'utf-8'); - tasks = parseTasksFile(tasksContent); + let tracksFileExists = false; + if (tracksFile) { + const tracksPath = path.join(changeDir, tracksFile); + tracksFileExists = fs.existsSync(tracksPath); + if (tracksFileExists) { + const tasksContent = await fs.promises.readFile(tracksPath, 'utf-8'); + tasks = parseTasksFile(tasksContent); + } } // Calculate progress @@ -481,27 +545,39 @@ async function generateApplyInstructions( const complete = tasks.filter((t) => t.done).length; const remaining = total - complete; - // Determine state + // Determine state and instruction let state: ApplyInstructions['state']; let instruction: string; - if (!hasTasks || missingArtifacts.length > 0) { + if (missingArtifacts.length > 0) { state = 'blocked'; instruction = `Cannot apply this change yet. Missing artifacts: ${missingArtifacts.join(', ')}.\nUse the openspec-continue-change skill to create the missing artifacts first.`; - } else if (remaining === 0 && total > 0) { + } else if (tracksFile && !tracksFileExists) { + // Tracking file configured but doesn't exist yet + const tracksFilename = path.basename(tracksFile); + state = 'blocked'; + instruction = `The ${tracksFilename} file is missing and must be created.\nUse openspec-continue-change to generate the tracking file.`; + } else if (tracksFile && tracksFileExists && total === 0) { + // Tracking file exists but contains no tasks + const tracksFilename = path.basename(tracksFile); + state = 'blocked'; + instruction = `The ${tracksFilename} file exists but contains no tasks.\nAdd tasks to ${tracksFilename} or regenerate it with openspec-continue-change.`; + } else if (tracksFile && remaining === 0 && total > 0) { state = 'all_done'; instruction = 'All tasks are complete! This change is ready to be archived.\nConsider running tests and reviewing the changes before archiving.'; - } else if (total === 0) { - state = 'blocked'; - instruction = 'The tasks.md file exists but contains no tasks.\nAdd tasks to tasks.md or regenerate it with openspec-continue-change.'; + } else if (!tracksFile) { + // No tracking file (e.g., TDD schema) - ready to apply + state = 'ready'; + instruction = schemaInstruction?.trim() ?? 'All required artifacts complete. Proceed with implementation.'; } else { state = 'ready'; - instruction = 'Read context files, work through pending tasks, mark complete as you go.\nPause if you hit blockers or need clarification.'; + instruction = schemaInstruction?.trim() ?? 'Read context files, work through pending tasks, mark complete as you go.\nPause if you hit blockers or need clarification.'; } return { changeName, changeDir, + schemaName: context.schemaName, contextFiles, progress: { total, complete, remaining }, tasks, @@ -541,9 +617,10 @@ async function applyInstructionsCommand(options: ApplyInstructionsOptions): Prom } function printApplyInstructionsText(instructions: ApplyInstructions): void { - const { changeName, contextFiles, progress, tasks, state, missingArtifacts, instruction } = instructions; + const { changeName, schemaName, contextFiles, progress, tasks, state, missingArtifacts, instruction } = instructions; console.log(`## Apply: ${changeName}`); + console.log(`Schema: ${schemaName}`); console.log(); // Warning for blocked state @@ -555,26 +632,26 @@ function printApplyInstructionsText(instructions: ApplyInstructions): void { console.log(); } - // Context files - console.log('### Context Files'); - if (contextFiles.proposal) { - console.log(`- proposal: ${contextFiles.proposal}`); - } - console.log(`- specs: ${contextFiles.specs}`); - if (contextFiles.design) { - console.log(`- design: ${contextFiles.design}`); + // Context files (dynamically from schema) + 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(); } - console.log(`- tasks: ${contextFiles.tasks}`); - console.log(); - // Progress - console.log('### Progress'); - if (state === 'all_done') { - console.log(`${progress.complete}/${progress.total} complete ✓`); - } else { - console.log(`${progress.complete}/${progress.total} complete`); + // Progress (only show if we have tracking) + if (progress.total > 0 || tasks.length > 0) { + console.log('### Progress'); + if (state === 'all_done') { + console.log(`${progress.complete}/${progress.total} complete ✓`); + } else { + console.log(`${progress.complete}/${progress.total} complete`); + } + console.log(); } - console.log(); // Tasks if (tasks.length > 0) { diff --git a/src/core/artifact-graph/types.ts b/src/core/artifact-graph/types.ts index c5e2637c..fb0d1270 100644 --- a/src/core/artifact-graph/types.ts +++ b/src/core/artifact-graph/types.ts @@ -10,16 +10,29 @@ export const ArtifactSchema = z.object({ requires: z.array(z.string()).default([]), }); +// Apply phase configuration for schema-aware apply instructions +export const ApplyPhaseSchema = z.object({ + // Artifact IDs that must exist before apply is available + requires: z.array(z.string()).min(1, { error: 'At least one required artifact' }), + // Path to file with checkboxes for progress (relative to change dir), or null if no tracking + tracks: z.string().nullable().optional(), + // Custom guidance for the apply phase + instruction: z.string().optional(), +}); + // Full schema YAML structure export const SchemaYamlSchema = z.object({ name: z.string().min(1, { error: 'Schema name is required' }), version: z.number().int().positive({ error: 'Version must be a positive integer' }), description: z.string().optional(), artifacts: z.array(ArtifactSchema).min(1, { error: 'At least one artifact required' }), + // Optional apply phase configuration (for schema-aware apply instructions) + apply: ApplyPhaseSchema.optional(), }); // Derived TypeScript types export type Artifact = z.infer; +export type ApplyPhase = z.infer; export type SchemaYaml = z.infer; // Per-change metadata schema diff --git a/test/commands/artifact-workflow.test.ts b/test/commands/artifact-workflow.test.ts index 4e3e61b3..d502514f 100644 --- a/test/commands/artifact-workflow.test.ts +++ b/test/commands/artifact-workflow.test.ts @@ -348,6 +348,209 @@ describe('artifact-workflow CLI commands', () => { }); }); + describe('instructions apply command', () => { + it('shows apply instructions for spec-driven schema with tasks', async () => { + await createTestChange('apply-change', ['proposal', 'design', 'specs', 'tasks']); + + const result = await runCLI(['instructions', 'apply', '--change', 'apply-change'], { + cwd: tempDir, + }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('## Apply: apply-change'); + expect(result.stdout).toContain('Schema: spec-driven'); + expect(result.stdout).toContain('### Context Files'); + expect(result.stdout).toContain('### Instruction'); + }); + + it('shows blocked state when required artifacts are missing', async () => { + // Only create proposal - missing tasks (required by spec-driven apply block) + await createTestChange('blocked-apply', ['proposal']); + + const result = await runCLI(['instructions', 'apply', '--change', 'blocked-apply'], { + cwd: tempDir, + }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('Blocked'); + expect(result.stdout).toContain('Missing artifacts: tasks'); + }); + + it('outputs JSON for apply instructions', async () => { + await createTestChange('json-apply', ['proposal', 'design', 'specs', 'tasks']); + + const result = await runCLI( + ['instructions', 'apply', '--change', 'json-apply', '--json'], + { cwd: tempDir } + ); + expect(result.exitCode).toBe(0); + + const json = JSON.parse(result.stdout); + expect(json.changeName).toBe('json-apply'); + expect(json.schemaName).toBe('spec-driven'); + expect(json.state).toBe('ready'); + expect(json.contextFiles).toBeDefined(); + expect(typeof json.contextFiles).toBe('object'); + }); + + it('shows schema instruction from apply block', async () => { + await createTestChange('instr-apply', ['proposal', 'design', 'specs', 'tasks']); + + const result = await runCLI(['instructions', 'apply', '--change', 'instr-apply'], { + cwd: tempDir, + }); + expect(result.exitCode).toBe(0); + // Should show the instruction from spec-driven schema apply block + expect(result.stdout).toContain('work through pending tasks'); + }); + + it('shows all_done state when all tasks are complete', async () => { + const changeDir = await createTestChange('done-apply', [ + 'proposal', + 'design', + 'specs', + 'tasks', + ]); + // Overwrite tasks with all completed + await fs.writeFile( + path.join(changeDir, 'tasks.md'), + '## Tasks\n- [x] Task 1\n- [x] Task 2' + ); + + const result = await runCLI(['instructions', 'apply', '--change', 'done-apply'], { + cwd: tempDir, + }); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('complete ✓'); + expect(result.stdout).toContain('ready to be archived'); + }); + + it('uses tdd schema apply configuration', async () => { + // Create a TDD-style change with spec and tests + const changeDir = path.join(changesDir, 'tdd-apply'); + await fs.mkdir(changeDir, { recursive: true }); + await fs.writeFile(path.join(changeDir, 'spec.md'), '## Feature\nTest spec.'); + const testsDir = path.join(changeDir, 'tests'); + await fs.mkdir(testsDir, { recursive: true }); + await fs.writeFile(path.join(testsDir, 'test.test.ts'), 'test("works", () => {})'); + + const result = await runCLI( + ['instructions', 'apply', '--change', 'tdd-apply', '--schema', 'tdd'], + { cwd: tempDir } + ); + expect(result.exitCode).toBe(0); + expect(result.stdout).toContain('Schema: tdd'); + // TDD schema has no task tracking, so should show schema instruction + expect(result.stdout).toContain('Run tests to see failures'); + }); + + it('spec-driven schema uses apply block configuration', async () => { + // Verify that spec-driven schema uses its apply block (requires: [tasks]) + await createTestChange('apply-config-test', ['proposal', 'design', 'specs', 'tasks']); + + const result = await runCLI( + ['instructions', 'apply', '--change', 'apply-config-test', '--json'], + { cwd: tempDir } + ); + expect(result.exitCode).toBe(0); + + const json = JSON.parse(result.stdout); + // spec-driven schema has apply block with requires: [tasks], so should be ready + expect(json.schemaName).toBe('spec-driven'); + expect(json.state).toBe('ready'); + }); + + it('fallback: requires all artifacts when schema has no apply block', async () => { + // Create a minimal schema without an apply block in user schemas dir + const userDataDir = path.join(tempDir, 'user-data'); + const noApplySchemaDir = path.join(userDataDir, 'openspec', 'schemas', 'no-apply'); + const templatesDir = path.join(noApplySchemaDir, 'templates'); + await fs.mkdir(templatesDir, { recursive: true }); + + // Minimal schema with 2 artifacts, no apply block + const schemaContent = ` +name: no-apply +version: 1 +description: Test schema without apply block +artifacts: + - id: first + generates: first.md + description: First artifact + template: first.md + requires: [] + - id: second + generates: second.md + description: Second artifact + template: second.md + requires: [first] +`; + await fs.writeFile(path.join(noApplySchemaDir, 'schema.yaml'), schemaContent); + await fs.writeFile(path.join(templatesDir, 'first.md'), '# First\n'); + await fs.writeFile(path.join(templatesDir, 'second.md'), '# Second\n'); + + // Create a change with only the first artifact (missing second) + const changeDir = path.join(changesDir, 'no-apply-test'); + await fs.mkdir(changeDir, { recursive: true }); + await fs.writeFile(path.join(changeDir, 'first.md'), '# First artifact content'); + + // Run with XDG_DATA_HOME pointing to our temp user data dir + const result = await runCLI( + ['instructions', 'apply', '--change', 'no-apply-test', '--schema', 'no-apply', '--json'], + { + cwd: tempDir, + env: { XDG_DATA_HOME: userDataDir }, + } + ); + expect(result.exitCode).toBe(0); + + const json = JSON.parse(result.stdout); + // Without apply block, fallback requires ALL artifacts - second is missing + expect(json.schemaName).toBe('no-apply'); + expect(json.state).toBe('blocked'); + expect(json.missingArtifacts).toContain('second'); + }); + + it('fallback: ready when all artifacts exist for schema without apply block', async () => { + // Create a minimal schema without an apply block + const userDataDir = path.join(tempDir, 'user-data-2'); + const noApplySchemaDir = path.join(userDataDir, 'openspec', 'schemas', 'no-apply-full'); + const templatesDir = path.join(noApplySchemaDir, 'templates'); + await fs.mkdir(templatesDir, { recursive: true }); + + const schemaContent = ` +name: no-apply-full +version: 1 +description: Test schema without apply block +artifacts: + - id: only + generates: only.md + description: Only artifact + template: only.md + requires: [] +`; + await fs.writeFile(path.join(noApplySchemaDir, 'schema.yaml'), schemaContent); + await fs.writeFile(path.join(templatesDir, 'only.md'), '# Only\n'); + + // Create a change with the artifact present + const changeDir = path.join(changesDir, 'no-apply-full-test'); + await fs.mkdir(changeDir, { recursive: true }); + await fs.writeFile(path.join(changeDir, 'only.md'), '# Content'); + + const result = await runCLI( + ['instructions', 'apply', '--change', 'no-apply-full-test', '--schema', 'no-apply-full', '--json'], + { + cwd: tempDir, + env: { XDG_DATA_HOME: userDataDir }, + } + ); + expect(result.exitCode).toBe(0); + + const json = JSON.parse(result.stdout); + // All artifacts exist, should be ready with default instruction + expect(json.schemaName).toBe('no-apply-full'); + expect(json.state).toBe('ready'); + expect(json.instruction).toContain('All required artifacts complete'); + }); + }); + describe('help text', () => { it('marks status command as experimental in help', async () => { const result = await runCLI(['status', '--help']);