Skip to content

Commit d737057

Browse files
authored
feat: make apply instructions schema-aware (#444)
* feat: make apply instructions schema-aware The `generateApplyInstructions` function was hardcoded to check for `spec-driven` artifacts. This change makes it read artifact definitions from the schema's new `apply` block, enabling support for different workflows like TDD. Changes: - Add `ApplyPhaseSchema` Zod schema with `requires`, `tracks`, and `instruction` fields to types.ts - Update `SchemaYamlSchema` to include optional `apply` field - Add `apply` block to spec-driven and tdd schemas - Refactor `generateApplyInstructions` to: - Load schema via `resolveSchema()` - Read `apply.requires` for required artifacts - Check artifact existence dynamically (supports glob patterns) - Use `apply.tracks` for progress tracking (or skip if null) - Use `apply.instruction` for custom guidance - Build `contextFiles` from all existing artifacts in schema - Handle fallback when schema has no `apply` block (require all artifacts) - Add 8 new tests for schema-aware apply behavior * fix: improve apply instructions robustness and consistency - Fix artifactOutputExists to properly handle glob patterns cross-platform by using path.sep and verifying actual file matches - Remove redundant if/else in contextFiles loop - Distinguish between missing tracks file vs empty tracks file - Use consistent { error: } key in ApplyPhaseSchema validation - Rename fallback test to accurately describe what it tests * test: add fallback behavior tests for schemas without apply block Add two tests that verify the fallback logic when a schema lacks an apply block: - Blocks when not all artifacts exist (requires ALL artifacts) - Ready state with default instruction when all artifacts exist Uses XDG_DATA_HOME to create temporary user schemas for testing.
1 parent ed924ff commit d737057

6 files changed

Lines changed: 382 additions & 75 deletions

File tree

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,35 @@
11
## Prerequisites
22

3-
- [ ] 0.1 Implement `add-per-change-schema-metadata` first (to auto-detect schema)
3+
- [x] 0.1 Implement `add-per-change-schema-metadata` first (to auto-detect schema)
44

55
## 1. Schema Format
66

7-
- [ ] 1.1 Add `ApplyPhaseSchema` Zod schema to `src/core/artifact-graph/types.ts`
8-
- [ ] 1.2 Update `SchemaYamlSchema` to include optional `apply` field
9-
- [ ] 1.3 Export `ApplyPhase` type
7+
- [x] 1.1 Add `ApplyPhaseSchema` Zod schema to `src/core/artifact-graph/types.ts`
8+
- [x] 1.2 Update `SchemaYamlSchema` to include optional `apply` field
9+
- [x] 1.3 Export `ApplyPhase` type
1010

1111
## 2. Update Existing Schemas
1212

13-
- [ ] 2.1 Add `apply` block to `schemas/spec-driven/schema.yaml`
14-
- [ ] 2.2 Add `apply` block to `schemas/tdd/schema.yaml`
13+
- [x] 2.1 Add `apply` block to `schemas/spec-driven/schema.yaml`
14+
- [x] 2.2 Add `apply` block to `schemas/tdd/schema.yaml`
1515

1616
## 3. Refactor generateApplyInstructions
1717

18-
- [ ] 3.1 Load schema via `resolveSchema(schemaName)`
19-
- [ ] 3.2 Read `apply.requires` to determine required artifacts
20-
- [ ] 3.3 Check artifact existence dynamically (not hardcoded paths)
21-
- [ ] 3.4 Use `apply.tracks` for progress tracking (or skip if null)
22-
- [ ] 3.5 Use `apply.instruction` for the instruction text
23-
- [ ] 3.6 Build `contextFiles` from all existing artifacts in schema
18+
- [x] 3.1 Load schema via `resolveSchema(schemaName)`
19+
- [x] 3.2 Read `apply.requires` to determine required artifacts
20+
- [x] 3.3 Check artifact existence dynamically (not hardcoded paths)
21+
- [x] 3.4 Use `apply.tracks` for progress tracking (or skip if null)
22+
- [x] 3.5 Use `apply.instruction` for the instruction text
23+
- [x] 3.6 Build `contextFiles` from all existing artifacts in schema
2424

2525
## 4. Handle Fallback
2626

27-
- [ ] 4.1 If schema has no `apply` block, require all artifacts to exist
28-
- [ ] 4.2 Default instruction: "All artifacts complete. Proceed with implementation."
27+
- [x] 4.1 If schema has no `apply` block, require all artifacts to exist
28+
- [x] 4.2 Default instruction: "All artifacts complete. Proceed with implementation."
2929

3030
## 5. Tests
3131

32-
- [ ] 5.1 Test apply instructions with spec-driven schema
33-
- [ ] 5.2 Test apply instructions with tdd schema
34-
- [ ] 5.3 Test fallback when schema has no apply block
35-
- [ ] 5.4 Test blocked state when required artifacts missing
32+
- [x] 5.1 Test apply instructions with spec-driven schema
33+
- [x] 5.2 Test apply instructions with tdd schema
34+
- [x] 5.3 Test fallback when schema has no apply block
35+
- [x] 5.4 Test blocked state when required artifacts missing

schemas/spec-driven/schema.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,3 +139,10 @@ artifacts:
139139
requires:
140140
- specs
141141
- design
142+
143+
apply:
144+
requires: [tasks]
145+
tracks: tasks.md
146+
instruction: |
147+
Read context files, work through pending tasks, mark complete as you go.
148+
Pause if you hit blockers or need clarification.

schemas/tdd/schema.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,3 +204,10 @@ artifacts:
204204
Reference the spec for requirements, implementation for details.
205205
requires:
206206
- implementation
207+
208+
apply:
209+
requires: [tests]
210+
tracks: null
211+
instruction: |
212+
Run tests to see failures. Implement minimal code to pass each test.
213+
Refactor while keeping tests green.

src/commands/artifact-workflow.ts

Lines changed: 134 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -44,12 +44,8 @@ interface TaskItem {
4444
interface ApplyInstructions {
4545
changeName: string;
4646
changeDir: string;
47-
contextFiles: {
48-
proposal?: string;
49-
specs: string;
50-
design?: string;
51-
tasks: string;
52-
};
47+
schemaName: string;
48+
contextFiles: Record<string, string>;
5349
progress: {
5450
total: number;
5551
complete: number;
@@ -429,8 +425,72 @@ function parseTasksFile(content: string): TaskItem[] {
429425
return tasks;
430426
}
431427

428+
/**
429+
* Checks if an artifact output exists in the change directory.
430+
* Supports glob patterns (e.g., "specs/*.md") by verifying at least one matching file exists.
431+
*/
432+
function artifactOutputExists(changeDir: string, generates: string): boolean {
433+
// Normalize the generates path to use platform-specific separators
434+
const normalizedGenerates = generates.split('/').join(path.sep);
435+
const fullPath = path.join(changeDir, normalizedGenerates);
436+
437+
// If it's a glob pattern (contains ** or *), check for matching files
438+
if (generates.includes('*')) {
439+
// Extract the directory part before the glob pattern
440+
const parts = normalizedGenerates.split(path.sep);
441+
const dirParts: string[] = [];
442+
let patternPart = '';
443+
for (const part of parts) {
444+
if (part.includes('*')) {
445+
patternPart = part;
446+
break;
447+
}
448+
dirParts.push(part);
449+
}
450+
const dirPath = path.join(changeDir, ...dirParts);
451+
452+
// Check if directory exists
453+
if (!fs.existsSync(dirPath) || !fs.statSync(dirPath).isDirectory()) {
454+
return false;
455+
}
456+
457+
// Extract expected extension from pattern (e.g., "*.md" -> ".md")
458+
const extMatch = patternPart.match(/\*(\.[a-zA-Z0-9]+)$/);
459+
const expectedExt = extMatch ? extMatch[1] : null;
460+
461+
// Recursively check for matching files
462+
const hasMatchingFiles = (dir: string): boolean => {
463+
try {
464+
const entries = fs.readdirSync(dir, { withFileTypes: true });
465+
for (const entry of entries) {
466+
if (entry.isDirectory()) {
467+
// For ** patterns, recurse into subdirectories
468+
if (generates.includes('**') && hasMatchingFiles(path.join(dir, entry.name))) {
469+
return true;
470+
}
471+
} else if (entry.isFile()) {
472+
// Check if file matches expected extension (or any file if no extension specified)
473+
if (!expectedExt || entry.name.endsWith(expectedExt)) {
474+
return true;
475+
}
476+
}
477+
}
478+
} catch {
479+
return false;
480+
}
481+
return false;
482+
};
483+
484+
return hasMatchingFiles(dirPath);
485+
}
486+
487+
return fs.existsSync(fullPath);
488+
}
489+
432490
/**
433491
* Generates apply instructions for implementing tasks from a change.
492+
* Schema-aware: reads apply phase configuration from schema to determine
493+
* required artifacts, tracking file, and instruction.
434494
*/
435495
async function generateApplyInstructions(
436496
projectRoot: string,
@@ -441,67 +501,83 @@ async function generateApplyInstructions(
441501
const context = loadChangeContext(projectRoot, changeName, schemaName);
442502
const changeDir = path.join(projectRoot, 'openspec', 'changes', changeName);
443503

444-
// Check if required artifacts exist (tasks.md is the minimum requirement)
445-
const tasksPath = path.join(changeDir, 'tasks.md');
446-
const proposalPath = path.join(changeDir, 'proposal.md');
447-
const designPath = path.join(changeDir, 'design.md');
448-
const specsPath = path.join(changeDir, 'specs');
504+
// Get the full schema to access the apply phase configuration
505+
const schema = resolveSchema(context.schemaName);
506+
const applyConfig = schema.apply;
449507

450-
const hasProposal = fs.existsSync(proposalPath);
451-
const hasDesign = fs.existsSync(designPath);
452-
const hasTasks = fs.existsSync(tasksPath);
453-
const hasSpecs = fs.existsSync(specsPath);
508+
// Determine required artifacts and tracking file from schema
509+
// Fallback: if no apply block, require all artifacts
510+
const requiredArtifactIds = applyConfig?.requires ?? schema.artifacts.map((a) => a.id);
511+
const tracksFile = applyConfig?.tracks ?? null;
512+
const schemaInstruction = applyConfig?.instruction ?? null;
454513

455-
// Determine state and missing artifacts
514+
// Check which required artifacts are missing
456515
const missingArtifacts: string[] = [];
457-
if (!hasTasks) {
458-
// Check what's missing to create tasks (design is optional)
459-
if (!hasProposal) missingArtifacts.push('proposal');
460-
if (!hasSpecs) missingArtifacts.push('specs');
461-
if (missingArtifacts.length === 0) missingArtifacts.push('tasks');
516+
for (const artifactId of requiredArtifactIds) {
517+
const artifact = schema.artifacts.find((a) => a.id === artifactId);
518+
if (artifact && !artifactOutputExists(changeDir, artifact.generates)) {
519+
missingArtifacts.push(artifactId);
520+
}
462521
}
463522

464-
// Build context files object
465-
const contextFiles: ApplyInstructions['contextFiles'] = {
466-
specs: path.join(changeDir, 'specs/**/*.md'),
467-
tasks: tasksPath,
468-
};
469-
if (hasProposal) contextFiles.proposal = proposalPath;
470-
if (hasDesign) contextFiles.design = designPath;
523+
// Build context files from all existing artifacts in schema
524+
const contextFiles: Record<string, string> = {};
525+
for (const artifact of schema.artifacts) {
526+
if (artifactOutputExists(changeDir, artifact.generates)) {
527+
contextFiles[artifact.id] = path.join(changeDir, artifact.generates);
528+
}
529+
}
471530

472-
// Parse tasks if file exists
531+
// Parse tasks if tracking file exists
473532
let tasks: TaskItem[] = [];
474-
if (hasTasks) {
475-
const tasksContent = await fs.promises.readFile(tasksPath, 'utf-8');
476-
tasks = parseTasksFile(tasksContent);
533+
let tracksFileExists = false;
534+
if (tracksFile) {
535+
const tracksPath = path.join(changeDir, tracksFile);
536+
tracksFileExists = fs.existsSync(tracksPath);
537+
if (tracksFileExists) {
538+
const tasksContent = await fs.promises.readFile(tracksPath, 'utf-8');
539+
tasks = parseTasksFile(tasksContent);
540+
}
477541
}
478542

479543
// Calculate progress
480544
const total = tasks.length;
481545
const complete = tasks.filter((t) => t.done).length;
482546
const remaining = total - complete;
483547

484-
// Determine state
548+
// Determine state and instruction
485549
let state: ApplyInstructions['state'];
486550
let instruction: string;
487551

488-
if (!hasTasks || missingArtifacts.length > 0) {
552+
if (missingArtifacts.length > 0) {
489553
state = 'blocked';
490554
instruction = `Cannot apply this change yet. Missing artifacts: ${missingArtifacts.join(', ')}.\nUse the openspec-continue-change skill to create the missing artifacts first.`;
491-
} else if (remaining === 0 && total > 0) {
555+
} else if (tracksFile && !tracksFileExists) {
556+
// Tracking file configured but doesn't exist yet
557+
const tracksFilename = path.basename(tracksFile);
558+
state = 'blocked';
559+
instruction = `The ${tracksFilename} file is missing and must be created.\nUse openspec-continue-change to generate the tracking file.`;
560+
} else if (tracksFile && tracksFileExists && total === 0) {
561+
// Tracking file exists but contains no tasks
562+
const tracksFilename = path.basename(tracksFile);
563+
state = 'blocked';
564+
instruction = `The ${tracksFilename} file exists but contains no tasks.\nAdd tasks to ${tracksFilename} or regenerate it with openspec-continue-change.`;
565+
} else if (tracksFile && remaining === 0 && total > 0) {
492566
state = 'all_done';
493567
instruction = 'All tasks are complete! This change is ready to be archived.\nConsider running tests and reviewing the changes before archiving.';
494-
} else if (total === 0) {
495-
state = 'blocked';
496-
instruction = 'The tasks.md file exists but contains no tasks.\nAdd tasks to tasks.md or regenerate it with openspec-continue-change.';
568+
} else if (!tracksFile) {
569+
// No tracking file (e.g., TDD schema) - ready to apply
570+
state = 'ready';
571+
instruction = schemaInstruction?.trim() ?? 'All required artifacts complete. Proceed with implementation.';
497572
} else {
498573
state = 'ready';
499-
instruction = 'Read context files, work through pending tasks, mark complete as you go.\nPause if you hit blockers or need clarification.';
574+
instruction = schemaInstruction?.trim() ?? 'Read context files, work through pending tasks, mark complete as you go.\nPause if you hit blockers or need clarification.';
500575
}
501576

502577
return {
503578
changeName,
504579
changeDir,
580+
schemaName: context.schemaName,
505581
contextFiles,
506582
progress: { total, complete, remaining },
507583
tasks,
@@ -541,9 +617,10 @@ async function applyInstructionsCommand(options: ApplyInstructionsOptions): Prom
541617
}
542618

543619
function printApplyInstructionsText(instructions: ApplyInstructions): void {
544-
const { changeName, contextFiles, progress, tasks, state, missingArtifacts, instruction } = instructions;
620+
const { changeName, schemaName, contextFiles, progress, tasks, state, missingArtifacts, instruction } = instructions;
545621

546622
console.log(`## Apply: ${changeName}`);
623+
console.log(`Schema: ${schemaName}`);
547624
console.log();
548625

549626
// Warning for blocked state
@@ -555,26 +632,26 @@ function printApplyInstructionsText(instructions: ApplyInstructions): void {
555632
console.log();
556633
}
557634

558-
// Context files
559-
console.log('### Context Files');
560-
if (contextFiles.proposal) {
561-
console.log(`- proposal: ${contextFiles.proposal}`);
562-
}
563-
console.log(`- specs: ${contextFiles.specs}`);
564-
if (contextFiles.design) {
565-
console.log(`- design: ${contextFiles.design}`);
635+
// Context files (dynamically from schema)
636+
const contextFileEntries = Object.entries(contextFiles);
637+
if (contextFileEntries.length > 0) {
638+
console.log('### Context Files');
639+
for (const [artifactId, filePath] of contextFileEntries) {
640+
console.log(`- ${artifactId}: ${filePath}`);
641+
}
642+
console.log();
566643
}
567-
console.log(`- tasks: ${contextFiles.tasks}`);
568-
console.log();
569644

570-
// Progress
571-
console.log('### Progress');
572-
if (state === 'all_done') {
573-
console.log(`${progress.complete}/${progress.total} complete ✓`);
574-
} else {
575-
console.log(`${progress.complete}/${progress.total} complete`);
645+
// Progress (only show if we have tracking)
646+
if (progress.total > 0 || tasks.length > 0) {
647+
console.log('### Progress');
648+
if (state === 'all_done') {
649+
console.log(`${progress.complete}/${progress.total} complete ✓`);
650+
} else {
651+
console.log(`${progress.complete}/${progress.total} complete`);
652+
}
653+
console.log();
576654
}
577-
console.log();
578655

579656
// Tasks
580657
if (tasks.length > 0) {

src/core/artifact-graph/types.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,29 @@ export const ArtifactSchema = z.object({
1010
requires: z.array(z.string()).default([]),
1111
});
1212

13+
// Apply phase configuration for schema-aware apply instructions
14+
export const ApplyPhaseSchema = z.object({
15+
// Artifact IDs that must exist before apply is available
16+
requires: z.array(z.string()).min(1, { error: 'At least one required artifact' }),
17+
// Path to file with checkboxes for progress (relative to change dir), or null if no tracking
18+
tracks: z.string().nullable().optional(),
19+
// Custom guidance for the apply phase
20+
instruction: z.string().optional(),
21+
});
22+
1323
// Full schema YAML structure
1424
export const SchemaYamlSchema = z.object({
1525
name: z.string().min(1, { error: 'Schema name is required' }),
1626
version: z.number().int().positive({ error: 'Version must be a positive integer' }),
1727
description: z.string().optional(),
1828
artifacts: z.array(ArtifactSchema).min(1, { error: 'At least one artifact required' }),
29+
// Optional apply phase configuration (for schema-aware apply instructions)
30+
apply: ApplyPhaseSchema.optional(),
1931
});
2032

2133
// Derived TypeScript types
2234
export type Artifact = z.infer<typeof ArtifactSchema>;
35+
export type ApplyPhase = z.infer<typeof ApplyPhaseSchema>;
2336
export type SchemaYaml = z.infer<typeof SchemaYamlSchema>;
2437

2538
// Per-change metadata schema

0 commit comments

Comments
 (0)