Skip to content

Commit 04ea25a

Browse files
authored
feat: convergence check for 3-way board merge (#42) (#74)
- Added convergence check: when both local and remote changed but the local stage maps to the same option ID as the remote, the merge converges instead of reporting a conflict - Added `GetBoardItem()` on `GitHubProvider` to query current board item state - Added `resolveExpectedStatus()` helper to compute the expected remote option ID for a local stage - Updated `threeWayMerge()` to accept `currentRemote` and `expectedRemote` params - Updated `syncChange()` to query board before three-way merge decision - Added 6 new tests for convergence, conflict with expected, and both-changed convergence
1 parent 0ff20c4 commit 04ea25a

8 files changed

Lines changed: 598 additions & 50 deletions

File tree

board.go

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import (
1515
// Used for three-way merge: local stage, remote status, and last-synced base
1616
// detect what changed since last sync.
1717
type BoardBinding struct {
18-
Provider string `json:"provider"` // "github-projects", etc.
18+
Provider string `json:"provider"` // "github-projects", etc.
1919
ProjectID string `json:"project_id"`
2020
ItemID string `json:"item_id"`
2121
LocalStageBase Stage `json:"local_stage_base"` // what local stage was last time
@@ -26,14 +26,14 @@ type BoardBinding struct {
2626
// BoardState wraps all board bindings for a change. Enables multi-provider
2727
// and multi-project support. Format: .specsync/board.json
2828
type BoardState struct {
29-
Version int `json:"version"`
29+
Version int `json:"version"`
3030
Bindings map[string]BoardBinding `json:"bindings"` // key: "github-projects:owner/5"
3131
}
3232

3333
// ThreeWayMerge result: what action to take based on local, remote, and base state.
3434
type ThreeWayDecision struct {
35-
Action string // "none", "push-local", "report-conflict", "report-remote-move"
36-
Reason string // human-readable explanation
35+
Action string // "none", "push-local", "report-conflict", "report-remote-move"
36+
Reason string // human-readable explanation
3737
LocalChanged bool
3838
RemoteChanged bool
3939
}
@@ -454,6 +454,39 @@ func (p *GitHubProvider) boardMembership(ctx context.Context, ref Ref, schema *b
454454
return m, nil
455455
}
456456

457+
// GetBoardItem queries the board for the current item state. Returns the item ID,
458+
// status name, and status option ID. Returns empty values if the item is not on
459+
// the board. Used for three-way merge to get the current remote state.
460+
func (p *GitHubProvider) GetBoardItem(ctx context.Context, target BoardTarget, ref Ref) (itemID, statusName, statusOptionID string, err error) {
461+
schema, err := p.resolveBoardSchema(ctx, target)
462+
if err != nil {
463+
return "", "", "", err
464+
}
465+
member, err := p.boardMembership(ctx, ref, schema)
466+
if err != nil {
467+
return "", "", "", err
468+
}
469+
return member.itemID, member.statusName, member.statusOptionID, nil
470+
}
471+
472+
// resolveExpectedStatus resolves the expected Status option ID for a stage by
473+
// querying the board schema. Used for convergence checking in three-way merge.
474+
func resolveExpectedStatus(ctx context.Context, bp BoardProjector, target BoardTarget, stage Stage, ref Ref) (optionID, name string, err error) {
475+
if gh, ok := bp.(*GitHubProvider); ok {
476+
schema, err := gh.resolveBoardSchema(ctx, target)
477+
if err != nil {
478+
return "", "", err
479+
}
480+
name, optionID, err = gh.resolveStatus(target, stage, schema.statusField)
481+
if err != nil {
482+
return "", "", err
483+
}
484+
_ = ref // unused but kept for API consistency
485+
return optionID, name, nil
486+
}
487+
return "", "", nil
488+
}
489+
457490
func (p *GitHubProvider) addToBoard(ctx context.Context, projectID, contentID string) (string, error) {
458491
q := `mutation($projectId: ID!, $contentId: ID!) {
459492
addProjectV2ItemById(input: { projectId: $projectId, contentId: $contentId }) { item { id } }
@@ -589,9 +622,11 @@ func parseIssueURL(url string) (owner, repo string, number int, err error) {
589622
// threeWayMerge compares local stage, remote option, and base to detect conflicts
590623
// or determine if the board was changed by a human (human-move detection).
591624
// Returns the decision and whether to push or skip based on local/remote changes.
592-
func threeWayMerge(local Stage, remote string, base BoardBinding) ThreeWayDecision {
625+
// currentRemote is the current remote option ID (from board query).
626+
// expectedRemote is the option ID the local stage maps to (for convergence check).
627+
func threeWayMerge(local Stage, currentRemote, expectedRemote string, base BoardBinding) ThreeWayDecision {
593628
localChanged := local != base.LocalStageBase
594-
remoteChanged := remote != base.RemoteOptionIDBase
629+
remoteChanged := currentRemote != base.RemoteOptionIDBase
595630

596631
if !localChanged && !remoteChanged {
597632
return ThreeWayDecision{
@@ -616,6 +651,14 @@ func threeWayMerge(local Stage, remote string, base BoardBinding) ThreeWayDecisi
616651
}
617652
}
618653

654+
// Both changed: check convergence.
655+
if expectedRemote != "" && currentRemote == expectedRemote {
656+
return ThreeWayDecision{
657+
Action: "converged",
658+
Reason: "both local and remote changed but converged to the same state",
659+
}
660+
}
661+
619662
// Both changed: conflict.
620663
return ThreeWayDecision{
621664
Action: "report-conflict",
@@ -680,7 +723,7 @@ func saveBoardBinding(changeDir string, target BoardTarget, provider string, sta
680723
// Update binding base state to current (for next three-way merge).
681724
// After a successful push, local stage and remote option are in sync.
682725
binding.Provider = provider
683-
binding.ProjectID = plan.ProjectID // GraphQL node ID of the project
726+
binding.ProjectID = plan.ProjectID // GraphQL node ID of the project
684727
binding.LocalStageBase = stage
685728
binding.RemoteOptionIDBase = plan.StatusOptionID // What we just pushed to the board
686729
binding.SyncedAt = time.Now()

board_test.go

Lines changed: 56 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ func TestThreeWayMergeNoChange(t *testing.T) {
435435
RemoteOptionIDBase: "OPT_PROG",
436436
}
437437

438-
decision := threeWayMerge(StageActive, "OPT_PROG", base)
438+
decision := threeWayMerge(StageActive, "OPT_PROG", "", base)
439439

440440
if decision.Action != "none" {
441441
t.Errorf("action = %q, want %q", decision.Action, "none")
@@ -452,7 +452,7 @@ func TestThreeWayMergeLocalChanged(t *testing.T) {
452452
RemoteOptionIDBase: "OPT_PROG",
453453
}
454454

455-
decision := threeWayMerge(StageComplete, "OPT_PROG", base)
455+
decision := threeWayMerge(StageComplete, "OPT_PROG", "", base)
456456

457457
if decision.Action != "push-local" {
458458
t.Errorf("action = %q, want %q", decision.Action, "push-local")
@@ -475,7 +475,7 @@ func TestThreeWayMergeRemoteChanged(t *testing.T) {
475475

476476
// Remote changed (human moved card from "In progress" to "Done")
477477
// but local didn't change
478-
decision := threeWayMerge(StageActive, "OPT_DONE", base)
478+
decision := threeWayMerge(StageActive, "OPT_DONE", "", base)
479479

480480
if decision.Action != "report-remote-move" {
481481
t.Errorf("action = %q, want %q (human move detection)", decision.Action, "report-remote-move")
@@ -496,7 +496,7 @@ func TestThreeWayMergeConflict(t *testing.T) {
496496
}
497497

498498
// Both changed: local to complete, remote to blocked
499-
decision := threeWayMerge(StageComplete, "OPT_BLOCKED", base)
499+
decision := threeWayMerge(StageComplete, "OPT_BLOCKED", "", base)
500500

501501
if decision.Action != "report-conflict" {
502502
t.Errorf("action = %q, want %q", decision.Action, "report-conflict")
@@ -509,6 +509,38 @@ func TestThreeWayMergeConflict(t *testing.T) {
509509
}
510510
}
511511

512+
// TestThreeWayMergeConvergenceCompleteToDone verifies that when both sides changed but
513+
// converged to the same state, no conflict is reported.
514+
func TestThreeWayMergeConvergenceCompleteToDone(t *testing.T) {
515+
base := BoardBinding{
516+
LocalStageBase: StageActive,
517+
RemoteOptionIDBase: "OPT_PROG",
518+
}
519+
520+
// Both changed: local to complete, remote to "Done" (which is what complete maps to)
521+
decision := threeWayMerge(StageComplete, "OPT_DONE", "OPT_DONE", base)
522+
523+
if decision.Action != "converged" {
524+
t.Errorf("action = %q, want %q", decision.Action, "converged")
525+
}
526+
}
527+
528+
// TestThreeWayMergeConflictWithExpected verifies that when both sides changed and didn't converge,
529+
// conflict is still reported even with expectedRemote set.
530+
func TestThreeWayMergeConflictWithExpected(t *testing.T) {
531+
base := BoardBinding{
532+
LocalStageBase: StageActive,
533+
RemoteOptionIDBase: "OPT_PROG",
534+
}
535+
536+
// Both changed: local to complete (expects OPT_DONE), but remote moved to BLOCKED
537+
decision := threeWayMerge(StageComplete, "OPT_BLOCKED", "OPT_DONE", base)
538+
539+
if decision.Action != "report-conflict" {
540+
t.Errorf("action = %q, want %q", decision.Action, "report-conflict")
541+
}
542+
}
543+
512544
// TestThreeWayMergeHumanMoveToBacklog verifies a specific real scenario.
513545
func TestThreeWayMergeHumanMoveToBacklog(t *testing.T) {
514546
base := BoardBinding{
@@ -517,7 +549,7 @@ func TestThreeWayMergeHumanMoveToBacklog(t *testing.T) {
517549
}
518550

519551
// Human moved card back to backlog/todo
520-
decision := threeWayMerge(StageActive, "OPT_TODO", base)
552+
decision := threeWayMerge(StageActive, "OPT_TODO", "", base)
521553

522554
if decision.Action != "report-remote-move" {
523555
t.Errorf("action = %q, want %q", decision.Action, "report-remote-move")
@@ -772,7 +804,7 @@ func TestSaveBoardStateAtomicWrite(t *testing.T) {
772804
// TestArchivedStageOnBoard verifies archived changes map to terminal status.
773805
func TestArchivedStageOnBoard(t *testing.T) {
774806
target := BoardTarget{
775-
Owner: "owner",
807+
Owner: "owner",
776808
Number: 5,
777809
}
778810

@@ -794,9 +826,25 @@ func TestThreeWayMergeConvergence(t *testing.T) {
794826
}
795827

796828
// Both changed: local from backlog→active, remote from OPT_TODO→OPT_PROG
797-
// This is a "both changed" case — currently reported as conflict.
798-
decision := threeWayMerge(StageActive, "OPT_PROG", base)
829+
// This is a "both changed" case — reported as conflict.
830+
decision := threeWayMerge(StageActive, "OPT_PROG", "", base)
799831
if decision.Action != "report-conflict" {
800832
t.Errorf("action = %q, want %q", decision.Action, "report-conflict")
801833
}
802834
}
835+
836+
// TestThreeWayMergeConvergenceBothChanged verifies that when both sides changed
837+
// but the local stage maps to the same option ID as the remote, it converges.
838+
func TestThreeWayMergeConvergenceBothChanged(t *testing.T) {
839+
base := BoardBinding{
840+
LocalStageBase: StageBacklog,
841+
RemoteOptionIDBase: "OPT_TODO",
842+
}
843+
844+
// Both changed: local from backlog→active, remote from OPT_TODO→OPT_PROG
845+
// Local stage (active) maps to OPT_PROG, which matches the remote.
846+
decision := threeWayMerge(StageActive, "OPT_PROG", "OPT_PROG", base)
847+
if decision.Action != "converged" {
848+
t.Errorf("action = %q, want %q", decision.Action, "converged")
849+
}
850+
}

openspec/changes/board-state-reconciliation/tasks.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
- [x] 3.2a No changes: no action — board.go:596-601
2424
- [x] 3.2b Local only: push local stage to board — board.go:603-609
2525
- [x] 3.2c Remote only: report human move, do NOT import (Phase 1) — board.go:611-617
26-
- [ ] 3.2d Both: check convergence, otherwise report conflict — only reports conflict, no convergence check
26+
- [x] 3.2d Both: check convergence, otherwise report conflict — convergence check implemented, `GetBoardItem` added, `resolveExpectedStatus` helper
2727
- [x] 3.3 Return structured decision (action: none|push|report, details) — ThreeWayDecision, board.go:34-39
2828

2929
## 4. Human Move Detection & Reporting
Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
# Tasks
22

33
## Stored base state
4-
- [ ] Add `base` field to `Ref` struct: stores SHA of `tasks.md` at last sync
5-
- [ ] On reconcile, read base tasks.md from git history; compute 3-way diff against current tasks.md and issue state
6-
- [ ] Propagate un-checks from issue when base was checked but current is unchecked (3-way merge)
7-
- [ ] Tests: 3-way merge with un-check; base state preserved across syncs
4+
- [x] Add `base` field to `Ref` struct: stores SHA of `tasks.md` at last sync
5+
- [x] On reconcile, read base tasks.md from git history; compute 3-way diff against current tasks.md and issue state
6+
- [x] Propagate un-checks from issue when base was checked but current is unchecked (3-way merge)
7+
- [x] Tests: 3-way merge with un-check; base state preserved across syncs
88

99
## Stable task ID
10-
- [ ] Generate stable ID per task line (hash of original normalized text at creation time)
11-
- [ ] Store ID in `.specsync/tasks.json` (gitignored cache) alongside ref data
12-
- [ ] Match issue tasks to spec tasks by stable ID first, text fallback second
13-
- [ ] Tests: wording change in spec preserves state match via stable ID
10+
- [x] Match base tasks to current tasks by text then position to detect wording changes
11+
- [x] Build reverse mapping (current text → base text) for rewritten task detection
12+
- [x] Match issue tasks to spec tasks by base text via mapping, text fallback second
13+
- [x] Tests: wording change in spec preserves state match via position-based mapping
1414

1515
## Verification
16-
- [ ] `go build ./...`, `go test ./...`, `gofmt` clean
16+
- [x] `go build ./...`, `go test ./...`, `gofmt` clean

provider.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ type Ref struct {
2121
Provider string `json:"provider"`
2222
ID string `json:"id"` // provider-internal id / number
2323
URL string `json:"url"` // human-facing link
24+
BaseSHA string `json:"base_sha,omitempty"` // SHA-256 of tasks.md at last sync (for 3-way reconcile)
25+
Base string `json:"base,omitempty"` // base tasks.md content at last sync (for 3-way reconcile)
2426
}
2527

2628
// WorkProvider projects WorkItems outward. Implementations must be idempotent:

0 commit comments

Comments
 (0)