refactor(source-control): forges recognise their own remotes and repository keys#23993

merged1
scp/01-change-request-urls
scp/02-remote-claims
opened19d agobyJJulius Marminge
View on GitHubTwo-way sync enabledPR last imported 18d ago

Layer 2 of 9 in the source-control plugin boundary stack: forge knowledge (GitHub, GitCafe, GitLab, Forgejo, Bitbucket, Azure DevOps) moves into packages/source-control-<forge>, and shared, contracts, server and clients ask forge-neutral hooks in source-control-core instead.

Shared and contracts held every forge's remote host rules, Azure DevOps's repository key spellings (twice, plus a byte-for-byte copy in the server), the Azure repository selector, and Forgejo's port-carrying change request host, and the pull request service special-cased Forgejo SSH remotes, dev.azure.com keys and a hardcoded list of public hosts.

Each forge's client definition now owns these behind hooks: recognizeRemote, changeRequestHost, repositorySelector, repositoryKeyFromRemote, canonicalRepositoryKey and sshRemoteHidesWebHost. The registry asks them in an explicit recognition order that reproduces the old precedence, and falls back to an unknown host. normalizeGitRemoteUrl is forge-neutral; persisted keys come from the registry's repositoryKeyFromRemote, which keeps the old output byte for byte (checked against a snapshot of every old function on ~90 remotes and keys). pullRequestHostOf, detectSourceControlProviderFromRemoteUrl, canonicalRepositoryKey, sourceControlRepositorySelector and the GitHub-only remote parser are gone. RepositoryIdentity.provider is now a SourceControlProviderKind; the encoded value is unchanged.

Behaviour changes: agent session import names a project's repository for every forge, not only GitHub. Forgejo's definition names codeberg.org as its public host. The GitHub, GitLab and Bitbucket plugins consult only their own recogniser, so a hostname carrying two forges' labels can now read as theirs inside that plugin.

Done by Claude Opus 5.5 in Claude Code, running inside T3 Code.

πŸ€– Generated with Claude Code

Checks

22 successful Β· 16 skipped
All checks passed
5549e22
Synced from GitHub

Thread transfer impact

βœ… Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

ProviderMetricMain baselineThis PRImpactPR ceiling
CodexTotal thread wireβ€”5.0 KiBβ€”6.8 KiBβœ…
CodexThread snapshot wireβ€”3.8 KiBβ€”4.9 KiBβœ…
CodexLive turn WebSocket wireβ€”1.2 KiBβ€”2.0 KiBβœ…
CodexLive turn WebSocket decodedβ€”20.9 KiBβ€”29.3 KiBβœ…
CodexLive turn messagesβ€”2β€”8βœ…
ClaudeTotal thread wireβ€”5.0 KiBβ€”6.8 KiBβœ…
ClaudeThread snapshot wireβ€”3.8 KiBβ€”4.9 KiBβœ…
ClaudeLive turn WebSocket wireβ€”1.2 KiBβ€”2.0 KiBβœ…
ClaudeLive turn WebSocket decodedβ€”21.2 KiBβ€”29.3 KiBβœ…
ClaudeLive turn messagesβ€”2β€”8βœ…

Baseline: unavailable Β· PR result: 5549e22 Β· Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

Synced from GitHub

Approvability

Verdict: Needs Human Approval

Macroscope's review found this PR requires human approval β€” This PR changes source-control recognition, persisted repository identity keys, pull-request routing, and public-host reads across shared contracts, server code, web code, and multiple providers. It also adds Codeberg/no-checkout behavior without a focused service-level test, leaving a broad production behavior change insufficiently verified.

No code changes detected at 5549e22. Prior analysis still applies.

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

Synced from GitHub

Review in Change Stack β†’

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 65e768341544c8fab5046c3c3caf47ec6ba74b0e and 5549e2213e4fb9d5255c887ac784369833894348.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • πŸ” Trigger review

No actionable comments were generated in the recent review.

External image Recent review info
External image Run configuration
External image Commits

Reviewing files that changed from the base of the PR and between 0e4df5107984bee94278c7f9128a75c5722b8cea and 65e768341544c8fab5046c3c3caf47ec6ba74b0e.

External image Files selected for processing: 6
  • apps/server/src/mcp/toolkits/pullRequests/handlers.test.ts
  • apps/server/src/mcp/toolkits/pullRequests/handlers.ts
  • packages/client-runtime/src/changeRequestLinks.test.ts
  • packages/client-runtime/src/changeRequestLinks.ts
  • packages/source-control-github/src/client/definition.test.ts
  • packages/source-control-github/src/client/definition.ts

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

Walkthrough

The pull request moves remote recognition, repository-key handling, repository selection, and change-request host resolution into the source-control client registry and provider definitions. Server and client code now use these registry operations. The repository identity contract narrows its provider field to SourceControlProviderKind.

