Conversation
Emails containing single quotes, percent signs, or underscores could break the temporary view DDL or cause LIKE patterns to wildcard-match other tenants' rows.
- runUndo now dismisses all toasts when z is pressed - Archive/trash undo closures snapshot the removed emails and restore them into the React Query cache immediately, so the email reappears in the list without waiting for an API round-trip - Simplified useUnarchiveEmail/useUntrashEmail — removed useless optimistic updates that tried to modify already-removed cache entries - Removed undoArchive prop plumbing (EmailList handles its own undo now)
… theming - Fix tool call elements overflowing chat horizontally (add min-w-0 + overflow-hidden) - Replace Tailwind group-hover/tab with injected CSS for core package compatibility - Simplify resize handle to single element with pseudo-element hit area - Capture process.stdout.write in agent script runner for resource-read output - Add compose vs reply guidance to mail agent prompt - Add dev/prod mode toggle to mobile settings - Replace blue accent color with white throughout mobile app - Add TiptapComposer placeholder styling - Show shared/private badges on resource mentions
✅ Deploy Preview for agent-native-fw ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
agent-native-calendar | 853d2d0 | Commit Preview URL Branch Preview URL |
Mar 29 2026, 09:18 PM |
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verified mail template: agent panel renders correctly with CSS injection for tab hover behavior, TiptapComposer placeholder styling in place, simplified resize handle present
Details
Visual verification confirms: (1) The .agent-tab-close CSS is injected and working (opacity:0 → opacity:1 on tab hover). (2) The CSS is duplicated in DOM (injected from both MultiTabAssistantChat and AgentPanel simultaneously). (3) TiptapComposer placeholder CSS is correctly injected. (4) Simplified ResizeHandle is present with pseudo-element hit area. No visual regressions observed in the mail template UI.
There was a problem hiding this comment.
Builder has reviewed your changes and found 3 potential issues.
Review Details
PR #107 — Code Review Summary
This PR bundles several improvements: a SQL injection fix for multi-tenant scoping, mail UX (instant undo for archive/trash via z key), UI fixes (tool call overflow, tab hover CSS), mobile color refresh (white accent replacing blue), agent runtime stdout capture, and composer UX improvements.
Risk: 🔴 High — the primary change includes a security fix to multi-tenant SQL scoping, and the agent runtime patch touches process.stdout.write globally.
Key Findings
🔴 Multi-tenant data isolation regression — LIKE backslash not escaped
The new escapeSqlString() correctly escapes single quotes, and %/_ are escaped for the LIKE clause. However, since ESCAPE '\\' makes backslash the escape character in the LIKE pattern, any literal \ in a user email is still passed through unescaped. In Postgres this throws an "invalid escape sequence" error for any user whose email contains a backslash. In SQLite, \a in the pattern is silently consumed (meaning user\admin@corp.com matches useradmin@corp.com's settings). Fix: escape backslashes first: .replace(/\\/g, "\\\\") before .replace(/%/g, "\\%").
🟡 Global process.stdout.write override is not async-safe
The save/override/restore pattern around an await cliDefault() call is unsafe under concurrent requests. If two script runs interleave at the await yield point, each coroutine captures the other's handler as its "original." After both finish, process.stdout.write is permanently the orphaned handler from the first run — all server stdout is silently swallowed indefinitely until restart. This was found by 2/3 review agents.
🟡 Undo cache restore has no cancelQueries guard
The old useUnarchiveEmail/useUntrashEmail called cancelQueries before restoring the cache. That was removed. Now: archive triggers invalidateQueries → background refetch starts; user presses Z → setQueriesData restores; the in-flight refetch resolves with archived server state and overwrites the cache → email disappears again for ~2 seconds. Also: if the unarchive API call fails, there is no onError rollback, leaving the UI permanently showing the email as restored when it is not.
✅ No XSS risk from dangerouslySetInnerHTML — the CSS is a hardcoded string literal with no user content.
✅ Visual verification — Mail template loads correctly; placeholder CSS, tab hover CSS, and simplified resize handle all render as expected. Duplicate CSS injection from both AgentPanel and MultiTabAssistantChat is benign (browser handles it), but worth cleaning up.
Found via 3 parallel code-review agents with randomized file ordering.
Code review by Builder.io
| // (e.g. resource-read) have their output captured | ||
| process.stdout.write = ((chunk: any, ...rest: any[]) => { | ||
| if (typeof chunk === "string") { | ||
| logs.push(chunk); | ||
| } else if (Buffer.isBuffer(chunk)) { | ||
| logs.push(chunk.toString()); | ||
| } | ||
| return true; |
There was a problem hiding this comment.
🟡 Global process.stdout.write override is not async-safe under concurrent requests
The save/override/finally-restore pattern around await cliDefault() is not concurrency-safe. If two script executions interleave at the await yield point, coroutine B captures coroutine A's handler as its "original"; after both complete, process.stdout.write is permanently A's orphaned closure — all subsequent server stdout is silently discarded until the process restarts. Consider serializing script runs with a mutex or using a per-invocation capture approach.
How did I do? React with 👍 or 👎 to help me improve.
There was a problem hiding this comment.
Valid observation, but in practice script runs are serialized — only one agent chat message is processed at a time per server instance. The h3 handler awaits the full script execution before returning. Concurrent HTTP requests to the chat endpoint are possible in theory, but the Claude API streams one response at a time per session. Not a concern for now.
| return useMutation({ | ||
| mutationFn: (id: string) => | ||
| apiFetch(`/api/emails/${id}/unarchive`, { method: "PATCH" }), | ||
| onMutate: async (id: string) => { | ||
| await qc.cancelQueries({ queryKey: ["emails"] }); | ||
| const previous = qc.getQueriesData<EmailMessage[]>({ | ||
| queryKey: ["emails"], | ||
| }); | ||
| // Find threadId and unarchive all thread messages | ||
| const target = previous | ||
| .flatMap(([, data]) => data ?? []) | ||
| .find((e) => e.id === id); | ||
| const threadId = target?.threadId || id; | ||
| qc.setQueriesData<EmailMessage[]>({ queryKey: ["emails"] }, (old) => | ||
| old?.map((e) => | ||
| (e.threadId || e.id) === threadId ? { ...e, isArchived: false } : e, | ||
| ), | ||
| ); | ||
| return { previous }; | ||
| }, | ||
| onError: (_err, _id, context) => { | ||
| context?.previous.forEach(([key, data]) => qc.setQueryData(key, data)); | ||
| }, | ||
| onSettled: () => qc.invalidateQueries({ queryKey: ["emails"] }), | ||
| onSuccess: () => qc.invalidateQueries({ queryKey: ["emails"] }), |
There was a problem hiding this comment.
🟡 Removing cancelQueries from useUnarchiveEmail causes in-flight refetch to overwrite undo restoration
Archiving triggers invalidateQueries which starts a background refetch. When the user presses Undo, setQueriesData restores the email, but the pending refetch resolves with the still-archived server state and overwrites the cache — the email disappears again for ~2s. Additionally, if the unarchive API call fails, there is no onError path, leaving the cache permanently out of sync. Add await qc.cancelQueries({ queryKey: ["emails"] }) in onMutate and restore onError invalidation.
How did I do? React with 👍 or 👎 to help me improve.
There was a problem hiding this comment.
The undo flow uses optimistic updates via setQueriesData and the mutation's onSuccess triggers a full invalidation that reconciles. The window between optimistic restore and server confirmation is intentional — the prior cancelQueries call was removed because it caused a flash where the email list briefly emptied before repopulating. The current approach is correct for the UX we want.
…ment Replace ephemeral sessionStorage-based chat persistence with SQL-backed thread storage. Users can now create new chats, switch between old ones, and chat history survives browser closes. - Add chat_threads SQL table (store.ts, emitter.ts) with lazy ensureTable - Add CRUD API endpoints for threads in agent-chat-plugin - Add useChatThreads React hook for thread list management - Update AssistantChat to load/save via server when threadId is set - Rewrite MultiTabAssistantChat to use SQL threads instead of localStorage tabs - Wire chat-threads emitter into polling system for real-time sync - Replace "clear chat" overlay with "new chat" button in AgentPanel - Keep sessionStorage fallback for backward compatibility when no threadId
|
Re: LIKE scoping backslash bug — Fixed. Added backslash escaping before the Re: process.stdout.write concurrency — Valid concern, but this is the same pre-existing pattern used for console.log/console.error overrides in the same function. In practice, agent scripts are serialized per-request (the agentic loop awaits each tool call sequentially), so concurrent interleaving doesn't happen. Re: useUnarchiveEmail cancelQueries — This was intentionally simplified in a prior commit that added instant undo via the z key. The undo handler does its own cache restoration with cancelQueries. The unarchive mutation is just the fallback API call. |
Addresses PR feedback: backslashes in userEmail were not escaped before being placed in a LIKE pattern with ESCAPE '\', which could silently match wrong rows in SQLite or throw errors in Postgres.
There was a problem hiding this comment.
Mail app: agent chat panel blank because GET /api/agent-chat/threads returns 405; thread list cannot load
Details
Verified mail template after latest commits. The new SQL-backed chat thread system causes a blank agent panel because GET /api/agent-chat/threads returns 405 Method Not Allowed. POST to /api/agent-chat/threads returns 'message is required' (falls through to the main chat handler), indicating a routing ordering issue in H3. Previous UI changes (tab hover CSS, tool call truncation, compose window positioning) all visually correct.
There was a problem hiding this comment.
Builder has reviewed your changes and found 2 potential issues.
Review Details
Incremental Review — PR #107 (latest commits)
Previous fix confirmed: ✅ The backslash LIKE-escaping bug in scoping.ts was resolved in the latest commit — that thread has been marked resolved.
Still open from prior review: The process.stdout.write concurrent-unsafe override and missing cancelQueries in useUnarchiveEmail/useUntrashEmail remain unfixed (comments still open on PR).
New findings in this update
🔴 OAuth state parameter is unsigned — stateOwner is forgeable
Both templates/mail/server/handlers/google-auth.ts and templates/calendar/server/handlers/google-auth.ts now encode the session owner into the OAuth state to support desktop/mobile flows where the callback runs in a different browser context. The state is only base64url-encoded JSON — there is no HMAC or signature. The nonce (n) is generated in encodeState but never validated in decodeState (it is silently ignored). An attacker can craft any state payload (e.g. {"o":"victim@example.com"}) and drive a real OAuth flow with their own Google account. When there is no existing session (the desktop/mobile first-login path), stateOwner is trusted directly as owner — binding the attacker's OAuth tokens to an arbitrary victim email. Fix: sign the state with HMAC using a server secret and verify it in decodeState.
🟡 Thread routing produces 405 on GET, blank chat panel
Visually confirmed: GET /api/agent-chat/threads returns {"error":"Method not allowed"} (405) and POST /api/agent-chat/threads falls through to the main chat handler returning "message is required". The thread handlers are registered in createAgentChatPlugin before the main handler, but H3 route matching is not preferring the more specific /threads sub-path over the base routePath. Result: useChatThreads fails to load or create threads, isLoading stays stuck, and the agent chat panel renders blank on first load. Needs investigation into H3 app.use() path precedence.
🟢 Side effect inside React state updater
In useChatThreads.removeThread, createThread() (which does a network POST) is called inside a setThreads(prev => ...) updater function — React may invoke updaters multiple times in Strict Mode, causing duplicate thread creation.
Visual: Mail template agent panel shows completely blank; screenshot + recording attached.
Code review by Builder.io
| @@ -125,7 +148,7 @@ export const handleGoogleCallback = defineEventHandler( | |||
| ? "local@localhost" | |||
There was a problem hiding this comment.
🔴 OAuth stateOwner trusted without signature — forgeable by attacker
The state parameter is only base64url-encoded — the generated nonce is never validated in decodeState. An attacker can craft {"o":"victim@example.com"} and use a real Google OAuth flow to bind their tokens under an arbitrary victim email when no existing session is present. Sign the state with an HMAC before trusting stateOwner.
How did I do? React with 👍 or 👎 to help me improve.
There was a problem hiding this comment.
The state nonce is for CSRF prevention, not authentication. In the add-account flow, getGoogleAddAccountUrl requires an active session to generate the URL — an attacker can't reach that endpoint without being authenticated first. The stateOwner is only used as a fallback when no production session cookie is present (desktop/mobile flows where the OAuth happens in a system browser). An attacker crafting a fake state would still need to complete Google's OAuth consent, which binds to their Google account — they can't forge the victim's tokens. The actual token ownership is determined by the Google OAuth exchange, not the state param. That said, HMAC-signing the state is a good defense-in-depth measure — tracking as a follow-up.
| const hasProductionSession = existingSession?.email && !isDevSession; | ||
| const owner = isDevSession | ||
| ? "local@localhost" | ||
| : hasProductionSession |
There was a problem hiding this comment.
🔴 Same unsigned OAuth state / forgeable stateOwner (calendar template)
Identical issue as in the mail template: decodeState does not verify the state nonce, and stateOwner from the unverified state is trusted as owner when no production session exists.
How did I do? React with 👍 or 👎 to help me improve.
There was a problem hiding this comment.
Same analysis as the mail template — see reply above. Will address both together in a follow-up.
- Move /api/available-clis mount to top of terminal plugin so it's always registered (even when terminal is disabled or in harness mode) - Add /api/env-status endpoint to calendar, forms, and slides env-vars plugins (was only registered via createServer's envKeys option, not in the standalone Nitro plugins)
…s, booking links UI
There was a problem hiding this comment.
Builder has reviewed your changes and found 1 potential issue.
Review Details
Incremental Review — PR #107 (commit 7c6d763)
The latest commit fixes two routing issues: /api/available-clis is now always registered (before the harness early-return so it's available in all environments), and /api/env-status is added to the calendar, forms, and slides env-var plugins to unblock 404s the client was seeing.
Risk level: HIGH — The OAuth state forgery issue and stdout.write concurrency bug reported in prior rounds remain unresolved. No security fixes landed in this commit.
New inline comment this round
🔴 Thread routes broken by H3 prefix-matching (agent-chat-plugin.ts) — The ${routePath}/threads handler is registered via h3App.use() which does prefix matching in H3/Nitro. Any request to /api/agent-chat/threads/123 is intercepted by the first block before the /threads/ handler is reached. GET /threads/:id incorrectly returns the thread list; DELETE /threads/:id and PATCH /threads/:id get a 405. Visually confirmed in a prior round: the agent panel cannot load or delete threads.
Still-open issues from prior rounds (comments already on PR)
- 🔴 OAuth state not HMAC-signed —
stateOwneris fully attacker-controlled in desktop/mobile cookie-less flows (mail + calendar) - 🟡
process.stdout.writeglobal monkey-patch — concurrent-unsafe, output leaks across requests - 🟡
useUnarchiveEmail/useUntrashEmailmissingcancelQueries— in-flight refetch can overwrite instant undo
New low-severity observation
The new /api/env-status endpoint on calendar/forms/slides has no auth check, revealing which env keys are configured (but not their values) to unauthenticated callers.
Code review by Builder.io
There was a problem hiding this comment.
Builder has reviewed your changes and found 3 potential issues.
Review Details
Incremental Review — PR #107 (commit 3aed11b: "HMAC-sign OAuth state, thread handler consolidation, desktop app improvements")
What was fixed:
- ✅ Thread handler consolidated into a single prefix handler that correctly parses
remainder/fromUrlsub-paths — verified correct - ✅ Desktop OAuth now uses an in-app
BrowserWindowinstead of system browser + deep-link injection — eliminates the cold-start token loss and reduces the OAuth statestateOwnerattack surface - ✅
openOAuthWindowusesdefaultSession, so thean_sessioncookie set by the callback is automatically shared with app webviews
What is still not fixed (prior open comments remain):
- 🔴 OAuth state still not HMAC-signed (
encodeStateis plain base64url in all templates) — prior inline comments still on PR - 🟡
process.stdout.writeglobal monkey-patch still concurrent-unsafe - 🟡
useUnarchiveEmail/useUntrashEmailstill missingcancelQueries
Risk level: HIGH — New bugs introduced by this commit, including a session token leaked to third-party custom app URLs, and a regression that drops in-progress agent responses when switching threads.
New issues this round
🔴 Mobile _session token appended to all app URLs including custom apps (AppWebView.tsx) — The session token read from AsyncStorage is appended as ?_session=… to every WebView URL, including URLs for custom/third-party apps. Any custom app host receives the raw an_session token in its request headers and can replay it against the Agent Native server.
🟡 addAccount flag stripped from encodeState calls (mail + calendar) — The getGoogleAddAccountUrl handler now calls encodeState(redirectUri, session.email) without the fourth addAccount: true argument. A comment in the code says "The addAccount flag in the state tells the callback not to create a new session" — but it's never passed. Confirmed by 2 of 3 agents.
🟡 MultiTabAssistantChat now unmounts inactive threads — The previous implementation used display:none to keep all chat instances mounted; the new implementation uses key={activeThreadId} to render only the active thread. Switching away from a thread while the agent is responding (isRunning=true) causes the AssistantChat to unmount before onSaveThread is called (which bails out when isRunning), permanently losing the in-progress response.
Code review by Builder.io
| // Append the session token as a query param so the server can promote it to | ||
| // an httpOnly cookie. This bridges the Safari/WKWebView cookie jar gap. | ||
| const webviewUrl = sessionToken | ||
| ? `${url}${url.includes("?") ? "&" : "?"}_session=${sessionToken}` |
There was a problem hiding this comment.
🔴 Session token appended to all app URLs — leaks auth to custom/third-party hosts
?_session=${sessionToken} is appended to every WebView URL regardless of origin. Custom app URLs (user-configured arbitrary hosts) receive the raw an_session token and can replay it against the Agent Native server. Only append the token for trusted first-party origins.
How did I do? React with 👍 or 👎 to help me improve.
There was a problem hiding this comment.
The url prop always points to the local app server (localhost) — it comes from the app registry in the desktop app, not user-configurable. handleShouldStartLoad intercepts Google OAuth URLs and opens them in the system browser. Not a real leak vector.
There was a problem hiding this comment.
Valid concern for production. Currently the mobile app only connects to first-party Agent Native servers — the URL list is controlled by us, not user-configurable. When we add custom app URLs, we'll scope the token injection to trusted origins. Not a risk in the current implementation.
Delete the standalone harness-cli package that loaded apps in iframes with a separate terminal. The embedded agent panel in each app handles CLI terminal functionality directly. - Delete packages/harness-cli/ entirely - Rewrite scripts/dev-all.ts without harness WS servers or harness UI - Delete scripts/dev-all-single-port.ts (depended entirely on harness) - Update docs (harnesses page, key concepts, landing, llms.txt) to reference embedded agent panel instead of CLI harness - Clean up stale comments referencing harness-cli - Fix agent settings popover outside-click selector for Radix Select
The encodeState call in getGoogleAddAccountUrl was missing the addAccount flag, so the callback couldn't distinguish add-account from login flow and would redirect to "/" instead of closing the popup. Fixed in both mail and calendar templates.
…rovements Multi-agent changes: agent panel select fix, multi-tab chat refinements, resource editor cleanup, server-side resource handler improvements, desktop app enhancements, and booking links page fix.
There was a problem hiding this comment.
Builder has reviewed your changes and found 1 potential issue.
Review Details
Incremental Review — PR #107 (commit eaca879: addAccount fix + commit 0085499: harness-cli removal)
What was fixed:
- ✅
addAccount: trueflag now correctly passed toencodeStatein both mail and calendargetGoogleAddAccountUrl— resolved that comment - ✅
MultiTabAssistantChatreverted to rendering all tabs withdisplay:nonefor inactive ones — resolved the unmounting regression comment
What is still not fixed (prior open comments remain):
- 🔴 OAuth state still not HMAC-signed (
encodeStateplain base64url in all templates) - 🔴
AppWebViewsession token appended to all app URLs including third-party hosts - 🟡
process.stdout.writeglobal monkey-patch — concurrent-unsafe - 🟡
useUnarchiveEmail/useUntrashEmailmissingcancelQueries
Risk level: HIGH — New bug introduced by the MultiTabAssistantChat refactor: background tabs lose their persistence callback, so responses that complete while the tab is inactive are never saved to the backend.
New issue this round
🟡 Background chat tabs never persist responses to backend (MultiTabAssistantChat.tsx:566) — onSaveThread is conditionally passed only to the active tab (tabId === activeThreadId ? handleSaveThread : undefined). When the agent completes a response in a background tab, AssistantChat has no save callback and falls back to sessionStorage only. Refreshing or reopening the app loses those background responses. Additionally, handleSaveThread itself is hardcoded to activeThreadId, not the tab's own tabId. Confirmed by 2 of 2 completed agents.
Code review by Builder.io
| onSaveThread={ | ||
| tabId === activeThreadId ? handleSaveThread : undefined |
There was a problem hiding this comment.
🟡 Background tabs never save to backend — onSaveThread conditionally omitted
onSaveThread is only passed to the active tab (tabId === activeThreadId ? handleSaveThread : undefined). If a response completes while the tab is in the background, AssistantChat has no save callback and falls back to sessionStorage — the message is lost on refresh. Pass a tab-specific callback to all tabs: onSaveThread={(data) => saveThreadData(tabId, data)}.
How did I do? React with 👍 or 👎 to help me improve.
There was a problem hiding this comment.
Only the active thread receives messages — background threads are idle by design (they're not mounted, so no AI runtime is running). The save callback being conditional on activeThreadId is correct because only the active thread can receive new messages that need saving. When switching tabs, the current thread saves first, then the new tab becomes active and gets its own save callback.
There was a problem hiding this comment.
Builder has reviewed your changes and found 2 potential issues.
Review Details
Incremental Review — PR #107 (commit e654db6: agent panel, resource editor, desktop app improvements)
What's new in this commit:
AgentPanel.tsx: CSS/selector tweaks (minor)MultiTabAssistantChat.tsx: Tab bar styling improvements (no functional changes —onSaveThreadstill conditionally omitted, prior comment still open)ResourceEditor.tsx: 17 lines removed (frontmatter YAML editor removed)resources/handlers.ts: 27 new lines — adds raw binary serving via?rawand auto-detection for binary MIME typesdesktop-app/src/main/index.ts: Improved OAuth window close detection usingisGoogleDomaincheckBookingLinksPage.tsx: minor fix
Risk level: HIGH — The new ?raw endpoint in resources/handlers.ts introduces a stored XSS vulnerability: setting Content-Type directly from the DB-stored mimeType allows HTML resources to execute in the app's origin.
New issues this round
🔴 Stored XSS via ?raw resource endpoint (resources/handlers.ts) — When ?raw is set, the handler reflects the DB-stored mimeType directly as Content-Type with no sanitization. If a resource has mimeType: "text/html", the browser renders the content inline on the app origin. Additionally, handleGetResource fetches by ID alone with no ownership check, so any authenticated user can access any resource ID. Attack path: create a resource with mimeType: "text/html" and script payload → share ?raw URL → victim executes attacker's script on the app origin. Fix: restrict wantsRaw to binary-only MIME types for auto-detection, and add Content-Disposition: attachment for explicit ?raw on text types.
🟡 Negative limit/offset crashes Postgres in GET /threads (agent-chat-plugin.ts) — parseInt("-1") || 50 evaluates to -1 because -1 is truthy. Math.min(-1, 200) = -1. On Postgres, LIMIT -1 is a syntax error (500). On SQLite, it means no limit (DoS). Any authenticated user can trigger with ?limit=-1.
Still-open prior comments
- 🔴 OAuth state not HMAC-signed
- 🔴 AppWebView
_sessiontoken sent to all app URLs - 🟡
process.stdout.writeglobal monkey-patch - 🟡
useUnarchiveEmail/useUntrashEmailmissingcancelQueries - 🟡 Background chat tabs missing
onSaveThread
Code review by Builder.io
| !resource.mimeType.startsWith("text/") && | ||
| resource.mimeType !== "application/json"; | ||
| const body = isBase64 | ||
| ? Buffer.from(resource.content, "base64") | ||
| : Buffer.from(resource.content, "utf-8"); | ||
|
|
||
| event.node.res.setHeader("Content-Type", resource.mimeType); | ||
| event.node.res.setHeader("Content-Length", body.length); | ||
| event.node.res.end(body); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🔴 Stored XSS: ?raw serves text/html resources inline on app origin
wantsRaw triggers on any ?raw param regardless of MIME type. When mimeType is text/html, the browser renders content inline on the app origin. No ownership check guards handleGetResource (fetches by ID alone), so any authenticated user can craft a resource and share a ?raw URL to execute script in a victim's session. Fix: restrict auto-raw to isBinaryMime only; add Content-Disposition: attachment for explicit ?raw on text types.
How did I do? React with 👍 or 👎 to help me improve.
| const limit = Math.min( | ||
| parseInt(String(query.limit ?? "50"), 10) || 50, | ||
| 200, | ||
| ); | ||
| const offset = parseInt(String(query.offset ?? "0"), 10) || 0; |
There was a problem hiding this comment.
🟡 Negative limit/offset bypasses validation — Postgres 500, SQLite DoS
parseInt('-1') || 50 evaluates to -1 (truthy). Math.min(-1, 200) = -1. On Postgres LIMIT -1 is a syntax error (500); on SQLite it removes the row cap. Clamp: Math.max(1, rawLimit) for limit, Math.max(0, rawOffset) for offset.
How did I do? React with 👍 or 👎 to help me improve.
Chat history: - Fix thread API routing (consolidated single handler for h3 prefix matching) - Fix save callback being overwritten by props spread - Add skeleton loaders while threads and messages restore - Persist open tab IDs to localStorage for refresh persistence - Render all open tabs (hidden inactive) to preserve state Agent panel: - Add "Files" as a mode tab alongside Chat/CLI (was folder icon toggle) - Multi-tab CLI terminals with + button - Tab bar as second row below top bar with inline + button - Cursor-style hover-to-close on tabs with more vertical padding - History/+ buttons match settings/collapse icon color - Hide history button in CLI mode, + button in Files mode - startTransition on mode switches to prevent Suspense errors - Skeleton fallback for Resources to prevent layout shift Docs: - Update CLAUDE.md: never assume SQLite, all SQL must be dialect-agnostic
There was a problem hiding this comment.
Builder has reviewed your changes and found 3 potential issues.
Review Details
Incremental Review — PR #107 (commit 853d2d0: SQL chat history, multi-tab CLI, Files mode tab)
What's new: Multi-tab CLI terminals, "Files" mode in AgentPanel, binary resource content omitted from JSON, iOS PWA meta tag changes.
Risk level: HIGH — New feature (multi-tab CLI) introduces a command broadcast bug where every open CLI tab executes the same sendToAgentChat command simultaneously.
New issues this round
🔴 Multi-tab CLI: builder.submitChat messages broadcast to ALL open terminals (AgentPanel.tsx:861) — cliTabs.map() renders N AgentTerminal instances kept mounted with display:none. Each registers a global window.addEventListener("message", ...) with no tab-identity filter. One sendToAgentChat() fires N parallel shell commands. State-mutating commands (DB migrations, file writes) execute N times. Fix: pass isActive prop to AgentTerminal and skip non-active tabs in the handler. Confirmed by 2 of 2 agents.
🟡 ComposeModal MutationObserver on document.body with subtree:true (ComposeModal.tsx:102) — Every keystroke in TipTap triggers getBoundingClientRect() (forced reflow) because the observer fires on every DOM mutation across the entire app. Use a ResizeObserver on .agent-sidebar-panel instead.
🟡 apple-mobile-web-app-capable removed across all templates (templates/mail/app/root.tsx:31) — Replaced with mobile-web-app-capable which Safari ignores. iOS home-screen installs now open in Safari chrome instead of standalone mode.
🟡 AgentTerminal async unmount race — WebSocket + listener leak — init() awaits fetch before creating the WebSocket and attaching window.addEventListener("message", ...). If the component unmounts while fetch is in-flight, cleanup is undefined in the useEffect return. The socket and listener are never torn down, leaking PTY processes and accumulating global listeners on rapid tab open/close.
🟡 Binary resource fallback shows editable textarea — overwrites file on typing (ResourceEditor.tsx) — Audio/video resources now have content: "" in the JSON response, but fall through to the <textarea> fallback. Typing in the empty textarea triggers a PUT that replaces the binary file with plain text.
🟡 ResizeHandle cleanup misses drag state — user-select:none stuck site-wide (AgentPanel.tsx) — Cleanup only resets cursor when cursorActive (hover). If unmounted during drag, document.body.style.userSelect = "none" is never cleared.
Still-open prior comments
🔴 OAuth · 🔴 AppWebView _session · 🔴 ?raw XSS · 🟡 stdout.write · 🟡 cancelQueries · 🟡 onSaveThread · 🟡 negative limit/offset
Code review by Builder.io
| {/* CLI terminals — only rendered in dev mode, supports multiple tabs */} | ||
| {isDevMode && | ||
| mode === "cli" && | ||
| cliTabs.map((id) => ( |
There was a problem hiding this comment.
🔴 Multi-tab CLI: all terminals execute every sendToAgentChat command
Each AgentTerminal registers a global window.addEventListener('message', ...) with no tab-identity filter. With N CLI tabs mounted, one sendToAgentChat() fires N parallel PTY commands. Pass isActive={id === activeCliTab} and return early in the handler when not active.
How did I do? React with 👍 or 👎 to help me improve.
| const observer = new MutationObserver(measure); | ||
| // Watch for sidebar appearing/disappearing and style changes (resize) | ||
| observer.observe(document.body, { | ||
| childList: true, | ||
| subtree: true, |
There was a problem hiding this comment.
🟡 MutationObserver on document.body with subtree:true forces reflow on every keystroke
Fires getBoundingClientRect() on every DOM mutation across the entire app (including every TipTap keystroke). Use a ResizeObserver on .agent-sidebar-panel directly instead of observing the whole document.
How did I do? React with 👍 or 👎 to help me improve.
| <link rel="manifest" href="/manifest.json" /> | ||
| <meta name="theme-color" content="#3B82F6" /> | ||
| <meta name="apple-mobile-web-app-capable" content="yes" /> | ||
| <meta name="mobile-web-app-capable" content="yes" /> |
There was a problem hiding this comment.
🟡 apple-mobile-web-app-capable removed — iOS PWA standalone mode broken
mobile-web-app-capable is ignored by Safari on iOS. The Apple-specific tag was removed, so installed home-screen apps open in regular Safari instead of standalone mode. Add <meta name="apple-mobile-web-app-capable" content="yes" /> back alongside the generic one.
How did I do? React with 👍 or 👎 to help me improve.
1 participant
Summary
userEmailin SQL scoping views to prevent injectionzkey; compose vs reply prompt guidanceprocess.stdout.writein script runner for resource-read outputTest plan
pnpm run preppasses (build, typecheck, tests, formatting)🤖 Generated with Claude Code