refactor(forge): add forge detection from git remote URL

[ggallen]

Summary

• Add DetectForge() function to internal/forge/ that determines forge platform from git remote URLs
• Handles both HTTPS and SSH URL formats, case-insensitive host matching
• Returns actionable error with --forge flag suggestion for unknown hosts

Part of ADR 0067 Phase 0.

Related Issue

Part of GitLab forge support (ADR 0067)

Changes

• internal/forge/detect.go: New file with DetectForge() and extractHost()
• internal/forge/detect_test.go: Comprehensive table-driven tests

Testing

• go test ./internal/forge/... passes
• go vet clean
• make lint passes

🤖 Generated with Claude Code

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:53 PM UTC · Completed 12:05 AM UTC
Commit: 6680d8c · View workflow run →

[qodo-code-review]

PR Summary by Qodo

Add forge detection from git remote URL (GitHub/GitLab)

✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

AI Description

• Add forge auto-detection by parsing git remote URL host (HTTPS or SSH).
• Recognize github.com and gitlab.com hosts case-insensitively.
• Return actionable errors suggesting --forge for unknown/self-hosted hosts.

Diagram

graph TD
  A["Caller (CLI)"] --> B["DetectForge()"] --> C["extractHost()"] --> D{"Known host?"}
  D -->|"github.com"| E["Return github"]
  D -->|"gitlab.com"| F["Return gitlab"]
  D -->|"other/empty"| G["Error: suggest --forge"]
  T["detect_test.go"] -->|"unit tests"| B

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use an existing SCP-like git remote parser (normalize to ssh:// then url.Parse)

• ➕ Less bespoke string parsing logic; fewer edge cases around odd SSH formats
• ➕ Potentially easier to extend to additional remote syntaxes (ports, ssh://, etc.)
• ➖ Adds dependency surface or extra code to normalize remotes
• ➖ Overkill for a Phase 0 feature limited to two known SaaS hosts

Recommendation: The current approach is appropriate for Phase 0: minimal logic, no new dependencies, and explicit failure for self-hosted instances (with a clear --forge remediation). Consider a normalization-based parser only if future requirements include broader SSH/remote syntax support (ports, ssh:// scheme, scp edge cases).

Files changed (2) +234 / -0

Enhancement (1) +43 / -0

detect.goAdd DetectForge() to infer forge from remote URL host +43/-0

Add DetectForge() to infer forge from remote URL host

• Introduces DetectForge(remoteURL) to map git remote URLs to "github" or "gitlab" based on the extracted host. Supports HTTPS URLs via net/url and SSH SCP-like remotes via a simple user@host:path split, returning actionable errors suggesting --forge for unknown or unparseable hosts.

internal/forge/detect.go

Tests (1) +191 / -0

detect_test.goAdd table-driven tests for DetectForge() and extractHost() +191/-0

Add table-driven tests for DetectForge() and extractHost()

• Adds comprehensive unit tests covering HTTPS and SSH formats, presence/absence of .git suffix, case-insensitive host matching, and error messaging for unknown or malformed inputs. Includes direct tests for extractHost() behavior across representative URL formats.

internal/forge/detect_test.go

[github-actions]

Site preview

Preview: https://7eb932c3-site.fullsend-ai.workers.dev

Commit: 41b7b8639e633bc71efb979820653fd72648817a

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 54 rules

✅ Skills: writing-user-docs, writing-adrs

1. Remote URL whitespace breaks ✓ Resolved 🐞 Bug ☼ Reliability

Description

extractHost parses remoteURL without trimming and only uses url.Parse results when err==nil, so a
newline/whitespace-suffixed remote string (common when passed directly from command output) can fail
host extraction and make DetectForge return "cannot extract host" for valid GitHub/GitLab remotes.

Code

internal/forge/detect.go[R31-34]

+func extractHost(remoteURL string) string {
+	if u, err := url.Parse(remoteURL); err == nil && u.Hostname() != "" {
+		return u.Hostname()
+	}

Relevance

⭐⭐⭐ High

Repo frequently accepts trimming/normalizing whitespace/line endings to harden parsing (e.g.,
TrimSpace guard in PR #1723).

PR-#1723
PR-#2168

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new helper does not trim and drops Hostname() unless parsing is error-free, which makes it
sensitive to newline-terminated command output; the repo already trims Output() results in
multiple places, indicating newline output is expected.

internal/forge/detect.go[31-34]
internal/cli/admin.go[70-85]
internal/gitfetch/gitfetch_test.go[458-466]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`extractHost()` rejects otherwise-valid remote URLs when the input contains trailing whitespace/newlines or minor parse errors because it (1) does not trim the input and (2) requires `url.Parse` to return `err == nil` before using `u.Hostname()`.

### Issue Context
Other parts of the codebase trim newline-terminated command output before use; this helper should do the same since it will likely be fed raw `git` output in future integration.

### Fix Focus Areas
- internal/forge/detect.go[12-43]

### Suggested fix
- At the start of `extractHost` (or `DetectForge`), do `remoteURL = strings.TrimSpace(remoteURL)`.
- Relax the parse gate to use `u.Hostname()` when available even if `err != nil`, e.g.:
 - `u, err := url.Parse(remoteURL)`
 - `if u != nil && u.Hostname() != "" { return u.Hostname() }`
- Add tests for newline/whitespace-suffixed remotes (e.g. `"https://github.com/org/repo.git\n"`, `"git@github.com:org/repo.git\n"`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Inconsistent forge host helpers 🐞 Bug ⚙ Maintainability

Description

DetectForge hardcodes github.com/gitlab.com host mapping, but internal/forge already has separate
host/platform helpers with different semantics (e.g., forgeHost knows gitlab.com while
IsSupportedForge still only accepts github.com), creating conflicting definitions of "supported"
hosts inside the same package.

Code

internal/forge/detect.go[R18-25]

+	switch strings.ToLower(host) {
+	case "github.com":
+		return "github", nil
+	case "gitlab.com":
+		return "gitlab", nil
+	default:
+		return "", fmt.Errorf("unknown forge host %q: use --forge flag for self-hosted instances", host)
+	}

Relevance

⭐ Low

Team previously rejected deduping “single source of truth” helpers; likely okay with parallel host
logic (PR #1021).

PR-#1021
PR-#2736

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new DetectForge switch introduces a second, divergent host mapping in the forge package; url.go
already contains overlapping host/key logic but still treats only github.com as supported, making
the package internally inconsistent.

internal/forge/detect.go[18-25]
internal/forge/url.go[165-179]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Forge host/platform knowledge is now duplicated across `DetectForge` and existing helpers in `internal/forge/url.go`, and those sources currently disagree about gitlab.com.

### Issue Context
- `DetectForge()` recognizes `gitlab.com`.
- `forgeHost()` maps the `gitlab` key to `gitlab.com`.
- `IsSupportedForge()` still only returns true for `github.com`.

### Fix Focus Areas
- internal/forge/detect.go[18-25]
- internal/forge/url.go[165-179]

### Suggested fix
- Introduce a single source of truth in `internal/forge` (e.g., `var knownForgeHosts = map[string]string{"github.com":"github","gitlab.com":"gitlab"}`) and reuse it in `DetectForge`.
- Either:
 - Update `IsSupportedForge()` to use the same mapping (if gitlab.com should now be treated as supported), or
 - Rename/document `IsSupportedForge()` to clarify it is *only* for URL parsing support (so it is not mistakenly used as a general "known forge host" predicate).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[fullsend-ai-review]

Review

✅ Approve — Clean, well-tested internal utility with comprehensive table-driven tests. All prior findings resolved or confirmed at low severity with proper documentation.

Re-review delta

Code unchanged since prior review (41b7b8639e633bc71efb979820653fd72648817a). Findings re-evaluated with severity anchoring:

• ✅ Resolved: PR title changed from feat(forge): to refactor(forge): — matches COMMITS.md guidance for internal utilities (prior low [pr-title-prefix])
• ✅ Confirmed low / no action needed: DetectForge vs IsSupportedForge distinction is well-documented via doc comment (lines 13–15) and explicitly asserted by TestDetectForgeDistinctFromIsSupportedForge. Detection (identifying which forge a remote points to) is intentionally separate from support (full HTTPS URL parsing via ParseForgeURL). Architecturally sound — no consumers exist yet and the doc comment guides future callers. (prior low [api-contract])

Dimensions reviewed

Dimension	Result
Correctness	✅ No issues — logic is sound, tests are comprehensive (HTTPS, SSH shorthand, ssh:// scheme, ports, case insensitivity, whitespace, error paths)
Security	✅ No issues — pure URL parsing with no network calls, no secrets, no permission changes, no injection vectors
Intent & coherence	✅ Authorized by ADR 0067 Phase 0, scope matches authorization, refactor(forge): prefix is correct
Style & conventions	✅ Follows all established internal/forge/ patterns: fmt.Errorf with %q, testify assert/require, table-driven tests, unexported helpers
Documentation currency	✅ No staleness — internal/ utility adds no user-facing behavior; existing docs correctly describe GitHub-only support until GitLab is fully enabled
Cross-repo contracts	⏭ Skipped — internal/ package, no exported interfaces modified

---

Previous run

Review

✅ Approve — Clean, well-tested internal utility with comprehensive table-driven tests covering HTTPS, SSH shorthand, ssh:// scheme, ports, case insensitivity, whitespace, and error paths.

Re-review delta

Code unchanged since prior review (6680d8cc). Findings re-evaluated with severity anchoring:

• ✅ Resolved: Test file now uses testify (assert/require) consistently — custom containsSubstring/searchSubstring helpers removed (prior medium)
• ✅ Addressed: ssh:// scheme and port test coverage added in TestExtractHost (prior low)
• ✅ Addressed: Doc comment (lines 13–15) and TestDetectForgeDistinctFromIsSupportedForge explain the DetectForge vs IsSupportedForge distinction — prior remediation applied (prior medium → downgraded to low)

Findings

Low

• [api-contract] internal/forge/detect.go:24 — DetectForge returns "gitlab" for gitlab.com while IsSupportedForge("gitlab.com") returns false. The doc comment now explains this as an intentional distinction: detection (identifying which forge a remote points to) vs. support (full HTTPS URL parsing via ParseForgeURL). TestDetectForgeDistinctFromIsSupportedForge explicitly asserts both behaviors. This is architecturally sound — IsSupportedForge gates ParseForgeURL, a separate concern. The harness layer already accepts "gitlab" via ValidForgePlatform. No consumers of DetectForge exist yet; the doc comment guides future callers.

• [pr-title-prefix] PR title uses feat(forge): but per COMMITS.md, feat is reserved for user-facing changes. DetectForge is an internal utility in internal/forge/ not yet wired into any CLI command or user-visible behavior. COMMITS.md states: "Adding internal packages, helpers, or abstractions that don't change user-visible behavior → refactor." Consider changing to refactor(forge): add forge detection from git remote URL. Per COMMITS.md, prefix violations are not cosmetic — GoReleaser uses PR titles to build release notes.

---

Labels: PR adds Go code in internal/forge/ package

Previous run

Review

Findings

Medium

• [api-contract] internal/forge/detect.go:21 — DetectForge recognizes gitlab.com as a supported forge (returns "gitlab"), but IsSupportedForge in url.go in the same package explicitly rejects gitlab.com (returns false). The test in url_test.go is named "gitlab.com not yet supported" and asserts false. Consumers calling different functions in the same package get contradictory answers about whether GitLab is a supported forge. Meanwhile forgeHost() in url.go already maps "gitlab" → "gitlab.com", showing GitLab is partially anticipated but inconsistently supported across the package API.
  Remediation: Either update IsSupportedForge to also recognize gitlab.com, or add a clear doc comment to DetectForge explaining the distinction between "detection" (identifying what remote you cloned from) and "support" (full API operations available). Keep both functions' views of supported forges in sync.

• [pattern-inconsistency] internal/forge/detect_test.go:180 — The test file implements custom containsSubstring/searchSubstring helper functions that reimplement strings.Contains from the standard library. Additionally, all assertions use raw t.Fatalf/t.Errorf instead of testify (assert/require), which is the established pattern in the same package — url_test.go imports and consistently uses github.com/stretchr/testify/assert and require throughout.
  Remediation: Replace containsSubstring with strings.Contains, and use testify assert/require for assertions to match the established test patterns in the forge package.

Low

• [edge-case] internal/forge/detect.go:33 — extractHost handles HTTPS and SSH shorthand (git@host:path) formats, but there is no test coverage for SSH URLs with explicit ssh:// scheme (e.g., ssh://git@github.com/org/repo.git) or URLs with port numbers. Go's url.Parse handles these correctly via the first branch, but without test coverage the behavior is unverified.

• [pattern-inconsistency] internal/forge/detect.go:12 — DetectForge returns raw strings ("github", "gitlab") rather than using defined type constants. This mirrors the current codebase pattern (ForgeURLInfo.Forge is also a raw string, forgeHost() switches on raw strings), so this is a pre-existing pattern rather than a defect introduced by this PR.

• [design-direction] internal/forge/detect.go — PR title uses feat(forge): prefix, but per COMMITS.md, feat is reserved for user-facing changes. DetectForge is an internal utility in internal/forge/ — not directly user-visible. COMMITS.md states: "Adding internal packages, helpers, or abstractions that don't change user-visible behavior → refactor." Consider changing to refactor(forge): add forge detection from git remote URL.

---

Labels: PR adds Go code in internal/forge/ package

Previous run

Review

✅ Approve — Clean, well-tested internal utility with comprehensive table-driven tests covering HTTPS, SSH shorthand, ssh:// scheme, ports, case insensitivity, whitespace, and error paths.

Re-review delta

Code unchanged since prior review (6680d8cc). Findings re-evaluated with severity anchoring:

• ✅ Resolved: Test file now uses testify (assert/require) consistently — custom containsSubstring/searchSubstring helpers removed (prior medium)
• ✅ Addressed: ssh:// scheme and port test coverage added in TestExtractHost (prior low)
• ✅ Addressed: Doc comment (lines 13–15) and TestDetectForgeDistinctFromIsSupportedForge explain the DetectForge vs IsSupportedForge distinction — prior remediation applied (prior medium → downgraded to low)

Findings

Low

• [api-contract] internal/forge/detect.go:24 — DetectForge returns "gitlab" for gitlab.com while IsSupportedForge("gitlab.com") returns false. The doc comment now explains this as an intentional distinction: detection (identifying which forge a remote points to) vs. support (full HTTPS URL parsing via ParseForgeURL). TestDetectForgeDistinctFromIsSupportedForge explicitly asserts both behaviors. This is architecturally sound — IsSupportedForge gates ParseForgeURL, a separate concern. The harness layer already accepts "gitlab" via ValidForgePlatform. No consumers of DetectForge exist yet; the doc comment guides future callers.

• [pr-title-prefix] PR title uses feat(forge): but per COMMITS.md, feat is reserved for user-facing changes. DetectForge is an internal utility in internal/forge/ not yet wired into any CLI command or user-visible behavior. COMMITS.md states: "Adding internal packages, helpers, or abstractions that don't change user-visible behavior → refactor." Consider changing to refactor(forge): add forge detection from git remote URL. Per COMMITS.md, prefix violations are not cosmetic — GoReleaser uses PR titles to build release notes.

---

Labels: PR adds Go code in internal/forge/ package

Previous run (2)

Review

Findings

Medium

• [api-contract] internal/forge/detect.go:21 — DetectForge recognizes gitlab.com as a supported forge (returns "gitlab"), but IsSupportedForge in url.go in the same package explicitly rejects gitlab.com (returns false). The test in url_test.go is named "gitlab.com not yet supported" and asserts false. Consumers calling different functions in the same package get contradictory answers about whether GitLab is a supported forge. Meanwhile forgeHost() in url.go already maps "gitlab" → "gitlab.com", showing GitLab is partially anticipated but inconsistently supported across the package API.
  Remediation: Either update IsSupportedForge to also recognize gitlab.com, or add a clear doc comment to DetectForge explaining the distinction between "detection" (identifying what remote you cloned from) and "support" (full API operations available). Keep both functions' views of supported forges in sync.

• [pattern-inconsistency] internal/forge/detect_test.go:180 — The test file implements custom containsSubstring/searchSubstring helper functions that reimplement strings.Contains from the standard library. Additionally, all assertions use raw t.Fatalf/t.Errorf instead of testify (assert/require), which is the established pattern in the same package — url_test.go imports and consistently uses github.com/stretchr/testify/assert and require throughout.
  Remediation: Replace containsSubstring with strings.Contains, and use testify assert/require for assertions to match the established test patterns in the forge package.

Low

• [edge-case] internal/forge/detect.go:33 — extractHost handles HTTPS and SSH shorthand (git@host:path) formats, but there is no test coverage for SSH URLs with explicit ssh:// scheme (e.g., ssh://git@github.com/org/repo.git) or URLs with port numbers. Go's url.Parse handles these correctly via the first branch, but without test coverage the behavior is unverified.

• [pattern-inconsistency] internal/forge/detect.go:12 — DetectForge returns raw strings ("github", "gitlab") rather than using defined type constants. This mirrors the current codebase pattern (ForgeURLInfo.Forge is also a raw string, forgeHost() switches on raw strings), so this is a pre-existing pattern rather than a defect introduced by this PR.

• [design-direction] internal/forge/detect.go — PR title uses feat(forge): prefix, but per COMMITS.md, feat is reserved for user-facing changes. DetectForge is an internal utility in internal/forge/ — not directly user-visible. COMMITS.md states: "Adding internal packages, helpers, or abstractions that don't change user-visible behavior → refactor." Consider changing to refactor(forge): add forge detection from git remote URL.

---

Labels: PR adds Go code in internal/forge/ package

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 11:27 AM UTC · Completed 11:38 AM UTC
Commit: 41b7b86 · View workflow run →

[ggallen]

/fs-review

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 1:22 PM UTC · Completed 1:30 PM UTC
Commit: e8381e3 · View workflow run →