Write JSON, CSV and the sidecar atomically

CHANGELOG.md

@@ -12,6 +12,42 @@ from a row count, it leads the section in a blockquote.
 
 ### Fixed
 
+- **An interrupted write destroyed the previous good run's output.** The
+  audit's second finding, and real. `open(path, "w")` truncates before a
+  single byte is written, so a crash, a kill or a full disk partway through
+  `json.dump` left a SHORTER file where a complete one had been. Measured
+  here before the fix: a 204,292-byte `out.json` came back **28 bytes and
+  invalid JSON**.
+
+  That breaks the same promise `save` keeps when it refuses to overwrite
+  good output with an empty result — by a different route, with the previous
+  run destroyed by the ATTEMPT to replace it rather than by its outcome. The
+  sidecar is the part that matters most and the part the audit did not
+  mention: `<out>.meta.json` is the file a consumer branches on, so a
+  truncated one beside good rows reads as a broken run over data that is
+  fine.
+
+  All three writers now go through one `_atomic` helper — a temporary file
+  in the TARGET's own directory (`os.replace` is atomic only within one
+  filesystem), `flush` + `fsync` before the rename, and the temp file
+  unlinked on any exception including `KeyboardInterrupt`.
+
+  Lifted from a sibling rather than written here, because the one part that
+  is easy to get wrong was got wrong across the family: `NamedTemporaryFile`
+  creates its file `0600` and a rename keeps that, so an output nobody else
+  can read is the default. Measured 2026-10-09 by CALLING each sibling's
+  writer and stat-ing the file it produced — **8 of 11 leave their output
+  0600**, one hardcodes `0644`, and two derive the mode from the umask. This
+  takes the third kind: a new file gets what `open()` would have given it,
+  and a target somebody tightened deliberately keeps its own mode.
+
+  `newline=""` is threaded through for the CSV, or the csv module doubles
+  the carriage return on every row.
+
+  Checked with its own control: the suite asserts that a truncating writer
+  really does destroy the file before asserting that this one does not, so
+  "the file survived" cannot be a property of the test.
+
 - **`--mode events` stopped at the first event page with no markets and
   reported the run as `complete`.** Found by a third-party audit of 8
   October and real. Reproduced against `main` end to end: one empty page at

output_writer.py

