fix(claude): honor Desktop native model opt-out - #1363
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughClaude Desktop configuration now receives desktop-specific visible native slugs. A regression test verifies that disabling native models produces an empty slug list while enablement succeeds. ChangesClaude Desktop native model filtering
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
✅ Deterministic PR hygiene checks passed. |
✅ READY
Review readiness checklist
✅ 4/4 boxes ticked. This pull request is already Ready for Review. |
|
Please rebase this PR onto the latest |
983088d to
03465c6
Compare
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
expected_dev="e8ce2b93dd3ef8074d96dee407704c5dc06e2699"
head="$(git rev-parse HEAD)"
printf 'HEAD=%s\n' "$head"
printf 'EXPECTED_DEV=%s\n' "$expected_dev"
if git cat-file -e "${expected_dev}^{commit}" 2>/dev/null; then
if git merge-base --is-ancestor "$expected_dev" HEAD; then
echo "EXPECTED_DEV_IS_ANCESTOR=true"
else
echo "EXPECTED_DEV_IS_ANCESTOR=false"
fi
else
echo "EXPECTED_DEV_AVAILABLE=false"
fi
printf '\nChanged files against first parent:\n'
git diff --stat HEAD^ HEAD
git diff --name-status HEAD^ HEAD
printf '\nSource outlines:\n'
ast-grep outline src/server/management/native-integration-routes.ts --items all 2>/dev/null || true
ast-grep outline tests/native-claude-desktop-toggle.test.ts --items all 2>/dev/null || true
printf '\nChanged hunks:\n'
git diff --unified=40 HEAD^ HEAD -- \
src/server/management/native-integration-routes.ts \
tests/native-claude-desktop-toggle.test.tsLength of output: 17574 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' 'Catalog helper definitions and call sites:'
rg -n -A18 -B8 --type ts \
'\bdesktopVisibleNativeSlugs\b|\bvisibleNativeSlugs\b' \
src tests
printf '%s\n' 'Test helper implementations:'
ast-grep outline tests/native-claude-desktop-toggle.test.ts --items all --type function
sed -n '1,70p' tests/native-claude-desktop-toggle.test.ts
printf '%s\n' 'Claude Desktop toggle implementation:'
sed -n '600,675p' src/server/management/native-integration-routes.ts
printf '%s\n' 'Catalog export implementation candidates:'
fd -t f -e ts . src/codex | while IFS= read -r file; do
if rg -q '\bdesktopVisibleNativeSlugs\b' "$file"; then
echo "--- $file"
rg -n -A24 -B12 '\bdesktopVisibleNativeSlugs\b' "$file"
fi
doneLength of output: 106107
I found no issue in the reviewed change. You are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo="lidge-jun/opencodex"
pr_number="1363"
required_dev_sha="e8ce2b93dd3ef8074d96dee407704c5dc06e2699"
echo "== PR metadata =="
gh pr view "$pr_number" --repo "$repo" \
--json number,url,state,isDraft,headRefName,headRefOid,baseRefName,baseRefOid,mergeable,mergeStateStatus,statusCheckRollup \
--jq '{
number, url, state, isDraft, headRefName, headRefOid, baseRefName, baseRefOid,
mergeable, mergeStateStatus,
checks: [.statusCheckRollup[] | {name, status, conclusion, detailsUrl}]
}'
head_sha="$(gh pr view "$pr_number" --repo "$repo" --json headRefOid --jq '.headRefOid')"
echo
echo "== Required dev commit ancestry =="
gh api "repos/$repo/compare/$required_dev_sha...$head_sha" \
--jq '{status, ahead_by, behind_by, merge_base_commit: .merge_base_commit.sha}'
echo
echo "== Exact-head check runs =="
gh api --paginate "repos/$repo/commits/$head_sha/check-runs?per_page=100" \
--jq '.check_runs[] | {name, status, conclusion, details_url}'Length of output: 12549
The exact-head CI requirement is not complete yet. The Wait for You are interacting with an AI system. |
|
Thanks @luvs01 — good catch and a clean fix. Reusing the Desktop-specific visibility helper keeps the native-model opt-out consistent across the integration surfaces, with a focused regression to lock it in. 🙏 |
2 participants
Summary
claudeCode.desktopNativeModels: falseopt-out.Previously this one route used the generic native model list, so enabling Claude Desktop could re-add native entries that the user had explicitly disabled for Desktop.
Verification
bun test tests/native-claude-desktop-toggle.test.ts— 10 passed, 0 failed on Bun 1.3.14.bun run typecheckbun run privacy:scangit diff --checkChecklist
Review readiness checklist
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
Bug Fixes
Tests