Skip to content

fix(server): preserve spaces in Git remote URLs - #34324

Open
voltcrash wants to merge 1 commit into
pingdotgg:mainfrom
voltcrash:t3/remote-url-spaces
Open

voltcrash wants to merge 1 commit into
pingdotgg:mainfrom
voltcrash:t3/remote-url-spaces

Conversation

@voltcrash

Copy link
Copy Markdown
Contributor

Problem

With git remote add origin '/tmp/team repos/shared.git', Git lists the remote correctly, but T3 drops its fetch and push entries. ensureRemote then attempts to add origin again and fails because that name already exists. listRemotes also returns an empty list.

Change

Both VCS parsers now use Git's tab and direction separators to preserve the complete URL field. Fetch/push handling, CRLF and partial-clone annotations remain supported. The regression checks remote reuse, collision suffixing, distinct push URLs and preservation of the configured URLs, including leading/trailing whitespace and literal direction markers in paths.

Scope and approval

This qualifies as a very small, focused fix for an obvious bug: an existing remote disappears solely because its local path contains spaces. It changes the two VCS parsers responsible for that failure, without contracts, UI changes or parser consolidation. The other three verbose-remote parsers serve repository identity (which intentionally excludes local paths) and host-specific provider operations.

Credit to @yashranaway's earlier fix in #11390. It was closed unmerged with a maintainer request to rebuild on current main with a newer model. This fix was independently reproduced and rebuilt on current main, including the second VCS parser. #28769 leaves URL spaces unresolved and owns parser consolidation; #29625 addresses multiple configured URLs, a separate problem.

Verification

Linux ARM64, Git 2.56.0. New real-Git regressions before the fix: 4 failed, 1 passed (the existing HTTPS/SSH row). After the fix, the focused remote-operation and listing run: 29 passed.

A separate runtime program constructed the actual server Git services, created temporary repositories and real bare remotes under team repos, configured separate fetch/push URLs and a partial-clone filter, and called listRemotes and ensureRemote:

                                 Before                     After
listRemotes                      []                         origin, exact fetch/push URLs
ensureRemote(fetch URL)           GitCommandError (exit 3)   origin
ensureRemote(distinct push URL)   GitCommandError (exit 3)   origin-1

Git's final listing confirmed that origin retained both URLs and origin-1 used the requested URL. No client UI or Windows runtime verification was performed; this is a server parser fix.

vp run check:changed upstream/main: format, lint, both knip passes, generated GraphQL, typecheck of 3 affected packages and all 328 tests in the 3 affected test files passed. The branch was then cleanly rebased onto current main, with unchanged behavior diff and the server runtime reproduction passing again.

Reviewed with review-loop: 1 round with Claude Opus 5.5 on high. Reviewer verdict: approved. No blockers or should-fix findings; shared-parser consolidation remains with #28769.

Created with GPT-6.1 Sol in the Codex harness through T3 Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 11, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 11, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 7729934

Macroscope's review found this PR approvable — The change preserves spaces in Git remote URLs when listing and matching existing remotes. It leaves remote operations and configuration-write safeguards unchanged, with no obvious material risk requiring human judgment.

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

@voltcrash
voltcrash force-pushed the t3/remote-url-spaces branch from 7729934 to 5fbccae Compare November 11, 2026 11:36
@coderabbitai

coderabbitai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The remote listing parsers now handle LF and CRLF output and preserve spaces in URLs. Tests cover remote URL parsing and remote selection.

Changes: Git remote URL parsing

Layer / File(s) Summary
Parse and validate remote listings
apps/server/src/vcs/GitVcsDriver.ts, apps/server/src/vcs/GitVcsDriverCore.ts, apps/server/src/vcs/GitVcsDriver.test.ts, apps/server/src/vcs/GitVcsDriverCore.test.ts
Both parsers use Git’s tab-delimited remote format and support LF or CRLF output. Tests check URLs containing spaces and remote selection.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Possibly related PRs

  • pingdotgg/t3code#28769: Replaces the same remote parsers with a shared parseGitRemoteEntries implementation.

Suggested reviewers: juliusmarminge

Merge Risk: Minimal · up to 5fbcc

This server-side fix keeps spaces in Git remote URLs, so listing remotes and reusing an existing remote name should work as intended. No merge-blocking risk remains.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: preserving spaces in Git remote URLs in the server.
Description check Passed The description covers the required Problem, Change, Scope and approval, and Verification sections. It explains the bug, implementation, scope rationale, regression coverage, observed results, limitat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description covers the required Problem, Change, Scope and approval, and Verification sections. It explains the bug, implementation, scope rationale, regression coverage, observed results, limitations, and review context.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between faa8ad6 and 5fbccae.

Files selected for processing: 4

  • 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

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

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:S 10-29 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.

1 participant