From a159200b42802994c3681fb86cb31ec34bb46ea0 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Sat, 7 Nov 2026 17:33:52 +0100 Subject: [PATCH 1/5] test(automation): red tests for empty and id-less automation.toml, file mode and links T-20261107-490692877 (finding of the review of #9). A definition that is empty, only a comment, only whitespace, has no id, an empty or non-string id, only a table, or an id that differs from its folder name is not a readable record: it must be unparseable with its own message ('empty' / 'missing id' / 'id mismatch'), and pause-active, activate-all and restore-ccc must never write into it (bytes unchanged, no temp file). _set_status revalidates right before writing. find_automation_anomalies reports empty and id-less files but keeps the duplicate-id detection (id and folder name may differ there). _write_toml copies the mode of the original to the temp file before os.replace (mkstemp makes 0600) and writes to the resolved target of a link. Against c381595 the 45 new tests: 40 fail and 5 are skipped on this Windows host (four POSIX mode tests, one real symlink test); on Linux (WSL, no skips) 44 of the 45 fail. Refs: T-20261107-490692877 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01THB2EysTVQFXz5moenpBvW --- tests/test_automation_control.py | 189 +++++++++++++++++++++++++++++++ 1 file changed, 189 insertions(+) diff --git a/tests/test_automation_control.py b/tests/test_automation_control.py index ec969b3..1da7607 100644 --- a/tests/test_automation_control.py +++ b/tests/test_automation_control.py @@ -2,6 +2,7 @@ import json import os +import stat from pathlib import Path import pytest @@ -406,3 +407,191 @@ def test_undecodable_bytes_are_reported_not_silently_rewritten(tmp_path: Path) - assert result.unparseable_ids == ["latin1-job"] assert result.status == "failed" assert path.read_bytes() == before + + +# --- Leere und id-lose Definitionen sind keine lesbaren Datensaetze (T-20261107-490692877) --------- +# Eine leere (etwa abgeschnittene) automation.toml parst als leeres TOML. Ohne diese Pruefung galt sie als +# Datensatz mit Status UNKNOWN, und activate-all haette ``status = "ACTIVE"`` hineingeschrieben: eine kaputte +# Definition waere mit einem Status "repariert" worden. + +BAD_DEFINITIONS = [ + pytest.param(b"", "empty", id="leere-datei"), + pytest.param(b"# abgeschnitten, nur ein Kommentar\n", "empty", id="nur-kommentar"), + pytest.param(b" \r\n\t\n \n", "empty", id="nur-leerraum"), + pytest.param(b'name = "x"\nstatus = "ACTIVE"\n', "missing id", id="id-fehlt"), + pytest.param(b'id = ""\nname = "x"\nstatus = "ACTIVE"\n', "missing id", id="id-leer"), + pytest.param(b'id = " "\nname = "x"\nstatus = "ACTIVE"\n', "missing id", id="id-nur-leerraum"), + pytest.param(b'id = 5\nname = "x"\nstatus = "ACTIVE"\n', "missing id", id="id-keine-zeichenkette"), + pytest.param(b'[section]\nstatus = "ACTIVE"\n', "missing id", id="nur-tabelle"), + pytest.param(b'id = "other-job"\nname = "x"\nstatus = "ACTIVE"\n', "id mismatch", id="id-weicht-vom-ordner-ab"), +] + + +def write_bad(codex_home: Path, folder_name: str, payload: bytes) -> Path: + folder = codex_home / "automations" / folder_name + folder.mkdir(parents=True) + path = folder / "automation.toml" + path.write_bytes(payload) + return path + + +@pytest.mark.parametrize(("payload", "token"), BAD_DEFINITIONS) +def test_empty_or_id_less_definition_is_unparseable_with_its_own_message( + tmp_path: Path, payload: bytes, token: str +) -> None: + config = make_config(tmp_path) + good = write_automation(config.codex_home, "good-job", "PAUSED") + bad = write_bad(config.codex_home, "bad-job", payload) + + records, unparseable = scan_automations(config) + + assert [record.path for record in records] == [good], "nicht als lesbarer Datensatz (UNKNOWN) fuehren" + assert [(item.id, item.path) for item in unparseable] == [("bad-job", bad)] + assert token in unparseable[0].error + assert [record.id for record in load_automations(config)] == ["good-job"] + + +@pytest.mark.parametrize(("payload", "token"), BAD_DEFINITIONS) +@pytest.mark.parametrize("action", ["pause-active", "activate-all", "restore-ccc"]) +def test_no_action_ever_writes_into_an_empty_or_id_less_definition( + tmp_path: Path, payload: bytes, token: str, action: str +) -> None: + config = make_config(tmp_path) + bad = write_bad(config.codex_home, "bad-job", payload) + write_carecenter_state(config, ["bad-job"]) + + result = run_automation_action(config, action) # type: ignore[arg-type] + + assert bad.read_bytes() == payload, "die kaputte Definition darf nie ueberschrieben werden" + assert result.unparseable_ids == ["bad-job"] + assert result.status != "ok" + assert any("bad-job" in error and token in error for error in result.errors) + assert [item.name for item in bad.parent.iterdir()] == ["automation.toml"], "keine tmp-Datei" + + +def test_status_change_revalidates_the_file_right_before_writing(tmp_path: Path) -> None: + """Zwischen Scan und Schreiben abgeschnittene Datei: weder ueberschreiben noch als lesbar behandeln.""" + config = make_config(tmp_path) + path = write_automation(config.codex_home, "job", "PAUSED") + record = scan_automations(config)[0][0] + path.write_bytes(b"") + + with pytest.raises(automation_control._UNREADABLE): + automation_control._set_status(record, "ACTIVE") + + assert path.read_bytes() == b"" + + +def test_anomaly_scan_reports_empty_and_id_less_files_but_keeps_duplicate_detection(tmp_path: Path) -> None: + config = make_config(tmp_path) + empty = write_bad(config.codex_home, "empty-job", b"") + no_id = write_bad(config.codex_home, "no-id-job", b'name = "x"\n') + first = write_automation_at(config.codex_home, ("folder-a",), "dupe") + second = write_automation_at(config.codex_home, ("folder-b",), "dupe") + + report = find_automation_anomalies(config) + + assert sorted(item.path for item in report.unparseable) == sorted([empty, no_id]) + assert any("empty" in item.error for item in report.unparseable) + assert any("missing id" in item.error for item in report.unparseable) + # id und Ordnername duerfen hier abweichen: sonst waere die Doppel-id-Erkennung nicht mehr moeglich. + assert [item.paths for item in report.duplicate_ids] == [sorted([first, second])] + + +# --- Rechte und Symlinks beim atomaren Schreiben --------------------------------------------------- + + +@pytest.mark.skipif(os.name == "nt", reason="POSIX-Dateirechte") +@pytest.mark.parametrize("mode", [0o644, 0o664, 0o640, 0o600]) +def test_status_change_keeps_the_permissions_of_the_original(tmp_path: Path, mode: int) -> None: + config = make_config(tmp_path) + path = write_automation(config.codex_home, "job", "ACTIVE") + path.chmod(mode) + + pause_active_automations(config) + + assert stat.S_IMODE(path.stat().st_mode) == mode, "mkstemp macht 0600; die Rechte des Originals uebernehmen" + assert read_status(path) == "PAUSED" + + +def test_status_change_applies_the_mode_of_the_original_to_the_temp_file( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Plattformunabhaengig: der Aufruf muss vor dem os.replace auf der tmp-Datei erfolgen.""" + config = make_config(tmp_path) + path = write_automation(config.codex_home, "job", "ACTIVE") + original_mode = stat.S_IMODE(path.stat().st_mode) + calls: list[tuple[str, int, bool]] = [] + real_chmod, real_replace = os.chmod, os.replace + + def spy_chmod(target: object, mode: int, *args: object, **kwargs: object) -> None: + calls.append(("chmod", mode, str(target).endswith(".tmp"))) + real_chmod(target, mode, *args, **kwargs) # type: ignore[arg-type] + + def spy_replace(src: object, dst: object) -> None: + calls.append(("replace", 0, False)) + real_replace(src, dst) # type: ignore[arg-type] + + monkeypatch.setattr(automation_control.os, "chmod", spy_chmod) + monkeypatch.setattr(automation_control.os, "replace", spy_replace) + + pause_active_automations(config) + + assert calls == [("chmod", original_mode, True), ("replace", 0, False)] + + +def _symlink_or_skip(link: Path, target: Path) -> None: + try: + link.symlink_to(target) + except (OSError, NotImplementedError): + pytest.skip("Symlinks sind hier nicht erlaubt (unter Windows: Entwicklermodus oder Administrator)") + + +def test_status_change_writes_through_a_symlink_and_keeps_the_link(tmp_path: Path) -> None: + config = make_config(tmp_path) + real_dir = tmp_path / "synced" + real_dir.mkdir() + target = write_automation(tmp_path / "elsewhere", "job", "ACTIVE") + target.replace(real_dir / "job.toml") + target = real_dir / "job.toml" + folder = config.codex_home / "automations" / "job" + folder.mkdir(parents=True) + link = folder / "automation.toml" + _symlink_or_skip(link, target) + + result = pause_active_automations(config) + + assert result.status == "ok" + assert link.is_symlink(), "der Link darf nicht durch eine normale Datei ersetzt werden" + assert Path(os.readlink(link)) == target + assert read_status(target) == "PAUSED" + assert sorted(item.name for item in real_dir.iterdir()) == ["job.toml"], "keine tmp-Datei zurueckgelassen" + + +def test_write_toml_writes_to_the_resolved_target_of_a_link( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Auch ohne Symlink-Recht pruefbar: aufgeloest wird der Pfad wie bei einem Symlink, geschrieben wird dort.""" + config = make_config(tmp_path) + link_path = write_automation(config.codex_home, "job", "ACTIVE") + real_dir = tmp_path / "synced" + real_dir.mkdir() + target = real_dir / "job.toml" + target.write_bytes(link_path.read_bytes()) + link_before = link_path.read_bytes() + real_realpath = os.path.realpath + + def fake_realpath(value: object, *args: object, **kwargs: object) -> str: + if Path(str(value)) == link_path: + return str(target) + return real_realpath(value, *args, **kwargs) # type: ignore[arg-type] + + monkeypatch.setattr(automation_control.os.path, "realpath", fake_realpath) + + document = tomlkit.parse(target.read_text(encoding="utf-8")) + document["status"] = "PAUSED" + automation_control._write_toml(link_path, document) + + assert read_status(target) == "PAUSED" + assert link_path.read_bytes() == link_before, "der Link-Pfad selbst bleibt unberuehrt" + assert sorted(item.name for item in real_dir.iterdir()) == ["job.toml"] From 91cea153a1727e80a002182733e9aeaa1ae92a50 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Sat, 7 Nov 2026 17:39:24 +0100 Subject: [PATCH 2/5] fix(automation): empty and id-less automation.toml are unreadable; keep file mode; write through links T-20261107-490692877. A truncated automation.toml parses as valid TOML and counted as a record with status UNKNOWN, so activate-all would have written 'status = "ACTIVE"' into it and "repaired" a broken definition with a status. - _read_automation() requires keys, a non-empty string id and (for everything that writes) an id that matches the folder name; otherwise AutomationDefinitionError with the messages 'empty', 'missing id' or 'id mismatch', handled like every other unreadable definition (listed in errors/unparseable_ids, status never ok, nothing written). - _set_status() reads through it, so a file truncated between scan and write is not overwritten. - find_automation_anomalies() reports empty and id-less files as unreadable but still lets the id differ from the folder name (needed for the duplicate-id detection). - _write_toml() copies the mode of the original to the temp file before os.replace (mkstemp creates 0600) and replaces the resolved target of a link, so the link stays. The red tests of e67d735 pass: 65 passed on Linux (WSL, including the four POSIX-mode tests and the symlink test), 60 passed and 5 skipped on Windows; related files (health, orchestrator, tray, metadata, docs contracts, exchange contract) 162 passed. ruff clean. Refs: T-20261107-490692877 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01THB2EysTVQFXz5moenpBvW --- CHANGELOG.md | 6 ++ .../automation_control.py | 65 +++++++++++++++---- 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d133dfb..69c7ff4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +- Automation control no longer treats an empty or id-less `automation.toml` as a readable record (T-20261107-490692877) on 2026-11-07: + - A truncated file parses as valid TOML. It used to count as a record with status `UNKNOWN`, and `activate-all` would have written `status = "ACTIVE"` into it, "repairing" a broken definition with a status. Now an empty file, a file with only a comment or whitespace, a missing, empty or non-string `id`, and an `id` that differs from the folder name are unparseable with their own message (`empty`, `missing id`, `id mismatch`). Pause, restore and activate-all never write into them; `_set_status()` checks the file again right before writing. + - `find_automation_anomalies()` reports empty and id-less files as unreadable too; an `id` that differs from the folder name stays a normal finding there (duplicate-id detection). + - `_write_toml()` copies the permissions of the original to the temp file before `os.replace` (`mkstemp` creates 0600) and writes through a link: the target is replaced, the link stays. + - Tests: `tests/test_automation_control.py` (45 new; the POSIX-mode and symlink tests are skipped on Windows without the right and were run on Linux). + - Automation control no longer hides unreadable `automation.toml` files and writes them atomically (T-20261101-844031160) on 2026-11-07: - Unreadable definitions are reported, not skipped: `scan_automations()` returns them next to the readable records (`UnparseableAutomation`: folder name, path, error text), `find_automation_anomalies()` has a new `unparseable` category, and `diagnose()` lists them in the report (`unparseable_automations`, shown by `to_text()`) without a new issue code, so the public health exchange contract v1 stays unchanged. `load_automations()` keeps its signature and still returns only the readable records. This matters for the duplicate `model`/`reasoning_effort` keys of 2026-10-31, which tomlkit rejects as `ParseError`: 94 of 113 files would have been invisible to pause, restore and the anomaly check. - Pause, restore and activate-all list every unreadable definition (folder, path, error) in `errors` and in the new `unparseable_ids`, so `status` is never `ok` and the tray and the orchestrator show it as partial or failed. IDs that CareCenter paused earlier stay remembered while their file is unreadable; an unreadable file CareCenter never paused is not adopted, so a later restore cannot activate something the user paused. diff --git a/src/codex_logdatenbank_wartung/automation_control.py b/src/codex_logdatenbank_wartung/automation_control.py index cb1b1cf..dc5fbc7 100644 --- a/src/codex_logdatenbank_wartung/automation_control.py +++ b/src/codex_logdatenbank_wartung/automation_control.py @@ -10,6 +10,7 @@ import json import os +import stat import tempfile import time from collections.abc import Callable @@ -35,9 +36,18 @@ ProgressCallback = Callable[[int, int, str], None] Sleeper = Callable[[float], None] -# Was beim Lesen einer automation.toml schiefgehen kann: Ein-/Ausgabe, kein gueltiges UTF-8 -# oder ein TOML-Fehler (tomlkit meldet doppelte Schluessel als ParseError). -_UNREADABLE = (OSError, UnicodeDecodeError, tomlkit.exceptions.TOMLKitError) + + +class AutomationDefinitionError(ValueError): + """Die Datei ist gueltiges TOML, aber keine brauchbare Automations-Definition. + + Die Meldung beginnt mit ``empty``, ``missing id`` oder ``id mismatch``. + """ + + +# Was beim Lesen einer automation.toml schiefgehen kann: Ein-/Ausgabe, kein gueltiges UTF-8, +# ein TOML-Fehler (tomlkit meldet doppelte Schluessel als ParseError) oder eine leere bzw. id-lose Definition. +_UNREADABLE = (OSError, UnicodeDecodeError, tomlkit.exceptions.TOMLKitError, AutomationDefinitionError) @dataclass(slots=True) @@ -153,20 +163,47 @@ def _read_toml(path: Path) -> tomlkit.TOMLDocument: return tomlkit.parse(path.read_bytes().decode("utf-8")) +def _read_automation(path: Path, *, match_folder: bool = True) -> tomlkit.TOMLDocument: + """Liest eine ``automation.toml`` und verlangt eine brauchbare Definition. + + Eine leere, abgeschnittene oder id-lose Datei parst als gueltiges TOML; ohne diese Pruefung galt sie + als Datensatz mit Status UNKNOWN, und ``activate-all`` haette ``status = "ACTIVE"`` hineingeschrieben. + ``match_folder=False`` laesst eine ``id`` zu, die vom Ordnernamen abweicht (die Doppel-id-Erkennung + braucht genau diesen Fall); alles, was Dateien aendert, verlangt die Uebereinstimmung. + """ + data = _read_toml(path) + if not data: + raise AutomationDefinitionError("empty: keine Schluessel (leere Datei, nur Kommentar oder nur Leerraum)") + raw_id = data.get("id") + automation_id = raw_id.strip() if isinstance(raw_id, str) else "" + if not automation_id: + raise AutomationDefinitionError("missing id: 'id' fehlt, ist leer oder keine Zeichenkette") + if match_folder and automation_id.casefold() != path.parent.name.casefold(): + raise AutomationDefinitionError( + f"id mismatch: id '{automation_id}' weicht vom Ordnernamen '{path.parent.name}' ab" + ) + return data + + def _write_toml(path: Path, document: tomlkit.TOMLDocument) -> None: """Schreibt atomar: erst vollstaendig kodieren, dann tmp im selben Ordner, fsync, ``os.replace``. Bricht der Vorgang ab, bleibt die Originaldatei unveraendert und die tmp-Datei wird entfernt. + ``mkstemp`` legt die Datei mit 0600 an: die Rechte des Originals kommen vor dem Austausch auf die tmp-Datei. + Ist ``path`` ein Link, wird sein Ziel ersetzt und der Link bleibt bestehen. """ payload = tomlkit.dumps(document).encode("utf-8") - descriptor, tmp_name = tempfile.mkstemp(dir=path.parent, prefix=f"{path.name}.", suffix=".tmp") + target = Path(os.path.realpath(path)) + mode = stat.S_IMODE(target.stat().st_mode) + descriptor, tmp_name = tempfile.mkstemp(dir=target.parent, prefix=f"{target.name}.", suffix=".tmp") temporary = Path(tmp_name) try: with os.fdopen(descriptor, "wb") as handle: handle.write(payload) handle.flush() os.fsync(handle.fileno()) - os.replace(temporary, path) + os.chmod(temporary, mode) + os.replace(temporary, target) except BaseException: temporary.unlink(missing_ok=True) raise @@ -177,6 +214,8 @@ def _quoted_string(value: object) -> str: def _unreadable_text(exc: Exception) -> str: + if isinstance(exc, AutomationDefinitionError): + return " ".join(str(exc).split())[:300] # beginnt mit empty / missing id / id mismatch return f"{type(exc).__name__}: {' '.join(str(exc).split())}"[:300] @@ -197,7 +236,7 @@ def scan_automations( unparseable: list[UnparseableAutomation] = [] for toml_path in sorted(root.glob("*/automation.toml")): try: - data = _read_toml(toml_path) + data = _read_automation(toml_path) except _UNREADABLE as exc: unparseable.append( UnparseableAutomation( @@ -205,7 +244,7 @@ def scan_automations( ) ) continue - automation_id = _quoted_string(data.get("id")) or toml_path.parent.name + automation_id = _quoted_string(data.get("id")).strip() records.append( AutomationRecord( id=automation_id, @@ -234,8 +273,10 @@ def find_automation_anomalies(config: MaintenanceConfig) -> AutomationAnomalyRep * **doppelte id** = derselbe ``id``-Wert in mehreren ``automation.toml`` (ueber alle Tiefenebenen). Ein rekursiver Scan in der Codex-App zaehlt die Automation dadurch doppelt. - * **unlesbar** = ``automation.toml`` mit TOML- oder Kodierungsfehler (Pfad und Fehlertext - im Befund). Sie zaehlt weder bei Verschachtelung noch bei Duplikaten mit. + * **unlesbar** = ``automation.toml`` mit TOML- oder Kodierungsfehler oder ohne brauchbare + Definition (leer, nur Kommentar, ``id`` fehlt oder leer; Pfad und Fehlertext im Befund). Sie + zaehlt weder bei Verschachtelung noch bei Duplikaten mit. Eine ``id``, die vom Ordnernamen + abweicht, ist hier ein normaler Befund (Doppel-id), kein Lesefehler. Aendert nichts auf der Platte und laesst ``load_automations`` (Tiefe 1) unberuehrt. """ @@ -247,7 +288,7 @@ def find_automation_anomalies(config: MaintenanceConfig) -> AutomationAnomalyRep findings: list[AutomationFinding] = [] for toml_path in sorted(root.rglob("automation.toml")): try: - data = _read_toml(toml_path) + data = _read_automation(toml_path, match_folder=False) except _UNREADABLE as exc: report.unparseable.append( UnparseableAutomation( @@ -259,7 +300,7 @@ def find_automation_anomalies(config: MaintenanceConfig) -> AutomationAnomalyRep depth = len(toml_path.parent.relative_to(root).parts) except ValueError: continue - automation_id = _quoted_string(data.get("id")) or toml_path.parent.name + automation_id = _quoted_string(data.get("id")).strip() findings.append(AutomationFinding(id=automation_id, path=toml_path, depth=depth)) report.nested = [ @@ -316,7 +357,7 @@ def _append_event(config: MaintenanceConfig, result: AutomationControlResult) -> def _set_status(record: AutomationRecord, status: str) -> bool: - data = _read_toml(record.path) + data = _read_automation(record.path) # erneut pruefen: die Datei kann seit dem Scan abgeschnitten worden sein current = _quoted_string(data.get("status")).strip().upper() if current == status.strip().upper(): return False From 3f19f9a0558691827b67dea143c8eb3dc24bdf35 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Sat, 7 Nov 2026 17:44:59 +0100 Subject: [PATCH 3/5] test(automation): compare the link target with samefile; Windows readlink adds a prefix The Python jobs of the CI (Windows runners, which may create links) ran the real symlink test and failed on one assertion only: os.readlink() returns the target with the extended-length prefix '\\?\' there, so the Path comparison differed although the link was kept and the target updated. Compare with resolve() and os.path.samefile() instead. Windows 60 passed / 5 skipped locally, Linux (WSL) 65 passed. Refs: T-20261107-490692877 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01THB2EysTVQFXz5moenpBvW --- tests/test_automation_control.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_automation_control.py b/tests/test_automation_control.py index 1da7607..84aa26b 100644 --- a/tests/test_automation_control.py +++ b/tests/test_automation_control.py @@ -563,7 +563,7 @@ def test_status_change_writes_through_a_symlink_and_keeps_the_link(tmp_path: Pat assert result.status == "ok" assert link.is_symlink(), "der Link darf nicht durch eine normale Datei ersetzt werden" - assert Path(os.readlink(link)) == target + assert link.resolve() == target.resolve() and os.path.samefile(link, target) # readlink liefert unter Windows ein Praefix assert read_status(target) == "PAUSED" assert sorted(item.name for item in real_dir.iterdir()) == ["job.toml"], "keine tmp-Datei zurueckgelassen" From 3ce9616a37d673b0e864d5f751e88533a00f1898 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Sat, 7 Nov 2026 18:25:32 +0100 Subject: [PATCH 4/5] test(automation): red test for a read-only automation.toml on Windows (r1 finding PR #10) Unter Windows vererbt _write_toml das Nur-Lesen-Attribut des Originals vor dem os.replace auf die tmp-Datei. os.replace scheitert dann mit WinError 5, das Aufraeumen (unlink) scheitert an der schreibgeschuetzten tmp-Datei ebenfalls: es bleibt automation.toml..tmp liegen, und die Fehlermeldung nennt nur diese tmp-Datei statt des Ersetzens in das Ziel. Das Original bleibt unveraendert, der Status ist failed. Der Test ist rot: der Ordner enthaelt nach dem Lauf noch die tmp-Datei. Er prueft ausserdem, dass der Fehlertext mit dem Zielpfad endet (Text von os.replace), nicht mit der tmp-Datei. Nur unter Windows (unter POSIX ersetzt os.replace auch eine schreibgeschuetzte Datei). Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01THB2EysTVQFXz5moenpBvW --- tests/test_automation_control.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_automation_control.py b/tests/test_automation_control.py index 84aa26b..0dbfad8 100644 --- a/tests/test_automation_control.py +++ b/tests/test_automation_control.py @@ -540,6 +540,26 @@ def spy_replace(src: object, dst: object) -> None: assert calls == [("chmod", original_mode, True), ("replace", 0, False)] +@pytest.mark.skipif(os.name != "nt", reason="nur unter Windows scheitert os.replace an einer schreibgeschuetzten Datei") +def test_read_only_original_fails_cleanly_without_leftovers(tmp_path: Path) -> None: + """r1-Befund PR #10: die tmp-Datei erbte das Nur-Lesen-Attribut, das Aufraeumen scheiterte ebenfalls, + es blieb ``automation.toml..tmp`` liegen und die Meldung nannte nur diese tmp-Datei.""" + config = make_config(tmp_path) + path = write_automation(config.codex_home, "job", "ACTIVE") + path.chmod(stat.S_IREAD) + try: + result = pause_active_automations(config) + finally: + path.chmod(stat.S_IWRITE | stat.S_IREAD) + + assert result.status == "failed" + assert read_status(path) == "ACTIVE", "das Original bleibt unveraendert" + assert list(path.parent.glob("*.tmp")) == [], "keine tmp-Reste" + (error,) = result.errors + # Fehlertext von os.replace(tmp -> Ziel) endet mit dem Zielpfad; der des Aufraeumens (unlink) nennt nur die tmp-Datei. + assert error.endswith("automation.toml'"), error + + def _symlink_or_skip(link: Path, target: Path) -> None: try: link.symlink_to(target) From e904acad200499545a37bde668fceac810ad56a0 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Sat, 7 Nov 2026 18:27:17 +0100 Subject: [PATCH 5/5] fix(automation): remove the temp file even when it inherited the read-only attribute (r1 finding PR #10) Beim Aufraeumen nach einem fehlgeschlagenen Austausch wird die tmp-Datei zuerst beschreibbar gemacht, dann geloescht. Fehler beim Aufraeumen ersetzen den urspruenglichen Fehler nicht mehr (der Text von os.replace nennt das Ziel). Das Original bleibt unveraendert; es bleibt kein automation.toml..tmp liegen. Macht den roten Test aus 3ce9616 gruen. Gewaehlt ist der Aufraeumweg (chmod im except) statt den Modus erst nach dem os.replace zu setzen: so traegt das Ziel nie, auch nicht kurz, die 0600 von mkstemp, und ein fehlschlagendes chmod nach dem Austausch kann keine ersetzte Datei mit falschem Modus hinterlassen. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01THB2EysTVQFXz5moenpBvW --- CHANGELOG.md | 3 ++- src/codex_logdatenbank_wartung/automation_control.py | 11 +++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 69c7ff4..f6e829a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,8 @@ - A truncated file parses as valid TOML. It used to count as a record with status `UNKNOWN`, and `activate-all` would have written `status = "ACTIVE"` into it, "repairing" a broken definition with a status. Now an empty file, a file with only a comment or whitespace, a missing, empty or non-string `id`, and an `id` that differs from the folder name are unparseable with their own message (`empty`, `missing id`, `id mismatch`). Pause, restore and activate-all never write into them; `_set_status()` checks the file again right before writing. - `find_automation_anomalies()` reports empty and id-less files as unreadable too; an `id` that differs from the folder name stays a normal finding there (duplicate-id detection). - `_write_toml()` copies the permissions of the original to the temp file before `os.replace` (`mkstemp` creates 0600) and writes through a link: the target is replaced, the link stays. - - Tests: `tests/test_automation_control.py` (45 new; the POSIX-mode and symlink tests are skipped on Windows without the right and were run on Linux). + - With a read-only original on Windows the temp file inherited the read-only attribute: `os.replace` failed (WinError 5), the cleanup failed too, `automation.toml..tmp` stayed behind, and the error text named only the temp file. The cleanup now makes the temp file writable first and never replaces the original error; the original stays unchanged and the error text ends with the target path (review finding on PR #10). The mode stays set before the replace on purpose, so the target never carries a wrong mode, not even briefly. + - Tests: `tests/test_automation_control.py` (46 new; the POSIX-mode and symlink tests are skipped on Windows without the right and were run on Linux). - Automation control no longer hides unreadable `automation.toml` files and writes them atomically (T-20261101-844031160) on 2026-11-07: - Unreadable definitions are reported, not skipped: `scan_automations()` returns them next to the readable records (`UnparseableAutomation`: folder name, path, error text), `find_automation_anomalies()` has a new `unparseable` category, and `diagnose()` lists them in the report (`unparseable_automations`, shown by `to_text()`) without a new issue code, so the public health exchange contract v1 stays unchanged. `load_automations()` keeps its signature and still returns only the readable records. This matters for the duplicate `model`/`reasoning_effort` keys of 2026-10-31, which tomlkit rejects as `ParseError`: 94 of 113 files would have been invisible to pause, restore and the anomaly check. diff --git a/src/codex_logdatenbank_wartung/automation_control.py b/src/codex_logdatenbank_wartung/automation_control.py index dc5fbc7..7daf397 100644 --- a/src/codex_logdatenbank_wartung/automation_control.py +++ b/src/codex_logdatenbank_wartung/automation_control.py @@ -188,7 +188,8 @@ def _read_automation(path: Path, *, match_folder: bool = True) -> tomlkit.TOMLDo def _write_toml(path: Path, document: tomlkit.TOMLDocument) -> None: """Schreibt atomar: erst vollstaendig kodieren, dann tmp im selben Ordner, fsync, ``os.replace``. - Bricht der Vorgang ab, bleibt die Originaldatei unveraendert und die tmp-Datei wird entfernt. + Bricht der Vorgang ab, bleibt die Originaldatei unveraendert und die tmp-Datei wird entfernt (auch eine + schreibgeschuetzte); der urspruengliche Fehler wird nicht durch einen Aufraeumfehler ersetzt. ``mkstemp`` legt die Datei mit 0600 an: die Rechte des Originals kommen vor dem Austausch auf die tmp-Datei. Ist ``path`` ein Link, wird sein Ziel ersetzt und der Link bleibt bestehen. """ @@ -205,7 +206,13 @@ def _write_toml(path: Path, document: tomlkit.TOMLDocument) -> None: os.chmod(temporary, mode) os.replace(temporary, target) except BaseException: - temporary.unlink(missing_ok=True) + # Die tmp-Datei kann das Nur-Lesen-Attribut des Originals tragen (Windows): ohne chmod scheitert unlink, + # laesst die Datei liegen und ueberdeckt den eigentlichen Fehler. Dessen Text bleibt der, der zaehlt. + try: + os.chmod(temporary, stat.S_IWRITE | stat.S_IREAD) + temporary.unlink(missing_ok=True) + except OSError: + pass raise