fix: detect existing PRs and always update stack comments on sync

[nvandessel]

Summary

• frond sync now always updates stack comments, not only when merges are detected. This ensures PRs tracked after creation (e.g., via frond track) get their stack comments posted on the next sync.
• frond track detects existing PRs on GitHub for the tracked branch and stores the PR number in state, so sync and push know about them immediately.
• frond push adopts pre-existing PRs instead of trying to create a duplicate. If a PR already exists for the branch on GitHub, it is adopted and its number stored in state.
• Adds gh.PRForBranch helper function, fakegh pr list support, and tests for all three fixes.

Test plan

• TestSyncUpdatesCommentsWithoutMerges — verifies stack comment API calls happen during sync even with no merges
• TestTrackDetectsExistingPR — verifies frond track stores the PR number when a PR exists
• TestTrackNoPRStoresNil — verifies frond track stores nil when no PR exists
• TestPushAdoptsExistingPR — verifies frond push adopts an existing PR instead of creating a new one
• All existing tests continue to pass

🤖 Generated with Claude Code

[greptile-apps]

Greptile Summary

This PR fixes three related gaps in how frond handles branches whose PRs were created outside of frond's knowledge: frond track now detects and stores pre-existing GitHub PRs, frond push adopts a pre-existing PR rather than attempting a duplicate creation, and frond sync always updates stack comments (not only when merges are detected). All three fixes are backed by new integration tests using the fakegh test double.

Key observations:

• The core logic in push.go, track.go, and sync.go is correct and well-structured; the adoption path in push.go correctly falls through to the existing PR-retargeting code after storing the adopted PR number.
• pushResult (JSON output) is not updated to include an adopted field, so --json consumers cannot distinguish between an adopted PR and a routine push update — the human-readable output handles this but the structured output does not.
• PRForBranch in gh.go contains a redundant strings.TrimSpace call since run() already trims output.
• FAKEGH_PR_FOR_BRANCH only supports a single branch:number pair, which may limit future tests needing multiple pre-existing PRs simultaneously.
• Moving updateStackComments outside the len(mergedBranches) > 0 guard means extra GitHub API calls on every sync, including syncs that result in "already up to date". This is the intended trade-off for the fix.

Confidence Score: 4/5

• Safe to merge; the fixes are well-scoped and covered by new tests with no critical logic errors.
• All three behavioral fixes are logically correct and accompanied by integration tests. The only gaps are a missing adopted field in the JSON output struct and a minor redundant trim — neither affects runtime correctness.
• cmd/results.go — pushResult is missing an adopted field for complete JSON API parity with human-readable output.

Important Files Changed

Filename	Overview
cmd/push.go	Adds pre-push PR adoption: checks GitHub for an existing open PR when none is in state, stores it, and continues with normal retargeting logic. State is written correctly on adoption. The adopted flag drives human-readable output but is not reflected in the JSON result struct.
cmd/track.go	Adds optional PR detection on frond track via gh.PRForBranch; correctly guards with gh.Available() since track doesn't require gh. PR number stored in state and surfaced in both human-readable and JSON output.
cmd/sync.go	Moves updateStackComments out of the merge-only block so it runs unconditionally on every sync. The call correctly occurs after merged-PR comment updates and before the "already up to date" early-return check, ensuring comment API calls fire even with no merges or rebases.
internal/gh/gh.go	New PRForBranch helper wraps gh pr list --head <branch> --state open --json number --limit 1. Logic is correct; minor redundant strings.TrimSpace call since run() already trims output.
internal/gh/testdata/fakegh/main.go	Adds pr list handler controlled by FAKEGH_PR_FOR_BRANCH=branch:number. Functional for current tests but limited to a single branch:number pair, which could be a constraint for future multi-branch adoption tests.
cmd/results.go	Adds PR *int field to trackResult for JSON output. pushResult still lacks an adopted field, creating an inconsistency between human-readable and JSON output for the adopted-PR case.
cmd/cmd_test.go	Four new integration tests covering all three fixes plus the nil-PR case. Tests correctly use FAKEGH_PR_FOR_BRANCH, verify state after commands, and check that comment API calls are made during sync without merges.

Sequence Diagram

sequenceDiagram
    participant User
    participant frond
    participant State
    participant gh as gh CLI / GitHub

    Note over User,gh: frond track <branch>
    User->>frond: track <branch> --on <parent>
    frond->>gh: gh pr list --head <branch> --state open
    gh-->>frond: PR #N (or empty)
    frond->>State: write {branch, parent, PR: #N or nil}
    frond-->>User: "Tracking branch..." [+ "Found existing PR #N"]

    Note over User,gh: frond push (no PR in state)
    User->>frond: push
    frond->>gh: git push origin <branch>
    frond->>gh: gh pr list --head <branch> --state open
    alt PR already exists on GitHub
        gh-->>frond: PR #N
        frond->>State: write {PR: #N} (adopted)
        frond->>gh: gh pr view #N (check base)
        frond-->>User: "Pushed <branch>. PR #N [adopted]"
    else No existing PR
        gh-->>frond: []
        frond->>gh: gh pr create --base <parent> --head <branch>
        gh-->>frond: PR #M
        frond->>State: write {PR: #M}
        frond-->>User: "Pushed <branch>. PR #M [created]"
    end
    frond->>gh: updateStackComments (API calls)

    Note over User,gh: frond sync
    User->>frond: sync
    frond->>gh: git fetch
    frond->>gh: gh pr view (check merged state per branch)
    opt Merges detected
        frond->>State: reparent children, delete merged branches
        frond->>gh: updateMergedComments
    end
    frond->>gh: updateStackComments (always)
    frond->>gh: git rebase (topological order)
    frond-->>User: sync summary

Loading

Comments Outside Diff (3)

1. cmd/results.go, line 24-28 (link)

   pushResult missing adopted field

   When a PR is adopted, --json output still emits "created": false, which is identical to the "updated" case. Scripts consuming the JSON output have no way to distinguish between an adopted PR (previously unknown to frond) and a routine push update. The human-readable path already distinguishes the three states (created, adopted, updated), but the JSON API does not.

   Then in push.go the JSON output should be updated to include Adopted: adopted.

2. internal/gh/testdata/fakegh/main.go, line 133-153 (link)

   FAKEGH_PR_FOR_BRANCH only supports a single branch:number pair

   The current format "branch:number" can only simulate one branch having a pre-existing PR at a time. Any future test that needs two tracked branches to both have existing PRs (e.g., testing stack adoption) would need to set the env var twice or use a different mechanism. Consider a comma-delimited format like "branch1:55,branch2:77" to future-proof this:

   prForBranch := os.Getenv("FAKEGH_PR_FOR_BRANCH")
   if prForBranch != "" {
       var headBranch string
       for i := 2; i < len(args); i++ {
           if args[i] == "--head" && i+1 < len(args) {
               headBranch = args[i+1]
               break
           }
       }
       for _, entry := range strings.Split(prForBranch, ",") {
           parts := strings.SplitN(entry, ":", 2)
           if len(parts) == 2 && parts[0] == headBranch {
               fmt.Printf("[{\"number\": %s}]\n", parts[1])
               os.Exit(0)
           }
       }
   }
   fmt.Println("[]")

   _{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

3. internal/gh/gh.go, line 196-219 (link)

   Redundant strings.TrimSpace call

   The run() helper (line 54) already calls strings.TrimSpace on stdout before returning. The second TrimSpace on line 204 is a no-op.

   (Remove the out = strings.TrimSpace(out) line 204.)

_{Last reviewed commit: 5db7fab}