fix(fs): detect an unwritable directory from the write, not from mkdir

.changeset/fs-drain-probe-the-write.md

@@ -0,0 +1,9 @@
+---
+"evlog": patch
+---
+
+The file system drain now detects an unwritable directory reliably.
+
+The previous check probed with `mkdir({ recursive: true })`, which is a no-op on a directory that already exists and therefore succeeds even when that directory is read-only. A deployment whose log directory already existed still threw on every batch. The probe also ran per call rather than per resolved state, so concurrent batches could each warn.
+
+The write itself is now the check: once an append fails with `EROFS`, `EACCES` or `EPERM`, the drain is disabled for that directory and warns once. Batches already in flight when that happens still attempt their own append. Any other failure, including a full disk, is reported by the drain as before.

.changeset/fs-drain-readonly-directory.md

@@ -11,4 +11,4 @@ The file system drain disables itself when its directory is not writable.
 const drain = process.env.VERCEL !== '1' ? createFsDrain() : undefined
 ```
 
-The drain now probes the directory once, and on `EROFS`, `EACCES` or `EPERM` warns a single time and stops, the same way it already bows out of the Edge runtime. Attach it unconditionally. Any other failure — a full disk, a genuine bug — still propagates.
+The drain now disables itself once it observes an `EROFS`, `EACCES` or `EPERM` write failure for that directory, warning a single time, the same way it already disables itself in the Edge runtime. Attach it unconditionally. Any other failure — a full disk, a genuine bug — still propagates.

packages/evlog/src/adapters/fs.ts

@@ -26,7 +26,7 @@ const FS_FIELDS: ConfigField<FsConfig>[] = [
 ]
 
 const gitignoreWritten = new Set<string>()
-const writableDirs = new Map<string, boolean>()
+const unwritableDirs = new Set<string>()
 let warnedFsEdgeRuntime = false
 
 function isEdgeRuntime(): boolean {
@@ -36,7 +36,7 @@ function isEdgeRuntime(): boolean {
 function warnFsEdgeRuntimeOnce(): void {
   if (warnedFsEdgeRuntime) return
   warnedFsEdgeRuntime = true
-  console.warn('[evlog/fs] File system drain is not available on the Edge runtime. Use evlog/memory or a HTTP adapter instead.')
+  console.warn('[evlog/fs] File system drain is not available on the Edge runtime. Use evlog/memory or an HTTP adapter instead.')
 }
 
 /** Read-only or permission-denied, as opposed to a full disk or a real bug. */
@@ -46,27 +46,15 @@ function isUnwritableError(error: unknown): boolean {
 }
 
 /**
- * Whether `dir` can be created and written to, probed once per directory.
+ * Record that `dir` cannot be written to, and say so once.
  *
- * Serverless hosts mount everything outside the temp directory read-only, so a
- * drain attached there would otherwise throw on every batch for the lifetime of
- * the deployment. Anything other than a permission failure still propagates.
+ * Synchronous on purpose: concurrent batches that all fail their first write
+ * reach this together, and the check-then-add pair cannot interleave.
  */
-async function isDirWritable(dir: string): Promise<boolean> {
-  const cached = writableDirs.get(dir)
-  if (cached !== undefined) return cached
-
-  try {
-    await mkdir(dir, { recursive: true })
-  } catch (error) {
-    if (!isUnwritableError(error)) throw error
-    writableDirs.set(dir, false)
-    console.warn(`[evlog/fs] "${dir}" is not writable, so the file system drain is disabled. This is expected on a serverless host, where only the temp directory is writable and does not outlive the instance — send events to a HTTP adapter instead, or set the drain's \`dir\` to a writable path.`)
-    return false
-  }
-
-  writableDirs.set(dir, true)
-  return true
+function markUnwritable(dir: string): void {
+  if (unwritableDirs.has(dir)) return
+  unwritableDirs.add(dir)
+  console.warn(`[evlog/fs] "${dir}" is not writable, so the file system drain is disabled. This is expected on a serverless host, where only the temp directory is writable and does not outlive the instance — send events to an HTTP adapter instead, or set the drain's \`dir\` to a writable path.`)
 }
 
 async function ensureGitignore(dir: string): Promise<void> {
@@ -176,6 +164,12 @@ export async function writeBatchToFs(events: WideEvent[], config: FsConfig): Pro
  *   pretty: true,
  * }))
  * ```
+ *
+ * @remarks
+ * A write that fails with `EROFS`, `EACCES` or `EPERM` marks the configured
+ * directory unavailable and disables the drain for the rest of the process,
+ * after warning once. Attaching it on a host without a writable directory is
+ * therefore safe, though the events go nowhere.
  */
 export function createFsDrain(overrides?: Partial<FsConfig>) {
   return defineDrain<FsConfig>({
@@ -187,15 +181,25 @@ export function createFsDrain(overrides?: Partial<FsConfig>) {
       }
       const resolved = await resolveAdapterConfig<FsConfig>('fs', FS_FIELDS, overrides)
       const dir = resolved.dir ?? '.evlog/logs'
-      if (!await isDirWritable(dir)) return null
+      if (unwritableDirs.has(dir)) return null
       return {
         dir,
         pretty: resolved.pretty ?? false,
         maxFiles: resolved.maxFiles,
         maxSizePerFile: resolved.maxSizePerFile,
       }
     },
-    send: writeBatchToFs,
+    // The write itself is the probe. `mkdir` is a no-op on a directory that
+    // already exists, so it succeeds on a read-only one and proves nothing;
+    // only the append tells you whether this host will take the events.
+    send: async (events, config) => {
+      try {
+        await writeBatchToFs(events, config)
+      } catch (error) {
+        if (!isUnwritableError(error)) throw error
+        markUnwritable(config.dir)
+      }
+    },
   })
 }
 

packages/evlog/test/adapters/fs.test.ts

@@ -339,11 +339,10 @@ describe('fs adapter', () => {
 
     it('warns once and disables itself when the directory is read-only', async () => {
       const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
-      const readOnly = Object.assign(new Error('EROFS'), { code: 'EROFS' })
 
       try {
         vi.resetModules()
-        mockedMkdir.mockRejectedValue(readOnly)
+        mockedAppendFile.mockRejectedValue(Object.assign(new Error('EROFS'), { code: 'EROFS' }))
         const { createFsDrain: createFsDrainFresh } = await import('../../src/adapters/fs')
         const drain = createFsDrainFresh({ dir: '/var/task/.evlog/logs' })
 
@@ -352,23 +351,67 @@ describe('fs adapter', () => {
 
         expect(warnSpy).toHaveBeenCalledTimes(1)
         expect(warnSpy.mock.calls[0]?.[0]).toContain('not writable')
-        expect(mockedAppendFile).not.toHaveBeenCalled()
-        // Probed once, then answered from cache.
-        expect(mockedMkdir).toHaveBeenCalledTimes(1)
+        // The first batch attempts the write; the second never reaches it.
+        expect(mockedAppendFile).toHaveBeenCalledTimes(1)
+      } finally {
+        warnSpy.mockRestore()
+      }
+    })
+
+    it('disables itself when the directory exists but rejects the write', async () => {
+      const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+
+      try {
+        vi.resetModules()
+        // `mkdir` is a no-op on an existing directory, so it succeeds even when
+        // that directory is read-only. Only the append reveals it.
+        mockedMkdir.mockResolvedValue(undefined)
+        mockedAppendFile.mockRejectedValue(Object.assign(new Error('EACCES'), { code: 'EACCES' }))
+        const { createFsDrain: createFsDrainFresh } = await import('../../src/adapters/fs')
+        const drain = createFsDrainFresh({ dir: '/var/task/.evlog/logs' })
+
+        await drain(createDrainContext())
+        await drain(createDrainContext())
+
+        expect(warnSpy).toHaveBeenCalledTimes(1)
+        expect(mockedAppendFile).toHaveBeenCalledTimes(1)
+      } finally {
+        warnSpy.mockRestore()
+      }
+    })
+
+    it('warns once when concurrent batches all fail their first write', async () => {
+      const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})
+
+      try {
+        vi.resetModules()
+        mockedAppendFile.mockRejectedValue(Object.assign(new Error('EROFS'), { code: 'EROFS' }))
+        const { createFsDrain: createFsDrainFresh } = await import('../../src/adapters/fs')
+        const drain = createFsDrainFresh({ dir: '/var/task/.evlog/logs' })
+
+        await Promise.all([
+          drain(createDrainContext({ action: 'a' })),
+          drain(createDrainContext({ action: 'b' })),
+          drain(createDrainContext({ action: 'c' })),
+        ])
+
+        expect(warnSpy).toHaveBeenCalledTimes(1)
       } finally {
         warnSpy.mockRestore()
       }
     })
 
-    it('propagates a write failure that is not a permission problem', async () => {
+    it('reports a write failure that is not a permission problem', async () => {
       vi.resetModules()
-      mockedMkdir.mockRejectedValue(Object.assign(new Error('ENOSPC'), { code: 'ENOSPC' }))
+      mockedAppendFile.mockRejectedValue(Object.assign(new Error('ENOSPC'), { code: 'ENOSPC' }))
       const { createFsDrain: createFsDrainFresh } = await import('../../src/adapters/fs')
       const drain = createFsDrainFresh({ dir: '.evlog/logs' })
       const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {})
 
       try {
-        await expect(drain(createDrainContext())).rejects.toThrow('ENOSPC')
+        await drain(createDrainContext())
+        expect(errorSpy).toHaveBeenCalled()
+        expect(String(errorSpy.mock.calls[0]?.[1])).toContain('ENOSPC')
       } finally {
         errorSpy.mockRestore()
       }