Skip to content

fix(git): never rewrite the refspec of a reused user-created fork remote - #25708

Open
ggbdpq wants to merge 1 commit into
stablyai:mainfrom
ggbdpq:fix/fork-remote-refspec-user-remote
Open

ggbdpq wants to merge 1 commit into
stablyai:mainfrom
ggbdpq:fix/fork-remote-refspec-user-remote

Conversation

@ggbdpq

@ggbdpq ggbdpq commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

ELI5

When a worktree's PR lives on a fork, Orca borrows whatever existing remote points at that fork — including one you added by hand — and rewrites its fetch configuration down to just the PR's branch. Every other branch you push to that fork then loses its upstream: @{u} fails, ahead/behind counts vanish, plain git fetch stops updating them, and Orca can no longer find those branches' PRs. Now Orca only rewrites remotes it created itself; for yours, it quietly adds the one line it needs and leaves everything else exactly as you configured it.

What Changed

  • prepareWorktreePushTargetWithExec (reuse path) and adoptExistingForkRemoteForBranch (deferred materialization path) now check the existing remote.<name>.orca-created marker before touching a remote's refspec:
    • marker set (Orca minted it) → ensureRemoteTracksBranchNarrowly, unchanged behavior;
    • no marker (user-created) → new ensureRemoteAlsoTracksBranch, which appends this branch's narrow refspec only if absent — nothing dropped, nothing rewritten, tagOpt untouched.
  • remoteHasOrcaProvenance is exported from the cleanup module and reused rather than reimplemented.
  • The migration sweep for pre-marker pr-* remotes is intentionally untouched (those predate the marker and rely on the sweep to get narrowed).
  • Tests: three unit cases for the add-only helper (wide default preserved, idempotent, no tagOpt), config-aware setup cases (user remote keeps its wide refspec plus the appended line — red before; the issue's workaround re-adding the wide refspec now survives — red before; minted remote still narrows — guard), and an adoption-path case asserting no --unset-all ever runs against a user remote.

Why

The narrowing exists to keep Orca-minted pr-* remotes single-branch (#17887 already stamps an ownership marker for exactly this distinction) — the reuse path just never consulted it. Deleting user refspecs was never necessary for Orca's own goal: the per-branch line only needs to exist, and --add achieves that without destroying anything. That also fixes the issue's workaround trap, where re-adding the wide refspec only lasted until the next fork-PR worktree on another branch stripped it again. Provenance-gating stays at the call sites so the refspec module remains plumbing with no knowledge of who created what.

Linked Issue

Fixes #25703

Visual Proof

N/A — no UI change; the before/after is in git config --get-all remote.<name>.fetch output, pinned by the tests described above.

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

Commands actually run locally (macOS): the three new red-before cases failed for the bug itself (user remote's wide refspec and other branch lines deleted; --unset-all called against a user remote); after the fix 61/61 across the three touched suites and 248/248 across the 24-file push-target/refspec/materialization/migration/cleanup/relay matrix; pnpm tc reports only the pre-existing structured-agent-session-stop-note-opened-turn.test.ts TS2353 failure that also reproduces on clean upstream/main (see the notes on #24689/#25245); oxlint/oxfmt --check clean on all seven touched files. The exact Google Drive / two-machine scenario from the report was not reproduced locally; the git-level behavior is pinned at the config-mutation boundary.

AI Disclosure

Investigation scoping and coding by GLM-5.3-Flash (ZCode agent); orchestrated, reviewed and committed by a main agent with the same model, human overseeing.

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

Git commands used are baseline-safe (config --get/--add). Provider-neutral: the marker check gates on Orca provenance, not on GitHub-specific state. Known scope edges, stated honestly: a pre-marker Orca pr-* remote reused on the adoption path now gets add-only treatment (its wide refspec cleanup waits for the migration sweep), and a user remote manually named pr-* would still be narrowed by the sweep's prefix heuristic — both pre-existing behaviors this PR deliberately does not change.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

When a worktree's PR came from a fork, Orca reused any existing remote
whose URL matched and ran the same narrowing it uses for remotes it
minted itself: the wide `+refs/heads/*:refs/remotes/<name>/*` was
deleted, `tagOpt` forced to --no-tags, and only the PR branch's narrow
refspec left behind. For a remote the user added by hand that broke
tracking for every other branch they push (`@{u}`, ahead/behind, plain
fetch) and, through the missing tracking ref, Orca's own PR detection.

The reuse and adoption paths now check the `remote.<name>.orca-created`
marker: remotes Orca minted keep the narrowing behavior unchanged, and
remotes it did not create get the branch's narrow refspec appended
add-only — nothing dropped, nothing rewritten, tagOpt untouched, and
the wide refspec survives re-runs. The migration sweep for pre-marker
`pr-*` remotes is intentionally untouched.

Fixes stablyai#25703

Generated-by: GLM-5.3-Flash (ZCode)
@coderabbitai

coderabbitai Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d5d2269b-c1f1-477e-a3c6-5b593b07efbc
📥 Commits

Reviewing files that changed from the base of the PR and between 9def4b9 and 8d9d25e.

📒 Files selected for processing (7)
  • src/main/git/fork-remote-refspec.test.ts
  • src/main/git/fork-remote-refspec.ts
  • src/main/ipc/worktree-push-target-cleanup.ts
  • src/main/ipc/worktree-push-target-setup.test.ts
  • src/main/ipc/worktree-push-target-setup.ts
  • src/main/ipc/worktree-remote-push-target-materialization.test.ts
  • src/main/ipc/worktree-remote.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

Reused fork remotes now use Orca provenance to select their refspec update path. Orca-created remotes retain narrow branch tracking. Other remotes receive the requested branch refspec without replacing existing refspecs or changing tagOpt. Tests cover helper behavior, repeated reuse, and both provenance paths.

Priority: ➖ Normal

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 8d9d2

The changed reuse paths preserve user-managed remote configuration while adding branch tracking. No issue identified here needs resolution before merge.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 8d9d2

The change protects user-managed remotes from destructive configuration updates. One legacy compatibility risk remains: older application-created remotes can retain fork-wide tracking, allowing later fetches to import more branches than intended into the shared repository.

Retained concerns

  • Low · reliability · inferred: Pre-marker application-created remotes can retain fork-wide fetch tracking after reuse. The new add-only path appends the desired branch refspec without removing the wide default. Deferred adoption skips migration, and a later sweep cannot remove the wide entry when all selected branch refspecs already exist because narrowing places removal inside the missing-entry condition. Subsequent plain fetches can therefore re-import a fork-controlled branch set into the repository shared by its worktrees, weakening the intended resource and ref-import containment.
Security review details

Security Blast Radius

  • inferred — The identified regression affects fetch configuration and remote-tracking refs within a shared Git repository and its worktrees. A fork owner controls the upstream branch set, but exploitation of excessive imports requires selection of that fork and a later fetch that uses the retained wide configuration; immediate materialization remains explicitly branch-scoped.

Trust Boundaries and Controls

  • observed — Cleanup still requires ownership evidence, excludes origin and upstream, checks other worktree and branch users, and verifies URL identity before removing a remote. These controls are unchanged by the PR; unmarked user remotes are not newly claimed by the add-only helper.

Resilience and Maintainability Implications

  • observed — Fetch failure can leave an appended refspec on a reused remote, but the base also retained reused-remote mutations after failure. The head makes this mutation non-destructive to existing user configuration, so this residual-state behavior is not retained as a separate introduced concern.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning [ #25703 ] The reuse and adoption paths now check remote.<name>.orca-created. They narrow marked remotes and add the branch refspec without replacing configuration on unmarked remotes. The reported … Update the migration sweep to distinguish legacy Orca remotes from user remotes, and preserve the refspecs of unmarked user remotes. Add a regression test for an unmarked user remote named pr-*.
Docstring Coverage ⚠️ Warning Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 7 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely states the main change: preserving refspecs on reused user-created fork remotes.
Description check ✅ Passed The description covers the required sections, explains the problem and solution, links issue #25703, gives testing details, and marks the visual proof as not applicable. It also discloses a pre-existi…
Out of Scope Changes check ✅ Passed The added helper, provenance export, reuse and adoption changes, and regression tests all support [#25703]. The migration sweep is unchanged, so it is not an unrelated change introduced by this PR.
Full details: Linked Issues check

Explanation

[ #25703 ] The reuse and adoption paths now check remote.&lt;name&gt;.orca-created. They narrow marked remotes and add the branch refspec without replacing configuration on unmarked remotes. The reported tests cover refspec preservation, tagOpt, idempotence, and both paths. However, the PR notes that the unchanged migration sweep can still narrow an unmarked user remote named pr-*. This conflicts with the issue requirement to narrow only remotes Orca created.

  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
🧰 Additional context used
📚 Code guidelines (1)
AGENTS.md — auto-discovered

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

Reviewed the complete diff at 8d9d25e (7 files, single commit). The reuse paths that previously narrowed the fetch refspec of any URL-matching remote now gate that rewrite on Orca provenance.

  • ensureRemoteAlsoTracksBranch — new add-only twin in fork-remote-refspec.ts; appends the branch's narrow refspec only when absent, leaving the user's wide default, other branches' entries, and tagOpt intact.
  • prepareWorktreePushTargetWithExec reuse path — reads remote.<name>.orca-created and dispatches to the narrow rewrite for Orca-minted remotes, add-only for user-created ones.
  • adoptExistingForkRemoteForBranch — the same provenance gate on the deferred materialize path.
  • remoteHasOrcaProvenance — exported from the cleanup module and reused rather than reimplemented.
  • Tests — unit cases for the add-only helper, red-before reuse cases (wide default preserved, the issue's workaround survives a second branch, a minted remote still narrows), and an adoption-path case asserting no --unset-all/tagOpt against a user remote.

I traced every caller of ensureRemoteTracksBranchNarrowly; the only remaining non-marker-gated rewrite of a reused remote is the refspec migration sweep, which the PR deliberately leaves untouched and documents. The added logic is correct and the behavior matches the issue's stated expectation (only add its per-branch line without removing the user's existing refspecs). Verified locally: 61/61 across the three touched suites, oxlint clean on all seven files.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Fork-PR push-target setup rewrites the fetch refspec of a user-created remote, breaking tracking and PR detection for its other branches

1 participant