fix web routing for explicitly selected skills - #7108
Conversation
|
Thanks @truman-t3 — this is a nicely scoped routing fix, and the regression coverage plus end-to-end verification in the body are helpful. Since the PR is still marked draft, I'll hold off on a full review for now and let the requested reviewer take it from here once it's ready for review. |
lefarcen
added
size/S
lefarcen
added
the
needs-validation
|
🧪 This PR has changes that need a manual QA pass before merge — please hold off self-merging for now; we'll loop QA in once it's merge-ready (and design/product have signed off, where applicable). |
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
@truman-t3 thanks for the clean, well-scoped routing fix — this is a nice catch from the industrial-product-design end-to-end path.
What I verified
- In
HomeViewsubmit,routedPluginIdnow prefers an explicit scenario plugin, then only falls back toDEFAULT_UNSELECTED_SCENARIO_PLUGIN_IDin Design mode when no skill is resolved. Explicit skill picks therefore sendskillIdwithpluginId: null, which matches the mutual-exclusivity comment above the change. - Free-form Design submits still keep the hidden default router (
routes free-form submits through the hidden default plugin…remains intact). - The new regression in
HomeView.prefill.test.tsxexercises the skill-use handoff path (createSkillUseHandoff→useSkillclears active plugin, seeds the example prompt) and assertspluginId: null+ the expectedskillId. useSkillalready clearsactive, so the submit path cannot double-route plugin + skill from a normal skill pick.
No correctness, safety, or test gaps stood out in the changed ranges. Appreciate the red-spec style coverage and the clear PR write-up.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.
Visual regression reviewHead: 0 changed · 49 unchanged · 0 new without baseline · 0 failed Unchanged cases
Visual diff is advisory only and does not block merging. |
mrcfps
left a comment
mrcfps
left a comment
There was a problem hiding this comment.
@truman-t3 thanks for the clean follow-up on explicit skill routing — the extra context-picker expectation updates make the mutual-exclusivity contract much clearer.
What I verified on 03aba09a
- In
HomeViewsubmit,routedPluginIdprefers an explicit scenario plugin, then only falls back toDEFAULT_UNSELECTED_SCENARIO_PLUGIN_IDin Design mode when no skill is resolved. Explicit skill picks therefore sendskillIdwithpluginId: null, matching the mutual-exclusivity comment above the change. - Free-form Design submits still keep the hidden default router (
routes free-form submits through the hidden default plugin…remains intact). - The new regression in
HomeView.prefill.test.tsxcovers the skill-use handoff path (createSkillUseHandoff→useSkillclears active plugin, seeds the example prompt) and assertspluginId: null+ the expectedskillId. HomeView.context-picker.test.tsxnow expectspluginId: nullfor skill picks (including after clearing a type chip), which aligns with the production fix rather than the old default-plugin leakage.useSkillalready clearsactive, so the submit path cannot double-route plugin + skill from a normal skill pick.
No correctness, safety, or test gaps stood out in the changed ranges. Appreciate the red-spec style coverage and the tidy test alignment commit.
🔁 Powered by Looper · runner=reviewer · agent=opencode · An autonomous AI dev team for your GitHub repos.
4 participants
Related to #6024.
Why
Selecting a Skill from the Home composer in Design mode should route the new project through that Skill alone. Instead, Home also attached the hidden
od-defaultplugin whenever no visible scenario plugin was selected.That produced conflicting creation metadata: the request carried the explicitly selected
skillId, but it also carriedpluginId: od-default. The default design router could then take precedence over the workflow the user deliberately selected.This surfaced while validating an imported industrial-product-design Skill end to end from Home.
What users will see
When a user explicitly selects a Skill on Home and starts a Design-mode project, Open Design now preserves that Skill as the only routing source. Free-form Design-mode prompts with no Skill still use the hidden default design router, and explicitly selected scenario plugins are unchanged.
Surface area
apps/weborapps/desktop(including Electron menu bar)odsubcommand or flag, newtools-dev/tools-packflag, or newOD_*env var/api/*endpoint, new SSE event, or changed shape inpackages/contractsskills/,design-systems/,design-templates/, orcraft/, or change to the skills protocolpackage.jsonScreenshots
No visual layout changes. End-to-end verification used the existing Home
@mention picker and confirmed that selectingIndustrial Product Designcleared the preselected Slide deck route before submission.Bug fix verification
apps/web/tests/components/HomeView.prefill.test.tsxpluginId: od-defaultinstead ofnull.skillId: industrial-product-design; the completed run recordedplugin_id: null, copied the Skill and both reference files into.od-skills, and returned the Skill-defined industrial-design clarification form.Validation
succeeded,skill_id: industrial-product-design,plugin_id: null.mainat4eacda5751ecdb36d41bb0b8c051b379da545ca1.git diff --check origin/main...HEAD: passed.pnpm guard,pnpm typecheck, and the complete package-scoped web test file on the latest baseline. The local partial clone could not hydrate the remaining web sources because repeated GitHub promisor transfers stalled or reset.