Skip to content

Commit 69e5b61

Browse files
authored
Merge pull request #706 from mosir/fix/atomic-file-writes
refactor(pkg/utils): add unified atomic file write utility
2 parents a5c8179 + b8c0d13 commit 69e5b61

File tree

10 files changed

+197
-71
lines changed

10 files changed

+197
-71
lines changed

pkg/agent/memory.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212
"path/filepath"
1313
"strings"
1414
"time"
15+
16+
"github.com/sipeed/picoclaw/pkg/fileutil"
1517
)
1618

1719
// MemoryStore manages persistent memory for the agent.
@@ -58,7 +60,9 @@ func (ms *MemoryStore) ReadLongTerm() string {
5860

5961
// WriteLongTerm writes content to the long-term memory file (MEMORY.md).
6062
func (ms *MemoryStore) WriteLongTerm(content string) error {
61-
return os.WriteFile(ms.memoryFile, []byte(content), 0o644)
63+
// Use unified atomic write utility with explicit sync for flash storage reliability.
64+
// Using 0o600 (owner read/write only) for secure default permissions.
65+
return fileutil.WriteFileAtomic(ms.memoryFile, []byte(content), 0o600)
6266
}
6367

6468
// ReadToday reads today's daily note.
@@ -78,7 +82,9 @@ func (ms *MemoryStore) AppendToday(content string) error {
7882

7983
// Ensure month directory exists
8084
monthDir := filepath.Dir(todayFile)
81-
os.MkdirAll(monthDir, 0o755)
85+
if err := os.MkdirAll(monthDir, 0o755); err != nil {
86+
return err
87+
}
8288

8389
var existingContent string
8490
if data, err := os.ReadFile(todayFile); err == nil {
@@ -95,7 +101,8 @@ func (ms *MemoryStore) AppendToday(content string) error {
95101
newContent = existingContent + "\n" + content
96102
}
97103

98-
return os.WriteFile(todayFile, []byte(newContent), 0o644)
104+
// Use unified atomic write utility with explicit sync for flash storage reliability.
105+
return fileutil.WriteFileAtomic(todayFile, []byte(newContent), 0o600)
99106
}
100107

101108
// GetRecentDailyNotes returns daily notes from the last N days.

pkg/auth/store.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import (
55
"os"
66
"path/filepath"
77
"time"
8+
9+
"github.com/sipeed/picoclaw/pkg/fileutil"
810
)
911

1012
type AuthCredential struct {
@@ -63,16 +65,13 @@ func LoadStore() (*AuthStore, error) {
6365

6466
func SaveStore(store *AuthStore) error {
6567
path := authFilePath()
66-
dir := filepath.Dir(path)
67-
if err := os.MkdirAll(dir, 0o755); err != nil {
68-
return err
69-
}
70-
7168
data, err := json.MarshalIndent(store, "", " ")
7269
if err != nil {
7370
return err
7471
}
75-
return os.WriteFile(path, data, 0o600)
72+
73+
// Use unified atomic write utility with explicit sync for flash storage reliability.
74+
return fileutil.WriteFileAtomic(path, data, 0o600)
7675
}
7776

7877
func GetCredential(provider string) (*AuthCredential, error) {

pkg/config/config.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,11 @@ import (
44
"encoding/json"
55
"fmt"
66
"os"
7-
"path/filepath"
87
"sync/atomic"
98

109
"github.com/caarlos0/env/v11"
10+
11+
"github.com/sipeed/picoclaw/pkg/fileutil"
1112
)
1213

1314
// rrCounter is a global counter for round-robin load balancing across models.
@@ -555,12 +556,8 @@ func SaveConfig(path string, cfg *Config) error {
555556
return err
556557
}
557558

558-
dir := filepath.Dir(path)
559-
if err := os.MkdirAll(dir, 0o755); err != nil {
560-
return err
561-
}
562-
563-
return os.WriteFile(path, data, 0o600)
559+
// Use unified atomic write utility with explicit sync for flash storage reliability.
560+
return fileutil.WriteFileAtomic(path, data, 0o600)
564561
}
565562

566563
func (c *Config) WorkspacePath() string {

pkg/cron/service.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,12 @@ import (
77
"fmt"
88
"log"
99
"os"
10-
"path/filepath"
1110
"sync"
1211
"time"
1312

1413
"github.com/adhocore/gronx"
14+
15+
"github.com/sipeed/picoclaw/pkg/fileutil"
1516
)
1617

1718
type CronSchedule struct {
@@ -330,17 +331,13 @@ func (cs *CronService) loadStore() error {
330331
}
331332

332333
func (cs *CronService) saveStoreUnsafe() error {
333-
dir := filepath.Dir(cs.storePath)
334-
if err := os.MkdirAll(dir, 0o755); err != nil {
335-
return err
336-
}
337-
338334
data, err := json.MarshalIndent(cs.store, "", " ")
339335
if err != nil {
340336
return err
341337
}
342338

343-
return os.WriteFile(cs.storePath, data, 0o600)
339+
// Use unified atomic write utility with explicit sync for flash storage reliability.
340+
return fileutil.WriteFileAtomic(cs.storePath, data, 0o600)
344341
}
345342

346343
func (cs *CronService) AddJob(

pkg/fileutil/file.go

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
// PicoClaw - Ultra-lightweight personal AI agent
2+
// Inspired by and based on nanobot: https://github.com/HKUDS/nanobot
3+
// License: MIT
4+
//
5+
// Copyright (c) 2026 PicoClaw contributors
6+
7+
// Package fileutil provides file manipulation utilities.
8+
package fileutil
9+
10+
import (
11+
"fmt"
12+
"os"
13+
"path/filepath"
14+
"time"
15+
)
16+
17+
// WriteFileAtomic atomically writes data to a file using a temp file + rename pattern.
18+
//
19+
// This guarantees that the target file is either:
20+
// - Completely written with the new data
21+
// - Unchanged (if any step fails before rename)
22+
//
23+
// The function:
24+
// 1. Creates a temp file in the same directory (original untouched)
25+
// 2. Writes data to temp file
26+
// 3. Syncs data to disk (critical for SD cards/flash storage)
27+
// 4. Sets file permissions
28+
// 5. Syncs directory metadata (ensures rename is durable)
29+
// 6. Atomically renames temp file to target path
30+
//
31+
// Safety guarantees:
32+
// - Original file is NEVER modified until successful rename
33+
// - Temp file is always cleaned up on error
34+
// - Data is flushed to physical storage before rename
35+
// - Directory entry is synced to prevent orphaned inodes
36+
//
37+
// Parameters:
38+
// - path: Target file path
39+
// - data: Data to write
40+
// - perm: File permission mode (e.g., 0o600 for secure, 0o644 for readable)
41+
//
42+
// Returns:
43+
// - Error if any step fails, nil on success
44+
//
45+
// Example:
46+
//
47+
// // Secure config file (owner read/write only)
48+
// err := utils.WriteFileAtomic("config.json", data, 0o600)
49+
//
50+
// // Public readable file
51+
// err := utils.WriteFileAtomic("public.txt", data, 0o644)
52+
func WriteFileAtomic(path string, data []byte, perm os.FileMode) error {
53+
dir := filepath.Dir(path)
54+
if err := os.MkdirAll(dir, 0o755); err != nil {
55+
return fmt.Errorf("failed to create directory: %w", err)
56+
}
57+
58+
// Create temp file in the same directory (ensures atomic rename works)
59+
// Using a hidden prefix (.tmp-) to avoid issues with some tools
60+
tmpFile, err := os.OpenFile(
61+
filepath.Join(dir, fmt.Sprintf(".tmp-%d-%d", os.Getpid(), time.Now().UnixNano())),
62+
os.O_WRONLY|os.O_CREATE|os.O_EXCL,
63+
perm,
64+
)
65+
if err != nil {
66+
return fmt.Errorf("failed to create temp file: %w", err)
67+
}
68+
69+
tmpPath := tmpFile.Name()
70+
cleanup := true
71+
72+
defer func() {
73+
if cleanup {
74+
tmpFile.Close()
75+
_ = os.Remove(tmpPath)
76+
}
77+
}()
78+
79+
// Write data to temp file
80+
// Note: Original file is untouched at this point
81+
if _, err := tmpFile.Write(data); err != nil {
82+
return fmt.Errorf("failed to write temp file: %w", err)
83+
}
84+
85+
// CRITICAL: Force sync to storage medium before any other operations.
86+
// This ensures data is physically written to disk, not just cached.
87+
// Essential for SD cards, eMMC, and other flash storage on edge devices.
88+
if err := tmpFile.Sync(); err != nil {
89+
return fmt.Errorf("failed to sync temp file: %w", err)
90+
}
91+
92+
// Set file permissions before closing
93+
if err := tmpFile.Chmod(perm); err != nil {
94+
return fmt.Errorf("failed to set permissions: %w", err)
95+
}
96+
97+
// Close file before rename (required on Windows)
98+
if err := tmpFile.Close(); err != nil {
99+
return fmt.Errorf("failed to close temp file: %w", err)
100+
}
101+
102+
// Atomic rename: temp file becomes the target
103+
// On POSIX: rename() is atomic
104+
// On Windows: Rename() is atomic for files
105+
if err := os.Rename(tmpPath, path); err != nil {
106+
return fmt.Errorf("failed to rename temp file: %w", err)
107+
}
108+
109+
// Sync directory to ensure rename is durable
110+
// This prevents the renamed file from disappearing after a crash
111+
if dirFile, err := os.Open(dir); err == nil {
112+
_ = dirFile.Sync()
113+
dirFile.Close()
114+
}
115+
116+
// Success: skip cleanup (file was renamed, no temp to remove)
117+
cleanup = false
118+
return nil
119+
}

pkg/heartbeat/service.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616

1717
"github.com/sipeed/picoclaw/pkg/bus"
1818
"github.com/sipeed/picoclaw/pkg/constants"
19+
"github.com/sipeed/picoclaw/pkg/fileutil"
1920
"github.com/sipeed/picoclaw/pkg/logger"
2021
"github.com/sipeed/picoclaw/pkg/state"
2122
"github.com/sipeed/picoclaw/pkg/tools"
@@ -275,7 +276,7 @@ This file contains tasks for the heartbeat service to check periodically.
275276
Add your heartbeat tasks below this line:
276277
`
277278

278-
if err := os.WriteFile(heartbeatPath, []byte(defaultContent), 0o644); err != nil {
279+
if err := fileutil.WriteFileAtomic(heartbeatPath, []byte(defaultContent), 0o644); err != nil {
279280
hs.logErrorf("Failed to create default HEARTBEAT.md: %v", err)
280281
} else {
281282
hs.logInfof("Created default HEARTBEAT.md template")

pkg/skills/installer.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"path/filepath"
1111
"time"
1212

13+
"github.com/sipeed/picoclaw/pkg/fileutil"
1314
"github.com/sipeed/picoclaw/pkg/utils"
1415
)
1516

@@ -66,7 +67,9 @@ func (si *SkillInstaller) InstallFromGitHub(ctx context.Context, repo string) er
6667
}
6768

6869
skillPath := filepath.Join(skillDir, "SKILL.md")
69-
if err := os.WriteFile(skillPath, body, 0o644); err != nil {
70+
71+
// Use unified atomic write utility with explicit sync for flash storage reliability.
72+
if err := fileutil.WriteFileAtomic(skillPath, body, 0o600); err != nil {
7073
return fmt.Errorf("failed to write skill file: %w", err)
7174
}
7275

pkg/state/state.go

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import (
88
"path/filepath"
99
"sync"
1010
"time"
11+
12+
"github.com/sipeed/picoclaw/pkg/fileutil"
1113
)
1214

1315
// State represents the persistent state for a workspace.
@@ -124,33 +126,20 @@ func (sm *Manager) GetTimestamp() time.Time {
124126
// saveAtomic performs an atomic save using temp file + rename.
125127
// This ensures that the state file is never corrupted:
126128
// 1. Write to a temp file
127-
// 2. Rename temp file to target (atomic on POSIX systems)
128-
// 3. If rename fails, cleanup the temp file
129+
// 2. Sync to disk (critical for SD cards/flash storage)
130+
// 3. Rename temp file to target (atomic on POSIX systems)
131+
// 4. If rename fails, cleanup the temp file
129132
//
130133
// Must be called with the lock held.
131134
func (sm *Manager) saveAtomic() error {
132-
// Create temp file in the same directory as the target
133-
tempFile := sm.stateFile + ".tmp"
134-
135-
// Marshal state to JSON
135+
// Use unified atomic write utility with explicit sync for flash storage reliability.
136+
// Using 0o600 (owner read/write only) for secure default permissions.
136137
data, err := json.MarshalIndent(sm.state, "", " ")
137138
if err != nil {
138139
return fmt.Errorf("failed to marshal state: %w", err)
139140
}
140141

141-
// Write to temp file
142-
if err := os.WriteFile(tempFile, data, 0o644); err != nil {
143-
return fmt.Errorf("failed to write temp file: %w", err)
144-
}
145-
146-
// Atomic rename from temp to target
147-
if err := os.Rename(tempFile, sm.stateFile); err != nil {
148-
// Cleanup temp file if rename fails
149-
os.Remove(tempFile)
150-
return fmt.Errorf("failed to rename temp file: %w", err)
151-
}
152-
153-
return nil
142+
return fileutil.WriteFileAtomic(sm.stateFile, data, 0o600)
154143
}
155144

156145
// load loads the state from disk.

0 commit comments

Comments
 (0)