Skip to content

Commit a2b965a

Browse files
authored
fix(workflow): keep no-spec schema changes valid (#1655)
* fix(workflow): scaffold valid no-spec changes * fix(workflow): normalize specs artifact paths
1 parent 98c7932 commit a2b965a

5 files changed

Lines changed: 145 additions & 8 deletions

File tree

src/core/artifact-graph/instruction-loader.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@ import * as path from 'node:path';
33
import { getSchemaDir, resolveSchema, listSchemasWithInfo } from './resolver.js';
44
import { ArtifactGraph } from './graph.js';
55
import { detectCompleted } from './state.js';
6-
import { resolveArtifactOutputPath, resolveArtifactOutputs } from './outputs.js';
6+
import {
7+
isSpecsArtifactPath,
8+
resolveArtifactOutputPath,
9+
resolveArtifactOutputs,
10+
} from './outputs.js';
711
import { readChangeMetadata, resolveSchemaForChange } from '../../utils/change-metadata.js';
812
import { FileSystemUtils } from '../../utils/file-system.js';
913
import {
@@ -285,12 +289,7 @@ export function loadChangeContext(
285289
const skippedArtifacts = new Set<string>();
286290
if (metadata?.skip_specs) {
287291
for (const artifact of graph.getAllArtifacts()) {
288-
// A schema may write generates as './specs/...' - the globs treat that
289-
// identically to 'specs/...', so the skip set must too, or validate
290-
// would honor the marker while instructions tell the agent to create
291-
// the very files the conflict gate polices.
292-
const generates = artifact.generates.replace(/^(?:\.\/)+/, '');
293-
if (generates.startsWith('specs/') && !completed.has(artifact.id)) {
292+
if (isSpecsArtifactPath(artifact.generates) && !completed.has(artifact.id)) {
294293
completed.add(artifact.id);
295294
skippedArtifacts.add(artifact.id);
296295
}

src/core/artifact-graph/outputs.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,14 @@ export function isGlobPattern(pattern: string): boolean {
1010
return pattern.includes('*') || pattern.includes('?') || pattern.includes('[');
1111
}
1212

13+
/**
14+
* Returns whether an artifact generates files under the change's specs/ tree.
15+
*/
16+
export function isSpecsArtifactPath(generates: string): boolean {
17+
const normalized = path.posix.normalize(FileSystemUtils.toPosixPath(generates));
18+
return normalized.startsWith('specs/');
19+
}
20+
1321
export function resolveArtifactOutputPath(changeDir: string, generates: string): string {
1422
const outputPath = path.join(changeDir, generates);
1523
FileSystemUtils.assertPathWithin(changeDir, outputPath);

src/utils/change-utils.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { writeChangeMetadata, validateSchemaName } from './change-metadata.js';
44
import { formatLocalDate } from './date.js';
55
import { readProjectConfig } from '../core/project-config.js';
66
import { isKebabId } from '../core/id.js';
7+
import { resolveSchema } from '../core/artifact-graph/resolver.js';
8+
import { isSpecsArtifactPath } from '../core/artifact-graph/outputs.js';
79
import type { ChangeMetadata } from '../core/change-metadata/index.js';
810

911
const DEFAULT_SCHEMA = 'spec-driven';
@@ -165,6 +167,11 @@ export async function createChange(
165167
throw new Error(`Change '${name}' already exists at ${changeDir}`);
166168
}
167169

170+
const schema = resolveSchema(schemaName, projectRoot);
171+
const skipsSpecs = !schema.artifacts.some(artifact =>
172+
isSpecsArtifactPath(artifact.generates)
173+
);
174+
168175
// Creating a change may scaffold or complete the root itself (an
169176
// implicit root, or a config-only/incomplete clone). Never leave a
170177
// half-root behind that doctor immediately calls unhealthy: ensure
@@ -190,6 +197,7 @@ export async function createChange(
190197
writeChangeMetadata(changeDir, {
191198
schema: schemaName,
192199
created: formatLocalDate(),
200+
...(skipsSpecs ? { skip_specs: true } : {}),
193201
...options.metadata,
194202
}, projectRoot);
195203

test/commands/artifact-workflow.test.ts

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,112 @@ describe('artifact-workflow CLI commands', () => {
444444
const changeDir = path.join(changesDir, 'my-new-feature');
445445
const stat = await fs.stat(changeDir);
446446
expect(stat.isDirectory()).toBe(true);
447+
448+
const metadata = await fs.readFile(path.join(changeDir, '.openspec.yaml'), 'utf-8');
449+
expect(metadata).not.toContain('skip_specs');
450+
});
451+
452+
it('marks changes as skip_specs when their schema cannot generate specs', async () => {
453+
const schemaDir = path.join(tempDir, 'openspec', 'schemas', 'no-specs');
454+
await fs.mkdir(path.join(schemaDir, 'templates'), { recursive: true });
455+
await fs.writeFile(
456+
path.join(schemaDir, 'schema.yaml'),
457+
`name: no-specs
458+
version: 1
459+
artifacts:
460+
- id: proposal
461+
generates: proposal.md
462+
description: Proposal
463+
template: proposal.md
464+
requires: []
465+
- id: tasks
466+
generates: tasks.md
467+
description: Tasks
468+
template: tasks.md
469+
requires: [proposal]
470+
apply:
471+
requires: [tasks]
472+
tracks: tasks.md
473+
`
474+
);
475+
await fs.writeFile(path.join(schemaDir, 'templates', 'proposal.md'), '# Proposal\n');
476+
await fs.writeFile(path.join(schemaDir, 'templates', 'tasks.md'), '# Tasks\n');
477+
await fs.writeFile(
478+
path.join(tempDir, 'openspec', 'config.yaml'),
479+
'schema: no-specs\n'
480+
);
481+
482+
const result = await runCLI(['new', 'change', 'no-spec-change'], { cwd: tempDir });
483+
expect(result.exitCode).toBe(0);
484+
485+
const metadata = await fs.readFile(
486+
path.join(changesDir, 'no-spec-change', '.openspec.yaml'),
487+
'utf-8'
488+
);
489+
expect(metadata).toContain('skip_specs: true');
490+
491+
const validation = await runCLI(
492+
['validate', 'no-spec-change', '--type', 'change'],
493+
{ cwd: tempDir }
494+
);
495+
expect(validation.exitCode).toBe(0);
496+
});
497+
498+
it('does not mark spec-producing schemas that use Windows separators', async () => {
499+
const schemaName = 'windows-specs';
500+
const generates = String.raw`specs\**\*.md`;
501+
const schemaDir = path.join(tempDir, 'openspec', 'schemas', schemaName);
502+
await fs.mkdir(path.join(schemaDir, 'templates'), { recursive: true });
503+
await fs.writeFile(
504+
path.join(schemaDir, 'schema.yaml'),
505+
`name: ${schemaName}
506+
version: 1
507+
artifacts:
508+
- id: specs
509+
generates: '${generates}'
510+
description: Specs
511+
template: spec.md
512+
requires: []
513+
`
514+
);
515+
await fs.writeFile(path.join(schemaDir, 'templates', 'spec.md'), '# Spec\n');
516+
await fs.writeFile(
517+
path.join(tempDir, 'openspec', 'config.yaml'),
518+
`schema: ${schemaName}\n`
519+
);
520+
521+
const changeName = `${schemaName}-change`;
522+
const result = await runCLI(['new', 'change', changeName], { cwd: tempDir });
523+
expect(result.exitCode).toBe(0);
524+
525+
const changeDir = path.join(changesDir, changeName);
526+
const metadata = await fs.readFile(path.join(changeDir, '.openspec.yaml'), 'utf-8');
527+
expect(metadata).not.toContain('skip_specs');
528+
529+
const specDir = path.join(changeDir, 'specs', 'example');
530+
await fs.mkdir(specDir, { recursive: true });
531+
await fs.writeFile(
532+
path.join(specDir, 'spec.md'),
533+
`## ADDED Requirements
534+
### Requirement: Example behavior
535+
The system SHALL support the example behavior.
536+
537+
#### Scenario: Example succeeds
538+
- **WHEN** the example runs
539+
- **THEN** it succeeds
540+
`
541+
);
542+
543+
const status = await runCLI(['status', '--change', changeName, '--json'], {
544+
cwd: tempDir,
545+
});
546+
expect(status.exitCode).toBe(0);
547+
expect(JSON.parse(status.stdout).artifacts[0].status).toBe('done');
548+
549+
const validation = await runCLI(['validate', changeName, '--type', 'change'], {
550+
cwd: tempDir,
551+
});
552+
expect(validation.exitCode).toBe(0);
447553
});
448554

449555
it('rejects --initiative and writes no change', async () => {

test/core/artifact-graph/outputs.test.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@ import * as fs from 'node:fs';
33
import * as path from 'node:path';
44
import * as os from 'node:os';
55
import { FileSystemUtils } from '../../../src/utils/file-system.js';
6-
import { artifactOutputExists, resolveArtifactOutputs } from '../../../src/core/artifact-graph/outputs.js';
6+
import {
7+
artifactOutputExists,
8+
isSpecsArtifactPath,
9+
resolveArtifactOutputs,
10+
} from '../../../src/core/artifact-graph/outputs.js';
711

812
describe('artifact-graph/outputs', () => {
913
let tempDir: string;
@@ -18,6 +22,18 @@ describe('artifact-graph/outputs', () => {
1822
fs.rmSync(tempDir, { recursive: true, force: true });
1923
});
2024

25+
it.each([
26+
['specs/**/*.md', true],
27+
['./specs/**/*.md', true],
28+
['.//specs/**/*.md', true],
29+
[String.raw`specs\**\*.md`, true],
30+
[String.raw`.\specs\**\*.md`, true],
31+
['docs/specs/**/*.md', false],
32+
['specs-note.md', false],
33+
])('classifies specs artifact path %s', (generates, expected) => {
34+
expect(isSpecsArtifactPath(generates)).toBe(expected);
35+
});
36+
2137
it('resolves a direct file path when it exists', () => {
2238
const filePath = path.join(tempDir, 'proposal.md');
2339
fs.writeFileSync(filePath, 'content');

0 commit comments

Comments
 (0)