feat(cli): Add integrations add and edit sub commads for postgres and mongo#287
feat(cli): Add integrations add and edit sub commads for postgres and mongo#287
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds interactive CLI commands for managing database integrations. New 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
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
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Codecov Report❌ Patch coverage is 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. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
packages/cli/src/commands/integrations/integrations-prompts/mongodb.ts
Outdated
Show resolved
Hide resolved
packages/cli/src/commands/integrations/integrations-prompts/mongodb.ts
Outdated
Show resolved
Hide resolved
…lution; remove unnecessary TypeScript error suppression in integration tests.
…er to V8 and adjust lint-staged commands
…s for @inquirer/testing; modify lint-staged commands to use nvm; remove inline dependency configuration from vitest setup.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
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 | 🔵 TrivialEmpty 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 | 🔵 TrivialAdd exhaustiveness check.
Switch handles only
pgsqlandmongodb. Aneverassertion 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 | 🟡 MinorPrompt exposes unsupported integration types.
Only
pgsqlandmongodbare 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.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
cspell.jsonpackage.jsonpackages/cli/package.jsonpackages/cli/src/cli.tspackages/cli/src/commands/integrations/add-integration.tspackages/cli/src/commands/integrations/edit-integration.test.tspackages/cli/src/commands/integrations/edit-integration.tspackages/cli/src/commands/integrations/integrations-prompts/mongodb.test.tspackages/cli/src/commands/integrations/integrations-prompts/mongodb.tspackages/cli/src/commands/integrations/integrations-prompts/pgsql.tspackages/cli/src/integrations/parse-integrations.tspackages/cli/src/skill/skill-files.tspackages/cli/src/utils/env-var-refs.test.tspackages/cli/src/utils/env-var-refs.tspackages/cli/src/utils/inquirer.tspackages/cli/tsdown.config.tspackages/reactivity/src/ast-analyzer.tspatches/@[email protected]tsconfig.json
packages/cli/src/commands/integrations/integrations-prompts/mongodb.ts
Outdated
Show resolved
Hide resolved
fc708a6
|
What's the reason to show unsupported integrations as selectable options when running the |
dinohamzic
left a comment
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
This should be a util at the cli level, there might already exist one that does the same thing.
Summary by CodeRabbit
Release Notes
New Features
integrations addcommand for creating new database integrations (PostgreSQL, MongoDB) with SSH tunnel and SSL certificate supportintegrations editcommand to modify existing integrations with the same advanced configuration options