1561374c04
* 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>
105 lines
3.5 KiB
YAML
105 lines
3.5 KiB
YAML
version: "2"
|
|
|
|
run:
|
|
timeout: 5m
|
|
tests: false
|
|
|
|
linters:
|
|
default: 'none'
|
|
enable:
|
|
- errcheck
|
|
- gosec
|
|
- misspell
|
|
- unconvert
|
|
- unparam
|
|
|
|
settings:
|
|
errcheck:
|
|
exclude-functions:
|
|
- (*database/sql.DB).Close
|
|
- (*database/sql.Rows).Close
|
|
- (*database/sql.Tx).Rollback
|
|
- (*database/sql.Stmt).Close
|
|
- (*database/sql.Conn).Close
|
|
- (*os.File).Close
|
|
- (os).RemoveAll
|
|
- (os).Remove
|
|
- (os).Setenv
|
|
- (os).Unsetenv
|
|
- (os).Chdir
|
|
- (os).MkdirAll
|
|
- (fmt).Sscanf
|
|
misspell:
|
|
locale: US
|
|
|
|
exclusions:
|
|
rules:
|
|
# G304: File inclusion via variable in tests is safe (test data)
|
|
- path: '_test\.go'
|
|
linters:
|
|
- gosec
|
|
text: "G304"
|
|
# G306: File permissions 0644 in tests are acceptable (test fixtures)
|
|
- path: '_test\.go'
|
|
linters:
|
|
- gosec
|
|
text: "G306"
|
|
# G304: Safe file reads from known JSONL and error paths
|
|
- path: 'cmd/bd/autoflush\.go|cmd/bd/daemon_autostart\.go|cmd/bd/doctor/fix/sync_branch\.go|cmd/bd/rename_prefix\.go|cmd/bd/sync_merge\.go|internal/beads/beads\.go|internal/daemon/discovery\.go|internal/daemonrunner/sync\.go|internal/syncbranch/worktree\.go'
|
|
linters:
|
|
- gosec
|
|
text: "G304"
|
|
# G302/G306: Directory/file permissions 0700/0750 are acceptable
|
|
- linters:
|
|
- gosec
|
|
text: "G302.*0700|G301.*0750"
|
|
# G302/G306: JSONL files and error logs need 0644 for debugging/sharing
|
|
- path: 'cmd/bd/autoflush\.go|cmd/bd/daemon\.go|cmd/bd/daemon_sync_branch\.go|cmd/bd/setup\.go|internal/daemon/registry\.go|internal/daemonrunner/daemon\.go|internal/git/worktree\.go'
|
|
linters:
|
|
- gosec
|
|
text: "G306"
|
|
# G306: Git hooks must be executable (0700)
|
|
- path: 'cmd/bd/init\.go'
|
|
linters:
|
|
- gosec
|
|
text: "G306.*0700"
|
|
# G204: Safe subprocess launches with validated arguments
|
|
- path: 'cmd/bd/daemon_autostart\.go|cmd/bd/daemon_sync_branch\.go|cmd/bd/doctor\.go|cmd/bd/doctor/fix/sync_branch\.go|cmd/bd/gate\.go|cmd/bd/gate_discover\.go|cmd/bd/jira\.go|cmd/bd/migrate_sync\.go|cmd/bd/show\.go|cmd/bd/sync\.go|internal/git/worktree\.go|internal/syncbranch/worktree\.go'
|
|
linters:
|
|
- gosec
|
|
text: 'G204'
|
|
# G104: Deferred file closes - errors are non-critical
|
|
- path: 'cmd/bd/show\.go'
|
|
linters:
|
|
- gosec
|
|
text: "G104.*Close"
|
|
# G115: Safe integer conversions in backoff calculations
|
|
- path: 'cmd/bd/daemon_autostart\.go'
|
|
linters:
|
|
- gosec
|
|
text: "G115"
|
|
# G201: SQL with fmt.Sprintf using placeholders (IN clause expansion)
|
|
- path: 'internal/storage/sqlite/(dependencies|batch_ops)\.go'
|
|
linters:
|
|
- gosec
|
|
text: "G201"
|
|
# errcheck: Ignore unchecked errors in test files for common cleanup patterns
|
|
- path: '_test\.go'
|
|
linters:
|
|
- errcheck
|
|
text: "Error return value of .*(Close|Rollback|RemoveAll|Setenv|Unsetenv|Chdir|MkdirAll|Remove|Write|SetReadDeadline|SetDeadline|Start|Stop).* is not checked"
|
|
|
|
# unparam: Placeholder functions that may return errors in future implementation
|
|
- path: 'cmd/bd/jira\.go'
|
|
linters:
|
|
- unparam
|
|
text: 'reimportConflicts|resolveConflictsByTimestamp'
|
|
# misspell: "cancelled" is intentional - matching GitHub API response values
|
|
- path: 'cmd/bd/gate\.go'
|
|
linters:
|
|
- misspell
|
|
text: 'cancelled'
|
|
|
|
issues:
|
|
uniq-by-line: true
|