Skip to content

Comments

DOs for storage#1503

Merged
MrgSub merged 1 commit intostagingfrom
06-26-dos_for_storage
Jun 27, 2025
Merged

DOs for storage#1503
MrgSub merged 1 commit intostagingfrom
06-26-dos_for_storage

Conversation

@MrgSub
Copy link
Collaborator

@MrgSub MrgSub commented Jun 27, 2025

READ CAREFULLY THEN REMOVE

Remove bullet points that are not relevant.

PLEASE REFRAIN FROM USING AI TO WRITE YOUR CODE AND PR DESCRIPTION. IF YOU DO USE AI TO WRITE YOUR CODE PLEASE PROVIDE A DESCRIPTION AND REVIEW IT CAREFULLY. MAKE SURE YOU UNDERSTAND THE CODE YOU ARE SUBMITTING USING AI.

  • Pull requests that do not follow these guidelines will be closed without review or comment.
  • If you use AI to write your PR description your pr will be close without review or comment.
  • If you are unsure about anything, feel free to ask for clarification.

Description

Please provide a clear description of your changes.


Type of Change

Please delete options that are not relevant.

  • 🐛 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature with breaking changes)
  • 📝 Documentation update
  • 🎨 UI/UX improvement
  • 🔒 Security enhancement
  • ⚡ Performance improvement

Areas Affected

Please check all that apply:

  • Email Integration (Gmail, IMAP, etc.)
  • User Interface/Experience
  • Authentication/Authorization
  • Data Storage/Management
  • API Endpoints
  • Documentation
  • Testing Infrastructure
  • Development Workflow
  • Deployment/Infrastructure

Testing Done

Describe the tests you've done:

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • Cross-browser testing (if UI changes)
  • Mobile responsiveness verified (if UI changes)

Security Considerations

For changes involving data or authentication:

  • No sensitive data is exposed
  • Authentication checks are in place
  • Input validation is implemented
  • Rate limiting is considered (if applicable)

Checklist

  • I have read the CONTRIBUTING document
  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in complex areas
  • I have updated the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix/feature works
  • All tests pass locally
  • Any dependent changes are merged and published

Additional Notes

Add any other context about the pull request here.

Screenshots/Recordings

Add screenshots or recordings here if applicable.


By submitting this pull request, I confirm that my contribution is made under the terms of the project's license.

Summary by CodeRabbit

  • New Features

    • Introduced label-based filtering for email threads, allowing users to filter threads by selected labels.
    • Added a new search hook to manage label filters via the URL.
    • Implemented local caching and synchronization for email threads to improve responsiveness.
  • Improvements

    • Read threads now appear visually dimmed for easier distinction.
    • Increased the height of the mail list container on larger screens for better visibility.
    • Enhanced reliability and consistency of email attachment header display.
    • Refined thread data fetching to use database-backed retrieval with label filtering support.
  • Bug Fixes

    • Improved thread data fetching and synchronization, reducing delays and inconsistencies.
  • Developer Experience

    • Strengthened type validation and schema enforcement for email data structures.
    • Updated logging for scheduled tasks with clearer tagging.

Copy link
Collaborator Author

MrgSub commented Jun 27, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jun 27, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

This update introduces a local database-backed caching layer for email threads, enabling label-based filtering and improved synchronization between server and client. Major changes include new hooks and schemas for label management, modifications to thread retrieval and synchronization logic, and updates to API procedures to utilize the local cache. Several files are refactored for type safety and consistency.

Changes

