Skip to content

fix(git): recognize partial-clone remotes in git remote -v output - #18084

Open
TinBane wants to merge 1 commit into
pingdotgg:mainfrom
TinBane:fix/partial-clone-remotes-v2
Open

TinBane wants to merge 1 commit into
pingdotgg:mainfrom
TinBane:fix/partial-clone-remotes-v2

Conversation

@TinBane

@TinBane TinBane commented Oct 11, 2026

Copy link
Copy Markdown

Supersedes #15172, which was closed because it did not name the model and harness used. This is a fresh rebuild on current main by the model and harness listed at the end, reviewed by two others.

What Changed

Git prints the partial-clone filter after the fetch direction on promisor remotes:

origin	https://github.com/pingdotgg/t3code (fetch) [blob:none]
origin	https://github.com/pingdotgg/t3code (push)

Five parsers anchored their regex right after (fetch)/(push) and dropped that line. They now share one parser in @t3tools/shared/git (next to normalizeGitRemoteUrl), which accepts Git's optional trailing […] annotations and keeps the name, URL and direction captures as they were. Lines without an annotation parse exactly as before.

The sites: repository identity (RepositoryIdentityResolver), source-control provider detection (GitVcsDriver.listRemotes), reusing an existing remote (GitVcsDriverCore.ensureRemote), GitHub repository resolution (also used by the PR-checkout remote lookup), and Forgejo host matching. Since #15172, GitHub and Forgejo moved into their own packages, so the parser now lives in packages/shared, which all of them already depend on.

Why

Fixes #12764. On a partial clone the fetch remote vanished from every one of those paths, so:

  • repository identity was null, the Pull Requests view skipped the project, and linked-PR sync was skipped;
  • with "Group by repository", a partial clone and a normal clone of the same repository showed as two projects;
  • ensureRemote added a second remote instead of reusing origin, GitHub base-repository resolution could skip origin, and Forgejo fell back to https for an http remote.

Julius's triage asked for one suffix-tolerant helper shared by all parsers, with synthetic-stdout tests rather than tests that depend on the installed Git printing the annotation. #7499 (open) covers the identity resolver only.

Verification

To reproduce: git clone --filter=blob:none <any repo>, add it as a project, and open the Pull Requests view.

  • Tests (vp test run … in each package), all passing: packages/shared git.test.ts 36 (new parser tests: plain lines, [blob:none], [tree:0], two annotations, CRLF and blank lines, malformed lines, the fetch-URL helper); apps/server RepositoryIdentityResolver, GitVcsDriver, GitVcsDriverCore 213; source-control-github resolution and provider tests 46; source-control-forgejo provider tests 22. One new consumer test per site goes through its public entry point.
  • Red before green: with the five consumer source files from main and the new tests kept, each site test fails: expected undefined to be 'github.com/pingdotgg/t3code', expected [] to deeply equal [ { name: 'origin', … } ], expected 'pingdotgg' to equal 'origin', expected { host: 'github.com', locator: null } to deeply equal …, and expected 'https://forgejo.local:3000' to equal 'http://forgejo.local:3000'.
  • Typecheck (t3, @t3tools/shared, @t3tools/source-control-github, @t3tools/source-control-forgejo): exit 0. vp lint and vp fmt --check on the changed files: clean apart from two existing warnings on untouched lines.
  • Review: one reviewer ran 1,152 old-versus-new comparisons over unannotated lines (tabs, spaces, CRLF, duplicates, fetch/push order); all matched.

In-app evidence from #15172 (macOS, Git 2.50.1, a partial clone of this repository as the only project on a fresh server state; same behaviour as this rebuild, not re-captured):

Before:

Before: Pull Requests view is empty for a partial clone

After:

After: the partial clone's repository pull requests are listed

Not checked: a live Azure DevOps or Forgejo remote (synthetic tests only), and Windows or Linux test runs. Known and unchanged: a remote URL containing a space is still dropped, as before; with one parser now, that would be a one-place follow-up.

Models

  • Authored by Claude Opus 5.5, in Claude Code (run from T3 Code).
  • Reviewed by GPT-6.1 Sol (Codex CLI, high reasoning) and Claude Opus 5.5 (separate Claude Code review agent). Neither found anything significant; the commit scope was changed from server to git on review.

On a partial clone, `git remote -v` prints the filter after the direction, as in `origin <url> (fetch) [blob:none]`. Every remote parser anchored right after `(fetch)`/`(push)` and dropped that line, so repository identity came back empty, `listRemotes` omitted the remote, `ensureRemote` added a duplicate, and GitHub repository resolution and Forgejo host matching skipped it.

All five sites now share one parser in `@t3tools/shared/git` that accepts trailing bracketed annotations after the direction. Unannotated output parses exactly as before.

Fixes pingdotgg#12764

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:L 100-499 changed lines (additions + deletions). labels Oct 11, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at de8687c

Macroscope's review found this PR approvable — This focused bug fix centralizes Git remote parsing and restores partial-clone remote recognition across the affected identity and source-control paths. The behavioral change is narrow, preserves ordinary output handling, and has direct parser and consumer regression coverage.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 11, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 286a5531-8211-40a4-8d68-e9fa4c59e8ec

📥 Commits

Reviewing files that changed from the base of the PR and between 75fca7d and de8687c.


