From 6b69505940e51e43e91a7bd6af0bb372dca57773 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Fri, 8 May 2026 15:08:19 +0900 Subject: [PATCH] feat(shared): wire tolerantFsync to record skips with path classification Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- src/shared/tolerant-fsync.test.ts | 25 ++++++++++++++++++- src/shared/tolerant-fsync.ts | 41 ++++++++++++++++++++++++++++--- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/src/shared/tolerant-fsync.test.ts b/src/shared/tolerant-fsync.test.ts index f7be152dd..0c785ec2e 100644 --- a/src/shared/tolerant-fsync.test.ts +++ b/src/shared/tolerant-fsync.test.ts @@ -1,7 +1,8 @@ -import { describe, expect, it } from "bun:test" +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 { @@ -60,6 +61,10 @@ describe("isToleratedFsyncError", () => { }) 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() @@ -100,6 +105,24 @@ describe("tolerantFsync (async)", () => { 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)", () => { diff --git a/src/shared/tolerant-fsync.ts b/src/shared/tolerant-fsync.ts index 00612ee3c..e47b791b5 100644 --- a/src/shared/tolerant-fsync.ts +++ b/src/shared/tolerant-fsync.ts @@ -1,6 +1,8 @@ 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 = new Set([ @@ -16,6 +18,13 @@ export function isToleratedFsyncError(error: unknown): boolean { 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, @@ -24,11 +33,23 @@ export async function tolerantFsync( 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: (error as NodeJS.ErrnoException).code, - message: error instanceof Error ? error.message : String(error), + code: errorCode, + message, + }) + + recordFsyncSkip({ + filePath, + contextLabel, + errorCode, + message, + pathClassification: classifyPathEnvironment(filePath), }) } } @@ -42,11 +63,23 @@ export function tolerantFsyncSync( 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: (error as NodeJS.ErrnoException).code, - message: error instanceof Error ? error.message : String(error), + code: errorCode, + message, + }) + + recordFsyncSkip({ + filePath, + contextLabel, + errorCode, + message, + pathClassification: classifyPathEnvironment(filePath), }) } }