File(s) Change Summary
apps/mail/components/mail/mail-list.tsx Updated thread container class to apply conditional opacity based on read status using the cn utility.
apps/mail/components/mail/mail.tsx Commented out category selector for Google inbox and adjusted container height calculation for medium screens.
apps/mail/components/party.tsx Added enums for message types and simplified message handling logic for mail thread updates.
apps/mail/components/ui/recursive-folder.tsx Switched from string-based to ID-based label filtering using a new useSearchLabels hook.
apps/mail/hooks/use-labels-search.ts Introduced useSearchLabels hook for managing label IDs in URL query state.
apps/mail/hooks/use-threads.ts Integrated label-based filtering into thread queries using useSearchLabels.
apps/server/src/lib/driver/google.ts Ensured attachment headers are always objects with string name and value fields.
apps/server/src/lib/driver/types.ts Made latest property in IGetThreadResponse optional and added a Zod schema for validation.
apps/server/src/lib/server-utils.ts Refactored notifyUser to use explicit result and threadId parameters and simplified notification construction.
apps/server/src/main.ts Prefixed scheduled task logs with [SCHEDULED] for clarity.
apps/server/src/pipelines.ts Added notification after thread retrieval in ThreadWorkflow using notifyUser.
apps/server/src/routes/chat.ts Implemented SQLite-based thread caching, synchronization methods, and updated message handling to use the cache.
apps/server/src/trpc/routes/mail.ts Updated procedures to use database-backed thread retrieval, added label filtering, and refined type handling.
apps/server/src/types.ts Replaced ParsedMessage interface with a Zod schema and derived type alias for validation and type safety.
apps/server/wrangler.jsonc Added R2 bucket bindings for thread storage in all environments.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant TRPC
    participant AgentRpcDO
    participant ZeroAgent
    participant SQLite
    participant KV
    participant MailDriver

    Client->>TRPC: listThreads({ labelIds, ... })
    TRPC->>AgentRpcDO: getThreadsFromDB({ labelIds, ... })
    AgentRpcDO->>ZeroAgent: getThreadsFromDB({ labelIds, ... })
    ZeroAgent->>SQLite: Query threads table (with label/folder filters)
    SQLite-->>ZeroAgent: Thread metadata rows
    ZeroAgent-->>AgentRpcDO: Thread IDs and pagination token
    AgentRpcDO-->>TRPC: Thread list
    TRPC-->>Client: Thread list

    Client->>TRPC: get(threadId)
    TRPC->>AgentRpcDO: getThreadFromDB(threadId)
    AgentRpcDO->>ZeroAgent: getThreadFromDB(threadId)
    ZeroAgent->>SQLite: Query threads table for threadId
    alt Thread found
        ZeroAgent->>KV: Get messages for threadId
        KV-->>ZeroAgent: Messages
        ZeroAgent-->>AgentRpcDO: Thread data
    else Thread not found
        ZeroAgent->>MailDriver: Fetch thread
        MailDriver-->>ZeroAgent: Thread data
        ZeroAgent->>KV: Store messages
        ZeroAgent->>SQLite: Insert thread metadata
        ZeroAgent-->>AgentRpcDO: Thread data
    end
    AgentRpcDO-->>TRPC: Thread data
    TRPC-->>Client: Thread data
Loading

Possibly related PRs

Suggested reviewers

  • nizzyabi

Poem

Hopping through code with a cache so neat,
Threads now sync and labels compete.
SQLite burrows, R2 stores,
Filtering mail through clever doors.
With Zod and hooks, the logic's tight—
A rabbit’s delight, inboxes light!
🐇💌


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c86fefa and 66cb890.

📒 Files selected for processing (15)
  • apps/mail/components/mail/mail-list.tsx (1 hunks)
  • apps/mail/components/mail/mail.tsx (2 hunks)
  • apps/mail/components/party.tsx (2 hunks)
  • apps/mail/components/ui/recursive-folder.tsx (2 hunks)
  • apps/mail/hooks/use-labels-search.ts (1 hunks)
  • apps/mail/hooks/use-threads.ts (2 hunks)
  • apps/server/src/lib/driver/google.ts (1 hunks)
  • apps/server/src/lib/driver/types.ts (1 hunks)
  • apps/server/src/lib/server-utils.ts (2 hunks)
  • apps/server/src/main.ts (4 hunks)
  • apps/server/src/pipelines.ts (2 hunks)
  • apps/server/src/routes/chat.ts (10 hunks)
  • apps/server/src/trpc/routes/mail.ts (6 hunks)
  • apps/server/src/types.ts (2 hunks)
  • apps/server/wrangler.jsonc (6 hunks)
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@MrgSub MrgSub force-pushed the 06-26-dos_for_storage branch 2 times, most recently from a8b245f to c86fefa Compare June 27, 2025 02:05
@MrgSub MrgSub marked this pull request as ready for review June 27, 2025 02:05
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

🧹 Nitpick comments (2)
apps/mail/components/mail/mail.tsx (1)

586-590: Clarify the purpose of commenting out the CategorySelect component.

The CategorySelect component has been commented out instead of being properly removed or conditionally rendered. This suggests the change might be temporary or incomplete.

If this is related to the new label filtering mechanism mentioned in the PR summary, consider:

  • Adding a feature flag or conditional logic instead of commenting out
  • Completely removing the code if the feature is permanently deprecated
  • Adding a TODO comment explaining when/why this should be restored
apps/server/src/routes/chat.ts (1)

827-885: Well-implemented thread synchronization with proper error handling

