fix(cli): apply client selector to registry integrations (#1798) - #1802
Conversation
Signed-off-by: Rares Popa <2606875+rarepops@users.noreply.github.com>
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
|
Thank you for the focused selector fix and regression coverage. I checked current I have labeled this as a high-priority editor-integration bug and queued it for review. CI is green. Our review queue is full, so detailed review may take a little time, but the PR is now classified and routed. Thank you for preserving the selector's restrictive semantics rather than force-enabling undetected clients. |
|
Approved on merit. I verified the gap on That matters more than a missing filter usually would, because it fails in the configuring direction. A user writing Two things I checked and want to credit: The polarity is right. Both new guards read You kept the restrictive semantics explicit: selected and detected. Not force-enabling an undetected client is the right call, and stating it in the description saved me from having to derive it from the guards. And a nice interaction you may not have noticed: adding registry Before mergeYou are 205 commits behind Nothing to change in the code. Thank you — this is the third careful PR of yours I have read today, and closing the |
|
Merged as The verification route is worth a note, since your branch was 205 commits behind and the green was stale. Our Actions pool has been servicing roughly one job at a time against a 38-deep queue, so rebasing would have cost you hours and everyone else a slot. Instead I built the actual merge result locally — current On the fix: what made it easy to accept was the polarity. |
What does this PR do?
Fixes #1798.
--clientspreviously filtered only the legacy client table, so registry-backed clients such as Qoder and Rovo Dev were still detected and configured. This change makes registry stable IDs part of the selector vocabulary and applies the same restriction to registry detection output and installation.The selector remains restrictive: it configures only selected clients that are detected, and does not force-enable undetected clients.
Validation covered the sanitizer build and both selector regressions. The full CLI suite passes with 283 tests. clang-format 20.1.8 and
git diff --checkpass; focused cppcheck reports only pre-existing style findings in test helpers and older test code.Checklist
git commit -s) - required, CI rejects unsigned commits (DCO, see CONTRIBUTING.md)cli: 283 passed)make -f Makefile.cbm lint-ci) - upstreamlint / lintandlint / lint-memchecks pass