feat: codebase refactor and improvements — modular architecture, tests, and backend enhancements#681
Conversation
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-Claude) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Extract 913-line component into focused sub-modules: - sidebar/chat-list.tsx: ChatItem, groupChatsByTime, ChatGroup, ChatList - sidebar/chat-list-dialogs.tsx: ChatContextMenu, DeleteChatDialog, BulkDeleteDialog, BulkSelectionBar - sidebar/sidebar-user.tsx: SidebarUser profile footer - sidebar/use-sidebar-actions.ts: all state + handlers as a hook - sidebar/index.ts: barrel exports app-sidebar.tsx reduced from 913 to 281 lines (orchestrator only)
Extract 1335-line compound component into 10 focused sub-files: - prompt-input-context.tsx: contexts, hooks, PromptInputProvider - prompt-input-attachments.tsx: attachment display components - prompt-input-primitives.tsx: Body, Textarea, Header, Footer, Tools, Button - prompt-input-action-menu.tsx: ActionMenu wrappers - prompt-input-submit.tsx: SubmitButton with motion animation - prompt-input-speech.tsx: SpeechButton with Web Speech API - prompt-input-select.tsx: Select wrappers - prompt-input-hover-card.tsx: HoverCard wrappers - prompt-input-tabs.tsx: Tab layout primitives - prompt-input-command.tsx: Command palette wrappers - prompt-input-context.tsx: re-exports all sub-modules, keeps PromptInput core (354 lines)
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
🚀 Preview Deployment ReadyVercel is rebuilding the frontend with the new Convex backend URL. Vercel will post the preview URL automatically. Convex Preview Backend
🤖 Deployed automatically by GitHub Actions |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| name: Run Tests | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Setup Bun | ||
| uses: oven-sh/setup-bun@v2 | ||
|
|
||
| - name: Install dependencies | ||
| run: bun install | ||
|
|
||
| - name: Run tests | ||
| run: bunx vitest run |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium test
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 2 days ago
To fix the problem, the workflow should explicitly restrict the GITHUB_TOKEN permissions to the minimal set needed. This job only checks out the repository and runs tests, so it only needs read access to repository contents. The best fix is to add a permissions: block at the workflow root (top level, alongside name: and on:) with contents: read. This applies to all jobs that don’t override permissions, including test, without changing any existing functionality.
Concretely, in .github/workflows/test.yml, insert:
permissions:
contents: readbetween the name: Tests line and the on: block. No additional imports, methods, or definitions are needed, and no steps within jobs.test need modification.
| @@ -1,5 +1,8 @@ | ||
| name: Tests | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: [main] |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| @@ -0,0 +1,363 @@ | |||
| // @vitest-environment jsdom | |||
|
|
|||
| vi.mock("react-dom", async (importOriginal) => { | |||
|
✅ No security issues found — scanned commits: |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
27 issues found across 109 files
Confidence score: 3/5
- High-severity risk:
apps/server/convex/userDeleteBatch.tscan delete DB records even when storage deletion fails, leaving orphaned files and potential data leakage or cleanup debt. - User-facing regressions likely in
apps/web/src/components/chat/chat-message-list.tsx(auto-scroll not updating during streaming and loading indicator hidden), which can make chat feel broken or stalled. - Pay close attention to
apps/server/convex/userDeleteBatch.ts,apps/web/src/components/chat/chat-message-list.tsx, andapps/server/convex/message_helpers.ts- data integrity and UX issues could surface in production.
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/server/convex/userDeleteBatch.ts">
<violation number="1" location="apps/server/convex/userDeleteBatch.ts:107">
P1: If `ctx.storage.delete` fails with an unexpected error (e.g., transient network issue), the code logs the error but proceeds to delete the file record from the database. This creates orphaned files in storage that can never be cleaned up (data leak).
Additionally, `logger.error` is async (it hashes PII) but is called with `void`, meaning the promise is not awaited. If the function completes quickly, the log may be lost.
Recommended fix: Rethrow the error to abort the transaction (allowing retry), and use `Promise.all` to process deletions concurrently for better performance.</violation>
</file>
<file name="apps/server/convex/streamQueries.ts">
<violation number="1" location="apps/server/convex/streamQueries.ts:57">
P2: `job.options` may include legacy fields (e.g., `dynamicPrompt`, `jonMode`) that aren't allowed by `streamOptionsValidator`, causing return validation errors for existing streamJobs documents. Sanitize options before returning or extend the validator to include the deprecated fields.</violation>
</file>
<file name="apps/server/convex/chatFork.ts">
<violation number="1" location="apps/server/convex/chatFork.ts:35">
P2: Collecting the entire chat history just to slice the last 200 messages can load up to 10k messages into memory and slow this mutation. Consider fetching the fork point first and then querying only the last `MAX_FORK_MESSAGE_COPY` messages around it instead of `.collect()` on the full chat.</violation>
<violation number="2" location="apps/server/convex/chatFork.ts:107">
P2: Forking inserts new message rows but never increments `STAT_KEYS.MESSAGES_TOTAL`, so dbStats will undercount messages compared with `insertOrUpdateMessage`. Add a message-stat increment based on `messagesToCopy.length`.</violation>
</file>
<file name="apps/web/src/components/chat/chat-chain-of-thought.tsx">
<violation number="1" location="apps/web/src/components/chat/chat-chain-of-thought.tsx:71">
P2: Tool steps in `input-available` state are still running, but they never mark `isAnyStreaming`, so the chain-of-thought panel can show a completed state and stay collapsed while the tool is executing. Treat `input-available` as active/streaming so the UI remains open until output is available.</violation>
</file>
<file name="apps/server/convex/userDelete.ts">
<violation number="1" location="apps/server/convex/userDelete.ts:113">
P3: The workflow step accepts batchSize but does not pass it to deleteUserChatReadStatuses, so callers cannot control batch size for this deletion step.</violation>
<violation number="2" location="apps/server/convex/userDelete.ts:117">
P3: The workflow step ignores the batchSize argument when deleting prompt templates, making the optional input ineffective for this step.</violation>
</file>
<file name="apps/server/convex/streamJobs.ts">
<violation number="1" location="apps/server/convex/streamJobs.ts:298">
P2: cleanupStaleJobs marks stale jobs as error but never clears the related chat’s activeStreamId/status, so chats can remain stuck in a streaming state after cleanup. Mirror the chat reset used by completeStream/failStream when a job is cleaned.</violation>
</file>
<file name="apps/web/src/components/ai-elements/prompt-input-speech.tsx">
<violation number="1" location="apps/web/src/components/ai-elements/prompt-input-speech.tsx:62">
P3: This new component is not referenced anywhere in the web app. If it isn’t wired into the UI, it becomes dead code that adds maintenance overhead. Either remove it or integrate it into the prompt input flow so it’s actually used.</violation>
</file>
<file name="apps/server/convex/streamWebSearch.ts">
<violation number="1" location="apps/server/convex/streamWebSearch.ts:42">
P1: Sequential execution of web searches causes significant latency.
Executing searches one-by-one in a loop (`await execute(...)`) blocks the entire prefetch process. If `targetSearchCount` is 5 and each search takes 2s, the user waits 10s before generation starts.
These should be executed in parallel using `Promise.all`. Note that parallelizing requires careful handling of `persistProgress` to avoid race conditions when saving state (e.g., ensuring `persistProgress` handles concurrent calls or serializing the save operations).</violation>
<violation number="2" location="apps/server/convex/streamWebSearch.ts:69">
P2: Truncation logic exceeds character limit.
The logic subtracts 1 from `remainingContextChars` but appends `...` (3 characters). This causes the chunk to exceed the remaining allowance by 2 characters.
For example, if `remaining` is 5 and we slice 4 chars and add `...`, the result is 7 chars.</violation>
</file>
<file name="apps/web/src/stores/model.ts">
<violation number="1" location="apps/web/src/stores/model.ts:132">
P2: The `useModels` hook has a state synchronization bug. It initializes local `models` state from the cache once on mount, but the subscription only triggers a re-render (`forceUpdate`) without updating the local state. This means the component will fail to reflect changes when the cache updates (e.g., after `reloadModels` or background fetches).
Recommended fix: Remove the redundant local state (`useState`) and derive values directly from the `cache` object during render. The existing subscription correctly forces re-renders when the cache changes.</violation>
</file>
<file name="apps/server/convex/streamExecution.ts">
<violation number="1" location="apps/server/convex/streamExecution.ts:93">
P2: Refund the reserved daily usage when the API key is missing. As written, osschat requests that fail before streaming still consume the reserved usage, causing users to hit the daily limit early.</violation>
</file>
<file name="apps/web/src/components/ai-elements/prompt-input-action-menu.tsx">
<violation number="1" location="apps/web/src/components/ai-elements/prompt-input-action-menu.tsx:24">
P2: Base UI render props require custom components to forward refs; `PromptInputButton` does not, so the trigger won’t pass its ref to the DOM element. Wrap `PromptInputButton` in `forwardRef` (or render a ref-forwarding button) before using it as a `render` target.</violation>
</file>
<file name="apps/web/src/components/chat/chat-message-list.tsx">
<violation number="1" location="apps/web/src/components/chat/chat-message-list.tsx:12">
P1: The AutoScroll component only triggers scrolling when the message count changes. It fails to scroll when an existing message grows in size (e.g., during streaming of a long response), forcing the user to manually scroll to follow the output.</violation>
<violation number="2" location="apps/web/src/components/chat/chat-message-list.tsx:86">
P2: The `processedMessages` memoization iterates and transforms the entire message history on every render where `messages` changes. During streaming, this array updates with every token, causing O(N) reprocessing of the entire chat history (where N is the number of messages). This will lead to performance degradation in long conversations.</violation>
<violation number="3" location="apps/web/src/components/chat/chat-message-list.tsx:420">
P2: The loading indicator is hidden when an assistant message has been added to the list but is empty or skipped (e.g., during the initial connection phase or before the first token arrives). The current condition only shows the indicator if the last message is from the 'user', failing to account for the hidden assistant message that now occupies the last position.</violation>
</file>
<file name="apps/server/convex/message_helpers.ts">
<violation number="1" location="apps/server/convex/message_helpers.ts:37">
P2: Sequential database queries in loop cause performance bottleneck. Use Promise.all for concurrent execution.</violation>
<violation number="2" location="apps/server/convex/message_helpers.ts:133">
P1: Attachment validation is silently skipped if userId is missing.</violation>
<violation number="3" location="apps/server/convex/message_helpers.ts:151">
P2: Missing check for chat existence allows orphaned messages.</violation>
</file>
<file name="apps/server/convex/chatTitle.ts">
<violation number="1" location="apps/server/convex/chatTitle.ts:96">
P2: `sanitizeTitle` returns "New Chat" by default if the input sanitizes to an empty string. This masks LLM generation failures (e.g., if the model returns only whitespace/punctuation). Pass an empty string as the default value to detect invalid generation and return `null` instead.</violation>
</file>
<file name="apps/web/src/components/model-selector.tsx">
<violation number="1" location="apps/web/src/components/model-selector.tsx:79">
P2: The dropdown positioning logic does not account for page scrolling. Since `DesktopDropdown` uses `fixed` positioning in a portal (attached to `document.body`), it will remain stationary while the trigger button moves with the content during a scroll, causing visual detachment.
Consider using `position: absolute` with `window.scrollY` adjustments, or a library like `@floating-ui/react` to handle positioning robustly.</violation>
<violation number="2" location="apps/web/src/components/model-selector.tsx:254">
P3: The trigger button has `aria-haspopup="listbox"` but lacks `aria-controls` pointing to the ID of the dropdown/drawer element. This attribute is important for screen readers to associate the button with the listbox it controls.</violation>
</file>
<file name="apps/server/convex/userAuth.ts">
<violation number="1" location="apps/server/convex/userAuth.ts:123">
P2: This condition triggers a database write even when `args.email` is undefined (omitted), because `undefined !== "some@email.com"` is true. This causes unnecessary `updatedAt` patches when the client doesn't send the email field.
Check that `args.email` is defined before comparing.</violation>
<violation number="2" location="apps/server/convex/userAuth.ts:135">
P2: This condition triggers a profile update even when `name` or `avatarUrl` are omitted in `args`, causing unnecessary database writes to `updatedAt`.
Verify that arguments are defined before checking for changes.</violation>
<violation number="3" location="apps/server/convex/userAuth.ts:158">
P2: This condition triggers a user update even when `name` or `avatarUrl` are omitted in `args`, causing unnecessary database writes.
Verify that arguments are defined before checking for changes.</violation>
<violation number="4" location="apps/server/convex/userAuth.ts:307">
P2: Optimizable database queries: `requireAuthUserId` fetches the user by external ID to verify ownership, and then `ctx.db.get` fetches the same user again by ID.
Combining these into a single fetch by ID with an inline identity check reduces database load and latency.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -0,0 +1,96 @@ | |||
| import { webSearch } from "@valyu/ai-sdk"; | |||
There was a problem hiding this comment.
P1: Sequential execution of web searches causes significant latency.
Executing searches one-by-one in a loop (await execute(...)) blocks the entire prefetch process. If targetSearchCount is 5 and each search takes 2s, the user waits 10s before generation starts.
These should be executed in parallel using Promise.all. Note that parallelizing requires careful handling of persistProgress to avoid race conditions when saving state (e.g., ensuring persistProgress handles concurrent calls or serializing the save operations).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/server/convex/streamWebSearch.ts, line 42:
<comment>Sequential execution of web searches causes significant latency.
Executing searches one-by-one in a loop (`await execute(...)`) blocks the entire prefetch process. If `targetSearchCount` is 5 and each search takes 2s, the user waits 10s before generation starts.
These should be executed in parallel using `Promise.all`. Note that parallelizing requires careful handling of `persistProgress` to avoid race conditions when saving state (e.g., ensuring `persistProgress` handles concurrent calls or serializing the save operations).</comment>
<file context>
@@ -0,0 +1,96 @@
+ throw new Error("Valyu webSearch tool is missing execute()");
+ }
+
+ for (let index = 0; index < searchQueries.length; index++) {
+ const searchQuery = searchQueries[index]!;
+ const toolCallId = `web-search-${Date.now()}-${index}`;
</file context>
| onTranscriptionChange?: (text: string) => void; | ||
| }; | ||
|
|
||
| export const PromptInputSpeechButton = ({ |
There was a problem hiding this comment.
P3: This new component is not referenced anywhere in the web app. If it isn’t wired into the UI, it becomes dead code that adds maintenance overhead. Either remove it or integrate it into the prompt input flow so it’s actually used.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/ai-elements/prompt-input-speech.tsx, line 62:
<comment>This new component is not referenced anywhere in the web app. If it isn’t wired into the UI, it becomes dead code that adds maintenance overhead. Either remove it or integrate it into the prompt input flow so it’s actually used.</comment>
<file context>
@@ -0,0 +1,156 @@
+ onTranscriptionChange?: (text: string) => void;
+};
+
+export const PromptInputSpeechButton = ({
+ className,
+ textareaRef,
</file context>
|
|
||
| return ( | ||
| <div className={cn("relative inline-block", className)}> | ||
| <button |
There was a problem hiding this comment.
P3: The trigger button has aria-haspopup="listbox" but lacks aria-controls pointing to the ID of the dropdown/drawer element. This attribute is important for screen readers to associate the button with the listbox it controls.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/model-selector.tsx, line 254:
<comment>The trigger button has `aria-haspopup="listbox"` but lacks `aria-controls` pointing to the ID of the dropdown/drawer element. This attribute is important for screen readers to associate the button with the listbox it controls.</comment>
<file context>
@@ -1,1019 +1,357 @@
+
+ return (
+ <div className={cn("relative inline-block", className)}>
+ <button
+ ref={triggerRef}
+ type="button"
</file context>
|
✅ No security issues found — scanned commits: |
There was a problem hiding this comment.
2 issues found across 13 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/server/convex/userAuth.ts">
<violation number="1" location="apps/server/convex/userAuth.ts:137">
P1: Partial Fix / Data Loss Risk: While this condition prevents updates when all fields are undefined, the `patch` call inside this block still receives `undefined` for fields that aren't being updated.
For example, if `name` is updated but `args.avatarUrl` is `undefined`, the patch object becomes `{ name: "...", avatarUrl: undefined }`. In Convex, passing `undefined` to `db.patch` **deletes** the field from the document, causing unintentional data loss.
Use spread syntax or a helper to only include defined fields in the patch object.</violation>
</file>
<file name="apps/web/src/components/chat/chat-message-list.tsx">
<violation number="1" location="apps/web/src/components/chat/chat-message-list.tsx:34">
P1: The dependency array prevents the auto-scroll effect from running during message streaming.
`isStreaming`, `isAtBottom`, and `scrollToBottom` typically remain stable while a message is being generated. Because the dependencies don't change, the effect doesn't re-run when new tokens arrive, causing the scroll position to stagnate instead of following the new content.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // Update existing profile if name/avatar changed | ||
| const needsProfileUpdate = | ||
| (args.name !== undefined && profile.name !== args.name) || | ||
| (args.avatarUrl !== undefined && profile.avatarUrl !== args.avatarUrl); |
There was a problem hiding this comment.
P1: Partial Fix / Data Loss Risk: While this condition prevents updates when all fields are undefined, the patch call inside this block still receives undefined for fields that aren't being updated.
For example, if name is updated but args.avatarUrl is undefined, the patch object becomes { name: "...", avatarUrl: undefined }. In Convex, passing undefined to db.patch deletes the field from the document, causing unintentional data loss.
Use spread syntax or a helper to only include defined fields in the patch object.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/server/convex/userAuth.ts, line 137:
<comment>Partial Fix / Data Loss Risk: While this condition prevents updates when all fields are undefined, the `patch` call inside this block still receives `undefined` for fields that aren't being updated.
For example, if `name` is updated but `args.avatarUrl` is `undefined`, the patch object becomes `{ name: "...", avatarUrl: undefined }`. In Convex, passing `undefined` to `db.patch` **deletes** the field from the document, causing unintentional data loss.
Use spread syntax or a helper to only include defined fields in the patch object.</comment>
<file context>
@@ -133,7 +133,8 @@ export const ensure = mutation({
const needsProfileUpdate =
- profile.name !== args.name || profile.avatarUrl !== args.avatarUrl;
+ (args.name !== undefined && profile.name !== args.name) ||
+ (args.avatarUrl !== undefined && profile.avatarUrl !== args.avatarUrl);
if (needsProfileUpdate) {
await ctx.db.patch(profile._id, {
</file context>
| prevCountRef.current = messageCount; | ||
| }, [messageCount, scrollToBottom, isAtBottom]); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
P1: The dependency array prevents the auto-scroll effect from running during message streaming.
isStreaming, isAtBottom, and scrollToBottom typically remain stable while a message is being generated. Because the dependencies don't change, the effect doesn't re-run when new tokens arrive, causing the scroll position to stagnate instead of following the new content.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/web/src/components/chat/chat-message-list.tsx, line 34:
<comment>The dependency array prevents the auto-scroll effect from running during message streaming.
`isStreaming`, `isAtBottom`, and `scrollToBottom` typically remain stable while a message is being generated. Because the dependencies don't change, the effect doesn't re-run when new tokens arrive, causing the scroll position to stagnate instead of following the new content.</comment>
<file context>
@@ -31,6 +31,14 @@ function AutoScroll({ messageCount }: { messageCount: number }) {
prevCountRef.current = messageCount;
}, [messageCount, scrollToBottom, isAtBottom]);
+ useEffect(() => {
+ if (isStreaming && isAtBottom) {
+ requestAnimationFrame(() => {
</file context>
… rejections startStream schedules executeStream via ctx.scheduler.runAfter(0). In convex-test the 0ms setTimeout fires after the test transaction closes, causing 'Write outside of transaction' unhandled rejections. On Linux/Node this yields exit code 1 even though all 484 tests pass. Fix: store each test's t instance and drain finishInProgressScheduledFunctions in afterEach so executeStream runs and completes within the test's lifetime. Also wraps modelsIpRatelimit.limit() in try/catch so an Upstash failure doesn't surface as a 500 on the /api/models endpoint.
leoisadev1
deleted the
feat/codebase-refactor-and-improvements-20260221-093034
branch
|
| Severity | Vulnerability Type | File | Line(s) |
|---|---|---|---|
| Medium | Sensitive Data Exposure | apps/server/convex/userApiKeys.ts |
44–61 |
| Low | Insufficient Authorization Hardening | apps/server/convex/userDelete.ts |
72–138 |
| Low | Information Disclosure via Error Messages | apps/server/convex/lib/upstashUsage.ts |
52–53 |
1. Medium — Encrypted API key ciphertext exposed to client via public query
File: apps/server/convex/userApiKeys.ts:44-61
getOpenRouterKey is a public query that returns the user's encryptedOpenRouterKey (AES-256-GCM ciphertext) directly to the browser. While the key is encrypted, exposing ciphertext to the client is unnecessary and increases risk — if the encryption key (OPENROUTER_ENCRYPTION_KEY) is ever compromised, all ciphertexts already exfiltrated to client-side storage/logs become recoverable.
The codebase already has the correct patterns:
hasOpenRouterKey(line 67) — public query returning only a booleangetOpenRouterKeyInternal(line 86) —internalQueryfor server-side use only
Fix: Convert getOpenRouterKey to an internalQuery so ciphertext never leaves the server:
// Change from public query to internalQuery
export const getOpenRouterKey = internalQuery({
args: {
userId: v.id("users"),
},
returns: v.union(v.string(), v.null()),
handler: async (ctx, args) => {
const profile = await getProfileByUserId(ctx, args.userId);
if (profile?.encryptedOpenRouterKey) {
return profile.encryptedOpenRouterKey;
}
const user = await ctx.db.get(args.userId);
return user?.encryptedOpenRouterKey ?? null;
},
});If the client needs to know whether a key exists, use the existing hasOpenRouterKey query instead. If the client needs to display a masked key, add a dedicated query that returns only a masked version (e.g., sk-...xxxx).
2. Low — deleteAccountWorkflowStep trusts externalId from client args
File: apps/server/convex/userDelete.ts:72-138
The deleteAccountWorkflowStep action accepts externalId from client-supplied args and passes it to internal.users.deleteUserRecord. While deleteUserRecord does validate user.externalId !== args.externalId (preventing actual misuse), the deprecated deleteAccount mutation correctly derives externalId from identity.subject — the new workflow-based approach should follow the same pattern.
Fix: Derive externalId from the authenticated identity instead of trusting client input:
handler: async (ctx, args) => {
const userId = await requireAuthUserIdFromAction(ctx, args.userId);
const identity = await ctx.auth.getUserIdentity();
if (!identity) throw new Error("Unauthorized");
const externalId = identity.subject; // derive from auth, not args
switch (args.step) {
// ... use externalId from identity in all cases
}
},3. Low — Upstash error response body leaked in thrown errors
File: apps/server/convex/lib/upstashUsage.ts:52-53
When the Upstash API returns an error, the raw response body (up to 200 chars) is included in the thrown Error:
const body = await response.text();
throw new Error(`Upstash pipeline failed: ${response.status} ${body.slice(0, 200)}`);If this error propagates to the client, it could leak internal infrastructure details (Upstash hostnames, Redis error codes, internal state).
Fix: Log the full error server-side and throw a sanitized message:
const body = await response.text();
void logger.error("Upstash pipeline HTTP error", { status: response.status, body: body.slice(0, 500) });
throw new Error(`Usage tracking temporarily unavailable (status: ${response.status})`);Summary
| Severity | Count |
|---|---|
| Critical | 0 |
| High | 0 |
| Medium | 1 |
| Low | 2 |
Overall the codebase demonstrates strong security practices: consistent auth checks via requireAuthUserId, comprehensive rate limiting on all sensitive endpoints, proper AES-256-GCM encryption, input validation via Convex validators, URL sanitization with protocol allowlisting, and clean separation of public vs. internal APIs.
1 participant
Summary
This PR encompasses a large-scale modular refactoring of both frontend and backend code, along with test infrastructure, CI, and backend feature additions.
Frontend Refactoring
chat-interface.tsxintochat/sub-modulesmodel-selector.tsxintomodel-selector/modulesapp-sidebar.tsxintosidebar/modulessettings.tsxintosettings/section componentsstores/model.tsinto data + store + utilsprompt-input.tsxintoai-elements/sub-componentsuse-persistent-chat.tsanduse-chat-actions.tsinto focused hooksBackend Refactoring
backgroundStream.tsinto focused modulesusers.tsintouserAuth,userProfile,userApiKeys,userDeletechats.ts— extract title and fork logicmessages.ts,files.tsinto focused modules with validators and helpersconsole.*to structuredcreateLoggerNew Features & Fixes
promptTemplates.autoSavegetChatReadStatusesandgetAllBenchmarksas anycasts and externalize constantsTests & CI
Automated PR for AI review workflow.
Summary by cubic
Refactored the app into a modular frontend and backend and introduced a streaming jobs architecture. Added structured logging, stricter validation (messages, files, URLs), rate limits, pagination, and new chat UX (title, fork, export, chain-of-thought) to improve reliability, performance, and test coverage.
Refactors
Tests & CI
Written for commit c75bd7a. Summary will update on new commits.