Skip to content

feat(cli): Add integrations add and edit sub commads for postgres and mongo#287

Merged
tkislan merged 38 commits intomainfrom
tk/cli-integrations-add-edit
Feb 27, 2026
Merged

feat(cli): Add integrations add and edit sub commads for postgres and mongo#287
tkislan merged 38 commits intomainfrom
tk/cli-integrations-add-edit

Conversation

@tkislan
Copy link
Contributor

@tkislan tkislan commented Feb 16, 2026

Summary by CodeRabbit

Release Notes

New Features

  • Added interactive integrations add command for creating new database integrations (PostgreSQL, MongoDB) with SSH tunnel and SSL certificate support
  • Added interactive integrations edit command to modify existing integrations with the same advanced configuration options
  • Step-by-step prompts guide users through integration setup and credential configuration

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 16, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

This PR adds interactive CLI commands for managing database integrations. New integrations add and integrations edit subcommands enable users to create and modify PostgreSQL and MongoDB integrations via prompted workflows. Features include SSH tunnel and SSL/CA certificate configuration, environment variable references for secrets, YAML-based persistence, and comprehensive test coverage. Changes span CLI wiring, reusable prompt utilities, type-specific prompts, environment variable resolution, and configuration updates.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant CLI as CLI Handler
    participant Prompts as Prompt System
    participant YAML as Integrations File
    participant Env as .env File
    participant Validation as Validation

    User->>CLI: integrations add
    CLI->>Prompts: promptForIntegrationType()
    Prompts-->>User: Select type
    User-->>Prompts: Choose pgsql/mongodb
    Prompts-->>CLI: Type selected

    CLI->>Prompts: promptForIntegrationName()
    Prompts-->>User: Enter name
    User-->>Prompts: Provide name
    Prompts-->>CLI: Name provided

    CLI->>Prompts: promptForIntegrationConfig(type, name)
    alt PostgreSQL
        Prompts-->>User: Prompt host, port, database, user, password
        User-->>Prompts: Provide credentials
        Prompts-->>User: Enable SSH? Enable SSL?
        User-->>Prompts: Optional SSH/SSL details
    else MongoDB
        Prompts-->>User: Select connection mode (credentials/string)
        User-->>Prompts: Choose mode
        Prompts-->>User: Prompt connection details
        User-->>Prompts: Provide details
    end
    Prompts-->>CLI: Config assembled

    CLI->>YAML: Read/create integrations
    CLI->>Validation: Validate config
    Validation-->>CLI: Valid
    CLI->>Env: Store secrets
    Env-->>CLI: Secrets persisted
    CLI->>YAML: Write integration entry
    YAML-->>CLI: File updated
    CLI-->>User: Success
Loading
sequenceDiagram
    actor User
    participant CLI as CLI Handler
    participant Prompts as Prompt System
    participant YAML as Integrations File
    participant Env as .env File
    participant Parsing as Parser

    User->>CLI: integrations edit [id]
    CLI->>YAML: Read integrations
    YAML-->>CLI: Integration entries
    
    alt No ID provided
        CLI->>Prompts: promptSelectIntegration(summaries)
        Prompts-->>User: Show integration list
        User-->>Prompts: Select integration
        Prompts-->>CLI: Selected integration
    else ID provided
        CLI->>YAML: Find integration by ID
    end

    CLI->>Env: Load .env variables
    Env-->>CLI: Variables
    CLI->>Parsing: resolveEnvVarRefsFromMap(integration, envVars)
    Parsing-->>CLI: Resolved config

    CLI->>Prompts: promptForIntegrationConfig(currentConfig)
    Prompts-->>User: Show current values, allow edits
    User-->>Prompts: Update fields (name, connection, SSH/SSL)
    Prompts-->>CLI: New config

    CLI->>YAML: Update integration entry
    CLI->>Env: Update secrets in .env
    CLI->>YAML: Write updated file
    YAML-->>CLI: File persisted
    CLI-->>User: Integration updated
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.08% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Updates Docs ⚠️ Warning PR adds integrations add/edit subcommands but lacks documentation in packages/cli/README.md. Document new integrations add and integrations edit subcommands in packages/cli/README.md with examples and options.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately captures the main changes: adding 'integrations add' and 'edit' subcommands for PostgreSQL and MongoDB support to the CLI.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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

