Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 74 additions & 34 deletions .claude/skills/upstream-sync.md
Original file line number Diff line number Diff line change
@@ -1,24 +1,43 @@
# Upstream Sync Skill

Sync fork with one or more upstream remotes via cherry-pick + merge-base advance.
Sync fork with one or more upstream remotes via cherry-pick + marker refs.

## When to use

User says: "sync with upstream", "cherry-pick from X", "merge upstream", or similar.

## How we track "already reviewed"

A ref per upstream, `sync-marker/<remote>`, points at the last upstream commit we
triaged. Divergence is `sync-marker/<remote>..<remote>/master` — nothing is inferred
from history shape.

**Never merge an upstream ref into `master`.** Doing so makes every upstream commit an
ancestor of `master`, and semantic-release analyses `<lastTag>..master` by walking all
parents — so skipped upstream commits land in the version calculation and the changelog.
One stray `feat:` upstream turns a patch into a minor and credits us with code we never
shipped. Marker refs are not ancestors of `master`, so releases never see them.

That also means the PR merge strategy is irrelevant: squash, rebase, and merge are all
safe. (Before markers, tracking lived in merge commits and squashing silently broke it.)

## Workflow

### Phase 1: Explore divergence

```bash
git fetch <remote>
git log --oneline --right-only <branch>...<remote>/master --no-merges
git log --oneline sync-marker/<remote>..<remote>/master --no-merges
```

If the marker does not exist yet, bootstrap it from the last real merge of that remote
(`git log --merges --oneline master | grep <remote>`) or ask the user, then create it:
`git branch sync-marker/<remote> <that-upstream-commit>`.

For each upstream-only commit, get files changed:

```bash
git log --right-only <branch>...<remote>/master --no-merges --format="%h %s" | while read hash msg; do
git log sync-marker/<remote>..<remote>/master --no-merges --format="%h %s" | while read hash msg; do
echo "=== $hash $msg ==="; git diff-tree --no-commit-id --name-only -r $hash; echo
done
```
Expand All @@ -38,7 +57,7 @@ done
| Refactoring | CHERRY-PICK (evaluate risk) |
| Docs/CI/tools | Ask user |

Check for equivalents: `git log --oneline --left-only <branch>...<remote>/master | grep -i "<keyword>"`
Check for equivalents: `git log --oneline <remote>/master..<branch> | grep -i "<keyword>"`

### Phase 3: Ask user

Expand All @@ -55,31 +74,7 @@ Order: TS fixes → Android fixes → iOS fixes → small features → large fea
If conflict: resolve, `git add`, `git cherry-pick --continue --no-edit`.
If empty after resolution: `git cherry-pick --skip`.

### Phase 5: Merge to advance merge-base

Without this, future merges replay ALL upstream commits including skipped ones.

```bash
git merge <remote>/master --no-commit

# Conflicted files — keep ours
git diff --name-only --diff-filter=U | xargs git checkout --ours
# Files deleted in our branch — remove
git rm <deleted-files>
# Auto-merged files — reset to our version
git diff --cached --name-only --diff-filter=M | xargs git checkout HEAD --
# Unwanted new files from upstream — remove
git diff --cached --name-only --diff-filter=A # review, then:
git rm -f <unwanted-files>

git add -A
git diff --cached --stat HEAD # should be empty or near-empty
git commit -m "merge: sync merge-base with <remote>/master"
```

Verify: `git log --oneline --right-only <branch>...<remote>/master | wc -l` should be `0`.

### Phase 6: Verify
### Phase 5: Verify

Run ALL of these. Do not skip any.

Expand All @@ -91,23 +86,64 @@ cd examples/GumTestApp/ios && pod install && \
-sdk iphonesimulator -configuration Debug build
```

### Phase 7: Format native files
Run these one at a time, never two concurrently against `examples/GumTestApp` — a second
run's `rm -rf Pods` will pull files out from under the first and both fail confusingly.

Use `set -o pipefail`, or check the build's own exit code. Piping `xcodebuild` into `tail`
reports *tail's* status, so a failed build looks like exit 0. Confirm by grepping for
`** BUILD SUCCEEDED **`.

If `pod install` cannot satisfy the pinned `StreamWebRTC` version, the local CocoaPods
spec repo is stale — `pod install --repo-update`. Not a sync problem; CI checks out fresh.

### Phase 6: Format native files

```bash
git ls-files | grep -e "\(\.java\|\.h\|\.m\)$" | grep -v examples | xargs npx clang-format -i
```

This reformats pre-existing drift across many untouched files, because the local
clang-format version differs from whatever produced the committed formatting. Keep only
the files this sync actually touched and revert the rest — formatting is not a CI gate,
and a sync PR full of unrelated reformatting is unreviewable.

Rebuild Android + iOS to confirm, then commit.

### Phase 8: Update package-lock.json
### Phase 7: Update package-lock.json

If `package.json` dependencies changed, lock file will be stale.
If `package.json` dependencies changed, lock file will be stale. Note a `scripts`-only
change does **not** require this.

```bash
npm install
git add package-lock.json && git commit -m "chore: update package-lock.json"
```

### Phase 8: After the sync PR merges, move the markers

Do this **only once the PR has landed on `master`** — not at cherry-pick time. Moving a
marker for a PR that is later abandoned skips those upstream commits permanently.

Any merge strategy is fine (squash, rebase, or merge).

```bash
# for each remote synced in this round
git fetch <remote>
git branch -f sync-marker/<remote> <the upstream commit triaged in Phase 1>
git push --force-with-lease origin sync-marker/<remote>
```

Point the marker at the upstream tip you actually triaged, not a freshly fetched one —
upstream may have moved on while the PR was in review, and those newer commits have not
been reviewed. Then verify the divergence is empty:

```bash
git log --oneline sync-marker/<remote>..<remote>/master --no-merges # only untriaged commits
```

Marker pushes trigger no CI: every workflow is scoped to `master` pushes, PRs targeting
`master`, or manual dispatch.

## Preservation rules

These MUST NOT change during sync:
Expand All @@ -134,12 +170,16 @@ Post-sync: `grep -r "org.webrtc:google-webrtc\|webrtc-ios" --include="*.gradle"

