Skip to content

Comments

cli fixes and tests#1048

Merged
elie222 merged 6 commits intomainfrom
feat/cli-tests
Dec 3, 2025
Merged

cli fixes and tests#1048
elie222 merged 6 commits intomainfrom
feat/cli-tests

Conversation

@elie222
Copy link
Owner

@elie222 elie222 commented Dec 3, 2025

Switch Outlook pagination to accept and return full @odata.nextLink URLs and add CLI .env template generation with test coverage

Update Outlook message queries to use full @odata.nextLink URLs instead of $skiptoken, and refactor the CLI to generate .env from a template with Docker/host-aware values and add Vitest tests.

📍Where to Start

Start with OutlookProvider.getMessages in microsoft.ts, then review CLI entry in main.ts and the .env generator in utils.ts.


📊 Macroscope summarized 78b32d8. 5 files reviewed, 18 issues evaluated, 18 issues filtered, 0 comments posted

🗂️ Filtered Issues

apps/web/utils/email/microsoft.ts — 0 comments posted, 5 evaluated, 5 filtered
  • line 1040: Incorrect folder filtering: using parentFolderId eq 'inbox' or 'archive' (and mapping labelId via toLowerCase() into parentFolderId) is wrong. parentFolderId contains the actual folder GUID/ID, not the well-known name. These filters will usually return no results. For well-known folders, use endpoints like /me/mailFolders('inbox')/messages; for arbitrary folders, resolve the folder ID first (e.g., via getFolderIds or lookup) and filter by that ID. [ Already posted ]
  • line 1043: Misuse of labelId from ThreadsQuery as Outlook folder ID. The query schema comment indicates labelId is "For Google". Treating it as parentFolderId in Outlook (filters.push("parentFolderId eq '${labelId.toLowerCase()}'")) may filter on an unrelated string and return empty or wrong results. Outlook categories are separate from folders; if the intent is to filter by folder, accept a dedicated Outlook folder ID/name and resolve to the actual folder ID; if by category, use categories in filters. [ Already posted ]
  • line 1071: Pagination token handling is inconsistent and ignores non-URL tokens. When options.pageToken is provided and is NOT a full URL, the code no longer applies skipToken(pageToken) to the request, causing the first page to be refetched and breaking pagination. Only URL tokens are supported now, which is a silent behavior change. This can lead to repeated results or stalled pagination if callers pass $skiptoken values. Fix by supporting both formats: if pageToken starts with http, fetch it directly; otherwise call .skipToken(pageToken) on the request. [ Low confidence ]
  • line 1147: Draft messages are not excluded. Unlike other paths that filter out drafts (e.g., convertMessages(...).filter(!isDraft)), this implementation builds ParsedMessage objects directly from Graph messages without checking message.isDraft. As a result, threads can include draft messages, which likely violates expected thread content semantics and may surface incomplete/internal drafts to clients. Add a check to skip messages with isDraft === true before grouping or when mapping to ParsedMessage. [ Low confidence ]
  • line 1187: Contract parity change for nextPageToken: previously code extracted $skiptoken via new URL(nextLink).searchParams.get("$skiptoken"), returning the token string. Now it returns the full @odata.nextLink URL. If upstream clients or APIs expect the bare $skiptoken (as suggested by earlier code and common Graph SDK patterns), this breaks pagination interoperability and may cause clients to pass non-URL tokens back, which this method no longer honors. Clarify and consistently enforce token format or support both formats on input and output. [ Low confidence ]
apps/web/utils/outlook/message.ts — 0 comments posted, 3 evaluated, 3 filtered
  • line 156: Inaccurate warning telemetry: When options.maxResults > 20, queryBatchMessages logs { maxResults } (lines 156-157), but maxResults has already been capped with Math.min(...) and will be ≤ 20. This misreports the requested value in logs, making diagnostics misleading. Log options.maxResults instead. [ Low confidence ]
  • line 164: Breaking pagination contract and ignoring non-URL page tokens: queryBatchMessages now returns nextPageToken as the full @odata.nextLink URL (see assignments to nextPageToken), and only consumes a pageToken when it startsWith("http") (lines 164-174). Any existing callers that previously stored and passed the extracted $skiptoken (a non-URL string) will now have their pageToken silently ignored, causing the function to re-fetch the first page and potentially loop or duplicate results. This is a runtime regression and externally-visible contract change. Ensure either backward compatibility (accept old $skiptoken via .skipToken) or explicitly migrate all callers to pass the full @odata.nextLink URL. [ Low confidence ]
  • line 308: Breaking pagination contract and ignoring non-URL page tokens: queryMessagesWithFilters now returns nextPageToken as the full @odata.nextLink URL (line 363), and only consumes a pageToken when it startsWith("http") (lines 308-314). If existing callers pass previously returned non-URL $skiptoken strings, the token is ignored and the function re-fetches the first page, breaking pagination and risking duplicate/looped results. Maintain backward compatibility by accepting legacy $skiptoken via .skipToken(pageToken) or require and enforce migration to URL tokens. [ Low confidence ]
packages/cli/src/main.ts — 0 comments posted, 5 evaluated, 5 filtered
  • line 608: When REPO_ROOT is truthy (running inside the repo), the code writes the env file to apps/web/.env without ensuring that the parent directory apps/web exists. Only ensureConfigDir(configDir) is called with configDir set to REPO_ROOT, which does not create apps/web. If the repository structure is missing apps/web (e.g., partial checkout, renamed folder, or different layout), writeFileSync(envFile, envContent) will throw at runtime with ENOENT. [ Low confidence ]
  • line 911: fetchEnvExample performs a network request without timeout/abort handling. In degraded networks the call can hang indefinitely, stalling the CLI with no fallback or user-visible outcome. [ Low confidence ]
  • line 912: fetch is used without a polyfill or environment guard. In Node.js versions prior to 18, fetch is not defined, causing a ReferenceError at runtime when fetchEnvExample is invoked. [ Low confidence ]
  • line 919: getEnvTemplate only falls back to fetchEnvExample when the file does not exist. If the file exists but is unreadable (e.g., EACCES) or becomes missing after the check, the function throws instead of providing a defined fallback path, violating the intended contract of always returning the template or fetching it remotely. [ Low confidence ]
  • line 924: Time-of-check-time-of-use race: existsSync(templatePath) can return true while readFileSync(templatePath, "utf-8") subsequently throws (e.g., file removed or permission denied), causing an unhandled exception instead of falling back to fetchEnvExample. [ Already posted ]