@codecov
Copy link

codecov bot commented Feb 16, 2026

Codecov Report

❌ Patch coverage is 85.26646% with 47 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.49%. Comparing base (154504e) to head (300f4e1).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
.../cli/src/commands/integrations/edit-integration.ts 81.31% 17 Missing ⚠️
packages/cli/src/utils/inquirer.ts 74.50% 13 Missing ⚠️
...mands/integrations/integrations-prompts/mongodb.ts 90.81% 9 Missing ⚠️
...s/cli/src/commands/integrations/add-integration.ts 82.22% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #287      +/-   ##
==========================================
+ Coverage   83.41%   83.49%   +0.08%     
==========================================
  Files         117      122       +5     
  Lines        7037     7345     +308     
  Branches     1939     1974      +35     
==========================================
+ Hits         5870     6133     +263     
- Misses       1167     1212      +45     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

🤖 Fix all issues with AI agents
In @.gitignore:
- Around line 1-1664: The .gitignore contains massive duplicate lines (e.g.
repeated "*.tsbuildinfo", ".DS_Store", ".env", ".npmrc", ".pnpm-store",
".qlty/cache", ".qlty/logs", ".qlty/out", ".qlty/plugin_cachedir",
".qlty/results", "coverage", "dist", "node_modules"); deduplicate so each ignore
pattern appears only once, group related entries (hidden files, env, package
manager stores, qlty folders, build/output folders) and ensure the file ends
with a single newline; update the .gitignore by removing the repeated lines and
leaving a clean list of unique patterns.

In `@packages/cli/package.json`:
- Line 49: The package.json currently references a non-existent version for the
dependency "@inquirer/testing"; update the version string for
"@inquirer/testing" in package.json to the published latest "3.0.4" (keeping the
other deps unchanged) so the dependency resolves correctly when installing;
specifically modify the "@inquirer/testing" entry in package.json to use
"3.0.4".

In `@packages/cli/src/commands/add-integration.ts`:
- Around line 6-15: The PromptContext interface is duplicated; extract it into a
shared type module (e.g., export interface PromptContext { input?:
NodeJS.ReadableStream; output?: NodeJS.WritableStream; clearPromptOnDone?:
boolean; signal?: AbortSignal }) and update both add-integration.ts and
edit-integration.ts to import { PromptContext } from that module instead of
declaring it locally; ensure the exported name matches callers and update any
local references to use the imported type so there is a single source of truth.
- Around line 174-180: In the catch block handling ExitPromptError inside the
add-integration command, add an immediate return after calling
program.error(chalk.yellow('Cancelled.'), { exitCode: ExitCode.Error }) so
execution doesn't fall through to the generic error handler; i.e., detect error
instanceof Error && error.name === 'ExitPromptError', call program.error(...)
and then return to avoid the subsequent program.error(chalk.red(...)) path.

In `@packages/cli/src/commands/edit-integration.test.ts`:
- Around line 17-55: The createPromptContext helper is duplicated; extract the
function (including its helpers typeText, pressEnter, typeAndEnter, pressKey,
tick and exposed inputStream/outputStream) into a shared test utility module
(e.g., tests/utils/promptContext.ts), export the helper, and replace the local
createPromptContext definitions in both edit-integration.test.ts and
add-integration.test.ts with imports from that module; ensure the returned
object shape and names remain identical so existing calls to
createPromptContext(), context.input/context.output, typeText, pressEnter,
typeAndEnter, pressKey, tick still work.

