From d3bc634c3d61b77f8baf21bbcd1862a4a4689d4f Mon Sep 17 00:00:00 2001 From: siddseethepalli Date: Sat, 18 Apr 2026 20:48:57 +0000 Subject: [PATCH] chore(assistant): delete obsolete update-bulletin materialization --- .../src/__tests__/update-bulletin.test.ts | 478 ------------------ assistant/src/prompts/update-bulletin.ts | 139 ----- 2 files changed, 617 deletions(-) delete mode 100644 assistant/src/__tests__/update-bulletin.test.ts delete mode 100644 assistant/src/prompts/update-bulletin.ts diff --git a/assistant/src/__tests__/update-bulletin.test.ts b/assistant/src/__tests__/update-bulletin.test.ts deleted file mode 100644 index 482fa05234..0000000000 --- a/assistant/src/__tests__/update-bulletin.test.ts +++ /dev/null @@ -1,478 +0,0 @@ -import * as fs from "node:fs"; -import { - existsSync, - mkdirSync, - readdirSync, - readFileSync, - rmSync, - writeFileSync, -} from "node:fs"; -import { tmpdir } from "node:os"; -import { join } from "node:path"; -import { - afterEach, - beforeEach, - describe, - expect, - it, - mock, - spyOn, -} from "bun:test"; - -import { getWorkspacePromptPath } from "../util/platform.js"; - -// --- In-memory checkpoint store --- -const store = new Map(); - -mock.module("../memory/checkpoints.js", () => ({ - getMemoryCheckpoint: mock((key: string) => store.get(key) ?? null), - setMemoryCheckpoint: mock((key: string, value: string) => - store.set(key, value), - ), -})); - -// --- Mutable config stub for updates.enabled tests --- -const updatesConfig = { enabled: true }; - -mock.module("../config/loader.js", () => ({ - getConfig: () => ({ updates: updatesConfig }), -})); - -// --- Temp directory for template files --- -// Avoids mutating the real source-controlled UPDATES.md template, preventing -// race conditions with parallel test execution and working tree corruption -// if the test process crashes. -let tempTemplateDir: string; - -mock.module("../version.js", () => ({ - APP_VERSION: "1.0.0", -})); - -// Mock the template path module so tests read from a temp directory instead -// of the real source-controlled template file. -mock.module("../prompts/update-bulletin-template-path.js", () => ({ - getTemplatePath: () => join(tempTemplateDir, "UPDATES.md"), -})); - -const { syncUpdateBulletinOnStartup } = - await import("../prompts/update-bulletin.js"); - -// Workspace path used by all tests — resolved via the preload's VELLUM_WORKSPACE_DIR. -const workspacePath = getWorkspacePromptPath("UPDATES.md"); - -const TEST_TEMPLATE = "## What's New\n\nTest release notes.\n"; -const COMMENT_ONLY_TEMPLATE = - "_ This is a comment-only template.\n_ No real content here.\n"; - -describe("syncUpdateBulletinOnStartup", () => { - beforeEach(() => { - store.clear(); - updatesConfig.enabled = true; - // Remove any leftover workspace UPDATES.md from a previous test - if (existsSync(workspacePath)) { - rmSync(workspacePath); - } - tempTemplateDir = join( - tmpdir(), - `update-bulletin-tpl-${Date.now()}-${Math.random().toString(36).slice(2)}`, - ); - mkdirSync(tempTemplateDir, { recursive: true }); - // Write a test template with real content so materialization proceeds - writeFileSync(join(tempTemplateDir, "UPDATES.md"), TEST_TEMPLATE, "utf-8"); - }); - - afterEach(() => { - // Clean up the workspace UPDATES.md so tests don't leak into each other - if (existsSync(workspacePath)) { - rmSync(workspacePath); - } - rmSync(tempTemplateDir, { recursive: true, force: true }); - }); - - it("creates workspace file on first eligible run", () => { - expect(existsSync(workspacePath)).toBe(false); - - syncUpdateBulletinOnStartup(); - - expect(existsSync(workspacePath)).toBe(true); - const content = readFileSync(workspacePath, "utf-8"); - expect(content).toContain(""); - expect(content).toContain("What's New"); - }); - - it("skips materialization entirely when updates.enabled is false", () => { - updatesConfig.enabled = false; - expect(existsSync(workspacePath)).toBe(false); - - syncUpdateBulletinOnStartup(); - - expect(existsSync(workspacePath)).toBe(false); - // No checkpoint writes should have happened either. - expect(store.get("updates:active_releases")).toBeUndefined(); - expect(store.get("updates:completed_releases")).toBeUndefined(); - }); - - it("appends release block when workspace file exists without current marker", () => { - const preExisting = - "\nOld release notes.\n"; - writeFileSync(workspacePath, preExisting, "utf-8"); - - syncUpdateBulletinOnStartup(); - - const content = readFileSync(workspacePath, "utf-8"); - expect(content).toContain(""); - expect(content).toContain(""); - expect(content).toContain("Old release notes."); - }); - - it("does not duplicate same marker on repeated runs", () => { - syncUpdateBulletinOnStartup(); - const afterFirst = readFileSync(workspacePath, "utf-8"); - - syncUpdateBulletinOnStartup(); - const afterSecond = readFileSync(workspacePath, "utf-8"); - - expect(afterSecond).toBe(afterFirst); - }); - - it("skips completed release", () => { - store.set("updates:completed_releases", JSON.stringify(["1.0.0"])); - - syncUpdateBulletinOnStartup(); - - expect(existsSync(workspacePath)).toBe(false); - }); - - it("adds current release to active set", () => { - syncUpdateBulletinOnStartup(); - - const raw = store.get("updates:active_releases"); - expect(raw).toBeDefined(); - const active: string[] = JSON.parse(raw!); - expect(active).toContain("1.0.0"); - }); - - it("marks active releases as completed when UPDATES.md is deleted", () => { - // Pre-populate active releases including the current one — simulates - // a normal session where 1.0.0 was materialized alongside leftover - // entries from prior versions. - store.set( - "updates:active_releases", - JSON.stringify(["0.8.0", "0.9.0", "1.0.0"]), - ); - - // Workspace file does not exist — simulates the assistant having deleted it - expect(existsSync(workspacePath)).toBe(false); - - syncUpdateBulletinOnStartup(); - - // Active set should be cleared - const activeRaw = store.get("updates:active_releases"); - expect(activeRaw).toBeDefined(); - const active: string[] = JSON.parse(activeRaw!); - expect(active).not.toContain("0.8.0"); - expect(active).not.toContain("0.9.0"); - expect(active).not.toContain("1.0.0"); - - // All releases should now be completed - const completedRaw = store.get("updates:completed_releases"); - expect(completedRaw).toBeDefined(); - const completed: string[] = JSON.parse(completedRaw!); - expect(completed).toContain("0.8.0"); - expect(completed).toContain("0.9.0"); - expect(completed).toContain("1.0.0"); - }); - - it("does not recreate completed release after deletion", () => { - // First run — creates the workspace file and marks 1.0.0 active - syncUpdateBulletinOnStartup(); - expect(existsSync(workspacePath)).toBe(true); - - // Simulate assistant deleting the file to signal completion - rmSync(workspacePath); - expect(existsSync(workspacePath)).toBe(false); - - // Second run — deletion-completion should mark 1.0.0 completed - syncUpdateBulletinOnStartup(); - - // The file should NOT be recreated since the release is now completed - expect(existsSync(workspacePath)).toBe(false); - }); - - it("treats an empty UPDATES.md as dismissal of the current release", () => { - // Pre-populate active so we're past the fresh-install guard. - store.set("updates:active_releases", JSON.stringify(["1.0.0"])); - writeFileSync(workspacePath, "", "utf-8"); - - syncUpdateBulletinOnStartup(); - - // File should be left alone (still empty, not refilled). - expect(existsSync(workspacePath)).toBe(true); - expect(readFileSync(workspacePath, "utf-8")).toBe(""); - - const completed: string[] = JSON.parse( - store.get("updates:completed_releases")!, - ); - expect(completed).toContain("1.0.0"); - }); - - it("treats a whitespace-only UPDATES.md as dismissal of the current release", () => { - // Pre-populate active with the current release so the dismissal branch - // is scoped correctly to this version. - store.set("updates:active_releases", JSON.stringify(["1.0.0"])); - writeFileSync(workspacePath, " \n\n\t\n", "utf-8"); - - syncUpdateBulletinOnStartup(); - - // File should be left alone — content must not be re-appended. - const content = readFileSync(workspacePath, "utf-8"); - expect(content).not.toContain(""); - - const completed: string[] = JSON.parse( - store.get("updates:completed_releases")!, - ); - expect(completed).toContain("1.0.0"); - }); - - it("materializes new version bulletin after a prior release was dismissed", () => { - // Verifies that a new version's bulletin is materialized even when a - // prior release exists in the completed set. Dismissal detection is - // scoped to the current release — only suppress if this version was - // already active or completed, not because an unrelated version was. - store.set("updates:completed_releases", JSON.stringify(["0.9.0"])); - expect(existsSync(workspacePath)).toBe(false); - - syncUpdateBulletinOnStartup(); - - expect(existsSync(workspacePath)).toBe(true); - const content = readFileSync(workspacePath, "utf-8"); - expect(content).toContain(""); - expect(content).toContain("What's New"); - - // 1.0.0 must NOT have been auto-completed. - const completed: string[] = JSON.parse( - store.get("updates:completed_releases")!, - ); - expect(completed).not.toContain("1.0.0"); - - // 1.0.0 should be in the active set. - const active: string[] = JSON.parse(store.get("updates:active_releases")!); - expect(active).toContain("1.0.0"); - }); - - it("creates file on fresh install even though it is missing", () => { - // Both active and completed are empty — brand-new DB. - expect(store.get("updates:active_releases")).toBeUndefined(); - expect(store.get("updates:completed_releases")).toBeUndefined(); - expect(existsSync(workspacePath)).toBe(false); - - syncUpdateBulletinOnStartup(); - - // Fresh install should materialize, not dismiss. - expect(existsSync(workspacePath)).toBe(true); - expect(readFileSync(workspacePath, "utf-8")).toContain( - "", - ); - }); - - it("merges pending old block with new release block", () => { - // Pre-create workspace file with an old release block - const oldContent = - "\nOld release notes for 0.9.0.\n\n"; - writeFileSync(workspacePath, oldContent, "utf-8"); - - syncUpdateBulletinOnStartup(); - - const content = readFileSync(workspacePath, "utf-8"); - // Both old and new release blocks should be present - expect(content).toContain(""); - expect(content).toContain("Old release notes for 0.9.0."); - expect(content).toContain(""); - }); - - it("idempotent on repeated sync calls", () => { - // First call - syncUpdateBulletinOnStartup(); - const afterFirst = readFileSync(workspacePath, "utf-8"); - - // Second call - syncUpdateBulletinOnStartup(); - const afterSecond = readFileSync(workspacePath, "utf-8"); - - expect(afterSecond).toBe(afterFirst); - - // Third call for good measure - syncUpdateBulletinOnStartup(); - const afterThird = readFileSync(workspacePath, "utf-8"); - - expect(afterThird).toBe(afterFirst); - }); - - it("write path produces valid UTF-8 with trailing newline", () => { - syncUpdateBulletinOnStartup(); - const content = readFileSync(workspacePath, "utf-8"); - - expect(content.length).toBeGreaterThan(0); - expect(content.endsWith("\n")).toBe(true); - - // Verify round-trip through Buffer produces identical content (valid UTF-8) - const roundTripped = Buffer.from(content, "utf-8").toString("utf-8"); - expect(roundTripped).toBe(content); - }); - - it("no temp file leftovers after successful write", () => { - syncUpdateBulletinOnStartup(); - - const wsDir = process.env.VELLUM_WORKSPACE_DIR!; - const entries = readdirSync(wsDir); - const tmpFiles = entries.filter((e) => e.includes(".tmp.")); - expect(tmpFiles).toHaveLength(0); - }); - - it("skips materialization when template is comment-only", () => { - // Write a comment-only template fixture (no real content after stripping) - writeFileSync( - join(tempTemplateDir, "UPDATES.md"), - COMMENT_ONLY_TEMPLATE, - "utf-8", - ); - syncUpdateBulletinOnStartup(); - - expect(existsSync(workspacePath)).toBe(false); - }); - - it("only appends new content blocks on version bump with extended template", () => { - // Workspace already has entries A and B from a prior release - const oldContent = [ - "", - "", - "## Entry A", - "Content for A.", - "", - "", - "", - "## Entry B", - "Content for B.", - "", - "", - ].join("\n"); - writeFileSync(workspacePath, oldContent, "utf-8"); - - // Template now has A, B, C — C is the only new entry - const extendedTemplate = [ - "", - "## Entry A", - "Content for A.", - "", - "", - "", - "## Entry B", - "Content for B.", - "", - "", - "", - "## Entry C", - "New content for C.", - "", - "", - ].join("\n"); - writeFileSync( - join(tempTemplateDir, "UPDATES.md"), - extendedTemplate, - "utf-8", - ); - - syncUpdateBulletinOnStartup(); - - const content = readFileSync(workspacePath, "utf-8"); - - // New release block should be present - expect(content).toContain(""); - - // Entry C should appear - expect(content).toContain("entry-c"); - expect(content).toContain("New content for C."); - - // Entries A and B should NOT be duplicated - const countA = ( - content.match(//g) || [] - ).length; - const countB = ( - content.match(//g) || [] - ).length; - expect(countA).toBe(1); - expect(countB).toBe(1); - }); - - it("skips append when all template content blocks already exist in workspace", () => { - // Workspace already has entries A and B from a prior release - const oldContent = [ - "", - "", - "## Entry A", - "", - "", - "", - "## Entry B", - "", - "", - ].join("\n"); - writeFileSync(workspacePath, oldContent, "utf-8"); - - // Template has the same A and B — nothing new - const sameTemplate = [ - "", - "## Entry A", - "", - "", - "", - "## Entry B", - "", - "", - ].join("\n"); - writeFileSync(join(tempTemplateDir, "UPDATES.md"), sameTemplate, "utf-8"); - - syncUpdateBulletinOnStartup(); - - const content = readFileSync(workspacePath, "utf-8"); - - // No 1.0.0 block should be added — all content already present - expect(content).not.toContain(""); - }); - - it("preserves existing file when atomic write fails", () => { - const originalContent = - "\nOriginal content.\n"; - writeFileSync(workspacePath, originalContent, "utf-8"); - - // Mock writeFileSync to throw when writing the temp file, simulating a - // disk-full or permission error deterministically (chmod-based approaches - // are unreliable when running as root or with CAP_DAC_OVERRIDE). - const originalWriteFileSync = fs.writeFileSync; - const spy = spyOn(fs, "writeFileSync").mockImplementation( - (...args: Parameters) => { - if (typeof args[0] === "string" && args[0].includes(".tmp.")) { - throw new Error("Simulated write failure"); - } - return originalWriteFileSync(...args); - }, - ); - try { - expect(() => syncUpdateBulletinOnStartup()).toThrow( - "Simulated write failure", - ); - } finally { - spy.mockRestore(); - } - - // Original content should be preserved (atomic write never renamed over it) - const content = readFileSync(workspacePath, "utf-8"); - expect(content).toBe(originalContent); - - // No temp file leftovers - const wsDir = process.env.VELLUM_WORKSPACE_DIR!; - const entries = readdirSync(wsDir); - const tmpFiles = entries.filter((e: string) => e.includes(".tmp.")); - expect(tmpFiles).toHaveLength(0); - }); -}); diff --git a/assistant/src/prompts/update-bulletin.ts b/assistant/src/prompts/update-bulletin.ts deleted file mode 100644 index e966890f0c..0000000000 --- a/assistant/src/prompts/update-bulletin.ts +++ /dev/null @@ -1,139 +0,0 @@ -import { - existsSync, - lstatSync, - readFileSync, - realpathSync, - renameSync, - unlinkSync, - writeFileSync, -} from "node:fs"; - -import { getConfig } from "../config/loader.js"; -import { getWorkspacePromptPath } from "../util/platform.js"; -import { stripCommentLines } from "../util/strip-comment-lines.js"; -import { APP_VERSION } from "../version.js"; -import { - appendReleaseBlock, - filterNewContentBlocks, - hasReleaseBlock, -} from "./update-bulletin-format.js"; -import { - addActiveRelease, - getActiveReleases, - isReleaseCompleted, - markReleasesCompleted, - setActiveReleases, -} from "./update-bulletin-state.js"; -import { getTemplatePath } from "./update-bulletin-template-path.js"; - -/** - * Writes content to a file via a temp-file + rename to prevent partial/truncated - * writes if the process crashes mid-write. - */ -function atomicWriteFileSync(filePath: string, content: string): void { - const tmpPath = `${filePath}.tmp.${process.pid}`; - try { - writeFileSync(tmpPath, content, "utf-8"); - // Resolve symlinks so we rename to the real target, preserving the link. - // If the symlink is dangling (target doesn't exist), fall back to writing - // through the symlink path directly — realpathSync throws ENOENT for dangling links. - let targetPath = filePath; - try { - if (lstatSync(filePath, { throwIfNoEntry: false })?.isSymbolicLink()) { - targetPath = realpathSync(filePath); - } - } catch (err: unknown) { - // Dangling symlink — fall back to writing through the symlink path. - // Only swallow ENOENT (dangling target); re-throw ELOOP, EACCES, I/O faults, etc. - if ( - err instanceof Error && - "code" in err && - (err as NodeJS.ErrnoException).code !== "ENOENT" - ) { - throw err; - } - } - renameSync(tmpPath, targetPath); - } catch (err) { - try { - unlinkSync(tmpPath); - } catch { - /* ignore cleanup errors */ - } - throw err; - } -} - -/** - * Materializes the current release's update bulletin on startup. - * - * First checks for dismissal: if the workspace UPDATES.md was deleted or - * emptied after we'd previously materialized it, that's an explicit signal - * (from the user or the assistant) that the current release has been handled. - * Mark any active releases plus the current release as completed and return - * without recreating the file. - * - * Otherwise reads the bundled UPDATES.md template, strips comment lines, and - * appends a release block to the workspace UPDATES.md if one doesn't already - * exist for this version. Skips completed releases entirely. - */ -export function syncUpdateBulletinOnStartup(): void { - if (!getConfig().updates.enabled) return; - - const currentReleaseId = APP_VERSION; - const workspacePath = getWorkspacePromptPath("UPDATES.md"); - - // --- Dismissal detection --- - // If UPDATES.md is missing or empty AND the CURRENT release has been - // materialized before (i.e. it's in the active set, or already marked - // completed), treat it as a dismissal: mark the active set plus the - // current release completed and stop. Scoping to the current release is - // critical — a prior release having been completed must NOT cause a new - // version's bulletin to be suppressed as "already dismissed". It also - // preserves fresh-install semantics: on a brand-new DB we want to create - // the file, not treat its absence as dismissal. - const activeReleases = getActiveReleases(); - const fileMissing = !existsSync(workspacePath); - const fileEmpty = - !fileMissing && readFileSync(workspacePath, "utf-8").trim().length === 0; - const currentReleaseMaterialized = - activeReleases.includes(currentReleaseId) || - isReleaseCompleted(currentReleaseId); - - if ((fileMissing || fileEmpty) && currentReleaseMaterialized) { - markReleasesCompleted([...activeReleases, currentReleaseId]); - setActiveReleases([]); - return; - } - - // --- Template materialization --- - const templatePath = getTemplatePath(); - if (!existsSync(templatePath)) return; - - const rawTemplate = readFileSync(templatePath, "utf-8"); - const templateContent = stripCommentLines(rawTemplate); - - if (!templateContent || templateContent.trim().length === 0) return; - - if (isReleaseCompleted(currentReleaseId)) return; - - if (!existsSync(workspacePath)) { - const content = appendReleaseBlock("", currentReleaseId, templateContent); - atomicWriteFileSync(workspacePath, content); - } else { - const existing = readFileSync(workspacePath, "utf-8"); - if (!hasReleaseBlock(existing, currentReleaseId)) { - const contentToAppend = filterNewContentBlocks(templateContent, existing); - if (contentToAppend.length > 0) { - const updated = appendReleaseBlock( - existing, - currentReleaseId, - contentToAppend, - ); - atomicWriteFileSync(workspacePath, updated); - } - } - } - - addActiveRelease(currentReleaseId); -}