[lukisch]

What

T-20261107-490692877, a finding of the review of #9. A truncated automation.toml parses as valid (empty) TOML. It counted as a readable record with status UNKNOWN, and activate-all would have written status = "ACTIVE" into it: a broken definition "repaired" with a status.

• Empty and id-less definitions are unreadable. New _read_automation() (used by scan_automations(), find_automation_anomalies() and _set_status()) rejects: an empty file, a file with only a comment or whitespace, a missing, empty, blank or non-string id, a file with only a table, and an id that differs from the folder name (casefolded). They end up in UnparseableAutomation with their own message (empty, missing id, id mismatch), in errors and unparseable_ids of every action result (so status is never ok), and nothing is written into them.
• _set_status() checks the file again right before writing, so a file that was truncated between the scan and the write is not overwritten.
• find_automation_anomalies() reports empty and id-less files as unreadable too, but lets the id differ from the folder name: the duplicate-id detection needs exactly that (existing test folder-a/folder-b with id = "dupe").
• POSIX permissions: _write_toml() copies the mode of the original to the temp file before os.replace (mkstemp creates 0600).
• Links: _write_toml() replaces the resolved target of a link and leaves the link alone. Since fix(automation): report unreadable automation.toml instead of skipping it; write it atomically #9 os.replace onto a symlink path would silently turn the link into a regular file and leave the target unchanged.

No real Codex files are touched; every test uses temp data.

Tests

Red first (a159200), then the fix (91cea15). 45 new tests in tests/test_automation_control.py:

• empty file, comment only, whitespace only, no id, empty id, blank id, non-string id, only a table, id differs from the folder: each as a scan result (9) and against pause-active, activate-all, restore-ccc (27): bytes unchanged, own message, unparseable_ids, status not ok, no temp file left;
• revalidation right before writing; anomaly scan reports the new cases but keeps the duplicate-id finding;
• file mode kept (0644, 0664, 0640, 0600; POSIX only), the order chmod on the temp file then os.replace (all platforms), write through a link (real symlink where allowed, plus a platform-independent test that resolves a path the way a link would).

Against c381595: 40 of the 45 fail and 5 are skipped on Windows without the right to create links; on Linux (WSL, no skips) 44 of the 45 fail. After the fix: Linux 65 passed in tests/test_automation_control.py (the four POSIX-mode tests and the symlink test run), Windows 60 passed and 5 skipped. Related files (health, orchestrator, tray, metadata, docs contracts, exchange contract, the bugsweep resilience test): 162 passed, 5 skipped. ruff check src tests clean. The full suite was not run (targeted groups only).

For the reviewer

Worth attacking: an id that differs only by case or by surrounding whitespace; a definition that is valid but has no status (still a record, activate-all adds the key as before); the revalidation window between scan and write; the link behaviour on Windows junctions; restore-ccc with a remembered id whose file became empty (the id stays remembered, as for other unreadable files).

Residual risk: a folder name that differs from the id in real Codex data now blocks the automation from being paused or restored (reported as id mismatch); the old code accepted it and could address the wrong file when two folders share an id.

The CodeQL follow-up (T-20261107-558181168) is a separate PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_01THB2EysTVQFXz5moenpBvW

[github-actions]

Welcome! 👋 Thanks for your first pull request in this repository.

A maintainer will review it soon. Please make sure:

• Your changes are tested
• Documentation is updated if needed
• The PR description explains what and why

Thanks for contributing!

[lukisch]

Independent review at 3f19f9a, from a session that is not the author. Not merged yet: one regression on Windows (read-only original). The fix is one line; everything else passes.

Base reproduced: test_automation_control.py against automation_control.py from c381595 gives 40 failed, 20 passed, 5 skipped. On this head, test_automation_control plus test_health give 76 passed and 5 skipped (Windows: symlink privilege), and ruff is clean. The red CodeQL/analyze is the duplicate CodeQL setup and goes away with #11 (merged as 28e49b2); updating the branch drops the check.

Own probe: 9 cases, temp .codex only. 8 behave as intended, 1 gap.

