Design discussion: Fork PR support

[kirisame-wang]

Background

PR #937 reverted fork PR support (#851) and unique branch naming (#931) to fix a P1 bug where commits were pushed to wrong branches. This blocks major OSS adoption (e.g., PyTorch where 90%+ PRs are from forks).

Opening this discussion to align on design approach before implementation.

Related: #851, #930, #931, #936, #937

Problem

Three conflicting requirements:

1. Local branch name must match PR's head ref for git push to work (as raised in Claude fails to make commits to PR branch #936)
2. Unique local naming (pr-{number}) needed to avoid conflicts (the concern in PR checkout fails due to branch name collision after #851 #930)
3. Fork PRs require fetching from pull/{number}/head refs (the issue noted in Fix PR checkout to support fork PRs #851)

Design Decisions Needed

1. Environment Setup Philosophy

Should claude-code-action remain self-contained or delegate to user setup?

Option A: Self-Contained

• Action handles all git setup automatically
• Pros: Zero config, consistent behavior, better UX
• Cons: More complex implementation

Option B: Delegated Setup

• Users run actions/checkout
• Pros: Simpler action code
• Cons: Higher user burden, inconsistent setups

---

2. Fork PR Support Implementation

Assuming self-contained approach, propose the following solution:

Proposed Solution

Explicit Push Target + Conditional Handling

// Key changes in setupBranch()
if (isForkPR) {
  const localBranch = `pr-${entityNumber}`;
  execGit(["fetch", "origin", `pull/${entityNumber}/head:${localBranch}`]);
  execGit(["checkout", localBranch, "--"]);
  return { currentBranch: localBranch, originalBranchRef: headRefName };
} else {
  // Same-repo: keep current behavior
  execGit(["fetch", "origin", `--depth=${fetchDepth}`, headRefName]);
  execGit(["checkout", headRefName, "--"]);
  return { currentBranch: headRefName };
}

// In push logic
const target = originalBranchRef || currentBranch;
execGit(["push", "origin", `HEAD:${target}`]);

Changes needed:

1. Add originalBranchRef?: string to BranchInfo type
2. Implement fork detection: headRepository.id !== baseRepository.id
3. Update push commands to use explicit target
4. Add GraphQL fields for repository IDs

Why it works:

• Fork PRs: unique local name (pr-123) + explicit push target
• Same-repo PRs: no change (keeps current behavior)
• Avoids branch conflicts while fixing push destination

Discussion Points

Feedback needed on these key decisions:

1. Environment setup: Should we keep the self-contained approach or allow delegated setup?
2. Fork detection: Is headRepository.id !== baseRepository.id sufficient, or do we need additional checks?
3. Fork support enablement: Should fork PR support be opt-out (default enabled) or opt-in?

[mojoX911]

+1, we are facing the same issue in our project. It seems like the right code has been reverted to the old problematic logic.

[awittzb]

An additional "con" of "self-contained" is

• makes it difficult for users to compose workflows that incorporate claude-code-action

... as you have an opinionated checkout & semantics that assume ownership of the git repo state, trying to be made to "play nice" with other tasks.

One example...

If you want to use `claude-code-action` as *part of* a workflow - say, not just as "claude review this" but

flowchart TD
O["Checkout"] --> A
A["CI Check 1"] --> B["Generate Resources"]
B --> C["Claude: reason on generated resources"]
C --> D["publish of some sort"]

Loading

to offer one example - a "self-contained" checkout/branch strategy dramatically complicates things.

A common pattern for addressing this in GitHub Actions is to have a skip_checkout: <bool> or similar input. When false, the action assumes it's all you're doing, and automagics the git state for you. When true, the action assumes it's part of something larger, and that the correct files & repository state are already set-up on-disk for it.