diff --git a/src/config/schema/hooks.ts b/src/config/schema/hooks.ts index 80e4c71dd..641825da1 100644 --- a/src/config/schema/hooks.ts +++ b/src/config/schema/hooks.ts @@ -55,6 +55,7 @@ export const HookNameSchema = z.enum([ "read-image-resizer", "todo-description-override", "webfetch-redirect-guard", + "fsync-skip-warning", "legacy-plugin-toast", ]) diff --git a/src/hooks/fsync-skip-warning/index.test.ts b/src/hooks/fsync-skip-warning/index.test.ts new file mode 100644 index 000000000..e7c241e83 --- /dev/null +++ b/src/hooks/fsync-skip-warning/index.test.ts @@ -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 } + + 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 } + 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 } + const afterOutput = { title: "ok", output: "base", metadata: {} as Record } + + 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 } + const afterOutput = { title: "ok", output: "base", metadata: {} as Record } + + 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 } + + 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 } + const outputB = { title: "ok", output: "B", metadata: {} as Record } + + 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") + }) +}) diff --git a/src/hooks/fsync-skip-warning/index.ts b/src/hooks/fsync-skip-warning/index.ts new file mode 100644 index 000000000..fb59399be --- /dev/null +++ b/src/hooks/fsync-skip-warning/index.ts @@ -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 +} + +type ToolAfterOutput = { + title: string + output: string + metadata: unknown +} + +export function createFsyncSkipWarningHook() { + const startTimesByCallId = new Map() + + const toolExecuteBefore = async ( + input: ToolExecuteInput, + _output: ToolBeforeOutput, + ): Promise => { + startTimesByCallId.set(input.callID, Date.now()) + } + + const toolExecuteAfter = async ( + input: ToolExecuteInput, + output: ToolAfterOutput, + ): Promise => { + 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, + } +} diff --git a/src/hooks/index.ts b/src/hooks/index.ts index 98473c67a..5ed94b813 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -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" diff --git a/src/plugin/hooks/create-tool-guard-hooks.ts b/src/plugin/hooks/create-tool-guard-hooks.ts index 8f6675350..7cd8ea166 100644 --- a/src/plugin/hooks/create-tool-guard-hooks.ts +++ b/src/plugin/hooks/create-tool-guard-hooks.ts @@ -18,6 +18,7 @@ import { createTodoDescriptionOverrideHook, createWebFetchRedirectGuardHook, createTeamToolGating, + createFsyncSkipWarningHook, } from "../../hooks" import { getOpenCodeVersion, @@ -42,6 +43,7 @@ export type ToolGuardHooks = { readImageResizer: ReturnType | null todoDescriptionOverride: ReturnType | null webfetchRedirectGuard: ReturnType | null + fsyncSkipWarning: ReturnType | null teamToolGating: ReturnType | 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, } } diff --git a/src/plugin/tool-execute-after.ts b/src/plugin/tool-execute-after.ts index f230d90db..7dabc7545 100644 --- a/src/plugin/tool-execute-after.ts +++ b/src/plugin/tool-execute-after.ts @@ -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) } diff --git a/src/plugin/tool-execute-before.ts b/src/plugin/tool-execute-before.ts index e903571fe..093c3b157 100644 --- a/src/plugin/tool-execute-before.ts +++ b/src/plugin/tool-execute-before.ts @@ -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) diff --git a/src/shared/fsync-skip-warning-formatter.test.ts b/src/shared/fsync-skip-warning-formatter.test.ts new file mode 100644 index 000000000..57b626e02 --- /dev/null +++ b/src/shared/fsync-skip-warning-formatter.test.ts @@ -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") + }) +}) diff --git a/src/shared/fsync-skip-warning-formatter.ts b/src/shared/fsync-skip-warning-formatter.ts new file mode 100644 index 000000000..91bd869d4 --- /dev/null +++ b/src/shared/fsync-skip-warning-formatter.ts @@ -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() + + 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") +}