From 269a385ee2a856316c0448f9224065641fcb2539 Mon Sep 17 00:00:00 2001 From: YOMXXX Date: Thu, 28 May 2026 11:54:48 +0800 Subject: [PATCH] fix(daemon): reconcile missing artifact manifests on run end (#2893) (#3110) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(daemon): reconcile missing artifact manifests on run end (#2893) When an agent writes HTML via write_file instead of create_artifact, no .artifact.json manifest sidecar is created. If the run then terminates (inactivity watchdog, user cancel, or process exit), the HTML file exists on disk but the manifest is missing — breaking the artifact panel, finalize, and export flows. Add a best-effort reconciliation step in the child.on('close') handler that lists project HTML files and calls reconcileHtmlArtifactManifest for any missing sidecars. The IIFE runs asynchronously after design.runs.finish() so it never blocks run finalisation. * fix(daemon): scope run-end reconciliation to files modified during the run The review on #3110 flagged that listing the entire project tree and reconciling every HTML file without a sidecar is too broad — for imported-folder projects (metadata.baseDir), pre-existing HTML files would receive spurious manifests. Record runStartTimeMs at the beginning of startChatRun and filter the reconciliation loop to only touch HTML files whose mtime >= that timestamp. Add a regression test that backdates a pre-existing HTML file and verifies it is skipped while a new file is reconciled. * test(daemon): fix mtime ordering in reconciliation regression test The runStartTimeMs was recorded after writing the new file, so its mtime fell before the threshold and the reconciliation filter skipped it. Move the timestamp capture to before the write to match the real startChatRun semantics. --- apps/daemon/src/server.ts | 35 ++++ ...fact-manifest-reconcile-on-run-end.test.ts | 166 ++++++++++++++++++ 2 files changed, 201 insertions(+) create mode 100644 apps/daemon/tests/artifact-manifest-reconcile-on-run-end.test.ts diff --git a/apps/daemon/src/server.ts b/apps/daemon/src/server.ts index 2c93461e4..72ae3d253 100644 --- a/apps/daemon/src/server.ts +++ b/apps/daemon/src/server.ts @@ -320,6 +320,7 @@ import { resolveProjectDir, resolveProjectFilePath, writeProjectFile, + reconcileHtmlArtifactManifest, } from './projects.js'; import { validateArtifactManifestInput } from './artifact-manifest.js'; import { ArtifactPublicationBlockedError } from './artifact-publication-guard.js'; @@ -10810,6 +10811,7 @@ export async function startServer({ persistRunEventToAssistantMessage(db, run, event, data); design.runs.emit(run, event, data); }; + const runStartTimeMs = Date.now(); const inactivityTimeoutMs = resolveChatRunInactivityTimeoutMs(); const artifactQuietPeriodMs = resolveChatRunArtifactQuietPeriodMs(); const inactivityKillGraceMs = 3_000; @@ -11600,6 +11602,39 @@ export async function startServer({ )); } } + // Reconcile any HTML artifacts that were written during this run + // without a manifest sidecar (e.g. agent used write_file instead of + // create_artifact, or the run terminated between HTML write and + // sidecar write). Only files modified after the run started are + // touched — pre-existing HTML in imported-folder projects must not + // receive spurious manifests. Best-effort; must not block finalisation. + // See issue #2893. + if (run.projectId) { + (async () => { + try { + const project = getProject(db, run.projectId); + const files = await listFiles(PROJECTS_DIR, run.projectId, { + metadata: project?.metadata, + }); + const dir = resolveProjectDir(PROJECTS_DIR, run.projectId, project?.metadata); + for (const f of files) { + const ext = f.name.slice(f.name.lastIndexOf('.')).toLowerCase(); + if (ext !== '.html' && ext !== '.htm') continue; + try { + const filePath = path.join(dir, f.name); + const st = await fs.promises.stat(filePath); + if (st.mtimeMs < runStartTimeMs) continue; + await reconcileHtmlArtifactManifest( + PROJECTS_DIR, + run.projectId, + f.name, + project?.metadata, + ); + } catch { /* per-file best-effort */ } + } + } catch { /* project-level best-effort */ } + })(); + } design.runs.finish(run, status, code, signal); }); if (writePromptToChildStdin && child.stdin) { diff --git a/apps/daemon/tests/artifact-manifest-reconcile-on-run-end.test.ts b/apps/daemon/tests/artifact-manifest-reconcile-on-run-end.test.ts new file mode 100644 index 000000000..885f977c7 --- /dev/null +++ b/apps/daemon/tests/artifact-manifest-reconcile-on-run-end.test.ts @@ -0,0 +1,166 @@ +// @ts-nocheck +// Verifies the run-end artifact manifest reconciliation added for #2893: +// when a chat run writes HTML via write_file (no artifactManifest) and then +// terminates, the close-handler reconciliation should create the missing +// .artifact.json sidecar so the file appears in the artifact panel. + +import { afterEach, describe, expect, it } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { closeDatabase, insertProject, openDatabase } from '../src/db.js'; +import { reconcileHtmlArtifactManifest, writeProjectFile } from '../src/projects.js'; + +const PROJECT_ID = 'reconcile-test'; +let tempDir = null; +let projectsRoot = null; + +afterEach(() => { + closeDatabase(); + if (tempDir) fs.rmSync(tempDir, { recursive: true, force: true }); + tempDir = null; + projectsRoot = null; +}); + +function setup() { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'od-reconcile-')); + const db = openDatabase(tempDir); + insertProject(db, { id: PROJECT_ID, name: 'Reconcile Test', createdAt: 1, updatedAt: 1 }); + projectsRoot = path.join(tempDir, 'projects'); + fs.mkdirSync(path.join(projectsRoot, PROJECT_ID), { recursive: true }); + return { db }; +} + +describe('run-end artifact manifest reconciliation (#2893)', () => { + it('creates a manifest sidecar for an HTML file written without artifactManifest', async () => { + setup(); + + // Simulate agent writing HTML via write_file (no artifactManifest param) + await writeProjectFile(projectsRoot, PROJECT_ID, 'output.html', '

Hello

'); + + const sidecarPath = path.join(projectsRoot, PROJECT_ID, 'output.html.artifact.json'); + // Before reconciliation: no sidecar exists + expect(fs.existsSync(sidecarPath)).toBe(false); + + // Simulate the reconciliation step from the child.on('close') handler + const dir = path.join(projectsRoot, PROJECT_ID); + const files = fs.readdirSync(dir); + for (const name of files) { + const ext = path.extname(name).toLowerCase(); + if (ext !== '.html' && ext !== '.htm') continue; + await reconcileHtmlArtifactManifest(projectsRoot, PROJECT_ID, name); + } + + // After reconciliation: sidecar exists with inferred metadata + expect(fs.existsSync(sidecarPath)).toBe(true); + const manifest = JSON.parse(fs.readFileSync(sidecarPath, 'utf8')); + expect(manifest.kind).toBe('html'); + expect(manifest.entry).toBe('output.html'); + expect(manifest.status).toBe('complete'); + expect(manifest.metadata?.inferred).toBe(true); + expect(manifest.metadata?.reconciled).toBe(true); + }); + + it('does not overwrite an existing manifest sidecar', async () => { + setup(); + + // Agent wrote HTML WITH an explicit manifest (create_artifact path) + await writeProjectFile(projectsRoot, PROJECT_ID, 'deck.html', '

pitch

', { + artifactManifest: { + version: 1, + kind: 'deck', + title: 'My Deck', + entry: 'deck.html', + renderer: 'deck-html', + status: 'complete', + exports: ['html', 'pdf'], + }, + }); + + const sidecarPath = path.join(projectsRoot, PROJECT_ID, 'deck.html.artifact.json'); + expect(fs.existsSync(sidecarPath)).toBe(true); + const original = JSON.parse(fs.readFileSync(sidecarPath, 'utf8')); + expect(original.kind).toBe('deck'); + expect(original.title).toBe('My Deck'); + + // Reconciliation should NOT overwrite the existing manifest + await reconcileHtmlArtifactManifest(projectsRoot, PROJECT_ID, 'deck.html'); + const after = JSON.parse(fs.readFileSync(sidecarPath, 'utf8')); + expect(after.kind).toBe('deck'); + expect(after.title).toBe('My Deck'); + expect(after.metadata?.inferred).toBeUndefined(); + }); + + it('ignores non-HTML files', async () => { + setup(); + + await writeProjectFile(projectsRoot, PROJECT_ID, 'README.md', '# notes\n'); + + // Reconciliation should not create a manifest for .md files + const dir = path.join(projectsRoot, PROJECT_ID); + const files = fs.readdirSync(dir); + for (const name of files) { + const ext = path.extname(name).toLowerCase(); + if (ext !== '.html' && ext !== '.htm') continue; + await reconcileHtmlArtifactManifest(projectsRoot, PROJECT_ID, name); + } + + const sidecarPath = path.join(projectsRoot, PROJECT_ID, 'README.md.artifact.json'); + expect(fs.existsSync(sidecarPath)).toBe(false); + }); + + it('handles missing project directory gracefully', async () => { + setup(); + // reconcileHtmlArtifactManifest should return null for non-existent files + const result = await reconcileHtmlArtifactManifest(projectsRoot, PROJECT_ID, 'missing.html'); + expect(result).toBeNull(); + }); + + it('reconciles .htm files as well', async () => { + setup(); + + await writeProjectFile(projectsRoot, PROJECT_ID, 'page.htm', '

page

'); + + const sidecarPath = path.join(projectsRoot, PROJECT_ID, 'page.htm.artifact.json'); + expect(fs.existsSync(sidecarPath)).toBe(false); + + await reconcileHtmlArtifactManifest(projectsRoot, PROJECT_ID, 'page.htm'); + + expect(fs.existsSync(sidecarPath)).toBe(true); + const manifest = JSON.parse(fs.readFileSync(sidecarPath, 'utf8')); + expect(manifest.kind).toBe('html'); + expect(manifest.entry).toBe('page.htm'); + }); + + it('skips pre-existing HTML files whose mtime is before the run start (imported-folder guard)', async () => { + setup(); + + // Pre-existing file: write it, then backdate its mtime to before the run + await writeProjectFile(projectsRoot, PROJECT_ID, 'old-index.html', '

old

'); + const oldPath = path.join(projectsRoot, PROJECT_ID, 'old-index.html'); + const pastTime = new Date('2020-01-01T00:00:00Z'); + fs.utimesSync(oldPath, pastTime, pastTime); + + // Run starts here — record the timestamp before the run writes files + const runStartTimeMs = Date.now(); + + // File written during the run + await writeProjectFile(projectsRoot, PROJECT_ID, 'new-output.html', '

new

'); + + // Simulate the close-handler reconciliation with mtime filter + const dir = path.join(projectsRoot, PROJECT_ID); + const files = fs.readdirSync(dir); + for (const name of files) { + const ext = path.extname(name).toLowerCase(); + if (ext !== '.html' && ext !== '.htm') continue; + const st = fs.statSync(path.join(dir, name)); + if (st.mtimeMs < runStartTimeMs) continue; + await reconcileHtmlArtifactManifest(projectsRoot, PROJECT_ID, name); + } + + // Pre-existing file should NOT have a sidecar + expect(fs.existsSync(path.join(dir, 'old-index.html.artifact.json'))).toBe(false); + // New file should have a sidecar + expect(fs.existsSync(path.join(dir, 'new-output.html.artifact.json'))).toBe(true); + }); +});