@@ -64,8 +64,12 @@
 README too, because a reader who joins on `sku` across a re-slug gets two
 rows and no warning.
 """
+import contextlib
 import csv
 import json
+import os
+import stat
+import tempfile
 from dataclasses import dataclass, asdict, field, fields
 from datetime import datetime, timezone
 from typing import Optional, List, Set, Sequence, Any, Type
@@ -330,8 +334,68 @@ def _csv_value(v: Any) -> Any:
     return v
 
 
+@contextlib.contextmanager
+def _atomic(path: str, newline: Optional[str] = None):
+    """Write to a temporary file beside `path`, then rename over it.
+
+    LIFTED from a sibling repo rather than written here. This is an
+    un-propagated family fix, not a design question, and the one part of it
+    that is easy to get wrong was got wrong in eight sibling repos — see the
+    mode note below. Measured across the family on 2026-10-09 by CALLING
+    each sibling's writer and stat-ing the file it produced: 8 of 11 atomic
+    implementations leave their output 0600, one hardcodes 0644, and two
+    derive it from the umask. This is one of the two.
+
+    Plain truncating `open` empties the file before a single byte is
+    written, so a crash, a kill or a full disk halfway through `json.dump`
+    leaves a SHORTER file where a complete one was. That breaks the promise
+    `save` keeps when it refuses to overwrite good output with an empty
+    result, by a different route: the previous good run is destroyed by the
+    ATTEMPT to replace it rather than by its outcome.
+
+    The sidecar matters most, and the audit that raised this did not mention
+    it: `<out>.meta.json` is the file a consumer branches on, so a truncated
+    one beside good rows reads as a broken run over data that is fine.
+
+    The temporary file goes in the SAME directory on purpose: `os.replace`
+    is atomic only within one filesystem, and a temp file under /tmp can be
+    on another one, where the rename degrades to an interruptible copy.
+    `fsync` before the rename makes the content durable, not merely visible.
+
+    `NamedTemporaryFile` creates the file 0600 and a rename keeps that, so
+    the mode is set explicitly: an existing target keeps its own (someone
+    may have tightened it deliberately), and a new one gets what `open()`
+    would have given it. A flat 0644 would be wrong in both directions.
+    """
+    directory = os.path.dirname(os.path.abspath(path)) or "."
+    handle = tempfile.NamedTemporaryFile(
+        mode="w", encoding="utf-8", newline=newline, dir=directory,
+        prefix=os.path.basename(path) + ".", suffix=".tmp", delete=False)
+    try:
+        with handle:
+            yield handle
+            handle.flush()
+            os.fsync(handle.fileno())
+        try:
+            mode = stat.S_IMODE(os.stat(path).st_mode)
+        except OSError:
+            umask = os.umask(0)
+            os.umask(umask)
+            mode = 0o666 & ~umask
+        os.chmod(handle.name, mode)
+        os.replace(handle.name, path)
+    except BaseException:
+        # Including KeyboardInterrupt and SystemExit: an interrupted run must
+        # not leave a stray .tmp beside the output it did not replace.
+        try:
+            os.unlink(handle.name)
+        except OSError:
+            pass
+        raise
+
+
 def write_json(rows: Sequence[Any], path: str) -> None:
-    with open(path, "w", encoding="utf-8") as f:
+    with _atomic(path) as f:
         json.dump([asdict(r) for r in rows], f, ensure_ascii=False, indent=2)
 
 
@@ -344,7 +408,7 @@ def write_csv(rows: Sequence[Any], path: str, row_cls: Type = Product) -> None:
     # The header comes from `row_cls`, not from the first row, so an empty
     # run still writes the columns of the mode that produced it.
     fieldnames = [f.name for f in fields(row_cls)]
-    with open(path, "w", encoding="utf-8", newline="") as f:
+    with _atomic(path, newline="") as f:
         writer = csv.DictWriter(f, fieldnames=fieldnames)
         writer.writeheader()
         for r in rows:
@@ -433,7 +497,7 @@ def write_run_meta(out_prefix: str, meta: dict) -> str:
     both complete, and between runs of different `mode`.
     """
     path = f"{out_prefix}.meta.json"
-    with open(path, "w", encoding="utf-8") as f:
+    with _atomic(path) as f:
         json.dump(meta, f, ensure_ascii=False, indent=2)
     print(f"[+] Wrote run metadata -> {path} (status={meta.get('status')})")
     return path

smoke_test.py

@@ -1135,6 +1135,137 @@ def test_one_page_is_complete_not_partial():
     return ok
 
 
