Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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 or whitespace-only artifact files are regenerated instead of being treated as completed.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
33 changes: 33 additions & 0 deletions src/core/artifact-graph/outputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,36 @@ 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.
*/
export function artifactOutputComplete(changeDir: string, generates: string): boolean {
return artifactOutputExists(changeDir, generates)
&& artifactOutputContentValid(changeDir, generates);
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

function isArtifactOutputFileContentValid(filePath: string): boolean {
try {
const content = fs.readFileSync(filePath, 'utf8');

return content
.split(/\r?\n/)
.some((line) => {
const trimmed = line.trim();

return trimmed.length > 0 && !trimmed.startsWith('<!--');
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
} 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);
}
43 changes: 42 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,42 @@ 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('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
54 changes: 54 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,45 @@ 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 does not exist', () => {
const schema = createSchema([
{ id: 'proposal', generates: 'proposal.md', description: 'Proposal', template: 't.md', requires: [] },
Expand Down Expand Up @@ -108,6 +147,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