diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index d5c7f5d..69b0306 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -10,7 +10,7 @@ Requirements for v1.2 Tag Editing milestone. Each maps to roadmap phases. ### Schema & Safety - [x] **SCHEMA-01**: FTS5 search_index migrated to `contentless_delete=1` for safe row-level updates -- [ ] **SCHEMA-02**: Atomic file write utility (write-to-temp-then-rename in same directory) +- [x] **SCHEMA-02**: Atomic file write utility (write-to-temp-then-rename in same directory) ### Tag Writing @@ -18,7 +18,7 @@ Requirements for v1.2 Tag Editing milestone. Each maps to roadmap phases. - [ ] **WRITE-02**: Write metadata tags to FLAC files via Vorbis Comments - [ ] **WRITE-03**: Write metadata tags to OGG Vorbis files via custom page rewriter - [ ] **WRITE-04**: Embed cover art image (JPEG/PNG) in MP3 and FLAC files -- [ ] **WRITE-05**: All file writes use atomic write-to-temp-then-rename to prevent corruption +- [x] **WRITE-05**: All file writes use atomic write-to-temp-then-rename to prevent corruption - [ ] **WRITE-06**: Currently-playing file is stopped before writing (player safety) ### Database Sync @@ -92,12 +92,12 @@ Which phases cover which requirements. Updated during roadmap creation. | Requirement | Phase | Status | |-------------|-------|--------| | SCHEMA-01 | Phase 15 | Complete | -| SCHEMA-02 | Phase 15 | Pending | +| SCHEMA-02 | Phase 15 | Complete | | WRITE-01 | Phase 16 | Pending | | WRITE-02 | Phase 16 | Pending | | WRITE-03 | Phase 19 | Pending | | WRITE-04 | Phase 16 | Pending | -| WRITE-05 | Phase 15 | Pending | +| WRITE-05 | Phase 15 | Complete | | WRITE-06 | Phase 16 | Pending | | SYNC-01 | Phase 16 | Pending | | SYNC-02 | Phase 16 | Pending | diff --git a/.planning/STATE.md b/.planning/STATE.md index a5a7536..899b4a5 100644 --- a/.planning/STATE.md +++ b/.planning/STATE.md @@ -31,7 +31,7 @@ Last activity: 2026-03-16 — Completed 15-02 (AtomicWrite utility) | Phase | Status | |-------|--------| -| 15. Schema Migration & Write Safety | In progress (1/2 plans) | +| 15. Schema Migration & Write Safety | Complete (2/2 plans) | | 16. Tag Writing & Database Sync | Not started | | 17. Single Track Edit | Not started | | 18. Batch Edit | Not started | @@ -58,6 +58,7 @@ Last activity: 2026-03-16 — Completed 15-02 (AtomicWrite utility) | Phase | Plan | Duration | Tasks | Files | |-------|------|----------|-------|-------| | 15 | 01 | 15min | 2 | 5 | +| 15 | 02 | 16min | 2 | 2 | ## Accumulated Context @@ -79,6 +80,8 @@ Decisions from v1.0 and v1.1 are archived in PROJECT.md Key Decisions table. Key | Decision | Rationale | |----------|-----------| | Inlined migration 8 SQL rather than calling DB struct methods | `runMigrations` receives raw `*sql.DB`, not `*DB` — cannot call receiver methods | +| Deterministic `.yj-tmp` suffix for temp files | Enables reliable orphan cleanup without directory scanning | +| `*slog.Logger` as first param for AtomicWrite | Matches codebase convention — all packages accept logger as first arg | ### v1.2 Roadmap Decisions @@ -119,9 +122,9 @@ Decisions from v1.0 and v1.1 are archived in PROJECT.md Key Decisions table. Key ### Last Session **Date:** 2026-03-16 -**What happened:** Executed Phase 15 Plan 01 — migrated FTS5 search_index to contentless_delete=1, implemented real DeleteSearchIndex, added migration 8, added 3 new tests. -**Where we stopped:** Completed 15-01-PLAN.md -**Next action:** Execute 15-02-PLAN.md (atomic write utility) +**What happened:** Executed Phase 15 Plan 02 — created backend/fileutil package with AtomicWrite function (callback API, .yj-tmp suffix, permission preservation, orphan cleanup, cross-device rejection) and 7 comprehensive tests. +**Where we stopped:** Completed 15-02-PLAN.md — Phase 15 complete +**Next action:** `/gsd-plan-phase 16` — Tag Writing & Database Sync --- *State initialized: 2026-02-27* @@ -135,4 +138,4 @@ Decisions from v1.0 and v1.1 are archived in PROJECT.md Key Decisions table. Key | 19 | fix phantom playlist tracks with multi-root path resolution | 2026-03-16 | 9144ded | [19-fix-phantom-playlist-tracks](./quick/19-fix-phantom-playlist-tracks/) | Last activity: 2026-03-16 - Completed quick task 19: fix phantom playlist tracks with multi-root path resolution -*Last updated: 2026-03-16 — Completed 15-01 (FTS5 migration + delete support)* +*Last updated: 2026-03-16 — Completed 15-02 (AtomicWrite utility) — Phase 15 complete* diff --git a/.planning/phases/15-schema-migration-write-safety/15-02-SUMMARY.md b/.planning/phases/15-schema-migration-write-safety/15-02-SUMMARY.md new file mode 100644 index 0000000..10cbde6 --- /dev/null +++ b/.planning/phases/15-schema-migration-write-safety/15-02-SUMMARY.md @@ -0,0 +1,118 @@ +--- +phase: 15-schema-migration-write-safety +plan: 02 +subsystem: database +tags: [atomic-write, file-safety, os-rename, temp-file] + +# Dependency graph +requires: + - phase: none + provides: standalone utility package +provides: + - General-purpose AtomicWrite function for safe file modifications + - ErrCrossDevice sentinel for cross-filesystem detection + - Orphan .yj-tmp cleanup on each write operation +affects: [16-tag-writing-database-sync, 19-ogg-vorbis-tag-writing] + +# Tech tracking +tech-stack: + added: [] + patterns: [write-to-temp-then-rename, callback-API, deterministic-temp-suffix] + +key-files: + created: + - backend/fileutil/atomicwrite.go + - backend/fileutil/atomicwrite_test.go + modified: [] + +key-decisions: + - "Used *slog.Logger as first parameter for consistency with codebase conventions" + - "Deterministic .yj-tmp suffix (not random) enables reliable orphan cleanup" + - "Cross-device rejection via ErrCrossDevice sentinel wrapping syscall.EXDEV — no copy fallback" + - "Default 0644 permissions for new files; stat-and-preserve for existing files" + +patterns-established: + - "AtomicWrite callback API: AtomicWrite(logger, path, func(tmp *os.File) error) error" + - "Deterministic temp file suffix .yj-tmp for all atomic writes" + +requirements-completed: [SCHEMA-02, WRITE-05] + +# Metrics +duration: 16min +completed: 2026-03-16 +--- + +# Phase 15 Plan 02: Atomic Write Utility Summary + +**General-purpose AtomicWrite function with write-to-temp-then-rename, permission preservation, orphan cleanup, and cross-device rejection** + +## Performance + +- **Duration:** 16 min +- **Started:** 2026-03-16T21:57:34Z +- **Completed:** 2026-03-16T22:13:45Z +- **Tasks:** 2 +- **Files modified:** 2 + +## Accomplishments +- Created `backend/fileutil` package with exported `AtomicWrite` function using callback API pattern +- Implemented deterministic `.yj-tmp` temp file suffix with automatic orphan cleanup +- Permission preservation (stat existing target, apply mode before rename) with 0644 default for new files +- Cross-device rejection via `ErrCrossDevice` sentinel wrapping `syscall.EXDEV` +- 7 comprehensive test functions covering success, new file, callback error rollback, orphan cleanup, same-dir constraint, permission preservation (3 modes), and 1MiB sync verification + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Create backend/fileutil package with AtomicWrite** - `4d64b5d` (feat) +2. **Task 2: Add comprehensive tests for AtomicWrite** - `0cdfe48` (test) + +## Files Created/Modified +- `backend/fileutil/atomicwrite.go` - AtomicWrite function with ErrCrossDevice sentinel, orphan cleanup, permission preservation, cross-device rejection +- `backend/fileutil/atomicwrite_test.go` - 7 test functions: success, new file, callback error, orphan cleanup, same-dir temp, permission preservation (table-driven), sync and close + +## Decisions Made +- Used `*slog.Logger` as the first parameter for consistency with the codebase convention (all packages accept logger as first arg) +- Deterministic `.yj-tmp` suffix instead of random temp file names — enables reliable orphan cleanup without directory scanning +- Cross-device rejection wraps both `ErrCrossDevice` and `syscall.EXDEV` using Go 1.20+ multi-`%w` in `fmt.Errorf` +- Default 0644 permissions for new files (target doesn't exist); stat-and-preserve for existing files + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Fixed errorlint violation in cross-device error wrapping** +- **Found during:** Task 1 (AtomicWrite implementation) +- **Issue:** `fmt.Errorf("%w: %s", ErrCrossDevice, err)` used `%s` for the second error, violating the `errorlint` linter rule that requires `%w` for all error format verbs +- **Fix:** Changed to `fmt.Errorf("%w: %w", ErrCrossDevice, err)` using Go 1.20+ multi-wrapping +- **Files modified:** backend/fileutil/atomicwrite.go +- **Verification:** `golangci-lint run` passes with 0 issues +- **Committed in:** 4d64b5d (Task 1 commit) + +**2. [Rule 1 - Bug] Fixed err113 lint violation in test code** +- **Found during:** Task 2 (test implementation) +- **Issue:** `errors.New("simulated write failure")` defined inline in test function violated `err113` linter (dynamic error creation) +- **Fix:** Extracted to package-level `var errSimulatedFailure = errors.New("simulated write failure")` +- **Files modified:** backend/fileutil/atomicwrite_test.go +- **Verification:** `golangci-lint run` passes with 0 issues +- **Committed in:** 0cdfe48 (Task 2 commit) + +--- + +**Total deviations:** 2 auto-fixed (2 bugs — linter violations) +**Impact on plan:** Both auto-fixes necessary for lint compliance. No scope creep. + +## Issues Encountered +None + +## User Setup Required +None - no external service configuration required. + +## Next Phase Readiness +- AtomicWrite utility ready for Phase 16 tag writers to import +- No blockers — Phase 15 infrastructure complete (Plan 01: FTS5 migration, Plan 02: atomic write) + +--- +*Phase: 15-schema-migration-write-safety* +*Completed: 2026-03-16*