fix(B-0272): address ROM tool review nits

docs/backlog/P1/B-0272-atari-rom-canonical-naming-tosec-lookup-2026-05-08.md

@@ -4,7 +4,7 @@ priority: P1
 status: open
 title: "Atari 2600 ROM canonical naming via TOSEC/No-Intro hash lookup"
 created: 2026-05-08
-last_updated: 2026-05-08
+last_updated: 2026-05-09
 parent: B-0083
 depends_on: []
 classification: buildable-now

tools/roms/canonicalize.test.ts

@@ -2,7 +2,13 @@ import { describe, expect, test } from "bun:test";
 import { existsSync, mkdtempSync, writeFileSync, readFileSync } from "node:fs";
 import { join } from "node:path";
 import { tmpdir } from "node:os";
-import { parseDatfile, hashFileSha1, scanRomFiles, matchAndReport } from "./canonicalize.ts";
+import {
+  main,
+  parseDatfile,
+  hashFileSha1,
+  scanRomFiles,
+  matchAndReport,
+} from "./canonicalize.ts";
 
 const FIXTURE_DATFILE = `<?xml version="1.0"?>
 <!DOCTYPE datafile SYSTEM "http://www.logiqx.com/Dats/datafile.dtd">
@@ -12,7 +18,7 @@ const FIXTURE_DATFILE = `<?xml version="1.0"?>
   </header>
   <game name="Combat (1977)(Atari)">
     <description>Combat (1977)(Atari)</description>
-    <rom name="Combat (1977)(Atari).bin" size="2048" crc="4020b4fe" md5="b35e9442525e1a0b30dc0264c tried" sha1="da39a3ee5e6b4b0d3255bfef95601890afd80709" />
+    <rom name="Combat (1977)(Atari).bin" size="2048" crc="4020b4fe" md5="b35e9442525e1a0b30dc0264c112233" sha1="da39a3ee5e6b4b0d3255bfef95601890afd80709" />
   </game>
   <game name="Adventure (1980)(Atari)">
     <description>Adventure (1980)(Atari)</description>
@@ -92,6 +98,8 @@ describe("scanRomFiles", () => {
     const tmp = mkdtempSync(join(tmpdir(), "rom-scan-"));
     writeFileSync(join(tmp, "notes.txt"), "data");
     writeFileSync(join(tmp, "save.sav"), "data");
+    writeFileSync(join(tmp, "README"), "data");
+    writeFileSync(join(tmp, ".DS_Store"), "data");
 
     const files = scanRomFiles(tmp);
     expect(files.length).toBe(0);
@@ -179,3 +187,27 @@ describe("matchAndReport", () => {
     expect(results[0]?.renamed).toBe(false);
   });
 });
+
+describe("main", () => {
+  test("reports missing datfile value clearly", () => {
+    const originalStderrWrite = process.stderr.write;
+    const originalExit = process.exit;
+    let stderr = "";
+
+    process.stderr.write = ((chunk: string | Uint8Array) => {
+      stderr += String(chunk);
+      return true;
+    }) as typeof process.stderr.write;
+    process.exit = ((code?: number) => {
+      throw new Error(`exit:${code}`);
+    }) as typeof process.exit;
+
+    try {
+      expect(() => main(["--datfile", "--dir", "/tmp"])).toThrow("exit:64");
+      expect(stderr).toContain("missing value for --datfile");
+    } finally {
+      process.stderr.write = originalStderrWrite;
+      process.exit = originalExit;
+    }
+  });
+});

tools/roms/canonicalize.ts

@@ -6,17 +6,21 @@
 //   bun tools/roms/canonicalize.ts --datfile <path.dat> --dir <rom-dir>
 //   bun tools/roms/canonicalize.ts --datfile <path.dat> --dir <rom-dir> --apply
 //
-// Output (default dry-run): JSON array of { file, sha1, match, canonicalName }.
+// Output (default dry-run): JSON array of
+// { file, sha1, matched, canonicalName, renamed }.
 // --apply: renames matched files to their canonical names.
 
 import { createHash } from "node:crypto";
 import {
+  closeSync,
+  openSync,
   readdirSync,
   readFileSync,
+  readSync,
   renameSync,
   existsSync,
 } from "node:fs";
-import { basename, extname, join } from "node:path";
+import { basename, dirname, extname, join } from "node:path";
 
 // --- Datfile parsing (Logiqx XML) ---
 
@@ -132,8 +136,21 @@ export function parseDatfile(xml: string): ReadonlyMap<string, DatEntry> {
 // --- File hashing ---
 
 export function hashFileSha1(path: string): string {
-  const data = readFileSync(path);
-  return createHash("sha1").update(data).digest("hex");
+  const hash = createHash("sha1");
+  const fd = openSync(path, "r");
+  const buffer = Buffer.allocUnsafe(1024 * 1024);
+  try {
+    let bytesRead = 0;
+    do {
+      bytesRead = readSync(fd, buffer, 0, buffer.length, null);
+      if (bytesRead > 0) {
+        hash.update(buffer.subarray(0, bytesRead));
+      }
+    } while (bytesRead > 0);
+  } finally {
+    closeSync(fd);
+  }
+  return hash.digest("hex");
 }
 
 // --- Directory scanning ---
@@ -143,7 +160,7 @@ export function scanRomFiles(dir: string): readonly string[] {
   for (const entry of readdirSync(dir, { withFileTypes: true })) {
     if (!entry.isFile()) continue;
     const ext = extname(entry.name).toLowerCase();
-    if (ROM_EXTENSIONS.has(ext) || ext === "") {
+    if (ROM_EXTENSIONS.has(ext)) {
       files.push(join(dir, entry.name));
     }
   }
@@ -189,7 +206,7 @@ export function matchAndReport(
     let renamed = false;
 
     if (apply && !alreadyCorrect) {
-      const dir = filePath.slice(0, filePath.length - currentName.length);
+      const dir = dirname(filePath);
       if (!isSafeCanonicalName(canonicalName)) {
         process.stderr.write(
           `skip: unsafe canonical name from datfile: ${canonicalName}\n`,
@@ -241,12 +258,23 @@ function parseArgs(argv: readonly string[]): Args {
   let dir: string | undefined;
   let apply = false;
 
+  function readOptionValue(index: number, flag: string): string {
+    const value = argv[index + 1];
+    if (value === undefined || value.startsWith("-")) {
+      process.stderr.write(`missing value for ${flag}\n`);
+      process.exit(64);
+    }
+    return value;
+  }
+
   for (let i = 0; i < argv.length; i++) {
     const arg = argv[i];
     if (arg === "--datfile" || arg === "-d") {
-      datfile = argv[++i];
+      datfile = readOptionValue(i, arg);
+      i++;
     } else if (arg === "--dir") {
-      dir = argv[++i];
+      dir = readOptionValue(i, arg);
+      i++;
     } else if (arg === "--apply") {
       apply = true;
     } else if (arg === "--help" || arg === "-h") {