In `@packages/cli/src/commands/edit-integration.ts`:
- Around line 235-241: In the catch block for the error handling in
edit-integration.ts, ensure you stop execution after handling the specific
ExitPromptError so the generic error path isn't also executed; specifically,
inside the catch where you check `if (error instanceof Error && error.name ===
'ExitPromptError') { program.error(chalk.yellow('Cancelled.'), { exitCode:
ExitCode.Error }) }`, add an immediate return (or otherwise exit the function)
after calling `program.error` to prevent the subsequent lines that call
`program.error(chalk.red(...))` from running; update the catch surrounding code
that references `ExitPromptError`, `program.error`, and the surrounding function
to reflect this control-flow change.

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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/cli/src/commands/integrations/add-integration.test.ts`:
- Around line 170-184: The inline snapshot in add-integration.test.ts hardcodes
a branch-specific schema URL (refs/heads/tk/integrations-config-file-schema);
update the expected YAML in the test (the inline snapshot that sets the schema
URL) to point to a stable reference (e.g., use "main" or a versioned tag) and
then regenerate the snapshot so yamlContent's expectation matches the new URL;
locate the expectation around the test that asserts yamlContent (search for
yamlContent and the inline snapshot string) and replace the branch ref with the
chosen stable ref.

In `@packages/cli/src/commands/integrations/add-integration.ts`:
- Around line 28-37: promptForIntegrationType currently builds choices from
databaseIntegrationTypes but promptForIntegrationConfig only supports 'pgsql',
causing failures for other selections; fix by filtering databaseIntegrationTypes
to only supported types before building choices (e.g., restrict to ['pgsql'] or
a new supportedIntegrationTypes constant) so select<DatabaseIntegrationType>
only shows types that promptForIntegrationConfig can handle; update references
in promptForIntegrationType and any related callers to use the filtered list.
- Around line 81-89: The current logic overwrites doc.commentBefore and drops
existing header comments; instead when updating doc.commentBefore in the
add-integration flow (after
readIntegrationsDocument/createNewDocument/getOrCreateIntegrationsFromDocument/addIntegrationToSeq),
if doc.commentBefore is null set it to SCHEMA_COMMENT, otherwise if it does not
already include 'yaml-language-server' or SCHEMA_COMMENT prepend SCHEMA_COMMENT
plus a newline to the existing doc.commentBefore to preserve existing headers
and avoid duplication.

In `@packages/cli/src/commands/integrations/edit-integration.ts`:
- Around line 212-218: The catch block in edit-integration.ts currently calls
program.error twice when error is an ExitPromptError; update the catch in the
function containing this try/catch so that after handling the ExitPromptError
case (the if checking error instanceof Error && error.name === 'ExitPromptError'
and calling program.error(chalk.yellow('Cancelled.'), { exitCode: ExitCode.Error
})), you immediately return to prevent falling through to the subsequent
program.error call; ensure the rest of the catch still converts
non-ExitPromptError values to message and calls
program.error(chalk.red(message), { exitCode: ExitCode.Error }).
- Around line 110-122: The switch over existingConfig.type only handles 'pgsql'
and should be made exhaustive so new DatabaseIntegrationType members trigger
compile errors; update the switch in edit-integration.ts (the block using
existingConfig.type and calling promptForFieldsPostgres) to add a default branch
that narrows existingConfig.type to never (e.g., const _exhaustiveCheck: never =
existingConfig.type) and throw using that variable, ensuring the compiler
enforces handling every DatabaseIntegrationType variant at compile time.

In `@packages/cli/src/commands/integrations/integrations-prompts/pgsql.ts`:
- Around line 74-80: The metadata object currently unconditionally sets
sslEnabled plus caCertificateName and caCertificateText, causing empty strings
to be persisted; update the block that builds metadata (the variable named
metadata) so it always sets sslEnabled: true but only includes caCertificateName
and caCertificateText when they are non-empty (truthy), e.g. by conditionally
spreading those properties into metadata or checking their values before
assignment; ensure keys are omitted rather than set to empty strings so
downstream consumers see absence instead of empty values.

In `@packages/cli/src/integrations/parse-integrations.ts`:
- Around line 50-66: The loop in parseIntegrationsFile duplicates the validation
logic already in validateIntegrationEntry; replace that duplicated block by
calling validateIntegrationEntry(entry, index) for each item, then use the
returned EntryValidationResult to push either config (on success) or formatted
issues (on failure) into the same arrays parseIntegrationsFile currently
populates; preserve existing behavior around labeling/formatting of issues
(formatZodIssues usage is already handled by validateIntegrationEntry) and
ensure you import/keep the EntryValidationResult type so types still align with
the rest of parseIntegrationsFile.

In `@packages/cli/src/utils/inquirer.ts`:
- Around line 90-104: The promptForOptionalStringPortField currently applies
stringPortValidate even when the user submits an empty value; change the flow so
empty strings bypass port validation: either (A) update
promptForOptionalStringPortField to call promptForStringField without
customValidate when defaultValue or user input is an empty string, or (B) adjust
stringPortValidate to treat '' as valid (return true) when the surrounding
prompt is not required. Locate promptForOptionalStringPortField and
promptForStringField and implement the conditional skip or modify
stringPortValidate to accept empty input for optional fields so empty optional
responses no longer trigger "Invalid value, expected a number".

In `@packages/reactivity/src/ast-analyzer.ts`:
- Line 7: The change to the _dirname declaration in ast-analyzer.ts is an
unrelated formatting adjustment and should not be bundled with the CLI
integrations feature; revert the modification to the const _dirname line (the
const named _dirname in packages/reactivity/src/ast-analyzer.ts) or remove it
from this PR and place the formatting change in a separate commit/PR so the
feature PR remains focused.

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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/cli/src/commands/integrations/edit-integration.test.ts`:
- Around line 791-862: The edit flow is passing the literal "env:..." string
into parseMongoConnectionString so the URL parser returns {} and no defaults are
shown; fix editIntegration to resolve env-backed connection strings before
parsing by reading the provided envFile (same source used by updateDotEnv),
extracting the variable named after the "env:VAR_NAME" prefix, and passing the
resolved connection string value to parseMongoConnectionString instead of the
raw "env:..." token (or update parseMongoConnectionString to accept an env map
and resolve "env:" internally). Ensure this logic runs where
metadata.connection_string is read during the prompt flow so
Host/Port/Database/User defaults populate correctly.

