Skip to content

fix(infra): make every atomic-write temp path collision-free - #200

Open
Ranjithsingh2004 wants to merge 1 commit into
spinabot:mainfrom
Ranjithsingh2004:fix/atomic-tmp-collision-free
Open

Ranjithsingh2004 wants to merge 1 commit into
spinabot:mainfrom
Ranjithsingh2004:fix/atomic-tmp-collision-free

Conversation

@Ranjithsingh2004

Copy link
Copy Markdown
Contributor

What

Every remaining bare <target>.tmp writer now draws its temp path from a shared atomicTempPath() helper in src/infra/json-file.ts:

<target>.tmp-<pid>-<base36 ms>-<seq>

Nine write sites across eight files, each a one-line swap plus an import:

File Writer
src/sessions/session-store.ts session store persist
src/auth/profiles.ts auth profiles + auth state (2 sites)
src/infra/json-file.ts saveJsonFile
src/core/gateway-probe.ts gateway heartbeat file
src/cron/service/store.ts cron store
src/cron/run-log.ts cron run-log prune rewrite
src/core/tunnel/state.ts tunnel state
src/agents/claude-cli/claude-config.ts Claude CLI credentials

New src/infra/json-file.test.ts pins the helper and the round-trip.

Why

A bare <target>.tmp is only safe when exactly one process ever writes the target — but several of these stores are written from more than one process (the gateway and a brigade CLI invocation both persist the session store, auth profiles, and the cron store). Two writers computing the same temp path can interleave their bytes, and the second rename then publishes a torn file: atomicity is decided by whoever renames last, not by the writer whose data survived. The repo already knew this — workspace/state.ts and agents/memory/extract.ts carry pid+timestamp temp names with comments explaining exactly this race; this PR finishes the sweep and centralizes the convention (pid separates processes, base36 ms separates writes seconds apart, a sequence counter separates same-millisecond bursts).

Deliberately not touched:

  • collaboration-journal.ts — its temp target already embeds Date.now() + randomUUID(), so it is unique by construction;
  • the sites that already carry suffixes (access-control/store, whatsapp/connection, exec-approvals, session-file-repair, blob-store, org-store, skill-store, workspace-store, sentinel) — behavior kept as-is.

Nothing reads *.tmp-* leftovers (verified by sweep); success renames them away, a crash leaves them inert.

Tests

  • New json-file.test.ts (6 tests): 5 000 rapid temp paths against one target are all distinct; every path is rooted at its target with the .tmp- marker and embeds the pid; saveJsonFile round-trips, survives 25 rapid rewrites with last-writer-wins and zero temp strays (directory read, no path probes); missing/empty/malformed load semantics unchanged.
  • Regression run over every touched module — session-store, claude-config, oauth-refresh-persist, profile-cooldown, gateway-probe, tunnel, run-log, cron store — 84 tests, 0 failures (2 pre-existing skips).
  • npm run typecheck clean.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant