fix: make _safe_write_json actually atomic with mkstemp + os.replace

[Quratulain-bilal]

Problem

Despite its name, _safe_write_json used write_text() which truncates the file before writing. A crash or power loss mid-write leaves a partial JSON file. This function is called by _merge_copilot_json, _merge_opencode_plugin_ref, etc., meaning all Copilot and OpenCode event config writes were non-atomic.

Fix

Now uses empfile.mkstemp + os.replace for atomic writes, matching the pattern used in _utils.py, shared_infra.py, and other safe-write utilities in the codebase.

[Copilot]

Pull request overview

Makes event configuration JSON writes atomic using a temporary file and os.replace.

Changes:

• Writes JSON to a same-directory temporary file.
• Cleans up temporary files on failure.

Show a summary per file

File	Description
src/specify_cli/events.py	Adds atomic JSON replacement logic.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (1)

src/specify_cli/events.py:2024

• mkstemp creates the staged file with mode 0600, and os.replace swaps that inode over the existing config. Unlike the previous write_text call, this silently changes permissions (and potentially ownership) on every merge, which can lock other users or processes out of shared 0640/0644 config files. Preserve the existing file metadata before replacing it, as the atomic writer in src/specify_cli/_utils.py:163-193 does.

        os.replace(tmp, dst)

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Balanced

[mnriem]

Please address Copilot feedback

[mnriem]

Thanks. Two things before this can be reviewed, per CONTRIBUTING: (1) please disclose any AI assistance and its extent; (2) this needs a test — assert the file round-trips after a successful write, and that a failure mid-write leaves the original intact and cleans up the temp file. One correctness note from the review: os.replace makes publication atomic but isn't durable without an fsync of the file and the parent directory — so as written it doesn't fully close the crash/power-loss window it targets. Worth adding those syncs (or reusing an existing atomic-write helper like _utils.atomic_write_json). Marking author-awaiting.

[Copilot]

🟡 Changes recommended

Ownership, descriptor cleanup, and durability issues must be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/specify_cli/events.py:2026

• os.replace provides atomic namespace replacement but not the power-loss guarantee stated in the PR: neither the temporary file nor the containing directory is synced, so a reboot can still expose missing or incomplete persisted data on filesystems without stronger ordering guarantees. If power-loss resilience is required, flush/fsync the temp before replacement and sync the directory afterward; otherwise narrow the PR's guarantee to process interruption.

        with os.fdopen(fd, "w", encoding="utf-8") as f:
            json.dump(data, f, indent=2)
            f.write("\n")
        os.replace(tmp, dst)

• Files reviewed: 1/1 changed files
• Comments generated: 2
• Review effort level: Balanced

[Quratulain-bilal]

AI Assistance Disclosure

I used AI assistance to identify that _safe_write_json used non-atomic write_text() despite its name. The atomic rewrite (mkstemp + os.replace + os.fchmod) was reviewed by me. I noted the maintainer feedback about fsync for durability and will address that along with the required regression test.

[Quratulain-bilal]

Closing in favor of consolidated PR #4588 which includes this change with tests and AI disclosure.