In `@packages/cli/src/commands/integrations/edit-integration.ts`:
- Around line 167-177: existingConfig currently contains raw env-ref strings
(e.g., "env:VAR") and is passed to promptForIntegrationConfig, so resolve those
before prompting; after validating and assigning existingConfig from
existingConfigResult.data, call resolveEnvVarRefs(existingConfig) and use its
returned/resolved object as the argument to promptForIntegrationConfig (i.e.,
replace promptForIntegrationConfig(existingConfig) with
promptForIntegrationConfig(resolvedConfig)) so prompts receive concrete values
rather than literal env references.

---

Duplicate comments:
In `@packages/cli/src/commands/integrations/add-integration.test.ts`:
- Around line 170-184: The inline snapshot in add-integration.test.ts contains a
branch-specific schema URL (refs/heads/tk/integrations-config-file-schema) —
update the snapshot to reference a stable URL (e.g., refs/heads/main or a
versioned tag) so it won't break on merge; locate the failing inline snapshot
assertion around yamlContent in the test and replace the schema URL string in
the snapshot with the canonical main/tag URL (or refactor the test to compute
the schema URL from a shared constant like INTEGRATIONS_SCHEMA_URL and use that
in the snapshot).

In `@packages/cli/src/commands/integrations/add-integration.ts`:
- Around line 29-39: promptForIntegrationType currently shows all entries from
databaseIntegrationTypes including unsupported ones; update it to only present
implemented types (pgsql and mongodb). Modify the choices creation in
promptForIntegrationType to filter databaseIntegrationTypes for the allowed set
(e.g., const allowed = ['pgsql','mongodb'] or introduce
implementedIntegrationTypes) and then map to { name, value } from that filtered
list before calling select<DatabaseIntegrationType>({ ... }). Ensure the
function still returns Promise<DatabaseIntegrationType>.
- Around line 92-94: The code currently replaces any existing header comment via
"doc.commentBefore = SCHEMA_COMMENT" which overwrites user comments; instead,
preserve existing commentBefore by prepending SCHEMA_COMMENT when the
yaml-language-server marker isn't present: if doc.commentBefore is null set it
to SCHEMA_COMMENT, otherwise set doc.commentBefore = SCHEMA_COMMENT + "\n" +
doc.commentBefore so the schema comment is added above existing headers; keep
the existing conditional that checks for 'yaml-language-server'.