5. **Watch for duplicates after conflict resolution.** Duplicate variable declarations, closing braces, or imports when keeping both sides of a conflict.

6. **Advance merge-base for EVERY upstream remote.** If syncing with multiple upstreams, merge each one separately. Otherwise the un-advanced remote replays all its history on the next merge.
6. **Move the marker for EVERY upstream remote.** If syncing with multiple upstreams, each needs its own `sync-marker/<remote>`. A remote whose marker was not moved replays all its history next round.

7. **Upstream podspec/build files leak into cherry-picks.** Other forks have their own podspec (e.g., `livekit-react-native-webrtc.podspec`). Always `git rm` them when they appear.

8. **Cross-check cherry-picks against all upstreams for reverts.** Before cherry-picking a commit from one upstream, search the other upstreams for the same change — it may have been tried and reverted. Run: `git log --all --oneline -S "<key code snippet>"` to find if the same change exists elsewhere in history with a subsequent revert.

9. **Verify cherry-picked changes still exist on upstream HEAD.** A commit could have been added and later reverted/modified by a subsequent commit on the same upstream. After cherry-picking, verify the actual code still matches upstream's current state: `git show <remote>/master:<file> | grep "<key code>"`. Don't just trust that a commit was made — it may have been undone.

10. **Squash-merging the sync PR destroys the merge-base advancement.** The merge commits from Phase 5 (`git merge <remote>/master`) are the only thing that tells git "we've seen up to this point." Squash-merge flattens them into a single commit, losing that information. When merging the sync PR, use **"Create a merge commit"** (regular merge), not squash. If already squashed, create a follow-up PR with ghost merges: `git merge -s ours <remote>/master` for each upstream, then merge that PR with a regular merge commit.
10. **Never merge an upstream ref into `master` to record progress.** This includes ghost merges (`git merge -s ours <remote>/master`), which keep the tree but still add the upstream parent. semantic-release analyses `<lastTag>..master` by walking all parents, so every skipped upstream commit enters the version calculation and the changelog — one upstream `feat:` turns a patch into a minor and credits us with code we never shipped. Move `sync-marker/<remote>` instead (Phase 8); it is not an ancestor of `master`, so releases never see it. Historical merges like `eface9a` / `d66ff60` predate this rule and are harmless only because later release tags absorbed them.

11. **Don't infer "already synced" from the tree.** A squashed sync PR copies upstream *content* into `master` without making upstream *commits* ancestors, so content checks and marker checks can disagree. `sync-marker/<remote>` is the only record of what was triaged; the tree tells you nothing about it.

12. **`grep` may be shadowed by a shell function or `ugrep`.** A wrapper can report zero matches for a string that is plainly present, which reads as "the cherry-pick did not apply." Confirm with `/usr/bin/grep` or `git show <ref>:<file> | sed -n 'N,Mp'` before concluding anything is missing.
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ Sanity check after any dependency edit: `grep -r "org.webrtc:google-webrtc\|webr

## Keeping the fork in sync with upstream

Pulling fixes/features from upstream `react-native-webrtc` (and sibling forks) is a recurring, error-prone task with strict preservation rules and merge-base mechanics. **Read `.claude/skills/upstream-sync.md` before doing any sync/cherry-pick/merge work** — it documents the triage table, the merge-base advancement step (and why squash-merging the sync PR breaks it), and the files that must never change during a sync.
Pulling fixes/features from upstream `react-native-webrtc` (and sibling forks) is a recurring, error-prone task with strict preservation rules. **Read `.claude/skills/upstream-sync.md` before doing any sync/cherry-pick/merge work** — it documents the triage table, the `sync-marker/<remote>` refs that record how far each upstream has been reviewed (and why upstream refs must never be merged into `master`), and the files that must never change during a sync.

## Releases

Expand Down
Loading