fix: batch argv by command-line length, and leave a usable log when a run fails

.husky/commit-msg

@@ -0,0 +1 @@
+pnpm exec ai-watermark-guard --message "$1" --fix

.husky/pre-commit

@@ -0,0 +1 @@
+pnpm exec ai-watermark-guard --staged --fix

package.json

@@ -1,6 +1,6 @@
 {
   "name": "oxfmt-quick",
-  "version": "1.0.0",
+  "version": "1.0.1",
   "description": "Run oxfmt on your changed files, and re-stage them. A pretty-quick for the oxc formatter.",
   "keywords": [
     "formatter",
@@ -67,7 +67,8 @@
     "typecheck": "tsc --noEmit",
     "test": "vitest run",
     "test:coverage": "vitest run --coverage",
-    "prepublishOnly": "pnpm build"
+    "prepublishOnly": "pnpm build",
+    "prepare": "husky"
   },
   "dependencies": {
     "mri": "^1.2.0",
@@ -77,6 +78,7 @@
     "@types/node": "^26.2.0",
     "@vitest/coverage-v8": "^4.1.10",
     "ai-watermark-guard": "^0.2.0",
+    "husky": "^9.1.7",
     "oxfmt": "^0.63.0",
     "oxlint": "^1.78.0",
     "tsdown": "^0.22.14",

release-notes/1.0.1.md

@@ -0,0 +1,32 @@
+## oxfmt-quick@1.0.1
+
+Survives repositories with thousands of changed files, and leaves something useful behind
+when it fails.
+
+### Fixes
+
+- **`ENAMETOOLONG` with a long changed-file list is gone** ([#1]). Every oxfmt invocation
+  carried the whole file list in one spawn, and Windows caps a command line at 32767
+  characters - 2416 CI paths hit the limit before oxfmt ran. File lists are now split into
+  batches by cumulative command-line length and spawned one batch at a time:
+  `--list-different` merges what the batches report, formatting stops at the first failing
+  batch, and re-staging - already batched, but by a fixed count that deep CI paths could
+  still overrun - moves to the same length-based split. Thanks [@lukpsaxo] for the report.
+
+### Changes
+
+- **Failures explain themselves in CI.** When `CI` is set, or locally under `--verbose`,
+  an unexpected error prints its full stack instead of the one-line message - which is all
+  the report in [#1] had to go on.
+- **Every crash writes a report file to attach.** The temp directory gets
+  `oxfmt-quick-error.log` with the version, Node and platform, the exact command, the
+  working directory, and the whole error - including the `code` and `syscall` a bare stack
+  drops, with long argument lists capped so a spawn error stays readable. The CLI prints
+  the path; filing an issue is attaching that file.
+
+### Packaging
+
+No dependency, engine or export changes. `oxfmt` remains a peer dependency at `>=0.60.0`.
+
+[#1]: https://github.com/soroush-tech/oxfmt-quick/issues/1
+[@lukpsaxo]: https://github.com/lukpsaxo

src/batch.ts

@@ -0,0 +1,34 @@
+/**
+ * Split `files` into batches that fit on one command line.
+ *
+ * Windows caps the whole line handed to CreateProcess at 32767 UTF-16 units, far below
+ * the megabytes POSIX allows, so the Windows figure is the budget everywhere - a few
+ * extra spawns on Linux cost less than platform-dependent behaviour. `reserved` is what
+ * the invocation spends before any file: executable, leading arguments and flags.
+ *
+ * Each path is costed at its length plus three, for the separating space and the quotes
+ * the spawn layer may add around a path with spaces. A single path dearer than the whole
+ * budget still gets a batch of its own: it cannot be split, and the spawn failing loudly
+ * beats this module guessing.
+ */
+const COMMAND_LINE_LIMIT = 30_000
+
+export const batchFiles = (files: string[], reserved: number): string[][] => {
+  const budget = COMMAND_LINE_LIMIT - reserved
+  const batches: string[][] = []
+  let current: string[] = []
+  let spent = 0
+
+  for (const file of files) {
+    const cost = file.length + 3
+    if (current.length > 0 && spent + cost > budget) {
+      batches.push(current)
+      current = []
+      spent = 0
+    }
+    current.push(file)
+    spent += cost
+  }
+  if (current.length > 0) batches.push(current)
+  return batches
+}

src/cli.mts

@@ -1,6 +1,7 @@
 #!/usr/bin/env node
 import mri from 'mri'
 import pc from 'picocolors'
+import { writeErrorReport } from './errorReport.js'
 import { oxfmtQuick } from './index.js'
 
 const args = mri(process.argv.slice(2), {
@@ -35,12 +36,30 @@ if (args.help) {
   process.exit(0)
 }
 
-/** A CLI should explain what went wrong, not print a stack trace at someone. */
+/**
+ * Interactively, a CLI should explain what went wrong, not print a stack trace at
+ * someone. In CI there is no someone: the log is the whole debugging session, and the
+ * stack is the difference between a report that can be acted on and a bare ENAMETOOLONG.
+ * `--verbose` asks for the same detail locally. Either way the full report goes to a
+ * file, so "please attach the log" is a thing a bug template can ask for.
+ */
+const wantStack = args.verbose || Boolean(process.env.CI && process.env.CI !== 'false')
+
 const run = <T,>(work: () => T): T => {
   try {
     return work()
   } catch (error) {
-    console.error(`✗  ${error instanceof Error ? error.message : String(error)}`)
+    if (wantStack && error instanceof Error) {
+      console.error(`✗  ${error.stack ?? error.message}`)
+    } else {
+      console.error(`✗  ${error instanceof Error ? error.message : String(error)}`)
+      console.error(pc.dim('   Rerun with --verbose for the full error.'))
+    }
+    // This file and package.json are siblings of one directory, in src and dist alike.
+    const report = writeErrorReport(error, new URL('../package.json', import.meta.url))
+    if (report) {
+      console.error(pc.dim(`   Report written to ${report} - attach it when filing an issue.`))
+    }
     process.exit(1)
   }
 }

src/errorReport.ts

@@ -0,0 +1,56 @@
+import { readFileSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { inspect } from 'node:util'
+
+interface Manifest {
+  version?: string
+  bugs?: { url?: string }
+}
+
+/** The manifest is metadata for a crash report; failing to read it must not add a crash. */
+export const readManifest = (url: URL): Manifest => {
+  try {
+    return JSON.parse(readFileSync(url, 'utf8')) as Manifest
+  } catch {
+    return {}
+  }
+}
+
+/**
+ * An error report a user can attach to an issue as-is: what ran, where, on what, and the
+ * whole error. `inspect` keeps an `Error`'s extra properties (`code`, `syscall`) that
+ * `stack` alone drops, while the array cap stops a spawn error from dumping every one of
+ * the thousands of paths it carried.
+ */
+export const renderReport = (error: unknown, manifest: Manifest, now = new Date()): string =>
+  [
+    `oxfmt-quick error report - attach this file to an issue: ${manifest.bugs?.url ?? ''}`,
+    `time: ${now.toISOString()}`,
+    `oxfmt-quick: ${manifest.version ?? 'unknown'}`,
+    `node: ${process.version} on ${process.platform} ${process.arch}`,
+    `command: oxfmt-quick ${process.argv.slice(2).join(' ')}`,
+    `cwd: ${process.cwd()}`,
+    '',
+    inspect(error, { depth: 4, maxArrayLength: 20 }),
+    '',
+  ].join('\n')
+
+/**
+ * Write the report and return its path, or null when even that write fails: reporting on
+ * the crash should not crash. The temp directory rather than the repository, so the log
+ * can never end up staged in the very commit that failed.
+ */
+export const writeErrorReport = (
+  error: unknown,
+  manifestUrl: URL,
+  directory = tmpdir()
+): string | null => {
+  try {
+    const path = join(directory, 'oxfmt-quick-error.log')
+    writeFileSync(path, renderReport(error, readManifest(manifestUrl)))
+    return path
+  } catch {
+    return null
+  }
+}

src/oxfmt.ts

@@ -1,3 +1,4 @@
+import { batchFiles } from './batch'
 import type { CommandResult, Run } from './types'
 
 /** `command` is the argv prefix from `resolveOxfmt` - usually `[node, .../oxfmt/bin/oxfmt]`. */
@@ -13,14 +14,30 @@ const invoke = (
   return run(executable, [...leading, ...configArgs, ...args], root)
 }
 
+/**
+ * The file list split to fit one command line per call, after costing everything the
+ * invocation already spends: command, config and flags. A changed-file list can run to
+ * thousands of paths, which is more than Windows lets one spawn carry.
+ */
+const batches = (
+  command: string[],
+  config: string | undefined,
+  flags: string[],
+  files: string[]
+): string[][] => {
+  const fixed = [...command, ...(config ? ['--config', config] : []), ...flags]
+  const reserved = fixed.reduce((sum, arg) => sum + arg.length + 3, 0)
+  return batchFiles(files, reserved)
+}
+
 /**
  * Which of `files` oxfmt would rewrite.
  *
  * `--list-different` exits **1** when it finds anything, which is a report rather than a
  * failure - so the status is deliberately ignored and stdout is read either way.
  *
  * Batching here is why the tool stays fast. `pretty-quick` reads, formats and compares
- * each file itself in Node; oxfmt answers for the whole set in one Rust process, applying
+ * each file itself in Node; oxfmt answers for a whole batch in one Rust process, applying
  * its own config resolution and ignore rules as it goes - so there is no `.oxfmtrc` lookup
  * or `.gitignore` matching to reimplement.
  *
@@ -35,13 +52,18 @@ export const listDifferent = (
   config?: string
 ): string[] => {
   if (files.length === 0) return []
-  return invoke(run, command, root, ['--list-different', ...files], config)
-    .stdout.split('\n')
-    .map((line) => line.trim())
-    .filter(Boolean)
+  return batches(command, config, ['--list-different'], files).flatMap((batch) =>
+    invoke(run, command, root, ['--list-different', ...batch], config)
+      .stdout.split('\n')
+      .map((line) => line.trim())
+      .filter(Boolean)
+  )
 }
 
-/** Format in place. Returns false if oxfmt reported a failure. */
+/**
+ * Format in place. Returns false if oxfmt reported a failure, stopping at the failing
+ * batch - the same stop-and-report `stageFiles` does.
+ */
 export const format = (
   run: Run,
   command: string[],
@@ -50,5 +72,7 @@ export const format = (
   config?: string
 ): boolean => {
   if (files.length === 0) return true
-  return invoke(run, command, root, files, config).status === 0
+  return batches(command, config, [], files).every(
+    (batch) => invoke(run, command, root, batch, config).status === 0
+  )
 }

src/scm/git.ts

@@ -1,3 +1,4 @@
+import { batchFiles } from '../batch'
 import type { Run } from '../types'
 
 /**
@@ -53,12 +54,14 @@ export const getFilesSince = (run: Run, root: string, revision: string): string[
  * Stage in batches, and report whether every batch landed.
  *
  * A single `git add` with thousands of paths blows past the command-line length limit,
- * which is far lower on Windows (~32k) than on Unix.
+ * which is far lower on Windows (~32k) than on Unix. Batched by length, not count: a
+ * hundred deeply nested CI paths overrun the limit just as surely as a thousand short
+ * ones.
  */
 export const stageFiles = (run: Run, root: string, files: string[]): boolean => {
-  const BATCH = 100
-  for (let index = 0; index < files.length; index += BATCH) {
-    const { status } = run('git', ['add', '--', ...files.slice(index, index + BATCH)], root)
+  const RESERVED = 'git add --'.length
+  for (const batch of batchFiles(files, RESERVED)) {
+    const { status } = run('git', ['add', '--', ...batch], root)
     if (status !== 0) return false
   }
   return true

test/batch.spec.ts

@@ -0,0 +1,52 @@
+import { describe, expect, it } from 'vitest'
+import { batchFiles } from '../src/batch'
+
+// The budget is 30k minus `reserved`; each path costs its length plus three. Reserving
+// all but a sliver keeps the arithmetic in these tests small enough to do by eye.
+const reserving = (budget: number) => 30_000 - budget
+
+describe('batchFiles', () => {
+  it('returns nothing for an empty list', () => {
+    expect(batchFiles([], 0)).toEqual([])
+  })
+
+  it('keeps a list that fits in a single batch, in order', () => {
+    expect(batchFiles(['a.ts', 'b.ts', 'c.ts'], 0)).toEqual([['a.ts', 'b.ts', 'c.ts']])
+  })
+
+  it('starts a new batch when the next path would overrun the budget', () => {
+    // Each path costs 7 + 3 = 10; a budget of 20 holds exactly two.
+    const files = ['aaaa.ts', 'bbbb.ts', 'cccc.ts', 'dddd.ts', 'eeee.ts']
+    expect(batchFiles(files, reserving(20))).toEqual([
+      ['aaaa.ts', 'bbbb.ts'],
+      ['cccc.ts', 'dddd.ts'],
+      ['eeee.ts'],
+    ])
+  })
+
+  it('spends the reserved length before any file', () => {
+    // The same two-per-batch budget, eaten into by one more reserved character.
+    const files = ['aaaa.ts', 'bbbb.ts', 'cccc.ts']
+    expect(batchFiles(files, reserving(19))).toEqual([['aaaa.ts'], ['bbbb.ts'], ['cccc.ts']])
+  })
+
+  it('gives a path dearer than the whole budget a batch of its own', () => {
+    const huge = 'x'.repeat(50)
+    expect(batchFiles(['a.ts', huge, 'b.ts'], reserving(20))).toEqual([['a.ts'], [huge], ['b.ts']])
+  })
+
+  it('splits a realistic long list under the real limit', () => {
+    // 2416 files of CI-sized paths, the shape of the report that motivated this module.
+    const files = Array.from(
+      { length: 2416 },
+      (_, i) => `D:/Builds/agent/_work/src/deep/nested/module/file-${i}.ts`
+    )
+    const batches = batchFiles(files, 200)
+    expect(batches.length).toBeGreaterThan(1)
+    expect(batches.flat()).toEqual(files)
+    for (const batch of batches) {
+      const spent = batch.reduce((sum, file) => sum + file.length + 3, 0)
+      expect(spent).toBeLessThanOrEqual(30_000 - 200)
+    }
+  })
+})