Skip to content

fix(atomicfile): ccp-sbp.5 — adopt atomicfile for durable-state writes #108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dkoosis merged 1 commit into from
Aug 12, 2026
Merged

fix(atomicfile): ccp-sbp.5 — adopt atomicfile for durable-state writes #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@ toolchain go1.26.5

require (
github.com/anthropics/anthropic-sdk-go v1.52.0
github.com/dkoosis/atomicfile v0.0.0-20260811102456-9091c28d4820
github.com/quasilyte/go-ruleguard/dsl v0.3.23
gonum.org/v1/gonum v0.17.0
)

require (
github.com/bahlo/generic-list-go v0.2.0 // indirect
github.com/buger/jsonparser v1.1.2 // indirect
github.com/google/renameio/v2 v2.0.2 // indirect
github.com/invopop/jsonschema v0.14.0 // indirect
github.com/pb33f/ordered-map/v2 v2.3.1 // indirect
github.com/standard-webhooks/standard-webhooks/libraries v0.0.1 // indirect
Expand All @@ -22,4 +24,5 @@ require (
github.com/tidwall/sjson v1.2.5 // indirect
go.yaml.in/yaml/v4 v4.0.0-rc.2 // indirect
golang.org/x/sync v0.16.0 // indirect
golang.org/x/sys v0.47.0 // indirect
)
6 changes: 6 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@ github.com/buger/jsonparser v1.1.2 h1:frqHqw7otoVbk5M8LlE/L7HTnIq2v9RX6EJ48i9AxJ
github.com/buger/jsonparser v1.1.2/go.mod h1:6RYKKt7H4d4+iWqouImQ9R2FZql3VbhNgx27UK13J/0=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/dkoosis/atomicfile v0.0.0-20260811102456-9091c28d4820 h1:x7UDk0isvTYxv8obfKfrancH99VtIJRZ3StZ9izJlvU=
github.com/dkoosis/atomicfile v0.0.0-20260811102456-9091c28d4820/go.mod h1:6wCWesghtzLYJ0IjlbbddyTCU8ZowoiIUzcy4Qi9fDc=
github.com/dnaeon/go-vcr v1.2.0 h1:zHCHvJYTMh1N7xnV7zf1m1GPBF9Ad0Jk/whtQ1663qI=
github.com/dnaeon/go-vcr v1.2.0/go.mod h1:R4UdLID7HZT3taECzJs4YgbbH6PIGXB6W/sc5OLb6RQ=
github.com/google/renameio/v2 v2.0.2 h1:qKZs+tfn+arruZZhQ7TKC/ergJunuJicWS6gLDt/dGw=
github.com/google/renameio/v2 v2.0.2/go.mod h1:OX+G6WHHpHq3NVj7cAOleLOwJfcQ1s3uUJQCrr78SWo=
github.com/invopop/jsonschema v0.14.0 h1:MHQqLhvpNUZfw+hM3AZDYK7jxO8FZoQeQM77g8iyZjg=
github.com/invopop/jsonschema v0.14.0/go.mod h1:ygm6C2EaVNMBDPpaPlnOA2pFAxBnxGjFlMZABxm9n2I=
github.com/pb33f/ordered-map/v2 v2.3.1 h1:5319HDO0aw4DA4gzi+zv4FXU9UlSs3xGZ40wcP1nBjY=
Expand All @@ -34,6 +38,8 @@ go.yaml.in/yaml/v4 v4.0.0-rc.2 h1:/FrI8D64VSr4HtGIlUtlFMGsm7H7pWTbj6vOLVZcA6s=
go.yaml.in/yaml/v4 v4.0.0-rc.2/go.mod h1:aZqd9kCMsGL7AuUv/m/PvWLdg5sjJsZ4oHDEnfPPfY0=
golang.org/x/sync v0.16.0 h1:ycBJEhp9p4vXvUZNszeOq0kGTPghopOL8q0fq3vstxw=
golang.org/x/sync v0.16.0/go.mod h1:1dzgHSNfp02xaA81J2MS99Qcpr2w7fw1gpm99rleRqA=
golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs=
golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
gonum.org/v1/gonum v0.17.0 h1:VbpOemQlsSMrYmn7T2OUvQ4dqxQXU+ouZFQsZOx50z4=
gonum.org/v1/gonum v0.17.0/go.mod h1:El3tOrEuMpv2UdMrbNlKEh9vd86bmQ6vqIcDwxEOc1E=
gopkg.in/yaml.v2 v2.2.8 h1:obN1ZagJSUGI0Ek/LBmuj4SNLPfIny3KsKFopxRdj10=
Expand Down
34 changes: 16 additions & 18 deletions internal/counts/refresh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"strings"
"time"

"github.com/dkoosis/atomicfile"
"github.com/dkoosis/strand/internal/bd"
"github.com/dkoosis/strand/internal/bdcounts"
)
Expand Down Expand Up @@ -208,6 +209,13 @@ func readRows(path string) map[string]Row {
// file + rename, so a concurrent reader never sees a half-written file. The meta rides
// under bdcounts.MetaKey — a reserved key no repo path collides with — so keyed readers
// (bdcounts.Reader.Lookup, the status line's `.[$repo]`) are untouched by its presence.
//
// NOTE (RMW race, not fixed by atomic write): two concurrent refreshes (the
// launchd --all run and a manual `strand counts`) each read-compute-write the
// whole rows set independently. atomicfile.WriteFile stops either write from
// being torn, but it does not serialize the two writers — whichever finishes
// last wins wholesale, silently dropping the other's rows. Follow-up: a lock
// around the refresh, or a merge-on-write like writeState's.
Comment on lines +212 to +218

@coderabbitai coderabbitai Bot Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 5 'readRows|writeRowsAtomic|readState|writeState|refresh|flock|Lock' internal/counts
rg -n -C 3 'launchd|strand counts|counts\.Run|refresh\(' --glob '*.go' .

Repository: dkoosis/strand

Length of output: 50370

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- refresh sequence ---'
sed -n '92,138p' internal/counts/refresh.go

printf '%s\n' '--- row writer ---'
sed -n '195,234p' internal/counts/refresh.go

printf '%s\n' '--- state writer ---'
sed -n '274,315p' internal/counts/refresh.go

printf '%s\n' '--- production call sites and background scheduling ---'
sed -n '120,145p' internal/server/server.go
sed -n '240,265p' internal/server/server.go
rg -n -C 4 'counts\.Run|defaultRefreshCounts|refreshCounts|goBackground' --glob '*.go' .

printf '%s\n' '--- atomicfile references and module metadata ---'
rg -n -C 3 'github.com/dkoosis/atomicfile|WithMkdirAll|func WriteFile|WriteFile' --glob '*.go' --glob 'go.mod' --glob 'go.sum' .

Repository: dkoosis/strand

Length of output: 43225

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- external refresh trigger ---'
rg -n -C 4 'counts( --all)?|StartInterval|bd-counts|launchd' \
  --glob '!internal/counts/refresh.go' --glob '!internal/counts/refresh_test.go' .

printf '%s\n' '--- pinned atomicfile implementation ---'
curl -fsSL https://raw.githubusercontent.com/dkoosis/atomicfile/9091c28d4820/atomicfile.go |
  sed -n '1,240p'

printf '%s\n' '--- standalone lost-update model ---'
python3 - <<'PY'
import json

base_rows = {"repo-a": "old-a", "repo-b": "old-b"}
a_rows = dict(base_rows)
b_rows = dict(base_rows)
a_rows["repo-a"] = "new-a"
b_rows["repo-b"] = "new-b"

# Both writers use atomic replacement. B completes last, so the file is valid
# JSON but contains neither writer's complete result.
final_rows = b_rows
assert json.loads(json.dumps(final_rows)) == {
    "repo-a": "old-a",
    "repo-b": "new-b",
}
assert final_rows["repo-a"] != a_rows["repo-a"]
print("last-writer-wins:", json.dumps(final_rows, sort_keys=True))
print("repo-a update lost while the resulting file remains valid JSON")
PY

Repository: dkoosis/strand

Length of output: 50370

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- external refresh trigger ---'
rg -n -C 4 'counts( --all)?|StartInterval|bd-counts|launchd' \
  --glob '!internal/counts/refresh.go' --glob '!internal/counts/refresh_test.go' .

printf '%s\n' '--- pinned atomicfile implementation ---'
curl -fsSL https://raw.githubusercontent.com/dkoosis/atomicfile/9091c28d4820/atomicfile.go |
  sed -n '1,240p'

printf '%s\n' '--- standalone lost-update model ---'
python3 - <<'PY'
import json

base_rows = {"repo-a": "old-a", "repo-b": "old-b"}
a_rows = dict(base_rows)
b_rows = dict(base_rows)
a_rows["repo-a"] = "new-a"
b_rows["repo-b"] = "new-b"

final_rows = b_rows
assert json.loads(json.dumps(final_rows)) == {
    "repo-a": "old-a",
    "repo-b": "new-b",
}
assert final_rows["repo-a"] != a_rows["repo-a"]
print("last-writer-wins:", json.dumps(final_rows, sort_keys=True))
print("repo-a update lost while the resulting file remains valid JSON")
PY

Repository: dkoosis/strand

Length of output: 50370

🏁 Script executed:

#!/bin/bash
set -euo pipefail

for log in \
  /tmp/coderabbit-shell-logs/shell-output-Lk08ky \
  /tmp/coderabbit-shell-logs/shell-output-wYN1HR
do
  echo "--- $log: atomicfile and model results ---"
  rg -n -A 80 -B 5 'pinned atomicfile|func WriteFile|atomic replacement|last-writer-wins|repo-a update lost|standalone lost-update' "$log" || true
done

Repository: dkoosis/strand

Length of output: 13000

Serialize the complete refresh read-modify-write sequence.

Concurrent counts.Run processes can read the same base, then atomically replace counts.json or counts-mtimes and silently discard the other update. atomicfile.WriteFile prevents torn writes, not lost updates. Add a cross-process lock around the entire refresh, or implement re-read and merge retry logic.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/counts/refresh.go` around lines 212 - 218, Serialize the entire
read-modify-write sequence in refresh, including reading existing counts state,
computing updates, and writing counts.json/counts-mtimes. Add a cross-process
lock around refresh (or equivalent re-read-and-merge retry logic) so concurrent
counts.Run processes cannot overwrite each other’s rows; retain
atomicfile.WriteFile for tear-free writes.

Source: MCP tools

func writeRowsAtomic(path string, rows map[string]Row, meta bdcounts.Meta) error {
out := make(map[string]any, len(rows)+1)
for k, v := range rows {
Expand All @@ -218,24 +226,12 @@ func writeRowsAtomic(path string, rows map[string]Row, meta bdcounts.Meta) error
if err != nil {
return fmt.Errorf("counts: marshal: %w", err)
}
tmp := tmpPath(path)
if err := os.WriteFile(tmp, append(data, '\n'), 0o600); err != nil {
if err := atomicfile.WriteFile(path, append(data, '\n'), 0o600); err != nil {
return fmt.Errorf("counts: write: %w", err)
}
Comment on lines +229 to 231

@coderabbitai coderabbitai Bot Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

go doc github.com/dkoosis/atomicfile.WithMkdirAll

for file in \
  internal/counts/refresh.go \
  internal/registry/registry.go \
  internal/strandmd/strandmd.go
do
  rg -n -C 3 'os\.MkdirAll|atomicfile\.WriteFile' "$file"
done

Repository: dkoosis/strand

Length of output: 4004

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- module declaration ---'
cat go.mod | sed -n '1,120p'

printf '%s\n' '--- relevant implementations and callers ---'
for file in internal/counts/refresh.go internal/registry/registry.go internal/strandmd/strandmd.go; do
  printf '\n--- %s ---\n' "$file"
  sed -n '100,140p' "$file" 2>/dev/null || true
  sed -n '280,315p' "$file" 2>/dev/null || true
done

printf '%s\n' '--- atomicfile source at the referenced revision ---'
curl -fsSL https://raw.githubusercontent.com/dkoosis/atomicfile/9091c28d4820/atomicfile.go | sed -n '1,260p'

Repository: dkoosis/strand

Length of output: 14038

Use atomicfile.WithMkdirAll(0o755) for all first writes.

os.MkdirAll does not durably persist newly created directories. Apply the option to the counts.json and state writes, repos.json, and default STRAND.md writes. Remove the separate os.MkdirAll calls where the option replaces them.

📍 Affects 3 files
  • internal/counts/refresh.go#L229-L231 (this comment)
  • internal/registry/registry.go#L305-L305
  • internal/strandmd/strandmd.go#L131-L132
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/counts/refresh.go` around lines 229 - 231, Update the first-write
paths to pass atomicfile.WithMkdirAll(0o755) to atomicfile.WriteFile: apply this
to the counts.json and state writes in internal/counts/refresh.go (229-231), the
repos.json write in internal/registry/registry.go (305), and the default
STRAND.md write in internal/strandmd/strandmd.go (131-132). Remove the separate
os.MkdirAll calls replaced by this option.

Source: MCP tools

if err := os.Rename(tmp, path); err != nil {
return fmt.Errorf("counts: rename: %w", err)
}
return nil
}

