Skip to content

fix: write the custom sources, project identity, bmad config and identity seed atomically - #187

Merged
arcaven merged 2 commits into
mainfrom
build/sideshow/185-state-tier
Oct 8, 2026
Merged

arcaven merged 2 commits into
mainfrom
build/sideshow/185-state-tier

Conversation

@arcaven

@arcaven arcaven commented Oct 8, 2026

Copy link
Copy Markdown
Member

Four more files sideshow writes for the user could be left half-written by an interruption, and for two of them nothing would ever rewrite the damage: the project identity and the bmad module configs are created once and skipped on later runs. This is PR 2 of the three tiers ruled for sideshow#185: the custom-sources registry, the project identity, the bmad config and the identity layer seed now use atomicfile.WriteFile, and their four allowlist entries are gone from the guard added in #186.

Additive to the write path only: contents, modes (an existing file keeps its mode, a new file gets 0644 minus the umask) and symlink handling are the helper's, as at the earlier sites.

Refs: sideshow#185

Sites: bindings/custom_sources.go saveCustomSources, project/project.go InitIdentity, init/init.go Run, cmd/sideshow/project_identity.go seedIdentity.

Allowlist count: the guard's own scan (findInPlaceWriters over internal/ and cmd/) returns 13 functions at this head, equal to the 13 allowlist entries (6 pending tier 3, 7 out with reasons), down from 17; TestNoUnlistedInPlaceWriters fails on a difference in either direction.

Acceptance: the two sites that overwrite (saveCustomSources, seedIdentity) have the four pins: a write into a directory that cannot take a new file fails and leaves the previous bytes (red before the fix); a write through a symlink keeps the link and updates the target; an existing 0600 mode is kept with no temp file left; a new file is 0644 under umask 022. The two create-only sites (InitIdentity, Run, which skip a file that exists) never overwrite, so they have no previous-bytes pin; they have the new-file mode and no-stray-file pin and a dangling-symlink pin instead. Their red evidence is the guard, which failed naming all four sites once their entries were deleted. The mode, link and new-file tests are guards that pass on arrival.

Commits: (1) tests and the four allowlist deletions, (2) the four call sites.

Blast radius: the four writers; nothing else changes.

Mutants, each compiles and fails a test:

  • saveCustomSources and seedIdentity back to os.WriteFile: killed by their failed-write test and by the guard
  • InitIdentity and Run back to os.WriteFile: killed by the guard only, since a create-only site has no previous-bytes pin
  • helper does not follow links: killed by the link tests at all four sites
  • helper skips the mode copy for an existing file: killed by the two overwrite sites' mode tests
  • helper creates the temp at 0600: killed by the new-file tests at all four sites
  • skipping the helper's Sync: not killed; no test can observe it without a crash

Remaining for #185: tier 3 (user files), whose six allowlist entries stay until that PR.

@arcavenai arcavenai left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. The four state writers now replace their files whole or not at all. The two create-only sites still never touch an existing file, and the guard from #186 now holds 13 entries, down from 17. I measured each claim in the body and they hold.

Red, then green, without flakes. At 783ca5f, TestNoUnlistedInPlaceWriters names exactly seedIdentity, saveCustomSources, init.go:Run and InitIdentity. The two failed-write tests (TestSeedIdentity_ and TestSaveCustomSources_AFailedWriteLeavesThePreviousBytes) fail, and everything else passes. At 3aa0022 the full suite passes. The new tests and the guard pass two runs of -count=20.

Mutants are killed as the body lists them:

  • saveCustomSources and seedIdentity reverted: the guard and their failed-write test.
  • InitIdentity and Run reverted: the guard only.
  • No link-follow: the link test at each of the four sites (the two dangling-symlink tests at the create-only sites).
  • Temp at 0600: the new-file test at each of the four sites.
  • Skipped mode copy: the two overwrite sites' mode tests.
  • No-sync survives, as disclosed.

Count and base. I ran findInPlaceWriters over internal/ and cmd/ from a scratch test: 13 found, 13 entries, 13 in both. The PR's merge base is 5d9b7a1, the squash of #186 on main, so it builds on the merged guard.

Create-only is real. InitIdentity returns early when LoadIdentity finds a file, and an empty file still parses as an identity. Run skips when os.Stat finds the config. I called both at main and at head on files that existed at 0644, 0600 and 0444, through a symlink, through a hard link, and in a read-only directory. Every case left the old bytes, with no error. On a dangling symlink, both sites keep the link and create the target, at 0644 under umask 022 and at 0600 under 077, the same as main.

Behavior against main. I called each of the four writers at main and at head on: no file, an existing 0644 file, an existing 0600 file, a symlink, and a dangling symlink, each under umask 022 and 077. Error, length, mode, link state, target, and directory listing were identical. This is at the function level; the call sites around the four functions are unchanged. Outside the listed cases, the two overwrite sites differ only in the ways the helper's doc comment already names: a 0444 file is replaced, keeping 0444; a hard link is split; and a read-only directory now fails with the old bytes kept.

CI on 3aa0022 passes. Under the stopping rule, nothing further to list.

Seat: reviewer / reviewer

@arcaven
arcaven marked this pull request as ready for review October 8, 2026 10:30
@arcaven
arcaven merged commit bafee50 into main Oct 8, 2026
10 checks passed
@arcaven
arcaven deleted the build/sideshow/185-state-tier branch October 8, 2026 10:31
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.

2 participants