diff --git a/.changeset/validate-artifact-content.md b/.changeset/validate-artifact-content.md new file mode 100644 index 000000000..9937d65c0 --- /dev/null +++ b/.changeset/validate-artifact-content.md @@ -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. diff --git a/src/core/artifact-graph/outputs.ts b/src/core/artifact-graph/outputs.ts index 9467552f2..0b4d2cebd 100644 --- a/src/core/artifact-graph/outputs.ts +++ b/src/core/artifact-graph/outputs.ts @@ -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); +} + +function isArtifactOutputFileContentValid(filePath: string): boolean { + try { + const content = fs.readFileSync(filePath, 'utf8'); + const withoutHtmlComments = content.replace(/|$)/g, ''); + + return withoutHtmlComments + .split(/\r?\n/) + .some((line) => line.trim().length > 0); + } catch { + return false; + } +} diff --git a/src/core/artifact-graph/state.ts b/src/core/artifact-graph/state.ts index 47c256ac9..dfe3aa0bf 100644 --- a/src/core/artifact-graph/state.ts +++ b/src/core/artifact-graph/state.ts @@ -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(); @@ -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); } diff --git a/test/core/artifact-graph/outputs.test.ts b/test/core/artifact-graph/outputs.test.ts index 988200e2c..2bbd0d005 100644 --- a/test/core/artifact-graph/outputs.test.ts +++ b/test/core/artifact-graph/outputs.test.ts @@ -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; @@ -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'), + '\n\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'), '\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); + }); + + 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', () => { diff --git a/test/core/artifact-graph/state.test.ts b/test/core/artifact-graph/state.test.ts index 758a7675b..0108b658c 100644 --- a/test/core/artifact-graph/state.test.ts +++ b/test/core/artifact-graph/state.test.ts @@ -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); + }); + + 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'), + '\n\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'), '\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: [] }, @@ -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: [] },