feat(hooks): surface fsync-skip warnings to AI agent via tool output
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -55,6 +55,7 @@ export const HookNameSchema = z.enum([
|
|||||||
"read-image-resizer",
|
"read-image-resizer",
|
||||||
"todo-description-override",
|
"todo-description-override",
|
||||||
"webfetch-redirect-guard",
|
"webfetch-redirect-guard",
|
||||||
|
"fsync-skip-warning",
|
||||||
"legacy-plugin-toast",
|
"legacy-plugin-toast",
|
||||||
])
|
])
|
||||||
|
|
||||||
|
|||||||
@@ -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 { createTodoDescriptionOverrideHook } from "./todo-description-override"
|
||||||
export { createWebFetchRedirectGuardHook } from "./webfetch-redirect-guard"
|
export { createWebFetchRedirectGuardHook } from "./webfetch-redirect-guard"
|
||||||
export { createLegacyPluginToastHook } from "./legacy-plugin-toast"
|
export { createLegacyPluginToastHook } from "./legacy-plugin-toast"
|
||||||
|
export { createFsyncSkipWarningHook } from "./fsync-skip-warning"
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ import {
|
|||||||
createTodoDescriptionOverrideHook,
|
createTodoDescriptionOverrideHook,
|
||||||
createWebFetchRedirectGuardHook,
|
createWebFetchRedirectGuardHook,
|
||||||
createTeamToolGating,
|
createTeamToolGating,
|
||||||
|
createFsyncSkipWarningHook,
|
||||||
} from "../../hooks"
|
} from "../../hooks"
|
||||||
import {
|
import {
|
||||||
getOpenCodeVersion,
|
getOpenCodeVersion,
|
||||||
@@ -42,6 +43,7 @@ export type ToolGuardHooks = {
|
|||||||
readImageResizer: ReturnType<typeof createReadImageResizerHook> | null
|
readImageResizer: ReturnType<typeof createReadImageResizerHook> | null
|
||||||
todoDescriptionOverride: ReturnType<typeof createTodoDescriptionOverrideHook> | null
|
todoDescriptionOverride: ReturnType<typeof createTodoDescriptionOverrideHook> | null
|
||||||
webfetchRedirectGuard: ReturnType<typeof createWebFetchRedirectGuardHook> | null
|
webfetchRedirectGuard: ReturnType<typeof createWebFetchRedirectGuardHook> | null
|
||||||
|
fsyncSkipWarning: ReturnType<typeof createFsyncSkipWarningHook> | null
|
||||||
teamToolGating: ReturnType<typeof createTeamToolGating> | null
|
teamToolGating: ReturnType<typeof createTeamToolGating> | null
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -139,6 +141,10 @@ export function createToolGuardHooks(args: {
|
|||||||
? safeHook("team-tool-gating", () => createTeamToolGating(ctx, pluginConfig.team_mode))
|
? safeHook("team-tool-gating", () => createTeamToolGating(ctx, pluginConfig.team_mode))
|
||||||
: null
|
: null
|
||||||
|
|
||||||
|
const fsyncSkipWarning = isHookEnabled("fsync-skip-warning")
|
||||||
|
? safeHook("fsync-skip-warning", () => createFsyncSkipWarningHook())
|
||||||
|
: null
|
||||||
|
|
||||||
return {
|
return {
|
||||||
commentChecker,
|
commentChecker,
|
||||||
toolOutputTruncator,
|
toolOutputTruncator,
|
||||||
@@ -154,6 +160,7 @@ export function createToolGuardHooks(args: {
|
|||||||
readImageResizer,
|
readImageResizer,
|
||||||
todoDescriptionOverride,
|
todoDescriptionOverride,
|
||||||
webfetchRedirectGuard,
|
webfetchRedirectGuard,
|
||||||
|
fsyncSkipWarning,
|
||||||
teamToolGating,
|
teamToolGating,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -153,6 +153,7 @@ export function createToolExecuteAfterHandler(args: {
|
|||||||
await hooks.readImageResizer?.["tool.execute.after"]?.(hookInput, output)
|
await hooks.readImageResizer?.["tool.execute.after"]?.(hookInput, output)
|
||||||
await hooks.hashlineReadEnhancer?.["tool.execute.after"]?.(hookInput, output)
|
await hooks.hashlineReadEnhancer?.["tool.execute.after"]?.(hookInput, output)
|
||||||
await hooks.webfetchRedirectGuard?.["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)
|
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.directoryReadmeInjector?.["tool.execute.before"]?.(input, output)
|
||||||
await hooks.rulesInjector?.["tool.execute.before"]?.(input, output)
|
await hooks.rulesInjector?.["tool.execute.before"]?.(input, output)
|
||||||
await hooks.tasksTodowriteDisabler?.["tool.execute.before"]?.(input, output)
|
await hooks.tasksTodowriteDisabler?.["tool.execute.before"]?.(input, output)
|
||||||
await hooks.webfetchRedirectGuard?.["tool.execute.before"]?.(input, output)
|
await hooks.webfetchRedirectGuard?.["tool.execute.before"]?.(input, output)
|
||||||
await hooks.prometheusMdOnly?.["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.sisyphusJuniorNotepad?.["tool.execute.before"]?.(input, output)
|
||||||
await hooks.atlasHook?.["tool.execute.before"]?.(input, output)
|
await hooks.atlasHook?.["tool.execute.before"]?.(input, output)
|
||||||
await hooks.teamToolGating?.["tool.execute.before"]?.(input, output)
|
await hooks.teamToolGating?.["tool.execute.before"]?.(input, output)
|
||||||
|
|||||||
@@ -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")
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user