📒 Files selected for processing (12)
  • apps/server/src/project/RepositoryIdentityResolver.test.ts
  • apps/server/src/project/RepositoryIdentityResolver.ts
  • apps/server/src/vcs/GitVcsDriver.test.ts
  • apps/server/src/vcs/GitVcsDriver.ts
  • apps/server/src/vcs/GitVcsDriverCore.test.ts
  • apps/server/src/vcs/GitVcsDriverCore.ts
  • packages/shared/src/git.test.ts
  • packages/shared/src/git.ts
  • packages/source-control-forgejo/src/server/ForgejoCli.ts
  • packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts
  • packages/source-control-github/src/server/gitHubRepositoryResolution.test.ts
  • packages/source-control-github/src/server/gitHubRepositoryResolution.ts

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



📝 Walkthrough

Walkthrough

The change adds shared parsing for verbose Git remote output and uses it in server, Forgejo, and GitHub repository resolution. The parser accepts trailing bracketed annotations and distinguishes fetch from push entries. Tests cover partial-clone remote output and repository resolution.

Changes

Git remote parsing and repository resolution

Layer / File(s) Summary
Shared remote-output parser
packages/shared/src/git.ts, packages/shared/src/git.test.ts
Adds shared functions to parse verbose Git remote output and map fetch URLs by remote name. Tests cover annotations, line endings, blank lines, malformed entries, and fetch-only mapping.
Server remote consumers
apps/server/src/project/RepositoryIdentityResolver.ts, apps/server/src/project/RepositoryIdentityResolver.test.ts, apps/server/src/vcs/GitVcsDriver.ts, apps/server/src/vcs/GitVcsDriver.test.ts, apps/server/src/vcs/GitVcsDriverCore.ts, apps/server/src/vcs/GitVcsDriverCore.test.ts
Server identity resolution, remote listing, and remote reuse now use the shared parser. Tests cover annotated fetch entries in these paths.
Forgejo and GitHub resolution
packages/source-control-forgejo/src/server/ForgejoCli.ts, packages/source-control-forgejo/src/server/ForgejoSourceControlProvider.test.ts, packages/source-control-github/src/server/gitHubRepositoryResolution.ts, packages/source-control-github/src/server/gitHubRepositoryResolution.test.ts
Forgejo and GitHub repository resolution now use the shared parser to identify fetch remotes. Tests cover annotated remote entries and repository resolution.

Priority: ⬆️ High

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge


Merge Risk | ⚪ Minimal · up to de868

Merge Risk: ⚪ Minimal · up to de868

Partial-clone checkouts now have their fetch remotes recognized. Repository identity, PR listing, remote reuse, and provider resolution therefore work as they do for ordinary clones. Unannotated output behaves as before. No merge-blocking risk was identified.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to de868

Partial-clone Forgejo remotes configured with HTTP can now select an unencrypted API origin where host-based resolution previously defaulted to HTTPS. This may expose stored credentials to an on-path attacker. Matching-host and authentication controls limit reachability, but production transport protections remain unconfirmed.

Retained concerns

  • High · security · inferred: Newly recognized annotated HTTP fetch remotes can select an HTTP Forgejo login and API origin in host-based resolution. After authentication succeeds, the existing request path attaches the stored host credential without an HTTPS requirement. This enables an additional potentially cleartext credential path; general HTTP support predates the PR, and actual production transmission remains unverified.
Security review details

Security Blast Radius

  • inferred — Potential exposure is the selected stored credential for a matching Forgejo host, not arbitrary-host credential routing. If recovered from cleartext traffic, that credential could authorize access beyond the current repository according to its permissions. Actual token privileges, affected deployments, and network accessibility are unknown.

Security Findings and Attack Paths

  • inferred — No verified Security finding was supplied. The deferred path is: a matching annotated HTTP fetch remote selects an HTTP login; successful fj authentication permits a token-bearing API request to that origin; an on-path adversary could recover the token if production sends that request without external transport protection. The origin-selection and request-construction steps are supported; wire-level exploitation is not verified.

Trust Boundaries and Controls

  • observed — Fetch-only selection and exact host-and-port matching constrain which remote chooses a credential origin. API requests must remain within the selected origin and API prefix, exclude URL credentials, and use manual redirects. These controls contain destination changes but do not require HTTPS before attaching the token.

Resilience and Maintainability Implications

  • observed — Authentication is serialized and cached only after successful whoami and token reread. HTTP and HTTPS do not share cache entries. Request failures do not evict authentication, and later API token reads occur outside the authentication lock; these preexisting behaviors are not retained as separate PR concerns.

Hardening Proposals

  • proposed — Separate API credential-transport policy from Git remote parsing: require HTTPS before attaching credentials, or allow HTTP only through an explicit, narrowly scoped trusted-host policy rather than remote syntax alone.

Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: recognizing partial-clone remotes in Git output.
Description check Passed The description explains the problem, implementation, affected components, issue context, verification results, limitations, and authoring details. It does not use the template headings exactly, and e…
Linked Issues check Passed Issue #12764 requires recognition of git remote -v fetch lines with trailing partial-clone annotations, source-control provider and repository identity detection, and restored PR discovery and linke…
Out of Scope Changes check Passed The changes stay within the remote-parsing and repository-discovery scope of #12764. The shared parser, five consumer updates, and synthetic regression tests directly support partial-clone remote reco…

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

size:L 100-499 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Partial-clone remote annotations break repository detection and hide linked PRs

1 participant