// tmpPath is a per-process temp name for the atomic write. The pid suffix keeps two
// concurrent refreshes (the launchd --all and a manual `strand counts`) off one
// shared tmp, where they would clobber each other's write and race the rename to a
// spurious ENOENT — the same guard the shell got from `$OUT.$$`.
func tmpPath(path string) string {
return fmt.Sprintf("%s.%d.tmp", path, os.Getpid())
}

// repoState is one repo's carry-forward bookkeeping between refresh runs: the
// last-observed change key (changeKey — the Dolt store mtime, or last-touched when
// no store), plus the st-3p8 pending bit (a change-triggered derive schedules exactly
Expand Down Expand Up @@ -283,6 +279,12 @@ func readState(path string) map[string]repoState {
// changed-mode run would then find no prior mtime for those repos and cold-recompute
// them all (st-dd9). Reading the prior state and overlaying keeps the untouched repos'
// gate entries intact.
//
// NOTE (RMW race, not fixed by atomic write): the read-merge-write above is not
// serialized against a concurrent writer — two refreshes racing this function can
// each read the same prior state, merge their own entries on top, and whichever
// atomicfile.WriteFile finishes last wins wholesale, silently dropping the other's
// merged entries. Same follow-up as writeRowsAtomic.
func writeState(path string, state map[string]repoState) error {
merged := readState(path)
maps.Copy(merged, state)
Expand All @@ -295,13 +297,9 @@ func writeState(path string, state map[string]repoState) error {
}
fmt.Fprintf(&b, "%s\t%d\t%s\n", root, st.mtime, pendingField)
}
tmp := tmpPath(path)
if err := os.WriteFile(tmp, []byte(b.String()), 0o600); err != nil {
if err := atomicfile.WriteFile(path, []byte(b.String()), 0o600); err != nil {
return fmt.Errorf("counts: write state: %w", err)
}
if err := os.Rename(tmp, path); err != nil {
return fmt.Errorf("counts: rename state: %w", err)
}
return nil
}

Expand Down
4 changes: 3 additions & 1 deletion internal/registry/registry.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import (
"strings"
"sync"
"time"

"github.com/dkoosis/atomicfile"
)

// ErrUnknownRepo means a switch targeted a path the registry doesn't hold.
Expand Down Expand Up @@ -300,7 +302,7 @@ func (r *Registry) saveLocked() error {
if err != nil {
return fmt.Errorf("marshal registry: %w", err)
}
if err := os.WriteFile(r.file, data, 0o600); err != nil {
if err := atomicfile.WriteFile(r.file, data, 0o600); err != nil {
return fmt.Errorf("write registry: %w", err)
}
return nil
Expand Down
4 changes: 3 additions & 1 deletion internal/strandmd/strandmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (
"path/filepath"
"strconv"
"strings"

"github.com/dkoosis/atomicfile"
)

// errHomeDirRequired is returned by Load when homeDir is empty, so a default is
Expand Down Expand Up @@ -126,7 +128,7 @@ func readOrInit(path, def string) (string, error) {
if mkErr := os.MkdirAll(filepath.Dir(path), 0o755); mkErr != nil {
return "", fmt.Errorf("strandmd: mkdir .strand: %w", mkErr)
}
if wErr := os.WriteFile(path, []byte(def), 0o600); wErr != nil {
if wErr := atomicfile.WriteFile(path, []byte(def), 0o600); wErr != nil {
return "", fmt.Errorf("strandmd: write default STRAND.md: %w", wErr)
}
return def, nil
Expand Down
Loading