Fix GetRemotes parsing for URLs with spaces

[bschellenberger2600]

Summary

• fix GetRemotes parsing so remote URLs are not truncated when the URL/path contains spaces
• parse git remote -v output by separating remote name from the remainder and removing trailing (fetch) / (push) suffixes
• add a regression test that covers a local bare remote path containing spaces

Test plan

• go test -count=1 ./...

Made with Cursor

---

Note

Low Risk
Low risk: changes are confined to test utilities and add regression coverage; main behavior change is more robust parsing of git remote -v output, which could only affect tests relying on GetRemotes.

Overview
Fixes GetRemotes parsing to preserve full remote URLs/paths (including spaces) by splitting the remote name from the remainder and trimming only the (fetch)/ (push) suffixes, with a fallback for non-tab formatted output.

Adds a regression test covering a local remote path containing spaces, and introduces new USB volume fixture helpers (usb_fixtures.go) to create/read/write .git-fire config, validate relative layout directories, assert repo layout, and generate canonical file:// URLs (with accompanying tests).

^{Reviewed by Cursor Bugbot for commit e3d6168. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

• Bug Fixes

  • Improved handling of git remote paths containing spaces and special characters.

• Tests

  • Enhanced test infrastructure for verifying remote path parsing with whitespace.
  • Added new test utilities for USB volume fixture setup and configuration management.

[coderabbitai]

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix GetRemotes parsing for URLs with spaces' directly and clearly summarizes the main change: fixing the GetRemotes function to handle URLs containing spaces, as confirmed by the primary commit and code changes in fixtures.go.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/getremotes-space-paths

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

usb_fixtures.go (1)

36-45: Constrain LayoutDir to stay under the fixture root.

Absolute or parent-relative values can make CreateReposDir write outside root, which weakens fixture isolation.

Suggested fix

 	if cfg.LayoutDir == "" {
 		cfg.LayoutDir = "repos"
 	}
+	cleanLayout := filepath.Clean(cfg.LayoutDir)
+	if filepath.IsAbs(cleanLayout) ||
+		cleanLayout == ".." ||
+		strings.HasPrefix(cleanLayout, ".."+string(os.PathSeparator)) {
+		t.Fatalf("layout_dir must be relative to fixture root: %q", cfg.LayoutDir)
+	}
+	cfg.LayoutDir = cleanLayout
 	if cfg.Strategy == "" {
 		cfg.Strategy = "git-mirror"
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@usb_fixtures.go` around lines 36 - 45, Sanitize and constrain cfg.LayoutDir
so it cannot point outside the fixture root: before calling WriteUSBVolumeConfig
and before using filepath.Join(root, cfg.LayoutDir), normalize cfg.LayoutDir
(use filepath.Clean), strip any leading volume/absolute markers or leading "/"
so it is relative, and reject or replace any value that contains parent
traversal (e.g., components "..") by resetting to the safe default "repos"; then
continue to call WriteUSBVolumeConfig and use os.MkdirAll(filepath.Join(root,
cfg.LayoutDir), 0o755) as before. Ensure you reference and update the
cfg.LayoutDir value (the symbol cfg.LayoutDir) and keep the existing uses in
WriteUSBVolumeConfig, opts.CreateReposDir, os.MkdirAll and filepath.Join.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@usb_fixtures.go`:
- Around line 95-99: When parsing the "schema_version" branch, don't ignore
strconv.Atoi errors; validate the conversion of val and handle failures by
returning or bubbling an explicit error instead of leaving cfg.SchemaVersion as
zero. In the switch case for key == "schema_version" (where you currently call
strconv.Atoi(val) and assign to cfg.SchemaVersion), capture the returned error,
and if non-nil return a descriptive error (e.g., include the raw val and context
like "invalid schema_version") so malformed .git-fire content is surfaced rather
than silently treated as 0.
- Around line 132-133: The file URL builder currently creates u :=
&url.URL{Scheme: "file", Path: filepath.ToSlash(abs)} which yields
"file://C:/..." on Windows; change the construction to ensure drive-letter
absolute paths get a leading slash before setting Path (e.g., detect Windows
drive-letter like ^[A-Za-z]: and prefix "/" if missing) so url.URL produces
"file:///C:/..."; update any tests that assert the output to expect the
three-slash canonical form ("file:///...") instead of accepting "file://...".

---

Nitpick comments:
In `@usb_fixtures.go`:
- Around line 36-45: Sanitize and constrain cfg.LayoutDir so it cannot point
outside the fixture root: before calling WriteUSBVolumeConfig and before using
filepath.Join(root, cfg.LayoutDir), normalize cfg.LayoutDir (use
filepath.Clean), strip any leading volume/absolute markers or leading "/" so it
is relative, and reject or replace any value that contains parent traversal
(e.g., components "..") by resetting to the safe default "repos"; then continue
to call WriteUSBVolumeConfig and use os.MkdirAll(filepath.Join(root,
cfg.LayoutDir), 0o755) as before. Ensure you reference and update the
cfg.LayoutDir value (the symbol cfg.LayoutDir) and keep the existing uses in
WriteUSBVolumeConfig, opts.CreateReposDir, os.MkdirAll and filepath.Join.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4e17f899-d32b-40f9-be1a-24b5c535cab1

📥 Commits

Reviewing files that changed from the base of the PR and between 0273a41 and 58fe7d0.

📒 Files selected for processing (4)

• fixtures.go
• fixtures_test.go
• usb_fixtures.go
• usb_fixtures_test.go

[bschellenberger2600]

CodeRabbit nitpick (layout_dir under fixture root) — Addressed in 929834e: added `mustRelativeLayoutDir` to reject absolute paths and `..` escapes; `MustUSBVolumeRoot` and `WriteUSBVolumeConfig` both normalize via this helper so `CreateReposDir` cannot target outside the temp root.