In `@packages/cli/src/commands/integrations/edit-integration.ts`:
- Around line 220-226: In the catch block that handles errors for the
edit-integration command, avoid falling through after handling ExitPromptError
by returning immediately: after detecting error instanceof Error && error.name
=== 'ExitPromptError' and calling program.error(chalk.yellow('Cancelled.'), {
exitCode: ExitCode.Error }), add a return (or make it an if/else) so the
subsequent program.error(chalk.red(...)) for generic errors is not invoked;
reference the catch block around ExitPromptError, program.error, and
ExitCode.Error to locate where to change control flow.

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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/cli/src/commands/integrations/add-integration.test.ts`:
- Line 631: The failing CSpell warning comes from the test string literal
"rawConnectionString:
env:AAAAAAAA_BBBB_CCCC_DDDD_EEEEEEEEEEEE__RAWCONNECTIONSTRING" in
add-integration.test.ts; either add RAWCONNECTIONSTRING to the cspell dictionary
(e.g., the project spell list) or change the env var name in the test to use
underscores like RAW_CONNECTION_STRING to satisfy the spell checker; update the
test string where the exact literal "rawConnectionString:
env:AAAAAAAA_BBBB_CCCC_DDDD_EEEEEEEEEEEE__RAWCONNECTIONSTRING" appears or add
the token RAWCONNECTIONSTRING to your cspell config so the pipeline stops
failing.

In `@packages/cli/src/commands/integrations/integrations-prompts/mongodb.ts`:
- Around line 63-65: The current conditional only sets credentials when both
user and password exist; change it to include credentials whenever user is
present (supporting user-only auth). Locate the credentials assignment (the
variable named credentials and the encodeURIComponent calls) and update the
logic so if user exists you build credentials as encodeURIComponent(user) plus
":" + encodeURIComponent(password) only when password is truthy, followed by
"@", otherwise just encodeURIComponent(user) + "@".
- Around line 26-28: The destructuring of connectionString into [protocol,
urlWithoutProtocol], [credentials, restOfTheUrl], and [username, password]
assumes a well-formed string and can yield undefined; update the parsing in the
function that uses connectionString so you validate each split result
(connectionString.split(/\/\/(.*)/), urlWithoutProtocol.split(/@(.*)/),
credentials.split(/:(.*)/)) before destructuring, and handle malformed cases by
throwing a clear error or returning a controlled failure value; specifically
check that urlWithoutProtocol, credentials, restOfTheUrl, username and password
are non-empty strings after each split, and fall back to a safe parsing approach
(e.g., regex.exec with null checks or use a proper URI parser) in the code paths
that reference connectionString, credentials, username, password.

In `@packages/cli/src/utils/inquirer.ts`:
- Around line 174-185: The function promptForOptionalBooleanField declares a
return type of Promise<boolean | undefined> while confirm(...) always returns a
boolean; update the function signature to return Promise<boolean> (remove |
undefined) and ensure the implementation still returns the confirm(...) call;
reference promptForOptionalBooleanField and confirm to locate and change the
type only (no runtime changes needed).

---

Duplicate comments:
In `@packages/cli/src/commands/integrations/add-integration.test.ts`:
- Around line 170-184: The test snapshot embeds a schema URL that points to the
feature branch ref `refs/heads/tk/integrations-config-file-schema`; update the
inline snapshot to use a stable reference (e.g. `main` or a versioned tag) so
the URL won't break after merge—locate the failing inline snapshot in
packages/cli/src/commands/integrations/add-integration.test.ts where the
variable yamlContent is asserted with toMatchInlineSnapshot and replace the
branch ref in the embedded schema URL with `main` or the appropriate release
tag.

In `@packages/cli/src/commands/integrations/edit-integration.ts`:
- Around line 243-249: The catch block handling ExitPromptError currently falls
through and calls the generic error handler; update the catch in
edit-integration.ts so that when error instanceof Error && error.name ===
'ExitPromptError' you call program.error(chalk.yellow('Cancelled.'), { exitCode:
ExitCode.Error }) and then immediately return (or otherwise stop further
execution) to avoid executing the subsequent generic program.error call;
reference the catch block around ExitPromptError, the ExitPromptError check,
program.error, and ExitCode.Error when making this change.
- Around line 112-131: The switch over existingConfig.type in edit-integration
should be made exhaustive: replace the current default that throws with a
never-assertion pattern so the compiler will error when new Integration types
are added; specifically, update the switch handling around existingConfig.type
(the case branches that call promptForFieldsPostgres and promptForFieldsMongodb)
and change the default to assert never (e.g., const _exhaustiveCheck: never =
existingConfig.type; throw new Error(...)) so missing cases are caught at
compile time while preserving the existing error message behavior.

@tkislan tkislan changed the title feat(cli): Add integrations add and edit sub commads feat(cli): Add integrations add and edit sub commads for postgres and mongo Feb 19, 2026
@tkislan tkislan marked this pull request as ready for review February 19, 2026 18:45
@tkislan tkislan requested a review from a team as a code owner February 19, 2026 18:45
@tkislan
Copy link
Contributor Author

tkislan commented Feb 23, 2026

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 23, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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

♻️ Duplicate comments (4)
packages/reactivity/src/ast-analyzer.ts (1)

7-7: Formatting-only change, logic intact.

Ternary is correct and readable. Scope concern already discussed in prior review.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/reactivity/src/ast-analyzer.ts` at line 7, No change required: keep
the existing ternary assignment for _dirname (const _dirname = typeof __dirname
!== 'undefined' ? __dirname : path.dirname(fileURLToPath(import.meta.url)))
as-is; the expression is correct and readable and there is no logic or scope fix
needed for the symbol _dirname.
packages/cli/src/commands/integrations/integrations-prompts/pgsql.ts (1)