packages/cli/src/utils.ts — 0 comments posted, 5 evaluated, 5 filtered
  • line 22: wrapInQuotes returns "${value}" without escaping inner quotes (") or backslashes (\). Values containing quotes/backslashes will produce ambiguous or invalid .env entries. Implement proper escaping (e.g., replace " with \" and \ with \\) before wrapping. [ Low confidence ]
  • line 33: When the template contains multiple occurrences of the same key (e.g., both commented and uncommented forms or duplicates), setValue only updates the first match and returns, leaving other occurrences stale. This silently produces duplicate or inconsistent entries in the output .env. To enforce uniqueness, update all occurrences or remove extras, then write a single canonical entry. [ Low confidence ]
  • line 35: Using String.prototype.replace with a regex and a literal replacement string that includes the user-supplied value can corrupt the output when value contains $ sequences (e.g., $1, $&, $$). In JavaScript, $ tokens in the replacement string are interpreted specially. This can produce malformed .env lines or strip parts of secrets. To avoid this, use the functional form of replace (e.g., content = content.replace(pattern, () => ${key}=${value})) or escape $ in value before replacing. [ Already posted ]
  • line 52: Most values are written to the .env file without quoting or escaping. If a value contains spaces, #, =, newlines, or quotes, the resulting line can be malformed or parsed incorrectly by dotenv parsers. Only DATABASE_URL and UPSTASH_REDIS_URL are wrapped in quotes; other keys (including secrets and tokens) are not. All values should be consistently quoted and inner quotes/backslashes escaped to ensure valid, parseable output. [ Already posted ]
  • line 111: llmProvider is used to decide which API key to set, but the code also writes DEFAULT_LLM_PROVIDER from env.DEFAULT_LLM_PROVIDER without validating consistency. If llmProvider differs from env.DEFAULT_LLM_PROVIDER, the output .env can specify provider A while configuring an API key for provider B, leading to misconfiguration at runtime. Validate and align these or derive one from the other. [ Low confidence ]

Summary by CodeRabbit

  • Refactor

    • Simplified Outlook/Microsoft Graph paging to use direct next-link paging and unified message retrieval for more reliable email fetching.
  • New Features

    • CLI: env file generation utilities and interactive Docker/web deployment options; template-based .env creation.
  • Tests

    • Added comprehensive CLI unit tests and Vitest configuration.
  • Chores

    • Bumped version to v2.21.29 and added Vitest test scripts/dev dependency.

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link

vercel bot commented Dec 3, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Updated (UTC)
inbox-zero Ready Ready Preview Dec 3, 2025 4:12am

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 3, 2025

Caution

Review failed

The pull request is closed.

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Refactors Outlook/Microsoft Graph paging to prefer direct nextLink URL-based pagination, updates message query functions to return raw @odata.nextLink, extracts CLI env/secret utilities into a shared module with template-driven env generation and tests, adds Vitest config/scripts, and bumps version to v2.21.29.

Changes

Cohort / File(s) Summary
Microsoft Graph pagination
apps/web/utils/email/microsoft.ts, apps/web/utils/outlook/message.ts
Add direct-URL paging branch when pageToken starts with http, fetch using the provided nextLink, use response["@odata.nextLink"] as nextPageToken, and keep existing sorting/threading/transformation logic. Remove skiptoken parsing and unify ordering to receivedDateTime DESC for relevant paths.
CLI environment utilities
packages/cli/src/utils.ts, packages/cli/src/utils.test.ts
Add EnvConfig type, generateSecret(bytes) and generateEnvFile({env,useDockerInfra,llmProvider,template}) to generate env files from templates; include extensive unit tests covering secret length/format, template substitution, Docker mode, LLM/provider key handling, and commented/uncommented entries.
CLI main refactoring
packages/cli/src/main.ts
Replace local secret/env logic with imports from ./utils, implement template-based .env generation (fetching remote/local templates with spinner), add Docker-host vs Docker-run branching for DB/Redis URLs, and guard main() with an isMainModule check for testability.
CLI testing framework
packages/cli/package.json, packages/cli/vitest.config.ts
Add vitest devDependency and test/test:watch scripts; introduce Vitest config forcing single-threaded pool for cleaner exits.
Version bump
version.txt
Update version from v2.21.27 to v2.21.29.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Review paging control flow in microsoft.ts and message.ts for edge cases (URL vs token branches, ordering, nextLink handling).
  • Validate generateEnvFile substitution logic and tests in packages/cli/src/utils.ts / utils.test.ts for template/comment handling and provider key mapping.
  • Check main.ts template fetch/fallback paths and Docker vs host URL assignment.

Possibly related PRs

  • Fix outlook pagination #1047 — Directly related: similar changes to Graph paging, @odata.nextLink handling, and GraphMessage usage in Outlook/Microsoft Graph code paths.
  • bulk move outlook #1046 — closely related: replaces skipToken parsing with nextLink/URL-based paging across the same message retrieval functions.
  • Load more data for outlook stats #977 — modifies message query/paging logic in the same Outlook utilities (queryBatchMessages/queryMessagesWithFilters), likely to overlap or interact.

Poem

🐰
I hopped through links and tokens old,
Found nextLink paths both brave and bold,
I stitched env templates with a twitchy paw,
Wrote tests (and nibbled bugs I saw),
A carrot-coded change—hip-hop hooray!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check ❓ Inconclusive The PR title 'cli fixes and tests' is vague and doesn't clearly convey the main changes; it uses generic terms without specifying what fixes or what tests were added. Consider a more descriptive title that highlights the primary change, such as 'Refactor email paging to use nextLink URL-based tokens' or 'Add CLI utilities and vitest configuration' to better communicate the changeset's purpose.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8dec95f and 78b32d8.

📒 Files selected for processing (4)
  • apps/web/utils/outlook/message.ts (5 hunks)
  • packages/cli/src/main.ts (8 hunks)
  • packages/cli/src/utils.test.ts (1 hunks)
  • packages/cli/src/utils.ts (1 hunks)

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

Comment @coderabbitai help to get the list of available commands and usage tips.

filters.push("parentFolderId eq 'inbox'");
}
// Determine endpoint and build filters based on query type
let endpoint = "/me/messages";
Copy link
Contributor