+def test_writes_are_atomic():
+    group("A write that dies partway leaves the previous good file alone")
+    import contextlib
+    import json as _json
+    import output_writer
+    import stat as _stat
+    import tempfile
+    ok = True
+
+    # The defect, stated as what it destroys: `open(path, "w")` truncates
+    # before a single byte is written, so a crash, a kill or a full disk
+    # halfway through `json.dump` leaves a SHORTER file where a complete one
+    # was. `save` already refuses to overwrite good output with an EMPTY
+    # result; this is the same promise broken by a different route — the
+    # previous good run destroyed by the attempt to replace it rather than
+    # by its outcome.
+    #
+    # Measured on this repo before the fix: a 204,292-byte good `out.json`
+    # came back 28 bytes and invalid.
+    rows = [Market(sku="%012x" % i, url="https://polymarket.com/event/e",
+                   title="market %d" % i) for i in range(200)]
+
+    class _Boom(Exception):
+        pass
+
+    def _die_partway(obj, fp, *a, **k):
+        # Signature matches json.dump(obj, fp, ...): the FILE is second.
+        fp.write('[\n  {"sku": "000000000000",\n')
+        raise _Boom("interrupted mid-write")
+
+    @contextlib.contextmanager
+    def _truncating(path, newline=None):
+        with open(path, "w", encoding="utf-8", newline=newline) as fh:
+            yield fh
+
+    def _survives(writer):
+        """Good file size, size after an interrupted write, stray files."""
+        directory = tempfile.mkdtemp()
+        target = os.path.join(directory, "out.json")
+        real_atomic, real_dump = output_writer._atomic, _json.dump
+        output_writer._atomic = writer
+        try:
+            output_writer.write_json(rows, target)
+            good = os.path.getsize(target)
+            _json.dump = _die_partway
+            try:
+                output_writer.write_json(rows, target)
+            except _Boom:
+                pass
+            finally:
+                _json.dump = real_dump
+            after = os.path.getsize(target)
+            try:
+                _json.load(open(target, encoding="utf-8"))
+                valid = True
+            except Exception:  # noqa: BLE001 — "did it parse" is the question
+                valid = False
+            strays = [n for n in os.listdir(directory) if n != "out.json"]
+            return good, after, valid, strays
+        finally:
+            output_writer._atomic = real_atomic
+
+    # The CONTROL first: without it, "the file survived" could be a property
+    # of the test rather than of the writer.
+    good, after, valid, _ = _survives(_truncating)
+    ok &= check("control: a truncating writer really does destroy it "
+                "(%d bytes -> %d, parses: %s)" % (good, after, valid),
+                after < good and not valid)
+
+    good, after, valid, strays = _survives(output_writer._atomic)
+    ok &= check("the previous good file survives an interrupted write",
+                after == good and valid)
+    ok &= check("...and no half-written temp file is left beside it",
+                strays == [])
+
+    # The half eight sibling repos get wrong. NamedTemporaryFile creates its
+    # file 0600 and a rename keeps that, so an output nobody else can read
+    # is the default unless the mode is set back deliberately.
+    directory = tempfile.mkdtemp()
+    target = os.path.join(directory, "mode.json")
+    previous = os.umask(0o022)
+    try:
+        output_writer.write_json(rows, target)
+        mode = _stat.S_IMODE(os.stat(target).st_mode)
+        ok &= check("a new output is readable, not 0600 (got %s under umask "
+                    "022)" % oct(mode), mode == 0o644)
+        os.chmod(target, 0o600)
+        output_writer.write_json(rows, target)
+        kept = _stat.S_IMODE(os.stat(target).st_mode)
+        ok &= check("...and a target somebody tightened keeps its own mode",
+                    kept == 0o600)
+    finally:
+        os.umask(previous)
+
+    # `newline=""` has to survive the move into the temp file, or the csv
+    # module's own "\r\n" is translated again and every row gains a doubled
+    # carriage return.
+    #
+    # The BEHAVIOUR cannot be tested here, and that is worth stating rather
+    # than leaving as a check that looks load-bearing: the translation only
+    # happens where os.linesep is "\r\n", so on Linux — and therefore on CI
+    # — dropping `newline=""` changes the bytes not at all. Planting that
+    # fault left the suite green, which is a hole in the suite and not a
+    # pass. So the row count below is a sanity check, and the assertion that
+    # can actually fail is the WIRING one after it.
+    csv_path = os.path.join(directory, "out.csv")
+    output_writer.write_csv(rows[:3], csv_path, row_cls=Market)
+    raw = open(csv_path, "rb").read()
+    ok &= check("the CSV has one line terminator per row plus the header",
+                raw.count(b"\r\n") == 4 and raw.count(b"\r\r\n") == 0)
+
+    # And the wiring: a writer that quietly goes back to `open` would pass
+    # every check above that does not call it.
+    source = open(os.path.join(REPO_ROOT, "output_writer.py"),
+                  encoding="utf-8").read()
+    tree = ast.parse(source)
+    for node in ast.walk(tree):
+        if not isinstance(node, ast.FunctionDef):
+            continue
+        if node.name not in ("write_json", "write_csv", "write_run_meta"):
+            continue
+        body = ast.get_source_segment(source, node) or ""
+        ok &= check("%s() writes through _atomic" % node.name,
+                    "_atomic(" in body and 'open(' not in body)
+        if node.name == "write_csv":
+            # The one that a Linux run cannot catch by behaviour.
+            ok &= check("...and hands it newline='' for the csv module",
+                        'newline=""' in body)
+    return ok
+
+
 def test_output_contract():
     group("The output contract (§9)")
     ok = True
@@ -2857,6 +2988,7 @@ def main() -> int:
     ok &= test_page_flow_policy()
     ok &= test_scroll_loop()
     ok &= test_one_page_is_complete_not_partial()
+    ok &= test_writes_are_atomic()
     ok &= test_output_contract()
     ok &= test_writers_and_finish_run()
     ok &= test_diff()