74-80: 🧹 Nitpick | 🔵 Trivial

Empty SSL fields stored in metadata.

When SSL enabled but certificate fields skipped, empty strings persist. May cause downstream issues.

Conditionally include non-empty values
     metadata = {
       ...metadata,
       sslEnabled: true,
-      caCertificateName,
-      caCertificateText,
+      ...(caCertificateName && { caCertificateName }),
+      ...(caCertificateText && { caCertificateText }),
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/integrations/integrations-prompts/pgsql.ts` around
lines 74 - 80, The metadata block is currently adding caCertificateName and
caCertificateText even when they are empty strings; update the logic around
metadata assignment (the object merging that sets sslEnabled, caCertificateName,
caCertificateText) so that sslEnabled is still set to true but caCertificateName
and caCertificateText are only added to metadata when they contain non-empty
values (e.g., check each string and conditionally include them) to avoid
persisting empty fields.
packages/cli/src/commands/integrations/edit-integration.ts (1)

108-133: 🧹 Nitpick | 🔵 Trivial

Add exhaustiveness check.

Switch handles only pgsql and mongodb. A never assertion catches missing cases at compile time.

Exhaustive switch
     case 'mongodb':
       return promptForFieldsMongodb({
         id: existingConfig.id,
         type: existingConfig.type,
         name: newName,
         defaultValues: existingConfig.metadata,
       })
-    default:
-      throw new Error(
-        `Integration type "${existingConfig.type}" is not yet implemented. Only "pgsql" and "mongodb" are currently supported.`
-      )
+    default: {
+      const _exhaustive: never = existingConfig.type
+      throw new Error(
+        `Integration type "${_exhaustive}" is not yet implemented.`
+      )
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/integrations/edit-integration.ts` around lines 108
- 133, The switch in promptForIntegrationConfig only handles 'pgsql' and
'mongodb' so add an exhaustive check for existingConfig.type to catch unhandled
integration types at compile time: after the default case replace the generic
Error throw with an assertion that accepts a parameter typed as never (e.g.,
call an assertUnreachable function or inline a const unreachable: never =
existingConfig.type; throw new Error(...) using unreachable) so the compiler
will error if a new DatabaseIntegrationConfig.type variant is added; reference
promptForIntegrationConfig and existingConfig.type when making this change.
packages/cli/src/commands/integrations/add-integration.ts (1)

29-38: ⚠️ Potential issue | 🟡 Minor

Prompt exposes unsupported integration types.

Only pgsql and mongodb are handled (lines 65-74), but all types appear in the selection. Users selecting others hit an error.

Proposed fix
+const SUPPORTED_TYPES: DatabaseIntegrationType[] = ['pgsql', 'mongodb']
+
 export async function promptForIntegrationType(): Promise<DatabaseIntegrationType> {
-  const choices = databaseIntegrationTypes.map(type => ({
+  const choices = SUPPORTED_TYPES.map(type => ({
     name: type,
     value: type,
   }))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/integrations/add-integration.ts` around lines 29 -
38, The promptForIntegrationType function currently builds choices from
databaseIntegrationTypes (exposing unsupported types), but only 'pgsql' and
'mongodb' are handled later; fix by filtering the choices to only supported
types (e.g., const supported = ['pgsql','mongodb']) before mapping, or create a
supportedIntegrationTypes array and use that when building choices for select;
update the mapping that creates { name, value } to iterate over
supportedIntegrationTypes so the prompt cannot return an unsupported type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/cli/src/commands/integrations/integrations-prompts/mongodb.ts`:
- Around line 233-239: The metadata block currently always sets sslEnabled plus
caCertificateName and caCertificateText even when those values are empty; change
it so that when enabling SSL you set sslEnabled: true but only add
caCertificateName and caCertificateText to the metadata object if their values
are non-empty (e.g., after trimming). Update the code that builds metadata (the
section that currently spreads metadata and assigns sslEnabled,
caCertificateName, caCertificateText) to conditionally add those certificate
keys so empty strings are not saved.

In `@packages/cli/tsdown.config.ts`:
- Line 6: The TS build is failing because TypeScript doesn't recognize the ESM
__dirname name in the expression that computes currentDir; either re-add a
one-line suppression above that statement (e.g. `@ts-expect-error`) or add a
proper ambient declaration for __dirname in a d.ts file; update the code around
currentDir (the line using __dirname, path.dirname, fileURLToPath and
import.meta.url) to either keep the runtime typeof check and precede it with
`@ts-expect-error` to silence the compile error, or create/extend a global
declaration (declare const __dirname: string | undefined) and import that
declaration so TypeScript knows the symbol exists.

---

Duplicate comments:
In `@packages/cli/src/commands/integrations/add-integration.ts`:
- Around line 29-38: The promptForIntegrationType function currently builds
choices from databaseIntegrationTypes (exposing unsupported types), but only
'pgsql' and 'mongodb' are handled later; fix by filtering the choices to only
supported types (e.g., const supported = ['pgsql','mongodb']) before mapping, or
create a supportedIntegrationTypes array and use that when building choices for
select; update the mapping that creates { name, value } to iterate over
supportedIntegrationTypes so the prompt cannot return an unsupported type.

In `@packages/cli/src/commands/integrations/edit-integration.ts`:
- Around line 108-133: The switch in promptForIntegrationConfig only handles
'pgsql' and 'mongodb' so add an exhaustive check for existingConfig.type to
catch unhandled integration types at compile time: after the default case
replace the generic Error throw with an assertion that accepts a parameter typed
as never (e.g., call an assertUnreachable function or inline a const
unreachable: never = existingConfig.type; throw new Error(...) using
unreachable) so the compiler will error if a new DatabaseIntegrationConfig.type
variant is added; reference promptForIntegrationConfig and existingConfig.type
when making this change.

In `@packages/cli/src/commands/integrations/integrations-prompts/pgsql.ts`:
- Around line 74-80: The metadata block is currently adding caCertificateName
and caCertificateText even when they are empty strings; update the logic around
metadata assignment (the object merging that sets sslEnabled, caCertificateName,
caCertificateText) so that sslEnabled is still set to true but caCertificateName
and caCertificateText are only added to metadata when they contain non-empty
values (e.g., check each string and conditionally include them) to avoid
persisting empty fields.

In `@packages/reactivity/src/ast-analyzer.ts`:
- Line 7: No change required: keep the existing ternary assignment for _dirname
(const _dirname = typeof __dirname !== 'undefined' ? __dirname :
path.dirname(fileURLToPath(import.meta.url))) as-is; the expression is correct
and readable and there is no logic or scope fix needed for the symbol _dirname.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2412e9b and 1e859be.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (19)
  • cspell.json
  • package.json
  • packages/cli/package.json
  • packages/cli/src/cli.ts
  • packages/cli/src/commands/integrations/add-integration.ts
  • packages/cli/src/commands/integrations/edit-integration.test.ts
  • packages/cli/src/commands/integrations/edit-integration.ts
  • packages/cli/src/commands/integrations/integrations-prompts/mongodb.test.ts
  • packages/cli/src/commands/integrations/integrations-prompts/mongodb.ts
  • packages/cli/src/commands/integrations/integrations-prompts/pgsql.ts
  • packages/cli/src/integrations/parse-integrations.ts
  • packages/cli/src/skill/skill-files.ts
  • packages/cli/src/utils/env-var-refs.test.ts
  • packages/cli/src/utils/env-var-refs.ts
  • packages/cli/src/utils/inquirer.ts
  • packages/cli/tsdown.config.ts
  • packages/reactivity/src/ast-analyzer.ts
  • patches/@[email protected]
  • tsconfig.json

coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 23, 2026
Artmann
Artmann previously approved these changes Feb 24, 2026
@tkislan tkislan dismissed stale reviews from Artmann and coderabbitai[bot] via fc708a6 February 24, 2026 11:15
@tkislan tkislan requested review from Artmann and dinohamzic and removed request for Artmann February 24, 2026 13:51
@dinohamzic
Copy link
Contributor

What's the reason to show unsupported integrations as selectable options when running the add command? Should we only show supported options?

(base) ➜  deepnote git:(tk/cli-integrations-add-edit) pnpm --filter cli run dev integrations add

> @deepnote/[email protected] dev /Users/dino/Development/deepnote/packages/cli
> tsx src/bin.ts integrations add

✔ Select integration type: big-query
✔ Integration name: Deepnote BigQuery - production
Integration type "big-query" is not yet implemented. Only "pgsql" and "mongodb" are currently supported.
/Users/dino/Development/deepnote/packages/cli:
 ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL  @deepnote/[email protected] dev: `tsx src/bin.ts integrations add`
Exit status 1
(base) ➜  deepnote git:(tk/cli-integrations-add-edit)

Copy link
Contributor

@dinohamzic dinohamzic left a comment

Choose a reason for hiding this comment

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

Code is looking good, just left a minor nit.

I'm having difficulties with having the run command pick up integrations from .env and .env.yaml automatically, erroring out as:

(base) ➜  cli git:(tk/cli-integrations-add-edit) ✗ pnpm dev run Untitled-project-ra-9907.deepnote

> @deepnote/[email protected] dev /Users/dino/Development/deepnote/packages/cli
> tsx src/bin.ts run Untitled-project-ra-9907.deepnote

Parsing /Users/dino/Development/deepnote/packages/cli/Untitled-project-ra-9907.deepnote...
Missing database integration configuration.

The following SQL blocks require database integrations that are not configured:
  - 3e2bed0f-ebc3-40fb-bb45-205b7d45b3ec

Add the integration configuration to your integrations file.
See: https://docs.deepnote.com/integrations for integration configuration.
 ELIFECYCLE  Command failed with exit code 2.

export const MONGO_PREFIX = 'mongodb://'
export const SRV_PREFIX = 'mongodb+srv://'

export function safeUrlParse(url: string): URL | null {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a util at the cli level, there might already exist one that does the same thing.

@tkislan tkislan merged commit 4f5b20f into main Feb 27, 2026
19 of 21 checks passed
@tkislan tkislan deleted the tk/cli-integrations-add-edit branch February 27, 2026 11:48
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.

3 participants