Choose a reason for hiding this comment

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

parentFolderId is a GUID, not a well-known name. Filtering with parentFolderId eq 'inbox'/'archive' and lowercasing labelId will return no results. Consider resolving folder IDs (e.g., via getFolderIds) and use those IDs, or switch to folder-specific endpoints like /me/mailFolders('inbox')/messages.

🚀 Reply to ask Macroscope to explain or update this suggestion.

👍 Helpful? React to give us feedback.

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: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/cli/src/main.ts (1)

66-71: Version mismatch with version.txt.

The CLI version is hardcoded as "2.21.16" but version.txt shows v2.21.29. Consider keeping these in sync or reading the version dynamically.

   program
     .name("inbox-zero")
     .description("CLI tool for running Inbox Zero - AI email assistant")
-    .version("2.21.16");
+    .version("2.21.29");
🧹 Nitpick comments (4)
packages/cli/src/utils.test.ts (1)

181-199: Test assertion may not verify intended behavior.

The test "should append values not found in template" expects CUSTOM_VAR to be appended, but the comment on lines 197-198 acknowledges that only values set via setValue will be added. Since CUSTOM_VAR is not explicitly handled in generateEnvFile, this test doesn't actually verify the append behavior—it only checks that AUTH_SECRET is set.

Consider either:

  1. Removing this test if append behavior isn't intended for arbitrary env keys
  2. Or adding an assertion that explicitly validates whether CUSTOM_VAR appears in the result
packages/cli/src/utils.ts (1)

22-37: Static analysis flagged potential ReDoS, but risk is low in current usage.

The static analysis tool flagged regex construction from variable input (CWE-1333). However, in the current implementation, key values are hardcoded environment variable names (e.g., DATABASE_URL, AUTH_SECRET), which are safe and don't contain regex metacharacters.

The risk would only materialize if setValue were exposed to untrusted input. Consider adding a brief comment noting this assumption for future maintainers:

   // Helper to set a value (handles both commented and uncommented lines)
