feat(cli): layered DATABASE_URL resolution for db / schema commands#377
Open
feat(cli): layered DATABASE_URL resolution for db / schema commands#377
Conversation
Onboarding users running `stash db install` (or any of the six other
DB-touching commands) without an exported `DATABASE_URL` would hit a
cryptic "Invalid stash.config.ts: databaseUrl received undefined"
error from Zod. The CLI already loads `.env.local` / `.env`* via
dotenv, but had no story for `--database-url` flags, local Supabase,
or a pasted-once one-off value.
Adds `src/config/database-url.ts` exporting `resolveDatabaseUrl()` —
called at the top of every DB-touching command before
`loadStashConfig`. It walks sources in order:
1. `--database-url <url>` flag (explicit override)
2. `process.env.DATABASE_URL` (shell, mise, direnv, dotenv files)
3. `supabase status --output env` -> DB_URL, gated on `--supabase`
or a `supabase/config.toml` in cwd
4. Interactive `p.text` prompt (skipped under `CI=true` or
non-TTY stdin)
5. Hard fail with a source-naming error message
The resolved URL is mutated into `process.env.DATABASE_URL` (only
when env was unset/empty, or the source was an explicit flag) so the
existing scaffolded `process.env.DATABASE_URL!` reference in
`stash.config.ts` resolves transparently. The connection string is
never written to disk — `stash.config.ts` keeps its declarative env
reference. The source label is logged (`Using DATABASE_URL from
--database-url flag` / `from supabase status` / `from prompt`) but
the URL itself is never echoed.
Wired through `bin/stash.ts` argv (the parser already supports
`--key value`), HELP text updated. All seven DB-touching commands
(`db install`, `db push`, `db upgrade`, `db status`, `db validate`,
`db test-connection`, `schema build`) accept `--database-url` and
call the resolver before `loadStashConfig`.
After a prompt-sourced run, a `p.note` nudges the user to set
DATABASE_URL in the existing dotenv file (or `.env` if none exists)
so they don't get re-prompted next time.
Tests:
- 13 unit tests (`__tests__/database-url.test.ts`) covering each
source, malformed flag, empty-string-env fallthrough, supabase
fallthrough on missing binary / no-DB_URL output / no project,
prompt-cancel, CI guard, non-TTY guard, hint-file detection.
- 2 E2E tests (`tests/e2e/database-url.e2e.test.ts`) driving the
built binary through the pty harness: `--database-url` flag
surfaces the source label and proceeds to (failing) connection,
CI=true with no creds exits 1 with the CI message.
`init`/build-schema is intentionally NOT wired through the resolver:
it reads `process.env.DATABASE_URL` for provider hints pre-config and
should not trigger the prompt flow.
Note: stacks on top of #375 (the jiti default-export unwrap fix). The
resolver populates env, but `loadStashConfig` still needs the jiti
fix to read the resulting config correctly. Both PRs together
deliver the full onboarding experience.
|
📝 WalkthroughWalkthroughIntroduces a layered database URL resolver with a Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as CLI Command
participant Resolver as resolveDatabaseUrl
participant Flag as Flag Parser
participant Env as Environment
participant Supabase as Supabase Binary
participant Prompt as Interactive Prompt
participant Config as stash.config.ts
participant DB as Database Client
User->>CLI: stash db push --database-url postgresql://...
CLI->>Resolver: resolveDatabaseUrl({databaseUrlFlag})
Resolver->>Flag: Validate flag value
Flag-->>Resolver: Valid URL or error
alt Flag provided and valid
Resolver->>Env: Set process.env.DATABASE_URL
Resolver-->>CLI: Return {url, source: 'flag'}
else Flag invalid
Resolver-->>CLI: Log error, exit(1)
else No flag
Resolver->>Env: Check process.env.DATABASE_URL
alt Env exists and non-empty
Resolver-->>CLI: Return {url, source: 'env'}
else Env missing/empty
Resolver->>Supabase: Run supabase status --output env
alt Supabase success and DB_URL found
Resolver->>Env: Set process.env.DATABASE_URL
Resolver-->>CLI: Return {url, source: 'supabase-status'}
else Supabase fails or no DB_URL
Resolver->>Prompt: Check CI and TTY
alt Not CI and TTY available
Resolver->>Prompt: Show interactive prompt
User->>Prompt: Paste URL or cancel
alt User provides URL
Prompt-->>Resolver: URL
Resolver->>Env: Set process.env.DATABASE_URL
Resolver-->>CLI: Return {url, source: 'prompt'}
else User cancels
Resolver-->>CLI: Exit(0)
end
else CI=true or non-TTY
Resolver-->>CLI: Log error, exit(1)
end
end
end
end
CLI->>Config: Load stash.config.ts
Config->>DB: Connect using process.env.DATABASE_URL
DB-->>Config: Connection established
Config-->>CLI: Config loaded
CLI->>User: Execute command
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/commands/db/test-connection.ts (1)
48-54:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the failure hint to include the new URL sources.
After this change, connection failures can come from
--database-url, the env var, or Supabase discovery. Pointing users only atstash.config.ts/.envis now misleading.💡 Suggested fix
- p.log.info('Check your databaseUrl in stash.config.ts or .env file.') + p.log.info( + 'Check the resolved DATABASE_URL source (--database-url, .env, or Supabase discovery).', + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/db/test-connection.ts` around lines 48 - 54, Update the failure hint printed in test-connection.ts so it reflects all possible URL sources instead of only stash.config.ts/.env: modify the p.log.info call in the error branch (the block that constructs message and calls p.log.error) to mention --database-url, the environment variable, and Supabase discovery as places to check; locate the error-handling block that defines message and calls p.log.error / p.log.info and change the info text accordingly.
🤖 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/config/database-url.ts`:
- Around line 160-161: The CI detection currently only checks process.env.CI ===
'true', causing CI values like '1' or other truthy strings to be missed; update
the isCi computation (used alongside isInteractive) to treat common truthy CI
values case-insensitively (e.g., '1', 'true', 'yes') or simply check for a
non-empty process.env.CI value (e.g., Boolean(process.env.CI) or a regex match)
so that isInteractive (which uses process.stdin.isTTY && !isCi) behaves
correctly in CI environments.
---
Outside diff comments:
In `@packages/cli/src/commands/db/test-connection.ts`:
- Around line 48-54: Update the failure hint printed in test-connection.ts so it
reflects all possible URL sources instead of only stash.config.ts/.env: modify
the p.log.info call in the error branch (the block that constructs message and
calls p.log.error) to mention --database-url, the environment variable, and
Supabase discovery as places to check; locate the error-handling block that
defines message and calls p.log.error / p.log.info and change the info text
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7896bf05-8179-4618-9876-ae1f3978a98b
📒 Files selected for processing (12)
packages/cli/src/__tests__/database-url.test.tspackages/cli/src/bin/stash.tspackages/cli/src/commands/db/install.tspackages/cli/src/commands/db/push.tspackages/cli/src/commands/db/status.tspackages/cli/src/commands/db/test-connection.tspackages/cli/src/commands/db/upgrade.tspackages/cli/src/commands/db/validate.tspackages/cli/src/commands/schema/build.tspackages/cli/src/config/database-url.tspackages/cli/src/messages.tspackages/cli/tests/e2e/database-url.e2e.test.ts
Comment on lines
+160
to
+161
| const isCi = process.env.CI === 'true' | ||
| const isInteractive = Boolean(process.stdin.isTTY) && !isCi |
There was a problem hiding this comment.
Normalize CI detection beyond exact 'true'.
At Line 160, CI is treated as enabled only when process.env.CI === 'true'. Environments using values like CI=1 can incorrectly fall into interactive behavior on TTY runs.
Suggested patch
- const isCi = process.env.CI === 'true'
+ const ci = process.env.CI
+ const isCi = typeof ci === 'string' && /^(1|true)$/i.test(ci.trim())📝 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
| const isCi = process.env.CI === 'true' | |
| const isInteractive = Boolean(process.stdin.isTTY) && !isCi | |
| const ci = process.env.CI | |
| const isCi = typeof ci === 'string' && /^(1|true)$/i.test(ci.trim()) | |
| const isInteractive = Boolean(process.stdin.isTTY) && !isCi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/config/database-url.ts` around lines 160 - 161, The CI
detection currently only checks process.env.CI === 'true', causing CI values
like '1' or other truthy strings to be missed; update the isCi computation (used
alongside isInteractive) to treat common truthy CI values case-insensitively
(e.g., '1', 'true', 'yes') or simply check for a non-empty process.env.CI value
(e.g., Boolean(process.env.CI) or a regex match) so that isInteractive (which
uses process.stdin.isTTY && !isCi) behaves correctly in CI environments.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a layered DATABASE_URL resolver to improve onboarding for DB-touching CLI commands by supporting multiple sources (flag/env/Supabase/prompt) before stash.config.ts is loaded, plus tests to cover the new resolution paths.
Changes:
- Introduce
resolveDatabaseUrl()and wire it into DB/schema commands, including a new--database-urlflag. - Add user-facing message handles for resolver output and failure modes.
- Add unit + E2E coverage for resolver behavior (flag + CI guard).
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/cli/src/config/database-url.ts | New layered resolver for DATABASE_URL, including Supabase and prompt fallbacks. |
| packages/cli/src/bin/stash.ts | Plumbs --database-url into db/schema commands and updates help text. |
| packages/cli/src/messages.ts | Adds message handles for resolver logging/prompts/errors. |
| packages/cli/src/commands/db/install.ts | Runs resolver before config load; accepts databaseUrl option. |
| packages/cli/src/commands/db/upgrade.ts | Runs resolver before config load; accepts databaseUrl option. |
| packages/cli/src/commands/db/validate.ts | Runs resolver before config load; accepts databaseUrl option. |
| packages/cli/src/commands/db/status.ts | Runs resolver before config load; accepts databaseUrl option. |
| packages/cli/src/commands/db/test-connection.ts | Runs resolver before config load; accepts databaseUrl option. |
| packages/cli/src/commands/db/push.ts | Adds databaseUrl option and calls resolver before config load. |
| packages/cli/src/commands/schema/build.ts | Runs resolver before config load; accepts databaseUrl option. |
| packages/cli/src/tests/database-url.test.ts | New unit tests for resolver source ordering and guards. |
| packages/cli/tests/e2e/database-url.e2e.test.ts | New E2E tests validating flag path + CI fail-fast behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+39
to
+40
| await resolveDatabaseUrl({ databaseUrlFlag: options.databaseUrl }) | ||
|
|
There was a problem hiding this comment.
resolveDatabaseUrl is called here but not imported in this module, which will cause a build/TypeScript error at runtime/compile time. Add the missing import (consistent with the other db commands) or remove the call if intentional.
Suggested change
| await resolveDatabaseUrl({ databaseUrlFlag: options.databaseUrl }) |
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
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.
Closes #376. Stacks on top of #375 (jiti default-export unwrap fix) — both together deliver the full onboarding experience.
Problem
Running any DB-touching command without
DATABASE_URLexported in the shell or in a loaded.env*file fails with the cryptic Zod error:Users provide DB URLs through many channels:
--database-urlflag, shell exports,mise.toml,direnv,dotenv-cli, local Supabase, or simply "let me paste it once". The CLI handled only the dotenv path.Fix
New resolver module
src/config/database-url.tsexportingresolveDatabaseUrl(). Called at the top of every DB-touching command beforeloadStashConfig. Walks sources in order:--database-url <url>flag (explicit override)process.env.DATABASE_URL(shell, mise, direnv, dotenv files already loaded bybin/stash.ts)supabase status --output env→DB_URL, gated on--supabaseor asupabase/config.tomlin cwdp.textprompt — skipped underCI=trueor non-TTY stdinThe resolved URL is mutated into
process.env.DATABASE_URL(only when env was unset/empty, or the source was an explicit flag) so the existing scaffoldedprocess.env.DATABASE_URL!reference instash.config.tsresolves transparently. The connection string is never written to disk —stash.config.tskeeps its declarative env reference. The source is logged (Using DATABASE_URL from --database-url flag/from supabase status/from prompt); the URL itself is never echoed.After a prompt-sourced run, a
p.notenudges the user to setDATABASE_URLin their existing dotenv file (or.envif none exists) so they don't get re-prompted.Surface
--database-url <url>accepted on all seven DB-touching commands:db install,db push,db upgrade,db status,db validate,db test-connection,schema build. HELP text updated.init/build-schema is not wired through the resolver — it readsprocess.env.DATABASE_URLfor provider hints pre-config and should not trigger the prompt flow.Test plan
pnpm --filter @cipherstash/cli test— 89 pass (was 76; +13 from new__tests__/database-url.test.ts)pnpm --filter @cipherstash/cli test:e2e— 10 pass (was 8; +2 from newtests/e2e/database-url.e2e.test.ts: flag-source surfaces the label, CI guard exits 1 with the CI message)biome checkclean on changed filespnpm --filter @cipherstash/cli buildcleanNote on coverage
The unit tests use
vi.mockto stubnode:child_process,@clack/prompts, anddetectSupabaseProject, so each source path is exercised in isolation. The E2E tests run the builtdist/bin/stash.jsthrough the pty harness against a tempstash.config.tsfor the flag and CI-guard cases. The non-TTY path is covered by the unit suite (Object.defineProperty(process.stdin, 'isTTY', false)) since the pty harness always provides a TTY.Summary by CodeRabbit
Release Notes
New Features
--database-urlCLI flag to database commands (db install,db push,db upgrade,db validate,db test-connection, andschema build) for explicit database URL specification.Improvements