* feat(sync): implement pull-first synchronization strategy - Add --pull-first flag and logic to sync command - Introduce 3-way merge stub for issue synchronization - Add concurrent edit tests for the pull-first flow Ensures local changes are reconciled with remote updates before pushing to prevent data loss. * feat(sync): implement 3-way merge and state tracking - Implement 3-way merge algorithm for issue synchronization - Add base state storage to track changes between syncs - Add comprehensive tests for merge logic and persistence Ensures data consistency and prevents data loss during concurrent issue updates. * feat(sync): implement field-level conflict merging - Implement field-level merge logic for issue conflicts - Add unit tests for field-level merge strategies Reduces manual intervention by automatically resolving overlapping updates at the field level. * refactor(sync): simplify sync flow by removing ZFC checks The previous sync implementation relied on Zero-False-Convergence (ZFC) staleness checks which are redundant following the transition to structural 3-way merging. This legacy logic added complexity and maintenance overhead without providing additional safety. This commit introduces a streamlined sync pipeline: - Remove ZFC staleness validation from primary sync flow - Update safety documentation to reflect current merge strategy - Eliminate deprecated unit tests associated with ZFC logic These changes reduce codebase complexity while maintaining data integrity through the robust structural 3-way merge implementation. * feat(sync): default to pull-first sync workflow - Set pull-first as the primary synchronization workflow - Refactor core sync logic for better maintainability - Update concurrent edit tests to validate 3-way merge logic Reduces merge conflicts by ensuring local state is current before pushing changes. * refactor(sync): clean up lint issues in merge code - Remove unused error return from MergeIssues (never returned error) - Use _ prefix for unused _base parameter in mergeFieldLevel - Update callers to not expect error from MergeIssues - Keep nolint:gosec for trusted internal file path * test(sync): add mode compatibility and upgrade safety tests Add tests addressing Steve's PR #918 review concerns: - TestSyncBranchModeWithPullFirst: Verifies sync-branch config storage and git branch creation work with pull-first - TestExternalBeadsDirWithPullFirst: Verifies external BEADS_DIR detection and pullFromExternalBeadsRepo - TestUpgradeFromOldSync: Validates upgrade safety when sync_base.jsonl doesn't exist (first sync after upgrade) - TestMergeIssuesWithBaseState: Comprehensive 3-way merge cases - TestLabelUnionMerge: Verifies labels use union (no data loss) Key upgrade behavior validated: - base=nil (no sync_base.jsonl) safely handles all cases - Local-only issues kept (StrategyLocal) - Remote-only issues kept (StrategyRemote) - Overlapping issues merged (LWW scalars, union labels) * fix(sync): report line numbers for malformed JSON Problem: - JSON decoding errors when loading sync base state lacked line numbers - Difficult to identify location of syntax errors in large state files Solution: - Include line number reporting in JSON decoder errors during state loading - Add regression tests for malformed sync base file scenarios Impact: - Users receive actionable feedback for corrupted state files - Faster troubleshooting of manual configuration errors * fix(sync): warn on large clock skew during sync Problem: - Unsynchronized clocks between systems could lead to silent merge errors - No mechanism existed to alert users of significant timestamp drift Solution: - Implement clock skew detection during sync merge - Log a warning when large timestamp differences are found - Add comprehensive unit tests for skew reporting Impact: - Users are alerted to potential synchronization risks - Easier debugging of time-related merge issues * fix(sync): defer state update until remote push succeeds Problem: - Base state updated before confirming remote push completion - Failed pushes resulted in inconsistent local state tracking Solution: - Defer base state update until after the remote push succeeds Impact: - Ensures local state accurately reflects remote repository status - Prevents state desynchronization during network or push failures * fix(sync): prevent concurrent sync operations Problem: - Multiple sync processes could run simultaneously - Overlapping operations risk data corruption and race conditions Solution: - Implement file-based locking using gofrs/flock - Add integration tests to verify locking behavior Impact: - Guarantees execution of a single sync process at a time - Eliminates potential for data inconsistency during sync * docs: document sync architecture and merge model - Detail the 3-way merge model logic - Describe the core synchronization architecture principles * fix(lint): explicitly ignore lock.Unlock return value errcheck linter flagged bare defer lock.Unlock() calls. Wrap in anonymous function with explicit _ assignment to acknowledge intentional ignore of unlock errors during cleanup. * fix(lint): add sync_merge.go to G304 exclusions The loadBaseState and saveBaseState functions use file paths derived from trusted internal sources (beadsDir parameter from config). Add to existing G304 exclusion list for safe JSONL file operations. * feat(sync): integrate sync-branch into pull-first flow When sync.branch is configured, doPullFirstSync now: - Calls PullFromSyncBranch before merge - Calls CommitToSyncBranch after export This ensures sync-branch mode uses the correct branch for pull/push operations. * test(sync): add E2E tests for sync-branch and external BEADS_DIR Adds comprehensive end-to-end tests: - TestSyncBranchE2E: verifies pull→merge→commit flow with remote changes - TestExternalBeadsDirE2E: verifies sync with separate beads repository - TestExternalBeadsDirDetection: edge cases for repo detection - TestCommitToExternalBeadsRepo: commit handling * refactor(sync): remove unused rollbackJSONLFromGit Function was defined but never called. Pull-first flow saves base state after successful push, making this safety net unnecessary. * test(sync): add export-only mode E2E test Add TestExportOnlySync to cover --no-pull flag which was the only untested sync mode. This completes full mode coverage: - Normal (pull-first): sync_test.go, sync_merge_test.go - Sync-branch: sync_modes_test.go:TestSyncBranchE2E (PR#918) - External BEADS_DIR: sync_external_test.go (PR#918) - From-main: sync_branch_priority_test.go - Local-only: sync_local_only_test.go - Export-only: sync_modes_test.go:TestExportOnlySync (this commit) Refs: #911 * docs(sync): add sync modes reference section Document all 6 sync modes with triggers, flows, and use cases. Include mode selection decision tree and test coverage matrix. Co-authored-by: Claude <noreply@anthropic.com> * test(sync): upgrade sync-branch E2E tests to bare repo - Replace mocked repository with real bare repo setup - Implement multi-machine simulation in sync tests - Refactor test logic to handle distributed states Coverage: sync-branch end-to-end scenarios * test(sync): add daemon sync-branch E2E tests - Implement E2E tests for daemon sync-branch flow - Add test cases for force-overwrite scenarios Coverage: daemon sync-branch workflow in cmd/bd * docs(sync): document sync-branch paths and E2E architecture - Describe sync-branch CLI and Daemon execution flow - Document the end-to-end test architecture * build(nix): update vendorHash for gofrs/flock dependency New dependency added for file-based sync locking changes the Go module checksum. --------- Co-authored-by: Claude <noreply@anthropic.com>
310 lines
10 KiB
Go
310 lines
10 KiB
Go
package main
|
|
|
|
import (
|
|
"cmp"
|
|
"context"
|
|
"encoding/json"
|
|
"fmt"
|
|
"os"
|
|
"path/filepath"
|
|
"slices"
|
|
"time"
|
|
|
|
"github.com/steveyegge/beads/internal/config"
|
|
"github.com/steveyegge/beads/internal/rpc"
|
|
"github.com/steveyegge/beads/internal/types"
|
|
"github.com/steveyegge/beads/internal/ui"
|
|
"github.com/steveyegge/beads/internal/validation"
|
|
)
|
|
|
|
// ExportResult contains information needed to finalize an export after git commit.
|
|
// This enables atomic sync by deferring metadata updates until after git commit succeeds.
|
|
// See GH#885 for the atomicity gap this fixes.
|
|
type ExportResult struct {
|
|
// JSONLPath is the path to the exported JSONL file
|
|
JSONLPath string
|
|
|
|
// ExportedIDs are the issue IDs that were exported
|
|
ExportedIDs []string
|
|
|
|
// ContentHash is the hash of the exported JSONL content
|
|
ContentHash string
|
|
|
|
// ExportTime is when the export was performed (RFC3339Nano format)
|
|
ExportTime string
|
|
}
|
|
|
|
// finalizeExport updates SQLite metadata after a successful git commit.
|
|
// This is the second half of atomic sync - it marks the export as complete
|
|
// only after the git commit succeeds. If git commit fails, the metadata
|
|
// remains unchanged so the system knows the sync is incomplete.
|
|
// See GH#885 for the atomicity gap this fixes.
|
|
func finalizeExport(ctx context.Context, result *ExportResult) {
|
|
if result == nil {
|
|
return
|
|
}
|
|
|
|
// Ensure store is initialized
|
|
if err := ensureStoreActive(); err != nil {
|
|
fmt.Fprintf(os.Stderr, "Warning: failed to initialize store for finalize: %v\n", err)
|
|
return
|
|
}
|
|
|
|
// Clear dirty flags for exported issues
|
|
if len(result.ExportedIDs) > 0 {
|
|
if err := store.ClearDirtyIssuesByID(ctx, result.ExportedIDs); err != nil {
|
|
// Non-fatal warning
|
|
fmt.Fprintf(os.Stderr, "Warning: failed to clear dirty flags: %v\n", err)
|
|
}
|
|
}
|
|
|
|
// Clear auto-flush state
|
|
clearAutoFlushState()
|
|
|
|
// Update jsonl_content_hash metadata to enable content-based staleness detection
|
|
if result.ContentHash != "" {
|
|
if err := store.SetMetadata(ctx, "jsonl_content_hash", result.ContentHash); err != nil {
|
|
// Non-fatal warning: Metadata update failures are intentionally non-fatal to prevent blocking
|
|
// successful exports. System degrades gracefully to mtime-based staleness detection if metadata
|
|
// is unavailable. This ensures export operations always succeed even if metadata storage fails.
|
|
fmt.Fprintf(os.Stderr, "Warning: failed to update jsonl_content_hash: %v\n", err)
|
|
}
|
|
}
|
|
|
|
// Update last_import_time
|
|
if result.ExportTime != "" {
|
|
if err := store.SetMetadata(ctx, "last_import_time", result.ExportTime); err != nil {
|
|
// Non-fatal warning (see above comment about graceful degradation)
|
|
fmt.Fprintf(os.Stderr, "Warning: failed to update last_import_time: %v\n", err)
|
|
}
|
|
}
|
|
|
|
// Update database mtime to be >= JSONL mtime (fixes #278, #301, #321)
|
|
// This prevents validatePreExport from incorrectly blocking on next export
|
|
if result.JSONLPath != "" {
|
|
beadsDir := filepath.Dir(result.JSONLPath)
|
|
dbPath := filepath.Join(beadsDir, "beads.db")
|
|
if err := TouchDatabaseFile(dbPath, result.JSONLPath); err != nil {
|
|
// Non-fatal warning
|
|
fmt.Fprintf(os.Stderr, "Warning: failed to update database mtime: %v\n", err)
|
|
}
|
|
}
|
|
}
|
|
|
|
// exportToJSONL exports the database to JSONL format.
|
|
// This is a convenience wrapper that exports and immediately finalizes.
|
|
// For atomic sync operations, use exportToJSONLDeferred + finalizeExport.
|
|
func exportToJSONL(ctx context.Context, jsonlPath string) error {
|
|
result, err := exportToJSONLDeferred(ctx, jsonlPath)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
// Immediately finalize for backward compatibility
|
|
finalizeExport(ctx, result)
|
|
return nil
|
|
}
|
|
|
|
// exportToJSONLDeferred exports the database to JSONL format but does NOT update
|
|
// SQLite metadata. The caller must call finalizeExport() after git commit succeeds.
|
|
// This enables atomic sync where metadata is only updated after git commit.
|
|
// See GH#885 for the atomicity gap this fixes.
|
|
func exportToJSONLDeferred(ctx context.Context, jsonlPath string) (*ExportResult, error) {
|
|
// If daemon is running, use RPC
|
|
// Note: daemon already handles its own metadata updates
|
|
if daemonClient != nil {
|
|
exportArgs := &rpc.ExportArgs{
|
|
JSONLPath: jsonlPath,
|
|
}
|
|
resp, err := daemonClient.Export(exportArgs)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("daemon export failed: %w", err)
|
|
}
|
|
if !resp.Success {
|
|
return nil, fmt.Errorf("daemon export error: %s", resp.Error)
|
|
}
|
|
// Daemon handles its own metadata updates, return nil result
|
|
return nil, nil
|
|
}
|
|
|
|
// Direct mode: access store directly
|
|
// Ensure store is initialized
|
|
if err := ensureStoreActive(); err != nil {
|
|
return nil, fmt.Errorf("failed to initialize store: %w", err)
|
|
}
|
|
|
|
// Get all issues including tombstones for sync propagation (bd-rp4o fix)
|
|
// Tombstones must be exported so they propagate to other clones and prevent resurrection
|
|
issues, err := store.SearchIssues(ctx, "", types.IssueFilter{IncludeTombstones: true})
|
|
if err != nil {
|
|
return nil, fmt.Errorf("failed to get issues: %w", err)
|
|
}
|
|
|
|
// Safety check: prevent exporting empty database over non-empty JSONL
|
|
// This blocks the catastrophic case where an empty/corrupted DB would overwrite
|
|
// a valid JSONL. For staleness handling, use --pull-first which provides
|
|
// structural protection via 3-way merge.
|
|
if len(issues) == 0 {
|
|
existingCount, countErr := countIssuesInJSONL(jsonlPath)
|
|
if countErr != nil {
|
|
// If we can't read the file, it might not exist yet, which is fine
|
|
if !os.IsNotExist(countErr) {
|
|
fmt.Fprintf(os.Stderr, "Warning: failed to read existing JSONL: %v\n", countErr)
|
|
}
|
|
} else if existingCount > 0 {
|
|
return nil, fmt.Errorf("refusing to export empty database over non-empty JSONL file (database: 0 issues, JSONL: %d issues)", existingCount)
|
|
}
|
|
}
|
|
|
|
// Filter out wisps - they should never be exported to JSONL
|
|
// Wisps exist only in SQLite and are shared via .beads/redirect, not JSONL.
|
|
// This prevents "zombie" issues that resurrect after mol squash deletes them.
|
|
filteredIssues := make([]*types.Issue, 0, len(issues))
|
|
for _, issue := range issues {
|
|
if issue.Ephemeral {
|
|
continue
|
|
}
|
|
filteredIssues = append(filteredIssues, issue)
|
|
}
|
|
issues = filteredIssues
|
|
|
|
// Sort by ID for consistent output
|
|
slices.SortFunc(issues, func(a, b *types.Issue) int {
|
|
return cmp.Compare(a.ID, b.ID)
|
|
})
|
|
|
|
// Populate dependencies for all issues (avoid N+1)
|
|
allDeps, err := store.GetAllDependencyRecords(ctx)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("failed to get dependencies: %w", err)
|
|
}
|
|
for _, issue := range issues {
|
|
issue.Dependencies = allDeps[issue.ID]
|
|
}
|
|
|
|
// Populate labels for all issues
|
|
for _, issue := range issues {
|
|
labels, err := store.GetLabels(ctx, issue.ID)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("failed to get labels for %s: %w", issue.ID, err)
|
|
}
|
|
issue.Labels = labels
|
|
}
|
|
|
|
// Populate comments for all issues
|
|
for _, issue := range issues {
|
|
comments, err := store.GetIssueComments(ctx, issue.ID)
|
|
if err != nil {
|
|
return nil, fmt.Errorf("failed to get comments for %s: %w", issue.ID, err)
|
|
}
|
|
issue.Comments = comments
|
|
}
|
|
|
|
// Create temp file for atomic write
|
|
dir := filepath.Dir(jsonlPath)
|
|
base := filepath.Base(jsonlPath)
|
|
tempFile, err := os.CreateTemp(dir, base+".tmp.*")
|
|
if err != nil {
|
|
return nil, fmt.Errorf("failed to create temp file: %w", err)
|
|
}
|
|
tempPath := tempFile.Name()
|
|
defer func() {
|
|
_ = tempFile.Close()
|
|
_ = os.Remove(tempPath)
|
|
}()
|
|
|
|
// Write JSONL
|
|
encoder := json.NewEncoder(tempFile)
|
|
exportedIDs := make([]string, 0, len(issues))
|
|
for _, issue := range issues {
|
|
if err := encoder.Encode(issue); err != nil {
|
|
return nil, fmt.Errorf("failed to encode issue %s: %w", issue.ID, err)
|
|
}
|
|
exportedIDs = append(exportedIDs, issue.ID)
|
|
}
|
|
|
|
// Close temp file before rename (error checked implicitly by Rename success)
|
|
_ = tempFile.Close()
|
|
|
|
// Atomic replace
|
|
if err := os.Rename(tempPath, jsonlPath); err != nil {
|
|
return nil, fmt.Errorf("failed to replace JSONL file: %w", err)
|
|
}
|
|
|
|
// Set appropriate file permissions (0600: rw-------)
|
|
if err := os.Chmod(jsonlPath, 0600); err != nil {
|
|
// Non-fatal warning
|
|
fmt.Fprintf(os.Stderr, "Warning: failed to set file permissions: %v\n", err)
|
|
}
|
|
|
|
// Compute hash and time for the result (but don't update metadata yet)
|
|
contentHash, _ := computeJSONLHash(jsonlPath)
|
|
exportTime := time.Now().Format(time.RFC3339Nano)
|
|
|
|
return &ExportResult{
|
|
JSONLPath: jsonlPath,
|
|
ExportedIDs: exportedIDs,
|
|
ContentHash: contentHash,
|
|
ExportTime: exportTime,
|
|
}, nil
|
|
}
|
|
|
|
// validateOpenIssuesForSync validates all open issues against their templates
|
|
// before export, based on the validation.on-sync config setting.
|
|
// Returns an error if validation.on-sync is "error" and issues fail validation.
|
|
// Prints warnings if validation.on-sync is "warn".
|
|
// Does nothing if validation.on-sync is "none" (default).
|
|
func validateOpenIssuesForSync(ctx context.Context) error {
|
|
validationMode := config.GetString("validation.on-sync")
|
|
if validationMode == "none" || validationMode == "" {
|
|
return nil
|
|
}
|
|
|
|
// Ensure store is active
|
|
if err := ensureStoreActive(); err != nil {
|
|
return fmt.Errorf("failed to initialize store for validation: %w", err)
|
|
}
|
|
|
|
// Get all issues (excluding tombstones) and filter to open ones
|
|
allIssues, err := store.SearchIssues(ctx, "", types.IssueFilter{})
|
|
if err != nil {
|
|
return fmt.Errorf("failed to get issues for validation: %w", err)
|
|
}
|
|
|
|
// Filter to only open issues (not closed, not tombstones)
|
|
var issues []*types.Issue
|
|
for _, issue := range allIssues {
|
|
if issue.Status != types.StatusClosed && issue.Status != types.StatusTombstone {
|
|
issues = append(issues, issue)
|
|
}
|
|
}
|
|
|
|
// Validate each issue
|
|
var warnings []string
|
|
for _, issue := range issues {
|
|
if err := validation.LintIssue(issue); err != nil {
|
|
warnings = append(warnings, fmt.Sprintf("%s: %v", issue.ID, err))
|
|
}
|
|
}
|
|
|
|
if len(warnings) == 0 {
|
|
return nil
|
|
}
|
|
|
|
// Report based on mode
|
|
if validationMode == "error" {
|
|
fmt.Fprintf(os.Stderr, "%s Validation failed for %d issue(s):\n", ui.RenderFail("✗"), len(warnings))
|
|
for _, w := range warnings {
|
|
fmt.Fprintf(os.Stderr, " - %s\n", w)
|
|
}
|
|
return fmt.Errorf("template validation failed: %d issues missing required sections (set validation.on-sync: none or warn to proceed)", len(warnings))
|
|
}
|
|
|
|
// warn mode: print warnings but proceed
|
|
fmt.Fprintf(os.Stderr, "%s Validation warnings for %d issue(s):\n", ui.RenderWarn("⚠"), len(warnings))
|
|
for _, w := range warnings {
|
|
fmt.Fprintf(os.Stderr, " - %s\n", w)
|
|
}
|
|
|
|
return nil
|
|
}
|