anthropics / anthropics/claude-code-action

Design discussion: Fork PR support

Aperta
#939 3 commenti 4 reazioni 0 assegnatari Vedi su GitHub
dev-experience feature-request p2
Lingua principale
TypeScript
Stelle
8.9k
Fork
2.1k
Merge medio
3g 9h
PR unite (30g)
10

Descrizione

## 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 #936)
2. Unique local naming (`pr-{number}`) needed to avoid conflicts (the concern in #930)
3. Fork PRs require fetching from `pull/{number}/head` refs (the issue noted in #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**

```typescript
// 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?

Guida per i contributori

Apri la guida per i contributori

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.