feat(cli): Add integrations add and edit sub commads#287
Draft
feat(cli): Add integrations add and edit sub commads#287
Conversation
Contributor
|
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 to create and edit database integrations. It implements Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Prompts as Prompt Flow
participant Validator as Parser/Validator
participant FS as File System
participant Env as .env Handler
User->>CLI: Run 'integrations add'
CLI->>Prompts: Begin interactive flow
Prompts->>User: Select type, name, and type-specific fields
User-->>Prompts: Provide inputs (host, port, creds, SSH/SSL, etc.)
Prompts->>Validator: Validate collected config
Validator-->>Prompts: Validation result
Prompts->>FS: Read integrations YAML
FS-->>Prompts: Return existing document
Prompts->>Env: Extract secrets and write/update .env
Env-->>FS: .env persisted
Prompts->>FS: Merge and write updated integrations YAML
FS-->>CLI: Persist complete
CLI->>User: Success confirmation
sequenceDiagram
participant User
participant CLI
participant FS as File System
participant Validator as Parser/Validator
participant Prompts as Prompt Flow
participant Env as .env Handler
User->>CLI: Run 'integrations edit [id]'
CLI->>FS: Read integrations YAML
FS-->>CLI: YAML document
CLI->>Validator: Extract integration summaries
Validator-->>CLI: Summaries list
alt id provided
CLI->>Prompts: Load selected integration
else
CLI->>Prompts: Prompt user to select integration
Prompts->>User: Show list
User-->>Prompts: Select entry
end
Prompts->>Validator: Validate current config
Validator-->>Prompts: Current config OK
Prompts->>User: Prompt for edits (fields, SSH/SSL, secrets)
User-->>Prompts: Provide updates
Prompts->>Validator: Validate new config
Validator-->>Prompts: Validation result
Prompts->>Env: Update .env with changed secrets
Env-->>FS: .env persisted
Prompts->>FS: Update and write integrations YAML
FS-->>CLI: Persist complete
CLI->>User: Edit confirmation
🚥 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 82.34% 82.50% +0.16%
==========================================
Files 110 115 +5
Lines 6785 7083 +298
Branches 1817 1972 +155
==========================================
+ Hits 5587 5844 +257
- Misses 1198 1239 +41 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
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.
| "@deepnote/database-integrations": "workspace:*", | ||
| "@deepnote/reactivity": "workspace:*", | ||
| "@deepnote/runtime-core": "workspace:*", | ||
| "@inquirer/prompts": "^8.2.1", |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check npm registry for each package version
echo "=== `@inquirer/prompts` ==="
curl -s https://registry.npmjs.org/@inquirer/prompts | jq '.dist-tags, (.versions | keys | sort | .[-5:])' 2>/dev/null
echo -e "\n=== `@inquirer/testing` ==="
curl -s https://registry.npmjs.org/@inquirer/testing | jq '.dist-tags, (.versions | keys | sort | .[-5:])' 2>/dev/null
echo -e "\n=== `@types/mute-stream` ==="
curl -s https://registry.npmjs.org/@types/mute-stream | jq '.dist-tags, (.versions | keys | sort | .[-5:])' 2>/dev/null
echo -e "\n=== mute-stream ==="
curl -s https://registry.npmjs.org/mute-stream | jq '.dist-tags, (.versions | keys | sort | .[-5:])' 2>/dev/nullRepository: deepnote/deepnote
Length of output: 162
🌐 Web query:
@inquirer/prompts npm package version 8.2.1
💡 Result:
@inquirer/prompts@8.2.1 is a published npm release (listed as published by ssboudrias). (socket.dev)
Install it with:
npm i `@inquirer/prompts`@8.2.1Note: some indexes/CDNs may still show 8.2.0 (likely not updated yet). (jsdelivr.com)
Citations:
- 1: https://socket.dev/npm/category/user-interface/other-inputs?utm_source=openai
- 2: https://www.jsdelivr.com/package/npm/%40inquirer/prompts?utm_source=openai
🌐 Web query:
@inquirer/testing npm package version 3.1.1
💡 Result:
@inquirer/testing does not have a published npm version 3.1.1 (the available versions jump from 3.0.4 back through 3.0.x, then 2.1.x, etc.). The latest published version is 3.0.4. [1] [2]
If you meant @inquirer/prompts@3.1.1, that version does exist (different package name). [3]
Install latest @inquirer/testing:
npm i -D `@inquirer/testing`@3.0.4Install @inquirer/prompts@3.1.1 (if that’s what you intended):
npm i `@inquirer/prompts`@3.1.1Sources: [1] [2] [3]
🌐 Web query:
mute-stream npm package version 3.0.0
💡 Result:
mute-stream@3.0.0 (npm package)
- Release date: 2025-10-22 (tag
v3.0.0). [1] - Breaking change: “align to npm 11 node engine range” (PR
#94). [1] - Node.js engines:
^20.17.0 || >=22.9.0. [2] - Runtime dependencies: none. [3]
- License: ISC. [2][3]
Sources: [1] [2] [3]
🌐 Web query:
@types/mute-stream npm package version 0.0.4
💡 Result:
@types/mute-stream@0.0.4 is the TypeScript type definitions package for mute-stream (from DefinitelyTyped). It is also the latest published version, released November 7, 2023. [1] [2]
- Install:
npm i -D@types/mute-stream@0.0.4[1] - Dependency:
@types/node[1] - Contents:
index.d.ts(exportsclass MuteStream extends Duplexwithmute(),unmute(),isTTY, andOptions { replace?, prompt? }) plusREADME.md,LICENSE,package.json. [1] [3]
Sources: [1] Socket package page, [2] Yarn Classic package page (version/date), [3] UNPKG file listing.
Fix @inquirer/testing version: 3.1.1 does not exist.
Version 3.1.1 is not published. Latest available version is 3.0.4. The other three packages (@inquirer/prompts@8.2.1, mute-stream@3.0.0, @types/mute-stream@0.0.4) exist and are correct.
🤖 Prompt for AI Agents
In `@packages/cli/package.json` at 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".
Contributor
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.
Comment on lines
+212
to
+218
| } catch (error) { | ||
| if (error instanceof Error && error.name === 'ExitPromptError') { | ||
| program.error(chalk.yellow('Cancelled.'), { exitCode: ExitCode.Error }) | ||
| } | ||
| const message = error instanceof Error ? error.message : String(error) | ||
| program.error(chalk.red(message), { exitCode: ExitCode.Error }) | ||
| } |
Contributor
There was a problem hiding this comment.
Double error call for ExitPromptError.
The catch block calls program.error for ExitPromptError, but execution continues to the second program.error. Add return after the first call.
🐛 Fix control flow
} catch (error) {
if (error instanceof Error && error.name === 'ExitPromptError') {
program.error(chalk.yellow('Cancelled.'), { exitCode: ExitCode.Error })
+ return
}
const message = error instanceof Error ? error.message : String(error)
program.error(chalk.red(message), { exitCode: ExitCode.Error })
}🤖 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 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 }).
Contributor
Author
There was a problem hiding this comment.
program.error returns never
Contributor
There was a problem hiding this comment.
@tkislan You're right—since program.error returns never, the second call is unreachable. My review comment was incorrect. Thanks for clarifying.
| ? __dirname | ||
| : // @ts-expect-error: Safe ESM fallback; import.meta.url is only evaluated in ESM where __dirname is undefined. | ||
| path.dirname(fileURLToPath(import.meta.url)) | ||
| const _dirname = typeof __dirname !== 'undefined' ? __dirname : path.dirname(fileURLToPath(import.meta.url)) |
Contributor
There was a problem hiding this comment.
Unrelated formatting change in this PR.
This formatting adjustment appears unrelated to the CLI integrations feature. Consider excluding it or moving to a separate commit to keep the PR focused.
🤖 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, 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.
Contributor
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.
Contributor
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.
Comment on lines
26
to
28
| const [protocol, urlWithoutProtocol] = connectionString.split(/\/\/(.*)/) as [string, string] | ||
| const [credentials, restOfTheUrl] = urlWithoutProtocol.split(/@(.*)/) as [string, string] | ||
| const [username, password] = credentials.split(/:(.*)/) |
Contributor
There was a problem hiding this comment.
Type assertions assume well-formed input.
If connection string is malformed (e.g., missing @), the as [string, string] assertions will produce undefined values causing runtime errors.
🛡️ Add defensive checks
const [protocol, urlWithoutProtocol] = connectionString.split(/\/\/(.*)/) as [string, string]
+ if (!urlWithoutProtocol) {
+ return connectionString
+ }
const [credentials, restOfTheUrl] = urlWithoutProtocol.split(/@(.*)/) as [string, string]
+ if (!restOfTheUrl) {
+ return connectionString
+ }
const [username, password] = credentials.split(/:(.*)/)🤖 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/mongodb.ts`
around lines 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.
Comment on lines
63
to
65
Contributor
There was a problem hiding this comment.
Credentials require both user and password.
MongoDB supports authentication with username only (no password). Current logic omits credentials entirely if password is missing.
🔧 Support user-only auth
- if (user && password) {
- credentials = `${encodeURIComponent(user)}:${encodeURIComponent(password)}@`
+ if (user) {
+ credentials = password
+ ? `${encodeURIComponent(user)}:${encodeURIComponent(password)}@`
+ : `${encodeURIComponent(user)}@`
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (user && password) { | |
| credentials = `${encodeURIComponent(user)}:${encodeURIComponent(password)}@` | |
| } | |
| if (user) { | |
| credentials = password | |
| ? `${encodeURIComponent(user)}:${encodeURIComponent(password)}@` | |
| : `${encodeURIComponent(user)}@` | |
| } |
🤖 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/mongodb.ts`
around lines 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) + "@".
packages/cli/src/utils/inquirer.ts
Outdated
Comment on lines
174
to
185
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Return type boolean | undefined is misleading.
confirm() always returns a boolean. The undefined in the return type is unnecessary.
♻️ Simplify return type
export async function promptForOptionalBooleanField({
label,
defaultValue,
}: {
label: string
defaultValue?: boolean
-}): Promise<boolean | undefined> {
+}): Promise<boolean> {
return confirm({
message: label,
default: defaultValue,
})
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/utils/inquirer.ts` around lines 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).
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
integrations addandintegrations editfor creating and modifying database integrationsTests
Chores