Skip to content

Add atomic stack storage and migration primitives - #527

Open
skarim wants to merge 3 commits into
skarim/worktrees-scoped-git-executionfrom
skarim/worktrees-atomic-storage
Open

skarim wants to merge 3 commits into
skarim/worktrees-scoped-git-executionfrom
skarim/worktrees-atomic-storage

Conversation

@skarim

@skarim skarim commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

Makes the files that record stacks and interrupted operations safer to save and read. It also prepares the move from separate tracking files in each checkout to one shared stack catalog, without losing existing stack definitions.

Functionality and user impact

  • Write each updated file in full before replacing the old one, keeping its permissions. Other commands cannot read a half-written file, and a failed save reports an error rather than deleting the old file first.
  • Prepare a migration that combines separate stacks and matching copies of the same stack, keeps backups, and stops when definitions conflict or an older operation still needs recovery. Interrupted migrations can resume.
  • Add separate locks for saving the catalog and running a whole command. A save also checks whether another command changed the file since it was loaded.

Boundary: commands still use their existing catalog locations. #528 turns on shared storage and migration. This PR changes how files are saved, not branch contents or worktree layouts. Worktree support requires Git 2.36+ and does not create/remove worktrees or stash changes automatically.

Key areas to review

  • internal/stack/atomic.go: WriteAtomic / writeFileAtomic write and flush a temporary file before replacing the saved file. Check that failures are reported and temporary files are cleaned up.
  • atomic_unix.go and atomic_windows.go: handle replacement on each platform, including Windows readers that still have the old file open. Creating a backup must never overwrite an existing backup.
  • internal/stack/stack.go: Save and checkStale prevent overwriting changes made since loading. SaveNonBlocking lets an optional metadata refresh skip saving when another writer is active.
  • internal/stack/lock.go: Lock is held only while saving the catalog. LockOperation can cover a whole command, including branch changes and the final save. Keeping them separate lets the command save without trying to acquire a lock it already holds.
  • internal/stack/migration.go: MigrateLegacyState, mergeMigrationCatalogs, and checkMigrationSnapshot decide which definitions can be combined. Follow how original data is saved before the new catalog replaces it, and how interrupted work resumes.

Related issues

Part 2 of the 4-PR split of #520

@skarim
skarim added this pull request to stack #530 September 28, 2026 17:20
@skarim
skarim marked this pull request as ready for review September 29, 2026 16:27
Copilot AI balanced review requested due to automatic review settings September 29, 2026 16:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Windows recovery-state readers do not permit delete sharing, so concurrent atomic replacement can fail and leave stale recovery data.

Review effort: Balanced
Findings: 2 Medium severity

Open (2)
What changed in this PR

Adds atomic state persistence, separate operation/catalog locks, and resumable migration primitives for future shared worktree storage.

Changes:

  • Implements cross-platform atomic file replacement.
  • Adds migration conflict detection, backups, journaling, and recovery.
  • Extends locking and stale-write protection with comprehensive tests.
File Description
cmd/​rebase.go Uses atomic rebase-state writes.
internal/​git/​gitops_test.go Improves rebase failure diagnostics.
internal/​modify/​state.go Uses atomic modify-state writes.
internal/​stack/​atomic.go Implements shared atomic-write logic.
internal/​stack/​atomic_unix.go Adds Unix publication and directory syncing.
internal/​stack/​atomic_windows.go Adds Windows replacement and share-delete reads.
internal/​stack/​atomic_windows_test.go Tests Windows reader-safe replacement.
internal/​stack/​lock.go Separates catalog and operation locks.
internal/​stack/​lock_test.go Tests locking and atomic publication.
internal/​stack/​migration.go Implements resumable catalog migration.
internal/​stack/​stack.go Integrates atomic saves and migration guards.
internal/​stack/​stack_test.go Tests migration, conflicts, and recovery.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread cmd/rebase.go
Comment thread internal/modify/state.go
@skarim
skarim force-pushed the skarim/worktrees-atomic-storage branch from 4c4713f to ed0be40 Compare September 29, 2026 22:47

This branch has not been deployed

No deployments
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