Expose provider/model picker to plugin apps - #1469
Conversation
…del-picker-to-plugin-apps-thr_fy2hvyjf8s # Conflicts: # packages/templates/src/generated/plugin-sdk-dts.generated.ts
|
🚨 SLOP COP 🚨 · I am SlopCop. I am reviewing this pull request for security, code quality, performance, architecture, and end-to-end behavior. |
SawyerHood
left a comment
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
ELI5: This PR lets plugin screens use bb's provider and model menu. One path can save a model before bb confirms it.
Finding
P2: A user can select an unverified Claude model from a provider preview.
Claude can show cached placeholder rows while the verified catalog loads. The automatic provider path correctly waits for verified data. A direct row selection does not wait and calls the plugin's onChange. This behavior breaks the documented SDK guarantee for coherent, verified pairs.
I left an inline comment with the exact path and a test case.
Performance and architecture
- The disabled shortcut mode still adds three global command handlers for each plugin picker.
- A provider preview creates two query observers for the same key. TanStack Query shares the network request.
- The adapter repeats model option conversion from the shared picker and the built-in selection hook.
- The shared picker renders all visible model rows and updates shared hover state. This design existed before this PR.
These items are not merge blockers for this change. A shared catalog projection helper can prevent future behavior differences.
Checks
- The security review found no issue.
- Turbo type checks passed for the app, plugin SDK, plugin build, and templates.
- Local Turbo tests passed for all four packages. The app ran 2,640 tests.
- Browser checks passed for desktop selection and the 390-pixel compact drawer.
- All current GitHub checks passed.
I cannot recommend a merge until the placeholder selection path uses verified catalog data.
…del-picker-to-plugin-apps-thr_fy2hvyjf8s # Conflicts: # apps/app/src/components/pickers/ModelReasoningPicker.tsx # packages/templates/src/generated/plugin-sdk-dts.generated.ts # packages/templates/src/generated/templates.generated.ts
…del-picker-to-plugin-apps-thr_fy2hvyjf8s
…del-picker-to-plugin-apps-thr_fy2hvyjf8s # Conflicts: # apps/server/src/services/skills/builtin-skills/bb-cli/SKILL.md # apps/server/src/services/skills/builtin-skills/bb-plugin-authoring/SKILL.md # packages/domain/src/plugin-sdk-version.ts # packages/plugin-sdk/package.json # packages/templates/src/generated/plugin-sdk-dts.generated.ts # packages/templates/src/generated/templates.generated.ts # packages/templates/src/templates/bb-guide-plugins.md # packages/templates/test/plugin-scaffold-external.test.ts
Summary
experimental_ProviderModelPickerfrom@bb/plugin-sdk/appas a controlled{ providerId, model }host capability with optional host routingVerification
origin/maininto the standalone feature branch and resolved the generated SDK/template conflicts without discarding feature work@bb/app,@bb/plugin-sdk,@bb/plugin-build, and@bb/templates: 2,711 app tests and 23 template tests passed; all 11 Turbo tasks succeededCORRECT; the placeholder-catalog finding is fixed and its review thread is resolved7be2c411f4f84863f64e2ad2c0e88f0deafc53d5(9 successful, 1 intentionally skipped)BB-Thread-ID: thr_fy2hvyjf8s