[drknowhow]

Follow-up to the intermittent windows-latest failure seen on the 2.118.0 merge.

What broke

services/shell_output._write_json_atomic is the publish step for every
c3_shell_job status update. On CI:

PermissionError: [WinError 5] Access is denied
  services/shell_output.py:966  _write_json_atomic  →  os.replace(tmp, path)

The supervisor died before running anything, and the caller got
failed before running for a command that was perfectly fine. The CI flake is
the visible half — the invisible half is that the same race can lose a job on a
real Windows box.

Three defects

1. A shared temp name. <name>.tmp is the same string for every writer, so
   the parent creating the job record and the supervisor immediately saving
   status raced on the temp itself — one mid-write while the other tried to
   publish it.

   The suffix is now a pid and a random token. A pid alone is not enough:
   this store is written from a threaded server, and threads share a pid.
   My first version of this fix used .tmp<pid> and the new concurrency test
   failed on it — six threads, PermissionError ×3 and FileNotFoundError ×3.
   Worth stating plainly because cli/_hook_utils has the pid-only form and is
   only safe there because hooks are separate processes.

2. No retry. Another process holding a read handle on the target — AV
   scanner, Search indexer, a concurrent reader — makes os.replace raise
   transiently. One attempt turned that into a lost write. Now four attempts
   with exponential backoff; a genuinely persistent error still raises rather
   than being swallowed.

3. Orphaned temp files. A failed replace abandoned its temp in the store
   forever. Cleaned up on every failure path.

Not a new invention

This is the construction cli/_hook_utils._atomic_write_json has had since
v2.66 — including a docstring that names "transient Windows sharing violations"
explicitly. shell_output simply never got it. The duplication is deliberate
(a PreToolUse hook subprocess imports nothing from services) and is now noted
in both files.

fsync is deliberately not added here: this store is ephemeral spill state,
not durable config, and an fsync per status update is not worth paying.

Verification

Five new tests in TestAtomicWrite. Reverted the implementation and re-ran to
prove they bite:

4 failed, 1 passed
  test_temp_name_is_per_process
  test_transient_permission_error_is_retried
  test_no_temp_file_is_left_behind_when_replace_fails
  test_concurrent_writers_all_publish_valid_json

(The fifth — a persistent PermissionError still raising — passes either way
by construction; it is there so a future retry loop cannot start swallowing
real failures.)

Full suite on this branch: ruff clean, 3660 passed, 6 skipped, 0 failed.

Known related, deliberately out of scope

Other fixed-temp-name os.replace sites exist (services/agent_locks.py,
services/access_guard._write_scope_config, cli/hub_server, mobile_api).
None has a demonstrated failure, and agent_locks is the one most worth a
second look since it is genuinely concurrent. Left for a separate change rather
than widened into this one.

https://claude.ai/code/session_01JjAQC4Uv3aBnv9oQLPMBMy