From dca846ec0635ef00b0572dfae2d82257fa5e5ea5 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Fri, 15 May 2026 16:53:40 -0700 Subject: [PATCH 1/3] fix: validate artifact content before marking workflow step done Closes #1084 --- .changeset/validate-artifact-content.md | 7 +++ src/core/artifact-graph/outputs.ts | 33 +++++++++++++++ src/core/artifact-graph/state.ts | 10 ++--- test/core/artifact-graph/outputs.test.ts | 43 ++++++++++++++++++- test/core/artifact-graph/state.test.ts | 54 ++++++++++++++++++++++++ 5 files changed, 141 insertions(+), 6 deletions(-) create mode 100644 .changeset/validate-artifact-content.md diff --git a/.changeset/validate-artifact-content.md b/.changeset/validate-artifact-content.md new file mode 100644 index 000000000..53c2f7f5c --- /dev/null +++ b/.changeset/validate-artifact-content.md @@ -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. diff --git a/src/core/artifact-graph/outputs.ts b/src/core/artifact-graph/outputs.ts index 9467552f2..e0a7f8747 100644 --- a/src/core/artifact-graph/outputs.ts +++ b/src/core/artifact-graph/outputs.ts @@ -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); +} + +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('\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'); diff --git a/test/core/artifact-graph/state.test.ts b/test/core/artifact-graph/state.test.ts index 9a8096320..e08a79f23 100644 --- a/test/core/artifact-graph/state.test.ts +++ b/test/core/artifact-graph/state.test.ts @@ -100,6 +100,22 @@ describe('artifact-graph/state', () => { 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 does not exist', () => { const schema = createSchema([ { id: 'proposal', generates: 'proposal.md', description: 'Proposal', template: 't.md', requires: [] }, From e60823069e380ab974d12ce4d8cfc79f71bfba2b Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Thu, 21 May 2026 23:14:58 -0700 Subject: [PATCH 3/3] fix: strip HTML comment blocks before validating artifact content Multi-line HTML comments like: were incorrectly treated as meaningful content because the line-based check only skipped lines starting with '|$)/g before splitting into lines, then any non-whitespace line counts as meaningful content. The non-greedy (-->|$) tail handles unterminated comment blocks safely. Add regression tests in outputs.test.ts and state.test.ts covering the multi-line comment-only case for both artifactOutputContentValid / artifactOutputComplete and detectCompleted. Update the changeset to mention HTML-comment-only specifically and note that heading-only or other partially-written artifacts are not yet detected (heading content still counts as meaningful). Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> --- .changeset/validate-artifact-content.md | 2 +- src/core/artifact-graph/outputs.ts | 9 +++------ test/core/artifact-graph/outputs.test.ts | 7 +++++++ test/core/artifact-graph/state.test.ts | 13 +++++++++++++ 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/.changeset/validate-artifact-content.md b/.changeset/validate-artifact-content.md index 20efc2253..9937d65c0 100644 --- a/.changeset/validate-artifact-content.md +++ b/.changeset/validate-artifact-content.md @@ -4,4 +4,4 @@ ### Bug Fixes -- Fixed workflow resumes so empty, whitespace-only, or comment-only artifact files are regenerated instead of being treated as completed. +- 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 f810ec162..0b4d2cebd 100644 --- a/src/core/artifact-graph/outputs.ts +++ b/src/core/artifact-graph/outputs.ts @@ -63,14 +63,11 @@ export function artifactOutputComplete(changeDir: string, generates: string): bo function isArtifactOutputFileContentValid(filePath: string): boolean { try { const content = fs.readFileSync(filePath, 'utf8'); + const withoutHtmlComments = content.replace(/|$)/g, ''); - return content + return withoutHtmlComments .split(/\r?\n/) - .some((line) => { - const trimmed = line.trim(); - - return trimmed.length > 0 && !trimmed.startsWith('\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'); diff --git a/test/core/artifact-graph/state.test.ts b/test/core/artifact-graph/state.test.ts index e08a79f23..0108b658c 100644 --- a/test/core/artifact-graph/state.test.ts +++ b/test/core/artifact-graph/state.test.ts @@ -116,6 +116,19 @@ describe('artifact-graph/state', () => { 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: [] },