From 0a3665a5a07f00fdd3f1e8d5ffa26a6575a107fd Mon Sep 17 00:00:00 2001 From: drknowhow Date: Sat, 5 Sep 2026 07:13:55 -0400 Subject: [PATCH] fix(atomic): the WinError 5 race had four more copies, including the lock store (2.118.2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 2.118.1 fixed the publish step in the shell job store: a shared `.tmp` plus a single `os.replace`, which on Windows can publish a half-written file or raise `PermissionError: [WinError 5] Access is denied`. That shape had been copied into four other small JSON stores, and fixing one copy of a bug is not fixing the bug. All five now go through one helper, `services/atomic_json.py`: - `services/agent_locks.py` — the lock store, and the most exposed of the four: written from a threaded server, `locks.json.tmp` is a name every writer on the box picks, and a lost write drops live leases. That is exactly what its loader refuses to do when it quarantines a corrupt file rather than resetting it to empty. - `services/access_guard._write_scope_config` — the CLI's `config.json`. - `cli/hub_server.py` — the hub's config route, writing the same file. - `oracle/services/mobile_api.py` — the mobile override-config writer, ditto. The last three write one file from three processes, one of them threaded. None had a reported failure. That is a statement about how often they are written, not about whether the race is real — the job store's version was also unreported until it started failing CI. `write_json_atomic` also fsyncs the temp before the replace, so a crash between the two cannot leave a zero-length `config.json`. The Access Guard reads a truncated config as `` and fails every path closed, which wedges the session that would otherwise fix it. The shell job store opts out (`fsync=False`): its records are ephemeral spill state that a crash invalidates anyway. Serialisation is unchanged at all five sites — indent, `ensure_ascii` and the trailing newline are per-caller arguments, so no file on disk reformats. `cli/_hook_utils._atomic_write_json` keeps its own copy on purpose: a PreToolUse hook is a separate short-lived single-threaded process that imports nothing from `services`, so pid-only is genuinely enough there. tests/test_atomic_json.py covers temp uniqueness (per write, not per pid — threads share a pid), retry vs. re-raise, no orphaned temps, serialisation parity, the fsync policy, a 6-thread hammer, and a source scan asserting no call site re-derives the shape. Local: 3675 passed / 6 skipped on Windows; ruff clean; container CI lint, jsx and test (ubuntu-latest, 3.12) all pass. Claude-Session: https://claude.ai/code/session_01JjAQC4Uv3aBnv9oQLPMBMy --- CHANGELOG.md | 41 ++++++ cli/c3.py | 2 +- cli/hub_server.py | 8 +- oracle/services/mobile_api.py | 10 +- pyproject.toml | 2 +- services/access_guard.py | 18 ++- services/agent_locks.py | 14 +- services/atomic_json.py | 118 +++++++++++++++++ services/shell_output.py | 72 +++------- tests/test_atomic_json.py | 240 ++++++++++++++++++++++++++++++++++ 10 files changed, 451 insertions(+), 74 deletions(-) create mode 100644 services/atomic_json.py create mode 100644 tests/test_atomic_json.py diff --git a/CHANGELOG.md b/CHANGELOG.md index c48ab90..e4d2e5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,47 @@ All notable changes to Code Context Control (C3) are documented here. The format is loosely based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.118.2] - 2026-09-05 + +### Fixed — the WinError 5 race had four more copies, including the lock store + +2.118.1 fixed the publish step in the shell job store: a shared `.tmp` +plus a single `os.replace`, which on Windows can publish a half-written file +or raise `PermissionError: [WinError 5] Access is denied`. That shape had been +copied into four other small JSON stores, and fixing one copy of a bug is not +fixing the bug. All five now go through one helper, `services/atomic_json.py`: + +- `services/agent_locks.py` — the lock store, and the most exposed of the + four: it is written from a threaded server, `locks.json.tmp` is a name every + writer on the box picks, and a lost write drops live leases. That is exactly + what its loader refuses to do when it quarantines a corrupt file rather than + resetting it to empty. +- `services/access_guard._write_scope_config` — the CLI's `config.json` writer. +- `cli/hub_server.py` — the hub's config route, writing the same file. +- `oracle/services/mobile_api.py` — the mobile override-config writer, ditto. + The last three write one file from three processes, one of them threaded. + +None had a reported failure. That is a statement about how often they are +written, not about whether the race is real — the job store's version was +also unreported until it started failing CI. + +### Changed — durable config is now flushed before it is published + +`write_json_atomic` fsyncs the temp before the replace, so a crash between the +two cannot leave a zero-length `config.json`. The Access Guard reads a +truncated config as `` and fails **every** path closed, which +wedges the session that would otherwise fix it — worth a flush that these +files take a few times a session. The shell job store opts out (`fsync=False`): +its records are ephemeral spill state that a crash invalidates anyway. + +Serialisation is unchanged at all five sites — indent, `ensure_ascii` and the +trailing newline are per-caller arguments, so no file on disk reformats. + +`cli/_hook_utils._atomic_write_json` still keeps its own copy: a PreToolUse +hook is a separate short-lived single-threaded process that imports nothing +from `services`, so pid-only is genuinely enough there. That one is deliberate +and is the only one left. + ## [2.118.1] - 2026-09-04 ### Fixed — the job store could crash a supervisor on Windows with WinError 5 diff --git a/cli/c3.py b/cli/c3.py index e11b9d6..08d2ab2 100644 --- a/cli/c3.py +++ b/cli/c3.py @@ -92,7 +92,7 @@ # Config CONFIG_DIR = ".c3" CONFIG_FILE = ".c3/config.json" -__version__ = "2.118.1" +__version__ = "2.118.2" def _compress_file_cli(compressor, path, mode="smart", **kw): diff --git a/cli/hub_server.py b/cli/hub_server.py index 69087c3..2dcad8d 100644 --- a/cli/hub_server.py +++ b/cli/hub_server.py @@ -32,6 +32,7 @@ from core.ide import PROFILES, detect_ide, get_profile, load_ide_config, normalize_ide_name from services import access_guard from services.activity_log import ActivityLog +from services.atomic_json import write_json_atomic from services.project_manager import ProjectManager from services.tool_classifier import CATEGORIES @@ -3078,9 +3079,10 @@ def api_projects_config_put(): else: merged[key] = val cfg[section] = merged - tmp = cfg_file.with_name("config.json.tmp") - tmp.write_text(json.dumps(cfg, indent=2), encoding="utf-8") - os.replace(tmp, cfg_file) + # Not `config.json.tmp`: the CLI (services.access_guard) and the mobile + # API write this same file, and the hub itself is threaded — one shared + # temp name is a name they all pick at once. See services.atomic_json. + write_json_atomic(cfg_file, cfg, trailing_newline=False) try: ActivityLog(str(resolved)).log("hub_config_write", { "section": section, "keys": sorted(values.keys()), "source": "hub"}) diff --git a/oracle/services/mobile_api.py b/oracle/services/mobile_api.py index 28d3df4..dc05f00 100644 --- a/oracle/services/mobile_api.py +++ b/oracle/services/mobile_api.py @@ -56,6 +56,7 @@ from oracle.services import api_auth, discovery_audit from oracle.services.api_auth import extract_bearer from oracle.services.tool_registry import _c3_version +from services.atomic_json import write_json_atomic # 2: /info reports EFFECTIVE capabilities (filtered by the config switches) # rather than a static list, so a client can gate its UI instead of probing. @@ -2090,10 +2091,11 @@ def _write_override_section(project: Path, section: dict) -> dict: else: block[key] = value cfg["override"] = block - tmp = cfg_file.with_name(f"{cfg_file.name}.tmp") - tmp.write_text(json.dumps(cfg, indent=2, ensure_ascii=False), - encoding="utf-8") - tmp.replace(cfg_file) + # Shared with the CLI and the hub, and served from a threaded API, so the + # publish goes through the one race-safe writer rather than a temp name + # every writer of this file would pick. See services.atomic_json. + write_json_atomic(cfg_file, cfg, ensure_ascii=False, + trailing_newline=False) return block diff --git a/pyproject.toml b/pyproject.toml index 0991c36..e9f4bd2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "code-context-control" -version = "2.118.1" +version = "2.118.2" description = "Local MCP code-intelligence for AI coding tools: surgical search/read/edit, agent-config version history, path-level access + masking guards, and a multi-project hub." readme = "README.md" requires-python = ">=3.10" diff --git a/services/access_guard.py b/services/access_guard.py index fb3e24b..78d679d 100644 --- a/services/access_guard.py +++ b/services/access_guard.py @@ -20,6 +20,8 @@ from dataclasses import dataclass from pathlib import Path +from services.atomic_json import write_json_atomic + # ── Rule model ────────────────────────────────────────────────────────────── _KIND_DENY = "deny" @@ -1473,11 +1475,17 @@ def _load_config_for_write(cfg: Path) -> tuple: def _write_scope_config(cfg: Path, data: dict) -> None: - """Atomic same-directory replace; privileged internal write.""" - cfg.parent.mkdir(parents=True, exist_ok=True) - tmp = cfg.with_name(cfg.name + ".tmp") - tmp.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") - os.replace(tmp, cfg) + """Atomic same-directory replace; privileged internal write. + + The CLI, the hub and the mobile API all reach this for the same scope + file, so the publish is via ``services.atomic_json`` — a shared + ``config.json.tmp`` is a name they would collide on. The fsync matters + more here than anywhere else in the codebase: a truncated ``config.json`` + comes back from ``load_all`` in its ``corrupt`` list, every path then + fails closed with ````, and the session can no longer run + the command that would fix it. + """ + write_json_atomic(cfg, data) def seed_default_global_rules() -> dict: diff --git a/services/agent_locks.py b/services/agent_locks.py index 70b27a8..17129ee 100644 --- a/services/agent_locks.py +++ b/services/agent_locks.py @@ -41,6 +41,7 @@ import uuid from pathlib import Path +from services.atomic_json import write_json_atomic from services.task_store import _FileLock SCHEMA_VERSION = 1 @@ -219,10 +220,15 @@ def _load(self) -> dict: return doc def _save(self, doc: dict) -> None: - self.data_dir.mkdir(parents=True, exist_ok=True) - tmp = self.state_file.with_name(self.state_file.name + ".tmp") - tmp.write_text(json.dumps(doc, indent=2) + "\n", encoding="utf-8") - os.replace(tmp, self.state_file) + # The `_FileLock` guard serialises the read-modify-write, but only + # among callers that take it — and it is advisory, held per process. + # The publish itself has to be safe on its own: a shared + # `locks.json.tmp` is one name every writer on the box picks, and two + # of them landing together either publishes a half-written lease file + # or raises WinError 5 (see services.atomic_json). Losing this write + # means losing live leases, which is exactly what `_load` refuses to + # do when it quarantines a corrupt file instead of resetting it. + write_json_atomic(self.state_file, doc) def _guard(self): return _FileLock(self.lock_file) diff --git a/services/atomic_json.py b/services/atomic_json.py new file mode 100644 index 0000000..bcc1b79 --- /dev/null +++ b/services/atomic_json.py @@ -0,0 +1,118 @@ +"""One durable same-directory replace for the small JSON stores C3 writes. + +Why this module exists +---------------------- +C3 keeps a handful of tiny JSON files that more than one writer touches: the +agent lock store, the per-scope ``config.json`` written by the CLI, the hub +and the mobile API, and the shell job store. Every one of them had grown the +same three-line publish step by copy — ``.tmp`` then a single +``os.replace`` — and on Windows that shape is a live bug rather than a style +nit: + +1. **A shared temp name races on the TEMP, not on the target.** + ``os.replace`` is atomic on the destination; nothing at all protects + ``.tmp``. Two writers of one path both create it, so one publishes + a file the other is still writing, or the replace raises + ``PermissionError: [WinError 5] Access is denied`` because the loser still + holds a handle to it. Observed for real in the shell job store, where it + crashed a supervisor and reported "failed before running" for a command + that was fine (fixed there in 2.118.1, PR #157). The other copies had the + identical shape and no demonstrated failure only because they are written + far less often — the lock store especially is a coin that had not landed + on its edge yet. +2. **A pid alone is not a unique suffix.** Threads share a pid, and the hub, + the Oracle and the lock store are all written from threaded servers. Six + threads are enough to drive a ``.tmp`` implementation into both + ``PermissionError`` and ``FileNotFoundError`` within a second, so the + suffix carries per-WRITE randomness as well. +3. **One attempt turns a transient sharing violation into a lost write.** An + AV scanner or the Search indexer holding the target open for a few + milliseconds is ordinary on Windows and clears on its own; a single + ``os.replace`` turns it into an exception at the call site. + +``cli/_hook_utils._atomic_write_json`` deliberately does NOT import this. A +PreToolUse hook is a separate, short-lived, single-threaded process that +imports nothing from ``services`` on purpose, so pid-only is genuinely +enough there. That one duplication stays. +""" + +from __future__ import annotations + +import json +import os +import secrets +import time +from pathlib import Path + +# Four attempts across ~0.14s of backoff. Sized for a scanner or indexer +# holding a handle, not for a lock a peer holds for seconds — a failure that +# survives this is real and is re-raised. +REPLACE_ATTEMPTS = 4 +REPLACE_BACKOFF_S = 0.02 + + +def atomic_tmp_path(path: Path) -> Path: + """``.tmp-`` beside ``path`` — unique per write.""" + return path.with_name(f"{path.name}.tmp{os.getpid()}-{secrets.token_hex(4)}") + + +def write_text_atomic(path, text: str, *, encoding: str = "utf-8", + fsync: bool = True) -> None: + """Publish ``text`` at ``path``: unique temp, then a retried ``os.replace``. + + ``fsync`` flushes the temp's bytes before the replace, so a crash or power + loss between the two cannot publish a zero-length or half-written file. + Durable state wants that: a truncated ``config.json`` makes the Access + Guard read ```` and fail closed on every subsequent call, + which wedges the whole session. Callers whose file is ephemeral spill + state pass ``fsync=False`` rather than pay a disk flush per update. + + Newline translation matches ``Path.write_text`` (the default), so this is + a drop-in for the ``write_text`` + ``os.replace`` pairs it replaces and + does not churn line endings in files already on disk. + """ + path = Path(path) + path.parent.mkdir(parents=True, exist_ok=True) + tmp = atomic_tmp_path(path) + try: + with open(tmp, "w", encoding=encoding) as fh: + fh.write(text) + if fsync: + fh.flush() + os.fsync(fh.fileno()) + last_exc: OSError | None = None + for attempt in range(REPLACE_ATTEMPTS): + try: + os.replace(tmp, path) + return + except PermissionError as exc: + last_exc = exc + if attempt < REPLACE_ATTEMPTS - 1: + time.sleep(REPLACE_BACKOFF_S * (2 ** attempt)) + if last_exc is not None: + raise last_exc + finally: + # A successful replace already consumed tmp, so this only fires on the + # failure paths — otherwise abandoned temps accumulate next to the + # file, and for `.c3/` that means they get indexed and committed. + try: + if tmp.exists(): + tmp.unlink() + except OSError: + pass + + +def write_json_atomic(path, data, *, indent: int = 2, + ensure_ascii: bool = True, + trailing_newline: bool = True, + fsync: bool = True) -> None: + """``write_text_atomic`` over ``json.dumps``. + + The serialisation knobs exist so each caller keeps the exact bytes it + wrote before this module: the point of the change is the publish step, + not a reformat of five JSON files that other tools and diffs already read. + """ + text = json.dumps(data, indent=indent, ensure_ascii=ensure_ascii) + if trailing_newline: + text += "\n" + write_text_atomic(path, text, fsync=fsync) diff --git a/services/shell_output.py b/services/shell_output.py index 506a82b..2677106 100644 --- a/services/shell_output.py +++ b/services/shell_output.py @@ -75,6 +75,8 @@ from datetime import datetime, timedelta, timezone from pathlib import Path +from services.atomic_json import write_json_atomic + __all__ = [ "STORE_ROOT", "RETENTION_DAYS", "RETENTION_BYTES", "HEAD_BYTES", "TAIL_BYTES", "TEXT_MAX_BYTES", "DEFAULT_READ_BYTES", "LINE_CLIP_CHARS", @@ -960,64 +962,22 @@ def _load_meta(path: Path) -> OutputMeta | None: return None -#: os.replace retries for when Windows briefly holds a handle on the target. -#: Same construction as ``cli/_hook_utils._atomic_write_json``; the hook layer -#: keeps its own copy on purpose (a PreToolUse subprocess imports nothing from -#: services), so the two are duplicated deliberately rather than by accident. -_REPLACE_ATTEMPTS = 4 -_REPLACE_BACKOFF_S = 0.02 - - def _write_json_atomic(path: Path, data: dict) -> None: - """Write JSON durably: unique temp file in the same directory + os.replace. - - Two bugs this fixes, both observed on Windows CI (2026-09-04, job store): - - 1. **A shared temp name.** ``.tmp`` is the same string for every - writer, so two processes writing the SAME job file — the parent that - creates the record and the supervisor that immediately saves status — - raced on the temp itself. One would be mid-write while the other tried - to publish it, and ``os.replace`` raised - ``PermissionError: [WinError 5] Access is denied``. The suffix is a pid - AND a random token, not a pid alone: this store is written from a - threaded server, and threads share a pid, so ``.tmp`` collides - exactly as the shared name did. With a per-WRITE temp the race is only - on the target, where ``os.replace`` is genuinely atomic. - 2. **No retry.** Even with unique temps, another process holding a read - handle on the target (an AV scanner, the Search indexer, a concurrent - reader) makes ``os.replace`` raise transiently. A single attempt turned - that into a crashed supervisor — the job never ran, and the caller got - "failed before running" for a command that was fine. - - A partial write can still be published without fsync, but this store is - ephemeral spill state rather than durable config, so the cost of an fsync - per status update is not worth paying here. + """Publish a job record durably — see ``services.atomic_json``. + + This was the first copy of the unique-temp + retried-``os.replace`` shape + (2.118.1, PR #157, after a shared ``.tmp`` crashed supervisors on + Windows CI with WinError 5 and reported "failed before running" for + commands that were fine). The implementation now lives in one place so the + lock store and the three ``config.json`` writers share it instead of + carrying four more copies of a bug. + + ``fsync=False`` is the one thing kept local: a job record is ephemeral + spill state that a crash invalidates anyway, so a disk flush per status + update buys nothing. The durable stores take the default. """ - path.parent.mkdir(parents=True, exist_ok=True) - tmp = path.with_name( - f"{path.name}.tmp{os.getpid()}-{secrets.token_hex(4)}") - try: - tmp.write_text(json.dumps(data, ensure_ascii=False, indent=1), - encoding="utf-8") - last_exc: OSError | None = None - for attempt in range(_REPLACE_ATTEMPTS): - try: - os.replace(tmp, path) - return - except PermissionError as exc: - last_exc = exc - if attempt < _REPLACE_ATTEMPTS - 1: - time.sleep(_REPLACE_BACKOFF_S * (2 ** attempt)) - if last_exc is not None: - raise last_exc - finally: - # A successful replace already consumed tmp, so this only fires on the - # failure paths — otherwise abandoned temps accumulate in the store. - try: - if tmp.exists(): - tmp.unlink() - except OSError: - pass + write_json_atomic(path, data, indent=1, ensure_ascii=False, + trailing_newline=False, fsync=False) def _move(src: Path, dst: Path) -> None: diff --git a/tests/test_atomic_json.py b/tests/test_atomic_json.py new file mode 100644 index 0000000..87f8908 --- /dev/null +++ b/tests/test_atomic_json.py @@ -0,0 +1,240 @@ +"""services.atomic_json — the one publish step for C3's small JSON stores. + +Every test here fails against the shape these call sites carried before +(`.tmp` + a single `os.replace`), which is the shape that crashed shell +job supervisors on Windows with `PermissionError: [WinError 5]`. The point of +the module is that the lock store and the three `config.json` writers stop +re-deriving it, so the last class checks each of those call sites actually +routes through it rather than only the helper being correct in isolation. +""" + +from __future__ import annotations + +import json +import os +import sys +import tempfile +import threading +import unittest +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) + +from services import atomic_json as aj # noqa: E402 + + +class TestTempNaming(unittest.TestCase): + """The temp must be unique per WRITE, not per path and not per process.""" + + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.path = Path(self._tmp.name) / "config.json" + + def tearDown(self): + self._tmp.cleanup() + + def _replace_spy(self): + seen: list[str] = [] + real = os.replace + + def spy(src, dst): + seen.append(Path(src).name) + return real(src, dst) + + return seen, spy + + def test_temp_is_not_the_shared_name(self): + seen, spy = self._replace_spy() + with patch.object(aj.os, "replace", spy): + aj.write_json_atomic(self.path, {"a": 1}) + self.assertEqual(len(seen), 1) + self.assertNotEqual(seen[0], "config.json.tmp", + "shared temp name — concurrent writers race on it") + self.assertTrue(seen[0].startswith("config.json.tmp")) + self.assertIn(str(os.getpid()), seen[0]) + + def test_temp_differs_between_writes_in_one_process(self): + """Threads share a pid, so `.tmp` collides exactly as `.tmp` did.""" + seen, spy = self._replace_spy() + with patch.object(aj.os, "replace", spy): + for i in range(5): + aj.write_json_atomic(self.path, {"a": i}) + self.assertEqual(len(set(seen)), 5, f"temp name reused: {seen}") + + +class TestReplaceRetry(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.path = Path(self._tmp.name) / "config.json" + + def tearDown(self): + self._tmp.cleanup() + + def test_transient_permission_error_is_retried(self): + calls = {"n": 0} + real = os.replace + + def flaky(src, dst): + calls["n"] += 1 + if calls["n"] < 3: # two transient failures, then success + raise PermissionError(5, "Access is denied") + return real(src, dst) + + with patch.object(aj.os, "replace", flaky): + aj.write_json_atomic(self.path, {"ok": True}) + self.assertEqual(calls["n"], 3) + self.assertEqual(json.loads(self.path.read_text(encoding="utf-8")), + {"ok": True}) + + def test_persistent_permission_error_still_raises(self): + """Retrying must not turn a real failure into a silently lost write.""" + def always(src, dst): + raise PermissionError(5, "Access is denied") + + with patch.object(aj.os, "replace", always): + with self.assertRaises(PermissionError): + aj.write_json_atomic(self.path, {"a": 1}) + + def test_a_non_permission_oserror_is_not_retried(self): + """Only the Windows sharing violation is transient; the rest surface.""" + calls = {"n": 0} + + def broken(src, dst): + calls["n"] += 1 + raise OSError(22, "Invalid argument") + + with patch.object(aj.os, "replace", broken): + with self.assertRaises(OSError): + aj.write_json_atomic(self.path, {"a": 1}) + self.assertEqual(calls["n"], 1) + + def test_no_temp_is_left_behind_when_replace_fails(self): + def always(src, dst): + raise PermissionError(5, "Access is denied") + + with patch.object(aj.os, "replace", always): + with self.assertRaises(PermissionError): + aj.write_json_atomic(self.path, {"a": 1}) + leftovers = list(self.path.parent.glob("config.json.tmp*")) + self.assertEqual(leftovers, [], f"orphaned temps: {leftovers}") + + +class TestSerialisationIsPreserved(unittest.TestCase): + """Each call site keeps the exact bytes it wrote before the refactor.""" + + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.path = Path(self._tmp.name) / "f.json" + + def tearDown(self): + self._tmp.cleanup() + + def test_default_is_indent_2_with_trailing_newline(self): + aj.write_json_atomic(self.path, {"a": 1}) + self.assertEqual(self.path.read_text(encoding="utf-8"), + '{\n "a": 1\n}\n') + + def test_trailing_newline_can_be_suppressed(self): + aj.write_json_atomic(self.path, {"a": 1}, trailing_newline=False) + self.assertEqual(self.path.read_text(encoding="utf-8"), + '{\n "a": 1\n}') + + def test_ensure_ascii_false_keeps_non_ascii_literal(self): + aj.write_json_atomic(self.path, {"k": "café"}, ensure_ascii=False) + self.assertIn("café", self.path.read_text(encoding="utf-8")) + + def test_parent_directory_is_created(self): + deep = Path(self._tmp.name) / "a" / "b" / "c.json" + aj.write_json_atomic(deep, {"a": 1}) + self.assertEqual(json.loads(deep.read_text(encoding="utf-8")), {"a": 1}) + + +class TestFsync(unittest.TestCase): + def setUp(self): + self._tmp = tempfile.TemporaryDirectory() + self.path = Path(self._tmp.name) / "f.json" + + def tearDown(self): + self._tmp.cleanup() + + def test_durable_by_default(self): + """A truncated config.json fails the guard closed; flush before publish.""" + with patch.object(aj.os, "fsync") as fs: + aj.write_json_atomic(self.path, {"a": 1}) + self.assertEqual(fs.call_count, 1) + + def test_opt_out_for_ephemeral_state(self): + with patch.object(aj.os, "fsync") as fs: + aj.write_json_atomic(self.path, {"a": 1}, fsync=False) + fs.assert_not_called() + + +class TestConcurrency(unittest.TestCase): + def test_threads_never_publish_a_truncated_file(self): + """The end-to-end property, and the one `.tmp` fails.""" + with tempfile.TemporaryDirectory() as td: + path = Path(td) / "locks.json" + errors: list[Exception] = [] + + def writer(i): + try: + for _ in range(20): + aj.write_json_atomic(path, {"writer": i}) + except Exception as exc: # noqa: BLE001 — surfaced below + errors.append(exc) + + threads = [threading.Thread(target=writer, args=(i,)) + for i in range(6)] + for t in threads: + t.start() + for t in threads: + t.join() + + self.assertEqual(errors, [], f"concurrent writes failed: {errors}") + doc = json.loads(path.read_text(encoding="utf-8")) + self.assertIn(doc["writer"], range(6)) + leftovers = list(path.parent.glob("locks.json.tmp*")) + self.assertEqual(leftovers, [], f"orphaned temps: {leftovers}") + + +class TestCallSitesUseIt(unittest.TestCase): + """The refactor's actual claim: no copy of the shape is left behind. + + A source scan rather than a behavioural probe, because the four sites sit + behind a Flask route, an advisory file lock and a scope resolver — and + because the thing being asserted IS textual: nobody re-derives the publish + step. `cli/_hook_utils` is exempt on purpose (a PreToolUse subprocess + imports nothing from `services`). + """ + + ROOT = Path(__file__).resolve().parents[1] + SITES = ( + "services/agent_locks.py", + "services/access_guard.py", + "services/shell_output.py", + "cli/hub_server.py", + "oracle/services/mobile_api.py", + ) + + def test_each_site_imports_the_helper(self): + for rel in self.SITES: + with self.subTest(file=rel): + src = (self.ROOT / rel).read_text(encoding="utf-8") + self.assertIn("from services.atomic_json import", src) + + def test_no_site_still_writes_a_shared_temp(self): + for rel in self.SITES: + with self.subTest(file=rel): + src = (self.ROOT / rel).read_text(encoding="utf-8") + for line in src.splitlines(): + stripped = line.strip() + if stripped.startswith("#") or '"""' in stripped: + continue # prose about the old shape is fine + self.assertNotIn('.name + ".tmp"', stripped) + self.assertNotIn('.name}.tmp"', stripped) + self.assertNotIn('"config.json.tmp"', stripped) + + +if __name__ == "__main__": + unittest.main()