+  // Note: key is assumed to be a safe env var name (alphanumeric + underscore)
   const setValue = (key: string, value: string | undefined) => {
packages/cli/src/main.ts (1)

947-959: Main module detection may have edge cases.

The isMainModule check relies on process.argv[1] ending with specific filenames. This could fail if:

  • The script is invoked via a symlink with a different name
  • The path contains URL-encoded characters

Consider using a more robust approach if this causes issues in practice:

-const isMainModule =
-  process.argv[1] &&
-  (process.argv[1].endsWith("main.ts") ||
-    process.argv[1].endsWith("inbox-zero.js") ||
-    process.argv[1].endsWith("inbox-zero"));
+import { fileURLToPath } from "node:url";
+
+const isMainModule = (() => {
+  try {
+    const currentFile = fileURLToPath(import.meta.url);
+    return process.argv[1] === currentFile;
+  } catch {
+    // Fallback for non-file URLs
+    return process.argv[1]?.endsWith("main.ts") ||
+           process.argv[1]?.endsWith("inbox-zero.js") ||
+           process.argv[1]?.endsWith("inbox-zero");
+  }
+})();
apps/web/utils/email/microsoft.ts (1)

1026-1028: Consider adding retry logic for the direct URL fetch.

The direct URL fetch path doesn't use withOutlookRetry, unlike the similar implementations in message.ts (lines 165-166, 309-310). This inconsistency could lead to failures on transient errors.

Apply this diff to add retry logic:

     // If pageToken is a URL, fetch directly (per MS docs, don't extract $skiptoken)
     if (options.pageToken?.startsWith("http")) {
-      response = await client.api(options.pageToken).get();
+      response = await withOutlookRetry(() => client.api(options.pageToken).get());
     } else {

Note: You'll need to import withOutlookRetry from @/utils/outlook/retry if not already imported.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee7664f and 8dec95f.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (8)
  • apps/web/utils/email/microsoft.ts (2 hunks)
  • apps/web/utils/outlook/message.ts (5 hunks)
  • packages/cli/package.json (1 hunks)
  • packages/cli/src/main.ts (8 hunks)
  • packages/cli/src/utils.test.ts (1 hunks)
  • packages/cli/src/utils.ts (1 hunks)
  • packages/cli/vitest.config.ts (1 hunks)
  • version.txt (1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
!(pages/_document).{jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

Don't use the next/head module in pages/_document.js on Next.js projects

Files:

  • version.txt
  • packages/cli/vitest.config.ts
  • packages/cli/src/utils.ts
  • packages/cli/src/utils.test.ts
  • apps/web/utils/email/microsoft.ts
  • apps/web/utils/outlook/message.ts
  • packages/cli/src/main.ts
  • packages/cli/package.json
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/data-fetching.mdc)

**/*.{ts,tsx}: For API GET requests to server, use the swr package
Use result?.serverError with toastError from @/components/Toast for error handling in async operations

**/*.{ts,tsx}: Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls
Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls
Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls

**/*.{ts,tsx}: For early access feature flags, create hooks using the naming convention use[FeatureName]Enabled that return a boolean from useFeatureFlagEnabled("flag-key")
For A/B test variant flags, create hooks using the naming convention use[FeatureName]Variant that define variant types, use useFeatureFlagVariantKey() with type casting, and provide a default "control" fallback
Use kebab-case for PostHog feature flag keys (e.g., inbox-cleaner, pricing-options-2)
Always define types for A/B test variant flags (e.g., type PricingVariant = "control" | "variant-a" | "variant-b") and provide type safety through type casting

**/*.{ts,tsx}: Don't use primitive type aliases or misleading types
Don't use empty type parameters in type aliases and interfaces
Don't use this and super in static contexts
Don't use any or unknown as type constraints
Don't use the TypeScript directive @ts-ignore
Don't use TypeScript enums
Don't export imported variables
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions
Don't use TypeScript namespaces
Don't use non-null assertions with the ! postfix operator
Don't use parameter properties in class constructors
Don't use user-defined types
Use as const instead of literal types and type annotations
Use either T[] or Array<T> consistently
Initialize each enum member value explicitly
Use export type for types
Use `impo...

Files:

  • packages/cli/vitest.config.ts
  • packages/cli/src/utils.ts
  • packages/cli/src/utils.test.ts
  • apps/web/utils/email/microsoft.ts
  • apps/web/utils/outlook/message.ts
  • packages/cli/src/main.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/prisma-enum-imports.mdc)

Always import Prisma enums from @/generated/prisma/enums instead of @/generated/prisma/client to avoid Next.js bundling errors in client components

Import Prisma using the project's centralized utility: import prisma from '@/utils/prisma'

Files:

  • packages/cli/vitest.config.ts
  • packages/cli/src/utils.ts
  • packages/cli/src/utils.test.ts
  • apps/web/utils/email/microsoft.ts
  • apps/web/utils/outlook/message.ts
  • packages/cli/src/main.ts
**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/security.mdc)

**/*.ts: ALL database queries MUST be scoped to the authenticated user/account by including user/account filtering in WHERE clauses to prevent unauthorized data access
Always validate that resources belong to the authenticated user before performing operations, using ownership checks in WHERE clauses or relationships
Always validate all input parameters for type, format, and length before using them in database queries
Use SafeError for error responses to prevent information disclosure. Generic error messages should not reveal internal IDs, logic, or resource ownership details
Only return necessary fields in API responses using Prisma's select option. Never expose sensitive data such as password hashes, private keys, or system flags
Prevent Insecure Direct Object References (IDOR) by validating resource ownership before operations. All findUnique/findFirst calls MUST include ownership filters
Prevent mass assignment vulnerabilities by explicitly whitelisting allowed fields in update operations instead of accepting all user-provided data
Prevent privilege escalation by never allowing users to modify system fields, ownership fields, or admin-only attributes through user input
All findMany queries MUST be scoped to the user's data by including appropriate WHERE filters to prevent returning data from other users
Use Prisma relationships for access control by leveraging nested where clauses (e.g., emailAccount: { id: emailAccountId }) to validate ownership

Files:

  • packages/cli/vitest.config.ts
  • packages/cli/src/utils.ts
  • packages/cli/src/utils.test.ts
  • apps/web/utils/email/microsoft.ts
  • apps/web/utils/outlook/message.ts
  • packages/cli/src/main.ts
**/*.{tsx,ts}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

**/*.{tsx,ts}: Use Shadcn UI and Tailwind for components and styling
Use next/image package for images
For API GET requests to server, use the swr package with hooks like useSWR to fetch data
For text inputs, use the Input component with registerProps for form integration and error handling

Files:

  • packages/cli/vitest.config.ts
  • packages/cli/src/utils.ts
  • packages/cli/src/utils.test.ts
  • apps/web/utils/email/microsoft.ts
  • apps/web/utils/outlook/message.ts
  • packages/cli/src/main.ts
**/*.{tsx,ts,css}

📄 CodeRabbit inference engine (.cursor/rules/ui-components.mdc)

Implement responsive design with Tailwind CSS using a mobile-first approach

Files:

  • packages/cli/vitest.config.ts
  • packages/cli/src/utils.ts
  • packages/cli/src/utils.test.ts
  • apps/web/utils/email/microsoft.ts
  • apps/web/utils/outlook/message.ts
  • packages/cli/src/main.ts
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{js,jsx,ts,tsx}: Don't use accessKey attribute on any HTML element
Don't set aria-hidden="true" on focusable elements
Don't add ARIA roles, states, and properties to elements that don't support them
Don't use distracting elements like <marquee> or <blink>
Only use the scope prop on <th> elements
Don't assign non-interactive ARIA roles to interactive HTML elements
Make sure label elements have text content and are associated with an input
Don't assign interactive ARIA roles to non-interactive HTML elements
Don't assign tabIndex to non-interactive HTML elements
Don't use positive integers for tabIndex property
Don't include "image", "picture", or "photo" in img alt prop
Don't use explicit role property that's the same as the implicit/default role
Make static elements with click handlers use a valid role attribute
Always include a title element for SVG elements
Give all elements requiring alt text meaningful information for screen readers
Make sure anchors have content that's accessible to screen readers
Assign tabIndex to non-interactive HTML elements with aria-activedescendant
Include all required ARIA attributes for elements with ARIA roles
Make sure ARIA properties are valid for the element's supported roles
Always include a type attribute for button elements
Make elements with interactive roles and handlers focusable
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden)
Always include a lang attribute on the html element
Always include a title attribute for iframe elements
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress
Accompany onMouseOver/onMouseOut with onFocus/onBlur
Include caption tracks for audio and video elements
Use semantic elements instead of role attributes in JSX
Make sure all anchors are valid and navigable
Ensure all ARIA properties (aria-*) are valid
Use valid, non-abstract ARIA roles for elements with ARIA roles
Use valid AR...

Files:

  • packages/cli/vitest.config.ts
  • packages/cli/src/utils.ts
  • packages/cli/src/utils.test.ts
  • apps/web/utils/email/microsoft.ts
  • apps/web/utils/outlook/message.ts
  • packages/cli/src/main.ts
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (.cursor/rules/utilities.mdc)

**/*.{js,ts,jsx,tsx}: Use lodash utilities for common operations (arrays, objects, strings)
Import specific lodash functions to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

Files:

  • packages/cli/vitest.config.ts
  • packages/cli/src/utils.ts
  • packages/cli/src/utils.test.ts
  • apps/web/utils/email/microsoft.ts
  • apps/web/utils/outlook/message.ts
  • packages/cli/src/main.ts
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/testing.mdc)

**/*.test.{ts,tsx}: Use vitest for testing the application
Tests should be colocated next to the tested file with .test.ts or .test.tsx extension (e.g., dir/format.ts and dir/format.test.ts)
Mock server-only using vi.mock("server-only", () => ({}))
Mock Prisma using vi.mock("@/utils/prisma") and import the mock from @/utils/__mocks__/prisma
Use vi.clearAllMocks() in beforeEach to clean up mocks between tests
Each test should be independent
Use descriptive test names
Mock external dependencies in tests
Do not mock the Logger
Avoid testing implementation details
Use test helpers getEmail, getEmailAccount, and getRule from @/__tests__/helpers for mocking emails, accounts, and rules

Files:

  • packages/cli/src/utils.test.ts
**/*.{test,spec}.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/ultracite.mdc)

**/*.{test,spec}.{js,jsx,ts,tsx}: Don't nest describe() blocks too deeply in test files
Don't use callbacks in asynchronous tests and hooks
Don't have duplicate hooks in describe blocks
Don't use export or module.exports in test files
Don't use focused tests
Make sure the assertion function, like expect, is placed inside an it() function call
Don't use disabled tests

Files:

  • packages/cli/src/utils.test.ts
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/web/CLAUDE.md)

apps/web/**/*.{ts,tsx}: Use TypeScript with strict null checks
Use @/ path aliases for imports from project root
Use proper error handling with try/catch blocks
Format code with Prettier
Follow consistent naming conventions using PascalCase for components
Centralize shared types in dedicated type files

Import specific lodash functions rather than entire lodash library to minimize bundle size (e.g., import groupBy from 'lodash/groupBy')

Files:

  • apps/web/utils/email/microsoft.ts
  • apps/web/utils/outlook/message.ts
**/{server,api,actions,utils}/**/*.ts

📄 CodeRabbit inference engine (.cursor/rules/logging.mdc)

**/{server,api,actions,utils}/**/*.ts: Use createScopedLogger from "@/utils/logger" for logging in backend code
Add the createScopedLogger instantiation at the top of the file with an appropriate scope name
Use .with() method to attach context variables only within specific functions, not on global loggers
For large functions with reused variables, use createScopedLogger().with() to attach context once and reuse the logger without passing variables repeatedly

Files:

  • apps/web/utils/email/microsoft.ts
  • apps/web/utils/outlook/message.ts
**/package.json

📄 CodeRabbit inference engine (.cursor/rules/installing-packages.mdc)

Use pnpm as the package manager

Files:

  • packages/cli/package.json
🧠 Learnings (26)
📚 Learning: 2025-11-25T14:37:56.430Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-11-25T14:37:56.430Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Use vitest imports (`describe`, `expect`, `test`, `vi`, `beforeEach`) in LLM test files

Applied to files:

  • packages/cli/vitest.config.ts
  • packages/cli/src/utils.test.ts
  • packages/cli/package.json
📚 Learning: 2025-11-25T14:40:00.833Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-25T14:40:00.833Z
Learning: Applies to **/*.test.{ts,tsx} : Use `vitest` for testing the application

Applied to files:

  • packages/cli/vitest.config.ts
  • packages/cli/package.json
📚 Learning: 2025-11-25T14:36:18.416Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-11-25T14:36:18.416Z
Learning: Applies to apps/web/**/{.env.example,env.ts,turbo.json} : Add environment variables to `.env.example`, `env.ts`, and `turbo.json`

Applied to files:

  • packages/cli/src/utils.ts
  • packages/cli/src/main.ts
📚 Learning: 2025-11-25T14:36:45.807Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/environment-variables.mdc:0-0
Timestamp: 2025-11-25T14:36:45.807Z
Learning: Applies to apps/web/env.ts : Add client-side environment variables to `apps/web/env.ts` under the `client` object with `NEXT_PUBLIC_` prefix and Zod schema validation

Applied to files:

  • packages/cli/src/utils.ts
📚 Learning: 2025-11-25T14:36:45.807Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/environment-variables.mdc:0-0
Timestamp: 2025-11-25T14:36:45.807Z
Learning: Applies to apps/web/env.ts : Add client-side environment variables to `apps/web/env.ts` under the `experimental__runtimeEnv` object to enable runtime access

Applied to files:

  • packages/cli/src/utils.ts
📚 Learning: 2025-11-25T14:36:43.454Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/environment-variables.mdc:0-0
Timestamp: 2025-11-25T14:36:43.454Z
Learning: Applies to apps/web/env.ts : Define environment variables in `apps/web/env.ts` using Zod schema validation, organizing them into `server` and `client` sections

Applied to files:

  • packages/cli/src/utils.ts
📚 Learning: 2025-11-25T14:36:43.454Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/environment-variables.mdc:0-0
Timestamp: 2025-11-25T14:36:43.454Z
Learning: Applies to apps/web/env.ts : For client-side environment variables in `apps/web/env.ts`, prefix them with `NEXT_PUBLIC_` and add them to both the `client` and `experimental__runtimeEnv` sections

Applied to files:

  • packages/cli/src/utils.ts
📚 Learning: 2025-11-25T14:36:45.807Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/environment-variables.mdc:0-0
Timestamp: 2025-11-25T14:36:45.807Z
Learning: Applies to {.env.example,apps/web/env.ts} : Client-side environment variables must be prefixed with `NEXT_PUBLIC_`

Applied to files:

  • packages/cli/src/utils.ts
📚 Learning: 2025-11-25T14:36:45.807Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/environment-variables.mdc:0-0
Timestamp: 2025-11-25T14:36:45.807Z
Learning: Applies to apps/web/env.ts : Add server-only environment variables to `apps/web/env.ts` under the `server` object with Zod schema validation

Applied to files:

  • packages/cli/src/utils.ts
📚 Learning: 2025-11-25T14:42:08.869Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-11-25T14:42:08.869Z
Learning: Applies to **/*.{js,jsx,ts,tsx} : Don't hardcode sensitive data like API keys and tokens

Applied to files:

  • packages/cli/src/utils.ts
📚 Learning: 2025-11-25T14:36:45.807Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/environment-variables.mdc:0-0
Timestamp: 2025-11-25T14:36:45.807Z
Learning: Applies to turbo.json : Add new environment variables to `turbo.json` under `tasks.build.env` as a global dependency for the build task

Applied to files:

  • packages/cli/src/utils.ts
📚 Learning: 2025-11-25T14:36:18.416Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: apps/web/CLAUDE.md:0-0
Timestamp: 2025-11-25T14:36:18.416Z
Learning: Applies to apps/web/**/*NEXT_PUBLIC_* : Prefix client-side environment variables with `NEXT_PUBLIC_`

Applied to files:

  • packages/cli/src/utils.ts
📚 Learning: 2025-11-25T14:39:04.892Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security-audit.mdc:0-0
Timestamp: 2025-11-25T14:39:04.892Z
Learning: No hardcoded secrets in code; all secrets must be stored in environment variables (e.g., CRON_SECRET)

Applied to files:

  • packages/cli/src/utils.ts
📚 Learning: 2025-11-25T14:39:23.326Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/security.mdc:0-0
Timestamp: 2025-11-25T14:39:23.326Z
Learning: Applies to **/*.test.ts : Include security tests in test suites to verify: authentication is required, IDOR protection works (other users cannot access resources), parameter validation rejects invalid inputs, and error messages don't leak information

Applied to files:

  • packages/cli/src/utils.test.ts
📚 Learning: 2025-11-25T14:40:00.833Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-25T14:40:00.833Z
Learning: Applies to **/__tests__/**/*.{ts,tsx} : AI tests must be placed in the `__tests__` directory and are not run by default (they use a real LLM)

Applied to files:

  • packages/cli/src/utils.test.ts
📚 Learning: 2025-11-25T14:40:00.833Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-25T14:40:00.833Z
Learning: Applies to **/*.test.{ts,tsx} : Use descriptive test names

Applied to files:

  • packages/cli/src/utils.test.ts
📚 Learning: 2025-11-25T14:37:56.430Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-11-25T14:37:56.430Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Use `console.debug()` for outputting generated LLM content in tests, e.g., `console.debug("Generated content:\n", result.content);`

Applied to files:

  • packages/cli/src/utils.test.ts
📚 Learning: 2025-11-25T14:40:00.833Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-25T14:40:00.833Z
Learning: Applies to **/*.test.{ts,tsx} : Each test should be independent

Applied to files:

  • packages/cli/src/utils.test.ts
📚 Learning: 2025-11-25T14:37:56.430Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-11-25T14:37:56.430Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Prefer using existing helpers from `@/__tests__/helpers.ts` (`getEmailAccount`, `getEmail`, `getRule`, `getMockMessage`, `getMockExecutedRule`) instead of creating custom test data helpers

Applied to files:

  • packages/cli/src/utils.test.ts
📚 Learning: 2025-11-25T14:37:56.430Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/llm-test.mdc:0-0
Timestamp: 2025-11-25T14:37:56.430Z
Learning: Applies to apps/web/__tests__/**/*.test.ts : Use `describe.runIf(isAiTest)` with environment variable `RUN_AI_TESTS === "true"` to conditionally run LLM tests

Applied to files:

  • packages/cli/src/utils.test.ts
📚 Learning: 2025-11-25T14:40:00.833Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-25T14:40:00.833Z
Learning: Applies to **/*.test.{ts,tsx} : Use test helpers `getEmail`, `getEmailAccount`, and `getRule` from `@/__tests__/helpers` for mocking emails, accounts, and rules

Applied to files:

  • packages/cli/src/utils.test.ts
📚 Learning: 2025-11-25T14:40:00.833Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-11-25T14:40:00.833Z
Learning: Applies to **/*.test.{ts,tsx} : Avoid testing implementation details

Applied to files:

  • packages/cli/src/utils.test.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail thread operations from @/utils/gmail/thread.ts instead of direct API calls

Applied to files:

  • apps/web/utils/email/microsoft.ts
  • apps/web/utils/outlook/message.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail message operations (get, list, batch, etc.) from @/utils/gmail/message.ts instead of direct API calls

Applied to files:

  • apps/web/utils/email/microsoft.ts
  • apps/web/utils/outlook/message.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to apps/web/utils/gmail/**/*.{ts,tsx} : Keep Gmail provider-specific implementation details isolated within the apps/web/utils/gmail/ directory

Applied to files:

  • apps/web/utils/email/microsoft.ts
📚 Learning: 2025-11-25T14:37:22.660Z
Learnt from: CR
Repo: elie222/inbox-zero PR: 0
File: .cursor/rules/gmail-api.mdc:0-0
Timestamp: 2025-11-25T14:37:22.660Z
Learning: Applies to **/*.{ts,tsx} : Use wrapper functions for Gmail label operations from @/utils/gmail/label.ts instead of direct API calls

Applied to files:

  • apps/web/utils/email/microsoft.ts
🧬 Code graph analysis (3)
packages/cli/src/utils.ts (2)
apps/web/scripts/check-enum-imports.js (1)
  • content (74-74)
apps/web/env.ts (1)
  • env (17-244)
apps/web/utils/outlook/message.ts (1)
apps/web/utils/outlook/retry.ts (1)
  • withOutlookRetry (19-80)
packages/cli/src/main.ts (2)
apps/web/env.ts (1)
  • env (17-244)
packages/cli/src/utils.ts (1)
  • generateEnvFile (11-134)
🪛 ast-grep (0.40.0)
packages/cli/src/utils.ts

[warning] 25-25: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${key}=.*$, "m")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html

(regexp-from-variable)


[warning] 26-26: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^# ${key}=.*$, "m")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html

(regexp-from-variable)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: Review for correctness
  • GitHub Check: test
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (19)
version.txt (1)

1-1: LGTM!

Version bump to v2.21.29 aligns with the CLI fixes and test additions in this PR.

packages/cli/package.json (1)

16-27: LGTM!

Test scripts and Vitest dependency are properly configured. Based on learnings, using vitest for testing aligns with the project's testing standards.

packages/cli/vitest.config.ts (1)

1-13: LGTM!

Clean Vitest configuration. Single-threaded execution is appropriate for CLI tests that may interact with filesystem or environment variables.

packages/cli/src/utils.test.ts (2)

1-26: LGTM!

Well-structured tests for generateSecret covering length, format validation, and uniqueness. Good use of descriptive test names as per coding guidelines.


28-389: Comprehensive test coverage for generateEnvFile.

Excellent coverage of:

  • Value replacement in templates
  • Docker-specific configuration
  • Multiple LLM providers (Anthropic, OpenAI, Bedrock)
  • Commented line handling
  • Template structure preservation

Tests are independent and use descriptive names as per coding guidelines.

packages/cli/src/utils.ts (3)

6-9: LGTM!

Secure secret generation using node:crypto's randomBytes with proper hex encoding.


43-56: Verify quoted value handling consistency.

DATABASE_URL and UPSTASH_REDIS_URL are wrapped in quotes (lines 48-49, 53-54), while UPSTASH_REDIS_TOKEN and other values are not. This is intentional for URLs containing special characters, but ensure the template and downstream parsing handle this consistently.


113-131: LGTM!

Clean provider-specific API key handling with a well-structured mapping. The conditional logic correctly distinguishes Bedrock (multi-key) from other providers (single API key).

packages/cli/src/main.ts (5)

9-9: LGTM!

Clean import of utilities from the new utils module, properly separating concerns.


168-193: LGTM!

Good UX enhancement asking users whether the web app runs on host or in Docker, with clear hints for each option. Proper cancellation handling.


524-533: LGTM!

Correctly differentiates container hostnames (db, serverless-redis-http) when web runs in Docker vs localhost with exposed ports when running on host.


584-608: LGTM!

Template fetching with proper error handling and spinner feedback. The generateEnvFile integration cleanly delegates env content generation to the utility function.


909-928: LGTM!

Clean template loading with local file fallback for in-repo development and remote fetch for standalone usage. Proper error propagation.

apps/web/utils/outlook/message.ts (3)

163-174: LGTM - Correct URL-based pagination implementation.

The direct fetch approach for URL-based pageToken follows Microsoft Graph API best practices. Using the full @odata.nextLink URL preserves all query parameters and state.

One consideration: the folder filtering after fetch (lines 168-170) could return fewer results than expected per page when folderId is specified, since filtering happens client-side.


306-314: LGTM - Consistent URL-based pagination handling.

The implementation mirrors the approach in queryBatchMessages, maintaining consistency across both query functions. The retry wrapper is correctly applied.


263-264: Clarify the orderby mechanism for subsequent pages.

The comment "Only add orderby for first page to avoid sorting complexity errors" doesn't explain how this is enforced. In reality, subsequent pages use URL-based pagination (@odata.nextLink) which bypasses this code path entirely via the early return at line 30. The unconditional orderby is correct but would be clearer if the comment explained that URL-based pagination on subsequent pages avoids reapplying it.

apps/web/utils/email/microsoft.ts (3)

1012-1022: Good addition of local type for type safety.

The GraphMessage type provides better type safety for the Microsoft Graph response. This is a clean approach for handling the API response structure.


1029-1090: LGTM - Well-structured request construction.

The request construction logic is cleanly organized with:

  • Proper endpoint selection based on query type
  • Appropriate filter building for different scenarios
  • Conditional ordering to avoid complexity errors with fromEmail filter

The use of escapeODataString for user inputs provides proper protection against OData injection.


1187-1187: LGTM - Consistent nextPageToken handling.

Using the raw @odata.nextLink value aligns with the pagination changes in message.ts, ensuring consistent behavior across the codebase.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 9 files

Prompt for AI agents (all 2 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/cli/src/utils.test.ts">

<violation number="1" location="packages/cli/src/utils.test.ts:181">
P2: Test title claims to verify &quot;append values not found in template&quot; but has no assertion for `CUSTOM_VAR`. The comment suggests uncertainty about the expected behavior. Either add an assertion to verify `CUSTOM_VAR` is/isn&#39;t in the result, or rename the test to accurately reflect what it tests.</violation>
</file>

<file name="packages/cli/src/utils.ts">

<violation number="1" location="packages/cli/src/utils.ts:48">
P1: When `env.DATABASE_URL` is undefined, the template literal produces the string `&quot;undefined&quot;` (not the `undefined` value), bypassing `setValue`&#39;s undefined check and writing an invalid `DATABASE_URL=&quot;undefined&quot;` to the env file. Same issue affects lines 48, 52, 53.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

expect(result).not.toContain("# DATABASE_URL=");
});

it("should append values not found in template", () => {
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P2: Test title claims to verify "append values not found in template" but has no assertion for CUSTOM_VAR. The comment suggests uncertainty about the expected behavior. Either add an assertion to verify CUSTOM_VAR is/isn't in the result, or rename the test to accurately reflect what it tests.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/utils.test.ts, line 181:

<comment>Test title claims to verify &quot;append values not found in template&quot; but has no assertion for `CUSTOM_VAR`. The comment suggests uncertainty about the expected behavior. Either add an assertion to verify `CUSTOM_VAR` is/isn&#39;t in the result, or rename the test to accurately reflect what it tests.</comment>

<file context>
@@ -0,0 +1,390 @@
+    expect(result).not.toContain(&quot;# DATABASE_URL=&quot;);
+  });
+
+  it(&quot;should append values not found in template&quot;, () =&gt; {
+    const minimalTemplate = `# Minimal
+AUTH_SECRET=
</file context>

✅ Addressed in a1612d8

setValue("POSTGRES_USER", env.POSTGRES_USER);
setValue("POSTGRES_PASSWORD", env.POSTGRES_PASSWORD);
setValue("POSTGRES_DB", env.POSTGRES_DB);
setValue("DATABASE_URL", `"${env.DATABASE_URL}"`);
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P1: When env.DATABASE_URL is undefined, the template literal produces the string "undefined" (not the undefined value), bypassing setValue's undefined check and writing an invalid DATABASE_URL="undefined" to the env file. Same issue affects lines 48, 52, 53.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/utils.ts, line 48:

<comment>When `env.DATABASE_URL` is undefined, the template literal produces the string `&quot;undefined&quot;` (not the `undefined` value), bypassing `setValue`&#39;s undefined check and writing an invalid `DATABASE_URL=&quot;undefined&quot;` to the env file. Same issue affects lines 48, 52, 53.</comment>

<file context>
@@ -0,0 +1,134 @@
+    setValue(&quot;POSTGRES_USER&quot;, env.POSTGRES_USER);
+    setValue(&quot;POSTGRES_PASSWORD&quot;, env.POSTGRES_PASSWORD);
+    setValue(&quot;POSTGRES_DB&quot;, env.POSTGRES_DB);
+    setValue(&quot;DATABASE_URL&quot;, `&quot;${env.DATABASE_URL}&quot;`);
+    setValue(&quot;UPSTASH_REDIS_URL&quot;, `&quot;${env.UPSTASH_REDIS_URL}&quot;`);
+    setValue(&quot;UPSTASH_REDIS_TOKEN&quot;, env.UPSTASH_REDIS_TOKEN);
</file context>

✅ Addressed in a1612d8

@elie222 elie222 merged commit 7f0aed5 into main Dec 3, 2025
11 of 12 checks passed

// Helper to wrap a value in quotes if defined (prevents "undefined" string bug)
const wrapInQuotes = (value: string | undefined): string | undefined =>
value !== undefined ? `"${value}"` : undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

wrapInQuotes doesn’t escape " or \, so values with quotes/backslashes can produce invalid .env lines. Consider escaping backslashes first, then quotes, before wrapping.

-    value !== undefined ? `"${value}"` : undefined;
+    value !== undefined ? `"${value.replace(/\\/g, "\\\\").replace(/"/g, "\\\"")}"` : undefined;

🚀 Reply to ask Macroscope to explain or update this suggestion.

👍 Helpful? React to give us feedback.

];
for (const pattern of patterns) {
if (pattern.test(content)) {
content = content.replace(pattern, `${key}=${value}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

String.prototype.replace treats $ in the replacement string as special tokens, so values like $1/$& corrupt the output. Consider using the functional form of replace to ensure value is inserted verbatim.

Suggested change
content = content.replace(pattern, `${key}=${value}`);
content = content.replace(pattern, () => `${key}=${value}`);

🚀 Reply to ask Macroscope to explain or update this suggestion.

👍 Helpful? React to give us feedback.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 4 files (changes from recent commits).

Prompt for AI agents (all 1 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/cli/src/main.ts">

<violation number="1" location="packages/cli/src/main.ts:661">
P2: Missing `--env-file ${envFile}` flag for standalone installs. For consistency with the `runWebInDocker` case and to ensure Docker Compose can find environment variables (POSTGRES_PASSWORD, UPSTASH_REDIS_TOKEN, etc.) needed by database and Redis containers, the `--env-file` flag should be included.

(Based on your team&#39;s feedback about cross-service env var dependency.) [FEEDBACK_USED]</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

// Web app runs on host (pnpm dev or pnpm start)
const dockerStep = useDockerInfra
? "# Start Docker services (database & Redis):\ndocker compose --profile local-db --profile local-redis up -d\n\n"
? `# Start Docker services (database & Redis):\n${composeCmd} --profile local-db --profile local-redis up -d\n\n`
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 3, 2025

Choose a reason for hiding this comment

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

P2: Missing --env-file ${envFile} flag for standalone installs. For consistency with the runWebInDocker case and to ensure Docker Compose can find environment variables (POSTGRES_PASSWORD, UPSTASH_REDIS_TOKEN, etc.) needed by database and Redis containers, the --env-file flag should be included.

(Based on your team's feedback about cross-service env var dependency.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/main.ts, line 661:

<comment>Missing `--env-file ${envFile}` flag for standalone installs. For consistency with the `runWebInDocker` case and to ensure Docker Compose can find environment variables (POSTGRES_PASSWORD, UPSTASH_REDIS_TOKEN, etc.) needed by database and Redis containers, the `--env-file` flag should be included.

(Based on your team&#39;s feedback about cross-service env var dependency.) </comment>

<file context>
@@ -640,26 +640,25 @@ Full guide: https://docs.getinboxzero.com/self-hosting/microsoft-oauth`,
     // Web app runs on host (pnpm dev or pnpm start)
     const dockerStep = useDockerInfra
-      ? &quot;# Start Docker services (database &amp; Redis):\ndocker compose --profile local-db --profile local-redis up -d\n\n&quot;
+      ? `# Start Docker services (database &amp; Redis):\n${composeCmd} --profile local-db --profile local-redis up -d\n\n`
       : &quot;&quot;;
     const migrateCmd = isDevMode
</file context>
Suggested change
? `# Start Docker services (database & Redis):\n${composeCmd} --profile local-db --profile local-redis up -d\n\n`
? `# Start Docker services (database & Redis):\n${composeCmd} --env-file ${envFile} --profile local-db --profile local-redis up -d\n\n`
Fix with Cubic

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