Repository navigation
fix(doctor): cover mixed push destinations in advice - #126
Conversation
Explain separate SSH and HTTPS checks after higher-priority warnings. Add real Git fixtures for both URL orders with injected SSH observations, priority coverage, and matching default/verbose advice. Preserve verdicts, short output, JSON, exits, offline behavior and runtime authentication. Version remains 0.13.0; no schema, storage or setup changes. Validation: build, unit, E2E, release-workflow and docs checks passed. 128 exact machine comparisons passed. Unit skips: 2 Fish tests; E2E skip: 1 optional install test. Independent local review found no code issue.
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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to The change updates guidance for mixed SSH/HTTPS destinations. No actionable issue is identified in the supplied review context; the PR reports passing validation, so it appears mergeable subject to normal checks. Pre-merge checks |
 |
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Title check | |
The title clearly and concisely describes the main change: updating doctor advice to handle mixed push destinations. |
| Description check | |
The description directly explains the mixed SSH/HTTPS destination issue, the implemented advice changes, preserved behavior, and validation results. |
✨ Finishing Touches
-
-
-
📝 Generate docstrings
-
-
-
-
-
-
- Commit to this branch
- Create a new PR
-
-
-
-
-
🧪 Generate unit tests (beta)
-
-
-
- Commit to this branch
- Create a new PR
-
-
-
-
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
A rabbit checks the paths at night
SSH and HTTPS in sight
Verbose details guide the way
Unknown routes get checked today
Then account checks can hop and play
Comment @coderabbitai help to get the list of available commands.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 944baa8516
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Name unsupported configuration when mixed SSH/HTTPS targets include an unknown transport or custom SSH command, and point to every destination in verbose details before choosing separate checks. Extend the real Git fixture matrix in both URL orders and online/offline. Preserve verdicts, short output, JSON, exits and authentication behavior. Validation: 404 unit tests, 54 E2E tests, 138 release-workflow tests, build and docs passed. Two Fish tests and one optional install test skipped. 160 exact machine comparisons passed across 320 paired CLI cases.
1 participant
Summary
Fix the human
nextadvice when one remote has both SSH and HTTPS push URLs. An aligned SSH observation and HTTPS pin previously led doctor to suggestgitrole auth test, which does not support the mixed destination set.What changed
--verbosedetails.Why
The suggested action must cover every push destination. Checking only one transport cannot resolve the whole mixed set. Addresses the advice issue identified in #124.
Testing
The new fixture failed on the unchanged baseline in both URL orders, then passed with the fix. A separate fresh-context review using the same model reproduced those failures, examined all six changed files, and found no code blockers.
Build, unit tests (404 passed, 2 Fish skips), E2E (54 passed, 1 optional install skip), release-workflow fixtures (138 passed), and docs build passed locally.
160 exact baseline/candidate machine comparisons preserved stdout, stderr, exit codes, and probe counts across both orders, online/offline, and verbose combinations, plus unsupported third targets and custom SSH commands. Across 320 CLI cases, human differences were limited to the intended next lines.
Renewed independent review exercised 56 additional real-Git cases, including every ordering of SSH/HTTPS/unsupported targets and four custom SSH configurations. The unsupported mixed-set regression failed before the correction and passed afterward.
A deliberate offline-probe mutation in an isolated compiled copy was detected; restored comparisons passed.
The initial sandbox unit run failed on restricted process inspection and one outdated advice assertion. The assertion was corrected and the full suite passed with process inspection available.
Hosted CI passed on final head
b0b37d2138e187f6d408c42a144719f69392d302: Node 20/22, docs, and hermetic release-confidence E2E. Each hosted unit run passed 396 tests with 10 shell-dependent skips (zsh/Fish unavailable); E2E passed 54 with 1 optional install skip; release-workflow tests passed 138.CodeRabbit completed review through the final head with no actionable findings. It selected the renderer only; filters excluded both docs files and all three test files across the PR, and collecting Node 20/22 check context timed out. Independent local review covered all six cumulative files.
Codex's initial unsupported-destination P2 was reproduced and corrected in
b0b37d2. A new-head hosted Codex review is not yet available, and its original review thread remains open.Risk
Advice selection and wording only. Verdicts,
status --short, JSON, exit codes, and runtime authentication behavior are unchanged. Final local full suites passed on Node 22 on macOS; hosted checks are reported separately.Out of scope
No dependencies, guided setup, role/config/keychain changes, hook installation, merge, or release.
Follow-ups
Complete a day of normal use of the merged concise output before guided setup. This remains open; fixtures and elapsed time do not establish that qualification.