Skip to content

Commit a9a61a3

Browse files
julianknutsenclaude
julianknutsen
and
claude
committed
fix: filter stale fork diffs from competing submissions
Fork branches that predate upstream updates show spurious diffs for items they never touched. Two filter rules now skip stale entries: 1. status "open" — untouched item (e.g. registration-only PRs) 2. claimed_by set to someone other than the PR author — inherited claim from a previous upstream state, not the fork owner's action Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 0fdd932 commit a9a61a3

2 files changed

Lines changed: 138 additions & 24 deletions

File tree

‎internal/remote/dolthub.go‎

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -620,32 +620,38 @@ func (d *DoltHubProvider) ListPendingWantedIDs(upstreamOrg, db string) (map[stri
620620
}
621621
}
622622

623-
// Fetch upstream main state. Used as baseline for PRs from "main"
624-
// (where dolt_diff is a no-op) and to filter stale fork diffs where a
625-
// branch was created before upstream moved an item forward.
623+
// For PRs from "main" (commits directly on the fork's main), dolt_diff
624+
// between main and main is a no-op. Fall back to snapshot comparison
625+
// against upstream main for those PRs.
626626
type wantedItem struct {
627627
status string
628628
claimedBy string
629629
}
630-
upstreamItems := make(map[string]wantedItem)
631630
snapshotQuery := "SELECT id, status, COALESCE(claimed_by, '') as claimed_by FROM wanted"
632-
upstreamURL := fmt.Sprintf("%s/%s/%s/main?q=%s",
633-
dolthubAPIBase, upstreamOrg, db, url.QueryEscape(snapshotQuery))
634-
if body, err := d.dolthubGet(upstreamURL); err == nil {
635-
var qr queryResponse
636-
if json.Unmarshal(body, &qr) == nil {
637-
for _, row := range qr.Rows {
638-
upstreamItems[row["id"]] = wantedItem{
639-
status: row["status"],
640-
claimedBy: row["claimed_by"],
631+
var upstreamItems map[string]wantedItem
632+
needsUpstream := false
633+
for _, pr := range prs {
634+
if pr.fromBranch == "main" {
635+
needsUpstream = true
636+
break
637+
}
638+
}
639+
if needsUpstream {
640+
upstreamItems = make(map[string]wantedItem)
641+
upstreamURL := fmt.Sprintf("%s/%s/%s/main?q=%s",
642+
dolthubAPIBase, upstreamOrg, db, url.QueryEscape(snapshotQuery))
643+
if body, err := d.dolthubGet(upstreamURL); err == nil {
644+
var qr queryResponse
645+
if json.Unmarshal(body, &qr) == nil {
646+
for _, row := range qr.Rows {
647+
upstreamItems[row["id"]] = wantedItem{
648+
status: row["status"],
649+
claimedBy: row["claimed_by"],
650+
}
641651
}
642652
}
643653
}
644654
}
645-
// stateRank orders wanted lifecycle states; higher = further along.
646-
stateRank := map[string]int{
647-
"open": 0, "claimed": 1, "in_review": 2, "completed": 3, "done": 3,
648-
}
649655

650656
// Query each PR's fork branch in parallel using dolt_diff to find rows
651657
// the branch actually changed. The old approach compared the fork's full
@@ -655,6 +661,7 @@ func (d *DoltHubProvider) ListPendingWantedIDs(upstreamOrg, db string) (map[stri
655661
type pendingEntry struct {
656662
wantedID string
657663
state PendingWantedState
664+
author string // PR author, for filtering inherited claims
658665
}
659666
diffCh := make(chan []pendingEntry, len(prs))
660667
var wg sync.WaitGroup
@@ -709,6 +716,7 @@ func (d *DoltHubProvider) ListPendingWantedIDs(upstreamOrg, db string) (map[stri
709716
}
710717
entries = append(entries, pendingEntry{
711718
wantedID: id,
719+
author: pr.author,
712720
state: PendingWantedState{
713721
RigHandle: rigHandle,
714722
Status: forkStatus,
@@ -759,6 +767,7 @@ func (d *DoltHubProvider) ListPendingWantedIDs(upstreamOrg, db string) (map[stri
759767
}
760768
entries = append(entries, pendingEntry{
761769
wantedID: id,
770+
author: pr.author,
762771
state: PendingWantedState{
763772
RigHandle: rigHandle,
764773
Status: forkStatus,
@@ -779,13 +788,19 @@ func (d *DoltHubProvider) ListPendingWantedIDs(upstreamOrg, db string) (map[stri
779788
ids := make(map[string][]PendingWantedState)
780789
for entries := range diffCh {
781790
for _, e := range entries {
782-
// Filter stale diffs: if the fork's status for this item is behind
783-
// upstream main, the fork never intentionally progressed it — the
784-
// diff is just because the branch predates an upstream update.
785-
if up, ok := upstreamItems[e.wantedID]; ok {
786-
if stateRank[e.state.Status] < stateRank[up.status] {
787-
continue
788-
}
791+
// Skip stale fork state that doesn't represent intentional action.
792+
// A diff appears when a branch predates an upstream update — the
793+
// branch carries forward old state the fork owner never touched.
794+
//
795+
// Filter rules:
796+
// 1. status "open" = untouched item (stale copy)
797+
// 2. claimed_by set to someone other than the PR author =
798+
// inherited claim from a previous upstream state
799+
if e.state.Status == "open" {
800+
continue
801+
}
802+
if e.state.ClaimedBy != "" && e.state.ClaimedBy != e.author {
803+
continue
789804
}
790805
ids[e.wantedID] = append(ids[e.wantedID], e.state)
791806
}

‎internal/remote/dolthub_test.go‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,105 @@ func TestDoltHubProvider_ListPendingWantedIDs_CompletionQueryFails_GracefulDegra
697697
}
698698
}
699699

700+
func TestDoltHubProvider_ListPendingWantedIDs_StaleEntriesFiltered(t *testing.T) {
701+
// Stale fork state should be filtered:
702+
// 1. status "open" = untouched item
703+
// 2. claimed_by someone other than PR author = inherited claim
704+
// Only intentional actions (claimed_by == author) pass through.
705+
mux := http.NewServeMux()
706+
mux.HandleFunc("/org/db/pulls", func(w http.ResponseWriter, r *http.Request) {
707+
if strings.Contains(r.URL.Path, "/pulls/") {
708+
return
709+
}
710+
w.Header().Set("Content-Type", "application/json")
711+
_ = json.NewEncoder(w).Encode(map[string]any{
712+
"pulls": []map[string]any{
713+
{"pull_id": "1", "state": "open"},
714+
{"pull_id": "2", "state": "open"},
715+
{"pull_id": "3", "state": "open"},
716+
},
717+
})
718+
})
719+
mux.HandleFunc("/org/db/pulls/1", func(w http.ResponseWriter, _ *http.Request) {
720+
w.Header().Set("Content-Type", "application/json")
721+
_ = json.NewEncoder(w).Encode(map[string]any{
722+
"from_branch": "wl/register/stale-user",
723+
"from_branch_owner": "stale-fork",
724+
"author": "stale-user",
725+
})
726+
})
727+
mux.HandleFunc("/org/db/pulls/2", func(w http.ResponseWriter, _ *http.Request) {
728+
w.Header().Set("Content-Type", "application/json")
729+
_ = json.NewEncoder(w).Encode(map[string]any{
730+
"from_branch": "wl/bob/w-001",
731+
"from_branch_owner": "bob-fork",
732+
"author": "bob",
733+
})
734+
})
735+
mux.HandleFunc("/org/db/pulls/3", func(w http.ResponseWriter, _ *http.Request) {
736+
w.Header().Set("Content-Type", "application/json")
737+
_ = json.NewEncoder(w).Encode(map[string]any{
738+
"from_branch": "wl/register/charlie",
739+
"from_branch_owner": "charlie-fork",
740+
"author": "charlie",
741+
})
742+
})
743+
// stale-user: dolt_diff shows w-001 at "open" (stale untouched).
744+
mux.HandleFunc("/stale-fork/db/", func(w http.ResponseWriter, _ *http.Request) {
745+
w.Header().Set("Content-Type", "application/json")
746+
_ = json.NewEncoder(w).Encode(map[string]any{
747+
"rows": []map[string]string{
748+
{"id": "w-001", "status": "open", "claimed_by": "", "diff_type": "modified"},
749+
},
750+
})
751+
})
752+
// bob: dolt_diff shows w-001 at "claimed" by bob (intentional action).
753+
mux.HandleFunc("/bob-fork/db/", func(w http.ResponseWriter, _ *http.Request) {
754+
w.Header().Set("Content-Type", "application/json")
755+
_ = json.NewEncoder(w).Encode(map[string]any{
756+
"rows": []map[string]string{
757+
{"id": "w-001", "status": "claimed", "claimed_by": "bob", "diff_type": "modified"},
758+
},
759+
})
760+
})
761+
// charlie: dolt_diff shows w-001 at "claimed" by alice (inherited, not charlie's).
762+
mux.HandleFunc("/charlie-fork/db/", func(w http.ResponseWriter, _ *http.Request) {
763+
w.Header().Set("Content-Type", "application/json")
764+
_ = json.NewEncoder(w).Encode(map[string]any{
765+
"rows": []map[string]string{
766+
{"id": "w-001", "status": "claimed", "claimed_by": "alice", "diff_type": "modified"},
767+
},
768+
})
769+
})
770+
771+
server := httptest.NewServer(mux)
772+
defer server.Close()
773+
dolthubAPIBase = server.URL
774+
dolthubRepoBase = server.URL + "/repositories"
775+
776+
provider := NewDoltHubProvider("token")
777+
ids, err := provider.ListPendingWantedIDs("org", "db")
778+
if err != nil {
779+
t.Fatalf("ListPendingWantedIDs() error: %v", err)
780+
}
781+
// Only bob's "claimed" entry should appear:
782+
// - stale-user filtered (status "open")
783+
// - charlie filtered (claimed_by "alice" != author "charlie")
784+
if len(ids) != 1 {
785+
t.Fatalf("expected 1 pending ID (stale filtered), got %d: %v", len(ids), ids)
786+
}
787+
pending := ids["w-001"]
788+
if len(pending) != 1 {
789+
t.Fatalf("expected 1 entry for w-001, got %d", len(pending))
790+
}
791+
if pending[0].RigHandle != "bob" {
792+
t.Errorf("expected rig_handle=bob, got %s", pending[0].RigHandle)
793+
}
794+
if pending[0].Status != "claimed" {
795+
t.Errorf("expected status=claimed, got %s", pending[0].Status)
796+
}
797+
}
798+
700799
func TestDoltHubProvider_ListPendingWantedIDs_NoDiffs(t *testing.T) {
701800
mux := http.NewServeMux()
702801
mux.HandleFunc("/org/db/pulls", func(w http.ResponseWriter, r *http.Request) {

0 commit comments

Comments
 (0)