• Unparseable: empty file, comment only, whitespace only, no id, blank id, integer id, and id ≠ folder name.
• Accepted: an id that differs from the folder only in case.
• Writes: pause, activate-all and restore never write into any of those files, and none of them reports ok.
• Anomaly scan: find_automation_anomalies keeps a mismatching id as a normal finding (match_folder=False). Only the six truly unusable shapes are reported as unparseable there.
• Truncated after the scan: _set_status re-reads, refuses, and the file stays empty.
• Junction: an automation folder that is a junction to a folder outside .codex is written through. The target gets PAUSED, the junction stays and no temp file is left; this is the intended "write through links".
• Gap: a read-only automation.toml (Windows) leaves a read-only temp file behind.
  • _write_toml now copies the original mode onto the temp file before os.replace. With a read-only original, the temp file becomes read-only too, and os.replace fails (WinError 5) as before.
  • New: the cleanup temporary.unlink() also fails because the temp file is read-only. automation.toml.<x>.tmp stays in the folder, and the reported error names the temp path instead of the real cause.
  • The original stays byte-identical and the status is failed, so nothing is lost. But each attempt leaves one more file, and #9 cleaned up correctly in the same case.
  • Fix: in the except branch, clear the read-only bit first (os.chmod(temporary, stat.S_IWRITE) before unlink). Alternatively, apply the mode only after a successful os.replace. Please add the case as a test.

On the id mismatch rule (no real data hits it today, according to the lead's count of 114/114): a folder whose name differs from its id can no longer be paused or restored by CareCenter. It is reported (partial/failed), and CareCenter never writes into it. That is the safe direction for "never activate the wrong thing". The consequence to keep in mind: during a CareCenter pause such an automation keeps running, so the visible error is the only signal.

Independent review by Claude Opus 5.5 (claude-opus-5-5). Posted with the owner's account (the bot account is suspended), so this is a comment, not a formal review.

[lukisch]

Nacharbeit zum Befund von r1 (schreibgeschützte automation.toml unter Windows). Neuer Head: daeb736.

Reproduziert (Scratch-Verzeichnis, keine echten ~/.codex-Dateien): pause_active_automations auf einer Datei mit Nur-Lesen-Attribut ergab status = failed, das Original blieb ACTIVE, aber automation.toml.<x>.tmp blieb liegen und die Meldung nannte nur diese tmp-Datei ([WinError 5] … automation.toml.<x>.tmp).

Commits

1. 3ce9616 roter Test zuerst (test_read_only_original_fails_cleanly_without_leftovers, nur Windows): schlug mit „keine tmp-Reste“ fehl.
2. e904aca Fix: im except wird die tmp-Datei zuerst beschreibbar gemacht (os.chmod(…, S_IWRITE | S_IREAD)), dann gelöscht; ein Aufräumfehler ersetzt den ursprünglichen Fehler nicht mehr. Jetzt endet der Fehlertext mit dem Zielpfad (os.replace), es bleibt kein .tmp liegen, das Original ist unverändert. Test ist grün.
3. daeb736 Merge von origin/main (28e49b2, enthält ci(codeql): remove the duplicate CodeQL workflow, keep the GitHub default setup #11) ohne Force-Push, ein Konflikt nur in CHANGELOG.md (beide Einträge erhalten). Damit verschwindet der rote analyze-Check, der aus dem doppelten CodeQL-Workflow stammte.

Warum der Aufräumweg statt „Modus erst nach dem Replace setzen“: So trägt das Ziel nie, auch nicht kurz, die 0600 von mkstemp, und ein fehlschlagendes chmod nach dem Austausch kann keine bereits ersetzte Datei mit falschem Modus hinterlassen. Geändert ist nur der Aufräumpfad.

Prüfung (Windows, Merge-Stand): pytest 579 passed, 6 skipped (davon 5 in test_automation_control.py: POSIX-Rechte und Symlinks; Datei allein: 61 passed, 5 skipped); ruff check . sauber. Nicht geprüft: ruff format --check (das Repo ist nicht damit formatiert, 63 Dateien vorher schon abweichend).

Kein Merge durch mich; r1 prüft erneut.

🤖 Generated with Claude Code

https://claude.ai/code/session_01THB2EysTVQFXz5moenpBvW

[lukisch]

Re-review at daeb736, from a session that is not the author. Merged (squash). The read-only gap from my earlier comment is closed: with a read-only automation.toml the status is failed, the original is byte-identical, no *.tmp is left, and the error is the original os.replace error naming the real target. My full probe from the first review now passes 9/9. The new test test_read_only_original_fails_cleanly_without_leftovers fails against the source of 3f19f9a and passes on this head. Full suite on Windows: 579 passed, 6 skipped; ruff is clean; all checks are green. Independent review by Claude Opus 5.5 (claude-opus-5-5); posted with the owner's account, so this is a comment, not a formal review.