Merge remote-tracking branch 'origin/dev' into feature/configurable-agent-ordering
This commit is contained in:
@@ -55,6 +55,7 @@ export const HookNameSchema = z.enum([
|
||||
"read-image-resizer",
|
||||
"todo-description-override",
|
||||
"webfetch-redirect-guard",
|
||||
"fsync-skip-warning",
|
||||
"legacy-plugin-toast",
|
||||
])
|
||||
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import { randomUUID } from "node:crypto"
|
||||
import { open, readFile, rename, rm, unlink, writeFile } from "node:fs/promises"
|
||||
|
||||
import { tolerantFsync } from "../../../shared/tolerant-fsync"
|
||||
|
||||
type LockOptions = {
|
||||
staleAfterMs?: number
|
||||
ownerTag?: string
|
||||
@@ -51,7 +53,7 @@ async function acquireLock(lockPath: string, ownerTag: string, staleAfterMs: num
|
||||
const fileHandle = await open(lockPath, "wx")
|
||||
try {
|
||||
await fileHandle.writeFile(buildOwnerContent(ownerTag))
|
||||
await fileHandle.sync()
|
||||
await tolerantFsync(fileHandle, `acquireLock:${lockPath}`)
|
||||
} finally {
|
||||
await fileHandle.close()
|
||||
}
|
||||
@@ -116,7 +118,7 @@ export async function atomicWrite(
|
||||
await writeFile(tmpPath, content)
|
||||
const fileHandle = await open(tmpPath, "r")
|
||||
try {
|
||||
await fileHandle.sync()
|
||||
await tolerantFsync(fileHandle, `atomicWrite:${filePath}`)
|
||||
} finally {
|
||||
await fileHandle.close()
|
||||
}
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
import { beforeEach, describe, expect, it } from "bun:test"
|
||||
|
||||
import { classifyPathEnvironment } from "../../shared/classify-path-environment"
|
||||
import { clearAllSkips, recordFsyncSkip } from "../../shared/fsync-skip-tracker"
|
||||
import { createFsyncSkipWarningHook } from "./index"
|
||||
|
||||
describe("createFsyncSkipWarningHook", () => {
|
||||
beforeEach(() => {
|
||||
clearAllSkips()
|
||||
})
|
||||
|
||||
it("records callID start timestamp in tool.execute.before", async () => {
|
||||
const hook = createFsyncSkipWarningHook()
|
||||
const input = { tool: "bash", sessionID: "ses1", callID: "call-1" }
|
||||
const output = { args: {} as Record<string, unknown> }
|
||||
|
||||
await hook["tool.execute.before"](input, output)
|
||||
await Bun.sleep(2)
|
||||
|
||||
recordFsyncSkip({
|
||||
filePath: "/tmp/a",
|
||||
contextLabel: "atomicWrite:/tmp/a",
|
||||
errorCode: "EPERM",
|
||||
message: "operation not permitted",
|
||||
pathClassification: classifyPathEnvironment("/tmp/a"),
|
||||
})
|
||||
|
||||
const afterOutput = { title: "ok", output: "done", metadata: {} as Record<string, unknown> }
|
||||
await hook["tool.execute.after"](input, afterOutput)
|
||||
|
||||
expect(afterOutput.output).toContain("[fsync-skipped]")
|
||||
})
|
||||
|
||||
it("drains skips after start time and appends warning to output text", async () => {
|
||||
const hook = createFsyncSkipWarningHook()
|
||||
const input = { tool: "write", sessionID: "ses1", callID: "call-2" }
|
||||
const beforeOutput = { args: {} as Record<string, unknown> }
|
||||
const afterOutput = { title: "ok", output: "base", metadata: {} as Record<string, unknown> }
|
||||
|
||||
await hook["tool.execute.before"](input, beforeOutput)
|
||||
await Bun.sleep(2)
|
||||
|
||||
recordFsyncSkip({
|
||||
filePath: "/Users/x/OneDrive/a",
|
||||
contextLabel: "atomicWrite:/Users/x/OneDrive/a",
|
||||
errorCode: "EPERM",
|
||||
message: "operation not permitted",
|
||||
pathClassification: classifyPathEnvironment("/Users/x/OneDrive/a"),
|
||||
})
|
||||
|
||||
await hook["tool.execute.after"](input, afterOutput)
|
||||
|
||||
expect(afterOutput.output).toContain("base\n\n---")
|
||||
expect(afterOutput.output).toContain("OneDrive")
|
||||
})
|
||||
|
||||
it("leaves output unchanged when no skips happen during window", async () => {
|
||||
const hook = createFsyncSkipWarningHook()
|
||||
const input = { tool: "write", sessionID: "ses1", callID: "call-3" }
|
||||
const beforeOutput = { args: {} as Record<string, unknown> }
|
||||
const afterOutput = { title: "ok", output: "base", metadata: {} as Record<string, unknown> }
|
||||
|
||||
await hook["tool.execute.before"](input, beforeOutput)
|
||||
await hook["tool.execute.after"](input, afterOutput)
|
||||
|
||||
expect(afterOutput.output).toBe("base")
|
||||
})
|
||||
|
||||
it("isolates multiple parallel calls by callID watermark", async () => {
|
||||
const hook = createFsyncSkipWarningHook()
|
||||
const beforeOutput = { args: {} as Record<string, unknown> }
|
||||
|
||||
const inputA = { tool: "write", sessionID: "ses1", callID: "call-A" }
|
||||
const inputB = { tool: "write", sessionID: "ses1", callID: "call-B" }
|
||||
|
||||
await hook["tool.execute.before"](inputA, beforeOutput)
|
||||
await Bun.sleep(2)
|
||||
await hook["tool.execute.before"](inputB, beforeOutput)
|
||||
await Bun.sleep(2)
|
||||
|
||||
recordFsyncSkip({
|
||||
filePath: "/tmp/a",
|
||||
contextLabel: "atomicWrite:/tmp/a",
|
||||
errorCode: "EPERM",
|
||||
message: "operation not permitted",
|
||||
pathClassification: classifyPathEnvironment("/tmp/a"),
|
||||
})
|
||||
|
||||
const outputA = { title: "ok", output: "A", metadata: {} as Record<string, unknown> }
|
||||
const outputB = { title: "ok", output: "B", metadata: {} as Record<string, unknown> }
|
||||
|
||||
await hook["tool.execute.after"](inputA, outputA)
|
||||
await hook["tool.execute.after"](inputB, outputB)
|
||||
|
||||
expect(outputA.output).toContain("[fsync-skipped]")
|
||||
expect(outputB.output).toBe("B")
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,50 @@
|
||||
import { drainSkipsAfter } from "../../shared/fsync-skip-tracker"
|
||||
import { formatFsyncSkipWarning } from "../../shared/fsync-skip-warning-formatter"
|
||||
|
||||
type ToolExecuteInput = {
|
||||
tool: string
|
||||
sessionID: string
|
||||
callID: string
|
||||
}
|
||||
|
||||
type ToolBeforeOutput = {
|
||||
args: Record<string, unknown>
|
||||
}
|
||||
|
||||
type ToolAfterOutput = {
|
||||
title: string
|
||||
output: string
|
||||
metadata: unknown
|
||||
}
|
||||
|
||||
export function createFsyncSkipWarningHook() {
|
||||
const startTimesByCallId = new Map<string, number>()
|
||||
|
||||
const toolExecuteBefore = async (
|
||||
input: ToolExecuteInput,
|
||||
_output: ToolBeforeOutput,
|
||||
): Promise<void> => {
|
||||
startTimesByCallId.set(input.callID, Date.now())
|
||||
}
|
||||
|
||||
const toolExecuteAfter = async (
|
||||
input: ToolExecuteInput,
|
||||
output: ToolAfterOutput,
|
||||
): Promise<void> => {
|
||||
if (typeof output.output !== "string") return
|
||||
|
||||
const startTimestamp = startTimesByCallId.get(input.callID) ?? 0
|
||||
startTimesByCallId.delete(input.callID)
|
||||
|
||||
const skips = drainSkipsAfter(startTimestamp)
|
||||
const warning = formatFsyncSkipWarning(skips)
|
||||
if (warning.length === 0) return
|
||||
|
||||
output.output = `${output.output}\n\n${warning}`
|
||||
}
|
||||
|
||||
return {
|
||||
"tool.execute.before": toolExecuteBefore,
|
||||
"tool.execute.after": toolExecuteAfter,
|
||||
}
|
||||
}
|
||||
@@ -65,3 +65,4 @@ export { createReadImageResizerHook } from "./read-image-resizer"
|
||||
export { createTodoDescriptionOverrideHook } from "./todo-description-override"
|
||||
export { createWebFetchRedirectGuardHook } from "./webfetch-redirect-guard"
|
||||
export { createLegacyPluginToastHook } from "./legacy-plugin-toast"
|
||||
export { createFsyncSkipWarningHook } from "./fsync-skip-warning"
|
||||
|
||||
@@ -18,6 +18,7 @@ import {
|
||||
createTodoDescriptionOverrideHook,
|
||||
createWebFetchRedirectGuardHook,
|
||||
createTeamToolGating,
|
||||
createFsyncSkipWarningHook,
|
||||
} from "../../hooks"
|
||||
import {
|
||||
getOpenCodeVersion,
|
||||
@@ -42,6 +43,7 @@ export type ToolGuardHooks = {
|
||||
readImageResizer: ReturnType<typeof createReadImageResizerHook> | null
|
||||
todoDescriptionOverride: ReturnType<typeof createTodoDescriptionOverrideHook> | null
|
||||
webfetchRedirectGuard: ReturnType<typeof createWebFetchRedirectGuardHook> | null
|
||||
fsyncSkipWarning: ReturnType<typeof createFsyncSkipWarningHook> | null
|
||||
teamToolGating: ReturnType<typeof createTeamToolGating> | null
|
||||
}
|
||||
|
||||
@@ -139,6 +141,10 @@ export function createToolGuardHooks(args: {
|
||||
? safeHook("team-tool-gating", () => createTeamToolGating(ctx, pluginConfig.team_mode))
|
||||
: null
|
||||
|
||||
const fsyncSkipWarning = isHookEnabled("fsync-skip-warning")
|
||||
? safeHook("fsync-skip-warning", () => createFsyncSkipWarningHook())
|
||||
: null
|
||||
|
||||
return {
|
||||
commentChecker,
|
||||
toolOutputTruncator,
|
||||
@@ -154,6 +160,7 @@ export function createToolGuardHooks(args: {
|
||||
readImageResizer,
|
||||
todoDescriptionOverride,
|
||||
webfetchRedirectGuard,
|
||||
fsyncSkipWarning,
|
||||
teamToolGating,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -153,6 +153,7 @@ export function createToolExecuteAfterHandler(args: {
|
||||
await hooks.readImageResizer?.["tool.execute.after"]?.(hookInput, output)
|
||||
await hooks.hashlineReadEnhancer?.["tool.execute.after"]?.(hookInput, output)
|
||||
await hooks.webfetchRedirectGuard?.["tool.execute.after"]?.(hookInput, output)
|
||||
await hooks.fsyncSkipWarning?.["tool.execute.after"]?.(hookInput, output)
|
||||
await hooks.jsonErrorRecovery?.["tool.execute.after"]?.(hookInput, output)
|
||||
}
|
||||
|
||||
|
||||
@@ -72,8 +72,9 @@ export function createToolExecuteBeforeHandler(args: {
|
||||
await hooks.directoryReadmeInjector?.["tool.execute.before"]?.(input, output)
|
||||
await hooks.rulesInjector?.["tool.execute.before"]?.(input, output)
|
||||
await hooks.tasksTodowriteDisabler?.["tool.execute.before"]?.(input, output)
|
||||
await hooks.webfetchRedirectGuard?.["tool.execute.before"]?.(input, output)
|
||||
await hooks.prometheusMdOnly?.["tool.execute.before"]?.(input, output)
|
||||
await hooks.webfetchRedirectGuard?.["tool.execute.before"]?.(input, output)
|
||||
await hooks.fsyncSkipWarning?.["tool.execute.before"]?.(input, output)
|
||||
await hooks.prometheusMdOnly?.["tool.execute.before"]?.(input, output)
|
||||
await hooks.sisyphusJuniorNotepad?.["tool.execute.before"]?.(input, output)
|
||||
await hooks.atlasHook?.["tool.execute.before"]?.(input, output)
|
||||
await hooks.teamToolGating?.["tool.execute.before"]?.(input, output)
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
import { describe, expect, it } from "bun:test"
|
||||
|
||||
import {
|
||||
classifyPathEnvironment,
|
||||
describePathClassification,
|
||||
} from "./classify-path-environment"
|
||||
|
||||
describe("classifyPathEnvironment", () => {
|
||||
it("classifies macOS iCloud path as icloud", () => {
|
||||
expect(
|
||||
classifyPathEnvironment(
|
||||
"/Users/x/Library/Mobile Documents/com~apple~CloudDocs/project/file.txt",
|
||||
),
|
||||
).toBe("icloud")
|
||||
})
|
||||
|
||||
it("classifies OneDrive path on unix style", () => {
|
||||
expect(classifyPathEnvironment("/Users/x/OneDrive/foo")).toBe("onedrive")
|
||||
})
|
||||
|
||||
it("classifies OneDrive path on windows style", () => {
|
||||
expect(classifyPathEnvironment("C:\\Users\\x\\OneDrive\\foo")).toBe("onedrive")
|
||||
})
|
||||
|
||||
it("classifies macOS Desktop path as desktop-sync", () => {
|
||||
expect(classifyPathEnvironment("/Users/x/Desktop/foo")).toBe("desktop-sync")
|
||||
})
|
||||
|
||||
it("classifies /Volumes path as network-drive", () => {
|
||||
expect(classifyPathEnvironment("/Volumes/NetworkShare/foo")).toBe("network-drive")
|
||||
})
|
||||
|
||||
it("classifies random path as unknown", () => {
|
||||
expect(classifyPathEnvironment("/tmp/foo")).toBe("unknown")
|
||||
})
|
||||
|
||||
it("classifies empty string as unknown", () => {
|
||||
expect(classifyPathEnvironment("")).toBe("unknown")
|
||||
})
|
||||
|
||||
it("matches OneDrive case-insensitively", () => {
|
||||
expect(classifyPathEnvironment("/Users/x/oNeDrIvE/foo")).toBe("onedrive")
|
||||
})
|
||||
})
|
||||
|
||||
describe("describePathClassification", () => {
|
||||
it("returns human-readable descriptions", () => {
|
||||
expect(describePathClassification("icloud")).toBe("iCloud Drive")
|
||||
expect(describePathClassification("onedrive")).toBe("OneDrive")
|
||||
expect(describePathClassification("desktop-sync")).toBe("Desktop sync (macOS)")
|
||||
expect(describePathClassification("network-drive")).toBe("Network drive")
|
||||
expect(describePathClassification("unknown")).toBe(
|
||||
"filesystem that does not support fsync",
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,68 @@
|
||||
import { homedir } from "node:os"
|
||||
import path from "node:path"
|
||||
|
||||
export type PathClassification =
|
||||
| "icloud"
|
||||
| "onedrive"
|
||||
| "desktop-sync"
|
||||
| "network-drive"
|
||||
| "unknown"
|
||||
|
||||
function normalizeInputPath(absolutePath: string): string {
|
||||
return absolutePath.replaceAll("\\", "/")
|
||||
}
|
||||
|
||||
function isUnderPath(normalizedPath: string, normalizedParentPath: string): boolean {
|
||||
return normalizedPath === normalizedParentPath || normalizedPath.startsWith(`${normalizedParentPath}/`)
|
||||
}
|
||||
|
||||
export function classifyPathEnvironment(absolutePath: string): PathClassification {
|
||||
if (absolutePath.length === 0) return "unknown"
|
||||
|
||||
const normalizedPath = normalizeInputPath(absolutePath)
|
||||
const lowercasePath = normalizedPath.toLowerCase()
|
||||
if (lowercasePath.includes("/onedrive") || lowercasePath.includes("/onedrive/")) {
|
||||
return "onedrive"
|
||||
}
|
||||
|
||||
if (normalizedPath.includes("/Library/Mobile Documents/")) {
|
||||
return "icloud"
|
||||
}
|
||||
|
||||
if (isUnderPath(normalizedPath, "/Volumes")) {
|
||||
return "network-drive"
|
||||
}
|
||||
|
||||
if (
|
||||
normalizedPath.startsWith("/Users/")
|
||||
&& (normalizedPath.includes("/Desktop/") || normalizedPath.endsWith("/Desktop")
|
||||
|| normalizedPath.includes("/Documents/") || normalizedPath.endsWith("/Documents"))
|
||||
) {
|
||||
return "desktop-sync"
|
||||
}
|
||||
|
||||
const normalizedHome = normalizeInputPath(homedir())
|
||||
const desktopPath = normalizeInputPath(path.join(normalizedHome, "Desktop"))
|
||||
const documentsPath = normalizeInputPath(path.join(normalizedHome, "Documents"))
|
||||
|
||||
if (isUnderPath(normalizedPath, desktopPath) || isUnderPath(normalizedPath, documentsPath)) {
|
||||
return "desktop-sync"
|
||||
}
|
||||
|
||||
return "unknown"
|
||||
}
|
||||
|
||||
export function describePathClassification(pathClassification: PathClassification): string {
|
||||
switch (pathClassification) {
|
||||
case "icloud":
|
||||
return "iCloud Drive"
|
||||
case "onedrive":
|
||||
return "OneDrive"
|
||||
case "desktop-sync":
|
||||
return "Desktop sync (macOS)"
|
||||
case "network-drive":
|
||||
return "Network drive"
|
||||
case "unknown":
|
||||
return "filesystem that does not support fsync"
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,100 @@
|
||||
import { beforeEach, describe, expect, it } from "bun:test"
|
||||
|
||||
import {
|
||||
clearAllSkips,
|
||||
drainSkipsAfter,
|
||||
recordFsyncSkip,
|
||||
} from "./fsync-skip-tracker"
|
||||
|
||||
type PathClassification =
|
||||
| "icloud"
|
||||
| "onedrive"
|
||||
| "desktop-sync"
|
||||
| "network-drive"
|
||||
| "unknown"
|
||||
|
||||
function recordSkip(index: number, pathClassification: PathClassification = "unknown"): void {
|
||||
recordFsyncSkip({
|
||||
filePath: `/tmp/file-${index}.txt`,
|
||||
contextLabel: `atomicWrite:/tmp/file-${index}.txt`,
|
||||
errorCode: "EPERM",
|
||||
message: "operation not permitted",
|
||||
pathClassification,
|
||||
})
|
||||
}
|
||||
|
||||
describe("fsync-skip-tracker", () => {
|
||||
beforeEach(() => {
|
||||
clearAllSkips()
|
||||
})
|
||||
|
||||
it("recordFsyncSkip adds entry with timestamp", () => {
|
||||
const before = Date.now()
|
||||
recordSkip(1)
|
||||
const entries = drainSkipsAfter(0)
|
||||
|
||||
expect(entries).toHaveLength(1)
|
||||
expect(entries[0]?.filePath).toBe("/tmp/file-1.txt")
|
||||
expect(entries[0]?.timestamp).toBeGreaterThanOrEqual(before)
|
||||
})
|
||||
|
||||
it("drainSkipsAfter(timestamp) returns entries strictly after the timestamp", async () => {
|
||||
recordSkip(1)
|
||||
const firstTimestamp = Date.now()
|
||||
|
||||
await Bun.sleep(2)
|
||||
|
||||
recordSkip(2)
|
||||
const drained = drainSkipsAfter(firstTimestamp)
|
||||
expect(drained).toHaveLength(1)
|
||||
expect(drained[0]?.filePath).toBe("/tmp/file-2.txt")
|
||||
})
|
||||
|
||||
it("drainSkipsAfter removes drained entries from buffer", () => {
|
||||
recordSkip(1)
|
||||
recordSkip(2)
|
||||
|
||||
const drained = drainSkipsAfter(0)
|
||||
expect(drained).toHaveLength(2)
|
||||
expect(drainSkipsAfter(0)).toEqual([])
|
||||
})
|
||||
|
||||
it("buffer is bounded to max 200 entries and drops oldest on overflow", () => {
|
||||
for (let index = 1; index <= 205; index += 1) {
|
||||
recordSkip(index)
|
||||
}
|
||||
|
||||
const drained = drainSkipsAfter(0)
|
||||
expect(drained).toHaveLength(200)
|
||||
expect(drained[0]?.filePath).toBe("/tmp/file-6.txt")
|
||||
expect(drained[199]?.filePath).toBe("/tmp/file-205.txt")
|
||||
})
|
||||
|
||||
it("multiple records with same path are kept", () => {
|
||||
recordSkip(1)
|
||||
recordFsyncSkip({
|
||||
filePath: "/tmp/file-1.txt",
|
||||
contextLabel: "acquireLock:/tmp/file-1.txt",
|
||||
errorCode: "EPERM",
|
||||
message: "second",
|
||||
pathClassification: "unknown",
|
||||
})
|
||||
|
||||
const drained = drainSkipsAfter(0)
|
||||
expect(drained).toHaveLength(2)
|
||||
expect(drained[0]?.filePath).toBe("/tmp/file-1.txt")
|
||||
expect(drained[1]?.filePath).toBe("/tmp/file-1.txt")
|
||||
})
|
||||
|
||||
it("drainSkipsAfter(0) returns all entries", () => {
|
||||
recordSkip(1)
|
||||
recordSkip(2)
|
||||
|
||||
const drained = drainSkipsAfter(0)
|
||||
expect(drained).toHaveLength(2)
|
||||
})
|
||||
|
||||
it("empty buffer returns empty array", () => {
|
||||
expect(drainSkipsAfter(0)).toEqual([])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,42 @@
|
||||
import type { PathClassification } from "./classify-path-environment"
|
||||
|
||||
export type FsyncSkipEntry = {
|
||||
filePath: string
|
||||
contextLabel: string
|
||||
errorCode: string
|
||||
message: string
|
||||
pathClassification: PathClassification
|
||||
timestamp: number
|
||||
}
|
||||
|
||||
const MAX_SKIPS = 200
|
||||
const fsyncSkips: FsyncSkipEntry[] = []
|
||||
|
||||
export function recordFsyncSkip(entry: Omit<FsyncSkipEntry, "timestamp">): void {
|
||||
fsyncSkips.push({ ...entry, timestamp: Date.now() })
|
||||
|
||||
if (fsyncSkips.length > MAX_SKIPS) {
|
||||
fsyncSkips.splice(0, fsyncSkips.length - MAX_SKIPS)
|
||||
}
|
||||
}
|
||||
|
||||
export function drainSkipsAfter(timestampMs: number): FsyncSkipEntry[] {
|
||||
const drainedEntries: FsyncSkipEntry[] = []
|
||||
const retainedEntries: FsyncSkipEntry[] = []
|
||||
|
||||
for (const entry of fsyncSkips) {
|
||||
if (entry.timestamp > timestampMs) {
|
||||
drainedEntries.push(entry)
|
||||
continue
|
||||
}
|
||||
|
||||
retainedEntries.push(entry)
|
||||
}
|
||||
|
||||
fsyncSkips.splice(0, fsyncSkips.length, ...retainedEntries)
|
||||
return drainedEntries
|
||||
}
|
||||
|
||||
export function clearAllSkips(): void {
|
||||
fsyncSkips.length = 0
|
||||
}
|
||||
@@ -0,0 +1,78 @@
|
||||
import { describe, expect, it } from "bun:test"
|
||||
|
||||
import type { FsyncSkipEntry } from "./fsync-skip-tracker"
|
||||
import { formatFsyncSkipWarning } from "./fsync-skip-warning-formatter"
|
||||
|
||||
function makeEntry(index: number, classification: FsyncSkipEntry["pathClassification"]): FsyncSkipEntry {
|
||||
return {
|
||||
filePath: `/path/${index}`,
|
||||
contextLabel: `atomicWrite:/path/${index}`,
|
||||
errorCode: "EPERM",
|
||||
message: "operation not permitted",
|
||||
pathClassification: classification,
|
||||
timestamp: 1000 + index,
|
||||
}
|
||||
}
|
||||
|
||||
describe("formatFsyncSkipWarning", () => {
|
||||
it("returns empty string for zero entries", () => {
|
||||
expect(formatFsyncSkipWarning([])).toBe("")
|
||||
})
|
||||
|
||||
it("includes iCloud environment, path, and code for one entry", () => {
|
||||
const warning = formatFsyncSkipWarning([makeEntry(1, "icloud")])
|
||||
expect(warning).toContain("iCloud Drive")
|
||||
expect(warning).toContain("/path/1")
|
||||
expect(warning).toContain("EPERM")
|
||||
})
|
||||
|
||||
it("shows all five paths when exactly five entries exist", () => {
|
||||
const warning = formatFsyncSkipWarning([
|
||||
makeEntry(1, "icloud"),
|
||||
makeEntry(2, "icloud"),
|
||||
makeEntry(3, "icloud"),
|
||||
makeEntry(4, "icloud"),
|
||||
makeEntry(5, "icloud"),
|
||||
])
|
||||
|
||||
expect(warning).toContain("/path/1")
|
||||
expect(warning).toContain("/path/5")
|
||||
expect(warning).not.toContain("and 1 more")
|
||||
})
|
||||
|
||||
it("shows five paths plus overflow summary when six entries exist", () => {
|
||||
const warning = formatFsyncSkipWarning([
|
||||
makeEntry(1, "icloud"),
|
||||
makeEntry(2, "icloud"),
|
||||
makeEntry(3, "icloud"),
|
||||
makeEntry(4, "icloud"),
|
||||
makeEntry(5, "icloud"),
|
||||
makeEntry(6, "icloud"),
|
||||
])
|
||||
|
||||
expect(warning).toContain("/path/5")
|
||||
expect(warning).not.toContain("/path/6")
|
||||
expect(warning).toContain("... and 1 more")
|
||||
})
|
||||
|
||||
it("uses the most common classification when entries are mixed", () => {
|
||||
const warning = formatFsyncSkipWarning([
|
||||
makeEntry(1, "onedrive"),
|
||||
makeEntry(2, "onedrive"),
|
||||
makeEntry(3, "icloud"),
|
||||
])
|
||||
|
||||
expect(warning).toContain("Detected environment: OneDrive")
|
||||
})
|
||||
|
||||
it("matches required section format", () => {
|
||||
const warning = formatFsyncSkipWarning([makeEntry(1, "unknown")])
|
||||
|
||||
expect(warning).toContain("[fsync-skipped] 1 write(s) bypassed fsync")
|
||||
expect(warning).toContain("Affected paths:")
|
||||
expect(warning).toContain("What this means:")
|
||||
expect(warning).toContain("The write+rename succeeded")
|
||||
expect(warning).not.toContain("Detected environment:")
|
||||
expect(warning).toContain("filesystem does not support fsync")
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,61 @@
|
||||
import { describePathClassification } from "./classify-path-environment"
|
||||
import type { FsyncSkipEntry } from "./fsync-skip-tracker"
|
||||
|
||||
const MAX_PATH_LINES = 5
|
||||
|
||||
function selectMostCommonClassification(
|
||||
entries: FsyncSkipEntry[],
|
||||
): FsyncSkipEntry["pathClassification"] {
|
||||
const counts = new Map<FsyncSkipEntry["pathClassification"], number>()
|
||||
|
||||
for (const entry of entries) {
|
||||
const currentCount = counts.get(entry.pathClassification) ?? 0
|
||||
counts.set(entry.pathClassification, currentCount + 1)
|
||||
}
|
||||
|
||||
let selected: FsyncSkipEntry["pathClassification"] = "unknown"
|
||||
let selectedCount = -1
|
||||
for (const [classification, count] of counts.entries()) {
|
||||
if (count > selectedCount) {
|
||||
selected = classification
|
||||
selectedCount = count
|
||||
}
|
||||
}
|
||||
|
||||
return selected
|
||||
}
|
||||
|
||||
export function formatFsyncSkipWarning(entries: FsyncSkipEntry[]): string {
|
||||
if (entries.length === 0) return ""
|
||||
|
||||
const selectedClassification = selectMostCommonClassification(entries)
|
||||
const selectedDescription = describePathClassification(selectedClassification)
|
||||
const shownEntries = entries.slice(0, MAX_PATH_LINES)
|
||||
const hiddenCount = Math.max(entries.length - shownEntries.length, 0)
|
||||
const pathLines = shownEntries.map((entry) => ` - ${entry.filePath} (code: ${entry.errorCode})`)
|
||||
if (hiddenCount > 0) {
|
||||
pathLines.push(` ... and ${hiddenCount} more`)
|
||||
}
|
||||
|
||||
const environmentLines = selectedClassification === "unknown"
|
||||
? []
|
||||
: [`Detected environment: ${selectedDescription}`]
|
||||
|
||||
const durabilityLine = selectedClassification === "unknown"
|
||||
? " - Crash durability is best-effort because this filesystem does not support fsync."
|
||||
: " - Crash durability is best-effort on this filesystem (this is normal for iCloud, OneDrive, network drives, antivirus-locked paths)."
|
||||
|
||||
return [
|
||||
"---",
|
||||
`[fsync-skipped] ${entries.length} write(s) bypassed fsync because the underlying filesystem rejected the syscall.`,
|
||||
"",
|
||||
...environmentLines,
|
||||
"Affected paths:",
|
||||
...pathLines,
|
||||
"",
|
||||
"What this means:",
|
||||
" - The write+rename succeeded — the file is on disk, atomicity is preserved.",
|
||||
durabilityLine,
|
||||
" - No action required. Operation completed successfully.",
|
||||
].join("\n")
|
||||
}
|
||||
@@ -0,0 +1,158 @@
|
||||
import { beforeEach, describe, expect, it } from "bun:test"
|
||||
import { fsyncSync } from "node:fs"
|
||||
import type { FileHandle } from "node:fs/promises"
|
||||
|
||||
import { clearAllSkips, drainSkipsAfter } from "./fsync-skip-tracker"
|
||||
import { isToleratedFsyncError, tolerantFsync, tolerantFsyncSync } from "./tolerant-fsync"
|
||||
|
||||
function makeFsError(code: string, message?: string): NodeJS.ErrnoException {
|
||||
const error = new Error(message ?? `${code}: simulated`) as NodeJS.ErrnoException
|
||||
error.code = code
|
||||
return error
|
||||
}
|
||||
|
||||
function fakeHandleWithSyncError(error: NodeJS.ErrnoException): FileHandle {
|
||||
return {
|
||||
sync: async () => {
|
||||
throw error
|
||||
},
|
||||
} as FileHandle
|
||||
}
|
||||
|
||||
describe("isToleratedFsyncError", () => {
|
||||
it("#given EPERM error #when checked #then returns true", () => {
|
||||
expect(isToleratedFsyncError(makeFsError("EPERM"))).toBe(true)
|
||||
})
|
||||
|
||||
it("#given EACCES error #when checked #then returns true", () => {
|
||||
expect(isToleratedFsyncError(makeFsError("EACCES"))).toBe(true)
|
||||
})
|
||||
|
||||
it("#given ENOTSUP error #when checked #then returns true", () => {
|
||||
expect(isToleratedFsyncError(makeFsError("ENOTSUP"))).toBe(true)
|
||||
})
|
||||
|
||||
it("#given EINVAL error #when checked #then returns true", () => {
|
||||
expect(isToleratedFsyncError(makeFsError("EINVAL"))).toBe(true)
|
||||
})
|
||||
|
||||
it("#given EIO error #when checked #then returns false", () => {
|
||||
expect(isToleratedFsyncError(makeFsError("EIO"))).toBe(false)
|
||||
})
|
||||
|
||||
it("#given ENOSPC error (disk full) #when checked #then returns false", () => {
|
||||
expect(isToleratedFsyncError(makeFsError("ENOSPC"))).toBe(false)
|
||||
})
|
||||
|
||||
it("#given EBADF error (bad fd) #when checked #then returns false", () => {
|
||||
expect(isToleratedFsyncError(makeFsError("EBADF"))).toBe(false)
|
||||
})
|
||||
|
||||
it("#given non-Error value #when checked #then returns false", () => {
|
||||
expect(isToleratedFsyncError("EPERM string")).toBe(false)
|
||||
expect(isToleratedFsyncError(null)).toBe(false)
|
||||
expect(isToleratedFsyncError(undefined)).toBe(false)
|
||||
expect(isToleratedFsyncError({ code: "EPERM" })).toBe(false)
|
||||
})
|
||||
|
||||
it("#given Error without code #when checked #then returns false", () => {
|
||||
expect(isToleratedFsyncError(new Error("no code"))).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("tolerantFsync (async)", () => {
|
||||
beforeEach(() => {
|
||||
clearAllSkips()
|
||||
})
|
||||
|
||||
it("#given fsync throws EPERM #when called #then resolves without throwing", async () => {
|
||||
const handle = fakeHandleWithSyncError(makeFsError("EPERM", "operation not permitted, fsync"))
|
||||
await expect(tolerantFsync(handle, "test:async-eperm")).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
it("#given fsync throws EACCES #when called #then resolves without throwing", async () => {
|
||||
const handle = fakeHandleWithSyncError(makeFsError("EACCES"))
|
||||
await expect(tolerantFsync(handle, "test:async-eacces")).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
it("#given fsync throws ENOTSUP #when called #then resolves without throwing", async () => {
|
||||
const handle = fakeHandleWithSyncError(makeFsError("ENOTSUP"))
|
||||
await expect(tolerantFsync(handle, "test:async-enotsup")).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
it("#given fsync throws EINVAL #when called #then resolves without throwing", async () => {
|
||||
const handle = fakeHandleWithSyncError(makeFsError("EINVAL"))
|
||||
await expect(tolerantFsync(handle, "test:async-einval")).resolves.toBeUndefined()
|
||||
})
|
||||
|
||||
it("#given fsync throws EIO #when called #then propagates the error", async () => {
|
||||
const handle = fakeHandleWithSyncError(makeFsError("EIO"))
|
||||
await expect(tolerantFsync(handle, "test:async-eio")).rejects.toThrow("EIO: simulated")
|
||||
})
|
||||
|
||||
it("#given fsync throws ENOSPC #when called #then propagates the error", async () => {
|
||||
const handle = fakeHandleWithSyncError(makeFsError("ENOSPC"))
|
||||
await expect(tolerantFsync(handle, "test:async-enospc")).rejects.toThrow("ENOSPC: simulated")
|
||||
})
|
||||
|
||||
it("#given fsync succeeds #when called #then resolves and sync was invoked", async () => {
|
||||
let syncCalled = false
|
||||
const handle = {
|
||||
sync: async () => {
|
||||
syncCalled = true
|
||||
},
|
||||
} as FileHandle
|
||||
await tolerantFsync(handle, "test:async-success")
|
||||
expect(syncCalled).toBe(true)
|
||||
})
|
||||
|
||||
it("#given fsync throws EPERM #when called #then tracker records one skip", async () => {
|
||||
const handle = fakeHandleWithSyncError(makeFsError("EPERM", "operation not permitted, fsync"))
|
||||
|
||||
await tolerantFsync(handle, "atomicWrite:/Users/x/Library/Mobile Documents/com~apple~CloudDocs/file.txt")
|
||||
|
||||
const entries = drainSkipsAfter(0)
|
||||
expect(entries).toHaveLength(1)
|
||||
expect(entries[0]?.errorCode).toBe("EPERM")
|
||||
})
|
||||
|
||||
it("#given fsync throws EIO #when called #then tracker remains empty", async () => {
|
||||
const handle = fakeHandleWithSyncError(makeFsError("EIO"))
|
||||
|
||||
await expect(tolerantFsync(handle, "atomicWrite:/tmp/file.txt")).rejects.toThrow("EIO: simulated")
|
||||
|
||||
expect(drainSkipsAfter(0)).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
|
||||
describe("tolerantFsyncSync (synchronous)", () => {
|
||||
it("#given fsyncSync throws EPERM #when called #then returns without throwing", () => {
|
||||
const fakeFsync = ((_fileDescriptor: number): void => {
|
||||
throw makeFsError("EPERM", "operation not permitted, fsync")
|
||||
}) as typeof fsyncSync
|
||||
expect(() => tolerantFsyncSync(123, "test:sync-eperm", fakeFsync)).not.toThrow()
|
||||
})
|
||||
|
||||
it("#given fsyncSync throws EACCES #when called #then returns without throwing", () => {
|
||||
const fakeFsync = ((_fileDescriptor: number): void => {
|
||||
throw makeFsError("EACCES")
|
||||
}) as typeof fsyncSync
|
||||
expect(() => tolerantFsyncSync(123, "test:sync-eacces", fakeFsync)).not.toThrow()
|
||||
})
|
||||
|
||||
it("#given fsyncSync throws EIO #when called #then propagates the error", () => {
|
||||
const fakeFsync = ((_fileDescriptor: number): void => {
|
||||
throw makeFsError("EIO")
|
||||
}) as typeof fsyncSync
|
||||
expect(() => tolerantFsyncSync(123, "test:sync-eio", fakeFsync)).toThrow("EIO: simulated")
|
||||
})
|
||||
|
||||
it("#given fsyncSync succeeds #when called #then returns and impl was invoked", () => {
|
||||
let called = false
|
||||
const fakeFsync = ((_fileDescriptor: number): void => {
|
||||
called = true
|
||||
}) as typeof fsyncSync
|
||||
tolerantFsyncSync(123, "test:sync-success", fakeFsync)
|
||||
expect(called).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,85 @@
|
||||
import { fsyncSync } from "node:fs"
|
||||
import type { FileHandle } from "node:fs/promises"
|
||||
|
||||
import { classifyPathEnvironment } from "./classify-path-environment"
|
||||
import { recordFsyncSkip } from "./fsync-skip-tracker"
|
||||
import { log } from "./logger"
|
||||
|
||||
const TOLERATED_FSYNC_CODES: ReadonlySet<string> = new Set([
|
||||
"EPERM",
|
||||
"EACCES",
|
||||
"ENOTSUP",
|
||||
"EINVAL",
|
||||
])
|
||||
|
||||
export function isToleratedFsyncError(error: unknown): boolean {
|
||||
if (!(error instanceof Error)) return false
|
||||
const code = (error as NodeJS.ErrnoException).code
|
||||
return code !== undefined && TOLERATED_FSYNC_CODES.has(code)
|
||||
}
|
||||
|
||||
function extractPathFromContextLabel(contextLabel: string): string {
|
||||
const separatorIndex = contextLabel.indexOf(":")
|
||||
if (separatorIndex < 0) return contextLabel
|
||||
|
||||
return contextLabel.slice(separatorIndex + 1)
|
||||
}
|
||||
|
||||
export async function tolerantFsync(
|
||||
fileHandle: FileHandle,
|
||||
contextLabel: string,
|
||||
): Promise<void> {
|
||||
try {
|
||||
await fileHandle.sync()
|
||||
} catch (error) {
|
||||
if (!isToleratedFsyncError(error)) throw error
|
||||
const errorCode = (error as NodeJS.ErrnoException).code ?? "UNKNOWN"
|
||||
const message = error instanceof Error ? error.message : String(error)
|
||||
const filePath = extractPathFromContextLabel(contextLabel)
|
||||
|
||||
log("fsync skipped due to filesystem limitation", {
|
||||
event: "fsync-skipped",
|
||||
contextLabel,
|
||||
code: errorCode,
|
||||
message,
|
||||
})
|
||||
|
||||
recordFsyncSkip({
|
||||
filePath,
|
||||
contextLabel,
|
||||
errorCode,
|
||||
message,
|
||||
pathClassification: classifyPathEnvironment(filePath),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
export function tolerantFsyncSync(
|
||||
fileDescriptor: number,
|
||||
contextLabel: string,
|
||||
fsyncImpl: typeof fsyncSync = fsyncSync,
|
||||
): void {
|
||||
try {
|
||||
fsyncImpl(fileDescriptor)
|
||||
} catch (error) {
|
||||
if (!isToleratedFsyncError(error)) throw error
|
||||
const errorCode = (error as NodeJS.ErrnoException).code ?? "UNKNOWN"
|
||||
const message = error instanceof Error ? error.message : String(error)
|
||||
const filePath = extractPathFromContextLabel(contextLabel)
|
||||
|
||||
log("fsync skipped due to filesystem limitation", {
|
||||
event: "fsync-skipped",
|
||||
contextLabel,
|
||||
code: errorCode,
|
||||
message,
|
||||
})
|
||||
|
||||
recordFsyncSkip({
|
||||
filePath,
|
||||
contextLabel,
|
||||
errorCode,
|
||||
message,
|
||||
pathClassification: classifyPathEnvironment(filePath),
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -51,4 +51,39 @@ describe("writeFileAtomically", () => {
|
||||
// when/then
|
||||
expect(() => writeFileAtomically(filePath, "content")).toThrow()
|
||||
})
|
||||
|
||||
it("#given fsync fails with EPERM (synced folder) #when writeFileAtomically called #then write succeeds", () => {
|
||||
// given
|
||||
const filePath = join(testDir, "synced-folder.txt")
|
||||
const content = "content from a synced folder where fsync is rejected"
|
||||
|
||||
// when
|
||||
writeFileAtomically(filePath, content, {
|
||||
fsyncSync: () => {
|
||||
const error = new Error("EPERM: operation not permitted, fsync") as NodeJS.ErrnoException
|
||||
error.code = "EPERM"
|
||||
throw error
|
||||
},
|
||||
})
|
||||
|
||||
// then
|
||||
expect(existsSync(filePath)).toBe(true)
|
||||
expect(readFileSync(filePath, "utf-8")).toBe(content)
|
||||
})
|
||||
|
||||
it("#given fsync fails with EIO (real I/O error) #when writeFileAtomically called #then propagates the error", () => {
|
||||
// given
|
||||
const filePath = join(testDir, "io-error.txt")
|
||||
|
||||
// when/then
|
||||
expect(() =>
|
||||
writeFileAtomically(filePath, "content", {
|
||||
fsyncSync: () => {
|
||||
const error = new Error("EIO: input/output error") as NodeJS.ErrnoException
|
||||
error.code = "EIO"
|
||||
throw error
|
||||
},
|
||||
}),
|
||||
).toThrow("EIO")
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,11 +1,24 @@
|
||||
import { closeSync, fsyncSync, openSync, renameSync, unlinkSync, writeFileSync } from "node:fs"
|
||||
import {
|
||||
closeSync,
|
||||
type fsyncSync as FsyncSync,
|
||||
openSync,
|
||||
renameSync,
|
||||
unlinkSync,
|
||||
writeFileSync,
|
||||
} from "node:fs"
|
||||
|
||||
export function writeFileAtomically(filePath: string, content: string): void {
|
||||
const tempPath = `${filePath}.tmp`
|
||||
writeFileSync(tempPath, content, "utf-8")
|
||||
import { tolerantFsyncSync } from "./tolerant-fsync"
|
||||
|
||||
export function writeFileAtomically(
|
||||
filePath: string,
|
||||
content: string,
|
||||
deps: { fsyncSync?: typeof FsyncSync } = {},
|
||||
): void {
|
||||
const tempPath = `${filePath}.tmp`
|
||||
writeFileSync(tempPath, content, "utf-8")
|
||||
const tempFileDescriptor = openSync(tempPath, "r")
|
||||
try {
|
||||
fsyncSync(tempFileDescriptor)
|
||||
tolerantFsyncSync(tempFileDescriptor, `writeFileAtomically:${filePath}`, deps.fsyncSync)
|
||||
} finally {
|
||||
closeSync(tempFileDescriptor)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user