From 9eb25e253b93dee316e044e245f04b94caa3c9ff Mon Sep 17 00:00:00 2001 From: Ouroboros <311266734+ouroboros-agent@users.noreply.github.com> Date: Wed, 2 Sep 2026 09:08:33 +0000 Subject: [PATCH] settings: every writer commits the one serializer's bytes through the one byte-exact helper The seam claimed "one spelling on disk whichever surface wrote it", and two of the three writers still committed with `Path.write_text`, which translates \n to \r\n on Windows. On the 3-OS matrix the config saver and the packaged bootstrap saver therefore emitted CRLF while the owner endpoint (through `atomic_write_json` -> `write_text_atomic`) emitted LF for the same document, and no byte comparison run on a single leg could see it. All three now commit `serialize_settings()` output through `utils.write_text_atomic`, which is byte-exact on every platform. The owner writer stops re-deriving the same JSON text inside `atomic_write_json` and calls the serializer itself, so "one serializer" is the code rather than two spellings pinned equal. The config saver keeps its OSError fallback, now byte-exact too. Three properties come with the shared helper rather than being added: the temp sibling carries the atomic signature the stale-temp sweep recognises (`settings.tmp` never did), the existing permission bits survive the replace (a 0600 settings.json stayed 0600 only by luck before), and the pytest live-data guard on `_atomic_overwrite` now covers this surface as well. The packaged bootstrap saver additionally takes the settings write guards on the path it actually writes: it went through neither the snapshot pin nor the live-data guard, so a bootstrap save was the one settings write that could land under a strict benchmark pin. --- ouroboros/config.py | 23 ++++++++++++----------- ouroboros/gateway/owner_settings.py | 8 ++++---- ouroboros/packaged_cli.py | 27 ++++++++++++++++----------- tests/test_cybergym_server.py | 2 +- 4 files changed, 33 insertions(+), 27 deletions(-) diff --git a/ouroboros/config.py b/ouroboros/config.py index 1827670dd..9a0a9f3ed 100644 --- a/ouroboros/config.py +++ b/ouroboros/config.py @@ -728,11 +728,14 @@ def normalize_settings_raw(raw: dict) -> dict: def serialize_settings(settings: dict) -> str: """THE bytes a settings document is persisted as, for every writer that persists one. - ``ouroboros.utils.atomic_write_json`` produces exactly this text, which is what lets the - owner-endpoint writer keep its atomic helper while the config saver and the packaged - bootstrap saver produce byte-identical output through the same function (pinned by - tests/test_settings_read_seam.py). Without one serializer the writers disagreed on - ``ensure_ascii`` alone, so the same document had two spellings on disk.""" + Every writer of this document puts exactly ``serialize_settings(document).encode("utf-8")`` + on disk on EVERY platform, because every one of them commits through + ``ouroboros.utils.write_text_atomic``, which does not translate newlines. Both halves are + load-bearing: without one serializer the writers disagreed on ``ensure_ascii``, and with one + serializer but a text-mode ``Path.write_text`` two of the three would still have written + CRLF on Windows for the LF the third wrote. Pinned by tests/test_settings_read_seam.py on + the bytes AND on the mechanism, because the byte comparison alone cannot see a difference + that only exists on another platform.""" return json.dumps(settings, ensure_ascii=False, indent=2) @@ -849,13 +852,11 @@ def save_settings( f"OUROBOROS_RUNTIME_MODE elevation refused: " f"{baseline_mode!r} -> {new_mode!r}.{hint}" ) + from ouroboros.utils import write_text_atomic try: - from ouroboros.utils import replace_atomic - tmp = SETTINGS_PATH.with_suffix(".tmp") - tmp.write_text(serialize_settings(settings), encoding="utf-8") - replace_atomic(str(tmp), str(SETTINGS_PATH)) - except OSError: - SETTINGS_PATH.write_text(serialize_settings(settings), encoding="utf-8") + write_text_atomic(SETTINGS_PATH, serialize_settings(settings)) + except OSError: # a filesystem that cannot rename a sibling: write in place, same bytes + SETTINGS_PATH.write_bytes(serialize_settings(settings).encode("utf-8")) finally: _release_settings_lock(fd) diff --git a/ouroboros/gateway/owner_settings.py b/ouroboros/gateway/owner_settings.py index e061e03f5..6e9a7f3ac 100644 --- a/ouroboros/gateway/owner_settings.py +++ b/ouroboros/gateway/owner_settings.py @@ -20,7 +20,7 @@ re-implemented (or silently skipped) per call site: about to overwrite — not against a read taken before a multi-second daemon call. A refusal aborts with ``SettingsPreconditionFailed`` and writes nothing. -3. **The commit boundary is visible.** Once ``atomic_write_json`` returns, the +3. **The commit boundary is visible.** Once ``write_text_atomic`` returns, the bytes ARE on disk; a failure in a LATER step (env projection, supervisor start, hot-reload side effects) must be reported as its own fact, never as "nothing was saved" (BIBLE P1). ``CommitBoundary`` carries that distinction @@ -49,7 +49,7 @@ from ouroboros.config import SETTINGS_DEFAULTS as _SETTINGS_DEFAULTS from ouroboros.context_mode_compat import normalize_context_mode_compat from ouroboros.gateway._helpers import json_error, request_drive_root from ouroboros.settings_integrity import SettingsIntegrityError, read_settings_json_verified -from ouroboros.utils import append_jsonl, atomic_write_json, utc_now_iso +from ouroboros.utils import append_jsonl, utc_now_iso, write_text_atomic log = logging.getLogger(__name__) @@ -253,7 +253,7 @@ def _owner_write_settings( """Write owner-controlled settings without applying the runtime-mode ratchet. Skipping that ONE ratchet is the whole reason this writer exists; everything else comes from - ``config.prepare_settings_for_persist``, the single point both persisting writers pass through. + ``config.prepare_settings_for_persist``, the single point every persisting writer passes through. An endpoint that genuinely authors a disk-authored key (context mode, safety mode, the false compatibility tombstone) must name it in ``authored_keys`` — otherwise a POST about an unrelated key would author a mode decision out of the defaults merge that ``_owner_read_settings_raw`` performs. @@ -343,7 +343,7 @@ def _owner_update_settings( dict(proposed), authored_keys=authored_keys, allow_context_lowering=allow_context_lowering, allow_safety_lowering=allow_safety_lowering) - atomic_write_json(_config.SETTINGS_PATH, to_write, trailing_newline=False) + write_text_atomic(_config.SETTINGS_PATH, _config.serialize_settings(to_write)) if boundary is not None: boundary.commit() finally: diff --git a/ouroboros/packaged_cli.py b/ouroboros/packaged_cli.py index 4a2070046..071706a2f 100644 --- a/ouroboros/packaged_cli.py +++ b/ouroboros/packaged_cli.py @@ -211,10 +211,10 @@ def _hidden_run(command: Sequence[str], **kwargs: object) -> subprocess.Complete def _save_settings(path: pathlib.Path, settings: dict) -> None: """The packaged bootstrap's settings writer — same prologue, same bytes. - It is a THIRD writer only in the sense that it owns its own path and its own - atomic rename; what it persists goes through the one persistence prologue (which - proves the owner-only context/safety ratchets against the value on disk) and the - one serializer, so a bootstrap save cannot be the save that skips them. The path + It is a THIRD writer only in the sense that it owns its own path; what it persists + goes through the one persistence prologue (which proves the owner-only context/safety + ratchets against the value on disk), the one serializer and the one byte-exact commit + helper, so a bootstrap save cannot be the save that skips them. The path it writes is the path the prologue reads: the packaged runtime resolves its data dir to ``~/Ouroboros/data``, which is exactly what ``config`` resolves in a process that was given no path overrides — pinned by @@ -223,14 +223,19 @@ def _save_settings(path: pathlib.Path, settings: dict) -> None: ignores the environment by design (the inner CLI child is handed the packaged paths explicitly), while the prologue proves against ``config.SETTINGS_PATH``, which honours one. Dormant today — ``BootstrapContext.save_settings`` has no - caller — and disclosed rather than papered over.""" - from ouroboros.config import prepare_settings_for_persist, serialize_settings - from ouroboros.utils import replace_atomic + caller — and disclosed rather than papered over. - path.parent.mkdir(parents=True, exist_ok=True) - tmp = path.with_suffix(path.suffix + ".tmp") - tmp.write_text(serialize_settings(prepare_settings_for_persist(dict(settings))), encoding="utf-8") - replace_atomic(tmp, path) + The write GUARDS are taken on the path this saver actually writes, not on + ``config.SETTINGS_PATH``: under a pinned benchmark snapshot, or from pytest against the + live install, a bootstrap save must refuse for the same reason the other two writers + refuse. Passing its own path is what keeps that honest while the disclosed override + split above exists.""" + from ouroboros.config import prepare_settings_for_persist, serialize_settings + from ouroboros.settings_integrity import guard_live_settings_write + from ouroboros.utils import write_text_atomic + + guard_live_settings_write(path, pathlib.Path.home()) + write_text_atomic(path, serialize_settings(prepare_settings_for_persist(dict(settings)))) def _run_inner_cli(runtime: PackagedRuntime, args: Sequence[str]) -> int: diff --git a/tests/test_cybergym_server.py b/tests/test_cybergym_server.py index c4af8b2db..4f9cd20cc 100644 --- a/tests/test_cybergym_server.py +++ b/tests/test_cybergym_server.py @@ -335,7 +335,7 @@ def test_runtime_config_strict_snapshot_cannot_be_mutated_by_owner_writer( nonlocal wrote wrote = True - monkeypatch.setattr(owner_settings, "atomic_write_json", record_write) + monkeypatch.setattr(owner_settings, "write_text_atomic", record_write) with pytest.raises(config.SettingsIntegrityError, match="immutable"): owner_settings._owner_write_settings({"OUROBOROS_MODEL": "wrong/model"}) assert not wrote