Changes: Source-control resolution

Layer / File(s)Summary
Registry contracts and shared parsing
packages/source-control-core/src/client/definition.ts, packages/source-control-core/src/client/threadPullRequests.ts, packages/shared/src/git.ts, packages/shared/src/sourceControl.ts, packages/contracts/src/environment.ts, packages/contracts/src/pullRequest.ts, packages/client-runtime/src/sourceControlClients.ts, related tests
The client registry adds remote recognition, repository-key operations, repository selection, and change-request host resolution. Shared Git utilities retain general remote parsing while provider-specific detection and repository helpers are removed. RepositoryIdentity.provider uses SourceControlProviderKind.
Provider recognition and repository rules
packages/source-control-*/src/client/definition.ts, packages/source-control-*/src/client/definition.test.ts, packages/source-control-*/src/server/*, packages/source-control-core/src/server/PullRequestProvider.ts
Provider definitions add remote recognition and provider-specific repository-key, repository-selection, and host rules. Tests cover remote forms and host matching. GitHub, Bitbucket, and Forgejo pull-request providers declare public-host read capability.
Server repository and pull-request flows
apps/server/src/git/*, apps/server/src/mcp/toolkits/pullRequests/*, apps/server/src/orchestration-v2/*, apps/server/src/project/*, apps/server/src/pullRequest/*, apps/server/src/sourceControl/*, apps/server/src/vcs/GitVcsDriverCore.ts
Server services use the registry for remote recognition, repository keys, repository selection, and change-request hosts. Pull-request project listing uses provider lookup and its public-host capability. Repository matching and sync-key logic use registry canonicalization.
Client repository and pull-request flows
apps/mobile/src/features/pull-requests/*, apps/web/src/components/*, apps/web/src/hooks/*, apps/web/src/lib/*, packages/client-runtime/src/*, related tests
Client runtime and web components use registry operations for repository selection and change-request host resolution. Provider test fixtures construct provider values with SourceControlProviderKind.make.

Priority: External image Normal

Estimated code review effort: 4 (Complex) | ~60 minutes

Change: Refactor

Sequence Diagram(s)

mermaid
1sequenceDiagram
2 participant RepositoryIdentityResolver
3 participant sourceControlClients
4 participant SourceControlClientDefinition
5 RepositoryIdentityResolver->>sourceControlClients: recognizeRemote(remoteUrl)
6 sourceControlClients->>SourceControlClientDefinition: try provider recognizers in configured order
7 SourceControlClientDefinition-->>sourceControlClients: recognized provider or no match
8 RepositoryIdentityResolver->>sourceControlClients: repositoryKeyFromRemote(remoteUrl)
9 sourceControlClients-->>RepositoryIdentityResolver: provider-aware repository key

Suggested reviewers: macroscopeapp

Merge Risk: External image Minimal Β· up to 65e76

No confirmed issue blocks merging based on the available evidence.

Pre-merge checks: Passed 3 Β· Failed 1

Failed Failed checks: 1 warning

Check nameStatusExplanationResolution
Description checkWarningThe description provides detailed problem context, implementation scope, behavior changes, and agent attribution. However, it does not follow the required section structure and does not provide the re…Add explicit Problem, Change, Scope and approval, and Verification sections. Include a linked triaged issue or maintainer approval with its approval comment, describe the focused tests and observed results, and state any checks that were no…
Passed Passed checks: 3 passed
Check nameStatusExplanation
Title checkPassedThe title clearly identifies the main change: forge-specific remote and repository-key recognition moves into source-control implementations.
Linked Issues checkPassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes checkPassedCheck skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description provides detailed problem context, implementation scope, behavior changes, and agent attribution. However, it does not follow the required section structure and does not provide the required triaged issue or explicit maintainer approval, focused verification results, or stated limitations.

Resolution

Add explicit Problem, Change, Scope and approval, and Verification sections. Include a linked triaged issue or maintainer approval with its approval comment, describe the focused tests and observed results, and state any checks that were not performed.

Finishing Touches: πŸ’‘ 1
πŸ“ Generate docstrings
  • Commit to this branch
  • Create a new PR
πŸ§ͺ Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
πŸ› οΈ Fix failing CI checks: πŸ’‘
  • Commit to this branch
  • 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.

Review follow-up, now on the pushed heads:

  • prcomment_01m5nmg5wsf7bb8acjwva0ejqn DISMISS: main already read codeberg.org without checkout (PullRequestService.ts:890 list); shared path covered by "#17167 refresh linked PRs without a checkout"

πŸ€– Generated with Claude Code

Added 1 commit

  • refactor(source-control): forges recognise their own remotes and repository keys5549e22