From 54960672d72eeedd1e826e321a25be4785c50280 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 10 Jul 2026 20:34:31 -0700 Subject: [PATCH] fix: address PR review feedback (surface coverage, list/search guard, Intel Homebrew, serve exit tests) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - store.ts: guard parseStepsFromPrompt in listTasks and searchTasks too, so one unreadable PROMPT.md can't reject the Promise.all and 500 the whole board list/search (CodeRabbit). Matches the getTask fallback. - update-check.ts: isHomebrewInstall now resolves symlinks and matches the real Cellar/opt install roots, fixing Intel-macOS Homebrew detection that only checked /usr/local/Homebrew/ (brew's repo dir) and would have shown npm/sudo guidance instead of `brew upgrade` (CodeRabbit). - task-detail-prompt-resilience.test.ts: extend to assert the invariant across all surfaces — listTasks(slim)/searchTasks, reopen-to-todo moveTask (resetPromptCheckboxes), and deleteTask — not just getTask/updateTask/archive (CodeRabbit; Surface Enumeration rule). - serve.test.ts: add SIGINT/SIGTERM exit-code assertions (130/143) so the serve path's POSIX exit contract can't regress independently of daemon (CodeRabbit). - update-check.test.ts: add Intel-Homebrew remediation test. Co-Authored-By: Claude Opus 4.8 --- .../cli/src/commands/__tests__/serve.test.ts | 16 +++++++++++ .../task-detail-prompt-resilience.test.ts | 23 +++++++++++++-- packages/core/src/store.ts | 24 +++++++++++++--- .../src/__tests__/update-check.test.ts | 19 +++++++++++++ packages/dashboard/src/update-check.ts | 28 +++++++++++++++---- 5 files changed, 99 insertions(+), 11 deletions(-) diff --git a/packages/cli/src/commands/__tests__/serve.test.ts b/packages/cli/src/commands/__tests__/serve.test.ts index a27e89bbca..2a49f925dd 100644 --- a/packages/cli/src/commands/__tests__/serve.test.ts +++ b/packages/cli/src/commands/__tests__/serve.test.ts @@ -765,6 +765,22 @@ describe("runServe", () => { expect(mockSyncStartupModels).toHaveBeenCalledTimes(1); }); + // FNXC:DaemonSignalExit 2026-07-10-16:00: `fn serve` must honor the same POSIX + // exit-code contract as `fn daemon` — a memory-pressure SIGTERM exits non-zero + // (143) so `Restart=on-failure` restarts it; SIGINT exits 130. Guards against + // the two headless-server paths regressing independently. + it("exits 143 on SIGTERM-initiated shutdown", async () => { + await runServe(0, {}); + await triggerSignal("SIGTERM"); + expect(process.exit).toHaveBeenCalledWith(143); + }); + + it("exits 130 on SIGINT-initiated shutdown", async () => { + await runServe(0, {}); + await triggerSignal("SIGINT"); + expect(process.exit).toHaveBeenCalledWith(130); + }); + it("registers built-in zai GLM-5.2 before refreshing models", async () => { await runServe(0, {}); diff --git a/packages/core/src/__tests__/task-detail-prompt-resilience.test.ts b/packages/core/src/__tests__/task-detail-prompt-resilience.test.ts index 43b32a3a10..2d685a89ca 100644 --- a/packages/core/src/__tests__/task-detail-prompt-resilience.test.ts +++ b/packages/core/src/__tests__/task-detail-prompt-resilience.test.ts @@ -57,18 +57,37 @@ describe("getTask PROMPT.md read resilience (task-write-API 500 regression)", () expect(detail.id).toBe(task.id); expect(detail.prompt).toBe(""); + // The board read paths must survive too — listTasks/searchTasks slim-sync + // steps from PROMPT.md for stepless tasks and would otherwise reject their + // Promise.all and 500 the whole board/search on one unreadable file. + const listed = await store.listTasks({ slim: true }); + expect(listed.some((t) => t.id === task.id)).toBe(true); + const found = await store.searchTasks("unreadable", { slim: true }); + expect(Array.isArray(found)).toBe(true); + // The mutation path must stay usable too. These store methods back the // reported failing endpoints and each independently touches PROMPT.md: - // PATCH -> updateTask (title/description PROMPT.md heading sync) - // archive-> archiveTask (readPromptForArchive) + // PATCH -> updateTask (title/description PROMPT.md heading sync) + // reset -> moveTask reopen-to-todo (resetPromptCheckboxes) + updateStep + // archive -> archiveTask (readPromptForArchive) + // delete -> deleteTask // A read failure in that PROMPT.md work must not brick the DB mutation. await expect(store.updateTask(task.id, { title: "renamed with broken PROMPT.md" })).resolves.toBeTruthy(); const afterMutation = await store.getTask(task.id); expect(afterMutation.title).toBe("renamed with broken PROMPT.md"); + // reset path: advance the task then reopen to todo, which triggers + // resetPromptCheckboxes against the unreadable PROMPT.md. (New tasks start + // in `triage`, so step through todo → in-progress → todo.) + await expect(store.moveTask(task.id, "todo")).resolves.toBeTruthy(); + await expect(store.moveTask(task.id, "in-progress")).resolves.toBeTruthy(); + await expect(store.moveTask(task.id, "todo")).resolves.toBeTruthy(); + await expect(store.archiveTask(task.id)).resolves.toBeTruthy(); const archived = await store.getTask(task.id); expect(archived.column).toBe("archived"); + + await expect(store.deleteTask(task.id)).resolves.toBeTruthy(); } finally { await store.close(); } diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index f75e79c238..a8a9530f72 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -6176,8 +6176,16 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS} return task; } - const steps = await this.parseStepsFromPrompt(task.id); - return steps.length > 0 ? { ...task, steps } : task; + // FNXC:TaskDetailPromptResilience 2026-07-10-16:00: an unreadable PROMPT.md + // must not reject this Promise.all and 500 the entire board list — degrade + // to the persisted (empty) steps and log, matching getTask. + try { + const steps = await this.parseStepsFromPrompt(task.id); + return steps.length > 0 ? { ...task, steps } : task; + } catch (err) { + storeLog.warn(`[task-detail] failed to sync steps from PROMPT.md for ${task.id} during listTasks: ${getErrorMessage(err)}`); + return task; + } })); const archivedTasks = includeArchived && (!columnFilter || columnFilter === "archived") ? this.archiveDb.list().map((entry) => this.archiveEntryToTask(entry, slim)) : []; // FNXC:BoardConsistency 2026-06-21-08:34: FN-6851's cache-sync fix is primary; listTasks still collapses duplicate storage sources so one task ID cannot render in two columns. Active SQLite rows are authoritative over archive snapshots. @@ -6906,8 +6914,16 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS} return task; } - const steps = await this.parseStepsFromPrompt(task.id); - return steps.length > 0 ? { ...task, steps } : task; + // FNXC:TaskDetailPromptResilience 2026-07-10-16:00: an unreadable PROMPT.md + // must not reject this Promise.all and 500 the entire search — degrade to + // the persisted (empty) steps and log, matching getTask/listTasks. + try { + const steps = await this.parseStepsFromPrompt(task.id); + return steps.length > 0 ? { ...task, steps } : task; + } catch (err) { + storeLog.warn(`[task-detail] failed to sync steps from PROMPT.md for ${task.id} during searchTasks: ${getErrorMessage(err)}`); + return task; + } })); const archiveMatches = includeArchived ? this.archiveDb.search(trimmedQuery, limit >= 0 ? limit : 100).map((entry) => this.archiveEntryToTask(entry, slim)) diff --git a/packages/dashboard/src/__tests__/update-check.test.ts b/packages/dashboard/src/__tests__/update-check.test.ts index 350d1328ec..08e5a5b967 100644 --- a/packages/dashboard/src/__tests__/update-check.test.ts +++ b/packages/dashboard/src/__tests__/update-check.test.ts @@ -260,6 +260,25 @@ describe("update-check", () => { expect(execFake).toHaveBeenCalledTimes(1); }); + // FNXC:UpdateInstallPermissions 2026-07-10-16:00: an Intel-macOS Homebrew install + // resolves through `/usr/local/Cellar/…` (not `/usr/local/Homebrew/`), so the + // remediation must recommend `brew upgrade`, not the npm/sudo guidance. + it("performUpdateInstall recommends brew for an Intel-macOS Homebrew install path", async () => { + const originalArgv1 = process.argv[1]; + process.argv[1] = "/usr/local/Cellar/fusion/0.57.0/bin/fn"; + try { + const execFake = vi.fn().mockRejectedValue( + Object.assign(new Error("EACCES"), { code: "EACCES", stderr: "npm error EACCES" }), + ); + const result = await performUpdateInstall("1.0.0", "2.0.0", { exec: execFake, fusionDir }); + expect(result.updated).toBe(false); + expect(result.error).toMatch(/brew upgrade fusion/); + expect(result.error).not.toMatch(/sudo npm/); + } finally { + process.argv[1] = originalArgv1; + } + }); + describe("frequency", () => { beforeEach(() => { __resetStartupRefreshFlag(); diff --git a/packages/dashboard/src/update-check.ts b/packages/dashboard/src/update-check.ts index a593be339d..55b2ebd7ea 100644 --- a/packages/dashboard/src/update-check.ts +++ b/packages/dashboard/src/update-check.ts @@ -1,5 +1,5 @@ import { exec } from "node:child_process"; -import { readFileSync } from "node:fs"; +import { readFileSync, realpathSync } from "node:fs"; import { mkdir, rm, writeFile } from "node:fs/promises"; import { join } from "node:path"; import { promisify } from "node:util"; @@ -131,12 +131,30 @@ function detectRunningBinaryPath(): string | null { return typeof process.execPath === "string" ? process.execPath : null; } +/* +FNXC:UpdateInstallPermissions 2026-07-10-16:00: +Detect a Homebrew-managed install so the remediation says `brew upgrade` rather than +the npm/sudo guidance. Formulae live under a Cellar and are symlinked into bin — on +Apple Silicon everything is under `/opt/homebrew/`, but on Intel macOS the bin symlink +is `/usr/local/bin/fn` -> `/usr/local/Cellar/...` (and `/usr/local/Homebrew/` is only +brew's own git repo, not where formulae install). So resolve the symlink and match the +real Cellar/opt install roots — checking only `/usr/local/Homebrew/` missed Intel Macs. +`/usr/local/bin` is deliberately NOT matched: it is shared with npm-global bins. +*/ function isHomebrewInstall(binaryPath: string | null): boolean { if (!binaryPath) return false; - return ( - binaryPath.startsWith("/opt/homebrew/") || - binaryPath.startsWith("/usr/local/Homebrew/") || - binaryPath.startsWith("/home/linuxbrew/") + let resolved = binaryPath; + try { + resolved = realpathSync(binaryPath); + } catch { + // Unresolvable symlink/path — fall back to the raw path. + } + return [binaryPath, resolved].some((p) => + p.startsWith("/opt/homebrew/") || // Apple Silicon (bin, opt, Cellar) + p.startsWith("/usr/local/Cellar/") || // Intel formula install root + p.startsWith("/usr/local/opt/") || // Intel formula opt symlinks + p.includes("/Homebrew/") || // brew's own repo checkout + p.startsWith("/home/linuxbrew/"), // Linuxbrew ); }