Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/validate-artifact-content.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@fission-ai/openspec": patch
---

### Bug Fixes

- Fixed workflow resumes so empty, whitespace-only, or HTML-comment-only artifact files are regenerated instead of being treated as completed. Heading-only or other partially-written artifacts are not yet detected and may still be treated as complete.
32 changes: 32 additions & 0 deletions src/core/artifact-graph/outputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,35 @@ export function resolveArtifactOutputs(changeDir: string, generates: string): st
export function artifactOutputExists(changeDir: string, generates: string): boolean {
return resolveArtifactOutputs(changeDir, generates).length > 0;
}

/**
* Checks if all resolved artifact output files contain meaningful content.
*/
export function artifactOutputContentValid(changeDir: string, generates: string): boolean {
const outputs = resolveArtifactOutputs(changeDir, generates);

return outputs.length > 0 && outputs.every(isArtifactOutputFileContentValid);
}

/**
* Checks if an artifact has resolved output files and each contains meaningful content.
* Resolves outputs once and reuses the result for both checks.
*/
export function artifactOutputComplete(changeDir: string, generates: string): boolean {
const outputs = resolveArtifactOutputs(changeDir, generates);

return outputs.length > 0 && outputs.every(isArtifactOutputFileContentValid);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

function isArtifactOutputFileContentValid(filePath: string): boolean {
try {
const content = fs.readFileSync(filePath, 'utf8');
const withoutHtmlComments = content.replace(/<!--[\s\S]*?(-->|$)/g, '');

return withoutHtmlComments
.split(/\r?\n/)
.some((line) => line.trim().length > 0);
} catch {
return false;
}
}
10 changes: 5 additions & 5 deletions src/core/artifact-graph/state.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import * as fs from 'node:fs';
import type { CompletedSet } from './types.js';
import type { ArtifactGraph } from './graph.js';
import { artifactOutputExists } from './outputs.js';
import { artifactOutputComplete } from './outputs.js';

/**
* Detects which artifacts are completed by checking file existence in the change directory.
* Detects which artifacts are completed by checking output files in the change directory.
* Returns a Set of completed artifact IDs.
*
* @param graph - The artifact graph to check
* @param changeDir - The change directory to scan for files
* @returns Set of artifact IDs whose generated files exist
* @returns Set of artifact IDs whose generated files exist and contain content
*/
export function detectCompleted(graph: ArtifactGraph, changeDir: string): CompletedSet {
const completed = new Set<string>();
Expand All @@ -29,9 +29,9 @@ export function detectCompleted(graph: ArtifactGraph, changeDir: string): Comple
}

/**
* Checks if an artifact is complete by checking if its generated file(s) exist.
* Checks if an artifact is complete by checking if its generated file(s) exist and contain content.
* Supports both simple paths and glob patterns.
*/
function isArtifactComplete(generates: string, changeDir: string): boolean {
return artifactOutputExists(changeDir, generates);
return artifactOutputComplete(changeDir, generates);
}
60 changes: 59 additions & 1 deletion test/core/artifact-graph/outputs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@ import * as fs from 'node:fs';
import * as path from 'node:path';
import * as os from 'node:os';
import { FileSystemUtils } from '../../../src/utils/file-system.js';
import { artifactOutputExists, resolveArtifactOutputs } from '../../../src/core/artifact-graph/outputs.js';
import {
artifactOutputComplete,
artifactOutputContentValid,
artifactOutputExists,
resolveArtifactOutputs,
} from '../../../src/core/artifact-graph/outputs.js';

describe('artifact-graph/outputs', () => {
let tempDir: string;
Expand All @@ -25,6 +30,59 @@ describe('artifact-graph/outputs', () => {

expect(resolveArtifactOutputs(tempDir, 'proposal.md')).toEqual([canonical(filePath)]);
expect(artifactOutputExists(tempDir, 'proposal.md')).toBe(true);
expect(artifactOutputComplete(tempDir, 'proposal.md')).toBe(true);
});

it('keeps existence checks true for empty files while completion rejects them', () => {
const filePath = path.join(tempDir, 'proposal.md');
fs.writeFileSync(filePath, '');

expect(resolveArtifactOutputs(tempDir, 'proposal.md')).toEqual([canonical(filePath)]);
expect(artifactOutputExists(tempDir, 'proposal.md')).toBe(true);
expect(artifactOutputContentValid(tempDir, 'proposal.md')).toBe(false);
expect(artifactOutputComplete(tempDir, 'proposal.md')).toBe(false);
});

it('rejects whitespace-only artifact output content', () => {
fs.writeFileSync(path.join(tempDir, 'proposal.md'), ' \n\t\n');

expect(artifactOutputContentValid(tempDir, 'proposal.md')).toBe(false);
expect(artifactOutputComplete(tempDir, 'proposal.md')).toBe(false);
});

it('rejects comment-only artifact output content', () => {
fs.writeFileSync(
path.join(tempDir, 'proposal.md'),
'<!-- placeholder -->\n<!-- generated by workflow -->\n'
);

expect(artifactOutputContentValid(tempDir, 'proposal.md')).toBe(false);
expect(artifactOutputComplete(tempDir, 'proposal.md')).toBe(false);
});

it('rejects multi-line comment-only artifact output content', () => {
fs.writeFileSync(path.join(tempDir, 'proposal.md'), '<!--\nplaceholder\n-->\n');

expect(artifactOutputContentValid(tempDir, 'proposal.md')).toBe(false);
expect(artifactOutputComplete(tempDir, 'proposal.md')).toBe(false);
});

it('accepts a non-empty heading as meaningful artifact output content', () => {
fs.writeFileSync(path.join(tempDir, 'proposal.md'), '# Proposal\n');

expect(artifactOutputContentValid(tempDir, 'proposal.md')).toBe(true);
expect(artifactOutputComplete(tempDir, 'proposal.md')).toBe(true);
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

it('rejects glob outputs when any resolved file lacks meaningful content', () => {
const specsDir = path.join(tempDir, 'specs');
fs.mkdirSync(specsDir, { recursive: true });
fs.writeFileSync(path.join(specsDir, 'feature-a.md'), '# Feature A\n');
fs.writeFileSync(path.join(specsDir, 'feature-b.md'), '');

expect(artifactOutputExists(tempDir, 'specs/*.md')).toBe(true);
expect(artifactOutputContentValid(tempDir, 'specs/*.md')).toBe(false);
expect(artifactOutputComplete(tempDir, 'specs/*.md')).toBe(false);
});

it('does not treat a directory as a resolved literal artifact output', () => {
Expand Down
83 changes: 83 additions & 0 deletions test/core/artifact-graph/state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,74 @@ describe('artifact-graph/state', () => {
expect(completed.has('proposal')).toBe(true);
});

it('should not mark artifact complete when file is empty', () => {
const schema = createSchema([
{ id: 'proposal', generates: 'proposal.md', description: 'Proposal', template: 't.md', requires: [] },
]);
const graph = ArtifactGraph.fromSchema(schema);

fs.writeFileSync(path.join(tempDir, 'proposal.md'), '');

const completed = detectCompleted(graph, tempDir);

expect(completed.has('proposal')).toBe(false);
});

it('should not mark artifact complete when file is whitespace-only', () => {
const schema = createSchema([
{ id: 'proposal', generates: 'proposal.md', description: 'Proposal', template: 't.md', requires: [] },
]);
const graph = ArtifactGraph.fromSchema(schema);

fs.writeFileSync(path.join(tempDir, 'proposal.md'), ' \n\t\n');

const completed = detectCompleted(graph, tempDir);

expect(completed.has('proposal')).toBe(false);
});

it('should mark artifact complete when file contains a single heading line', () => {
const schema = createSchema([
{ id: 'proposal', generates: 'proposal.md', description: 'Proposal', template: 't.md', requires: [] },
]);
const graph = ArtifactGraph.fromSchema(schema);

fs.writeFileSync(path.join(tempDir, 'proposal.md'), '# Proposal\n');

const completed = detectCompleted(graph, tempDir);

expect(completed.has('proposal')).toBe(true);
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.

it('should not mark artifact complete when file only contains HTML comments', () => {
const schema = createSchema([
{ id: 'proposal', generates: 'proposal.md', description: 'Proposal', template: 't.md', requires: [] },
]);
const graph = ArtifactGraph.fromSchema(schema);

fs.writeFileSync(
path.join(tempDir, 'proposal.md'),
'<!-- placeholder -->\n<!-- generated by workflow -->\n'
);

const completed = detectCompleted(graph, tempDir);

expect(completed.has('proposal')).toBe(false);
});

it('should not mark artifact complete when file only contains a multi-line HTML comment', () => {
const schema = createSchema([
{ id: 'proposal', generates: 'proposal.md', description: 'Proposal', template: 't.md', requires: [] },
]);
const graph = ArtifactGraph.fromSchema(schema);

fs.writeFileSync(path.join(tempDir, 'proposal.md'), '<!--\nplaceholder\n-->\n');

const completed = detectCompleted(graph, tempDir);

expect(completed.has('proposal')).toBe(false);
});

it('should not mark artifact complete when file does not exist', () => {
const schema = createSchema([
{ id: 'proposal', generates: 'proposal.md', description: 'Proposal', template: 't.md', requires: [] },
Expand Down Expand Up @@ -108,6 +176,21 @@ describe('artifact-graph/state', () => {
expect(completed.has('specs')).toBe(true);
});

it('should not mark glob pattern complete when one matched file is empty', () => {
const schema = createSchema([
{ id: 'specs', generates: 'specs/*.md', description: 'Specs', template: 't.md', requires: [] },
]);
const graph = ArtifactGraph.fromSchema(schema);

fs.mkdirSync(path.join(tempDir, 'specs'), { recursive: true });
fs.writeFileSync(path.join(tempDir, 'specs', 'feature-a.md'), '# Feature A\n');
fs.writeFileSync(path.join(tempDir, 'specs', 'feature-b.md'), '');

const completed = detectCompleted(graph, tempDir);

expect(completed.has('specs')).toBe(false);
});

it('should not mark glob pattern complete when directory is empty', () => {
const schema = createSchema([
{ id: 'specs', generates: 'specs/*.md', description: 'Specs', template: 't.md', requires: [] },
Expand Down