feat: Vue→React migration + reusable jcode-ui component library - #122
Conversation
Two-package monorepo for the reusable AI chat UI: - jcode-ui-core: framework-agnostic types (Message/ToolCall/Approval/ThreadItem), ChatRuntime abstraction + ExternalStoreRuntime (wraps any Redux-shaped store), MockRuntime (for demos/tests), ToolRendererRegistry (plugin seam), and headless React primitives (Thread with virtualization + auto-follow, MessageView, Composer, ToolCallView, ApprovalBlock, AskUserBlock). - jcode-ui: styled components wrapping the primitives with token-driven Tailwind 4 styling, the marked+highlight.js+DOMPurify markdown pipeline, and 9 default tool renderers (terminal/file-viewer/diff/search/todo/skill/team/browser-shot/generic). Both packages typecheck and build clean. Core dist + CSS bundle verified. Root pnpm-workspace.yaml added; .gitignore updated for node_modules/ and dist/.
- ChatDemo: scripted mock-runtime playground component (the 'website footprint') that streams a full conversation through message/tool/approval item kinds. - /chat-ui page: hero, live demo, feature grid, quick-start code sample. - site/docs/chat-ui/: runtime, primitives, tool-renderers, theming pages. - SiteNav + routing wired. Site typechecks and builds clean (ChatUIPage bundles at 350KB gzip — highlight.js + marked dominate, expected for a live demo). - jcode-ui core dep switched to file: so the isolated site workspace resolves it.
…ct shell Replaces the Vue app's runtime layer + shell with a React equivalent that consumes the jcode-ui component library: - lib/ : framework-agnostic ports (api.ts 384-line client, apiBase.ts dual-host contract, authToken.ts, useDesktop.ts Tauri bridge, ws.ts singleton client, types.ts full backend contract). Nearly verbatim from web/src/composables. - app/store.ts : RTK store split across 4 slices (chat/session/model/ui) with async thunks (sendMessage/stopAgent/resolveApproval/submitAskUser/editMessage). - app/runtime.ts : createExternalStoreRuntime adapter — the single seam between RTK and jcode-ui's RuntimeState. - app/wsBridge.ts : WS events → Redux dispatches (replaces Vue App.vue coupling). - components/ : product shell (Sidebar, ChatView, ProjectHeader, GoalBanner, AutomationsView, ChannelsView, CommandPalette, AuthGate, SetupView). typechecks + builds clean (626 modules, 421KB gzip bundle — code-splitting is a follow-up). Splits the 1.2k-line Vue chat store into focused slices per the migration assessment. The dual-host (browser/Tauri) contract is preserved.
- Makefile: add build-web-react (builds core → jcode-ui → CSS → web-react →
dist-react), FRONTEND var to select web/web-react, lint-react target.
Tolerates pnpm ERR_PNPM_IGNORED_BUILDS (esbuild/@parcel/watcher) so the build
chain is non-fragile.
- pnpm-workspace.yaml: onlyBuiltDependencies for esbuild + @parcel/watcher.
- .npmrc: auto-install-peers for the monorepo.
- AGENTS.md: document the Vue→React migration, web-react/, packages/, and the
build-web-react target; mark web/ as production-during-migration.
- packages/jcode-ui{,-core}/: README.md, LICENSE, .npmignore, publishConfig
(access public), files array — npm pack dry-run verified (styles.css + dist
included, src excluded).
Verified: make build-web-react end-to-end green; all 4 TS projects typecheck
(jcode-ui-core, jcode-ui, web-react, site); Go backend builds (embed intact);
Tauri frontendDist still points at the Vue dist (React is parallel).
|
Warning Review limit reached
Next review available in: 37 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR establishes a pnpm monorepo for a Vue→React migration, adding ChangesMonorepo and build tooling
jcode-ui-core package
jcode-ui package
web-react application
Documentation site and demo
Generated build output
Tool-search architecture draft
Estimated code review effort: 5 (Critical) | ~150 minutes Sequence Diagram(s)sequenceDiagram
participant WSClient
participant wsBridge
participant ReduxStore
participant useChatRuntime
participant ChatView
WSClient->>wsBridge: deliver websocket event
wsBridge->>ReduxStore: dispatch chat/model/session actions
ReduxStore-->>useChatRuntime: updated RootState
useChatRuntime-->>ChatView: RuntimeState via ChatRuntime
Possibly related PRs
🚥 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 |
cnjack
left a comment
cnjack
left a comment
There was a problem hiding this comment.
Reviewed as a full subsystem sweep (core runtime, styled components/markdown pipeline, WS/auth/store layer, product shell, build chain) since this is a large scaffold PR (107 files, ~12.8k lines). Vue stays production and nothing in the build chain switches the served frontend, which is good — but the new code itself has two critical, likely-blocking defects plus several real correctness/reliability gaps versus the Vue original it's porting from. Inline comments mark the 8 most load-bearing issues with exact fixes; full list below.
Overall Risk: High
Not high because of scope — high because of two defects that mean the new library/app doesn't actually work as shipped (infinite render loop, XSS), plus a packaging bug that would break the published npm package on the first npm install. None of this is caught by CI today (see the CI finding below) since the new workspace isn't wired into .github/workflows.
Top Findings
- Infinite re-render loop in
useRuntimeState/useRuntimeSelector(packages/jcode-ui-core/src/runtime/context.tsx:93, andexternalStore.ts) —getSnapshotcallsnormalizeState(runtime.getState()), which allocates a new object every call, violatinguseSyncExternalStore's "stable snapshot while unchanged" contract. This affects every component underRuntimeProvider— i.e., essentially the whole chat UI (Thread,Composer, etc.). This is a foundational break, not an edge case. - Stored XSS via unsanitized
dangerouslySetInnerHTML(packages/jcode-ui/src/components/ToolCallCard.tsx:78) —tool.displayInfo?.subtitle(LLM/tool-arg-derived content) is injected as raw HTML with noDOMPurifypass, unlike every otherdangerouslySetInnerHTMLuse in the package. The headless sibling component renders the same value safely as escaped JSX text — this wrapper diverges and reopens the hole. - Auth/setup gate ordering inverted (
web-react/src/App.tsx:107-110) —needsSetupis checked beforeneedsAuth, but/api/setup/*is itself auth-protected (per the Vue original's explicit comment on this exact ordering requirement). A server requiring both drops users into a broken, unusableSetupViewinstead of the login screen. sendMessagethunk has no error handling (store.ts:381-399) — a failedapi.chat()call leavesisRunning: trueforever with no recovery path and no user-visible error.editMessagesilently drops history-truncation and timeline-splice logic (store.ts:431-438) — resends without truncating backend history (agent still sees the stale tail) and wipes the entire frontend timeline instead of just the edited tail.jcode-ui's dependency onjcode-ui-coreusesfile:../jcode-ui-coreinstead ofworkspace:*(packages/jcode-ui/package.json:69) —pnpm publishdoesn't rewritefile:specifiers, so the published npm package would ship an unresolvable dependency path, breaking installs for every external consumer.- WS client zombie-socket under React StrictMode (
web-react/src/lib/ws.ts:132) —disconnect()doesn't neutralize the asynconclosehandler, so StrictMode's dev double-mount produces a duplicate reconnecting client that double-dispatches WS events into the shared store. resolveApprovaldrops the store-level re-entrancy guard (store.ts:406-417) — repeated clicks can now fire duplicate POSTs (only a UIdisabledprop guards it, which is timing-dependent); failure is also silent (no user-visible error), unlike the Vue original.task_idecho for approval/ask-user resolution dropped, and no pending-gate recovery after reload/reconnect (app/wsBridge.ts,store.ts) — with concurrent tasks this can resolve/misroute the wrong task's gate; a reload while a gate is pending leaves the agent blocked with no way to unblock from the UI (the already-portedapprovalPending/askPendingendpoints are never called).- DOMPurify
ADD_ATTR: ['target']without forcingrel="noopener noreferrer"(packages/jcode-ui/src/lib/markdown.ts:34) — reverse-tabnabbing viawindow.openerfrom untrusted markdown links.
Additional lower-severity findings (not inline, listed for completeness)
session.wsConnectedandmodel.autoApprovereducers exist but are never dispatched from anywhere — the connection indicator is permanently stuck and any UI bound toautoApproveshows a stale value.SetupView's provider→models fetch has no stale-response guard; rapid provider switching can submit a model belonging to the wrong provider.AutomationsView/ChannelsViewswallow fetch errors (.catch(() => {})) and render the same copy for "empty" and "failed to load" — no way to distinguish a real backend outage from an empty state.internal/web/dist-react/(committed Vite build output, ~1.3MB minified JS) has no.gitignoreentry unlike the Vuedist/, and isn't referenced by the Go embed anywhere — dead, growing weight in git history that looks like an accidentalgit add -A.- The new workspace (
packages/jcode-ui-core,packages/jcode-ui,web-react) has zero CI coverage —.github/workflows/ci.ymlis unchanged, so the newlint-react/build-web-reactMake targets are never invoked automatically and this code can silently bitrot. useIsAtBottom(jcode-ui-core/src/hooks/index.ts) doesn't match its own docstring ("re-renders when the flag flips") — it never tracks/returns the flag, so any future consumer relying on the documented contract gets a silent no-op.- Thread virtualization: the auto-follow scroll effect can race TanStack Virtual's async row remeasurement during rapid streaming (under-scroll/jank), and the pending-row/overscan spacer aren't measured, so
getTotalSize()can disagree with real DOM height.
Nothing found that changes what's served in production today — Vue remains the default build/embed target, and desktop (Tauri) config is untouched. The concerns above are all in the new, not-yet-switched-on code paths, which is the right time to fix them before this becomes the production surface.
(Note: submitted as COMMENT rather than "Request changes" — GitHub doesn't allow requesting changes on one's own PR — but findings #1, #2, and #6 above are blocking-severity and should be treated as such.)
Generated by Claude Code
| const subscribe = runtime.subscribe | ||
| const getSnapshot = () => normalizeState(runtime.getState()) | ||
| // Prime the cache on first read / after a store change useSyncExternalStore detected. | ||
| const snapshot = useSyncExternalStore(subscribe, getSnapshot, getSnapshot) |
There was a problem hiding this comment.
Critical — getSnapshot violates the useSyncExternalStore contract, causing an infinite re-render loop.
getSnapshot here is () => normalizeState(runtime.getState()), and normalizeState (runtime/index.ts) unconditionally builds a brand-new object literal on every call. React requires repeated getSnapshot() calls to return the same value (by Object.is) while the store hasn't changed. Since this never stabilizes, React's tearing-detection re-check after commit will always see a "changed" snapshot and force another re-render, which calls getSnapshot again, which again differs — an unconditional infinite loop (or at minimum React's dev-mode "getSnapshot should be cached" warning firing on every render) for every component under RuntimeProvider (Thread, Composer, etc. — essentially the whole chat UI).
This also affects createExternalStoreRuntime.getState (externalStore.ts), which has the same normalizeState(select(...)) pattern.
Fix: memoize getSnapshot's result — only recompute normalizeState(...) when the underlying runtime.getState()/host store reference actually changes, caching the last result otherwise (the standard pattern used by use-sync-external-store/with-selector / react-redux).
Generated by Claude Code
| </span> | ||
| <span className="truncate text-[var(--color-foreground)]">{title}</span> | ||
| {subtitle && ( | ||
| <span className="truncate text-[var(--color-muted-foreground)]" dangerouslySetInnerHTML={{ __html: subtitle }} /> |
There was a problem hiding this comment.
Critical — stored/DOM XSS via unsanitized dangerouslySetInnerHTML.
tool.displayInfo?.subtitle is documented (jcode-ui-core/src/types/index.ts) as "surfaced from the backend or extracted client-side from args" — i.e. it can contain LLM/tool-controlled strings (file paths, shell commands, grep patterns). Rendering it via dangerouslySetInnerHTML with no DOMPurify.sanitize() (unlike every other dangerouslySetInnerHTML use in this package, which goes through renderMarkdown()) is a direct XSS: a subtitle containing '<img src=x onerror=...>' executes in the app's origin.
Note the sibling headless component ToolCallView.tsx's DefaultToolHeader renders the exact same value safely as {subtitle} (JSX auto-escaping) — this wrapper diverges and reintroduces the hole, seemingly unintentionally.
Fix: render as plain text (<span>{subtitle}</span>), or if rich formatting is required, pipe through renderMarkdown() first.
Generated by Claude Code
| if (connectionError) { | ||
| return <ErrorScreen message={connectionError} /> | ||
| } | ||
| if (needsSetup) { |
There was a problem hiding this comment.
High — auth/setup gate ordering is inverted relative to the Vue original, and a stuck/broken state results.
needsSetup is checked (and <SetupView/> rendered) before needsAuth. But /api/setup/* endpoints are themselves auth-protected — the Vue original (web/src/App.vue) explicitly checks auth first with the comment "Auth gate must run BEFORE the setup gate: /api/setup/* is itself protected, so without a valid token the wizard's own calls would 401." On a server requiring both auth and first-run setup, a user now lands on a broken SetupView (its provider-list fetch 401s silently) with no way to log in, instead of the login screen.
Separately: the loadSessions/loadTasks/loadSlashCommands effect below (line ~98) fires unconditionally on mount, not gated on boot() resolving or on these auth/setup checks passing — protected data requests go out before we know whether the user is even authenticated, and (combined with needsAuth/needsSetup defaulting to false) the authenticated Shell can flash briefly before boot resolves.
Fix: swap the two gate checks (auth before setup), and gate the sidebar-data-loading effect on boot completion + gates having passed.
Generated by Claude Code
| } | ||
| dispatch(chatActions.addMessage({ role: 'user', content: payload.text, images: payload.images })) | ||
| dispatch(chatActions.setRunning(true)) | ||
| const resp = await api.chat(payload.text, payload.mode, sessionId, payload.images) |
There was a problem hiding this comment.
High — sendMessage has no error handling: a failed request leaves the UI permanently "running".
There's no try/catch around api.chat(...). If it rejects (network drop, 5xx, expired token), chat.isRunning stays true forever with no recovery short of a full reload, and the user sees no error. The Vue original wraps the equivalent call in try/catch and resets isRunning + surfaces the error as a system message on failure.
Fix: wrap in try/catch, dispatch setRunning(false) and an error message on failure.
Generated by Claude Code
| }, | ||
| ) | ||
|
|
||
| export const editMessage = createAsyncThunk( |
There was a problem hiding this comment.
Medium/High — editMessage doesn't truncate backend history and wipes the entire frontend timeline, not just the edited tail.
payload.id is accepted but never used. This dispatches clearChat() (wiping the whole UI timeline, not just messages after the edit point) and resends into the same backend session without calling truncateHistory — so the backend agent still sees the stale original conversation tail alongside the new prompt. The Vue original (editAndResend in web/src/stores/chat.ts) locates the message by id, calls api.truncateHistory(...) server-side, and only splices the frontend timeline from that index onward.
Fix: port the original logic — locate the message index, truncate backend history via the existing api.truncateHistory, splice (not clear) the frontend timeline from that point, then resend.
Generated by Claude Code
| this.send({ type: 'approval', data: { id, approved, approve_all: approveAll, task_id: taskId } }) | ||
| } | ||
|
|
||
| disconnect(): void { |
There was a problem hiding this comment.
Medium — disconnect() doesn't neutralize the async onclose handler, producing a zombie socket under React StrictMode's double-mount.
disconnect() closes the socket and clears local fields, but doesn't null this.ws.onclose. The browser's close event still fires asynchronously afterward, running the original onclose (line ~111), which re-arms a 3s reconnect on the now-orphaned client — one still wired to the shared Redux store via bridgeWS. Since App.tsx's WS effect creates/tears down a WSClient on every effect run, and main.tsx wraps the app in <StrictMode> (mount→cleanup→remount in dev), this zombie client reconnects ~3s after mount and double-dispatches every WS event alongside the real client, duplicating messages/tool calls in dev.
This latent pattern also exists in the Vue original, but Vue components aren't double-invoked the way StrictMode exercises this in React.
Fix: add a destroyed flag checked in onclose before scheduling a reconnect, and/or null out ws.onclose/onmessage/onerror in disconnect() before calling ws.close().
Generated by Claude Code
| "@tailwindcss/typography": "^0.5.16", | ||
| "dompurify": "^3.2.4", | ||
| "highlight.js": "^11.11.1", | ||
| "jcode-ui-core": "file:../jcode-ui-core", |
There was a problem hiding this comment.
Medium (packaging) — file: specifier will publish a broken dependency reference.
web-react/package.json correctly uses "jcode-ui-core": "workspace:*", which pnpm publish rewrites to a concrete semver range. This package instead uses "file:../jcode-ui-core", which pnpm/npm publish verbatim — it is not rewritten. Since the PR's stated goal is to publish jcode-ui to npm (publishConfig.access: "public" is already set), the published package.json would reference a local path that doesn't exist for any external consumer, breaking npm install jcode-ui entirely.
Fix: change to "jcode-ui-core": "workspace:*" (or "workspace:^").
Generated by Claude Code
| export function renderMarkdown(text: string): string { | ||
| const raw = marked.parse(text) as string | ||
| return DOMPurify.sanitize(raw, { | ||
| ADD_ATTR: ['target'], |
There was a problem hiding this comment.
Low/Medium — ADD_ATTR: ['target'] without forcing rel="noopener noreferrer" re-enables reverse-tabnabbing.
Allowing target on sanitized anchors (reachable via raw <a target="_blank"> HTML that marked passes through) without also forcing rel="noopener noreferrer" lets a link opened in a new tab use window.opener to redirect the parent page — a phishing vector exploitable from untrusted markdown (tool output, rendered file content, etc.).
Fix: add a DOMPurify afterSanitizeAttributes hook that force-sets rel="noopener noreferrer" whenever target is present on <a> (the standard DOMPurify recipe), or drop ADD_ATTR: ['target'] if not needed.
Generated by Claude Code
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (21)
pnpm-workspace.yaml-13-14 (1)
13-14: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove placeholder
allowBuildsconfig block.Line 14 contains
esbuild: set this to true or false, which is instructional placeholder text mistakenly left as YAML configuration rather than a comment. The actual native-build allowlist is correctly defined below inonlyBuiltDependencies(lines 18–20). pnpm may silently ignore the unknownallowBuildskey, but this is confusing and could break on stricter pnpm versions.🧹 Proposed fix
packages: - 'packages/*' - 'web-react' -allowBuilds: - esbuild: set this to true or false # Native build scripts to allow. esbuild (Vite's bundler) and `@parcel/watcher` # (dev-server file watching) are trusted toolchain deps — whitelisting them # avoids ERR_PNPM_IGNORED_BUILDS failing `pnpm install` (and the Makefile build). onlyBuiltDependencies: - esbuild - '`@parcel/watcher`'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pnpm-workspace.yaml` around lines 13 - 14, Remove the placeholder allowBuilds block from pnpm-workspace.yaml and keep only the real native-build allowlist under onlyBuiltDependencies. The stray instructional entry for esbuild should not remain as YAML configuration, so delete that block entirely rather than converting it into a comment. Use the existing onlyBuiltDependencies section as the authoritative place for build अनुमति entries.packages/jcode-ui/.npmignore-1-4 (1)
1-4: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
srcinfilescontradictssrc/in.npmignore.
package.jsonlists"src"in thefilesarray, but.npmignoreexcludessrc/. The.npmignoredenylist takes precedence over thefilesallowlist, sosrc/will be excluded from the published tarball regardless. This is contradictory — either remove"src"fromfiles(if you only want to shipdist/) or removesrc/from.npmignore(if you intend to publish source for debugging).Given the
exportsfield only points to./dist/, removing"src"fromfilesis likely the correct fix.Proposed fix for package.json files field
"files": [ "dist", - "src", "README.md", "LICENSE" ],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui/.npmignore` around lines 1 - 4, The package publish config is contradictory because .npmignore excludes src/ while package.json still lists src in the files array, so the source will never be included in the tarball. Update the publish settings by removing src from package.json’s files list, since the package’s exports and runtime entrypoints already point to dist/; use the package.json files field and .npmignore together to keep only the intended build output.packages/jcode-ui/src/components/ChatInput.tsx-85-104 (1)
85-104: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a stable key for image attachments instead of array index.
key={i}causes incorrect React reconciliation when an image is removed from the middle of the list — remaining items shift indices and React may reuse the wrong DOM node, producing a brief flash of the previous image. Use a stable identifier from the image data.🐛 Proposed fix
{imgs.map((img, i) => ( - <div key={i} className="relative"> + <div key={img.name ?? `${img.media_type}-${i}`} className="relative">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui/src/components/ChatInput.tsx` around lines 85 - 104, The attachment list in ChatInput’s renderAttachments uses the array index as the React key, which can cause incorrect reuse when an item is removed. Update the mapping to use a stable identifier derived from each image object in imgs instead of key={i}, and keep remove(i) unchanged so deletion still targets the correct item. If the attachment data model does not already expose a unique id, derive one from stable image fields available in this renderAttachments path rather than the item position.packages/jcode-ui/src/styles/components.css-48-52 (1)
48-52: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReplace deprecated
word-break: break-wordwithoverflow-wrap.
word-break: break-wordis deprecated per CSS spec. Useoverflow-wrap: break-word(oranywherefor stricter wrapping) to maintain the same behavior in current and future browsers.🔧 Proposed fix for both occurrences
.jcode-diff-table td { padding: 0 0.5rem; white-space: pre-wrap; - word-break: break-word; + overflow-wrap: break-word; }.jcode-file-table td { padding: 0 0.5rem; white-space: pre-wrap; - word-break: break-word; + overflow-wrap: break-word; }Also applies to: 72-76
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui/src/styles/components.css` around lines 48 - 52, Replace the deprecated word-breaking rule in the stylesheet by updating the .jcode-diff-table td declaration to use overflow-wrap instead of word-break: break-word, and make the same change in the other matching rule referenced in the comment. Keep the existing wrapping behavior intact by using overflow-wrap: break-word (or anywhere if tighter wrapping is desired) and remove the deprecated property from both affected style blocks.Source: Linters/SAST tools
web-react/src/components/Sidebar.tsx-81-95 (1)
81-95: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd
aria-currentto the active session button for screen reader accessibility.The active session is only indicated by visual styling. Screen reader users cannot identify which session is currently selected. Add
aria-current="true"to the active session button.♿ Proposed fix: add aria-current
<button key={s.uuid} type="button" onClick={() => openSession(s)} + aria-current={s.uuid === currentSessionId ? 'true' : undefined} className={`group flex w-full items-center gap-2 rounded-[var(--radius-md)] px-2.5 py-1.5 text-left text-sm transition-colors ${🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-react/src/components/Sidebar.tsx` around lines 81 - 95, The active session button in the sessions.map render is only visually highlighted, so screen readers can’t identify the current selection. Update the button in Sidebar so the one matching currentSessionId also sets aria-current="true" while keeping the existing openSession(s) behavior and visual styles unchanged.web-react/src/app/store.ts-406-417 (1)
406-417: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSurface approval resolution errors to the user.
The
catchblock only clears theresolvingflag but shows no error message. The user can retry but has no idea why the previous attempt failed. This is inconsistent withsubmitAskUser(lines 419–429) which dispatches a system error message on failure.🛡️ Proposed fix: add error message on approval failure
try { await api.approval(payload.id, payload.approved, payload.approveAll ?? false) dispatch(chatActions.resolveApprovalItem({ id: payload.id, approved: payload.approved })) } catch { dispatch(chatActions.setApprovalResolving({ id: payload.id, resolving: false })) + dispatch(chatActions.addMessage({ role: 'system', content: 'Failed to resolve approval', level: 'error' })) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-react/src/app/store.ts` around lines 406 - 417, The resolveApproval thunk currently swallows approval failures by only resetting the resolving state, so surface the error to the user as well. Update the catch path in resolveApproval to dispatch the same kind of system error message used by submitAskUser, while keeping the resolving flag reset and preserving the existing resolveApprovalItem success flow. Use the resolveApproval and submitAskUser async thunks, along with chatActions.setApprovalResolving and the system error dispatch pattern, to locate and mirror the behavior.web-react/src/components/SetupView.tsx-26-35 (1)
26-35: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winSilent error swallowing on provider/model fetch.
Both
catchblocks on lines 27 and 34 discard errors entirely. If the API is unreachable, the user sees empty dropdowns with no indication of what went wrong. Consider setting an error message in state to surface the failure.🛡️ Proposed fix: surface fetch errors
useEffect(() => { - api.setupProviders().then(setProviders).catch(() => {}) + api.setupProviders().then(setProviders).catch((e) => setError(e instanceof Error ? e.message : String(e))) }, []) useEffect(() => { if (!selected) return setModels([]) setModel('') - api.setupProviderModels(selected.id).then(setModels).catch(() => {}) + api.setupProviderModels(selected.id).then(setModels).catch((e) => setError(e instanceof Error ? e.message : String(e))) }, [selected])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-react/src/components/SetupView.tsx` around lines 26 - 35, The fetch logic in SetupView silently swallows failures in both api.setupProviders() and api.setupProviderModels(), leaving the UI empty with no feedback. Update the two useEffect handlers in SetupView to catch the error object, store a user-visible error message in component state, and surface it in the UI instead of using empty catch blocks. Make sure the fix is applied around the setupProviders and setupProviderModels calls so failures are visible when loading providers or models.web-react/src/components/Sidebar.tsx-28-40 (1)
28-40: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
newChatclears state before confirming session creation succeeds.
clearChat()andsetCurrentSession('')are dispatched synchronously before the asyncapi.newSession()call. If the API fails, the user is left with a blank chat and no active session with no visible error. Consider creating the session first, then clearing once successful.🛡️ Proposed fix: create session before clearing
async function newChat() { - dispatch(chatActions.clearChat()) - dispatch(sessionActions.setCurrentSession('')) - dispatch(uiActions.setView('chat')) try { const resp = await api.newSession() + dispatch(chatActions.clearChat()) + dispatch(sessionActions.setCurrentSession(resp.session_id)) + dispatch(uiActions.setView('chat')) const fresh = await api.sessions() dispatch(sessionActions.setSessions(fresh)) } catch { - // surfaced via health/gate + dispatch(chatActions.addMessage({ role: 'system', content: 'Failed to create new session', level: 'error' })) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-react/src/components/Sidebar.tsx` around lines 28 - 40, The newChat flow clears chat and session state before confirming api.newSession() succeeds, which can leave the UI empty if the request fails. Update newChat in Sidebar.tsx to create the session first, then only call chatActions.clearChat(), sessionActions.setCurrentSession(), and uiActions.setView('chat') after the session is successfully created and resp.session_id is available; keep the existing api.sessions() refresh after that.web-react/src/components/ChannelsView.tsx-30-45 (1)
30-45: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winEnable/Disable buttons don't update status after the action.
channelEnableandchannelDisableboth return{ status: string; state: string }but the response is discarded. The status display remains stale after toggling. Updatestatusfrom the API response to reflect the new state immediately.💚 Proposed fix
<button type="button" - onClick={() => api.channelEnable().catch(() => {})} + onClick={() => api.channelEnable().then((r) => setStatus({ available: true, state: r.state })).catch(() => {})} className="rounded-[var(--radius-md)] bg-[var(--color-primary)] px-3 py-1 text-xs text-[var(--color-on-primary)]" > Enable </button> <button type="button" - onClick={() => api.channelDisable().catch(() => {})} + onClick={() => api.channelDisable().then((r) => setStatus({ available: false, state: r.state })).catch(() => {})} className="rounded-[var(--radius-md)] bg-[var(--color-muted)] px-3 py-1 text-xs" > Disable </button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-react/src/components/ChannelsView.tsx` around lines 30 - 45, The Enable/Disable handlers in ChannelsView discard the return value from api.channelEnable and api.channelDisable, so the displayed status never refreshes. Update the onClick logic to await the API response, read the returned status/state from those calls, and set the component’s status from that response so the UI updates immediately after toggling.web-react/src/app/wsBridge.ts-51-59 (1)
51-59: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the unnecessary
as nevercast
AppDispatchis alreadytypeof store.dispatch, sodispatch(sendMessage(...))should typecheck without a cast. The same cast also appears inweb-react/src/App.tsx; remove both instead of bypassing the type system.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-react/src/app/wsBridge.ts` around lines 51 - 59, The unnecessary type cast in onAgentDone is bypassing the existing AppDispatch typing; update the wsBridge.ts send flow so dispatch(sendMessage({ text: next.text, images: next.images })) typechecks without any cast, and remove the same as never cast in App.tsx as well. Use the existing sendMessage action and dispatch/AppDispatch typing to resolve the mismatch rather than suppressing it.web-react/src/components/AuthGate.tsx-27-28 (1)
27-28: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNetwork errors show "Invalid token" misleadingly.
The catch block treats all failures (including network timeouts or server unreachable) as "Invalid token". Consider distinguishing connectivity errors from auth failures to help users troubleshoot.
💡 Suggested improvement
} catch { - setError('Invalid token') + setError('Unable to connect. Check your network and try again.') } finally {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-react/src/components/AuthGate.tsx` around lines 27 - 28, The catch block in AuthGate is treating every failure as an auth failure, so connectivity issues are surfaced as “Invalid token.” Update the error handling in AuthGate to distinguish network/server-unreachable cases from actual token validation failures by inspecting the thrown error in the catch path, and set a different message for connectivity problems while keeping “Invalid token” only for genuine auth errors.web-react/src/components/CommandPalette.tsx-42-70 (1)
42-70: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winModal lacks ARIA attributes and focus management.
The palette is a modal dialog but is missing
role="dialog",aria-modal="true", andaria-label. There is also no focus trap — Tab can escape into background content. Since the comment notes this is a skeleton, consider adding a TODO or tracking this for when the palette is fleshed out.♿ Suggested accessibility improvements
<div className="fixed inset-0 z-[var(--z-modal)] flex items-start justify-center bg-[var(--backdrop)] pt-[15vh]" onClick={() => dispatch(uiActions.setPaletteOpen(false))} + role="dialog" + aria-modal="true" + aria-label="Command palette" >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-react/src/components/CommandPalette.tsx` around lines 42 - 70, The CommandPalette modal is missing required accessibility semantics and focus handling. Update the outer dialog container in CommandPalette to include role="dialog", aria-modal="true", and an accessible aria-label, and add a focus trap so keyboard navigation cannot escape to the page behind it. If this is still a skeleton, leave a TODO in CommandPalette and/or the modal wrapper to track the focus-management work for later.web-react/src/styles.css-3-8 (1)
3-8: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMove
@import 'jcode-ui/styles.css'before@custom-variantto satisfy CSS@importordering.CSS spec requires
@importto precede all other at-rules. The current order (@custom-variantthen@import) triggers a Stylelintno-invalid-position-at-import-ruleerror. Reordering is a no-op functionally —@custom-variantproduces no CSS output by itself.🔧 Proposed fix
`@import` 'tailwindcss'; +@import 'jcode-ui/styles.css'; + `@custom-variant` dark (&:where(.dark, .dark *)); - -/* jcode-ui ships its own tokens + component styles via jcode-ui/styles.css. - We import it here (single import site for the product app). It brings in the - base :root/.dark tokens, animations, and component-local CSS. */ -@import 'jcode-ui/styles.css';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-react/src/styles.css` around lines 3 - 8, Move the jcode-ui/styles.css import in the stylesheet so it comes before the `@custom-variant` dark declaration, since `@import` must be the first at-rule in the file. Update the top of the web-react/src/styles.css entrypoint accordingly, keeping the existing comment and the `@custom-variant` definition in place after the import. This is a ordering-only change in the main CSS entrypoint and should not affect styling output.Source: Linters/SAST tools
packages/jcode-ui-core/src/primitives/Composer.tsx-44-45 (1)
44-45: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRename
maxRowstomaxHeight— the value is pixels, not rows.The prop is documented as "Max textarea height in px" and defaults to
160(px), but the namemaxRowsstrongly implies a row count. A consumer could reasonably pass5expecting five text rows and instead get a 5px-tall textarea.♻️ Proposed rename
export interface ComposerProps extends ComposerRenderSlots { /** Placeholder text. */ placeholder?: string - /** Max textarea height in px before it scrolls internally. */ - maxRows?: number + /** Max textarea height in px before it scrolls internally. */ + maxHeight?: number /** Slash commands (fetched by the host). Empty/undefined disables the menu. */ slashCommands?: SlashCommand[]-const DEFAULT_MAX_ROWS_PX = 160 +const DEFAULT_MAX_HEIGHT_PX = 160 export function Composer({ placeholder = 'Send a message…', - maxRows = DEFAULT_MAX_ROWS_PX, + maxHeight = DEFAULT_MAX_HEIGHT_PX, slashCommands,useLayoutEffect(() => { const el = textareaRef.current if (!el) return el.style.height = 'auto' - el.style.height = `${Math.min(el.scrollHeight, maxRows)}px` - }, [text, maxRows]) + el.style.height = `${Math.min(el.scrollHeight, maxHeight)}px` + }, [text, maxHeight])Also applies to: 72-72
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui-core/src/primitives/Composer.tsx` around lines 44 - 45, Rename the Composer textarea sizing prop from maxRows to maxHeight in the Composer component API, since the value is pixel-based rather than row-based. Update the prop declaration and any related references in Composer to use maxHeight consistently, including the default value handling and any internal logic tied to the current maxRows name. Keep the documentation/comments aligned with the new name so consumers understand it expects pixels.packages/jcode-ui-core/src/primitives/ToolCallView.tsx-121-141 (1)
121-141: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd
aria-expandedto the default toggle button.
DefaultToolHeaderrenders a<button>that toggles expansion, but it lacksaria-expanded. Screen readers cannot announce whether the tool call is expanded or collapsed. Adding the attribute is a one-line fix that improves the out-of-box accessibility of the default header.♿ Proposed fix
<button type="button" onClick={onToggle} aria-expanded={expanded} style={{ display: 'flex', gap: 8, alignItems: 'center', cursor: 'pointer', background: 'none', border: 'none', padding: 0, textAlign: 'left' }}>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui-core/src/primitives/ToolCallView.tsx` around lines 121 - 141, The default toggle button in DefaultToolHeader is missing expanded/collapsed state for assistive tech; add an aria-expanded attribute to the button and bind it to the expanded prop so screen readers can announce the current state. Keep the change in the DefaultToolHeader component alongside the existing onClick and button attributes, and ensure the value reflects the current expanded boolean.packages/jcode-ui-core/src/primitives/ApprovalBlock.tsx-51-55 (1)
51-55: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReset
armedstate when choosing "Allow once" or "Deny".If the user clicks "Allow all…" (arming the two-step confirm), then selects "Allow once" or "Deny" instead, the
armedstate remainstrue. If the resolve request fails and the approval stays pending, the user sees the armed "Confirm allow all / Cancel" buttons rather than the normal pending controls.🛡️ Proposed fix
- const allowOnce = () => actions.resolveApproval(approval.id, true, false) + const allowOnce = () => { setArmed(false); actions.resolveApproval(approval.id, true, false) } const allowAllArm = () => setArmed(true) const allowAllConfirm = () => actions.resolveApproval(approval.id, true, true) const allowAllCancel = () => setArmed(false) - const deny = () => actions.resolveApproval(approval.id, false, false) + const deny = () => { setArmed(false); actions.resolveApproval(approval.id, false, false) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui-core/src/primitives/ApprovalBlock.tsx` around lines 51 - 55, The ApprovalBlock action handlers leave the local armed state stuck on after "Allow all…" is armed, so update the allowOnce and deny handlers to clear armed before resolving the approval. Keep the change scoped to the ApprovalBlock component and use the existing setArmed, allowOnce, deny, and allowAllConfirm/allowAllCancel handlers so the normal pending controls are restored whenever the user chooses "Allow once" or "Deny", even if actions.resolveApproval fails.packages/jcode-ui-core/src/primitives/MessageView.tsx-62-70 (1)
62-70: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClear the
copiedtimeout on unmount.The
setTimeoutincopyis never cleared if the component unmounts within 1500ms. While React 18 no longer warns about state updates on unmounted components, clearing the timer is good hygiene and prevents the callback from holding a reference to stale state.🛡️ Proposed fix
export function MessageView({ message, canEdit = false, showCopy = true, className, renderContent, renderAvatar, }: MessageViewProps): ReactNode { const actions = useRuntimeActions() const [editing, setEditing] = useState(false) const [draft, setDraft] = useState(message.content) const [copied, setCopied] = useState(false) + const copyTimer = useRef<ReturnType<typeof setTimeout> | null>(null) + + useEffect(() => () => { + if (copyTimer.current) clearTimeout(copyTimer.current) + }, [])const copy = useCallback(async () => { try { await navigator.clipboard.writeText(message.content) setCopied(true) - setTimeout(() => setCopied(false), 1500) + if (copyTimer.current) clearTimeout(copyTimer.current) + copyTimer.current = setTimeout(() => setCopied(false), 1500) } catch { // clipboard unavailable } }, [message.content])🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui-core/src/primitives/MessageView.tsx` around lines 62 - 70, The timeout created in MessageView’s copy callback is never cleaned up, so store the timer id in a ref and clear it when the component unmounts. Update the useCallback in MessageView to keep the timeout handle, and add a cleanup effect that clears any pending copied-reset timer so setCopied(false) cannot fire after unmount.site/src/playground/mockScript.ts-109-174 (1)
109-174: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep the scripted fix consistent with the rendered story.
wg.Add/wg.Donenever actually waits, and the closing narration says the goroutine is “joined on shutdown” even though the script never callswg.Wait()or models a shutdown hook. Either add the missing wait path or revise the copy so the demo matches the code.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@site/src/playground/mockScript.ts` around lines 109 - 174, The mock script in mockScript.ts is inconsistent with the narrated outcome: the edited server.go snippet adds a WaitGroup in handle() but never models any shutdown path or calls wg.Wait(), so the “joined on shutdown” copy is inaccurate. Either update the scripted sequence around tool/approval steps to include a real wait/shutdown action tied to the handle/process flow, or change the final appendText narration so it only claims the goroutine is tracked, not joined.site/src/playground/ChatDemo.tsx-70-88 (1)
70-88: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the body height conditional on
chrome.
h-[calc(100%-2.25rem)]still subtracts the titlebar height whenchrome={false}, so the demo leaves a blank strip and shortens the chat area.Suggested fix
- <div className="flex h-[calc(100%-2.25rem)] flex-col"> + <div className={`flex flex-col ${chrome ? 'h-[calc(100%-2.25rem)]' : 'h-full'}`}>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@site/src/playground/ChatDemo.tsx` around lines 70 - 88, The ChatDemo layout still applies the chrome titlebar offset even when chrome is disabled, so the main body is too short and leaves blank space. Update the wrapper in ChatDemo so the height calculation is conditional on the chrome prop, and only subtract the titlebar height when the chrome header is actually rendered; keep the adjustment aligned with the existing chrome and runtime layout blocks.site/src/playground/ChatDemo.tsx-67-85 (1)
67-85: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winMake the inner height conditional on
chrome
h-[calc(100%-2.25rem)]still subtracts the header height whenchrome={false}, so the demo body renders too short. Subtract that offset only when the chrome bar is shown.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@site/src/playground/ChatDemo.tsx` around lines 67 - 85, The inner layout in ChatDemo is always using the chrome header offset, so the body height is too small when chrome is disabled. Update the wrapper inside the RuntimeProvider/ToolRegistryProvider block to make the height calculation conditional on chrome, and only apply the h-[calc(100%-2.25rem)] subtraction when the chrome bar is actually rendered. Use the existing chrome prop and the surrounding flex container in ChatDemo to keep the demo body full-height without the header.packages/jcode-ui-core/.npmignore-1-4 (1)
1-4: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
src/exclusion conflicts withpackage.jsonfilesarray.
.npmignoreexcludessrc/, butpackage.jsonlists"src"in itsfilesarray. npm's.npmignoretakes precedence and can exclude entries from thefilesallowlist, sosrc/will likely be omitted from the published tarball. Meanwhile,tsconfig.build.jsonenablesdeclarationMap: true, which generates.d.ts.mapfiles referencing../src/...paths that would be broken withoutsrcpublished.Decide the intent: either remove
src/from.npmignore(to publish source for declaration maps), or remove"src"from thefilesarray (if source should not be shipped).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui-core/.npmignore` around lines 1 - 4, The package publishing config is conflicting: .npmignore excludes src/ while package.json’s files array includes src, so the source may be dropped from the tarball and break declarationMap references. Update the packaging setup by choosing one intent in the relevant package.json/.npmignore pair: either stop excluding src/ in .npmignore so the source ships with the package, or remove src from the files allowlist if source should not be published. Recheck the package root settings to keep the publish output consistent with tsconfig.build.json and the generated declaration maps.
🧹 Nitpick comments (14)
Makefile (1)
70-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the
build-web-reactrecipe into sub-targets.checkmake flags this target body as exceeding its 5-line guideline (6 lines). While cosmetic, splitting the package builds (
jcode-ui-core,jcode-ui, tailwind) into a reusablebuild-jcode-uiprerequisite target would reduce the recipe body and allowlint-reactto share the same build step if needed later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Makefile` around lines 70 - 76, The build-web-react recipe is too long and should be split into smaller reusable targets. Extract the package build steps for jcode-ui-core, jcode-ui, and the Tailwind CSS generation into a new prerequisite such as build-jcode-ui, then make build-web-react depend on it and keep only the remaining frontend build steps there. Use the existing build-web-react target and the package build commands as the main anchors when refactoring so lint-react can reuse the shared build step later if needed.Source: Linters/SAST tools
packages/jcode-ui/package.json (1)
48-51: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider enabling npm provenance for supply-chain integrity.
"provenance": falsedisables npm package provenance attestation, which links published artifacts to their source build. This is a supply-chain security posture gap. If the CI supports it (GitHub Actions with OIDC), consider enabling provenance to give consumers verifiable build provenance.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui/package.json` around lines 48 - 51, The package publish configuration currently disables npm provenance, so update the publishConfig in package.json to enable provenance if the CI/publish flow supports it. Keep the existing public access setting, and change the provenance option so published artifacts from this package can carry build attestation for supply-chain integrity.packages/jcode-ui/src/lib/apiBaseContext.tsx (1)
7-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport
ReactNodeexplicitly instead of relying on the globalReactnamespace.
React.ReactNodeis used on line 14 but onlycreateContextis imported fromreact. This works because@types/reactdeclares a globalReactnamespace, but relying on globals is fragile and inconsistent with the explicit import style used elsewhere in the file.Proposed refactor
-import { createContext } from 'react' +import { createContext, type ReactNode } from 'react'And update the type reference:
- children: React.ReactNode + children: ReactNode🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui/src/lib/apiBaseContext.tsx` around lines 7 - 15, The ApiBaseProviderProps type is relying on the global React namespace for React.ReactNode instead of importing the type explicitly. Update the react import in apiBaseContext to bring in ReactNode alongside createContext, and change the children field in ApiBaseProviderProps to use that imported ReactNode type so the file no longer depends on implicit globals.packages/jcode-ui/src/lib/markdown.ts (1)
10-10: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse
highlight.js/lib/commonto reduce bundle size.Importing the full
highlight.jspulls in ~190+ language grammars, significantly bloating the bundle for a publishable library. Thecommonexport includes ~30 frequently-used languages and covers the vast majority of chat code blocks. This also speeds uphighlightAuto()since it tests against fewer grammars.♻️ Proposed refactor
- import hljs from 'highlight.js' + import hljs from 'highlight.js/lib/common'If specific less-common languages are needed, register them individually:
+import hljs from 'highlight.js/lib/core' +import typescript from 'highlight.js/lib/languages/typescript' +import go from 'highlight.js/lib/languages/go' +// ... register only what you need +hljs.registerLanguage('typescript', typescript) +hljs.registerLanguage('go', go)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui/src/lib/markdown.ts` at line 10, The markdown highlighter import in `markdown.ts` is pulling in the full `highlight.js` package, which inflates the library bundle. Update the import used by the markdown rendering/highlighting path to `highlight.js/lib/common` instead, and keep the existing highlighting logic (including any `highlightAuto()` usage) wired through that shared instance so the code still works with the smaller common grammar set.packages/jcode-ui/src/components/ContextBar.tsx (1)
43-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd accessible name to the SVG ring.
The occupancy ring conveys information visually but has no
roleoraria-label, making it inaccessible to screen reader users. The hover popover is also mouse-only (group-hoverwithpointer-events-none), so keyboard users can't reach the breakdown.♿ Proposed accessibility improvements
<svg width={size} height={size} className="-rotate-90" + role="img" + aria-label={`Context ${Math.round(pct * 100)}% full`} >To make the popover keyboard-accessible, add
focus-withinalongsidehover:- <div className="pointer-events-none absolute bottom-full right-0 mb-2 w-56 rounded-[var(--radius-lg)] border border-[var(--color-border)] bg-[var(--color-surface)] p-3 opacity-0 shadow-[var(--shadow-md)] transition-opacity group-hover:opacity-100"> + <div className="pointer-events-none absolute bottom-full right-0 mb-2 w-56 rounded-[var(--radius-lg)] border border-[var(--color-border)] bg-[var(--color-surface)] p-3 opacity-0 shadow-[var(--shadow-md)] transition-opacity group-hover:opacity-100 group-focus-within:opacity-100">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui/src/components/ContextBar.tsx` around lines 43 - 64, The occupancy ring in ContextBar is missing an accessible name and the breakdown popover is only exposed on mouse hover, so screen reader and keyboard users can’t access it. Update the SVG ring element in the ContextBar component to include a proper accessible role and aria-label (or equivalent labeling) that describes the occupancy state, and adjust the popover trigger/visibility logic so it also appears on keyboard focus by using focus-within alongside hover instead of relying only on group-hover and pointer-events-none.packages/jcode-ui/src/toolRenderers/terminal.tsx (1)
9-16: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
commandis parsed on every re-render without memoization.When
outputstreams in, the memoized component re-renders and re-parsesargseach time. Wrap the parse inuseMemokeyed onargsto avoid redundant JSON.parse calls.♻️ Proposed refactor
export const TerminalRenderer = memo(function TerminalRenderer({ args, output, error, status }: ToolRendererProps) { - let command = '' - try { - const parsed = JSON.parse(args) - command = parsed.command ?? '' - } catch { - // ignore - } + const command = useMemo(() => { + try { + return JSON.parse(args).command ?? '' + } catch { + return '' + } + }, [args]) const isError = status === 'error' || !!error🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui/src/toolRenderers/terminal.tsx` around lines 9 - 16, The TerminalRenderer component is re-parsing args on every memoized re-render, which causes redundant JSON.parse work as output updates. Update TerminalRenderer to derive command with useMemo keyed only on args, and keep the parsing logic inside that memo so the parsed command is reused unless args changes.packages/jcode-ui/src/toolRenderers/diff.tsx (1)
63-82: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
buildDiffproduces a before/after dump, not a real diff.The current implementation marks every old line as
deland every new line asadd. For a 100-line edit where only one line changed, this renders 200 rows instead of ~3, creating excessive noise and poor performance on large edits.Consider using a lightweight LCS-based diff (e.g., the
diffnpm package or a minimal Myers diff) to produce interleaved context/add/del rows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui/src/toolRenderers/diff.tsx` around lines 63 - 82, `buildDiff` currently emits a full before/after dump by marking every `old_string` line as `del` and every `new_string` line as `add`, which creates noisy output and unnecessary rows for small edits. Update `buildDiff` in `diff.tsx` to compute an actual line diff for each `EditSpec` instead of blindly dumping both sides, using a lightweight LCS/Myers-style approach or a small diff library, and keep the existing `path`/`rows` shape while producing interleaved context, add, and delete rows.packages/jcode-ui/src/toolRenderers/fileViewer.tsx (1)
16-24: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
pathis parsed on every re-render whilelinesis memoized.The
pathJSON.parse runs outsideuseMemo, so whenoutputchanges during streaming, the component re-renders and re-parsesargsunnecessarily. Consolidate both into a singleuseMemofor consistency and to avoid redundant parsing.♻️ Proposed refactor
export const FileViewerRenderer = memo(function FileViewerRenderer({ args, output }: ToolRendererProps) { - let path = '' - try { - const parsed = JSON.parse(args) - path = parsed.path ?? parsed.file_path ?? '' - } catch { - // ignore - } - const lines = useMemo(() => parseLines(output), [output]) + const { path, lines } = useMemo(() => { + let p = '' + try { + const parsed = JSON.parse(args) + p = parsed.path ?? parsed.file_path ?? '' + } catch { + // ignore + } + return { path: p, lines: parseLines(output) } + }, [args, output]) return (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui/src/toolRenderers/fileViewer.tsx` around lines 16 - 24, The FileViewerRenderer component is re-parsing args on every render while output parsing is already memoized. Move the JSON.parse logic for args and the path extraction into a useMemo alongside the existing parseLines(output) memoization in FileViewerRenderer so both derived values are computed consistently and only recomputed when their inputs change.web-react/src/App.tsx (1)
126-126: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
useRef(createDefaultToolRegistry())calls the factory on every render.
useRef(initialValue)evaluatesinitialValueon every render but only assigns it to.currenton the first render.createDefaultToolRegistry()is invoked unnecessarily on all subsequent renders. UseuseStatewith a lazy initializer instead.♻️ Proposed fix
- const registry = useRef(createDefaultToolRegistry()).current + const [registry] = useState(() => createDefaultToolRegistry())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-react/src/App.tsx` at line 126, The App component is eagerly invoking createDefaultToolRegistry on every render via useRef(createDefaultToolRegistry()). Replace this with a lazy initialization approach using useState so the default tool registry is created only once on the first render and then reused; keep the registry variable in App as the stable reference used by the rest of the component.web-react/src/components/AuthGate.tsx (1)
65-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
signOuttakingdispatchas a parameter is an unusual pattern.Exporting a standalone function that requires callers to pass
dispatchmanually is brittle and inconsistent with the hook-based patterns used elsewhere. Consider making it a custom hook or a plain action creator.♻️ Option A: custom hook
-export function signOut(dispatch: ReturnType<typeof useAppDispatch>) { +export function useSignOut() { + const dispatch = useAppDispatch() + return () => { clearAuthToken() dispatch(uiActions.setNeedsAuth(true)) + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-react/src/components/AuthGate.tsx` around lines 65 - 69, The exported signOut helper is using a manually passed dispatch argument, which is inconsistent with the hook-based patterns in AuthGate. Refactor signOut into a custom hook (or otherwise move dispatch access inside the helper) so callers no longer need to pass dispatch explicitly, and update any call sites to use the new hook-based API while keeping clear references to signOut, clearAuthToken, and uiActions.setNeedsAuth.web-react/src/lib/ws.ts (1)
111-118: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueConsider exponential backoff for reconnection.
The fixed 3s retry runs indefinitely with no backoff or max-retry cap. For the desktop sidecar this is fine, but in browser mode against a remote server, a prolonged outage would produce relentless reconnection attempts. Consider exponential backoff (e.g., 3s → 6s → 12s, capped at 30s) and/or a max retry count with a connection-status dispatch to Redux.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-react/src/lib/ws.ts` around lines 111 - 118, The reconnect logic in the WebSocket close handler uses a fixed 3s delay with no cap, so update the reconnect flow in ws.ts (the onclose handler and connect retry logic) to use exponential backoff with a maximum delay and optionally a max retry limit. Track retry state in the WebSocket client class so each failed reconnect increases the delay, reset it on successful connection, and dispatch a connection-status update to Redux when retries are exhausted or the client is offline too long.web-react/src/lib/authToken.ts (1)
22-24: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueToken stored in
localStorageis accessible to any script (CWE-312).The comment explains the rationale (shared key with Vue, avoiding circular imports), and this is a ported pattern. Consider migrating to
httpOnlycookie-based auth in a future PR to prevent XSS token exfiltration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web-react/src/lib/authToken.ts` around lines 22 - 24, The auth token persistence in setAuthToken uses localStorage, which leaves it readable by any script and is a security risk. Update the auth flow to stop storing the token in localStorage and migrate token handling toward an httpOnly cookie-based approach, coordinating with the shared KEY/authToken usage so the Vue/shared-key pattern is preserved without exposing the token to JavaScript.Source: Linters/SAST tools
packages/jcode-ui-core/package.json (1)
69-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
@tanstack/react-virtualas a hard dependency contradicts the optional-React claim.The package marks
react/react-domas optional peer dependencies and the README states non-React entries work in any TS project. However,@tanstack/react-virtualis a harddependency— it will always be installed even by consumers who only usetypes,runtime, oradapters. Consider making it an optional peer dependency (grouped withreact) or documenting that the package always pulls in the virtualization library.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui-core/package.json` around lines 69 - 83, The package currently treats react and react-dom as optional in the peerDependencies/peerDependenciesMeta section, but `@tanstack/react-virtual` is still installed unconditionally via dependencies, which conflicts with the non-React usage claim. Update package.json so the virtualization package is either moved into the same optional peer dependency setup as react-related entries or clearly documented as always required, and make the dependency grouping in the package manifest match the intended behavior for consumers of the non-React APIs.packages/jcode-ui-core/src/runtime/index.ts (1)
45-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse named
ChatImageandAskUserAnswertypes instead of inline duplicates.
RuntimeActionsdefinesimagesas{ data: string; media_type: string }[]andanswersas{ question_header: string; answer: string; selected?: string[] }[]— structurally identical toChatImageandAskUserAnsweralready exported fromtypes/index.ts. If either type evolves, these inline definitions will silently drift.♻️ Proposed refactor
import type { ThreadItem, TokenSnapshot, Goal, TodoItem, QueuedMessage } from '../types/index.js' +import type { ChatImage, AskUserAnswer } from '../types/index.js' // ... export interface RuntimeActions { /** Send a user-authored message. `images` are base64 payloads. */ - sendMessage: (text: string, images?: { data: string; media_type: string }[]) => void + sendMessage: (text: string, images?: ChatImage[]) => void /** Enqueue a message while a turn is running (type-ahead). */ - enqueueMessage: (text: string, images?: { data: string; media_type: string }[]) => void + enqueueMessage: (text: string, images?: ChatImage[]) => void /** Remove a queued message by id (before it is sent). */ removeQueuedMessage: (id: string) => void /** Cancel the in-flight turn. */ stop: () => void /** Resolve an approval gate. `approveAll` arms "allow all future" semantics. */ resolveApproval: (id: string, approved: boolean, approveAll?: boolean) => void /** Answer an `ask_user` batch. */ - submitAskUser: (id: string, answers: { question_header: string; answer: string; selected?: string[] }[]) => void + submitAskUser: (id: string, answers: AskUserAnswer[]) => void /** Edit a past user message and resend from that point. */ editMessage: (id: string, newText: string) => void }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/jcode-ui-core/src/runtime/index.ts` around lines 45 - 60, The RuntimeActions interface is duplicating the shapes already defined by ChatImage and AskUserAnswer, which can drift over time. Update the sendMessage, enqueueMessage, and submitAskUser signatures in RuntimeActions to reference the exported ChatImage and AskUserAnswer types from types/index.ts instead of inline object arrays, keeping the existing method names and behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 30e04f68-473a-483d-bbca-2c5f370499eb
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsite/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (105)
.gitignore.npmrcAGENTS.mdMakefiledocs/tool-search-architecture-draft.mdinternal/model/registry_generated.gointernal/web/dist-react/assets/core-DV6XEvTN.jsinternal/web/dist-react/assets/index-Bg8rBW4i.jsinternal/web/dist-react/assets/index-Cs-LfJ7j.cssinternal/web/dist-react/index.htmlpackages/jcode-ui-core/.npmignorepackages/jcode-ui-core/LICENSEpackages/jcode-ui-core/README.mdpackages/jcode-ui-core/package.jsonpackages/jcode-ui-core/src/adapters/index.tspackages/jcode-ui-core/src/hooks/index.tspackages/jcode-ui-core/src/index.tspackages/jcode-ui-core/src/primitives/ApprovalBlock.tsxpackages/jcode-ui-core/src/primitives/AskUserBlock.tsxpackages/jcode-ui-core/src/primitives/Composer.tsxpackages/jcode-ui-core/src/primitives/MessageView.tsxpackages/jcode-ui-core/src/primitives/Thread.tsxpackages/jcode-ui-core/src/primitives/ToolCallView.tsxpackages/jcode-ui-core/src/primitives/index.tspackages/jcode-ui-core/src/runtime/context.tsxpackages/jcode-ui-core/src/runtime/externalStore.tspackages/jcode-ui-core/src/runtime/index.tspackages/jcode-ui-core/src/runtime/mockRuntime.tspackages/jcode-ui-core/src/types/index.tspackages/jcode-ui-core/tsconfig.build.jsonpackages/jcode-ui-core/tsconfig.jsonpackages/jcode-ui/.npmignorepackages/jcode-ui/LICENSEpackages/jcode-ui/README.mdpackages/jcode-ui/package.jsonpackages/jcode-ui/src/components/ApprovalBanner.tsxpackages/jcode-ui/src/components/AskUserCard.tsxpackages/jcode-ui/src/components/ChatInput.tsxpackages/jcode-ui/src/components/ContextBar.tsxpackages/jcode-ui/src/components/Message.tsxpackages/jcode-ui/src/components/Thread.tsxpackages/jcode-ui/src/components/ToolCallCard.tsxpackages/jcode-ui/src/components/ToolRegistryContext.tsxpackages/jcode-ui/src/index.tspackages/jcode-ui/src/lib/apiBaseContext.tsxpackages/jcode-ui/src/lib/markdown.tspackages/jcode-ui/src/styles/animations.csspackages/jcode-ui/src/styles/components.csspackages/jcode-ui/src/styles/entry.csspackages/jcode-ui/src/styles/tokens.csspackages/jcode-ui/src/toolRenderers/browserShot.tsxpackages/jcode-ui/src/toolRenderers/diff.tsxpackages/jcode-ui/src/toolRenderers/fileViewer.tsxpackages/jcode-ui/src/toolRenderers/generic.tsxpackages/jcode-ui/src/toolRenderers/index.tspackages/jcode-ui/src/toolRenderers/search.tsxpackages/jcode-ui/src/toolRenderers/skill.tsxpackages/jcode-ui/src/toolRenderers/team.tsxpackages/jcode-ui/src/toolRenderers/terminal.tsxpackages/jcode-ui/src/toolRenderers/todo.tsxpackages/jcode-ui/tsconfig.build.jsonpackages/jcode-ui/tsconfig.jsonpnpm-workspace.yamlsite/docs/chat-ui/index.mdsite/docs/chat-ui/primitives.mdsite/docs/chat-ui/runtime.mdsite/docs/chat-ui/theming.mdsite/docs/chat-ui/tool-renderers.mdsite/package.jsonsite/src/App.tsxsite/src/components/SiteNav.tsxsite/src/pages/ChatUIPage.tsxsite/src/pages/chatui.csssite/src/playground/ChatDemo.tsxsite/src/playground/mockScript.tssite/tsconfig.app.tsbuildinfoweb-react/index.htmlweb-react/package.jsonweb-react/src/App.tsxweb-react/src/app/hooks.tsweb-react/src/app/runtime.tsweb-react/src/app/store.tsweb-react/src/app/wsBridge.tsweb-react/src/components/AuthGate.tsxweb-react/src/components/AutomationsView.tsxweb-react/src/components/ChannelsView.tsxweb-react/src/components/ChatView.tsxweb-react/src/components/CommandPalette.tsxweb-react/src/components/GoalBanner.tsxweb-react/src/components/ProjectHeader.tsxweb-react/src/components/SetupView.tsxweb-react/src/components/Sidebar.tsxweb-react/src/lib/api.tsweb-react/src/lib/apiBase.tsweb-react/src/lib/authToken.tsweb-react/src/lib/automation.tsweb-react/src/lib/types.tsweb-react/src/lib/useDesktop.tsweb-react/src/lib/ws.tsweb-react/src/main.tsxweb-react/src/styles.cssweb-react/tsconfig.app.jsonweb-react/tsconfig.jsonweb-react/tsconfig.node.jsonweb-react/vite.config.ts
| import type { ComponentType } from 'react' | ||
| import type { ToolCall, ToolDisplayInfo, ToolStatus } from '../types/index.js' | ||
|
|
||
| export type { ToolStatus } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Remove export type { ToolStatus } — it creates an ambiguous barrel re-export.
ToolStatus is already exported from ../types/index.js. When index.ts does export * from './types/index.js' and export * from './adapters/index.js', TypeScript silently drops ToolStatus from the barrel because it's ambiguous. This means import { type ToolStatus } from 'jcode-ui-core' will fail with "Module has no exported member 'ToolStatus'."
🐛 Proposed fix
import type { ComponentType } from 'react'
import type { ToolCall, ToolDisplayInfo, ToolStatus } from '../types/index.js'
-export type { ToolStatus }
-
/** Props every tool renderer receives. */
export interface ToolRendererProps {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type { ToolStatus } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/jcode-ui-core/src/adapters/index.ts` at line 14, Remove the
redundant ToolStatus type re-export from the adapters barrel so the root barrel
no longer has an ambiguous export. Update the adapters/index.ts barrel to stop
exporting ToolStatus, since it is already exposed through types/index.js and the
top-level index.ts re-exports both barrels. Keep the shared type available only
from the types barrel so import { type ToolStatus } from 'jcode-ui-core'
resolves correctly.
| export function useIsAtBottom<T extends HTMLElement>(threshold = 80) { | ||
| const { ref, onScroll, scrollToBottom } = useAutoScroll<T>(threshold) | ||
| return { ref, onScroll, scrollToBottom } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
useIsAtBottom doesn't re-render as documented — implementation is incomplete.
The doc comment claims it "re-renders the component when the flag flips" and "intentionally tracks a coarse boolean," but the implementation has no useState, returns no isAtBottom value, and is just a subset of useAutoScroll's return. Consumers using this hook will get no re-renders and no flag to read.
🐛 Proposed fix
export function useIsAtBottom<T extends HTMLElement>(threshold = 80) {
- const { ref, onScroll, scrollToBottom } = useAutoScroll<T>(threshold)
- return { ref, onScroll, scrollToBottom }
+ const { ref, onScroll: baseOnScroll, scrollToBottom, getIsAtBottom } = useAutoScroll<T>(threshold)
+ const [isAtBottom, setIsAtBottom] = useState(true)
+
+ const onScroll = useCallback(() => {
+ baseOnScroll()
+ const next = getIsAtBottom()
+ setIsAtBottom((prev) => (prev !== next ? next : prev))
+ }, [baseOnScroll, getIsAtBottom])
+
+ return { ref, onScroll, scrollToBottom, isAtBottom }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function useIsAtBottom<T extends HTMLElement>(threshold = 80) { | |
| const { ref, onScroll, scrollToBottom } = useAutoScroll<T>(threshold) | |
| return { ref, onScroll, scrollToBottom } | |
| } | |
| export function useIsAtBottom<T extends HTMLElement>(threshold = 80) { | |
| const { ref, onScroll: baseOnScroll, scrollToBottom, getIsAtBottom } = useAutoScroll<T>(threshold) | |
| const [isAtBottom, setIsAtBottom] = useState(true) | |
| const onScroll = useCallback(() => { | |
| baseOnScroll() | |
| const next = getIsAtBottom() | |
| setIsAtBottom((prev) => (prev !== next ? next : prev)) | |
| }, [baseOnScroll, getIsAtBottom]) | |
| return { ref, onScroll, scrollToBottom, isAtBottom } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/jcode-ui-core/src/hooks/index.ts` around lines 56 - 59, The
`useIsAtBottom` hook is only forwarding `useAutoScroll` and never tracks or
exposes the bottom state, so it cannot re-render as documented. Update
`useIsAtBottom` to include local state and scroll handling logic that derives a
coarse boolean flag, returns that flag (for example alongside `ref` and
`scrollToBottom`), and triggers re-renders when the bottom status flips; use the
existing `useIsAtBottom` and `useAutoScroll` symbols to keep the API aligned
with the docs.
| const followDep = useMemo(() => { | ||
| if (!last) return items.length | ||
| if (last.kind === 'message') return `${items.length}:${last.data.content.length}` | ||
| return items.length | ||
| }, [items, last]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify ThreadItem type definition and kind variants
rg -n 'kind.*tool|kind.*message|kind.*approval' packages/jcode-ui-core/src/types/index.ts -C3Repository: cnjack/jcode
Length of output: 1019
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== Thread.tsx =='
ast-grep outline packages/jcode-ui-core/src/primitives/Thread.tsx --view expanded || true
echo
nl -ba packages/jcode-ui-core/src/primitives/Thread.tsx | sed -n '1,240p'
echo
echo '== useStreamFollow references =='
rg -n "useStreamFollow|followDep|scrollIntoView|autoScroll" packages/jcode-ui-core/src -C3
echo
echo '== ToolCall shape =='
rg -n "type ToolCall|interface ToolCall|export type ToolCall|output:" packages/jcode-ui-core/src/types -C4Repository: cnjack/jcode
Length of output: 1184
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '== packages/jcode-ui-core/src/primitives/Thread.tsx (relevant slice) =='
sed -n '1,260p' packages/jcode-ui-core/src/primitives/Thread.tsx
echo
echo '== useStreamFollow / related hook implementations =='
rg -n "function useStreamFollow|const useStreamFollow|export function useStreamFollow|useStreamFollow" packages/jcode-ui-core/src -C4
echo
echo '== ToolCall type definition =='
rg -n "export type ToolCall|interface ToolCall|type ToolCall" packages/jcode-ui-core/src/types -C8Repository: cnjack/jcode
Length of output: 11717
Track tool output in followDep packages/jcode-ui-core/src/primitives/Thread.tsx:66-70
followDep only reacts to message text length. When the last item is a tool call, changes to ToolCall.output won't retrigger useStreamFollow, so the thread can stop auto-scrolling during streamed tool output.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/jcode-ui-core/src/primitives/Thread.tsx` around lines 66 - 70, The
followDep memo in Thread should also depend on streamed tool output, not just
message content length. Update the logic in Thread.tsx around
followDep/useStreamFollow so that when the last item is a tool call it
incorporates ToolCall.output (or a stable representation of it) into the
dependency value, ensuring changes to tool output retrigger auto-follow during
streaming.
| const cacheRef = useRef<{ snapshot: RuntimeState; value: T } | null>(null) | ||
| const subscribe = runtime.subscribe | ||
| const getSnapshot = () => normalizeState(runtime.getState()) | ||
| // Prime the cache on first read / after a store change useSyncExternalStore detected. | ||
| const snapshot = useSyncExternalStore(subscribe, getSnapshot, getSnapshot) | ||
| const cache = cacheRef.current | ||
| if (!cache || cache.snapshot !== snapshot) { | ||
| const value = selector(snapshot) | ||
| if (!cache || !isEqual(cache.value, value)) { | ||
| cacheRef.current = { snapshot, value } | ||
| } else { | ||
| // keep old value identity, just refresh the snapshot stamp | ||
| cacheRef.current = { snapshot, value: cache.value } | ||
| } | ||
| } | ||
| return cacheRef.current!.value |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
getSnapshot returns a new object on every call — causes infinite re-render loop with useSyncExternalStore.
normalizeState(runtime.getState()) creates a fresh object each invocation. React's useSyncExternalStore requires getSnapshot to return a referentially stable value when the store hasn't changed; otherwise its internal useLayoutEffect detects a "change" on every render, calls forceStore, and loops infinitely. The cacheRef layer below runs after useSyncExternalStore returns, so it cannot prevent the loop.
Additionally, getSnapshot is a new arrow function on every render, which re-triggers the effect's [subscribe, getSnapshot] dependency check each time.
Two fixes are needed (the second is in externalStore.ts):
- Remove the redundant
normalizeStatecall —ChatRuntime.getState()already returnsRuntimeState. - Memoize
getSnapshotwithuseCallbackso the function reference is stable. - Cache
getState()increateExternalStoreRuntimeso it returns a stableRuntimeStatereference when the underlying store hasn't changed.
🐛 Proposed fix for context.tsx
import { createContext, useContext, useMemo, useRef, useSyncExternalStore } from 'react'
+import { useCallback } from 'react'
import type { ReactNode } from 'react'
import type { ChatRuntime, RuntimeState } from './index.js'
-import { normalizeState } from './index.js' function useRuntimeSelectorInternal<T>(
runtime: ChatRuntime,
selector: (state: RuntimeState) => T,
isEqual: (a: T, b: T) => boolean,
): T {
const cacheRef = useRef<{ snapshot: RuntimeState; value: T } | null>(null)
const subscribe = runtime.subscribe
- const getSnapshot = () => normalizeState(runtime.getState())
+ const getSnapshot = useCallback(() => runtime.getState(), [runtime])
// Prime the cache on first read / after a store change useSyncExternalStore detected.
const snapshot = useSyncExternalStore(subscribe, getSnapshot, getSnapshot)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const cacheRef = useRef<{ snapshot: RuntimeState; value: T } | null>(null) | |
| const subscribe = runtime.subscribe | |
| const getSnapshot = () => normalizeState(runtime.getState()) | |
| // Prime the cache on first read / after a store change useSyncExternalStore detected. | |
| const snapshot = useSyncExternalStore(subscribe, getSnapshot, getSnapshot) | |
| const cache = cacheRef.current | |
| if (!cache || cache.snapshot !== snapshot) { | |
| const value = selector(snapshot) | |
| if (!cache || !isEqual(cache.value, value)) { | |
| cacheRef.current = { snapshot, value } | |
| } else { | |
| // keep old value identity, just refresh the snapshot stamp | |
| cacheRef.current = { snapshot, value: cache.value } | |
| } | |
| } | |
| return cacheRef.current!.value | |
| const cacheRef = useRef<{ snapshot: RuntimeState; value: T } | null>(null) | |
| const subscribe = runtime.subscribe | |
| const getSnapshot = useCallback(() => runtime.getState(), [runtime]) | |
| // Prime the cache on first read / after a store change useSyncExternalStore detected. | |
| const snapshot = useSyncExternalStore(subscribe, getSnapshot, getSnapshot) | |
| const cache = cacheRef.current | |
| if (!cache || cache.snapshot !== snapshot) { | |
| const value = selector(snapshot) | |
| if (!cache || !isEqual(cache.value, value)) { | |
| cacheRef.current = { snapshot, value } | |
| } else { | |
| // keep old value identity, just refresh the snapshot stamp | |
| cacheRef.current = { snapshot, value: cache.value } | |
| } | |
| } | |
| return cacheRef.current!.value |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/jcode-ui-core/src/runtime/context.tsx` around lines 89 - 104, The
`useRuntimeState` hook in `context.tsx` is causing `useSyncExternalStore` to see
a changing snapshot on every render because `getSnapshot` wraps
`runtime.getState()` in `normalizeState` and is recreated as a new arrow
function each time. Update `getSnapshot` to return the runtime state directly,
memoize it with `useCallback`, and keep the existing `cacheRef` logic only for
selector/value stabilization. Also adjust `createExternalStoreRuntime` in
`externalStore.ts` so `getState()` returns the same `RuntimeState` reference
when the store has not changed, ensuring the snapshot stays referentially
stable.
| "dependencies": { | ||
| "@heroicons/react": "^2.2.0", | ||
| "@tailwindcss/typography": "^0.5.16", | ||
| "dompurify": "^3.2.4", | ||
| "highlight.js": "^11.11.1", | ||
| "jcode-ui-core": "file:../jcode-ui-core", | ||
| "marked": "^18.0.0", | ||
| "marked-highlight": "^2.2.2" | ||
| }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
file:../jcode-ui-core will break for npm consumers.
"jcode-ui-core": "file:../jcode-ui-core" only resolves inside the pnpm workspace. When jcode-ui is published to npm, the file: protocol is not rewritten by pnpm's publish step (only workspace:* is). External consumers running npm install jcode-ui will fail because ../jcode-ui-core doesn't exist on their machine.
Use workspace:* (pnpm replaces it with the real version at publish time) or a concrete version range:
Proposed fix
"dependencies": {
"`@heroicons/react`": "^2.2.0",
"`@tailwindcss/typography`": "^0.5.16",
"dompurify": "^3.2.4",
"highlight.js": "^11.11.1",
- "jcode-ui-core": "file:../jcode-ui-core",
+ "jcode-ui-core": "workspace:*",
"marked": "^18.0.0",
"marked-highlight": "^2.2.2"
},If you prefer a pinned range instead of workspace:*, use "jcode-ui-core": "^0.1.0" matching the core package's published version.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "dependencies": { | |
| "@heroicons/react": "^2.2.0", | |
| "@tailwindcss/typography": "^0.5.16", | |
| "dompurify": "^3.2.4", | |
| "highlight.js": "^11.11.1", | |
| "jcode-ui-core": "file:../jcode-ui-core", | |
| "marked": "^18.0.0", | |
| "marked-highlight": "^2.2.2" | |
| }, | |
| "dependencies": { | |
| "`@heroicons/react`": "^2.2.0", | |
| "`@tailwindcss/typography`": "^0.5.16", | |
| "dompurify": "^3.2.4", | |
| "highlight.js": "^11.11.1", | |
| "jcode-ui-core": "workspace:*", | |
| "marked": "^18.0.0", | |
| "marked-highlight": "^2.2.2" | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/jcode-ui/package.json` around lines 64 - 72, The dependency
declaration for jcode-ui-core is workspace-only and will break for published npm
consumers. Update the jcode-ui/package.json dependency entry in the dependencies
block to use a publish-safe reference such as workspace:* or a concrete semver
range matching the published jcode-ui-core version, so the package can be
installed outside the pnpm workspace.
| export const editMessage = createAsyncThunk( | ||
| 'chat/edit', | ||
| async (payload: { id: string; text: string }, { dispatch }) => { | ||
| // Trim the timeline up to (and including) the edited message, then resend. | ||
| dispatch(chatActions.clearChat()) | ||
| await dispatch(sendMessage({ text: payload.text })) | ||
| }, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
editMessage clears the entire timeline instead of trimming to the edited message.
The comment says "Trim the timeline up to (and including) the edited message, then resend," but clearChat() wipes the full timeline, goal, todos, and queued messages. The id parameter is accepted but never used. Additionally, images from the original message are not passed to sendMessage, so they are silently lost on edit.
If this is an intentional simplification for the migration skeleton, the comment should reflect the actual behavior. If not, the thunk should use id to find the message index, truncate the timeline up to that point, and forward the original images.
🔧 Proposed fix: trim timeline to edited message
export const editMessage = createAsyncThunk(
'chat/edit',
- async (payload: { id: string; text: string }, { dispatch }) => {
- // Trim the timeline up to (and including) the edited message, then resend.
- dispatch(chatActions.clearChat())
- await dispatch(sendMessage({ text: payload.text }))
+ async (payload: { id: string; text: string }, { dispatch, getState }) => {
+ const state = getState() as RootState
+ const idx = state.chat.timeline.findIndex(
+ (i) => i.kind === 'message' && i.data.id === payload.id,
+ )
+ if (idx === -1) return
+ // Trim everything after (and including) the edited message.
+ dispatch(chatActions.trimTimeline(idx))
+ // Forward original images if present.
+ const orig = state.chat.timeline[idx]
+ const images = orig.kind === 'message' ? orig.data.images : undefined
+ await dispatch(sendMessage({ text: payload.text, images }))
},
)This requires adding a trimTimeline reducer to the chat slice:
+ trimTimeline(s, a: { payload: number }) {
+ s.timeline = s.timeline.slice(0, a.payload)
+ s.isRunning = false
+ streamingText = ''
+ streamingMsgId = ''
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const editMessage = createAsyncThunk( | |
| 'chat/edit', | |
| async (payload: { id: string; text: string }, { dispatch }) => { | |
| // Trim the timeline up to (and including) the edited message, then resend. | |
| dispatch(chatActions.clearChat()) | |
| await dispatch(sendMessage({ text: payload.text })) | |
| }, | |
| ) | |
| export const editMessage = createAsyncThunk( | |
| 'chat/edit', | |
| async (payload: { id: string; text: string }, { dispatch, getState }) => { | |
| const state = getState() as RootState | |
| const idx = state.chat.timeline.findIndex( | |
| (i) => i.kind === 'message' && i.data.id === payload.id, | |
| ) | |
| if (idx === -1) return | |
| // Trim everything after (and including) the edited message. | |
| dispatch(chatActions.trimTimeline(idx)) | |
| // Forward original images if present. | |
| const orig = state.chat.timeline[idx] | |
| const images = orig.kind === 'message' ? orig.data.images : undefined | |
| await dispatch(sendMessage({ text: payload.text, images })) | |
| }, | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-react/src/app/store.ts` around lines 431 - 438, The editMessage thunk is
wiping the whole chat state via chatActions.clearChat() instead of trimming back
to the edited message, and it ignores the payload.id and any original images.
Update editMessage to locate the message by id, truncate the timeline only up to
that message, preserve the other chat state that should remain, and pass the
original images through to sendMessage; use the existing editMessage and
sendMessage symbols to keep the behavior aligned with the comment.
| dispatch(modelActions.setProvider(d.provider)) | ||
| dispatch(modelActions.setModel(d.model)) | ||
| }, | ||
| onModeChanged: (d) => dispatch(modelActions.setMode(d as never)), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
onModeChanged passes the data object instead of the mode string — runtime bug.
d is typed { mode: string } per the WSHandlers interface, but modelActions.setMode expects an AgentMode string (confirmed by App.tsx line 59 which calls normalizeMode(h.mode)). The as never cast masks the type mismatch — at runtime the reducer receives { mode: "plan" } instead of "plan", breaking all downstream mode comparisons like state.mode === 'plan'.
Compare with onModelChanged (lines 82-85) which correctly extracts d.provider and d.model from its data object.
🐛 Proposed fix
import { api } from '../lib/api'
+import { normalizeMode } from '../lib/types'
import type { Goal } from 'jcode-ui-core'
@@
- onModeChanged: (d) => dispatch(modelActions.setMode(d as never)),
+ onModeChanged: (d) => dispatch(modelActions.setMode(normalizeMode(d.mode))),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onModeChanged: (d) => dispatch(modelActions.setMode(d as never)), | |
| import { normalizeMode } from '../lib/types' | |
| onModeChanged: (d) => dispatch(modelActions.setMode(normalizeMode(d.mode))), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-react/src/app/wsBridge.ts` at line 86, The onModeChanged handler in
wsBridge.ts is forwarding the entire data object to modelActions.setMode, which
expects the mode string. Update the onModeChanged callback to extract the mode
field from the WSHandlers payload (matching how onModelChanged pulls
provider/model) and pass that string through instead of using the as never cast.
Remove the unsafe cast so the reducer receives an AgentMode value and downstream
mode comparisons continue to work correctly.
| <label className="mt-4 block text-xs font-medium text-[var(--color-muted-foreground)]">Provider</label> | ||
| <select | ||
| value={selected?.id ?? ''} | ||
| onChange={(e) => setSelected(providers.find((p) => p.id === e.target.value) ?? null)} | ||
| className="mt-1 w-full rounded-[var(--radius-md)] border border-[var(--color-border)] bg-[var(--color-muted)] px-3 py-2 text-sm" | ||
| > | ||
| <option value="">Select…</option> | ||
| {providers.map((p) => ( | ||
| <option key={p.id} value={p.id}> | ||
| {p.name} | ||
| </option> | ||
| ))} | ||
| </select> | ||
|
|
||
| <label className="mt-3 block text-xs font-medium text-[var(--color-muted-foreground)]">API key</label> | ||
| <input | ||
| type="password" | ||
| value={apiKey} | ||
| onChange={(e) => setApiKey(e.target.value)} | ||
| className="mt-1 w-full rounded-[var(--radius-md)] border border-[var(--color-border)] bg-[var(--color-muted)] px-3 py-2 text-sm outline-none focus:border-[var(--color-primary)]" | ||
| /> | ||
|
|
||
| {models.length > 0 && ( | ||
| <> | ||
| <label className="mt-3 block text-xs font-medium text-[var(--color-muted-foreground)]">Model</label> | ||
| <select | ||
| value={model} | ||
| onChange={(e) => setModel(e.target.value)} | ||
| className="mt-1 w-full rounded-[var(--radius-md)] border border-[var(--color-border)] bg-[var(--color-muted)] px-3 py-2 text-sm" | ||
| > | ||
| <option value="">Default</option> | ||
| {models.map((m) => ( | ||
| <option key={m.id} value={m.id}> | ||
| {m.name} | ||
| </option> | ||
| ))} | ||
| </select> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Associate labels with form controls for screen reader accessibility.
The <label> elements are not linked to their <select>/<input> counterparts via htmlFor/id attributes. Screen reader users cannot determine which label belongs to which control. Add id attributes to the controls and htmlFor to the labels, or wrap each control inside its <label>.
♿ Proposed fix: add htmlFor/id associations
- <label className="mt-4 block text-xs font-medium text-[var(--color-muted-foreground)]">Provider</label>
+ <label htmlFor="setup-provider" className="mt-4 block text-xs font-medium text-[var(--color-muted-foreground)]">Provider</label>
<select
+ id="setup-provider"
value={selected?.id ?? ''}
onChange={(e) => setSelected(providers.find((p) => p.id === e.target.value) ?? null)}
className="mt-1 w-full rounded-[var(--radius-md)] border border-[var(--color-border)] bg-[var(--color-muted)] px-3 py-2 text-sm"
>
<option value="">Select…</option>
{providers.map((p) => (
<option key={p.id} value={p.id}>
{p.name}
</option>
))}
</select>
- <label className="mt-3 block text-xs font-medium text-[var(--color-muted-foreground)]">API key</label>
+ <label htmlFor="setup-apikey" className="mt-3 block text-xs font-medium text-[var(--color-muted-foreground)]">API key</label>
<input
+ id="setup-apikey"
type="password"
value={apiKey}
onChange={(e) => setApiKey(e.target.value)}
className="mt-1 w-full rounded-[var(--radius-md)] border border-[var(--color-border)] bg-[var(--color-muted)] px-3 py-2 text-sm outline-none focus:border-[var(--color-primary)]"
/>
{models.length > 0 && (
<>
- <label className="mt-3 block text-xs font-medium text-[var(--color-muted-foreground)]">Model</label>
+ <label htmlFor="setup-model" className="mt-3 block text-xs font-medium text-[var(--color-muted-foreground)]">Model</label>
<select
+ id="setup-model"
value={model}
onChange={(e) => setModel(e.target.value)}
className="mt-1 w-full rounded-[var(--radius-md)] border border-[var(--color-border)] bg-[var(--color-muted)] px-3 py-2 text-sm"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <label className="mt-4 block text-xs font-medium text-[var(--color-muted-foreground)]">Provider</label> | |
| <select | |
| value={selected?.id ?? ''} | |
| onChange={(e) => setSelected(providers.find((p) => p.id === e.target.value) ?? null)} | |
| className="mt-1 w-full rounded-[var(--radius-md)] border border-[var(--color-border)] bg-[var(--color-muted)] px-3 py-2 text-sm" | |
| > | |
| <option value="">Select…</option> | |
| {providers.map((p) => ( | |
| <option key={p.id} value={p.id}> | |
| {p.name} | |
| </option> | |
| ))} | |
| </select> | |
| <label className="mt-3 block text-xs font-medium text-[var(--color-muted-foreground)]">API key</label> | |
| <input | |
| type="password" | |
| value={apiKey} | |
| onChange={(e) => setApiKey(e.target.value)} | |
| className="mt-1 w-full rounded-[var(--radius-md)] border border-[var(--color-border)] bg-[var(--color-muted)] px-3 py-2 text-sm outline-none focus:border-[var(--color-primary)]" | |
| /> | |
| {models.length > 0 && ( | |
| <> | |
| <label className="mt-3 block text-xs font-medium text-[var(--color-muted-foreground)]">Model</label> | |
| <select | |
| value={model} | |
| onChange={(e) => setModel(e.target.value)} | |
| className="mt-1 w-full rounded-[var(--radius-md)] border border-[var(--color-border)] bg-[var(--color-muted)] px-3 py-2 text-sm" | |
| > | |
| <option value="">Default</option> | |
| {models.map((m) => ( | |
| <option key={m.id} value={m.id}> | |
| {m.name} | |
| </option> | |
| ))} | |
| </select> | |
| <label htmlFor="setup-provider" className="mt-4 block text-xs font-medium text-[var(--color-muted-foreground)]">Provider</label> | |
| <select | |
| id="setup-provider" | |
| value={selected?.id ?? ''} | |
| onChange={(e) => setSelected(providers.find((p) => p.id === e.target.value) ?? null)} | |
| className="mt-1 w-full rounded-[var(--radius-md)] border border-[var(--color-border)] bg-[var(--color-muted)] px-3 py-2 text-sm" | |
| > | |
| <option value="">Select…</option> | |
| {providers.map((p) => ( | |
| <option key={p.id} value={p.id}> | |
| {p.name} | |
| </option> | |
| ))} | |
| </select> | |
| <label htmlFor="setup-apikey" className="mt-3 block text-xs font-medium text-[var(--color-muted-foreground)]">API key</label> | |
| <input | |
| id="setup-apikey" | |
| type="password" | |
| value={apiKey} | |
| onChange={(e) => setApiKey(e.target.value)} | |
| className="mt-1 w-full rounded-[var(--radius-md)] border border-[var(--color-border)] bg-[var(--color-muted)] px-3 py-2 text-sm outline-none focus:border-[var(--color-primary)]" | |
| /> | |
| {models.length > 0 && ( | |
| <> | |
| <label htmlFor="setup-model" className="mt-3 block text-xs font-medium text-[var(--color-muted-foreground)]">Model</label> | |
| <select | |
| id="setup-model" | |
| value={model} | |
| onChange={(e) => setModel(e.target.value)} | |
| className="mt-1 w-full rounded-[var(--radius-md)] border border-[var(--color-border)] bg-[var(--color-muted)] px-3 py-2 text-sm" | |
| > | |
| <option value="">Default</option> | |
| {models.map((m) => ( | |
| <option key={m.id} value={m.id}> | |
| {m.name} | |
| </option> | |
| ))} | |
| </select> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-react/src/components/SetupView.tsx` around lines 63 - 99, The form
controls in SetupView are missing accessible label associations, so link each
label to its control by adding matching id/htmlFor pairs (or wrapping the
control inside the label) for the Provider select, API key input, and Model
select. Update the relevant JSX in SetupView so the existing label/select/input
blocks use unique identifiers and remain functionally unchanged.
| /** Update the handler set (e.g. when the active task changes). */ | ||
| setHandlers(handlers: WSHandlers): void { | ||
| this.handlers = handlers | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
setHandlers() doesn't take effect — handlerMap captures the old handlers object.
handlerFor() builds handlerMap lazily with const h = this.handlers, capturing the handlers reference at build time. When setHandlers() replaces this.handlers with a new object, the closures still call methods on the old object. The setHandlers comment says "Update the handler set (e.g. when the active task changes)" — so this is intended to be called at runtime, but the update is silently ignored.
Fix: null out this.handlerMap in setHandlers() to force a rebuild on the next message. (Included in the diff above.)
Also applies to: 140-168
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-react/src/lib/ws.ts` around lines 58 - 61, `setHandlers()` in `WS` is
replacing `this.handlers` but leaving the cached `handlerMap` intact, so
`handlerFor()` keeps invoking closures bound to the old handlers object. Update
`setHandlers()` to clear `this.handlerMap` whenever the handler set changes, so
the next call to `handlerFor()` rebuilds the map against the new `WSHandlers`
instance and the runtime update takes effect.
| connect(): void { | ||
| if (this.ws) { | ||
| this.ws.close() | ||
| this.ws = null | ||
| } | ||
| const token = getAuthToken() | ||
| this.ws = token | ||
| ? new WebSocket(`${wsBase()}/api/ws`, ['jcode-auth', token]) | ||
| : new WebSocket(`${wsBase()}/api/ws`) | ||
|
|
||
| this.ws.onopen = () => { | ||
| this.connected = true | ||
| if (this.pingTimer) clearInterval(this.pingTimer) | ||
| this.pingTimer = setInterval(() => this.send({ type: 'ping' }), 30000) | ||
| } | ||
|
|
||
| this.ws.onmessage = (event) => { | ||
| try { | ||
| const msg: WSMessage = JSON.parse(event.data) | ||
| const active = this.handlers.activeTaskId?.() | ||
| if (msg.task_id && active && msg.task_id !== active) return | ||
| const handler = this.handlerFor(msg.type) | ||
| if (handler) { | ||
| let data = msg.data | ||
| if ( | ||
| msg.task_id && | ||
| (msg.type === 'approval_request' || msg.type === 'ask_user_request') && | ||
| data && | ||
| typeof data === 'object' | ||
| ) { | ||
| data = { ...(data as Record<string, unknown>), task_id: msg.task_id } | ||
| } | ||
| handler(data) | ||
| } | ||
| } catch { | ||
| // parse error — drop | ||
| } | ||
| } | ||
|
|
||
| this.ws.onerror = () => { | ||
| this.connected = false | ||
| } | ||
|
|
||
| this.ws.onclose = () => { | ||
| this.connected = false | ||
| if (this.pingTimer) { | ||
| clearInterval(this.pingTimer) | ||
| this.pingTimer = null | ||
| } | ||
| this.ws = null | ||
| this.retryTimer = setTimeout(() => this.connect(), 3000) | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
disconnect() does not prevent reconnection; connect() doesn't clear pending retry timer.
Two related bugs in the reconnection logic:
-
disconnect()→ auto-reconnect:this.ws?.close()indisconnect()triggersoncloseasynchronously, which unconditionally setsthis.retryTimer = setTimeout(() => this.connect(), 3000). The client reconnects 3s after "disconnecting" — defeating the method's purpose. -
connect()→ stale retry timer: Ifconnect()is called while a retry is pending, the old timer still fires and callsconnect()again, closing the just-established connection.
Fix: add a disconnected flag and clear the retry timer at the start of connect().
🔒 Proposed fix
export class WSClient {
private ws: WebSocket | null = null
private retryTimer: ReturnType<typeof setTimeout> | null = null
private pingTimer: ReturnType<typeof setInterval> | null = null
private connected = false
private handlers: WSHandlers
+ private disconnected = false
constructor(handlers: WSHandlers) {
this.handlers = handlers
}
/** Update the handler set (e.g. when the active task changes). */
setHandlers(handlers: WSHandlers): void {
this.handlers = handlers
+ this.handlerMap = null
}
/** True when the WS is open. */
isConnected(): boolean {
return this.connected
}
connect(): void {
+ this.disconnected = false
+ if (this.retryTimer) {
+ clearTimeout(this.retryTimer)
+ this.retryTimer = null
+ }
if (this.ws) {
this.ws.close()
this.ws = null
}
const token = getAuthToken()
this.ws = token
? new WebSocket(`${wsBase()}/api/ws`, ['jcode-auth', token])
: new WebSocket(`${wsBase()}/api/ws`)
this.ws.onopen = () => {
this.connected = true
if (this.pingTimer) clearInterval(this.pingTimer)
this.pingTimer = setInterval(() => this.send({ type: 'ping' }), 30000)
}
this.ws.onmessage = (event) => {
try {
const msg: WSMessage = JSON.parse(event.data)
const active = this.handlers.activeTaskId?.()
if (msg.task_id && active && msg.task_id !== active) return
const handler = this.handlerFor(msg.type)
if (handler) {
let data = msg.data
if (
msg.task_id &&
(msg.type === 'approval_request' || msg.type === 'ask_user_request') &&
data &&
typeof data === 'object'
) {
data = { ...(data as Record<string, unknown>), task_id: msg.task_id }
}
handler(data)
}
} catch {
// parse error — drop
}
}
this.ws.onerror = () => {
this.connected = false
}
this.ws.onclose = () => {
this.connected = false
if (this.pingTimer) {
clearInterval(this.pingTimer)
this.pingTimer = null
}
this.ws = null
- this.retryTimer = setTimeout(() => this.connect(), 3000)
+ if (!this.disconnected) {
+ this.retryTimer = setTimeout(() => this.connect(), 3000)
+ }
}
}
send(msg: WSMessage): void {
if (this.ws && this.ws.readyState === WebSocket.OPEN) {
this.ws.send(JSON.stringify(msg))
}
}
sendApproval(id: string, approved: boolean, approveAll = false, taskId?: string): void {
this.send({ type: 'approval', data: { id, approved, approve_all: approveAll, task_id: taskId } })
}
disconnect(): void {
+ this.disconnected = true
if (this.retryTimer) clearTimeout(this.retryTimer)
if (this.pingTimer) clearInterval(this.pingTimer)
this.ws?.close()
this.ws = null
this.connected = false
}Also applies to: 132-138
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web-react/src/lib/ws.ts` around lines 68 - 120, The WebSocket reconnection
flow in connect() and disconnect() allows unintended reconnects and stale retry
timers. Add a disconnected state check so ws.onclose only schedules retryTimer
when the client is not intentionally disconnected, and clear any pending
retryTimer at the start of connect() before opening a new WebSocket. Update
disconnect() to set the flag before closing the socket, and ensure connect()
resets it when a real connection attempt begins.
…replay + separated chat-ui docs Runtime fixes (found via headless-browser testing against the live Go backend): - externalStore.ts: cache the normalized RuntimeState keyed on the host state reference so getState() returns a stable identity between dispatches. Without this, useSyncExternalStore infinite-looped (Maximum update depth) and crashed every page that rendered <Thread>. This was the root cause of the blank app. - context.tsx: trust the runtime's snapshot stability (remove the redundant and buggy double-cache); useRuntimeState/useRuntimeSelector now pass runtime.getState directly. - Thread.tsx VirtualizedThread: the scroll container resolved to height 0 inside flex parents, so the virtualizer rendered 0 rows. Restructured to flex:1 + min-height:0 so the height resolves through the chain. Verified the demo now streams its scripted conversation. web-react (product app): - loadSession thunk: replay a session's JSONL history into the timeline (was a TODO — the app booted to an empty chat). Walks entries, rebuilds messages + tool calls, matches tool_call_id, falls back to most-recent session on 404. - App.tsx boot: load current session, fall back to most-recent if empty. - toolInfo.ts: ported extractToolDisplayInfo (mirrors backend, for replay). - Sidebar openSession now loads the session via the thunk. - vite.config.ts: pin port 5173 (matches Tauri devUrl). site (component library docs + showcase): - Separated chat-ui docs from jcode product docs. New /chat-ui/docs/* route with its own pipeline (chatUiDocs.ts), nav tree, ChatUiDocsLayout, index, and DocPage. The sidebar now lists ONLY jcode-ui docs — no mixing with the jcode product docs (Agent/Plan Mode/Browser). Back-link to the chat-ui landing. - components.md: new component reference page with an assistant-ui→jcode-ui mapping table + props tables for every component (closes the assistant-ui feature-parity gap). - ChatDemo: rewrote the layout to flex/flex-col + min-h-0 so the virtualized Thread gets a concrete height (was rendering empty). Verified end-to-end with headless Chrome: - web-react: boots, loads session timeline (real history), streams a live prompt (model replied), tool cards render (✓ shell/read/edit +N/-M), Stop button swaps correctly, theme tokens applied, no JS errors. - site /chat-ui: ChatDemo streams the full scripted conversation (message→tool →approval) with virtualization; docs index + sub-pages render standalone. Build artifacts (jcode-new binary, *.tsbuildinfo) gitignored.
…ui parity); wire ⌘K
Closes the assistant-ui component-catalog gaps found in the parity audit. Every
assistant-ui component now has a jcode-ui equivalent, documented in components.md
with a full mapping table (✅ library / 🟡 product-level / field-driven).
New components (jcode-ui):
- Reasoning: collapsible model thinking block ('Thought for Ns'), markdown,
driven by message.reasoning. Mirrors assistant-ui Reasoning.
- Sources: citation chip list with snippet popovers, driven by
message.sources (MessageSource[]). Mirrors assistant-ui Sources.
- Attachment + AttachmentList: standalone image-attachment thumbnails (also
embedded in ChatInput). Mirrors assistant-ui Attachment.
Message now renders Reasoning (before body) + Sources (after body) automatically
when those fields are present. Added reasoning/sources/MessageSource to the core
Message type.
components.md: rewrote the assistant-ui→jcode-ui mapping to cover ALL 16 catalog
entries (Thread, ThreadList, Composer, Attachment, Markdown, DiffViewer, Image,
Context Display, Message Timing, Reasoning, Sources, Tool Fallback, Tool Group,
Assistant Modal/Sidebar, Model Selector, makeAssistantToolUI) with status +
notes. Added reference sections for Reasoning/Sources/Attachment with props.
web-react:
- App.tsx: wired global keyboard shortcuts (⌘K command palette, ⌘N new chat,
Esc closes overlays) — was missing entirely.
- main.tsx: dev-only window.__jcodeStore exposure for testing (stripped in prod).
- vite-env.d.ts: added (vite/client types for import.meta.env).
Demo (site/playground): the scripted conversation now ends with a message that
has reasoning + sources, so the ChatUIPage live demo showcases those components.
Verified end-to-end (headless Chrome):
- web-react: all 6 views render (ChatView, Automations w/ real data, Channels,
SetupView, AuthGate, CommandPalette via ⌘K + store toggle). No JS errors.
- site: all 8 pages render without regression (Home/Desktop/CLI/Showcase/Docs/
Chat-UI/Chat-UI Docs/Privacy).
- Demo single-play: Reasoning ('Thought process') + Sources (2 chips) render on
the final assistant message; production build bundles them.
…cts it) Tauri 2 strictly validates the config schema and rejects unknown fields. The override config had a $comment field for documentation, which crashed desktop-react-dev on startup: Error: Additional properties are not allowed (`$comment` was unexpected) Removed the $comment (and the redundant $schema). The build block alone is the only override needed; everything else is inherited from tauri.conf.json via deep merge. Verified: make desktop-react-dev now boots the Tauri window + Go sidecar + Vite dev server end-to-end.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/jcode-ui/src/components/Reasoning.tsx`:
- Around line 27-34: The toggle button in Reasoning should expose its
open/closed state to assistive tech by adding the missing aria-expanded
attribute. Update the button in the Reasoning component so the disclosure
control reflects the expanded state using the existing expanded value, keeping
the rest of the onClick and ChevronDownIcon behavior unchanged.
In `@packages/jcode-ui/src/components/Sources.tsx`:
- Around line 24-31: The source toggle button in Sources.tsx is missing the
expanded-state accessibility attribute. Update the button in the Sources
component so it exposes aria-expanded based on whether openId matches s.id,
alongside the existing onClick toggle logic, to let screen readers announce the
open/closed state of each source snippet.
- Around line 34-38: The link in Sources.tsx renders s.url directly in the
anchor href, which can allow javascript: URLs to execute; update the Sources
component to validate or sanitize the URL before rendering. In the Sources
render block around the s.url check, use a safe URL helper or protocol whitelist
so only http/https (or other approved schemes) are allowed, and fall back to not
rendering the link when the value is unsafe. Keep the existing anchor styling
and behavior for valid URLs, but make the href assignment depend on the
sanitized result.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9aa64423-5322-480f-8aae-bdae5d74a839
📒 Files selected for processing (13)
desktop/src-tauri/tauri.react.conf.jsonpackages/jcode-ui-core/src/types/index.tspackages/jcode-ui/src/components/Attachment.tsxpackages/jcode-ui/src/components/Message.tsxpackages/jcode-ui/src/components/Reasoning.tsxpackages/jcode-ui/src/components/Sources.tsxpackages/jcode-ui/src/index.tssite/docs/chat-ui/components.mdsite/src/playground/ChatDemo.tsxsite/src/playground/mockScript.tsweb-react/src/App.tsxweb-react/src/main.tsxweb-react/src/vite-env.d.ts
💤 Files with no reviewable changes (1)
- desktop/src-tauri/tauri.react.conf.json
✅ Files skipped from review due to trivial changes (2)
- web-react/src/vite-env.d.ts
- site/docs/chat-ui/components.md
🚧 Files skipped from review as they are similar to previous changes (7)
- web-react/src/main.tsx
- packages/jcode-ui/src/index.ts
- site/src/playground/ChatDemo.tsx
- packages/jcode-ui/src/components/Message.tsx
- packages/jcode-ui-core/src/types/index.ts
- site/src/playground/mockScript.ts
- web-react/src/App.tsx
| <button | ||
| type="button" | ||
| onClick={() => setExpanded((e) => !e)} | ||
| className="flex items-center gap-1.5 rounded-[var(--radius-md)] px-2 py-1 text-[0.75rem] italic text-[var(--color-muted-foreground)] transition-colors hover:bg-[var(--neutral-wash-soft)]" | ||
| > | ||
| <ChevronDownIcon className={`h-3 w-3 transition-transform ${expanded ? 'rotate-180' : ''}`} /> | ||
| {label} | ||
| </button> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add aria-expanded to the toggle button.
The disclosure button lacks aria-expanded, so screen reader users can't determine whether the reasoning content is open or collapsed. Adding aria-expanded={expanded} is a one-attribute fix.
♿ Proposed fix
<button
type="button"
+ aria-expanded={expanded}
onClick={() => setExpanded((e) => !e)}
className="flex items-center gap-1.5 rounded-[var(--radius-md)] px-2 py-1 text-[0.75rem] italic text-[var(--color-muted-foreground)] transition-colors hover:bg-[var(--neutral-wash-soft)]"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| type="button" | |
| onClick={() => setExpanded((e) => !e)} | |
| className="flex items-center gap-1.5 rounded-[var(--radius-md)] px-2 py-1 text-[0.75rem] italic text-[var(--color-muted-foreground)] transition-colors hover:bg-[var(--neutral-wash-soft)]" | |
| > | |
| <ChevronDownIcon className={`h-3 w-3 transition-transform ${expanded ? 'rotate-180' : ''}`} /> | |
| {label} | |
| </button> | |
| <button | |
| type="button" | |
| aria-expanded={expanded} | |
| onClick={() => setExpanded((e) => !e)} | |
| className="flex items-center gap-1.5 rounded-[var(--radius-md)] px-2 py-1 text-[0.75rem] italic text-[var(--color-muted-foreground)] transition-colors hover:bg-[var(--neutral-wash-soft)]" | |
| > | |
| <ChevronDownIcon className={`h-3 w-3 transition-transform ${expanded ? 'rotate-180' : ''}`} /> | |
| {label} | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/jcode-ui/src/components/Reasoning.tsx` around lines 27 - 34, The
toggle button in Reasoning should expose its open/closed state to assistive tech
by adding the missing aria-expanded attribute. Update the button in the
Reasoning component so the disclosure control reflects the expanded state using
the existing expanded value, keeping the rest of the onClick and ChevronDownIcon
behavior unchanged.
| <button | ||
| type="button" | ||
| onClick={() => setOpenId((id) => (id === s.id ? null : s.id))} | ||
| className="inline-flex items-center gap-1 rounded-[var(--radius-pill)] border border-[var(--color-border)] bg-[var(--color-muted)] px-2 py-0.5 text-[0.7rem] text-[var(--color-foreground)] transition-colors hover:bg-[var(--neutral-wash-soft)]" | ||
| > | ||
| {s.url && <LinkIcon className="h-2.5 w-2.5" />} | ||
| <span className="max-w-[180px] truncate">{i + 1}. {s.title}</span> | ||
| </button> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add aria-expanded to the source toggle button.
The source chip button lacks aria-expanded, so screen reader users can't determine whether the snippet dropdown is open. Adding aria-expanded={openId === s.id} is a one-attribute fix.
♿ Proposed fix
<button
type="button"
+ aria-expanded={openId === s.id}
onClick={() => setOpenId((id) => (id === s.id ? null : s.id))}
className="inline-flex items-center gap-1 rounded-[var(--radius-pill)] border border-[var(--color-border)] bg-[var(--color-muted)] px-2 py-0.5 text-[0.7rem] text-[var(--color-foreground)] transition-colors hover:bg-[var(--neutral-wash-soft)]"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| type="button" | |
| onClick={() => setOpenId((id) => (id === s.id ? null : s.id))} | |
| className="inline-flex items-center gap-1 rounded-[var(--radius-pill)] border border-[var(--color-border)] bg-[var(--color-muted)] px-2 py-0.5 text-[0.7rem] text-[var(--color-foreground)] transition-colors hover:bg-[var(--neutral-wash-soft)]" | |
| > | |
| {s.url && <LinkIcon className="h-2.5 w-2.5" />} | |
| <span className="max-w-[180px] truncate">{i + 1}. {s.title}</span> | |
| </button> | |
| <button | |
| type="button" | |
| aria-expanded={openId === s.id} | |
| onClick={() => setOpenId((id) => (id === s.id ? null : s.id))} | |
| className="inline-flex items-center gap-1 rounded-[var(--radius-pill)] border border-[var(--color-border)] bg-[var(--color-muted)] px-2 py-0.5 text-[0.7rem] text-[var(--color-foreground)] transition-colors hover:bg-[var(--neutral-wash-soft)]" | |
| > | |
| {s.url && <LinkIcon className="h-2.5 w-2.5" />} | |
| <span className="max-w-[180px] truncate">{i + 1}. {s.title}</span> | |
| </button> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/jcode-ui/src/components/Sources.tsx` around lines 24 - 31, The
source toggle button in Sources.tsx is missing the expanded-state accessibility
attribute. Update the button in the Sources component so it exposes
aria-expanded based on whether openId matches s.id, alongside the existing
onClick toggle logic, to let screen readers announce the open/closed state of
each source snippet.
| {s.url && ( | ||
| <a href={s.url} target="_blank" rel="noreferrer" className="mb-1 block truncate font-medium text-[var(--color-primary)]"> | ||
| {s.title} | ||
| </a> | ||
| )} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate s.url to prevent javascript: protocol XSS.
href={s.url} renders the URL without protocol validation. If MessageSource.url contains a javascript: URI (e.g., via manipulated model/backend output), clicking the link executes arbitrary JavaScript. React 18 does not block javascript: URLs in href attributes.
🔒️ Proposed fix
{s.url && (
- <a href={s.url} target="_blank" rel="noreferrer" className="mb-1 block truncate font-medium text-[var(--color-primary)]">
+ <a href={s.url && /^https?:\/\//i.test(s.url) ? s.url : undefined} target="_blank" rel="noreferrer" className="mb-1 block truncate font-medium text-[var(--color-primary)]">
{s.title}
</a>
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {s.url && ( | |
| <a href={s.url} target="_blank" rel="noreferrer" className="mb-1 block truncate font-medium text-[var(--color-primary)]"> | |
| {s.title} | |
| </a> | |
| )} | |
| {s.url && ( | |
| <a href={s.url && /^https?:\/\//i.test(s.url) ? s.url : undefined} target="_blank" rel="noreferrer" className="mb-1 block truncate font-medium text-[var(--color-primary)]"> | |
| {s.title} | |
| </a> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/jcode-ui/src/components/Sources.tsx` around lines 34 - 38, The link
in Sources.tsx renders s.url directly in the anchor href, which can allow
javascript: URLs to execute; update the Sources component to validate or
sanitize the URL before rendering. In the Sources render block around the s.url
check, use a safe URL helper or protocol whitelist so only http/https (or other
approved schemes) are allowed, and fall back to not rendering the link when the
value is unsafe. Keep the existing anchor styling and behavior for valid URLs,
but make the href assignment depend on the sanitized result.
…le + Automations/Channels parity Closes the major feature gaps vs the Vue app. Each ported file was read in full from the Vue source and typechecks individually. ChatInput.tsx (product composer, ~1.2k lines): - Full port of the Vue 2.2k-line composer: autosizing textarea, send/queue/stop (IME-safe), slash-command menu, MODE picker (approval/plan/full_access), MODEL picker (current/favorites/recent/all-providers with capability dots + context limit + manage-models dialog), EFFORT picker, '+' menu (attach images, slash insert, Goal arming), image attachments (paste + file picker + thumbnails), type-ahead queue chips, ⌘L focus, click-outside. ChatView now uses this product ChatInput instead of the library's minimal one. SettingsDialog.tsx (~1.5k lines): - Full port of the Vue 2.7k-line settings: 8 tabs (Providers/Models/MCP/Skills/ Appearance/Browser/Remote/Usage). Providers tab fully ported (CRUD + catalog + advanced config + custom models). MCP has OAuth-login polling. Browser has site-permissions editor. Usage has totals + trend chart. Opened via ⌘, and the header gear button. useTheme.ts + ThemeToggle.tsx: - Theme system ported (system/light/dark + 7 named themes, useSyncExternalStore, localStorage persistence, applies data-theme + .dark class). Toggle button in the header (sun/moon flip + swatch dropdown). AutomationsView.tsx: full CRUD (create/edit form with schedule/mode/project, run history with filter, templates picker, enable/disable, run-now, delete). ChannelsView.tsx: WeChat QR-login flow (login → QR → 2s poll → online → logout) + enable/disable + BLE card. ProjectHeader: added settings gear + ThemeToggle. App.tsx: renders SettingsDialog, ⌘, shortcut. Verified via headless Chrome: model+mode pickers render, Settings opens with all 8 tabs + real provider data, theme toggle flips .dark class, Automations shows real automation + templates + create, Channels shows WeChat card. No JS errors. typecheck + production build pass. Also: untracked the accidentally-committed internal/web/dist-react build output and gitignored it.
1 participant
Summary
Migrates the product UI from Vue 3 to React 18, built on a new reusable, npm-publishable component library. This is the full scaffold + working build chain; the Vue app stays as production until feature parity is verified.
What's added
packages/jcode-ui-core— framework-agnostic core`packages/jcode-ui` — styled components (→ npm: `jcode-ui`)
`web-react/` — React product app
`site/` — live demo + docs
Verification (each step gated)
Migration status
Vue remains production. The React app builds to `internal/web/dist-react/` (parallel). The switch-over (`make build-web FRONTEND=web-react` + Go embed + Tauri frontendDist) happens once `web-react` reaches feature parity. Full plan + risk register in the conversation; design rationale in `packages/jcode-ui/README.md` + `site/docs/chat-ui/`.
The component library is the migration's organizing principle and is independently valuable — it's npm-publishable as `jcode-ui` + `jcode-ui-core` for anyone building an agent/copilot UI.
Test plan
🤖 Generated with ZCode
Summary by CodeRabbit
jcode-ui+ headlessjcode-ui-core) with default tool renderers and theme-aware styling./chat-uidocumentation and a dedicated docs area covering runtime, primitives, theming, and tool renderers.