Skip to content

refactor(pkg/utils): add unified atomic file write utility#706

Open
mosir wants to merge 11 commits intosipeed:mainfrom
mosir:fix/atomic-file-writes
Open

refactor(pkg/utils): add unified atomic file write utility#706
mosir wants to merge 11 commits intosipeed:mainfrom
mosir:fix/atomic-file-writes

Conversation

@mosir
Copy link

@mosir mosir commented Feb 24, 2026

Summary

This PR introduces a unified atomic file write utility (WriteFileAtomic) that eliminates code duplication
across 9 locations while improving data reliability on low-power edge devices.

Problem

The existing atomic write pattern (temp file + rename) was missing explicit Sync() calls in multiple
locations. On flash storage with write caching (SD cards, eMMC, SPI flash), this could lead to data corruption
on sudden power loss:

  1. Write data to temp file (cached)
  2. Rename temp to target (metadata updated)
  3. ⚡ Power loss before cache flush
  4. Result: target file exists but contains stale/corrupted data

Solution

Created pkg/utils/file.go with WriteFileAtomic() function that:

  • ✅ Uses temp file + rename pattern for atomicity
  • ✅ Calls Sync() before rename to ensure data is physically written
  • ✅ Uses secure 0o600 permissions by default
  • ✅ Handles cleanup on error via defer

Changes

New file:

  • pkg/utils/file.go - Unified atomic write utility (97 lines)

Refactored files (all now use utils.WriteFileAtomic):

  • pkg/tools/filesystem.go - hostFs, sandboxFs
  • pkg/config/config.go - SaveConfig
  • pkg/auth/store.go - SaveStore
  • pkg/cron/service.go - saveStoreUnsafe
  • pkg/agent/memory.go - WriteLongTerm, AppendToday
  • pkg/heartbeat/service.go - HEARTBEAT.md creation (0o644, user-editable)
  • pkg/skills/installer.go - skill installation
  • pkg/tools/skills_install.go - skill metadata
  • pkg/state/state.go - saveAtomic

Benefits

Metric Before After Improvement
Code duplication 9 locations × ~30 lines 1 utility function -68%
Maintenance cost Modify 9 files Modify 1 file -89%
Data safety Missing Sync() Explicit Sync() Critical fix
Default permissions Mixed 0o644/0o600 Consistent 0o600 More secure

Testing

All existing tests pass:
est ./pkg/tools/... ./pkg/config/... ./pkg/auth/... ./pkg/cron/... ./pkg/agent/... ./pkg/state/...
g/utils/...

Note: Some tests fail on Windows due to pre-existing platform-specific permission handling issues (unrelated
to this PR).

Security Notes

  • Default permissions: 0o600 (owner read/write only) for sensitive files
  • Exception: HEARTBEAT.md uses 0o644 intentionally as it's a user-editable config file without sensitive
    data

Related Issues

Fixes potential data corruption on power loss for edge devices running PicoClaw on flash storage (SD cards,
eMMC, SPI flash).


  • Code follows project conventions
  • All tests pass
  • No breaking changes (internal refactoring only)

//
// Copyright (c) 2026 PicoClaw contributors

package utils
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we need a better package name instead of utils

Copy link
Author

Choose a reason for hiding this comment

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

How about fileutil?

//
// // Public readable file
// err := utils.WriteFileAtomic("public.txt", data, 0o644)
func WriteFileAtomic(path string, data []byte, perm os.FileMode) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

The way you implement "atomicwrite" just using tmpfile for the job, it will corrupt the file if write to tmp file failed too.

Copy link
Author

Choose a reason for hiding this comment

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

You're correct. I've fixed the issue。

@mosir mosir requested a review from mengzhuo February 26, 2026 04:15
@alexhoshina
Copy link
Collaborator

Please run golangci-lint run --fix to pass the checks

@mosir
Copy link
Author

mosir commented Feb 26, 2026

Please run golangci-lint run --fix to pass the checks

Done — I ran golangci-lint with --fix and pushed the follow-up commit. The import-order/gci issues are resolved on
fix/atomic-file-writes.

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.

3 participants