The syncThread method properly handles data persistence to both R2 and the database, with appropriate error handling and conditional broadcasting.

Consider adding more context to the error message:

     } catch (error) {
-      console.error(`Failed to sync thread ${threadId}:`, error);
+      console.error(`Failed to sync thread ${threadId} for connection ${this.name}:`, error);
       throw error;
     }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0677ec4 and c86fefa.

📒 Files selected for processing (15)
  • apps/mail/components/mail/mail-list.tsx (1 hunks)
  • apps/mail/components/mail/mail.tsx (2 hunks)
  • apps/mail/components/party.tsx (2 hunks)
  • apps/mail/components/ui/recursive-folder.tsx (2 hunks)
  • apps/mail/hooks/use-labels-search.ts (1 hunks)
  • apps/mail/hooks/use-threads.ts (2 hunks)
  • apps/server/src/lib/driver/google.ts (1 hunks)
  • apps/server/src/lib/driver/types.ts (1 hunks)
  • apps/server/src/lib/server-utils.ts (2 hunks)
  • apps/server/src/main.ts (4 hunks)
  • apps/server/src/pipelines.ts (2 hunks)
  • apps/server/src/routes/chat.ts (10 hunks)
  • apps/server/src/trpc/routes/mail.ts (7 hunks)
  • apps/server/src/types.ts (2 hunks)
  • apps/server/wrangler.jsonc (3 hunks)
