Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a focused error-message correction that preserves provider detection, operation behavior, structured fields, and named-provider output. The added tests cover all fallback operations and transport formatting, with no production defaults or static-analysis configuration changes. You can add or adjust custom eligibility rules. Learn more. |
|
Note Written by Thanks for this. I'm closing it as part of PR triage. Right now we're only taking focused bug fixes from outside contributors (see Read this first), and this PR is not one. From the review: This only changes the wording of undetected source control provider errors; provider detection and operation behavior stay the same. Copy changes need prior maintainer agreement. If you think this was a mistake, message me on Twitter at @maria_rcks and I'll take another look. |
Problem
When T3 cannot detect a project's source-control provider, explicit provider operations report
No unknown source control provider is registered.This treats an undetected provider as a named provider with a missing registration instead of explaining the detection failure.Resolve the provider for a repository with no remotes, then call
getChangeRequestwith reference#42. The error changes from:to:
Change
Choose the fallback error detail once based on whether the provider is undetected or named but unregistered. Apply it to all seven fallback operations. The shared contract formatter omits
unknownfrom the user-facing provider name while preserving the failed operation and the structured provider, cwd, repository, and reference fields.The contract change is needed because the old formatter would otherwise still prefix the corrected detail with
Source control provider unknown. Named-provider messages stay unchanged. Tests cover both fallback cases and error encoding/decoding used across clients.Scope and approval
Fixes #19667. Maintainer confirmation and invitation to submit this wording fix.
This PR changes only error wording. It does not change provider detection, lookup results, or warning frequency. #19030 handles the empty-list fallback for unknown-host change-request lookups separately; the remaining explicit operations still need this message correction.
No client-specific UI or documentation change is needed. The error schema fields are unchanged.
Verification
Checked on Linux x86_64 with Node.js v24.21.0, based on current upstream
main.vp test run apps/server/src/sourceControl/SourceControlProviderRegistry.test.ts packages/contracts/src/sourceControl.test.ts apps/server/src/sourceControl/SourceControlRepositoryService.test.tsAll 36 tests across 3 files pass. The registry regression resolves a repository with no remotes and exercises all seven fallback operations, checking the detection explanation and failed operation. A named, unregistered provider retains its registration explanation. The contract tests verify the exact unknown and GitHub message formats before and after schema encoding/decoding, with request context preserved. Existing repository-service tests also pass.
Both package type checks pass.
Lint exits successfully with one pre-existing empty-import warning at
SourceControlProviderRegistry.ts:12. Formatting and whitespace checks pass.No browser, desktop, mobile, remote-connection, or live hosting-service verification was run. This is a backend/shared-contract wording change; the focused tests use mocked external dependencies. No repo-wide checks were run.
Model:
codex/gpt-6.1-sol, xhigh reasoning effort. Harness: Codex in T3 Code.