feat: add OpenUI integration#8218
Conversation
Review: PR #8218 - feat: add OpenUI integrationSummaryThis is a substantial feature PR that adds experimental OpenUI integration to Jan. When enabled, the assistant can describe UI elements in OpenUI Lang, and Jan renders them as interactive components (buttons, forms, etc.) instead of plain markdown. User interactions with these components are routed back into the conversation. Architecture OverviewThe implementation follows a clean layered approach:
Detailed FindingsStrengths:
Concerns:
Missing
VerdictThis is a well-architected feature with good code separation and lazy loading. However, it introduces a large dependency footprint and has security considerations around unsanitized model-generated UI that need attention before merging. The lack of i18n on the settings page is inconsistent with the rest of the codebase. Recommendation: fix needed Priority action items:
|
7908d7f to
1985c8f
Compare
|
Thanks for the review, I will refactor the pr and push the changes soon. |
80f761b to
143da66
Compare
Re-review: PR #8218 - feat: add OpenUI integrationUpdated review after the author's changes (commit What improved since the last reviewThe author addressed several of the original concerns:
Remaining concerns1. // Keep ref in sync with latest closure so the OpenUI event listener
// (registered once with []) always calls the current handleSendMessage.
const handleSendMessageRef = useRef(handleSendMessage)
useEffect(() => { handleSendMessageRef.current = handleSendMessage })2. System prompt is appended to every chat when enabled globally. 3. Bundle size from
4. No XSS sanitization within the OpenUI renderer itself. 5. 6. Zustand peer dependency override in Test coverage assessmentThe PR includes solid test coverage:
Missing: No e2e tests for the full enable-to-interact flow, but this is understandable for an experimental feature. ArchitectureThe layered approach is clean and well-separated:
The two library modes (Chat vs Standard) are well-isolated: Chat uses the locally defined VerdictThe author has addressed the most critical issues from the first review (URL sanitization, i18n, detection accuracy, documentation). The remaining items are either minor (missing comment on ref pattern), known trade-offs (bundle size, global prompt injection), or require upstream verification (XSS in Renderer). Recommendation: improve needed Minimum before merge:
Nice-to-have for follow-up: |
143da66 to
41d305b
Compare
|
Updated the PR to make OpenUI opt-in per chat instead of global. The OpenUI prompt is still included on each model request where OpenUI is enabled, because the model needs those instructions in-context to reliably emit OpenUI Lang. However, it is now scoped to the active chat only:
Validation run locally:
|
Follow-up Review (new commit since Jun 6)Reviewing commit: What ChangedThe author addressed the highest-priority concern from the previous review: OpenUI is no longer a global toggle.
This directly resolves concern #2 from the last review. Remaining Items1. 2. XSS audit of 3. What Looks Good
Recommendation: improve needed (minor)Required before merge:
Nice-to-have: |
- Update docs to describe enabling OpenUI via the Tools menu switch instead of the removed composer button - Document that HTML-safety relies on the upstream @openuidev/react-lang renderer's internal sanitization - Remove orphaned noToolsAvailable key from all 17 locales - Point the settings-page enable description at the per-chat Tools menu toggle in all 17 locales Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Follow-up Review (new commits detected since last review) Reviewing commits What Changed Since Last ReviewThree new commits were pushed on June 10 -- all follow-on polish to the per-chat activation refactor reviewed on June 9:
Previous Required Items -- Status
Both minimum-before-merge items from the last review have been addressed. Review of New ChangesUX improvement: Consolidating the OpenUI toggle into the Tools menu is a cleaner design -- fewer icons in the composer bar, one consistent place for per-chat tool options. The active-state styling (
Locale cleanup: Removing
Minor observation -- Remaining Nice-to-Haves (not blocking)
Recommendation: can mergeAll minimum-before-merge items from the previous reviews have been addressed:
The refactor to move OpenUI activation into the Tools menu is a sound UX decision. The code is clean, tests pass locally per the author, and the PR is well-scoped. The remaining items are appropriate for follow-up issues rather than blocking this merge. Suggested before merging: a quick manual smoke test confirming the tools button renders sensibly in agent mode, since its visibility condition changed (it is now always shown rather than gated on MCP availability). |
qnixsynapse
left a comment
There was a problem hiding this comment.
This seems like adding an entire new third party library. Need to decide on whether we want to include it. I am moving it to 0.8.5 release.
Follow-up — branch update notedThanks for keeping the branch up to date with Picking up from the previous review: the outstanding item is a manual smoke test confirming the tools/OpenUI button renders correctly in agent mode — specifically that the No other blocking issues from the diff. Once that's confirmed, this looks ready. Recommendation: improve needed (pending confirmation of manual smoke test) |
Summary
Adds an experimental OpenUI integration to Jan.
What Changed
Validation
yarn workspace @janhq/web-app lint yarn workspace @janhq/web-app exec vitest --run yarn workspace @janhq/web-app buildAll passed.
closes #8214