🧰 Additional context used
🧠 Learnings (5)
apps/mail/hooks/use-threads.ts (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/components/mail/mail-list.tsx (3)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
Learnt from: danteissaias
PR: Mail-0/Zero#902
File: apps/mail/components/connection/add.tsx:77-77
Timestamp: 2025-05-07T16:55:46.513Z
Learning: For the "Upgrade" link in AddConnectionDialog, using a proper <button> element instead of a <span> with onClick is recognized as an accessibility improvement but was deferred as out of scope in PR #902 (CSS variables PR).
Learnt from: snehendu098
PR: Mail-0/Zero#1323
File: apps/mail/lib/themes/theme-utils.ts:318-318
Timestamp: 2025-06-24T06:22:58.753Z
Learning: In the Mail-0/Zero theme system (apps/mail/lib/themes/theme-utils.ts), when color themes are being applied, all color values come in HSL format, so there's no need for additional format validation when converting colors with hslToHex().
apps/mail/components/ui/recursive-folder.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/components/mail/mail.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
apps/mail/components/party.tsx (1)
Learnt from: retrogtx
PR: Mail-0/Zero#1328
File: apps/mail/lib/hotkeys/mail-list-hotkeys.tsx:202-209
Timestamp: 2025-06-18T17:26:50.918Z
Learning: In apps/mail/lib/hotkeys/mail-list-hotkeys.tsx, the switchCategoryByIndex function using hardcoded indices for category hotkeys does not break when users reorder categories, contrary to the theoretical index-shifting issue. The actual implementation has constraints or mechanisms that prevent hotkey targeting issues.
🧬 Code Graph Analysis (7)
apps/server/src/pipelines.ts (1)
apps/server/src/lib/server-utils.ts (1)
  • notifyUser (59-87)
apps/mail/components/mail/mail-list.tsx (1)
apps/mail/lib/utils.ts (1)
  • cn (51-51)
apps/server/src/types.ts (1)
apps/mail/types/index.ts (1)
  • ParsedMessage (61-87)
apps/server/src/lib/driver/types.ts (2)
apps/server/src/types.ts (2)
  • ParsedMessage (144-144)
  • ParsedMessageSchema (105-142)
apps/mail/types/index.ts (1)
  • ParsedMessage (61-87)
apps/server/src/lib/server-utils.ts (1)
apps/server/src/routes/chat.ts (1)
  • OutgoingMessage (100-128)
apps/server/src/trpc/routes/mail.ts (2)
apps/server/src/lib/driver/types.ts (1)
  • IGetThreadResponseSchema (14-20)
apps/server/src/lib/server-utils.ts (1)
  • getZeroAgent (14-19)
apps/server/src/routes/chat.ts (3)
apps/server/src/lib/driver/types.ts (2)
  • IGetThreadResponse (6-12)
  • MailManager (50-105)
apps/server/src/types.ts (1)
  • ParsedMessage (144-144)
apps/mail/types/index.ts (1)
  • ParsedMessage (61-87)
🔇 Additional comments (20)
apps/server/wrangler.jsonc (1)

22-27: LGTM! Well-structured R2 bucket configuration.

The R2 bucket configurations are properly structured across all environments with appropriate naming conventions. The binding name THREADS_BUCKET is descriptive and the bucket names follow a consistent pattern.

Also applies to: 187-192, 292-297

apps/server/src/main.ts (1)

692-692: LGTM! Improved logging for better observability.

The addition of [SCHEDULED] prefixes to console logs enhances debugging and monitoring capabilities for scheduled subscription checks. The logging is consistent and well-structured.

Also applies to: 694-694, 708-710, 719-721, 730-730

apps/mail/hooks/use-threads.ts (1)

6-6: LGTM! Proper integration of label-based filtering.

The integration of the useSearchLabels hook is implemented correctly and enables label-based thread filtering as intended. The destructuring and usage pattern follows established conventions.

Note: This integration depends on fixing the null handling issue in the useSearchLabels hook to ensure labels is always a defined array.

Also applies to: 21-21, 28-28

apps/server/src/pipelines.ts (1)

22-22: LGTM! Proper integration of user notification after thread retrieval.

The notifyUser call is correctly placed after thread retrieval and properly passes the required parameters (connectionId, result, threadId). This enables real-time broadcasting of thread updates to connected clients, supporting the new caching and synchronization features.

Also applies to: 400-404

apps/mail/components/mail/mail-list.tsx (1)

272-275: LGTM! Good UX improvement for read/unread thread distinction.

The conditional opacity application correctly implements the visual dimming of read threads while keeping unread threads at full opacity, providing clear visual distinction for users.

apps/server/src/lib/driver/google.ts (1)

356-360: LGTM! Excellent defensive programming for header consistency.

The nullish coalescing ensures that header name and value properties are always strings, preventing potential null/undefined issues in downstream processing and supporting the typed schema validation mentioned in the broader system changes.

apps/mail/components/mail/mail.tsx (1)

599-599: Height adjustment correctly compensates for removed CategorySelect.

The height calculation change from calc(100dvh - 9.8rem) to calc(100dvh - 7rem) logically compensates for the commented-out CategorySelect component, making more space available for the MailList component.

apps/server/src/types.ts (3)

2-2: LGTM!

The Zod import is correctly added to support the new schema-based validation approach.


105-142: Excellent migration to Zod schema for runtime validation.

The comprehensive ParsedMessageSchema properly replaces the TypeScript interface with runtime validation capabilities. The schema correctly handles:

  • All required and optional fields from the original interface
  • Nested object validation for tags, sender, and recipients
  • Proper nullable handling for cc/bcc arrays
  • Well-structured attachment schema with proper typing

This change enhances type safety at runtime and provides better error handling for API boundaries.


144-144: Proper use of Zod type inference.

Using z.infer<typeof ParsedMessageSchema> correctly derives the TypeScript type from the schema, ensuring the type and validation rules stay synchronized.

apps/mail/components/ui/recursive-folder.tsx (3)

6-6: LGTM!

The import of useSearchLabels hook supports the improved ID-based label filtering mechanism.


22-23: Improved filtering mechanism with ID-based approach.

The migration from string-based search to ID-based filtering using useSearchLabels is a significant improvement that provides:

  • More reliable label matching
  • Better performance
  • Cleaner logic with array inclusion checks

28-37: Well-implemented label toggle logic.

The array-based label filtering logic is clean and efficient:

  • Properly toggles label IDs in the array
  • Uses appropriate array methods for state management
  • Correctly updated dependency array for the callback

The implementation follows React best practices and is more maintainable than the previous string manipulation approach.

apps/server/src/lib/driver/types.ts (3)

2-4: LGTM!

The imports correctly support the new schema-based validation approach, maintaining consistency with the broader codebase migration to Zod.


8-8: Improved TypeScript syntax for optional property.

Changing from latest: ParsedMessage | undefined to latest?: ParsedMessage is cleaner and more idiomatic TypeScript syntax for optional properties.


14-20: Well-structured Zod schema for thread response validation.

The IGetThreadResponseSchema correctly validates the thread response structure:

  • Properly uses ParsedMessageSchema for message validation
  • Correctly handles the optional latest property
  • Maintains consistency with the interface definition
  • Provides runtime validation for API responses
apps/mail/components/party.tsx (2)

9-24: Excellent addition of message type enums.

The IncomingMessageType and OutgoingMessageType enums provide:

  • Better type safety for WebSocket message handling
  • Centralized message type definitions
  • Clear and consistent naming conventions
  • Improved code maintainability

49-52: Streamlined message handling improves efficiency.

The simplified message handling logic provides:

  • Direct cache updates instead of query invalidations
  • More efficient real-time thread updates
  • Cleaner code with focused responsibility
  • Proper use of the new message type enum

The direct setQueryData approach is more performant than invalidating and refetching queries.

apps/server/src/trpc/routes/mail.ts (1)

23-23: Good addition of output schema for type safety

Adding the output schema ensures type-safe responses from the get procedure.

apps/server/src/routes/chat.ts (1)

519-546: Good integration of database-backed methods in WebSocket handlers

The WebSocket message handling properly integrates with the new caching layer, using getThreadsFromDB and getThreadFromDB methods.

Comment on lines 328 to 330
await agent.sendDraft(draftId, mail as any);
} else {
await agent.create(input);
await agent.create(input as any);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid casting to any - fix type definitions instead

Casting to any defeats TypeScript's type safety and can hide potential runtime errors. The underlying type mismatch should be resolved.

Instead of casting, ensure the types match properly:

-        await agent.sendDraft(draftId, mail as any);
+        await agent.sendDraft(draftId, mail);
       } else {
-        await agent.create(input as any);
+        await agent.create(input);

If there's a type mismatch, update the method signatures or transform the data to match the expected types.

📝 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.

Suggested change
await agent.sendDraft(draftId, mail as any);
} else {
await agent.create(input);
await agent.create(input as any);
await agent.sendDraft(draftId, mail);
} else {
await agent.create(input);
🤖 Prompt for AI Agents
In apps/server/src/trpc/routes/mail.ts around lines 328 to 330, the code casts
variables to 'any' which bypasses TypeScript's type checking. To fix this,
review the expected parameter types for the sendDraft and create methods and
update the input data or method signatures so the types align correctly without
using 'any'. This may involve adjusting the input object's type or transforming
it to match the method requirements.

Comment on lines +948 to +1106
async getThreadsFromDB(params: {
labelIds?: string[];
folder?: string;
q?: string;
max?: number;
cursor?: string;
}) {
const { labelIds = [], folder, q, max = 50, cursor } = params;

try {
// Build WHERE conditions
const whereConditions: string[] = [];

// Add folder condition (maps to specific label)
if (folder) {
const folderLabel = folder.toUpperCase();
whereConditions.push(`EXISTS (
SELECT 1 FROM json_each(latest_label_ids) WHERE value = '${folderLabel}'
)`);
}

// Add label conditions (OR logic for multiple labels)
if (labelIds.length > 0) {
if (labelIds.length === 1) {
whereConditions.push(`EXISTS (
SELECT 1 FROM json_each(latest_label_ids) WHERE value = '${labelIds[0]}'
)`);
} else {
// Multiple labels with OR logic
const multiLabelCondition = labelIds
.map(
(labelId) =>
`EXISTS (SELECT 1 FROM json_each(latest_label_ids) WHERE value = '${labelId}')`,
)
.join(' OR ');
whereConditions.push(`(${multiLabelCondition})`);
}
}

// // Add search query condition
// if (q) {
// const searchTerm = q.replace(/'/g, "''"); // Escape single quotes
// whereConditions.push(`(
// latest_subject LIKE '%${searchTerm}%' OR
// latest_sender LIKE '%${searchTerm}%' OR
// messages LIKE '%${searchTerm}%'
// )`);
// }

// Add cursor condition
if (cursor) {
whereConditions.push(`latest_received_on < '${cursor}'`);
}

// Execute query based on conditions
let result;

if (whereConditions.length === 0) {
// No conditions
result = await this.sql`
SELECT id, latest_received_on
FROM threads
ORDER BY latest_received_on DESC
LIMIT ${max}
`;
} else if (whereConditions.length === 1) {
// Single condition
const condition = whereConditions[0];
if (condition.includes('latest_received_on <')) {
const cursorValue = cursor!;
result = await this.sql`
SELECT id, latest_received_on
FROM threads
WHERE latest_received_on < ${cursorValue}
ORDER BY latest_received_on DESC
LIMIT ${max}
`;
} else if (folder) {
// Folder condition
const folderLabel = folder.toUpperCase();
result = await this.sql`
SELECT id, latest_received_on
FROM threads
WHERE EXISTS (
SELECT 1 FROM json_each(latest_label_ids) WHERE value = ${folderLabel}
)
ORDER BY latest_received_on DESC
LIMIT ${max}
`;
} else {
// Single label condition
const labelId = labelIds[0];
result = await this.sql`
SELECT id, latest_received_on
FROM threads
WHERE EXISTS (
SELECT 1 FROM json_each(latest_label_ids) WHERE value = ${labelId}
)
ORDER BY latest_received_on DESC
LIMIT ${max}
`;
}
} else {
// Multiple conditions - handle combinations
if (folder && labelIds.length === 0 && cursor) {
// Folder + cursor
const folderLabel = folder.toUpperCase();
result = await this.sql`
SELECT id, latest_received_on
FROM threads
WHERE EXISTS (
SELECT 1 FROM json_each(latest_label_ids) WHERE value = ${folderLabel}
) AND latest_received_on < ${cursor}
ORDER BY latest_received_on DESC
LIMIT ${max}
`;
} else if (labelIds.length === 1 && cursor && !folder) {
// Single label + cursor
const labelId = labelIds[0];
result = await this.sql`
SELECT id, latest_received_on
FROM threads
WHERE EXISTS (
SELECT 1 FROM json_each(latest_label_ids) WHERE value = ${labelId}
) AND latest_received_on < ${cursor}
ORDER BY latest_received_on DESC
LIMIT ${max}
`;
} else {
// For now, fallback to just cursor if complex combinations
const cursorValue = cursor || '';
result = await this.sql`
SELECT id, latest_received_on
FROM threads
WHERE latest_received_on < ${cursorValue}
ORDER BY latest_received_on DESC
LIMIT ${max}
`;
}
}

const threads = result.map((row: any) => ({
id: row.id,
historyId: null,
}));

// Use latest_received_on for pagination cursor
const nextPageToken =
threads.length === max && result.length > 0
? result[result.length - 1].latest_received_on
: null;

return {
threads,
nextPageToken,
};
} catch (error) {
console.error('Failed to get threads from database:', error);
throw error;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

❌ Incorrect review comment

Potential SQL injection risk in query building

The getThreadsFromDB method builds SQL queries using string interpolation for label IDs (lines 964-984), which could lead to SQL injection if these values contain malicious input.

Use parameterized queries consistently throughout:

-        whereConditions.push(`EXISTS (
-          SELECT 1 FROM json_each(latest_label_ids) WHERE value = '${folderLabel}'
-        )`);
+        // Use parameterized query instead
+        whereConditions.push(`EXISTS (
+          SELECT 1 FROM json_each(latest_label_ids) WHERE value = ?
+        )`);
+        // And bind the parameter when executing

Let me verify if labelIds can contain user input:


🏁 Script executed:

#!/bin/bash
# Check where labelIds come from in the codebase
rg -A 5 "labelIds.*=" --type ts

Length of output: 4302


Potential SQL injection risk in query building

The getThreadsFromDB method builds SQL queries using string interpolation for label IDs (lines 964-984), which could lead to SQL injection if these values contain malicious input.

Use parameterized queries consistently throughout:

-        whereConditions.push(`EXISTS (
-          SELECT 1 FROM json_each(latest_label_ids) WHERE value = '${folderLabel}'
-        )`);
+        // Use parameterized query instead
+        whereConditions.push(`EXISTS (
+          SELECT 1 FROM json_each(latest_label_ids) WHERE value = ?
+        )`);
+        // And bind the parameter when executing

Let me verify if labelIds can contain user input:

#!/bin/bash
# Check where labelIds come from in the codebase
rg -A 5 "labelIds.*=" --type ts
🤖 Prompt for AI Agents
In apps/server/src/routes/chat.ts between lines 948 and 1108, the
getThreadsFromDB method constructs SQL queries by directly interpolating
labelIds and folder values into query strings, which risks SQL injection. To fix
this, replace all string interpolations for labelIds and folder with
parameterized query bindings using the SQL library's templating syntax to safely
pass these values as parameters instead of embedding them directly in the query
strings.

@MrgSub MrgSub force-pushed the 06-26-dos_for_storage branch from c86fefa to 66cb890 Compare June 27, 2025 02:43
Copy link
Collaborator Author

MrgSub commented Jun 27, 2025

Merge activity

  • Jun 27, 2:44 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jun 27, 2:45 AM UTC: @MrgSub merged this pull request with Graphite.

@MrgSub MrgSub merged commit 18016c6 into staging Jun 27, 2025
5 of 6 checks passed
@MrgSub MrgSub deleted the 06-26-dos_for_storage branch June 27, 2025 02:45
@coderabbitai coderabbitai bot mentioned this pull request Jul 7, 2025
34 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant