fix: address PR review feedback (surface coverage, list/search guard, Intel Homebrew, serve exit tests)
- 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 <noreply@anthropic.com>
This commit is contained in:
@@ -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, {});
|
||||
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
@@ -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))
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user