Originally created by: Akarsh-Hegde
Resolves the outstanding review comments on [#495]. Opened against pre-main because [#495]'s head is pre-main - commits cannot be pushed onto it directly, so fixes land here and flow into that promotion.
Of the 11 inline comments on [#495], 5 were already fixed by [#496] (self-referential workflow comments, contents: write scoping, ${{ }} shell interpolation, the placeholder-overwrite bug, the duplicated detect_install_mode). Those threads are stale and resolved. This PR covers the 4 that were not, and re-declines 2.
merge-windows-updater.py sets PYTHONIOENCODING=utf-8 in the workflow, which covers stdin/stdout/stderr only. Path.read_text() and Path.write_text() fall back to the locale encoding - cp1252 on the Windows runner this script runs on.
So [#500] fixed the ✓ in the success print and left the file I/O exposed to the identical failure one line away. A single non-ASCII character in latest.json and the read dies, in the script whose entire job is to not corrupt the manifest macOS auto-update depends on. Encoding is now pinned on all three operations.
Good catch by the reviewer, and a fair one against me.
ui/lib/api-types.tsInstallOutcome was defined in LlmProviderPicker.tsx. Per the repo rule, Rust-mirrored invoke response types live in api-types.ts so they cannot drift from src/llm/detect.rs unnoticed.
Moved all four (InstallOutcome, ProviderStatus, ProviderTestResult, ProviderTestOutcome) rather than only the one flagged - the review listed the siblings as candidates, and moving one of four would leave exactly the split the rule exists to prevent. The picker re-exports them so the existing import sites in steps.tsx and LlmProviderDetail.tsx keep resolving.
install_provider and cursor_sign_in shell out to vendor installers - real work, and the observability guideline requires a span with an explicit failure status, not just tracing::info! lines.
Both are now instrumented, recording ok and the resolved cli_path. The neat part: install_failed is the single funnel every failure path in both functions returns through, so recording on the current span there marks whichever is running as failed without threading a span handle through a dozen early returns.
sourceOf and stripComments were near-identical comment-stripping helpers in two suites. Extracted to __tests__/helpers/source.ts.
dtolnay/rust-toolchain@master to a SHA. It appears in all seven places across every workflow in the repo. Pinning one file is inconsistent rather than safer. The review cites a blanket repo policy, but nothing in .github/workflows is pinned today - so this is a real repo-wide change, and it deserves to be one.value_is_set rejects anything containing your-, _your_ or -here, which can in principle reject a valid Jira URL. But the failing examples are contrived, and tightening the rule is a product judgement about what a real credential looks like - not something to settle inside a review-fix PR.cargo clippy --all-targets -- -D warnings cleanui production build succeeds; no typecheck errors in the touched fileswindows-x86_64 with darwin entries byte-intact; error path prints its real message🤖 Generated with Claude Code
Originally posted by: coderabbitai[bot]
✨ Finishing Touches
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests - [ ] Commit unit tests in branch `fix/review-followups-495-round2`Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
❤️ Share
- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)Comment
@coderabbitai helpto get the list of available commands.Ticket changed by: Akarsh-Hegde